[llvm] 05884d3 - Make FoldBranchToCommonDest poison-safe by default

Juneyoung Lee via llvm-commits llvm-commits at lists.llvm.org
Sat Mar 27 03:05:25 PDT 2021


Author: Juneyoung Lee
Date: 2021-03-27T19:05:12+09:00
New Revision: 05884d3b525a1072dd9d834593a7899fe8284f43

URL: https://github.com/llvm/llvm-project/commit/05884d3b525a1072dd9d834593a7899fe8284f43
DIFF: https://github.com/llvm/llvm-project/commit/05884d3b525a1072dd9d834593a7899fe8284f43.diff

LOG: Make FoldBranchToCommonDest poison-safe by default

This is a small patch to make FoldBranchToCommonDest poison-safe by default.
After fc3f0c9c, only two syntactic changes are needed to fix unit tests.
This does not cause any assembly difference in testsuite as well (-O3, X86-64 Manjaro).

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D99452

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/Local.h
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/Transforms/IRCE/bad_expander.ll
    llvm/test/Transforms/LoopSimplify/pr26682.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index c2c8061c85c7..f7efeeb56fd3 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -186,15 +186,10 @@ bool FlattenCFG(BasicBlock *BB, AAResults *AA = nullptr);
 /// If this basic block is ONLY a setcc and a branch, and if a predecessor
 /// branches to us and one of our successors, fold the setcc into the
 /// predecessor and use logical operations to pick the right destination.
-/// If PoisonSafe is true, use select i1 rather than and/or i1 to successfully
-/// block unexpected propagation of poison when merging the branches. This is
-/// set to false by default when used by LoopSimplify for performance, but this
-/// should be turned on by default.
 bool FoldBranchToCommonDest(BranchInst *BI, llvm::DomTreeUpdater *DTU = nullptr,
                             MemorySSAUpdater *MSSAU = nullptr,
                             const TargetTransformInfo *TTI = nullptr,
-                            unsigned BonusInstThreshold = 1,
-                            bool PoisonSafe = false);
+                            unsigned BonusInstThreshold = 1);
 
 /// This function takes a virtual register computed by an Instruction and
 /// replaces it with a slot in the stack frame, allocated via alloca.

diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index b5f14530875f..9a5e864170bc 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -2902,7 +2902,6 @@ shouldFoldCondBranchesToCommonDestination(BranchInst *BI, BranchInst *PBI,
 static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
                                              DomTreeUpdater *DTU,
                                              MemorySSAUpdater *MSSAU,
-                                             bool PoisonSafe,
                                              const TargetTransformInfo *TTI) {
   BasicBlock *BB = BI->getParent();
   BasicBlock *PredBlock = PBI->getParent();
@@ -3004,9 +3003,8 @@ static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
   // or/and the two conditions together.
   Value *NewCond = nullptr;
   Value *BICond = VMap[BI->getCondition()];
-  bool UseBinOp = !PoisonSafe || impliesPoison(BICond, PBI->getCondition());
 
-  if (UseBinOp)
+  if (impliesPoison(BICond, PBI->getCondition()))
     NewCond = Builder.CreateBinOp(Opc, PBI->getCondition(), BICond, "or.cond");
   else
     NewCond =
@@ -3035,8 +3033,7 @@ static bool performBranchToCommonDestFolding(BranchInst *BI, BranchInst *PBI,
 bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
                                   MemorySSAUpdater *MSSAU,
                                   const TargetTransformInfo *TTI,
-                                  unsigned BonusInstThreshold,
-                                  bool PoisonSafe) {
+                                  unsigned BonusInstThreshold) {
   // If this block ends with an unconditional branch,
   // let SpeculativelyExecuteBB() deal with it.
   if (!BI->isConditional())
@@ -3140,8 +3137,7 @@ bool llvm::FoldBranchToCommonDest(BranchInst *BI, DomTreeUpdater *DTU,
   // Ok, we have the budget. Perform the transformation.
   for (BasicBlock *PredBlock : Preds) {
     auto *PBI = cast<BranchInst>(PredBlock->getTerminator());
-    return performBranchToCommonDestFolding(BI, PBI, DTU, MSSAU, PoisonSafe,
-                                            TTI);
+    return performBranchToCommonDestFolding(BI, PBI, DTU, MSSAU, TTI);
   }
   return Changed;
 }
@@ -6360,7 +6356,7 @@ bool SimplifyCFGOpt::simplifyUncondBranch(BranchInst *BI,
   // predecessor and use logical operations to update the incoming value
   // for PHI nodes in common successor.
   if (FoldBranchToCommonDest(BI, DTU, /*MSSAU=*/nullptr, &TTI,
-                             Options.BonusInstThreshold, true))
+                             Options.BonusInstThreshold))
     return requestResimplify();
   return false;
 }
@@ -6424,7 +6420,7 @@ bool SimplifyCFGOpt::simplifyCondBranch(BranchInst *BI, IRBuilder<> &Builder) {
   // branches to us and one of our successors, fold the comparison into the
   // predecessor and use logical operations to pick the right destination.
   if (FoldBranchToCommonDest(BI, DTU, /*MSSAU=*/nullptr, &TTI,
-                             Options.BonusInstThreshold, true))
+                             Options.BonusInstThreshold))
     return requestResimplify();
 
   // We have a conditional branch to two blocks that are only reachable

diff  --git a/llvm/test/Transforms/IRCE/bad_expander.ll b/llvm/test/Transforms/IRCE/bad_expander.ll
index 8eb769526aaa..a3d6a054ef05 100644
--- a/llvm/test/Transforms/IRCE/bad_expander.ll
+++ b/llvm/test/Transforms/IRCE/bad_expander.ll
@@ -98,7 +98,7 @@ define void @test_03(i64* %p1, i64* %p2, i1 %maybe_exit) {
 ; CHECK-NEXT:    %iv = phi i64 [ %iv.next, %guarded ], [ 0, %loop.preheader ]
 ; CHECK-NEXT:    %iv.next = add nuw nsw i64 %iv, 1
 ; CHECK-NEXT:    %rc = icmp slt i64 %iv.next, %div_result
-; CHECK-NEXT:    %or.cond = and i1 %maybe_exit, true
+; CHECK-NEXT:    %or.cond = select i1 %maybe_exit, i1 true, i1 false
 ; CHECK-NEXT:    br i1 %or.cond, label %guarded, label %exit.loopexit1
 ; CHECK:       guarded:
 ; CHECK-NEXT:    %gep = getelementptr i64, i64* %p1, i64 %iv.next

diff  --git a/llvm/test/Transforms/LoopSimplify/pr26682.ll b/llvm/test/Transforms/LoopSimplify/pr26682.ll
index 092c0c3f0b04..4016b4e89b52 100644
--- a/llvm/test/Transforms/LoopSimplify/pr26682.ll
+++ b/llvm/test/Transforms/LoopSimplify/pr26682.ll
@@ -7,7 +7,7 @@ target triple = "x86_64-unknown-unknown"
 ; Check that loop-simplify merges two loop exits, but preserves LCSSA form.
 ; CHECK-LABEL: @foo
 ; CHECK: for:
-; CHECK: %or.cond = and i1 %cmp1, %cmp2
+; CHECK: %or.cond = select i1 %cmp1, i1 %cmp2, i1 false
 ; CHECK-NOT: for.cond:
 ; CHECK: for.end:
 ; CHECK: %a.lcssa = phi i32 [ %a, %for ]


        


More information about the llvm-commits mailing list