I implemented this change to add a new macro. I tried to find the variables used in MANGLE and change those to use the new DECLARE_ASM_ALIGNED, and the build succeeds with these changes. New changes are in cl/172133815.
Teresa On Wed, Nov 1, 2017 at 7:25 AM, Teresa Johnson <tejohn...@google.com> wrote: > > > 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 <(408)%20460-2413> > -- 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
