From 05884d3b525a1072dd9d834593a7899fe8284f43 Mon Sep 17 00:00:00 2001 From: Juneyoung Lee Date: Sat, 27 Mar 2021 17:12:20 +0900 Subject: [PATCH] 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 --- llvm/include/llvm/Transforms/Utils/Local.h | 7 +------ llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 14 +++++--------- llvm/test/Transforms/IRCE/bad_expander.ll | 2 +- llvm/test/Transforms/LoopSimplify/pr26682.ll | 2 +- 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h index c2c8061..f7efeeb 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 b5f1453..9a5e864 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(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 8eb7695..a3d6a05 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 092c0c3..4016b4e 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 ] -- 2.7.4