[llvm] 8045ba8 - [ThinLTO/WPD] Handle function alias in vtable correctly
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 16 18:20:27 PST 2023
Author: Teresa Johnson
Date: 2023-02-16T18:20:12-08:00
New Revision: 8045ba89488c9f7cc1c291e6c939e02e04fbcd2e
URL: https://github.com/llvm/llvm-project/commit/8045ba89488c9f7cc1c291e6c939e02e04fbcd2e
DIFF: https://github.com/llvm/llvm-project/commit/8045ba89488c9f7cc1c291e6c939e02e04fbcd2e.diff
LOG: [ThinLTO/WPD] Handle function alias in vtable correctly
We were not summarizing a function alias in the vtable, leading to
incorrect WPD in some cases, and missing WPD in others.
Specifically, we would end up ignoring function aliases as they aren't
summarized, so we could incorrectly devirtualize if there was a single
other non-alias function in a compatible vtable. And if there was only
one implementation, but it was an alias, we would not be able to
identify and perform the single implementation devirtualization.
Handling the alias summary correctly also required fixing the handling
in mustBeUnreachableFunction, so that it is not incorrectly ignored.
Regular LTO is conservatively correct because it will skip
devirtualizing when any pointer within a vtable is not a function.
However, it needs additional work to be able to take advantage of
function alias within the vtable that is in fact the only
implementation. For that reason, the Regular LTO testing in the second
test case is currently disabled, and will be enabled along with a follow
on enhancement fix for Regular LTO WPD.
Differential Revision: https://reviews.llvm.org/D144209
Added:
llvm/test/ThinLTO/X86/devirt_function_alias.ll
llvm/test/ThinLTO/X86/devirt_function_alias2.ll
Modified:
llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
index 3dfa2d821e837..26ff84f2324cc 100644
--- a/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
+++ b/llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
@@ -583,12 +583,17 @@ static void findFuncPointers(const Constant *I, uint64_t StartingOffset,
VTableFuncList &VTableFuncs) {
// First check if this is a function pointer.
if (I->getType()->isPointerTy()) {
- auto Fn = dyn_cast<Function>(I->stripPointerCasts());
- // We can disregard __cxa_pure_virtual as a possible call target, as
- // calls to pure virtuals are UB.
- if (Fn && Fn->getName() != "__cxa_pure_virtual")
- VTableFuncs.push_back({Index.getOrInsertValueInfo(Fn), StartingOffset});
- return;
+ auto C = I->stripPointerCasts();
+ auto A = dyn_cast<GlobalAlias>(C);
+ if (isa<Function>(C) || (A && isa<Function>(A->getAliasee()))) {
+ auto GV = dyn_cast<GlobalValue>(C);
+ assert(GV);
+ // We can disregard __cxa_pure_virtual as a possible call target, as
+ // calls to pure virtuals are UB.
+ if (GV && GV->getName() != "__cxa_pure_virtual")
+ VTableFuncs.push_back({Index.getOrInsertValueInfo(GV), StartingOffset});
+ return;
+ }
}
// Walk through the elements in the constant struct or array and recursively
diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index 7410b37c2d9b0..274bb7e120e40 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -379,6 +379,7 @@ namespace {
// conditions
// 1) All summaries are live.
// 2) All function summaries indicate it's unreachable
+// 3) There is no non-function with the same GUID (which is rare)
bool mustBeUnreachableFunction(ValueInfo TheFnVI) {
if ((!TheFnVI) || TheFnVI.getSummaryList().empty()) {
// Returns false if ValueInfo is absent, or the summary list is empty
@@ -391,12 +392,13 @@ bool mustBeUnreachableFunction(ValueInfo TheFnVI) {
// In general either all summaries should be live or all should be dead.
if (!Summary->isLive())
return false;
- if (auto *FS = dyn_cast<FunctionSummary>(Summary.get())) {
+ if (auto *FS = dyn_cast<FunctionSummary>(Summary->getBaseObject())) {
if (!FS->fflags().MustBeUnreachable)
return false;
}
- // Do nothing if a non-function has the same GUID (which is rare).
- // This is correct since non-function summaries are not relevant.
+ // Be conservative if a non-function has the same GUID (which is rare).
+ else
+ return false;
}
// All function summaries are live and all of them agree that the function is
// unreachble.
diff --git a/llvm/test/ThinLTO/X86/devirt_function_alias.ll b/llvm/test/ThinLTO/X86/devirt_function_alias.ll
new file mode 100644
index 0000000000000..62e36fb039a38
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/devirt_function_alias.ll
@@ -0,0 +1,106 @@
+; REQUIRES: x86-registered-target
+
+;; Ensure we don't incorrectly devirtualization when one vtable contains an
+;; alias (i.e. ensure analysis does not improperly ignore this implementation).
+
+;; Test pure ThinLTO
+
+;; Generate unsplit module with summary for ThinLTO index-based WPD.
+; RUN: opt -thinlto-bc -o %t1.o %s
+
+;; Check that we have properly recorded the alias in the vtable summary.
+; RUN llvm-dis -o - %t1.o | FileCheck %s --check-prefix SUMMARY
+; SUMMARY: gv: (name: "_ZTV1D", {{.*}} vTableFuncs: ((virtFunc: ^[[ALIAS:([0-9]+)]], offset: 16))
+; SUMMARY: ^[[ALIAS]] = gv: (name: "_ZN1D1mEiAlias"
+
+; RUN: llvm-lto2 run %t1.o -save-temps -pass-remarks=. \
+; RUN: -whole-program-visibility \
+; RUN: -wholeprogramdevirt-print-index-based \
+; RUN: -o %t2 \
+; RUN: -r=%t1.o,test,px \
+; RUN: -r=%t1.o,_ZTV1D,px \
+; RUN: -r=%t1.o,_ZN1D1mEi,px \
+; RUN: -r=%t1.o,_ZN1D1mEiAlias,px \
+; RUN: -r=%t1.o,_ZTV1B,px \
+; RUN: -r=%t1.o,_ZN1B1mEi,px \
+; RUN: 2>&1 | FileCheck %s --implicit-check-not {{[Dd]}}evirtualized --allow-empty
+; RUN: llvm-dis %t2.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
+
+;; Test hybrid Thin/Regular LTO
+
+;; Generate split module with summary for hybrid Thin/Regular LTO WPD.
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t3.o %s
+
+; RUN: llvm-lto2 run %t3.o -save-temps -pass-remarks=. \
+; RUN: -whole-program-visibility \
+; RUN: -o %t4 \
+; RUN: -r=%t3.o,test,px \
+; RUN: -r=%t3.o,_ZTV1D, \
+; RUN: -r=%t3.o,_ZTV1D,px \
+; RUN: -r=%t3.o,_ZN1D1mEi,px \
+; RUN: -r=%t3.o,_ZN1D1mEiAlias,px \
+; RUN: -r=%t3.o,_ZN1D1mEiAlias, \
+; RUN: -r=%t3.o,_ZTV1B, \
+; RUN: -r=%t3.o,_ZTV1B,px \
+; RUN: -r=%t3.o,_ZN1B1mEi,px \
+; RUN: -r=%t3.o,_ZN1B1mEi, \
+; RUN: 2>&1 | FileCheck %s --implicit-check-not {{[Dd]}}evirtualized --allow-empty
+; RUN: llvm-dis %t4.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
+
+;; Test Regular LTO
+
+; RUN: opt -o %t5.o %s
+; RUN: llvm-lto2 run %t5.o -save-temps -pass-remarks=. \
+; RUN: -whole-program-visibility \
+; RUN: -o %t6 \
+; RUN: -r=%t5.o,test,px \
+; RUN: -r=%t5.o,_ZTV1D,px \
+; RUN: -r=%t5.o,_ZN1D1mEi,px \
+; RUN: -r=%t5.o,_ZN1D1mEiAlias,px \
+; RUN: -r=%t5.o,_ZTV1B,px \
+; RUN: -r=%t5.o,_ZN1B1mEi,px \
+; RUN: 2>&1 | FileCheck %s --implicit-check-not {{[Dd]}}evirtualized --allow-empty
+; RUN: llvm-dis %t6.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
+
+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"
+
+%struct.D = type { ptr }
+%struct.B = type { %struct.D }
+
+ at _ZTV1D = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1D1mEiAlias] }, !type !0
+ at _ZTV1B = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1B1mEi] }, !type !0, !type !1
+
+
+define i32 @_ZN1D1mEi(ptr %this, i32 %a) {
+ ret i32 0;
+}
+
+ at _ZN1D1mEiAlias = unnamed_addr alias i32 (ptr, i32), ptr @_ZN1D1mEi
+
+define i32 @_ZN1B1mEi(ptr %this, i32 %a) {
+ ret i32 0;
+}
+
+; CHECK-IR-LABEL: define i32 @test
+define i32 @test(ptr %obj2, i32 %a) {
+entry:
+ %vtable2 = load ptr, ptr %obj2
+ %p2 = call i1 @llvm.type.test(ptr %vtable2, metadata !"_ZTS1D")
+ call void @llvm.assume(i1 %p2)
+
+ %fptr33 = load ptr, ptr %vtable2, align 8
+
+ ;; Confirm the call was not devirtualized.
+ ;; CHECK-IR: %call4 = tail call i32 %fptr33
+ %call4 = tail call i32 %fptr33(ptr nonnull %obj2, i32 %a)
+ ret i32 %call4
+}
+; CHECK-IR-LABEL: ret i32
+; CHECK-IR-LABEL: }
+
+declare i1 @llvm.type.test(ptr, metadata)
+declare void @llvm.assume(i1)
+
+!0 = !{i64 16, !"_ZTS1D"}
+!1 = !{i64 16, !"_ZTS1B"}
diff --git a/llvm/test/ThinLTO/X86/devirt_function_alias2.ll b/llvm/test/ThinLTO/X86/devirt_function_alias2.ll
new file mode 100644
index 0000000000000..5c430950b761e
--- /dev/null
+++ b/llvm/test/ThinLTO/X86/devirt_function_alias2.ll
@@ -0,0 +1,94 @@
+; REQUIRES: x86-registered-target
+
+;; Check for successful devirtualization when vtable contains an alias,
+;; and there is a single implementation.
+
+;; Test pure ThinLTO
+
+;; Generate unsplit module with summary for ThinLTO index-based WPD.
+; RUN: opt -thinlto-bc -o %t1.o %s
+
+;; Check that we have properly recorded the alias in the vtable summary.
+; RUN: llvm-dis -o - %t1.o | FileCheck %s --check-prefix SUMMARY
+; SUMMARY: gv: (name: "_ZTV1D", {{.*}} vTableFuncs: ((virtFunc: ^[[ALIAS:([0-9]+)]], offset: 16))
+; SUMMARY: ^[[ALIAS]] = gv: (name: "_ZN1D1mEiAlias"
+
+; RUN: llvm-lto2 run %t1.o -save-temps -pass-remarks=. \
+; RUN: -whole-program-visibility \
+; RUN: -wholeprogramdevirt-print-index-based \
+; RUN: -o %t2 \
+; RUN: -r=%t1.o,test,px \
+; RUN: -r=%t1.o,_ZTV1D,px \
+; RUN: -r=%t1.o,_ZN1D1mEi,px \
+; RUN: -r=%t1.o,_ZN1D1mEiAlias,px \
+; RUN: 2>&1 | FileCheck %s --check-prefix=REMARK --check-prefix=PRINT
+; RUN: llvm-dis %t2.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR1
+
+; PRINT-DAG: Devirtualized call to {{.*}} (_ZN1D1mEiAlias)
+; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEiAlias
+
+;; Test hybrid Thin/Regular LTO
+
+;; Generate split module with summary for hybrid Thin/Regular LTO WPD.
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t3.o %s
+
+; RUN: llvm-lto2 run %t3.o -save-temps -pass-remarks=. \
+; RUN: -whole-program-visibility \
+; RUN: -o %t4 \
+; RUN: -r=%t3.o,test,px \
+; RUN: -r=%t3.o,_ZTV1D, \
+; RUN: -r=%t3.o,_ZTV1D,px \
+; RUN: -r=%t3.o,_ZN1D1mEi,px \
+; RUN: -r=%t3.o,_ZN1D1mEiAlias,px \
+; RUN: -r=%t3.o,_ZN1D1mEiAlias, \
+; RUN: 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN: llvm-dis %t4.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR1
+
+;; TODO: Enable the below lines once Regular LTO support for devirtualizing
+;; aliases in the vtable is supported.
+;; Test Regular LTO
+; RUN opt -o %t5.o %s
+; RUN llvm-lto2 run %t5.o -save-temps -pass-remarks=. \
+; RUN -whole-program-visibility \
+; RUN -o %t6 \
+; RUN -r=%t5.o,test,px \
+; RUN -r=%t5.o,_ZTV1D,px \
+; RUN -r=%t5.o,_ZN1D1mEi,px \
+; RUN -r=%t5.o,_ZN1D1mEiAlias,px \
+; RUN 2>&1 | FileCheck %s --check-prefix=REMARK
+; RUN llvm-dis %t6.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR1
+
+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"
+
+%struct.D = type { ptr }
+
+ at _ZTV1D = constant { [3 x ptr] } { [3 x ptr] [ptr null, ptr undef, ptr @_ZN1D1mEiAlias] }, !type !3
+
+define i32 @_ZN1D1mEi(ptr %this, i32 %a) {
+ ret i32 0;
+}
+
+ at _ZN1D1mEiAlias = unnamed_addr alias i32 (ptr, i32), ptr @_ZN1D1mEi
+
+; CHECK-IR1-LABEL: define i32 @test
+define i32 @test(ptr %obj2, i32 %a) {
+entry:
+ %vtable2 = load ptr, ptr %obj2
+ %p2 = call i1 @llvm.type.test(ptr %vtable2, metadata !"_ZTS1D")
+ call void @llvm.assume(i1 %p2)
+
+ %fptr33 = load ptr, ptr %vtable2, align 8
+
+ ;; Check that the call was devirtualized.
+ ;; CHECK-IR1: %call4 = tail call i32 @_ZN1D1mEi
+ %call4 = tail call i32 %fptr33(ptr nonnull %obj2, i32 %a)
+ ret i32 %call4
+}
+; CHECK-IR1-LABEL: ret i32
+; CHECK-IR1-LABEL: }
+
+declare i1 @llvm.type.test(ptr, metadata)
+declare void @llvm.assume(i1)
+
+!3 = !{i64 16, !"_ZTS1D"}
More information about the llvm-commits
mailing list