From 7af078847cd287256213b2db061a4e14eae6bb5d Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 21 Apr 2017 15:53:42 +0000 Subject: [PATCH] [SimplifyCFG] Fix the determination of PostBB in conditional store merging to handle the targets on the second branch being commuted Currently we choose PostBB as the single successor of QFB, but its possible that QTB's single successor is QFB which would make QFB the correct choice. Differential Revision: https://reviews.llvm.org/D32323 llvm-svn: 300992 --- llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 12 ++++++++++-- llvm/test/Transforms/SimplifyCFG/merge-cond-stores.ll | 18 ++++++------------ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp index 2f575b9..e625766 100644 --- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp @@ -3055,6 +3055,15 @@ static bool mergeConditionalStores(BranchInst *PBI, BranchInst *QBI) { BasicBlock *QFB = QBI->getSuccessor(1); BasicBlock *PostBB = QFB->getSingleSuccessor(); + // Make sure we have a good guess for PostBB. If QTB's only successor is + // QFB, then QFB is a better PostBB. + if (QTB->getSingleSuccessor() == QFB) + PostBB = QFB; + + // If we couldn't find a good PostBB, stop. + if (!PostBB) + return false; + bool InvertPCond = false, InvertQCond = false; // Canonicalize fallthroughs to the true branches. if (PFB == QBI->getParent()) { @@ -3079,8 +3088,7 @@ static bool mergeConditionalStores(BranchInst *PBI, BranchInst *QBI) { auto HasOnePredAndOneSucc = [](BasicBlock *BB, BasicBlock *P, BasicBlock *S) { return BB->getSinglePredecessor() == P && BB->getSingleSuccessor() == S; }; - if (!PostBB || - !HasOnePredAndOneSucc(PFB, PBI->getParent(), QBI->getParent()) || + if (!HasOnePredAndOneSucc(PFB, PBI->getParent(), QBI->getParent()) || !HasOnePredAndOneSucc(QFB, QBI->getParent(), PostBB)) return false; if ((PTB && !HasOnePredAndOneSucc(PTB, PBI->getParent(), QBI->getParent())) || diff --git a/llvm/test/Transforms/SimplifyCFG/merge-cond-stores.ll b/llvm/test/Transforms/SimplifyCFG/merge-cond-stores.ll index 0174abf..a4bda96 100644 --- a/llvm/test/Transforms/SimplifyCFG/merge-cond-stores.ll +++ b/llvm/test/Transforms/SimplifyCFG/merge-cond-stores.ll @@ -37,23 +37,17 @@ end: } ; This is the same as test_simple, but the branch target order has been swapped -; TODO: This test should succeed and end up if-converted. define void @test_simple_commuted(i32* %p, i32 %a, i32 %b) { ; CHECK-LABEL: @test_simple_commuted( ; CHECK-NEXT: entry: ; CHECK-NEXT: [[X1:%.*]] = icmp eq i32 [[A:%.*]], 0 -; CHECK-NEXT: br i1 [[X1]], label [[YES1:%.*]], label [[FALLTHROUGH:%.*]] -; CHECK: yes1: -; CHECK-NEXT: store i32 0, i32* [[P:%.*]], align 4 -; CHECK-NEXT: br label [[FALLTHROUGH]] -; CHECK: fallthrough: ; CHECK-NEXT: [[X2:%.*]] = icmp eq i32 [[B:%.*]], 0 -; CHECK-NEXT: br i1 [[X2]], label [[YES2:%.*]], label [[END:%.*]] -; CHECK: yes2: -; CHECK-NEXT: store i32 1, i32* [[P]], align 4 -; CHECK-NEXT: br label [[END]] -; CHECK: end: -; CHECK-NEXT: ret void +; CHECK-NEXT: [[TMP0:%.*]] = or i1 [[X1]], [[X2]] +; CHECK-NEXT: br i1 [[TMP0]], label [[TMP1:%.*]], label [[TMP2:%.*]] +; CHECK: [[DOT:%.*]] = zext i1 [[X2]] to i32 +; CHECK-NEXT: store i32 [[DOT]], i32* [[P:%.*]], align 4 +; CHECK-NEXT: br label [[TMP2]] +; CHECK: ret void ; entry: %x1 = icmp eq i32 %a, 0 -- 2.7.4