[FFmpeg-devel] [PATCH] DECLARE_ALIGNED
Pavel Pavlov
pavel
Thu Jun 4 21:03:06 CEST 2009
> -----Original Message-----
> We prefer patches made with svn diff from the root directory,
> so they can be applied with patch -p0 < blah. I for example
> keep one clean svn tree just for the purpose of creating
> diffs if they're part of some big changes, and it really
> helps us out in applying & reviewing patches.
>
> > Let me know if what I have sent is unusable so I wouldn't
> waste my and
> > others' time...
>
> Your patches are welcome, but they'll be more welcome after a
> few minor tweaks (it's just standard procedure around here).
Well, I have tortoise svn, and I also have two c:\FFMPEG and C:\-FFMPEG,
the second one is
unmodified version for creating the diffs and comparing my changes.
On windows I right click and from svn submenu there is "Create patch
option", that's what
I did.
Maybe you can manually modify diffs I sent so that they could be
applied?
From
Index: C:/FFMPEG/libswscale/swscale_internal.h
===================================================================
--- C:/FFMPEG/libswscale/swscale_internal.h (revision 29346)
+++ C:/FFMPEG/libswscale/swscale_internal.h (working copy)
To
Index: libswscale/swscale_internal.h
===================================================================
--- libswscale/swscale_internal.h (revision 29346)
+++ libswscale/swscale_internal.h (working copy)
Is that's all required?
> fdct_mmx.c: Who maintains it? What is up with the "one single
> array within a struct" mess, can we get rid of the struct
> somehow, otherwise this is quite unreadable IMO.
> idct_mmx.c: the \s at the end of the lines should stay aligned.
I also noticed that mess; what's the point to declare aligned struct,
and
then member of the struct?.. Maybe on some comilers generated code
wasn't
correct and that fixed the problem. If it is not a fix and isn't
required
then definetly that mess should be removed :)
> Well... That it doesn't change anything in that regard
> doesn't mean that it can't be wrong before and after.
> Also I only meant to explain the specific purpose of
> DECLARE_ASM_CONST, which maybe isn't be best-chose name.
>
> > Check all the diffs I sent. They are small and you can
> inspect all of
> > them in 5 minutes.
>
> Yes, they seem ok in that regard, those files don't really
> use MANGLE much.
I understand what's the meaning and reason for DECLARE_ASM_CONST
And as I understand DECLARE_ASM_CONST should be used only for
variables that are referenced as part of MANGLE'd inline asm code.
I can't confirm about gcc, but icl definetly has that problem with
standalone 64 bit static const variables - it doesn't try to use
it by referencing it, but it tries to use it as immidiate value
(copies it to stack and then referneces it through stack pointer).
I don't know if it whould actually be better in 64 bit code, but in
32-bit it's just a big preformance hit. And in most of the places it
wouldn't compile inline asm, because it required extra registers to
do the copying. I fixed it for inline asm, but all the places that
have 64 bit consts in c code will have that problem.
More information about the ffmpeg-devel
mailing list