[llvm] 47cc9db - [WPD]Regard unreachable function as a possible devirtualizable target (#115668)

[via llvm-commits]

Author: Mingming Liu
Date: 2024-11-13T11:28:36-08:00
New Revision: 47cc9db797b1e1da94af91cf3d0f2999d11c1cbc

URL: https://github.com/llvm/llvm-project/commit/47cc9db797b1e1da94af91cf3d0f2999d11c1cbc
DIFF: https://github.com/llvm/llvm-project/commit/47cc9db797b1e1da94af91cf3d0f2999d11c1cbc.diff

LOG: [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

Added: 
    

Modified: 
    llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
    llvm/test/Transforms/WholeProgramDevirt/devirt_single_after_filtering_unreachable_function.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index 78516cadcf2313..2f171c3c981d40 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -167,6 +167,28 @@ static cl::list<std::string>
                       cl::desc("Prevent function(s) from being devirtualized"),
                       cl::Hidden, cl::CommaSeparated);
 
+/// With Clang, a pure virtual class's deleting destructor is emitted as a
+/// `llvm.trap` intrinsic followed by an unreachable IR instruction. In the
+/// context of whole program devirtualization, the deleting destructor of a pure
+/// virtual class won't be invoked by the source code so safe to skip as a
+/// devirtualize target.
+///
+/// However, not all unreachable functions are safe to skip. In some cases, the
+/// program intends to run such functions and terminate, for instance, a unit
+/// test may run a death test. A non-test program might (or allowed to) invoke
+/// such functions to report failures (whether/when it's a good practice or not
+/// is a 
diff erent topic).
+///
+/// This option is enabled to keep an unreachable function as a possible
+/// devirtualize target to conservatively keep the program behavior.
+///
+/// TODO: Make a pure virtual class's deleting destructor precisely identifiable
+/// in Clang's codegen for more devirtualization in LLVM.
+static cl::opt<bool> WholeProgramDevirtKeepUnreachableFunction(
+    "wholeprogramdevirt-keep-unreachable-function",
+    cl::desc("Regard unreachable functions as possible devirtualize targets."),
+    cl::Hidden, cl::init(true));
+
 /// If explicitly specified, the devirt module pass will stop transformation
 /// once the total number of devirtualizations reach the cutoff value. Setting
 /// this option to 0 explicitly will do 0 devirtualization.
@@ -386,6 +408,9 @@ template <> struct DenseMapInfo<VTableSlotSummary> {
 //   2) All function summaries indicate it's unreachable
 //   3) There is no non-function with the same GUID (which is rare)
 static bool mustBeUnreachableFunction(ValueInfo TheFnVI) {
+  if (WholeProgramDevirtKeepUnreachableFunction)
+    return false;
+
   if ((!TheFnVI) || TheFnVI.getSummaryList().empty()) {
     // Returns false if ValueInfo is absent, or the summary list is empty
     // (e.g., function declarations).
@@ -2241,6 +2266,8 @@ DevirtModule::lookUpFunctionValueInfo(Function *TheFn,
 
 bool DevirtModule::mustBeUnreachableFunction(
     Function *const F, ModuleSummaryIndex *ExportSummary) {
+  if (WholeProgramDevirtKeepUnreachableFunction)
+    return false;
   // First, learn unreachability by analyzing function IR.
   if (!F->isDeclaration()) {
     // A function must be unreachable if its entry block ends with an

diff  --git a/llvm/test/Transforms/WholeProgramDevirt/devirt_single_after_filtering_unreachable_function.ll b/llvm/test/Transforms/WholeProgramDevirt/devirt_single_after_filtering_unreachable_function.ll
index 457120b9c6f410..1e420f13ae938c 100644
--- a/llvm/test/Transforms/WholeProgramDevirt/devirt_single_after_filtering_unreachable_function.ll
+++ b/llvm/test/Transforms/WholeProgramDevirt/devirt_single_after_filtering_unreachable_function.ll
@@ -1,11 +1,15 @@
+; Test that static devirtualization doesn't happen because there are two
+; devirtualizable targets. Unreachable functions are kept in the devirtualizable
+; target set by default.
+; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt %s 2>&1 | FileCheck %s  --implicit-check-not="single-impl"
+
 ; Test that regular LTO will analyze IR, detect unreachable functions and discard unreachable functions
-; when finding virtual call targets.
+; when finding virtual call targets under `wholeprogramdevirt-keep-unreachable-function=false` option.
 ; In this test case, the unreachable function is the virtual deleting destructor of an abstract class.
+; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-keep-unreachable-function=false %s 2>&1 | FileCheck %s --check-prefix=DEVIRT
 
-; RUN: opt -S -passes=wholeprogramdevirt -whole-program-visibility -pass-remarks=wholeprogramdevirt %s 2>&1 | FileCheck %s
-
-; CHECK: remark: tmp.cc:21:3: single-impl: devirtualized a call to _ZN7DerivedD0Ev
-; CHECK: remark: <unknown>:0:0: devirtualized _ZN7DerivedD0Ev
+; DEVIRT: remark: tmp.cc:21:3: single-impl: devirtualized a call to _ZN7DerivedD0Ev
+; DEVIRT: remark: <unknown>:0:0: devirtualized _ZN7DerivedD0Ev
 
 source_filename = "tmp.cc"
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"