[CodeGen] don't emit addrsig symbol if it's used only by metadata

[zequanwu]

Summary

Value only used by metadata can be removed from .addrsig table.
This solves the undefined symbol error when enabling addrsig table on COFF LTO.

[rnk]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1854

I see, so, the GV is used by a constant, which is then used by metadata. Metadata uses do not appear in the use list.

I think this condition is slightly incorrect, though, the bitcast constant expression could be used by metadata *and* a real Value. So, consider your test case, but then add in a real instruction that uses the same bitcast. In this case, we should still put GV into the addrsig table.

One refinement would be to check that each user is a Constant with no uses, instead of checking if it is used by metadata. This could be:

if (isa<Constant>(U) && U->use_empty())

However, this will not handle nested constant expressions. Consider a series of getelementptr and bitcast constant expressions. To be fully general, you need a worklist to check that all transitive users of GV are non-GlobalValue Constants that are only used by other constants.

[zequanwu]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1854

Yea, I see the problem.

if (isa<Constant>(U) && U->use_empty())

I understand this, but confused by below.

check that all transitive users of GV are non-GlobalValue Constants that are only used by other constants.

IIUC, if all transitive users of the GV are non-GlobalValue constants that are not transitively used by any non-GlobalValue constants, the GV is not address significant. For example, in the test case below, @a1 and @i1 are not address significant, right?

[rnk]

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1854

In the test below, I see two Instructions that use @a1 and @i1, so I think they are address significant.

I mentioned GlobalValues because they are also subclasses of Constant, but if a GlobalVariable uses a Function, for example, that is a real use and we should consider the function address significant. Consider:

int someFunc(int x) { return x + 1; }
void *p = (void *)&someFunc;

The only user of someFunc will be a bitcast constant, and then the only use of that bitcast will be p.

[zequanwu]

Address comment.

[rnk]

Looks good, but please factor it out for readability.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

1851

Please factor this out into its own function and give it some comments. I can suggest a name of hasTransitiveNonMetadataUse, or maybe just hasNonMetadataUse.

[zequanwu]

Refactor.

llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp

bool AsmPrinter::doFinalization(Module &M) {
   Function *InitTrampolineIntrinsic = M.getFunction("llvm.init.trampoline");
   if (!InitTrampolineIntrinsic || InitTrampolineIntrinsic->use_empty())
     if (MCSection *S = MAI->getNonexecutableStackSection(OutContext))
       OutStreamer->SwitchSection(S);
 
   if (TM.Options.EmitAddrsig) {
     // Emit address-significance attributes for all globals.
     OutStreamer->emitAddrsig();
-    for (const GlobalValue &GV : M.global_values())
+    for (const GlobalValue &GV : M.global_values()) {
+      if (!GV.use_empty()) {
+        llvm::SmallVector<const User *, 32> WorkList;
+        llvm::SmallPtrSet<const User *, 32> Visited;
+        WorkList.insert(WorkList.begin(), GV.user_begin(), GV.user_end());
+        bool Used = false;
+        while (!WorkList.empty()) {
+          const User *U = WorkList.back();
+          WorkList.pop_back();
+          Visited.insert(U);
+          if (!isa<Constant>(U) || isa<GlobalValue>(U)) {
+            Used = true;
+            break;
+          }
+          for (const User *UU : U->users())
+            if (!Visited.count(UU))
+              WorkList.push_back(UU);
+        }
+        if (!Used)
+          continue;
+      }
       if (!GV.use_empty() && !GV.isThreadLocal() &&
           !GV.hasDLLImportStorageClass() && !GV.getName().startswith("llvm.") &&
           !GV.hasAtLeastLocalUnnamedAddr())
         OutStreamer->emitAddrsigSym(getSymbol(&GV));
     }
+  }
 
   // Emit symbol partition specifications (ELF only).
   if (TM.getTargetTriple().isOSBinFormatELF()) {
     unsigned UniqueID = 0;
     for (const GlobalValue &GV : M.global_values()) {
       if (!GV.hasPartition() || GV.isDeclarationForLinker() ||
           GV.getVisibility() != GlobalValue::DefaultVisibility)
         continue;

llvm/test/CodeGen/X86/addrsig.ll

define void @f1() {
   %g2 = bitcast i32* @g2 to i8*
   %g3 = bitcast i32* @g3 to i8*
   %dllimport = bitcast i32* @dllimport to i8*
   %tls = bitcast i32* @tls to i8*
   %a1 = bitcast i32* @a1 to i8*
   %a2 = bitcast i32* @a2 to i8*
   %i1 = bitcast void()* @i1 to i8*
   %i2 = bitcast void()* @i2 to i8*
+  call void @llvm.dbg.value(metadata i8* bitcast (void()* @metadata_f1 to i8*), metadata !5, metadata !DIExpression()), !dbg !7
+  call void @llvm.dbg.value(metadata i8* bitcast (void()* @metadata_f2 to i8*), metadata !5, metadata !DIExpression()), !dbg !7
+  call void @f4(i8* bitcast (void()* @metadata_f2 to i8*))
   unreachable
 }
 
+declare void @f4(i8*) unnamed_addr
+
+; CHECK-NOT: .addrsig_sym metadata_f1
+declare void @metadata_f1()
+
+; CHECK: .addrsig_sym metadata_f2
+declare void @metadata_f2()
+
 ; CHECK-NOT: .addrsig_sym f2
 define internal void @f2() local_unnamed_addr {
   unreachable
 }
 
 ; CHECK-NOT: .addrsig_sym f3
 declare void @f3() unnamed_addr
 

 @a1 = alias i32, i32* @g1
 ; CHECK-NOT: .addrsig_sym a2
 @a2 = internal local_unnamed_addr alias i32, i32* @g2
 
 ; CHECK: .addrsig_sym i1
 @i1 = ifunc void(), void()* @f1
 ; CHECK-NOT: .addrsig_sym i2
 @i2 = internal local_unnamed_addr ifunc void(), void()* @f2
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!2}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, isOptimized: false, runtimeVersion: 0, emissionKind: NoDebug)
+!1 = !DIFile(filename: "a", directory: "")
+!2 = !{i32 2, !"Debug Info Version", i32 3}
+!3 = distinct !DISubprogram(scope: null, isLocal: false, isDefinition: true, isOptimized: false, unit: !0)
+!4 = !DILocation(line: 0, scope: !3)
+!5 = !DILocalVariable(scope: !6)
+!6 = distinct !DISubprogram(scope: null, isLocal: false, isDefinition: true, isOptimized: false, unit: !0)
+!7 = !DILocation(line: 0, scope: !6, inlinedAt: !4)