Skip to content

Commit a89df72

Browse files
authored
WholeProgramDevirt: Fix importing in llvm.type.checked.load case.
We were clearing SummaryTypeCheckedLoadUsers to prevent devirtualized llvm.type.checked.load calls from being converted to llvm.type.test, which meant that AddCalls would not see them in the list of callsites and they would not get imported. Fix that by not clearing SummaryTypeCheckedLoadUsers so that the list survives to AddCalls and using AllCallSitesDevirted to control whether to convert them instead. Reviewers: teresajohnson Reviewed By: teresajohnson Pull Request: #144019
1 parent 5064a5b commit a89df72

File tree

3 files changed

+74
-22
lines changed

3 files changed

+74
-22
lines changed

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -494,28 +494,28 @@ struct CallSiteInfo {
494494
/// Whether all call sites represented by this CallSiteInfo, including those
495495
/// in summaries, have been devirtualized. This starts off as true because a
496496
/// default constructed CallSiteInfo represents no call sites.
497+
///
498+
/// If at the end of the pass there are still undevirtualized calls, we will
499+
/// need to add a use of llvm.type.test to each of the function summaries in
500+
/// the vector.
497501
bool AllCallSitesDevirted = true;
498502

499503
// These fields are used during the export phase of ThinLTO and reflect
500504
// information collected from function summaries.
501505

502-
/// Whether any function summary contains an llvm.assume(llvm.type.test) for
503-
/// this slot.
504-
bool SummaryHasTypeTestAssumeUsers = false;
505-
506506
/// CFI-specific: a vector containing the list of function summaries that use
507507
/// the llvm.type.checked.load intrinsic and therefore will require
508508
/// resolutions for llvm.type.test in order to implement CFI checks if
509-
/// devirtualization was unsuccessful. If devirtualization was successful, the
510-
/// pass will clear this vector by calling markDevirt(). If at the end of the
511-
/// pass the vector is non-empty, we will need to add a use of llvm.type.test
512-
/// to each of the function summaries in the vector.
509+
/// devirtualization was unsuccessful.
513510
std::vector<FunctionSummary *> SummaryTypeCheckedLoadUsers;
511+
512+
/// A vector containing the list of function summaries that use
513+
/// assume(llvm.type.test).
514514
std::vector<FunctionSummary *> SummaryTypeTestAssumeUsers;
515515

516516
bool isExported() const {
517-
return SummaryHasTypeTestAssumeUsers ||
518-
!SummaryTypeCheckedLoadUsers.empty();
517+
return !SummaryTypeCheckedLoadUsers.empty() ||
518+
!SummaryTypeTestAssumeUsers.empty();
519519
}
520520

521521
void addSummaryTypeCheckedLoadUser(FunctionSummary *FS) {
@@ -525,16 +525,10 @@ struct CallSiteInfo {
525525

526526
void addSummaryTypeTestAssumeUser(FunctionSummary *FS) {
527527
SummaryTypeTestAssumeUsers.push_back(FS);
528-
SummaryHasTypeTestAssumeUsers = true;
529528
AllCallSitesDevirted = false;
530529
}
531530

532-
void markDevirt() {
533-
AllCallSitesDevirted = true;
534-
535-
// As explained in the comment for SummaryTypeCheckedLoadUsers.
536-
SummaryTypeCheckedLoadUsers.clear();
537-
}
531+
void markDevirt() { AllCallSitesDevirted = true; }
538532
};
539533

540534
// Call site information collected for a specific VTableSlot.
@@ -2465,11 +2459,14 @@ bool DevirtModule::run() {
24652459
if (ExportSummary && isa<MDString>(S.first.TypeID)) {
24662460
auto GUID = GlobalValue::getGUIDAssumingExternalLinkage(
24672461
cast<MDString>(S.first.TypeID)->getString());
2468-
for (auto *FS : S.second.CSInfo.SummaryTypeCheckedLoadUsers)
2469-
FS->addTypeTest(GUID);
2462+
auto AddTypeTestsForTypeCheckedLoads = [&](CallSiteInfo &CSI) {
2463+
if (!CSI.AllCallSitesDevirted)
2464+
for (auto *FS : CSI.SummaryTypeCheckedLoadUsers)
2465+
FS->addTypeTest(GUID);
2466+
};
2467+
AddTypeTestsForTypeCheckedLoads(S.second.CSInfo);
24702468
for (auto &CCS : S.second.ConstCSInfo)
2471-
for (auto *FS : CCS.second.SummaryTypeCheckedLoadUsers)
2472-
FS->addTypeTest(GUID);
2469+
AddTypeTestsForTypeCheckedLoads(CCS.second);
24732470
}
24742471
}
24752472

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
; ModuleID = 'foo.cpp'
2+
source_filename = "foo.cpp"
3+
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
4+
target triple = "x86_64-unknown-linux-gnu"
5+
6+
%struct.A = type { ptr }
7+
8+
; Function Attrs: uwtable
9+
define hidden i32 @_Z3fooP1A(ptr %pA) local_unnamed_addr {
10+
entry:
11+
%vtable = load ptr, ptr %pA, align 8, !tbaa !2
12+
%0 = call { ptr, i1 } @llvm.type.checked.load(ptr %vtable, i32 0, metadata !"_ZTS1A")
13+
%1 = extractvalue { ptr, i1 } %0, 0
14+
%call = tail call i32 %1(ptr %pA)
15+
%add = add nsw i32 %call, 10
16+
ret i32 %add
17+
}
18+
19+
declare { ptr, i1 } @llvm.type.checked.load(ptr, i32, metadata)
20+
21+
; Function Attrs: nounwind willreturn
22+
declare void @llvm.assume(i1)
23+
24+
!llvm.module.flags = !{!0}
25+
!llvm.ident = !{!1}
26+
27+
!0 = !{i32 1, !"wchar_size", i32 4}
28+
!1 = !{!"clang version 10.0.0 (trunk 373596)"}
29+
!2 = !{!3, !3, i64 0}
30+
!3 = !{!"vtable pointer", !4, i64 0}
31+
!4 = !{!"Simple C++ TBAA"}

llvm/test/ThinLTO/X86/devirt_single_hybrid.ll

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,32 @@
22
; when we're running hybrid LTO.
33
;
44
; RUN: opt -thinlto-bc -thinlto-split-lto-unit %s -o %t-main.bc
5-
; RUN: opt -thinlto-bc -thinlto-split-lto-unit %p/Inputs/devirt_single_hybrid_foo.ll -o %t-foo.bc
65
; RUN: opt -thinlto-bc -thinlto-split-lto-unit %p/Inputs/devirt_single_hybrid_bar.ll -o %t-bar.bc
6+
7+
; Test the assume(type.test) case.
8+
9+
; RUN: opt -thinlto-bc -thinlto-split-lto-unit %p/Inputs/devirt_single_hybrid_foo.ll -o %t-foo.bc
10+
; RUN: llvm-lto2 run -save-temps %t-main.bc %t-foo.bc %t-bar.bc -pass-remarks=. -o %t \
11+
; RUN: -whole-program-visibility \
12+
; RUN: -r=%t-foo.bc,_Z3fooP1A,pl \
13+
; RUN: -r=%t-main.bc,main,plx \
14+
; RUN: -r=%t-main.bc,_Z3barv,l \
15+
; RUN: -r=%t-bar.bc,_Z3barv,pl \
16+
; RUN: -r=%t-bar.bc,_Z3fooP1A, \
17+
; RUN: -r=%t-bar.bc,_ZNK1A1fEv,pl \
18+
; RUN: -r=%t-bar.bc,_ZTV1A,l \
19+
; RUN: -r=%t-bar.bc,_ZTVN10__cxxabiv117__class_type_infoE, \
20+
; RUN: -r=%t-bar.bc,_ZTS1A,pl \
21+
; RUN: -r=%t-bar.bc,_ZTI1A,pl \
22+
; RUN: -r=%t-bar.bc,_ZNK1A1fEv, \
23+
; RUN: -r=%t-bar.bc,_ZTV1A,pl \
24+
; RUN: -r=%t-bar.bc,_ZTI1A, 2>&1 | FileCheck %s --check-prefix=REMARK
25+
; RUN: llvm-dis %t.1.3.import.bc -o - | FileCheck %s --check-prefix=IMPORT
26+
; RUN: llvm-dis %t.1.5.precodegen.bc -o - | FileCheck %s --check-prefix=CODEGEN
27+
28+
; Test the type.checked.load case.
29+
30+
; RUN: opt -thinlto-bc -thinlto-split-lto-unit %p/Inputs/devirt_single_hybrid_foo_tcl.ll -o %t-foo.bc
731
; RUN: llvm-lto2 run -save-temps %t-main.bc %t-foo.bc %t-bar.bc -pass-remarks=. -o %t \
832
; RUN: -whole-program-visibility \
933
; RUN: -r=%t-foo.bc,_Z3fooP1A,pl \

0 commit comments

Comments
 (0)