On Tue, Oct 31, 2017 at 5:42 PM, Michael Niedermayer <mich...@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 > <mich...@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 > <mich...@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@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 | tejohn...@google.com | > 408-460-2413 > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@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 | tejohn...@google.com | 408-460-2413 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
