[WPD] Reduce ThinLTO link time by avoiding unnecessary summary analysis (llvm#164046)

We are scanning through every single definition of a vtable across all
translation units which is unnecessary in most cases.

If this is a local, we want to make sure there isn't another local with
the same GUID due to it having the same relative path. However, we were
always scanning through every single summary in all cases.

We can now check the new HasLocal flag added in PR164647 ahead of the
loop,
instead of checking on every iteration.

This cut down a large thin link by around 6%, which was over half the
time it spent in WPD.

Note that we previously took the last conforming vtable summary, and now
we use the first. This caused a test difference in one somewhat
contrived test for vtables in comdats.

Author: teresajohnson

llvm/include/llvm/IR/ModuleSummaryIndex.h

@@ -178,6 +178,8 @@ struct alignas(8) GlobalValueSummaryInfo {
   /// only be called prior to index-based internalization and promotion.
   inline void verifyLocal() const;
 
+  bool hasLocal() const { return HasLocal; }
+
 private:
   /// List of global value summary structures for a particular value held
   /// in the GlobalValueMap. Requires a vector in the case of multiple
@@ -239,6 +241,8 @@ struct ValueInfo {
 
   void verifyLocal() const { getRef()->second.verifyLocal(); }
 
+  bool hasLocal() const { return getRef()->second.hasLocal(); }
+
   // Even if the index is built with GVs available, we may not have one for
   // summary entries synthesized for profiled indirect call targets.
   bool hasName() const { return !haveGVs() || getValue(); }

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

@@ -1161,14 +1161,10 @@ bool DevirtIndex::tryFindVirtualCallTargets(
     // and therefore the same GUID. This can happen if there isn't enough
     // distinguishing path when compiling the source file. In that case we
     // conservatively return false early.
+    if (P.VTableVI.hasLocal() && P.VTableVI.getSummaryList().size() > 1)
+      return false;
     const GlobalVarSummary *VS = nullptr;
-    bool LocalFound = false;
     for (const auto &S : P.VTableVI.getSummaryList()) {
-      if (GlobalValue::isLocalLinkage(S->linkage())) {
-        if (LocalFound)
-          return false;
-        LocalFound = true;
-      }
       auto *CurVS = cast<GlobalVarSummary>(S->getBaseObject());
       if (!CurVS->vTableFuncs().empty() ||
           // Previously clang did not attach the necessary type metadata to
@@ -1184,6 +1180,7 @@ bool DevirtIndex::tryFindVirtualCallTargets(
         // with public LTO visibility.
         if (VS->getVCallVisibility() == GlobalObject::VCallVisibilityPublic)
           return false;
+        break;
       }
     }
     // There will be no VS if all copies are available_externally having no
@@ -2591,6 +2588,11 @@ void DevirtIndex::run() {
   if (ExportSummary.typeIdCompatibleVtableMap().empty())
     return;
 
+  // Assert that we haven't made any changes that would affect the hasLocal()
+  // flag on the GUID summary info.
+  assert(!ExportSummary.withInternalizeAndPromote() &&
+         "Expect index-based WPD to run before internalization and promotion");
+
   DenseMap<GlobalValue::GUID, std::vector<StringRef>> NameByGUID;
   for (const auto &P : ExportSummary.typeIdCompatibleVtableMap()) {
     NameByGUID[GlobalValue::getGUIDAssumingExternalLinkage(P.first)].push_back(

llvm/test/ThinLTO/X86/devirt_external_comdat_same_guid.ll

@@ -24,9 +24,9 @@
 ; RUN: llvm-dis %t5.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR1
 ; RUN: llvm-dis %t5.2.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR2
 
-; PRINT-DAG: Devirtualized call to {{.*}} (_ZN1A1nEi)
+; PRINT-DAG: Devirtualized call to {{.*}} (_ZN1B1nEi)
 
-; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
+; REMARK-DAG: single-impl: devirtualized a call to _ZN1B1nEi
 
 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-grtev4-linux-gnu"
@@ -55,7 +55,7 @@ entry:
   ret i32 0
 }
 
-; CHECK-IR1: define i32 @test(
+; CHECK-IR1: define noundef i32 @test(
 define i32 @test(ptr %obj, i32 %a) {
 entry:
   %vtable = load ptr, ptr %obj
@@ -65,15 +65,15 @@ entry:
   %fptr1 = load ptr, ptr %fptrptr, align 8
 
   ; Check that the call was devirtualized.
-  ; CHECK-IR1: tail call i32 {{.*}}@_ZN1A1nEi
+  ; CHECK-IR1: tail call i32 {{.*}}@_ZN1B1nEi
   %call = tail call i32 %fptr1(ptr nonnull %obj, i32 %a)
 
   ret i32 %call
 }
 
 ; CHECK-IR2: define i32 @test2
 ; Check that the call was devirtualized.
-; CHECK-IR2:   tail call i32 @_ZN1A1nEi
+; CHECK-IR2:   tail call i32 @_ZN1B1nEi
 
 declare i1 @llvm.type.test(ptr, metadata)
 declare void @llvm.assume(i1)