[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Leonard Chan via Phabricator via cfe-commits]

leonardchan abandoned this revision.
leonardchan added a comment.

In D76389#2120868 , @arsenm wrote:

> This seems like it covers a different case than D82735 
> ?


I haven't rebased in a while, but D82735  
covers this. Abandoning since this is addressed in that patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

This seems like it covers a different case than D82735 
?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 257800.
leonardchan marked an inline comment as done.
leonardchan edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389

Files:
  llvm/lib/Passes/PassBuilder.cpp


Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -440,7 +440,7 @@
 
   // Speculative execution if the target has divergent branches; otherwise nop.
   if (Level.getSpeedupLevel() > 1) {
-FPM.addPass(SpeculativeExecutionPass());
+FPM.addPass(SpeculativeExecutionPass(/*OnlyIfDivergentTarget=*/true));
 
 // Optimize based on known information about branches, and cleanup 
afterward.
 FPM.addPass(JumpThreadingPass());


Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -440,7 +440,7 @@
 
   // Speculative execution if the target has divergent branches; otherwise nop.
   if (Level.getSpeedupLevel() > 1) {
-FPM.addPass(SpeculativeExecutionPass());
+FPM.addPass(SpeculativeExecutionPass(/*OnlyIfDivergentTarget=*/true));
 
 // Optimize based on known information about branches, and cleanup afterward.
 FPM.addPass(JumpThreadingPass());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: llvm/include/llvm/Transforms/Scalar/SpeculativeExecution.h:97
+
+class SpeculativeExecutionIfHasBranchDivergencePass
+: public SpeculativeExecutionPassImpl,

Still define a second pass



Comment at: llvm/lib/Passes/PassBuilder.cpp:435
   if (Level.getSpeedupLevel() > 1) {
-FPM.addPass(SpeculativeExecutionPass());
+FPM.addPass(SpeculativeExecutionIfHasBranchDivergencePass());
 

leonardchan wrote:
> arsenm wrote:
> > I don't expect this to be a separate pass, just added based on a divergent 
> > target
> Updated such that the current pass is used, although it's a bit more 
> difficult to test since the same pass name will appear when dumping the 
> passes.
There's a check for divergent branches in the builder so you don't need to run 
the pass at all


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Leonard Chan via Phabricator via cfe-commits]

leonardchan marked an inline comment as done.
leonardchan added inline comments.



Comment at: llvm/lib/Passes/PassBuilder.cpp:435
   if (Level.getSpeedupLevel() > 1) {
-FPM.addPass(SpeculativeExecutionPass());
+FPM.addPass(SpeculativeExecutionIfHasBranchDivergencePass());
 

arsenm wrote:
> I don't expect this to be a separate pass, just added based on a divergent 
> target
Updated such that the current pass is used, although it's a bit more difficult 
to test since the same pass name will appear when dumping the passes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 255474.
leonardchan edited the summary of this revision.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389

Files:
  llvm/include/llvm/Transforms/Scalar/SpeculativeExecution.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

Index: llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
===
--- llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
+++ llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp
@@ -123,7 +123,7 @@
   // Variable preserved purely for correct name printing.
   const bool OnlyIfDivergentTarget;
 
-  SpeculativeExecutionPass Impl;
+  SpeculativeExecutionPassImpl Impl;
 };
 } // namespace
 
@@ -150,7 +150,8 @@
 
 namespace llvm {
 
-bool SpeculativeExecutionPass::runImpl(Function &F, TargetTransformInfo *TTI) {
+bool SpeculativeExecutionPassImpl::runImpl(Function &F,
+   TargetTransformInfo *TTI) {
   if (OnlyIfDivergentTarget && !TTI->hasBranchDivergence()) {
 LLVM_DEBUG(dbgs() << "Not running SpeculativeExecution because "
  "TTI->hasBranchDivergence() is false.\n");
@@ -165,7 +166,7 @@
   return Changed;
 }
 
-bool SpeculativeExecutionPass::runOnBasicBlock(BasicBlock &B) {
+bool SpeculativeExecutionPassImpl::runOnBasicBlock(BasicBlock &B) {
   BranchInst *BI = dyn_cast(B.getTerminator());
   if (BI == nullptr)
 return false;
@@ -251,8 +252,8 @@
   }
 }
 
-bool SpeculativeExecutionPass::considerHoistingFromTo(
-BasicBlock &FromBlock, BasicBlock &ToBlock) {
+bool SpeculativeExecutionPassImpl::considerHoistingFromTo(BasicBlock &FromBlock,
+  BasicBlock &ToBlock) {
   SmallPtrSet NotHoisted;
   const auto AllPrecedingUsesFromBlockHoisted = [&NotHoisted](User *U) {
 for (Value* V : U->operand_values()) {
@@ -299,12 +300,13 @@
   return new SpeculativeExecutionLegacyPass(/* OnlyIfDivergentTarget = */ true);
 }
 
-SpeculativeExecutionPass::SpeculativeExecutionPass(bool OnlyIfDivergentTarget)
+SpeculativeExecutionPassImpl::SpeculativeExecutionPassImpl(
+bool OnlyIfDivergentTarget)
 : OnlyIfDivergentTarget(OnlyIfDivergentTarget ||
 SpecExecOnlyIfDivergentTarget) {}
 
-PreservedAnalyses SpeculativeExecutionPass::run(Function &F,
-FunctionAnalysisManager &AM) {
+PreservedAnalyses
+SpeculativeExecutionPassImpl::run(Function &F, FunctionAnalysisManager &AM) {
   auto *TTI = &AM.getResult(F);
 
   bool Changed = runImpl(F, TTI);
Index: llvm/lib/Passes/PassBuilder.cpp
===
--- llvm/lib/Passes/PassBuilder.cpp
+++ llvm/lib/Passes/PassBuilder.cpp
@@ -440,7 +440,7 @@
 
   // Speculative execution if the target has divergent branches; otherwise nop.
   if (Level.getSpeedupLevel() > 1) {
-FPM.addPass(SpeculativeExecutionPass());
+FPM.addPass(SpeculativeExecutionPass(/*OnlyIfDivergentTarget=*/true));
 
 // Optimize based on known information about branches, and cleanup afterward.
 FPM.addPass(JumpThreadingPass());
Index: llvm/include/llvm/Transforms/Scalar/SpeculativeExecution.h
===
--- llvm/include/llvm/Transforms/Scalar/SpeculativeExecution.h
+++ llvm/include/llvm/Transforms/Scalar/SpeculativeExecution.h
@@ -66,10 +66,9 @@
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
-class SpeculativeExecutionPass
-: public PassInfoMixin {
+class SpeculativeExecutionPassImpl {
 public:
-  SpeculativeExecutionPass(bool OnlyIfDivergentTarget = false);
+  SpeculativeExecutionPassImpl(bool OnlyIfDivergentTarget = false);
 
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
 
@@ -86,6 +85,22 @@
 
   TargetTransformInfo *TTI = nullptr;
 };
+
+class SpeculativeExecutionPass
+: public SpeculativeExecutionPassImpl,
+  public PassInfoMixin {
+public:
+  SpeculativeExecutionPass(bool OnlyIfDivergentTarget = false)
+  : SpeculativeExecutionPassImpl(OnlyIfDivergentTarget) {}
+};
+
+class SpeculativeExecutionIfHasBranchDivergencePass
+: public SpeculativeExecutionPassImpl,
+  public PassInfoMixin {
+public:
+  SpeculativeExecutionIfHasBranchDivergencePass()
+  : SpeculativeExecutionPassImpl(/*OnlyIfDivergentTarget=*/true) {}
+};
 }
 
 #endif //LLVM_TRANSFORMS_SCALAR_SPECULATIVEEXECUTION_H
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/test/CodeGen/thinlto-distributed-newpm.ll:110
 ; CHECK-O: Running analysis: OuterAnalysisManagerProxy
-; CHECK-O: Running pass: SpeculativeExecutionPass on main
+; CHECK-O: Running pass: SpeculativeExecutionIfHasBranchDivergencePass on main
 ; CHECK-O: Running pass: JumpThreadingPass on main

The pass name shouldn't change here?



Comment at: llvm/lib/Passes/PassBuilder.cpp:435
   if (Level.getSpeedupLevel() > 1) {
-FPM.addPass(SpeculativeExecutionPass());
+FPM.addPass(SpeculativeExecutionIfHasBranchDivergencePass());
 

I don't expect this to be a separate pass, just added based on a divergent 
target


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76389: [NewPM] Run the Speculative Execution Pass only if the target has divergent branches

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added a comment.

In D76389#1951135 , @arsenm wrote:

> Commit message should say only if?


Updated


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76389/new/

https://reviews.llvm.org/D76389



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits