[LLVMdev] How to tell whether a GlobalValue is user-defined

[Akira Hatanaka]

I noticed that some of the passes are creating GlobalVariables that should
be merged using InternalLinkage instead of PrivateLinkage. For example,
global array ".memset_pattern" created by LoopIdiomRecognize has
InternalLinkage. Since these constants won't be prefixed with "L" or "l",
will these constants be placed in section __const too? Your patch isn't
preventing any constants that could be merged before from being merged, but
it just seems to me that those constants should go into the mergeable
sections too, like "switch.table" does.

Otherwise, the patch looks fine to me.

On Wed, Aug 27, 2014 at 9:13 PM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> >> Agreed. If ld64 can drop support for .o produced by the old gcc that
> >> would be awesome. Failing that, what is really needed is
> > Because of static archives, the linker has to support old .o files for
> quite a while.   I also don’t know when clang starting getting this right.
>
> r123585 (Jan 16 17:19:34 2011) I think.
>
> > Also, this seems like linker complexity for a very unlikely optimization
> (two named constants with same value).  If someone cares about this level
> of optimization, they can use LTO which will do all the constant merging in
> LLVM.
> >
> >>
> >> LLVM should only put constants in mergeable sections only if (among
> >> other things) they require only symbols that start with 'l' or 'L'.
> > Not sure what you mean here. What is "requiring”?  Are we talking about
> this code in TargetLoweringObjectFileMachO::SelectSectionForGlobal()
>
> I mean "the correspoinding symbol name will start with".
>
> >  if (Kind.isMergeableConst()) {
> >     if (Kind.isMergeableConst4())
> >       return FourByteConstantSection;
> >     if (Kind.isMergeableConst8())
> >       return EightByteConstantSection;
> >     if (Kind.isMergeableConst16())
> >       return SixteenByteConstantSection;
> >   }
> >
> > Can’t we just change the first ‘if’ to:
> >
> >  if (Kind.isMergeableConst() && !GV.hasName()) {
> >
> > That should leave any “named” constants in the __const section instead
> of moving them to the __literal section. (Though I don’t actually know if
> anonymous constants were given some name earlier so hasName() is useless at
> this point).
>
> That seems too strict. A private GV can have a name, but it will be
> printed with a 'L' or 'l' prefix, so it should not be a problem.
>
> In other words, it looks like you want something like the attached patch.
>
> Cheers,
> Rafael
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140828/2acbf739/attachment.html>