[PATCH] D152406: [ThinLTO] Ignore callee edge to global variable

[Teresa Johnson via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4638eb26603c: [ThinLTO] Ignore callee edge to global variable (authored by tejohnson).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152406/new/

https://reviews.llvm.org/D152406

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


Index: llvm/test/Transforms/FunctionImport/funcimport_var.ll
===================================================================
--- llvm/test/Transforms/FunctionImport/funcimport_var.ll
+++ 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 different 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}
Index: llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
===================================================================
--- llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
+++ llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
@@ -3,6 +3,9 @@
 
 @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
Index: llvm/lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -179,7 +179,15 @@
           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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D152406.529579.patch
Type: text/x-patch
Size: 3796 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230608/d1d8867c/attachment.bin>