Skip to content
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

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Conversation

mingmingl-llvm
Copy link
Contributor

@mingmingl-llvm mingmingl-llvm commented Oct 22, 2024

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).

@llvmbot
Copy link
Member

llvmbot commented Oct 22, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Mingming Liu (minglotus-6)

Changes

This option applies for import WPD (i.e., when DevirtModule pass de-virtualizes according to an imported summary, in ThinLTO backend pipeline).


Full diff: https://github.com/llvm/llvm-project/pull/113383.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp (+12)
  • (modified) llvm/test/Transforms/WholeProgramDevirt/import.ll (+6)
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"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/devirtualization/devirtualizations/

Copy link
Contributor Author

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)
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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
Copy link
Contributor

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".

Copy link
Contributor Author

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.

@mingmingl-llvm mingmingl-llvm merged commit 6094417 into llvm:main Oct 24, 2024
8 checks passed
@mingmingl-llvm mingmingl-llvm deleted the wpdlimit branch October 24, 2024 06:47
@frobtech frobtech mentioned this pull request Oct 25, 2024
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants