[compiler-rt] r208603 - Move .subsections_via_symbols directives into DEFINE_COMPILERRT_PRIVATE_FUNCTION

[Reid Kleckner]

On Mon, May 12, 2014 at 4:44 PM, Nick Kledzik <kledzik at apple.com> wrote:

>
> On May 12, 2014, at 4:18 PM, Jonathan Roelofs <jonathan at codesourcery.com>
> wrote:
>
> >
> >
> > On 5/12/14, 3:47 PM, Nick Kledzik wrote:
> >> FYI, this is a semantic change.   The .subsection_via_symbols directive
> tells the linker that the .o file has no functions that will fall into the
> next one (the compiler never does that, but it is common enough in hand
> written assembly).  That means the linker can dead strip functions not
> referenced.  By moving .subsection_via_symbols into FILE_LEVEL_DIRECTIVE,
> you’ve caused every file to opt-in to the directive, even if they have fall
> through code.
> >>
> > I think the real gotcha here is that DEFINE_COMPILERRT_FUNCTION, which
> defines something that looks like function level scope had/has a global
> directive in it affecting file level scope…
> Yes!
>
> >> I just did a quick survey of the .S files in compiler-rt and do not see
> any with fall through code.  Mostly because compiler-rt has the convention
> of one function per file.
> > Does it really though? I didn't move .subsection_via_symbols into
> FILE_LEVEL_DIRECTIVE, it was already there. This only changes it for
> private functions, and further it makes those behave the same way as
> non-private ones, which already had it.
> Oh, sorry I was looking at the final result and did not realize it was
> half broke before your change.
>
> >>
> >> So, this change should be benign now, but may cause surprises on darwin
> in the future if someone tries to write fall-through assembly code.
> > So to eliminate this gotcha, it sounds like you are suggesting that I:
> >  a) Remove FILE_LEVEL_DIRECTIVE from DEFINE_COMPILERRT_FUNCTION and
> DEFINE_COMPILERRT_PRIVATE_FUNCTION
> >  b) Maybe also remove .subsections_via_symbols from FILE_LEVEL_DIRECTIVE
> >  c) Add .subsections_via_symbols to every *.S file (possibly inside some
> other appropriately named macro)
> >
> > I'm happy to do this, but I want to make sure I'm clear on what you want.
> That would be a clean solution, but it is a bunch of work to support a
> case we don’t have and probably don’t want.  A simpler solution would be:
> a) Remove use of FILE_LEVEL_DIRECTIVE from DEFINE_COMPILERRT_FUNCTION and
> DEFINE_COMPILERRT_PRIVATE_FUNCTION
> b) Remove definition of FILE_LEVEL_DIRECTIVE
> c) Having assembly.h out right use  .subsections_via_symbols , e.g.:
>
> #if defined(__APPLE__) && !defined(COMPILERRT_NON_STD_DARWIN)
>   /* Any assembly code that has multiple entry points needs to
>        #define COMPILERRT_NON_STD_DARWIN
>     before including assembly.h
>   */
>   .subsections_via_symbols
> #endif
>
> So, by default it assumes all code follows the darwin conventions (as it
> currently does), but if an issue arises, there is an easy way to disable
> .subsections_via_symbols on a file-by-file basis.


I'm not a compiler-rt person, but I don't think it's good design for
headers to expect includers to define macros before inclusion.  I think the
current situation is fine.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140512/57af328b/attachment.html>