[SimplifyCFG] Add option to not speculate blocks

[aeubanks]

Summary

Required for phase ordering changes to not regress Rust code with D145265.

[aeubanks]

fixes both regressions reported in D145265

edit: or rather, D153392 does

[nikic]

llvm/test/Transforms/SimplifyCFG/speculate-blocks.ll

80

This test can be much simpler. You don't really need more than if (c) { v = inst } phi(v, 0) to test this.

[aeubanks]

simplify test

[nikic]

LGTM

[vitalybuka]

Probably caused by the patch https://lab.llvm.org/buildbot/#/builders/77/builds/27866

[aeubanks]

In D153391#4442049, @vitalybuka wrote:

Probably caused by the patch https://lab.llvm.org/buildbot/#/builders/77/builds/27866

this patch doesn't actually change any behavior in any of the pipelines

[vitalybuka]

In D153391#4442057, @aeubanks wrote:

In D153391#4442049, @vitalybuka wrote:

Probably caused by the patch https://lab.llvm.org/buildbot/#/builders/77/builds/27866

this patch doesn't actually change any behavior in any of the pipelines

D153468 maybe

[aeubanks]

In D153391#4442059, @vitalybuka wrote:

In D153391#4442057, @aeubanks wrote:

In D153391#4442049, @vitalybuka wrote:

Probably caused by the patch https://lab.llvm.org/buildbot/#/builders/77/builds/27866

this patch doesn't actually change any behavior in any of the pipelines

D153468 maybe

that's definitely a no-op even if you pass the flag that actually enables more verification

[vitalybuka]

In D153391#4442080, @aeubanks wrote:

In D153391#4442059, @vitalybuka wrote:

In D153391#4442057, @aeubanks wrote:

In D153391#4442049, @vitalybuka wrote:

Probably caused by the patch https://lab.llvm.org/buildbot/#/builders/77/builds/27866

this patch doesn't actually change any behavior in any of the pipelines

D153468 maybe

that's definitely a no-op even if you pass the flag that actually enables more verification

Looks like it was unrelated flake which failed few times in a row. We will look how to improve that.
Sorry for the noise.

llvm/include/llvm/Transforms/Utils/SimplifyCFGOptions.h

struct SimplifyCFGOptions {
   int BonusInstThreshold = 1;
   bool ForwardSwitchCondToPhi = false;
   bool ConvertSwitchRangeToICmp = false;
   bool ConvertSwitchToLookupTable = false;
   bool NeedCanonicalLoop = true;
   bool HoistCommonInsts = false;
   bool SinkCommonInsts = false;
   bool SimplifyCondBranch = true;
-  bool FoldTwoEntryPHINode = true;
+  bool SpeculateBlocks = true;
 
   AssumptionCache *AC = nullptr;
 
   // Support 'builder' pattern to set members by name at construction time.
   SimplifyCFGOptions &bonusInstThreshold(int I) {
     BonusInstThreshold = I;
     return *this;
   }
SimplifyCFGOptions &setAssumptionCache(AssumptionCache *Cache) {
     AC = Cache;
     return *this;
   }
   SimplifyCFGOptions &setSimplifyCondBranch(bool B) {
     SimplifyCondBranch = B;
     return *this;
   }
 
-  SimplifyCFGOptions &setFoldTwoEntryPHINode(bool B) {
-    FoldTwoEntryPHINode = B;
+  SimplifyCFGOptions &speculateBlocks(bool B) {
+    SpeculateBlocks = B;
     return *this;
   }
 };
 
 } // namespace llvm
 
 #endif // LLVM_TRANSFORMS_UTILS_SIMPLIFYCFGOPTIONS_H

llvm/lib/Passes/PassBuilder.cpp

 /// Parser of parameters for SimplifyCFG pass.
 Expected<SimplifyCFGOptions> parseSimplifyCFGOptions(StringRef Params) {
   SimplifyCFGOptions Result;
   while (!Params.empty()) {
     StringRef ParamName;
     std::tie(ParamName, Params) = Params.split(';');
 
     bool Enable = !ParamName.consume_front("no-");
-    if (ParamName == "fold-two-entry-phi") {
-      Result.setFoldTwoEntryPHINode(Enable);
+    if (ParamName == "speculate-blocks") {
+      Result.speculateBlocks(Enable);
     } else if (ParamName == "simplify-cond-branch") {
       Result.setSimplifyCondBranch(Enable);
     } else if (ParamName == "forward-switch-cond") {
       Result.forwardSwitchCondToPhi(Enable);
     } else if (ParamName == "switch-range-to-icmp") {
       Result.convertSwitchRangeToICmp(Enable);
     } else if (ParamName == "switch-to-lookup") {
       Result.convertSwitchToLookupTable(Enable);

llvm/lib/Transforms/Scalar/SimplifyCFGPass.cpp

void SimplifyCFGPass::printPipeline(
   OS << (Options.ForwardSwitchCondToPhi ? "" : "no-") << "forward-switch-cond;";
   OS << (Options.ConvertSwitchRangeToICmp ? "" : "no-")
      << "switch-range-to-icmp;";
   OS << (Options.ConvertSwitchToLookupTable ? "" : "no-")
      << "switch-to-lookup;";
   OS << (Options.NeedCanonicalLoop ? "" : "no-") << "keep-loops;";
   OS << (Options.HoistCommonInsts ? "" : "no-") << "hoist-common-insts;";
   OS << (Options.SinkCommonInsts ? "" : "no-") << "sink-common-insts;";
-  OS << (Options.FoldTwoEntryPHINode ? "" : "no-") << "fold-two-entry-phi;";
+  OS << (Options.SpeculateBlocks ? "" : "no-") << "speculate-blocks;";
   OS << (Options.SimplifyCondBranch ? "" : "no-") << "simplify-cond-branch";
   OS << '>';
 }
 
 PreservedAnalyses SimplifyCFGPass::run(Function &F,
                                        FunctionAnalysisManager &AM) {
   auto &TTI = AM.getResult<TargetIRAnalysis>(F);
   Options.AC = &AM.getResult<AssumptionAnalysis>(F);

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

 ///     %sub = sub %x, %y
 ///     %cond = select i1 %cmp, 0, %sub
 ///     ...
 /// \endcode
 ///
 /// \returns true if the conditional block is removed.
 bool SimplifyCFGOpt::SpeculativelyExecuteBB(BranchInst *BI,
                                             BasicBlock *ThenBB) {
+  if (!Options.SpeculateBlocks)
+    return false;
+
   // Be conservative for now. FP select instruction can often be expensive.
   Value *BrCond = BI->getCondition();
   if (isa<FCmpInst>(BrCond))
     return false;
 
   BasicBlock *BB = BI->getParent();
   BasicBlock *EndBB = ThenBB->getTerminator()->getSuccessor(0);
   InstructionCost Budget =
if (SinkCommonCodeFromPredecessors(BB, DTU) ||
       // Let's rerun EliminateDuplicatePHINodes() first,
       // before FoldTwoEntryPHINode() potentially converts them into select's,
       // after which we'd need a whole EarlyCSE pass run to cleanup them.
       return true;
     }
 
   IRBuilder<> Builder(BB);
 
-  if (Options.FoldTwoEntryPHINode &&
+  if (Options.SpeculateBlocks &&
       !BB->getParent()->hasFnAttribute(Attribute::OptForFuzzing)) {
     // If there is a trivial two-entry PHI node in this basic block, and we can
     // eliminate it, do so now.
     if (auto *PN = dyn_cast<PHINode>(BB->begin()))
       if (PN->getNumIncomingValues() == 2)
         if (FoldTwoEntryPHINode(PN, TTI, DTU, DL))
           return true;
   }

llvm/test/Other/new-pm-print-pipeline.ll

 ; CHECK-14: hwasan<>,hwasan<kernel;recover>
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='module(loop-extract<>,loop-extract<single>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-16
 ; CHECK-16: loop-extract<>,loop-extract<single>
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(print<stack-lifetime><may>,print<stack-lifetime><must>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-17
 ; CHECK-17: function(print<stack-lifetime><may>,print<stack-lifetime><must>)
 
-; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;switch-to-lookup;keep-loops;hoist-common-insts;sink-common-insts;fold-two-entry-phi;simplify-cond-branch>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-sink-common-insts;no-fold-two-entry-phi;no-simplify-cond-branch>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-18
-; CHECK-18: function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;no-switch-range-to-icmp;switch-to-lookup;keep-loops;hoist-common-insts;sink-common-insts;fold-two-entry-phi;simplify-cond-branch>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-range-to-icmp;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-sink-common-insts;no-fold-two-entry-phi;no-simplify-cond-branch>)
+; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;switch-to-lookup;keep-loops;hoist-common-insts;sink-common-insts;speculate-blocks;simplify-cond-branch>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-sink-common-insts;no-speculate-blocks;no-simplify-cond-branch>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-18
+; CHECK-18: function(simplifycfg<bonus-inst-threshold=5;forward-switch-cond;no-switch-range-to-icmp;switch-to-lookup;keep-loops;hoist-common-insts;sink-common-insts;speculate-blocks;simplify-cond-branch>,simplifycfg<bonus-inst-threshold=7;no-forward-switch-cond;no-switch-range-to-icmp;no-switch-to-lookup;no-keep-loops;no-hoist-common-insts;no-sink-common-insts;no-speculate-blocks;no-simplify-cond-branch>)
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='function(loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only>,loop-vectorize<interleave-forced-only;vectorize-forced-only>)' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-19
 ; CHECK-19: function(loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only;>,loop-vectorize<interleave-forced-only;vectorize-forced-only;>)
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='inliner-wrapper,inliner-wrapper-no-mandatory-first' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-20
 ; CHECK-20: cgscc(inline<only-mandatory>,inline),cgscc(inline)
 
 ; RUN: opt -disable-output -disable-verify -print-pipeline-passes -passes='scc-oz-module-inliner' < %s | FileCheck %s --match-full-lines --check-prefixes=CHECK-21

llvm/test/Transforms/SimplifyCFG/speculate-blocks.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
+; RUN: opt < %s -S -passes='simplifycfg<speculate-blocks>' | FileCheck %s --check-prefix=YES
+; RUN: opt < %s -S -passes='simplifycfg<no-speculate-blocks>' | FileCheck %s --check-prefix=NO
+
+define i32 @f(i1 %a) {
+; YES-LABEL: define i32 @f
+; YES-SAME: (i1 [[A:%.*]]) {
+; YES-NEXT:  entry:
+; YES-NEXT:    [[SPEC_SELECT:%.*]] = select i1 [[A]], i32 5, i32 2
+; YES-NEXT:    ret i32 [[SPEC_SELECT]]
+;
+; NO-LABEL: define i32 @f
+; NO-SAME: (i1 [[A:%.*]]) {
+; NO-NEXT:  entry:
+; NO-NEXT:    br i1 [[A]], label [[BB:%.*]], label [[BB2:%.*]]
+; NO:       bb:
+; NO-NEXT:    br label [[BB2]]
+; NO:       bb2:
+; NO-NEXT:    [[R:%.*]] = phi i32 [ 2, [[ENTRY:%.*]] ], [ 5, [[BB]] ]
+; NO-NEXT:    ret i32 [[R]]
+;
+entry:
+  br i1 %a, label %bb, label %bb2
+bb:
+  br label %bb2
+bb2:
+  %r = phi i32 [ 2, %entry ], [ 5, %bb ]
+  ret i32 %r
+}