[FFmpeg-devel] [PATCH] Fix missing used attribute for inline assembly variables

[Teresa Johnson]

On Tue, Oct 31, 2017 at 5:42 PM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> Hi
>
> On Tue, Oct 31, 2017 at 04:29:18PM +0000, Teresa Johnson wrote:
> > It's needed for the same reason the used attribute was already added to
> the
> > "static const" variables. For those, when building with just -O2, they
> > could be removed by optimization since they had local (file) scope, and
> we
> > couldn't see the uses in the inline assembly (without the used
> attribute).
> > With ThinLTO we have whole program scope, so even though they are
> > non-static and have global scope we can do dead code elimination on them
> if
> > we don't see that they are used.
>
> currently we add "used" to DECLARE_ASM_CONST()
> which is specific to inline asm use.
>
> DECLARE_ALIGNED() is not specific to use in asm.
>
> For DECLARE_ASM_CONST() the "used" is unneeded only in the subset of
> inline asm cases where it is accessed through the asm operands like:
>     __asm__ volatile(
>         "movq          %0, %%mm7    \n\t"
>         "movq          %1, %%mm6    \n\t"
>         ::"m"(red_16mask),"m"(green_16mask));
>
> The compiler has full vissibility here of the access and if it removes
> it its a  compiler bug.
>
> this is compared to code like:
>  "pand "MANGLE(mask24l)", %%mm0\n\t"
>
> Here the compiler has no easy vissibility of the use, it is part of
> the asm string.
>
> and comparing this to DECLARE_ALIGNED(), the big difference is
> that DECLARE_ALIGNED() is used by plain C code which never should need
> "used". So adding "used" to DECLARE_ALIGNED() would add alot more
> "unused detection overriding" than what is needed
>

Perhaps then an additional macro is needed for variables that are currently
DECLARED_ALIGNED but used by MANGLE, which adds the used attribute. What do
you suggest?
Teresa


>
>
>
> >
> > Teresa
> >
> > On Tue, Oct 31, 2017 at 4:19 PM, Michael Niedermayer
> <michael at niedermayer.cc
> > > wrote:
> >
> > > On Tue, Oct 31, 2017 at 03:52:14PM +0000, Thomas Köppe wrote:
> > > > +Teresa, who drafted the patch initially.
> > > >
> > > > On 31 October 2017 at 15:38, Michael Niedermayer
> <michael at niedermayer.cc
> > > >
> > > > wrote:
> > > >
> > > > > On Tue, Oct 31, 2017 at 12:16:18PM +0000, Thomas Köppe wrote:
> > > > > > Variables used in inline assembly need to be marked with
> > > > > attribute((used)).
> > > > >
> > > > > This should not be the case.
> > > > > Variables referenced through in/out operands should not need that.
> > > > > Only ones accessed directly bypassing operands should need this
> > > > >
> > > >
> > > > I've added Teresa to the thread, who initially analyzed the problem.
> (For
> > > > background on ThinLTO, see here cppcon talk:
> > > > https://www.youtube.com/watch?v=p9nH2vZ2mNo.)
> > > >
> > > > [...]
> > > > > > -    #define DECLARE_ALIGNED(n,t,v)      t __attribute__
> ((aligned
> > > (n)))
> > > > > v
> > > > > > +    #define DECLARE_ALIGNED(n,t,v)      t av_used __attribute__
> > > > > ((aligned (n))) v
> > > > >
> > > > > which variables actually are affected/ need this ?
> > > > >
> > > >
> > > > Without this annotation, and when compiling and linking with
> ThinLTO, I
> > > get
> > > > an undefined reference to "ff_h264_cabac_tables", and also to
> > > > "ff_pw_96", "ff_pw_53",
> > > > "ff_pw_42", "ff_w1111" and many more.
> > >
> > > i guess these are all accessed directly, like through MANGLE()
> > >
> > >
> > > >
> > > >
> > > > > Marking all aligned variables as used makes it harder to spot
> unused
> > > > > variables leading to accumulation of cruft
> > > > >
> > > >
> > > > I see. Maybe we can annotate only the affected variables more
> granularly?
> > > > _______________________________________________
> > > > ffmpeg-devel mailing list
> > > > ffmpeg-devel at ffmpeg.org
> > > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > --
> > > Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC7
> 87040B0FAB
> > >
> > > Avoid a single point of failure, be that a person or equipment.
> > >
> >
> >
> >
> > --
> > Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> I am the wisest man alive, for I know one thing, and that is that I know
> nothing. -- Socrates
>



-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413