[ThinLTO] Remove unused symbol declarations when possible

[tejohnson]

Summary

Follow on to D42816 which dropped declarations of dead symbols. With
this change we also drop declarations which were never defined in this
module, but which are also unused after dead symbols are removed. These
symbols may or may not be dead in their defining modules.

Removing the unused declarations will prevent indirect call promotion in
the ThinLTO Backend from adding a new reference to a symbol, which can
result in linker unsats if that symbol was in fact dead and therefore
removed in its defining module. This can happen when we compile with a
sample profile collected from one binary but used for another, which may
have profiled targets that aren't used in the new binary.

Some test changes occur as a result (for whole program devirtualization,
we end up with a cast if there is no declaration in the module, for ICP
tests we may need to add a direct call to keep the declaration around).

[tejohnson]

Subsumed by D92804

clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll

trap:
   tail call void @llvm.trap()
   unreachable
 
 cont:
   %3 = extractvalue { i8*, i1 } %1, 0
   %4 = bitcast i8* %3 to i32 (%struct.A*, i32)*
 
   ; Check that the call was devirtualized.
-  ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
-  ; SKIP-IR-NOT: %call = tail call i32 @_ZN1A1nEi
+  ; CHECK-IR: %call = tail call i32 bitcast (void ()* @_ZN1A1nEi
+  ; SKIP-IR-NOT: %call = tail call i32 {{.*}}@_ZN1A1nEi
   %call = tail call i32 %4(%struct.A* nonnull %obj, i32 %a)
   %vtable16 = load i8*, i8** %0
   %5 = tail call { i8*, i1 } @llvm.type.checked.load(i8* %vtable16, i32 0, metadata !"_ZTS1A")
   %6 = extractvalue { i8*, i1 } %5, 1
   br i1 %6, label %cont2, label %trap
 
 cont2:
   %7 = extractvalue { i8*, i1 } %5, 0

llvm/lib/LTO/LTOBackend.cpp

if (ParallelCodeGenParallelismLevel == 1) {
     splitCodeGen(C, TM.get(), AddStream, ParallelCodeGenParallelismLevel,
                  std::move(Mod), CombinedIndex);
   }
   return Error::success();
 }
 
 static void dropDeadSymbols(Module &Mod, const GVSummaryMapTy &DefinedGlobals,
                             const ModuleSummaryIndex &Index) {
-  std::vector<GlobalValue*> DeadGVs;
-  for (auto &GV : Mod.global_values())
-    if (GlobalValueSummary *GVS = DefinedGlobals.lookup(GV.getGUID()))
+  std::vector<GlobalValue *> Declarations;
+  for (auto &GV : Mod.global_values()) {
+    if (GlobalValueSummary *GVS = DefinedGlobals.lookup(GV.getGUID())) {
       if (!Index.isGlobalValueLive(GVS)) {
-        DeadGVs.push_back(&GV);
+        Declarations.push_back(&GV);
         convertToDeclaration(GV);
       }
+    } else if (GV.isDeclaration())
+      // Check if any pre-existing declarations are unused after dead
+      // definitions are removed.
+      Declarations.push_back(&GV);
+  }
 
-  // Now that all dead bodies have been dropped, delete the actual objects
-  // themselves when possible.
-  for (GlobalValue *GV : DeadGVs) {
+  // Now that all dead bodies have been dropped, delete the declarations
+  // themselves when possible. Note this is important to avoid introducing
+  // calls to these functions during indirect call promotion, e.g. when using a
+  // sample profile from a different binary or different version of the source,
+  // that may reference virtual functions not live in this binary.
+  for (GlobalValue *GV : Declarations) {
     GV->removeDeadConstantUsers();
-    // Might reference something defined in native object (i.e. dropped a
-    // non-prevailing IR def, but we need to keep the declaration).
+    // Might have a reference to something defined in native object
+    // (i.e. dropped a non-prevailing IR def), so we need to keep the
+    // declaration. In this case there is a definition somewhere, so
+    // keeping the declaration should not cause any issues.
     if (GV->use_empty())
       GV->eraseFromParent();
   }
 }
 
 Error lto::thinBackend(const Config &Conf, unsigned Task, AddStreamFn AddStream,
                        Module &Mod, const ModuleSummaryIndex &CombinedIndex,
                        const FunctionImporter::ImportMapTy &ImportList,

llvm/test/LTO/Resolution/X86/load-sample-prof-icp.ll

 ; Checks that the call instruction is promoted to direct call and has
 ; profile count annotated on the direct call.
 define void @test(void ()*) #0 !dbg !7 {
   %2 = alloca void ()*
   store void ()* %0, void ()** %2
   %3 = load void ()*, void ()** %2
   ; CHECK: call void @bar(),{{.*}}!prof
   call void %3(), !dbg !10
+  ; Call bar directly so we don't eliminate the unused declaration before ICP,
+  ; which will block it.
+  ; CHECK: if.end.icp:
+  ; CHECK-NEXT: call void @bar()
+  call void @bar()
   ret void
 }
 
 declare void @bar() local_unnamed_addr
 
 attributes #0 = {"use-sample-profile"}
 
 !llvm.dbg.cu = !{!0}

llvm/test/ThinLTO/X86/cfi-devirt.ll

trap:
   tail call void @llvm.trap()
   unreachable
 
 cont:
   %3 = extractvalue { i8*, i1 } %1, 0
   %4 = bitcast i8* %3 to i32 (%struct.A*, i32)*
 
   ; Check that the call was devirtualized.
-  ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
+  ; CHECK-IR: %call = tail call i32 bitcast (void ()* @_ZN1A1nEi
   %call = tail call i32 %4(%struct.A* nonnull %obj, i32 %a)
   %vtable16 = load i8*, i8** %0
   %5 = tail call { i8*, i1 } @llvm.type.checked.load(i8* %vtable16, i32 0, metadata !"_ZTS1A")
   %6 = extractvalue { i8*, i1 } %5, 1
   br i1 %6, label %cont2, label %trap
 
 cont2:
   %7 = extractvalue { i8*, i1 } %5, 0

llvm/test/ThinLTO/X86/devirt-after-icp.ll

if.true.direct_targ:                              ; preds = %entry
   %6 = bitcast i32 (%class.A*)** %vtable to i8*
   %7 = tail call i1 @llvm.type.test(i8* %6, metadata !"_ZTS1B")
   tail call void @llvm.assume(i1 %7)
   %vfn.i1 = getelementptr inbounds i32 (%class.A*)*, i32 (%class.A*)** %vtable, i64 1
   %vfn.i = bitcast i32 (%class.A*)** %vfn.i1 to i32 (%class.B*)**
   %8 = load i32 (%class.B*)*, i32 (%class.B*)** %vfn.i, align 8
 ; Call to bar() can be devirtualized to call to B::bar(), since it was
 ; inlined from B::foo() after ICP introduced the guarded promotion.
-; CHECK-IR: %call.i = tail call i32 @_ZN1B3barEv(%class.B* %3)
+; CHECK-IR: %call.i = tail call i32 bitcast (void ()* @_ZN1B3barEv to i32 (%class.B*)*)(%class.B* %3)
   %call.i = tail call i32 %8(%class.B* %5)
   br label %if.end.icp
 
 ; This block contains the fallback indirect call a->foo()
 ; CHECK-IR: if.false.orig_indirect:
 if.false.orig_indirect:                           ; preds = %entry
 ; Fallback indirect call to foo() cannot be devirtualized.
 ; CHECK-IR: %call = tail call i32 %

llvm/test/ThinLTO/X86/index-const-prop-alias.ll

 ; RUN:               %t2.bc -r=%t2.bc,g,pl -r=%t2.bc,g.alias,pl -save-temps -o %t4
 ; RUN: llvm-dis %t4.1.3.import.bc -o - | FileCheck %s --check-prefix=PRESERVED
 
 ; When g.alias is preserved we can't internalize aliasee either
 ; RUN: llvm-lto2 run %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,ret_ptr,pl -r=%t1.bc,g.alias,l -r=%t1.bc,g,l \
 ; RUN:               %t2.bc -r=%t2.bc,g,pl -r=%t2.bc,g.alias,plx -save-temps -o %t5
 ; RUN: llvm-dis %t5.1.3.import.bc -o - | FileCheck %s --check-prefix=PRESERVED
 
-; We currently don't support importing aliases
-; IMPORT:       @g.alias = external global i32
-; IMPORT-NEXT:  @g = internal global i32 42, align 4 #0
+; IMPORT:  @g = internal global i32 42, align 4 #0
 ; IMPORT:  attributes #0 = { "thinlto-internalize" }
 
 ; CODEGEN:      define dso_local i32 @main
 ; CODEGEN-NEXT:    ret i32 42
 
-; PRESERVED:      @g.alias = external global i32
-; PRESERVED-NEXT: @g = available_externally global i32 42, align 4
+; PRESERVED: @g = available_externally global i32 42, align 4
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @g.alias = external global i32
 @g = external global i32
 
 define i32 @main() {
   %v = load i32, i32* @g
   ret i32 %v
 }
 
 define i32* @ret_ptr() {
   ret i32* @g.alias
 }

llvm/test/ThinLTO/X86/index-const-prop-dead.ll

 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/index-const-prop-define-g.ll -o %t2.bc
 ; RUN: llvm-lto2 run -save-temps %t2.bc -r=%t2.bc,g,pl \
 ; RUN:               %t1.bc -r=%t1.bc,main,plx -r=%t1.bc,foo,pl -r=%t1.bc,g, -o %t3
 ; RUN: llvm-dis %t3.2.3.import.bc -o - | FileCheck %s
 
-; Dead globals are converted to declarations by ThinLTO in dropDeadSymbols
-; If we try to internalize such we'll get a broken module. 
-; CHECK: @g = external global i32
+; Dead globals are converted to declarations by ThinLTO, and unused declarations
+; are removed, in dropDeadSymbols,
+; CHECK-NOT: @g
 
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @g = external global i32
 
 ; We need at least one live symbol to enable dead stripping
 ; Otherwise ModuleSummaryIndex::isGlobalValueLive will always