Author: Sanjay Patel Date: 2021-08-02T13:52:37-07:00 New Revision: 2f43c816f18a4e953e07b9d2be663b9509b7a342
URL: https://github.com/llvm/llvm-project/commit/2f43c816f18a4e953e07b9d2be663b9509b7a342 DIFF: https://github.com/llvm/llvm-project/commit/2f43c816f18a4e953e07b9d2be663b9509b7a342.diff LOG: [DivRemPairs] make sure we have a valid CFG for hoisting division This transform was added with e38b7e894808ec2 and as shown in: https://llvm.org/PR51241 ...it could crash without an extra check of the blocks. There might be a more compact way to write this constraint, but we can't just count the successors/predecessors without affecting a test that includes a switch instruction. (cherry picked from commit 5b83261c1518a39636abe094123f1704bbfd972f) Added: Modified: llvm/lib/Transforms/Scalar/DivRemPairs.cpp llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll Removed: ################################################################################ diff --git a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp index c77769368ede1..66c9d9f0902a4 100644 --- a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp +++ b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp @@ -272,9 +272,10 @@ static bool optimizeDivRem(Function &F, const TargetTransformInfo &TTI, if (PredBB && IsSafeToHoist(RemInst, RemBB) && IsSafeToHoist(DivInst, DivBB) && - llvm::all_of(successors(PredBB), [&](BasicBlock *BB) { - return BB == DivBB || BB == RemBB; - })) { + all_of(successors(PredBB), + [&](BasicBlock *BB) { return BB == DivBB || BB == RemBB; }) && + all_of(predecessors(DivBB), + [&](BasicBlock *BB) { return BB == RemBB || BB == PredBB; })) { DivDominates = true; DivInst->moveBefore(PredBB->getTerminator()); Changed = true; diff --git a/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll b/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll index e93556c0c6844..685d15458b43c 100644 --- a/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll +++ b/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll @@ -529,3 +529,35 @@ return: ; preds = %if.then, %if.end3 %retval.0 = phi i64 [ %div, %if.end3 ], [ 0, %if.then ] ret i64 %retval.0 } + +; Negative test (this would create invalid IR and crash). +; The div block can't have predecessors other than the rem block +; and the common single pred block (it is reachable from entry here). + +define i32 @PR51241(i1 %b1, i1 %b2, i32 %t0) { +; CHECK-LABEL: @PR51241( +; CHECK-NEXT: entry: +; CHECK-NEXT: br i1 [[B1:%.*]], label [[DIVBB:%.*]], label [[PREDBB:%.*]] +; CHECK: predbb: +; CHECK-NEXT: br i1 [[B2:%.*]], label [[DIVBB]], label [[REMBB:%.*]] +; CHECK: rembb: +; CHECK-NEXT: [[REM2:%.*]] = srem i32 7, [[T0:%.*]] +; CHECK-NEXT: br label [[DIVBB]] +; CHECK: divbb: +; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 7, [[T0]] +; CHECK-NEXT: ret i32 [[DIV]] +; +entry: + br i1 %b1, label %divbb, label %predbb + +predbb: + br i1 %b2, label %divbb, label %rembb + +rembb: + %rem2 = srem i32 7, %t0 + br label %divbb + +divbb: + %div = sdiv i32 7, %t0 + ret i32 %div +} _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
