Thank you, I'll package this up as a patch and send it as a separate mail. On 9 November 2017 at 11:52, Teresa Johnson <tejohn...@google.com> wrote:
> 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 <https://critique.corp.google.com/#review/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
