Skip to content

Commit

Permalink
[ThinLTO] Ignore callee edge to global variable
Browse files Browse the repository at this point in the history
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
  • Loading branch information
teresajohnson committed Jun 8, 2023
1 parent 2f1fb5d commit 4638eb2
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 7 deletions.
10 changes: 9 additions & 1 deletion llvm/lib/Transforms/IPO/FunctionImport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions llvm/test/Transforms/FunctionImport/Inputs/funcimport_var2.ll
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ target triple = "x86_64-unknown-linux-gnu"

@link = internal global i32 0, align 4

; guid = 12887606300320728018
@globalvar = global i32 0, align 4

; Function Attrs: norecurse nounwind readnone uwtable
define nonnull ptr @get_link() local_unnamed_addr {
ret ptr @link
Expand Down
19 changes: 13 additions & 6 deletions llvm/test/Transforms/FunctionImport/funcimport_var.ll
Original file line number Diff line number Diff line change
@@ -1,27 +1,34 @@
; 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

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}

0 comments on commit 4638eb2

Please sign in to comment.