How to tell whether a GlobalValue is user-defined

Is there a way to distinguish between GlobalValues that are user-defined and those that are compiler-defined? I am looking for a function that I can use to tell if a GlobalValue is user-defined , something like “GlobalValue::isUserDefined”, which returns true for user-defined GlobalValue.

I’m trying to make changes to prevent llvm from placing user-defined constant arrays in the merge able constant sections. Currently, clang places 16-byte constant arrays that are marked “unnamed_addr” into __literal16 for macho (see following example).

$ cat test1.c

static const int s_dashArraysSize1[4] = {2, 2, 4, 6};

int foo1(int a) {

return s_dashArraysSize1[a];

}

$ clang test1.c -S -O3 -o - | tail -n 10

.section __TEXT,__literal16,16byte_literals

.align 4 ## @s_dashArraysSize1

_s_dashArraysSize1:

.long 2 ## 0x2

.long 2 ## 0x2

.long 4 ## 0x4

.long 6 ## 0x6

This is not desirable because macho linker wasn’t originally designed to handle user-defined symbols in those sections and having to handle them complicates the linker. Also, there is no benefit in doing so, since the linker currently doesn’t try to merge user-defined variables anyway.

I would also appreciate opinions on whether this issue is relevant for other platforms. In general, should we put user-defined symbols into literal sections?

In Akira’s example above, GlobalOpt is checking that the variable does not have its address taken and marking them it unnamed_addr. Even if that is a legal optimization, it may cause problems, e.g., for debugging, if the linker removes the symbol. I don’t know whether other linkers will keep all the symbols in literal sections or not. I think you could also make a reasonable argument that we don’t guarantee that the variable will remain visible when debugging optimized code.

What does "user-defined" means in here? Since the linker can is
involved, I assume it has something to do with the final symbol name.

At the linker level (symbol names, sections, atoms, relocations, etc),
what exactly that is not supported?

Cheers,
Rafael

The literalN sections were developed long ago to support coalescing of unnamed constants like 9.897 in source code for architectures that could not embed large constants in instructions. The linker could knew how to break up the section (e.g. __literal8 is always 8 byte chunks) and coalesce copies by content.

~6 years ago we discovered that gcc would sometimes put user named constants into the literal sections (e.g. const double foo 9.897). This was an issue because C language rules say &a != &b, but if ‘a’ and ‘b’ are the contain the same literal value from different translation units, the linker could merge them to the same address. For whatever reason, we could not fix gcc, so we changed to linker to never coalesce items in literal sections if there was a (non ‘L’ and non ‘l’) symbol on it.

The current state of LLVM is that is it going out of its way to move “named” constants from __const section to __literalN section. But the only possible advantage to doing that is that the hopes that the linker might coalesce it. But the linker won’t coalesce it because it is named. So, is there a way to keep the named values in the __const section?

-Nick

>> Is there a way to distinguish between GlobalValues that are
user-defined and
>> those that are compiler-defined? I am looking for a function that I can
use
>> to tell if a GlobalValue is user-defined , something like
>> "GlobalValue::isUserDefined", which returns true for user-defined
>> GlobalValue.
>>
>> I'm trying to make changes to prevent llvm from placing user-defined
>> constant arrays in the merge able constant sections. Currently, clang
places
>> 16-byte constant arrays that are marked "unnamed_addr" into __literal16
for
>> macho (see following example).
>>
>> $ cat test1.c
>>
>> static const int s_dashArraysSize1[4] = {2, 2, 4, 6};
>>
>>
>> int foo1(int a) {
>>
>> return s_dashArraysSize1[a];
>>
>> }
>>
>>
>> $ clang test1.c -S -O3 -o - | tail -n 10
>>
>> .section __TEXT,__literal16,16byte_literals
>>
>> .align 4 ## @s_dashArraysSize1
>>
>> _s_dashArraysSize1:
>>
>> .long 2 ## 0x2
>>
>> .long 2 ## 0x2
>>
>> .long 4 ## 0x4
>>
>> .long 6 ## 0x6
>>
>>
>>
>> This is not desirable because macho linker wasn't originally designed to
>> handle user-defined symbols in those sections and having to handle them
>> complicates the linker. Also, there is no benefit in doing so, since the
>> linker currently doesn't try to merge user-defined variables anyway.
>
> What does "user-defined" means in here? Since the linker can is
> involved, I assume it has something to do with the final symbol name.
>
> At the linker level (symbol names, sections, atoms, relocations, etc),
> what exactly that is not supported?

The literalN sections were developed long ago to support coalescing of
unnamed constants like 9.897 in source code for architectures that could
not embed large constants in instructions. The linker could knew how to
break up the section (e.g. __literal8 is always 8 byte chunks) and coalesce
copies by content.

~6 years ago we discovered that gcc would sometimes put user named
constants into the literal sections (e.g. const double foo 9.897). This
was an issue because C language rules say &a != &b, but if ‘a’ and ‘b’ are
the contain the same literal value from different translation units, the
linker could merge them to the same address. For whatever reason, we could
not fix gcc, so we changed to linker to never coalesce items in literal
sections if there was a (non ‘L’ and non ‘l’) symbol on it.

The current state of LLVM is that is it going out of its way to move
“named” constants from __const section to __literalN section. But the only
possible advantage to doing that is that the hopes that the linker might
coalesce it. But the linker won’t coalesce it because it is named. So, is
there a way to keep the named values in the __const section?

I believe the following patch would be the minimal needed to do this, there
is some dead code that could be removed as well.

diff --git a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 55e1756..bf78ce1 100644
--- a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -667,12 +667,6 @@
TargetLoweringObjectFileMachO::getSectionForConstant(SectionKind Kind,
   if (Kind.isDataRel() || Kind.isReadOnlyWithRel())
     return ConstDataSection;

- if (Kind.isMergeableConst4())
- return FourByteConstantSection;
- if (Kind.isMergeableConst8())
- return EightByteConstantSection;
- if (Kind.isMergeableConst16())
- return SixteenByteConstantSection;
   return ReadOnlySection; // .const
}

I think this is preventing constants in the constant pool (e.g., floating point literal) from being placed in the mergeable constant sections?

We want to keep the const arrays declared in the program (s_dashArraySize1) out of the mergeable constant sections, but don’t mind placing constants in the constant pool or constant arrays that the compiler defines, such as switch.table and memset_pattern, in the mergeable sections.

Ah, ok.

diff --git a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 55e1756..93f33af 100644
--- a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -626,15 +626,6 @@ SelectSectionForGlobal(const GlobalValue *GV,
SectionKind Kind,
           cast<GlobalVariable>(GV)) < 32)
     return UStringSection;

- if (Kind.isMergeableConst()) {
- if (Kind.isMergeableConst4())
- return FourByteConstantSection;
- if (Kind.isMergeableConst8())
- return EightByteConstantSection;
- if (Kind.isMergeableConst16())
- return SixteenByteConstantSection;
- }

The literalN sections were developed long ago to support coalescing of
unnamed constants like 9.897 in source code for architectures that could
not embed large constants in instructions. The linker could knew how to
break up the section (e.g. __literal8 is always 8 byte chunks) and coalesce
copies by content.

~6 years ago we discovered that gcc would sometimes put user named
constants into the literal sections (e.g. const double foo 9.897). This
was an issue because C language rules say &a != &b, but if ‘a’ and ‘b’ are
the contain the same literal value from different translation units, the
linker could merge them to the same address. For whatever reason, we could
not fix gcc, so we changed to linker to never coalesce items in literal
sections if there was a (non ‘L’ and non ‘l’) symbol on it.

Thanks for the info!

The current state of LLVM is that is it going out of its way to move
“named” constants from __const section to __literalN section. But the only
possible advantage to doing that is that the hopes that the linker might
coalesce it. But the linker won’t coalesce it because it is named. So, is
there a way to keep the named values in the __const section?

Right, LLVM has proven that the address of the data is insignificant, hence
it is "unnamed", and can be placed in a mergeable section. Is there any
reason not to change the linker to merge this stuff, if gcc is no longer
supported? We won't violate the semantics of C. Is there some immediate
problem with keeping the data in these sections?

I think this is preventing constants in the constant pool (e.g., floating
point literal) from being placed in the mergeable constant sections?

We want to keep the const arrays declared in the program
(s_dashArraySize1) out of the mergeable constant sections, but don't mind
placing constants in the constant pool or constant arrays that the
compiler defines, such as switch.table and memset_pattern, in the mergeable
sections.

Ah, ok.

diff --git a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
index 55e1756..93f33af 100644
--- a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -626,15 +626,6 @@ SelectSectionForGlobal(const GlobalValue *GV,
SectionKind Kind,
           cast<GlobalVariable>(GV)) < 32)
     return UStringSection;

- if (Kind.isMergeableConst()) {
- if (Kind.isMergeableConst4())
- return FourByteConstantSection;
- if (Kind.isMergeableConst8())
- return EightByteConstantSection;
- if (Kind.isMergeableConst16())
- return SixteenByteConstantSection;
- }
-
   // Otherwise, if it is readonly, but not something we can specially
optimize,
   // just drop it in .const.
   if (Kind.isReadOnly())

$ echo 'static const int s_dashArraysSize1[4] = {2, 2, 4, 6};

int foo1(int a) {
  return s_dashArraysSize1[a];
}' | ~/llvm/Debug+Asserts/bin/clang -x c -target i686-apple-darwin11 - -S
-o -

        .section __TEXT,__text,regular,pure_instructions
        .macosx_version_min 10, 7
        .globl _foo1
        .align 4, 0x90
_foo1: ## @foo1
## BB#0: ## %entry
        pushl %ebp
        movl %esp, %ebp
        pushl %eax
        calll L0$pb
L0$pb:
        popl %eax
        movl 8(%ebp), %ecx
        movl %ecx, -4(%ebp)
        movl -4(%ebp), %ecx
        movl _s_dashArraysSize1-L0$pb(%eax,%ecx,4), %eax
        addl $4, %esp
        popl %ebp
        retl

        .section __TEXT,__const
        .align 2 ## @s_dashArraysSize1
_s_dashArraysSize1:
        .long 2 ## 0x2
        .long 2 ## 0x2
        .long 4 ## 0x4
        .long 6 ## 0x6

Is this what you wanted?

This assembly output looks fine, but probably the change made in
SelectSectionForGlobal will place constants such as switch.table and
memset_pattern in __const too, which isn't what we want.

The literalN sections were developed long ago to support coalescing of
unnamed constants like 9.897 in source code for architectures that could not
embed large constants in instructions. The linker could knew how to break
up the section (e.g. __literal8 is always 8 byte chunks) and coalesce copies
by content.

~6 years ago we discovered that gcc would sometimes put user named
constants into the literal sections (e.g. const double foo 9.897). This was
an issue because C language rules say &a != &b, but if ‘a’ and ‘b’ are the
contain the same literal value from different translation units, the linker
could merge them to the same address. For whatever reason, we could not fix
gcc, so we changed to linker to never coalesce items in literal sections if
there was a (non ‘L’ and non ‘l’) symbol on it.

Thanks for the info!

+1

The current state of LLVM is that is it going out of its way to move
“named” constants from __const section to __literalN section. But the only
possible advantage to doing that is that the hopes that the linker might
coalesce it. But the linker won’t coalesce it because it is named. So, is
there a way to keep the named values in the __const section?

Right, LLVM has proven that the address of the data is insignificant, hence
it is "unnamed", and can be placed in a mergeable section. Is there any
reason not to change the linker to merge this stuff, if gcc is no longer
supported? We won't violate the semantics of C. Is there some immediate
problem with keeping the data in these sections?

Agreed. If ld64 can drop support for .o produced by the old gcc that
would be awesome. Failing that, what is really needed is

LLVM should only put constants in mergeable sections only if (among
other things) they require only symbols that start with 'l' or 'L'.

Correct?

Cheers,
Rafael

The literalN sections were developed long ago to support coalescing of
unnamed constants like 9.897 in source code for architectures that could not
embed large constants in instructions. The linker could knew how to break
up the section (e.g. __literal8 is always 8 byte chunks) and coalesce copies
by content.

~6 years ago we discovered that gcc would sometimes put user named
constants into the literal sections (e.g. const double foo 9.897). This was
an issue because C language rules say &a != &b, but if ‘a’ and ‘b’ are the
contain the same literal value from different translation units, the linker
could merge them to the same address. For whatever reason, we could not fix
gcc, so we changed to linker to never coalesce items in literal sections if
there was a (non ‘L’ and non ‘l’) symbol on it.

Thanks for the info!

+1

The current state of LLVM is that is it going out of its way to move
“named” constants from __const section to __literalN section. But the only
possible advantage to doing that is that the hopes that the linker might
coalesce it. But the linker won’t coalesce it because it is named. So, is
there a way to keep the named values in the __const section?

Right, LLVM has proven that the address of the data is insignificant, hence
it is "unnamed", and can be placed in a mergeable section. Is there any
reason not to change the linker to merge this stuff, if gcc is no longer
supported? We won't violate the semantics of C. Is there some immediate
problem with keeping the data in these sections?

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.

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()

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).

-Nick

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

t.patch (2.19 KB)

>> 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.

Rafael, would it be more correct to use hasLocalLinkage() instead of
hasPrivateLinkage() ?

Rafael, would it be more correct to use hasLocalLinkage() instead of
hasPrivateLinkage() ?

No, hasLocalLinkage covers InternalLinkage too. The difference from
private to internal is that internal shows up in the symbol table. For
private llvm mangles the name to add a L or l prefix which causes the
assembler (or linker) to omit it

Cheers,
Rafael

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.

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?

Correct.

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.

They should use private linkage then. It seems an unavoidable
consequence of the ld64 restriction. Since it requires mergeable
constants to start with 'l' or 'L', we have to use a linkage that
allows LLVM to remove the symbol from the symbol table. That linkage
is private.

Otherwise, the patch looks fine to me.

r216682.

Cheers,
Rafael