[llvm-branch-commits] [llvm] 2f43c81 - [DivRemPairs] make sure we have a valid CFG for hoisting division

[Tom Stellard via llvm-branch-commits]

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
+}