Skip to content

Commit 47cc9db

Browse files
[WPD]Regard unreachable function as a possible devirtualizable target (#115668)
https://reviews.llvm.org/D115492 skips unreachable functions and potentially allows more static de-virtualizations. The motivation is to ignore virtual deleting destructor of abstract class (e.g., `Base::~Base()` in https://gcc.godbolt.org/z/dWMsdT9Kz). * Note WPD already handles most pure virtual functions (like `Base::x()` in the godbolt example above), which becomes a `__cxa_pure_virtual` in the vtable slot. This PR proposes to undo the change, because it turns out there are other unreachable functions that a general program wants to run and fail intentionally, with `LOG(FATAL)` or `CHECK` [1] for example. While many real-world applications are encouraged to check-fail sparingly, they are allowed to do so on critical errors (e.g., misconfiguration or bug is detected during server startup). * Implementation-wise, this PR keeps the one-bit 'unreachable' state in bitcode and updates WPD analysis. https://gcc.godbolt.org/z/T1aMhczYr is a minimum reproducible example extracted from unit test. `Base::func` is a one-liner of `LOG(FATAL) << "message"`, and lowered to one basic block ending with `unreachable`. A real-world program is _allowed_ to invoke Base::func to terminate the program as a way to report errors (in server initialization stage for example), even if errors on the serving path should be handled more gracefully. [1] https://abseil.io/docs/cpp/guides/logging#CHECK and https://abseil.io/docs/cpp/guides/logging#configuration-and-flags
1 parent 8ac6af2 commit 47cc9db

File tree

2 files changed

+36
-5
lines changed

2 files changed

+36
-5
lines changed

llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,28 @@ static cl::list<std::string>
167167
cl::desc("Prevent function(s) from being devirtualized"),
168168
cl::Hidden, cl::CommaSeparated);
169169

170+
/// With Clang, a pure virtual class's deleting destructor is emitted as a
171+
/// `llvm.trap` intrinsic followed by an unreachable IR instruction. In the
172+
/// context of whole program devirtualization, the deleting destructor of a pure
173+
/// virtual class won't be invoked by the source code so safe to skip as a
174+
/// devirtualize target.
175+
///
176+
/// However, not all unreachable functions are safe to skip. In some cases, the
177+
/// program intends to run such functions and terminate, for instance, a unit
178+
/// test may run a death test. A non-test program might (or allowed to) invoke
179+
/// such functions to report failures (whether/when it's a good practice or not
180+
/// is a different topic).
181+
///
182+
/// This option is enabled to keep an unreachable function as a possible
183+
/// devirtualize target to conservatively keep the program behavior.
184+
///
185+
/// TODO: Make a pure virtual class's deleting destructor precisely identifiable
186+
/// in Clang's codegen for more devirtualization in LLVM.
187+
static cl::opt<bool> WholeProgramDevirtKeepUnreachableFunction(
188+
"wholeprogramdevirt-keep-unreachable-function",
189+
cl::desc("Regard unreachable functions as possible devirtualize targets."),
190+
cl::Hidden, cl::init(true));
191+
170192
/// If explicitly specified, the devirt module pass will stop transformation
171193
/// once the total number of devirtualizations reach the cutoff value. Setting
172194
/// this option to 0 explicitly will do 0 devirtualization.
@@ -386,6 +408,9 @@ template <> struct DenseMapInfo<VTableSlotSummary> {
386408
// 2) All function summaries indicate it's unreachable
387409
// 3) There is no non-function with the same GUID (which is rare)
388410
static bool mustBeUnreachableFunction(ValueInfo TheFnVI) {
411+
if (WholeProgramDevirtKeepUnreachableFunction)
412+
return false;
413+
389414
if ((!TheFnVI) || TheFnVI.getSummaryList().empty()) {
390415
// Returns false if ValueInfo is absent, or the summary list is empty
391416
// (e.g., function declarations).
@@ -2241,6 +2266,8 @@ DevirtModule::lookUpFunctionValueInfo(Function *TheFn,
22412266

22422267
bool DevirtModule::mustBeUnreachableFunction(
22432268
Function *const F, ModuleSummaryIndex *ExportSummary) {
2269+
if (WholeProgramDevirtKeepUnreachableFunction)
2270+
return false;
22442271
// First, learn unreachability by analyzing function IR.
22452272
if (!F->isDeclaration()) {
22462273
// A function must be unreachable if its entry block ends with an

llvm/test/Transforms/WholeProgramDevirt/devirt_single_after_filtering_unreachable_function.ll

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
1+
; Test that static devirtualization doesn't happen because there are two
2+
; devirtualizable targets. Unreachable functions are kept in the devirtualizable
3+
; target set by default.
4+
; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt %s 2>&1 | FileCheck %s --implicit-check-not="single-impl"
5+
16
; Test that regular LTO will analyze IR, detect unreachable functions and discard unreachable functions
2-
; when finding virtual call targets.
7+
; when finding virtual call targets under `wholeprogramdevirt-keep-unreachable-function=false` option.
38
; In this test case, the unreachable function is the virtual deleting destructor of an abstract class.
9+
; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-keep-unreachable-function=false %s 2>&1 | FileCheck %s --check-prefix=DEVIRT
410

5-
; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt %s 2>&1 | FileCheck %s
6-
7-
; CHECK: remark: tmp.cc:21:3: single-impl: devirtualized a call to _ZN7DerivedD0Ev
8-
; CHECK: remark: <unknown>:0:0: devirtualized _ZN7DerivedD0Ev
11+
; DEVIRT: remark: tmp.cc:21:3: single-impl: devirtualized a call to _ZN7DerivedD0Ev
12+
; DEVIRT: remark: <unknown>:0:0: devirtualized _ZN7DerivedD0Ev
913

1014
source_filename = "tmp.cc"
1115
target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

0 commit comments

Comments
 (0)