-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WPD][ThinLTO]Add cutoff option for WPD #113383
Conversation
@llvm/pr-subscribers-llvm-transforms Author: Mingming Liu (minglotus-6) ChangesThis option applies for import WPD (i.e., when Full diff: https://github.com/llvm/llvm-project/pull/113383.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
index 36a1841b363463..f193bb11a06319 100644
--- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
+++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
@@ -168,6 +168,14 @@ static cl::list<std::string>
cl::desc("Prevent function(s) from being devirtualized"),
cl::Hidden, cl::CommaSeparated);
+/// If this value is other than 0, the devirt module pass will stop
+/// transformation once the total number of devirtualizations reach the cutoff
+/// value.
+static cl::opt<unsigned> WholeProgramDevirtCutoff(
+ "wholeprogramdevirt-cutoff",
+ cl::desc("Max number of devirtualization for devirt module pass"),
+ cl::init(0));
+
/// Mechanism to add runtime checking of devirtualization decisions, optionally
/// trapping or falling back to indirect call on any that are not correct.
/// Trapping mode is useful for debugging undefined behavior leading to failures
@@ -1169,6 +1177,10 @@ void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo,
if (!OptimizedCalls.insert(&VCallSite.CB).second)
continue;
+ if (WholeProgramDevirtCutoff.getNumOccurrences() > 0 &&
+ NumSingleImpl >= WholeProgramDevirtCutoff)
+ return;
+
if (RemarksEnabled)
VCallSite.emitRemark("single-impl",
TheFn->stripPointerCasts()->getName(), OREGetter);
diff --git a/llvm/test/Transforms/WholeProgramDevirt/import.ll b/llvm/test/Transforms/WholeProgramDevirt/import.ll
index 4e4df5f35a777d..de25bc10a7c123 100644
--- a/llvm/test/Transforms/WholeProgramDevirt/import.ll
+++ b/llvm/test/Transforms/WholeProgramDevirt/import.ll
@@ -8,6 +8,12 @@
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -wholeprogramdevirt-read-summary=%S/Inputs/import-vcp-branch-funnel.yaml < %s | FileCheck --check-prefixes=CHECK,VCP,VCP-X86,VCP64,BRANCH-FUNNEL %s
; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -wholeprogramdevirt-read-summary=%S/Inputs/import-branch-funnel.yaml < %s | FileCheck --check-prefixes=CHECK,BRANCH-FUNNEL,BRANCH-FUNNEL-NOVCP %s
+; Cutoff value is not explicitly set. Expect 3 remark messages.
+; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml < %s 2>&1 | grep "single-impl" | count 3
+; Cutoff value is set to 1. Expect one remark messages.
+; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-cutoff=1 -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml < %s 2>&1 | grep "single-impl" | count 1
+; Cutoff value is explicitly set to zero. Expect no remark message.
+; RUN: opt -S -passes=wholeprogramdevirt -wholeprogramdevirt-summary-action=import -pass-remarks=wholeprogramdevirt -wholeprogramdevirt-cutoff=0 -wholeprogramdevirt-read-summary=%S/Inputs/import-single-impl.yaml < %s 2>&1 | FileCheck -implicit-check-not="remark" %s
target datalayout = "e-p:64:64"
target triple = "x86_64-unknown-linux-gnu"
|
/// value. | ||
static cl::opt<unsigned> WholeProgramDevirtCutoff( | ||
"wholeprogramdevirt-cutoff", | ||
cl::desc("Max number of devirtualization for devirt module pass"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/devirtualization/devirtualizations/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@@ -1169,6 +1177,10 @@ void DevirtModule::applySingleImplDevirt(VTableSlotInfo &SlotInfo, | |||
if (!OptimizedCalls.insert(&VCallSite.CB).second) | |||
continue; | |||
|
|||
if (WholeProgramDevirtCutoff.getNumOccurrences() > 0 && | |||
NumSingleImpl >= WholeProgramDevirtCutoff) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NumSingleImpl is a STATISTIC - I'm not sure if it is safe to use this here as I believe the compiler can be built with statistics disabled, in which case this variable would not be correctly tracking the number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the catch!
I added static unsigned NumDevirtCalls
to track the number of de-virtualized calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the updated PR (using static unsigned NumDevirtCalls
) using clang and verified the new option works
-wholeprogramdevirt-cutoff=0
gives no devirtualization-wholeprogramdevirt-cutoff=1
allows at most one in one clang invocation (and catches the potentially interesting call-site)
@@ -168,6 +168,14 @@ static cl::list<std::string> | |||
cl::desc("Prevent function(s) from being devirtualized"), | |||
cl::Hidden, cl::CommaSeparated); | |||
|
|||
/// If this value is other than 0, the devirt module pass will stop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the way the code is written, value 0 is not special and will cause no devirts to happen - assuming the option was specified (i.e. getNumOccurrences() > 0). And in fact we probably want a way to say "do 0 devirtualizations".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the comment, and yes it makes sense to do 0 devirtualization when this option is explicitly set to zero.
This option applies for _import_ WPD (i.e., when `DevirtModule` pass de-virtualizes according to an imported summary, in ThinLTO backend pipeline). It's meant for debugging (e.g., bisection).
This option applies for import WPD (i.e., when
DevirtModule
pass de-virtualizes according to an imported summary, in ThinLTO backend pipeline). It's meant for debugging (e.g., bisection).