From 830c0d493aee2f86a4e8e0e6711df93f53399364 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Sun, 10 Nov 2024 11:34:30 -0800 Subject: [PATCH 1/4] [WPD]Regard unreachable function as a possible devirtualizable target --- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp | 17 +++++++++++++++++ ...ngle_after_filtering_unreachable_function.ll | 12 ++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp index 78516cadcf231..f72cffcf75d2d 100644 --- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -167,6 +167,18 @@ static cl::list cl::desc("Prevent function(s) from being devirtualized"), cl::Hidden, cl::CommaSeparated); +/// A function is unreachable if its entry block ends with 'unreachable' IR +/// instruction. 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 different topic). Regard +/// unreachable function as possible devirtualize targets to keep the program +/// behavior. +static cl::opt 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 +398,9 @@ template <> struct DenseMapInfo { // 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 +2256,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 457120b9c6f41..599d3296bb163 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. ; 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: :0:0: devirtualized _ZN7DerivedD0Ev +; DEVIRT: remark: tmp.cc:21:3: single-impl: devirtualized a call to _ZN7DerivedD0Ev +; DEVIRT: remark: :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" From eaa1f19a675b7c15ad48bdfcad98d8649974c9ba Mon Sep 17 00:00:00 2001 From: mingmingl Date: Tue, 12 Nov 2024 21:06:43 -0800 Subject: [PATCH 2/4] resolve review feedback --- .../lib/Transforms/IPO/WholeProgramDevirt.cpp | 24 +++++++++++++------ ...le_after_filtering_unreachable_function.ll | 2 +- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp index f72cffcf75d2d..c5dc147888dce 100644 --- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -167,13 +167,23 @@ static cl::list cl::desc("Prevent function(s) from being devirtualized"), cl::Hidden, cl::CommaSeparated); -/// A function is unreachable if its entry block ends with 'unreachable' IR -/// instruction. 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 different topic). Regard -/// unreachable function as possible devirtualize targets to keep the program -/// behavior. +/// 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 different topic). +/// +/// This option is enabled to keep unreachable function as possible devirtualize +/// targets to conservatively keep the program behavior. +/// +/// TODO: Make a pure virtual class's deleting destructor precisely identifiable +/// in LLVM for more devirtualization. static cl::opt WholeProgramDevirtKeepUnreachableFunction( "wholeprogramdevirt-keep-unreachable-function", cl::desc("Regard unreachable functions as possible devirtualize targets."), 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 599d3296bb163..1e420f13ae938 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 @@ -4,7 +4,7 @@ ; 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 From f1a6f73910e205021c7133b18ecbb95e79f0ae72 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Tue, 12 Nov 2024 21:11:44 -0800 Subject: [PATCH 3/4] fix typos to keep singular / plural consistent --- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp index c5dc147888dce..6007b2b034f84 100644 --- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -179,8 +179,8 @@ static cl::list /// such functions to report failures (whether/when it's a good practice or not /// is a different topic). /// -/// This option is enabled to keep unreachable function as possible devirtualize -/// targets to conservatively keep the program behavior. +/// 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 LLVM for more devirtualization. From 5cf173a834d10f6cfc0a738df05e84380e80cc37 Mon Sep 17 00:00:00 2001 From: mingmingl Date: Tue, 12 Nov 2024 21:13:26 -0800 Subject: [PATCH 4/4] polish comment --- llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp index 6007b2b034f84..2f171c3c981d4 100644 --- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -183,7 +183,7 @@ static cl::list /// devirtualize target to conservatively keep the program behavior. /// /// TODO: Make a pure virtual class's deleting destructor precisely identifiable -/// in LLVM for more devirtualization. +/// in Clang's codegen for more devirtualization in LLVM. static cl::opt WholeProgramDevirtKeepUnreachableFunction( "wholeprogramdevirt-keep-unreachable-function", cl::desc("Regard unreachable functions as possible devirtualize targets."),