[AsmPrinter][ELF] Define local aliases (.Lfoo$local) for GlobalObjects

[MaskRay]

Summary

For MC_GlobalAddress operands referencing certain GlobalObjects,
we can lower them to STB_LOCAL aliases to avoid costs brought by
assembler/linker's conservative decisions about symbol interposition:

• An assembler conservatively assumes a global default visibility symbol interposable (ELF semantics). So relocations in object files are needed even if the code generator assumed the definition exact and non-interposable.
• The relocations can cause the creation of PLT entries on some targets for -shared links. A linker conservatively assumes a global default visibility symbol interposable (if not otherwise constrained by -Bsymbolic/--dynamic-list/VER_NDX_LOCAL/etc).

"certain" refers to GlobalObjects in the intersection of
hasExactDefinition() and !isInterposable(): external, appending, internal, private.
Local linkages (internal and private) cannot be interposed. appending is for very
few objects LLVM interpret specially. So the set just includes external.

This patch emits STB_LOCAL aliases (.Lfoo$local) for such GlobalObjects, so that targets can lower
MC_GlobalAddress operands to STB_LOCAL aliases if applicable.
We may extend the scope and include GlobalAlias in the future.

LLVM's existing -fno-semantic-interposition behaviors give us license to do such optimizations:

• Various optimizations (ipconstprop, inliner, sccp, sroa, etc) treat normal ExternalLinkage GlobalObjects as non-interposable.
• Before D72197, MC resolved a PC-relative VK_None fixup to a non-local symbol at assembly time (no outstanding relocation), if the target is defined in the same section. Put it simply, even if IR optimizations failed to optimize and allowed interposition for the function call in void foo() {} void bar() { foo(); }, the assembler would disallow it.

This patch sets up AsmPrinter infrastructure to make -fno-semantic-interposition more so.
With and without the patch, the object file output should be identical:
.Lfoo$local does not take a symbol table entry.

[merge_guards_bot]

Unit tests: pass. 62112 tests passed, 0 failed and 808 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[MaskRay]

Ping D73228 and D73230 (tested on a large code base).

[sfertile]

Patch makes sense to me.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

454

I think you explained why we only are doing this for ExternalLinkage well in the patch description, do you think the reasoning should be here as a comment?

456

Ideally GV.isDSOLocal() should fully determine if a symbol is replaceable at runtime or not. Not sure if its safer to use TargetMachine::shouldAssumeDSOLocal instead. I believe the only difference right now for ELF would be the copy relocation case.

That said, this conditional is only true if the symbol is not preemptable. IIUC the compiler shouldn't be accessing data GOT-indirect, and the linker won't be emitting PLTs for the functions. Which means:

costs brought by potential symbol interpositionloc
(relocations in object files, PLT/GOT in executables/shared objects).

There shouldn't be any saving related to PLT/GOT. I'm not very familiar with linking on archs other then Power though so I could definitely be wrong here.

[peter.smith]

To make sure I understand, this is adding a local alias as at assembly time that can be used as a fixup target. This is necessary as at assembly time all we have is a global default visibility symbol which we have to assume can be interposed, even if the code-generator has already assumed it. Are there other uses that I'm missing?
As it stands this sounds reasonable to me, I'm trying to think if there are any other alternatives and what the trade-offs would be. For example is there no way that the assembler could find out the information at assembly time, or is that more complex than adding local aliases?

llvm/test/CodeGen/AMDGPU/r600-constant-array-fixup.ll

4 ↗

(On Diff #239696)

As I understand it, this patch should just add local aliases for some known DSO local global symbols. This looks to have changed the relocation target and the value of arr. Intuitively I wouldn't have expected this from adding the aliases alone. There must be something I'm missing either in the patch, or this could be something to do with the AMDGPU backend. I'd be grateful if you could explain the difference?

[MaskRay]

Improve comment for AsmPrinter::getSymbolPreferLocal()

[MaskRay]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

456

It is not convenient to access Module in this function. We probably should improve GlobalVariable::maybeSetDsoLocal to take account of TargetMachine::shouldAssumeDSOLocal, so that isDSOLocal() and TargetMachine::shouldAssumeDSOLocal will return the same value.

We require a definition (!GV.isDeclaration()). So for copy relocation, e.g.

// x86 x86-64
// or aarch64 -mpie-copy-relocations
extern int var;
int foo() { return var; }

We don't use a local alias (no issue here).

[MaskRay]

llvm/test/CodeGen/AMDGPU/r600-constant-array-fixup.ll

4 ↗

(On Diff #239696)

After rebase, this change goes away.

It seemed an issue fixed by @arsenm in rG7dc49f77ee508b4152f9291c8e804e4eda3653d3 . It does look weird to me: adding a new symbol somehow alters relocations.

[MaskRay]

In D73228#1845102, @peter.smith wrote:

To make sure I understand, this is adding a local alias as at assembly time that can be used as a fixup target. This is necessary as at assembly time all we have is a global default visibility symbol which we have to assume can be interposed, even if the code-generator has already assumed it. Are there other uses that I'm missing?

Yes. STV_DEFAULT STB_GLOBAL preemptivity depends on whether the link uses -no-pie/-pie (executable) or -shared (shared object). Referencing a local alias makes -shared similar to the executable case.

(STV_PROTECTED/STV_INTERNAL/STV_HIDDEN) STB_GLOBAL does not need a local alias. But for simplicity, leave it to the linker.

As it stands this sounds reasonable to me, I'm trying to think if there are any other alternatives and what the trade-offs would be. For example is there no way that the assembler could find out the information at assembly time, or is that more complex than adding local aliases?

The downside is that textual assembly output will be slightly larger, but it is acceptable. Object assembly output can only be smaller because of elimination of certain relocations. .L symbols do not take space.

An alternative is to make the assembler aware of executable/shared use cases. For example, gas RISC-V has two directives .option nopic and .option pic. I believe that is a wrong direction.

The compiler has had to know the eventual use case (-fno-pic / -fpie / -fpic). The linker has to know the use case (-no-pie / -pie / -shared). The combinations already provide sufficient flexibility that makes a mode switch on the assembler side unnecessary.

[merge_guards_bot]

Unit tests: pass. 62275 tests passed, 0 failed and 827 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[peter.smith]

Thanks for the update, I agree that informing the assembler of pic status isn't the right way to go, particularly if the assembler can't check that the code that you have written is actually pic and also the potentially incompatibility with GNU as. I've no objections.

[sfertile]

LGTM.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

456

Good point, thanks.

[MaskRay]

Update description

[merge_guards_bot]

Unit tests: pass. 62303 tests passed, 0 failed and 838 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[aymanmus]

Hi @MaskRay,

This change has broke the update_llc_test_checks.py tool's functionality for any test that includes a local function.
The tool now removes all "; CHECK" lines from such tests and emits nothing instead.
You can check this by adding "dso_local" attribute to any of the X86 existing tests (for example /llvm/CodeGen/X86/add-i64.ll) and run "update_llc_test_checks.py on it.

I think that it's crucial to fix this (hint: the regex strings defined at the beginning of llvm-org/llvm/utils/UpdateTestChecks/asm.py don't capture these test cases anymore).

Thanks,
Ayman

llvm/include/llvm/CodeGen/AsmPrinter.h

public:
   /// Return the current section we are emitting to.
   const MCSection *getCurrentSection() const;
 
   void getNameWithPrefix(SmallVectorImpl<char> &Name,
                          const GlobalValue *GV) const;
 
   MCSymbol *getSymbol(const GlobalValue *GV) const;
 
+  /// Similar to getSymbol() but preferred for references. On ELF, this uses a
+  /// local symbol if a reference to GV is guaranteed to be resolved to the
+  /// definition in the same module.
+  MCSymbol *getSymbolPreferLocal(const GlobalValue &GV) const;
+
   //===------------------------------------------------------------------===//
   // XRay instrumentation implementation.
   //===------------------------------------------------------------------===//
 public:
   // This describes the kind of sled we're storing in the XRay table.
   enum class SledKind : uint8_t {
     FUNCTION_ENTER = 0,
     FUNCTION_EXIT = 1,

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

void AsmPrinter::getNameWithPrefix(SmallVectorImpl<char> &Name,
                                    const GlobalValue *GV) const {
   TM.getNameWithPrefix(Name, GV, getObjFileLowering().getMangler());
 }
 
 MCSymbol *AsmPrinter::getSymbol(const GlobalValue *GV) const {
   return TM.getSymbol(GV);
 }
 
+MCSymbol *AsmPrinter::getSymbolPreferLocal(const GlobalValue &GV) const {
+  // On ELF, use .Lfoo$local if GV is a non-interposable GlobalObject with an
+  // exact definion (intersection of GlobalValue::hasExactDefinition() and
+  // !isInterposable()). These linkages include: external, appending, internal,
+  // private. It may be profitable to use a local alias for external. The
+  // assembler would otherwise be conservative and assume a global default
+  // visibility symbol can be interposable, even if the code generator already
+  // assumed it.
+  if (TM.getTargetTriple().isOSBinFormatELF() &&
+      GlobalObject::isExternalLinkage(GV.getLinkage()) && GV.isDSOLocal() &&
+      !GV.isDeclaration() && isa<GlobalObject>(GV))
+    return getSymbolWithGlobalValueBase(&GV, "$local");
+  return TM.getSymbol(&GV);
+}
+
 /// EmitGlobalVariable - Emit the specified global variable to the .s file.
 void AsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
   bool IsEmuTLSVar = TM.useEmulatedTLS() && GV->isThreadLocal();
   assert(!(IsEmuTLSVar && GV->hasCommonLinkage()) &&
          "No emulated TLS variables in the common section");
 
   // Never emit TLS variable xyz in emulated TLS model.
   // The initialization value is in __emutls_t.xyz instead of xyz.
void AsmPrinter::EmitGlobalVariable(const GlobalVariable *GV) {
   MCSymbol *EmittedInitSym = GVSym;
 
   OutStreamer->SwitchSection(TheSection);
 
   EmitLinkage(GV, EmittedInitSym);
   EmitAlignment(Alignment, GV);
 
   OutStreamer->EmitLabel(EmittedInitSym);
+  MCSymbol *LocalAlias = getSymbolPreferLocal(*GV);
+  if (LocalAlias != EmittedInitSym)
+    OutStreamer->EmitLabel(LocalAlias);
 
   EmitGlobalConstant(GV->getParent()->getDataLayout(), GV->getInitializer());
 
   if (MAI->hasDotTypeDotSizeDirective())
     // .size foo, 42
     OutStreamer->emitELFSize(EmittedInitSym,
                              MCConstantExpr::create(Size, OutContext));
 
void AsmPrinter::EmitFunctionEntryLabel() {
   // conflicting due to asm renaming.  Detect this and emit an error.
   if (CurrentFnSym->isVariable())
     report_fatal_error("'" + Twine(CurrentFnSym->getName()) +
                        "' is a protected alias");
   if (CurrentFnSym->isDefined())
     report_fatal_error("'" + Twine(CurrentFnSym->getName()) +
                        "' label emitted multiple times to assembly file");
 
-  return OutStreamer->EmitLabel(CurrentFnSym);
+  OutStreamer->EmitLabel(CurrentFnSym);
+
+  if (TM.getTargetTriple().isOSBinFormatELF()) {
+    MCSymbol *Sym = getSymbolPreferLocal(MF->getFunction());
+    if (Sym != CurrentFnSym)
+      OutStreamer->EmitLabel(Sym);
+  }
 }
 
 /// emitComments - Pretty-print comments for instructions.
 static void emitComments(const MachineInstr &MI, raw_ostream &CommentOS) {
   const MachineFunction *MF = MI.getMF();
   const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
 
   // Check for spills and reloads

llvm/test/CodeGen/AArch64/emutls.ll

 ; ARM64:      .section .rodata,
 ; ARM64-LABEL: __emutls_t.i3:
 ; ARM64-NEXT: .word 15
 
 ; ARM64:      .hidden __emutls_v.i4
 ; ARM64:      .data{{$}}
 ; ARM64:      .globl __emutls_v.i4
 ; ARM64-LABEL: __emutls_v.i4:
+; ARM64-NEXT: .L__emutls_v.i4$local:
 ; ARM64-NEXT: .xword 4
 ; ARM64-NEXT: .xword 4
 ; ARM64-NEXT: .xword 0
 ; ARM64-NEXT: .xword __emutls_t.i4
 
 ; ARM64:      .section .rodata,
 ; ARM64-LABEL: __emutls_t.i4:
+; ARM64-NEXT: .L__emutls_t.i4$local:
 ; ARM64-NEXT: .word 15
 
 ; ARM64-NOT:   __emutls_v.i5:
 ; ARM64:      .hidden __emutls_v.i5
 ; ARM64-NOT:   __emutls_v.i5:
 
 ; ARM64:      .data{{$}}
 ; ARM64:      .globl __emutls_v.s1

llvm/test/CodeGen/AArch64/machine-outliner-retaddr-sign-sp-mod.ll

 ; RUN: llc -mtriple aarch64-arm-none-eabi -enable-machine-outliner \
 ; RUN:  -verify-machineinstrs %s -o - | FileCheck %s
 
 @v = common dso_local global i32* null, align 8
 
 ; CHECK-LABEL:  foo:                                    // @foo
+; CHECK-NEXT:   .Lfoo$local:
 ; CHECK-NEXT:   // %bb.0:                               // %entry
 ; CHECK-NEXT:       paciasp
 ; CHECK-NOT:        OUTLINED_FUNCTION_
 ; CHECK:            retaa
 define dso_local void @foo(i32 %x) #0 {
 entry:
   %0 = zext i32 %x to i64
   %vla = alloca i32, i64 %0, align 4
   store volatile i32* %vla, i32** @v, align 8
   store volatile i32* %vla, i32** @v, align 8
   store volatile i32* %vla, i32** @v, align 8
   store volatile i32* %vla, i32** @v, align 8
   store volatile i32* %vla, i32** @v, align 8
   store volatile i32* %vla, i32** @v, align 8
   ret void
 }
 
 ; CHECK-LABEL:  bar:                                    // @bar
+; CHECK-NEXT:   .Lbar$local:
 ; CHECK-NEXT:   // %bb.0:                               // %entry
 ; CHECK-NEXT:       paciasp
 ; CHECK-NOT:        OUTLINED_FUNCTION_
 ; CHECK:            retaa
 define dso_local void @bar(i32 %x) #0 {
 entry:
   %0 = zext i32 %x to i64
   %vla = alloca i32, i64 %0, align 4

llvm/test/CodeGen/ARM/emutls.ll

 
 ; ARM32:      .section .rodata,
 ; ARM32-LABEL: __emutls_t.i3:
 ; ARM32-NEXT: .long 15
 
 ; ARM32:      .data{{$}}
 ; ARM32:      .globl __emutls_v.i4
 ; ARM32-LABEL: __emutls_v.i4:
+; ARM32-NEXT:  .L__emutls_v.i4$local:
 ; ARM32-NEXT: .long 4
 ; ARM32-NEXT: .long 4
 ; ARM32-NEXT: .long 0
 ; ARM32-NEXT: .long __emutls_t.i4
 
 ; ARM32:      .section .rodata,
 ; ARM32-LABEL: __emutls_t.i4:
+; ARM32-NEXT:  .L__emutls_t.i4$local:
 ; ARM32-NEXT: .long 15
 
 ; ARM32-NOT:   __emutls_v.i5:
 ; ARM32:      .hidden __emutls_v.i5
 ; ARM32-NOT:   __emutls_v.i5:
 
 ; ARM32:      .data{{$}}
 ; ARM32:      .globl __emutls_v.s1

llvm/test/CodeGen/X86/emutls.ll

 ; X32-NEXT: .long __emutls_t.i3
 
 ; X32       .section .rodata,
 ; X32-LABEL: __emutls_t.i3:
 ; X32-NEXT: .long 15
 
 ; X32       .section .data.rel.local,
 ; X32-LABEL: __emutls_v.i4:
+; X32-NEXT: .L__emutls_v.i4$local:
 ; X32-NEXT: .long 4
 ; X32-NEXT: .long 4
 ; X32-NEXT: .long 0
 ; X32-NEXT: .long __emutls_t.i4
 
 ; X32       .section .rodata,
 ; X32-LABEL: __emutls_t.i4:
+; X32-NEXT: .L__emutls_t.i4$local:
 ; X32-NEXT: .long 15
 
 ; X32-NOT:   __emutls_v.i5:
 ; X32       .hidden __emutls_v.i5
 ; X32-NOT:   __emutls_v.i5:
 
 ; X32 .section .data.rel.local,
 ; X32-LABEL: __emutls_v.s1:

 ; X64-NEXT: .quad __emutls_t.i3
 
 ; X64       .section .rodata,
 ; X64-LABEL: __emutls_t.i3:
 ; X64-NEXT: .long 15
 
 ; X64       .section .data.rel.local,
 ; X64-LABEL: __emutls_v.i4:
+; X64-NEXT: .L__emutls_v.i4$local:
 ; X64-NEXT: .quad 4
 ; X64-NEXT: .quad 4
 ; X64-NEXT: .quad 0
 ; X64-NEXT: .quad __emutls_t.i4
 
 ; X64       .section .rodata,
 ; X64-LABEL: __emutls_t.i4:
+; X64-NEXT: .L__emutls_t.i4$local:
 ; X64-NEXT: .long 15
 
 ; X64-NOT:   __emutls_v.i5:
 ; X64       .hidden __emutls_v.i5
 ; X64-NOT:   __emutls_v.i5:
 
 ; X64       .section .data.rel.local,
 ; X64-LABEL: __emutls_v.s1:

llvm/test/CodeGen/X86/linux-preemption.ll

-; RUN: llc -mtriple x86_64-pc-linux \
-; RUN:     -relocation-model=static  < %s | FileCheck --check-prefix=STATIC %s
-; RUN: llc -mtriple x86_64-pc-linux \
-; RUN:     -relocation-model=pic             < %s | FileCheck %s
-; RUN: llc -mtriple x86_64-pc-linux \
-; RUN:     -relocation-model=dynamic-no-pic  < %s | FileCheck %s
+; RUN: llc -mtriple x86_64-pc-linux -relocation-model=static < %s | \
+; RUN:   FileCheck --check-prefixes=COMMON,STATIC %s
+; RUN: llc -mtriple x86_64-pc-linux -relocation-model=pic < %s | \
+; RUN:   FileCheck --check-prefixes=COMMON,CHECK %s
+; RUN: llc -mtriple x86_64-pc-linux -relocation-model=dynamic-no-pic < %s | \
+; RUN:   FileCheck --check-prefixes=COMMON,CHECK %s
 
 ; 32 bits
 
 ; RUN: llc -mtriple i386-pc-linux \
 ; RUN:     -relocation-model=pic     < %s | FileCheck --check-prefix=CHECK32 %s
 
 ; globals
 

 ; CHECK32: movl external_default_function@GOT(%eax), %eax
 
 define dso_local void @strong_local_function() {
   ret void
 }
 define void()* @get_strong_local_function() {
   ret void()* @strong_local_function
 }
+; COMMON:     {{^}}strong_local_function:
+; COMMON-NEXT .Lstrong_local_function:
 ; CHECK: leaq strong_local_function(%rip), %rax
 ; STATIC: movl $strong_local_function, %eax
 ; CHECK32: leal strong_local_function@GOTOFF(%eax), %eax
 
 define weak dso_local void @weak_local_function() {
   ret void
 }
 define void()* @get_weak_local_function() {

 
 declare dso_preemptable void @external_preemptable_function()
 define void()* @get_external_preemptable_function() {
   ret void()* @external_preemptable_function
 }
 ; CHECK: movq external_preemptable_function@GOTPCREL(%rip), %rax
 ; STATIC: movl $external_preemptable_function, %eax
 ; CHECK32: movl external_preemptable_function@GOT(%eax), %eax
+
+; COMMON:     {{^}}strong_local_global:
+; COMMON-NEXT .Lstrong_local_global: