[llvm] 4638eb2 - [ThinLTO] Ignore callee edge to global variable

[Teresa Johnson via llvm-commits]

Author: Teresa Johnson
Date: 2023-06-08T06:44:06-07:00
New Revision: 4638eb26603c8d1cd3c46167734356e2585afdfe

URL: https://github.com/llvm/llvm-project/commit/4638eb26603c8d1cd3c46167734356e2585afdfe
DIFF: https://github.com/llvm/llvm-project/commit/4638eb26603c8d1cd3c46167734356e2585afdfe.diff

LOG: [ThinLTO] Ignore callee edge to global variable

Since the symbols in the ThinLTO summary are indexed by GUID we can end
up in corner cases where a callee edge in the combined index goes to a
summary for a global variable. This could happen in the case of hash
collisions, and in the case of SamplePGO profiles could potentially happen
due to code changes (since we synthesize call edges to GUIDs that were
inlined callees in the profiled code).

Handle this by simply ignoring any non-FunctionSummary callees.

Differential Revision: https://reviews.llvm.org/D152406

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/FunctionImport.cpp
    llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
    llvm/test/Transforms/FunctionImport/funcimport_var.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index 10718bb077552..f635b14cd2a90 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -179,7 +179,15 @@ static auto qualifyCalleeCandidates(
           return {FunctionImporter::ImportFailureReason::InterposableLinkage,
                   GVSummary};
 
-        auto *Summary = cast<FunctionSummary>(GVSummary->getBaseObject());
+        auto *Summary = dyn_cast<FunctionSummary>(GVSummary->getBaseObject());
+
+        // Ignore any callees that aren't actually functions. This could happen
+        // in the case of GUID hash collisions. It could also happen in theory
+        // for SamplePGO profiles collected on old versions of the code after
+        // renaming, since we synthesize edges to any inlined callees appearing
+        // in the profile.
+        if (!Summary)
+          return {FunctionImporter::ImportFailureReason::GlobalVar, GVSummary};
 
         // If this is a local function, make sure we import the copy
         // in the caller's module. The only time a local function can

diff  --git a/llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll b/llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
index 3ee4b9204b424..72da66008dc1e 100644
--- a/llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
+++ b/llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
@@ -3,6 +3,9 @@ target triple = "x86_64-unknown-linux-gnu"
 
 @link = internal global i32 0, align 4
 
+; guid = 12887606300320728018
+ at globalvar = global i32 0, align 4
+
 ; Function Attrs: norecurse nounwind readnone uwtable
 define nonnull ptr @get_link() local_unnamed_addr {
   ret ptr @link

diff  --git a/llvm/test/Transforms/FunctionImport/funcimport_var.ll b/llvm/test/Transforms/FunctionImport/funcimport_var.ll
index e0a0da58dd939..7726a06f45e43 100644
--- a/llvm/test/Transforms/FunctionImport/funcimport_var.ll
+++ b/llvm/test/Transforms/FunctionImport/funcimport_var.ll
@@ -1,16 +1,20 @@
-; This test makes sure a static var is not selected as a callee target
-; (which will crash compilation).
+;; This test makes sure a static var is not selected as a callee target (it will
+;; have a 
diff erent GUID). Similarly, ensure that a callee GUID annotated from a
+;; sample pgo profile that matches a global variable GUID (either due to code
+;; changes or a hash collision), does not get considered for importing. Either
+;; of these can crash compilation.
 ; RUN: opt -module-summary %s -o %t.bc
 ; RUN: opt -module-summary %p/Inputs/funcimport_var2.ll -o %t2.bc
 ; RUN: llvm-lto -thinlto -thinlto-action=thinlink -o %t3 %t.bc %t2.bc
 ; RUN: llvm-lto -thinlto -thinlto-action=import -thinlto-index=%t3 %t.bc %t2.bc
 ; RUN: llvm-lto -thinlto -thinlto-action=run %t.bc %t2.bc -exported-symbol=_Z4LinkPKcS0_
-; RUN: llvm-nm %t.bc.thinlto.o | FileCheck %s
+; RUN: llvm-nm %t.bc.thinlto.o | FileCheck %s --implicit-check-not=globalvar
 ; RUN: llvm-lto2 run %t.bc %t2.bc -o %t.out \
 ; RUN:   -r %t.bc,_Z4LinkPKcS0_,plx \
 ; RUN:   -r %t.bc,link,l \
+; RUN:   -r %t2.bc,globalvar,plx \
 ; RUN:   -r %t2.bc,get_link,plx
-; RUN: llvm-nm %t.out.1 | FileCheck %s
+; RUN: llvm-nm %t.out.1 | FileCheck %s --implicit-check-not=globalvar
 ; CHECK: U link
 
 ; REQUIRES: x86-registered-target
@@ -18,10 +22,13 @@
 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"
 
-define i32 @_Z4LinkPKcS0_(ptr, ptr) local_unnamed_addr  {
+define i32 @_Z4LinkPKcS0_(ptr, ptr) local_unnamed_addr !prof !1 {
   %3 = tail call i32 @link(ptr %0, ptr %1) #2
   ret i32 %3
 }
 
 ; Function Attrs: nounwind
-declare i32 @link(ptr, ptr) local_unnamed_addr 
+declare i32 @link(ptr, ptr) local_unnamed_addr
+
+;; This matches the GUID of global variable @globalvar in the other input.
+!1 = !{!"function_entry_count", i64 110, i64 12887606300320728018}