Skip to content

Commit

Permalink
[DivRemPairs] make sure we have a valid CFG for hoisting division
Browse files Browse the repository at this point in the history
This transform was added with e38b7e8
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.
  • Loading branch information
rotateright committed Jul 28, 2021
1 parent d3c70d9 commit 5b83261
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
7 changes: 4 additions & 3 deletions llvm/lib/Transforms/Scalar/DivRemPairs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
32 changes: 32 additions & 0 deletions llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

0 comments on commit 5b83261

Please sign in to comment.