[llvm] 88e85aa - Handle simple diamond CFG hoisting in DivRemPairs.
Owen Anderson via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 28 10:24:25 PST 2022
Author: Owen Anderson
Date: 2022-12-28T11:24:18-07:00
New Revision: 88e85aa580062c0f2b5882eef71bed498a6af159
URL: https://github.com/llvm/llvm-project/commit/88e85aa580062c0f2b5882eef71bed498a6af159
DIFF: https://github.com/llvm/llvm-project/commit/88e85aa580062c0f2b5882eef71bed498a6af159.diff
LOG: Handle simple diamond CFG hoisting in DivRemPairs.
Previous we only handled triangle CFGs. This patch expands that
to support diamonds, where the div and rem appear in the then/else
sides of a condition. In that case, we can hoist the div into the
shared predecessor.
This could be generalized further to use nearest common ancestors,
but some of the conditions for hoisting would then require
post-dominator information.
Reviewed By: nikic, lebedev.ri
Differential Revision: https://reviews.llvm.org/D140647
Added:
llvm/test/Transforms/DivRemPairs/MSP430/div-rem-pairs.ll
llvm/test/Transforms/DivRemPairs/RISCV/div-rem-pairs.ll
Modified:
llvm/lib/Transforms/Scalar/DivRemPairs.cpp
llvm/test/Transforms/DivRemPairs/Mips/div-rem-pairs.ll
llvm/test/Transforms/DivRemPairs/PowerPC/div-rem-pairs.ll
llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll
llvm/test/Transforms/DivRemPairs/X86/div-rem-pairs.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
index fe7fdcd2b5a2c..303951643a0b8 100644
--- a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
+++ b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
@@ -267,12 +267,32 @@ static bool optimizeDivRem(Function &F, const TargetTransformInfo &TTI,
// DivBB will always reach the Div/Rem, we can hoist Div to PredBB. If
// we have a DivRem operation we can also hoist Rem. Otherwise we'll leave
// Rem where it is and rewrite it to mul/sub.
- // FIXME: We could handle more hoisting cases.
- if (RemBB->getSingleSuccessor() == DivBB)
+ if (RemBB->getSingleSuccessor() == DivBB) {
PredBB = RemBB->getUniquePredecessor();
- if (PredBB && IsSafeToHoist(RemInst, RemBB) &&
- IsSafeToHoist(DivInst, DivBB) &&
+ // Look for something like this
+ // PredBB
+ // / \
+ // Div Rem
+ //
+ // If the Rem and Din blocks share a unique predecessor, and all
+ // paths from PredBB go to either RemBB or DivBB, and execution of RemBB
+ // and DivBB will always reach the Div/Rem, we can hoist Div to PredBB.
+ // If we have a DivRem operation we can also hoist Rem. By hoisting both
+ // ops to the same block, we reduce code size and allow the DivRem to
+ // issue sooner. Without a DivRem op, this transformation is
+ // unprofitable because we would end up performing an extra Mul+Sub on
+ // the Rem path.
+ } else if (BasicBlock *RemPredBB = RemBB->getUniquePredecessor()) {
+ // This hoist is only profitable when the target has a DivRem op.
+ if (HasDivRemOp && RemPredBB == DivBB->getUniquePredecessor())
+ PredBB = RemPredBB;
+ }
+ // FIXME: We could handle more hoisting cases.
+
+ if (PredBB && !isa<CatchSwitchInst>(PredBB->getTerminator()) &&
+ isGuaranteedToTransferExecutionToSuccessor(PredBB->getTerminator()) &&
+ IsSafeToHoist(RemInst, RemBB) && IsSafeToHoist(DivInst, DivBB) &&
all_of(successors(PredBB),
[&](BasicBlock *BB) { return BB == DivBB || BB == RemBB; }) &&
all_of(predecessors(DivBB),
diff --git a/llvm/test/Transforms/DivRemPairs/MSP430/div-rem-pairs.ll b/llvm/test/Transforms/DivRemPairs/MSP430/div-rem-pairs.ll
new file mode 100644
index 0000000000000..dafa1b587584d
--- /dev/null
+++ b/llvm/test/Transforms/DivRemPairs/MSP430/div-rem-pairs.ll
@@ -0,0 +1,35 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=div-rem-pairs -S -mtriple=msp430-unknown-unknown | FileCheck %s
+
+; Do not hoist to the common predecessor block since we don't
+; have a div-rem operation.
+
+define i32 @no_domination(i1 %cmp, i32 %a, i32 %b) {
+; CHECK-LABEL: @no_domination(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[CMP:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
+; CHECK: if:
+; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: br label [[END:%.*]]
+; CHECK: else:
+; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
+; CHECK-NEXT: br label [[END]]
+; CHECK: end:
+; CHECK-NEXT: [[RET:%.*]] = phi i32 [ [[DIV]], [[IF]] ], [ [[REM]], [[ELSE]] ]
+; CHECK-NEXT: ret i32 [[RET]]
+;
+entry:
+ br i1 %cmp, label %if, label %else
+
+if:
+ %div = sdiv i32 %a, %b
+ br label %end
+
+else:
+ %rem = srem i32 %a, %b
+ br label %end
+
+end:
+ %ret = phi i32 [ %div, %if ], [ %rem, %else ]
+ ret i32 %ret
+}
diff --git a/llvm/test/Transforms/DivRemPairs/Mips/div-rem-pairs.ll b/llvm/test/Transforms/DivRemPairs/Mips/div-rem-pairs.ll
index a35a6346b3870..904189de09e1f 100644
--- a/llvm/test/Transforms/DivRemPairs/Mips/div-rem-pairs.ll
+++ b/llvm/test/Transforms/DivRemPairs/Mips/div-rem-pairs.ll
@@ -317,18 +317,17 @@ end:
ret i128 %ret
}
-; We don't hoist if one op does not dominate the other,
-; but we could hoist both ops to the common predecessor block?
+; Hoist both ops to the common predecessor block.
define i32 @no_domination(i1 %cmp, i32 %a, i32 %b) {
; CHECK-LABEL: @no_domination(
; CHECK-NEXT: entry:
+; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
; CHECK-NEXT: br i1 [[CMP:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
; CHECK: if:
-; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: br label [[END:%.*]]
; CHECK: else:
-; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
; CHECK-NEXT: br label [[END]]
; CHECK: end:
; CHECK-NEXT: [[RET:%.*]] = phi i32 [ [[DIV]], [[IF]] ], [ [[REM]], [[ELSE]] ]
diff --git a/llvm/test/Transforms/DivRemPairs/PowerPC/div-rem-pairs.ll b/llvm/test/Transforms/DivRemPairs/PowerPC/div-rem-pairs.ll
index 2c7e327a5c12e..08095c87ea726 100644
--- a/llvm/test/Transforms/DivRemPairs/PowerPC/div-rem-pairs.ll
+++ b/llvm/test/Transforms/DivRemPairs/PowerPC/div-rem-pairs.ll
@@ -331,8 +331,8 @@ end:
ret i128 %ret
}
-; We don't hoist if one op does not dominate the other,
-; but we could hoist both ops to the common predecessor block?
+; Do not hoist to the common predecessor block since we don't
+; have a div-rem operation.
define i32 @no_domination(i1 %cmp, i32 %a, i32 %b) {
; CHECK-LABEL: @no_domination(
diff --git a/llvm/test/Transforms/DivRemPairs/RISCV/div-rem-pairs.ll b/llvm/test/Transforms/DivRemPairs/RISCV/div-rem-pairs.ll
new file mode 100644
index 0000000000000..5ee788d86c204
--- /dev/null
+++ b/llvm/test/Transforms/DivRemPairs/RISCV/div-rem-pairs.ll
@@ -0,0 +1,35 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=div-rem-pairs -S -mtriple=riscv64-unknown-unknown | FileCheck %s
+
+; Do not hoist to the common predecessor block since we don't
+; have a div-rem operation.
+
+define i32 @no_domination(i1 %cmp, i32 %a, i32 %b) {
+; CHECK-LABEL: @no_domination(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[CMP:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
+; CHECK: if:
+; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: br label [[END:%.*]]
+; CHECK: else:
+; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
+; CHECK-NEXT: br label [[END]]
+; CHECK: end:
+; CHECK-NEXT: [[RET:%.*]] = phi i32 [ [[DIV]], [[IF]] ], [ [[REM]], [[ELSE]] ]
+; CHECK-NEXT: ret i32 [[RET]]
+;
+entry:
+ br i1 %cmp, label %if, label %else
+
+if:
+ %div = sdiv i32 %a, %b
+ br label %end
+
+else:
+ %rem = srem i32 %a, %b
+ br label %end
+
+end:
+ %ret = phi i32 [ %div, %if ], [ %rem, %else ]
+ ret i32 %ret
+}
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 7d135b32dfa43..9a13d8bb9e1fc 100644
--- a/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll
+++ b/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll
@@ -174,14 +174,14 @@ end:
define i32 @can_have_divrem_in_mutually_nondominating_bbs(i1 %cmp, i32 %a, i32 %b) {
; CHECK-LABEL: @can_have_divrem_in_mutually_nondominating_bbs(
; CHECK-NEXT: entry:
+; CHECK-NEXT: [[T3:%.*]] = udiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: [[T2_RECOMPOSED:%.*]] = urem i32 [[A]], [[B]]
; CHECK-NEXT: br i1 [[CMP:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
-; CHECK-NEXT: [[T0:%.*]] = udiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: [[T0:%.*]] = udiv i32 [[A]], [[B]]
; CHECK-NEXT: [[T1:%.*]] = mul nuw i32 [[T0]], [[B]]
-; CHECK-NEXT: [[T2_RECOMPOSED:%.*]] = urem i32 [[A]], [[B]]
; CHECK-NEXT: br label [[END:%.*]]
; CHECK: if.else:
-; CHECK-NEXT: [[T3:%.*]] = udiv i32 [[A]], [[B]]
; CHECK-NEXT: br label [[END]]
; CHECK: end:
; CHECK-NEXT: [[RET:%.*]] = phi i32 [ [[T2_RECOMPOSED]], [[IF_THEN]] ], [ [[T3]], [[IF_ELSE]] ]
diff --git a/llvm/test/Transforms/DivRemPairs/X86/div-rem-pairs.ll b/llvm/test/Transforms/DivRemPairs/X86/div-rem-pairs.ll
index 86bf3b2e1c46c..8bf75bb5bcb2e 100644
--- a/llvm/test/Transforms/DivRemPairs/X86/div-rem-pairs.ll
+++ b/llvm/test/Transforms/DivRemPairs/X86/div-rem-pairs.ll
@@ -308,18 +308,17 @@ end:
ret i128 %ret
}
-; We don't hoist if one op does not dominate the other,
-; but we could hoist both ops to the common predecessor block?
+; Hoist both ops to the common predecessor block.
define i32 @no_domination(i1 %cmp, i32 %a, i32 %b) {
; CHECK-LABEL: @no_domination(
; CHECK-NEXT: entry:
+; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
; CHECK-NEXT: br i1 [[CMP:%.*]], label [[IF:%.*]], label [[ELSE:%.*]]
; CHECK: if:
-; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: br label [[END:%.*]]
; CHECK: else:
-; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
; CHECK-NEXT: br label [[END]]
; CHECK: end:
; CHECK-NEXT: [[RET:%.*]] = phi i32 [ [[DIV]], [[IF]] ], [ [[REM]], [[ELSE]] ]
@@ -341,3 +340,228 @@ end:
ret i32 %ret
}
+define i64 @diamond_pred_has_other_sucessors(i64 %a, i64 %b, i64 %c) {
+; CHECK-LABEL: @diamond_pred_has_other_sucessors(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: switch i64 [[A:%.*]], label [[RETURN:%.*]] [
+; CHECK-NEXT: i64 0, label [[SW_BB:%.*]]
+; CHECK-NEXT: i64 1, label [[SW_BB1:%.*]]
+; CHECK-NEXT: ]
+; CHECK: sw.bb:
+; CHECK-NEXT: [[DIV:%.*]] = udiv i64 [[B:%.*]], [[C:%.*]]
+; CHECK-NEXT: br label [[RETURN]]
+; CHECK: sw.bb1:
+; CHECK-NEXT: [[REM:%.*]] = urem i64 [[B]], [[C]]
+; CHECK-NEXT: br label [[RETURN]]
+; CHECK: return:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i64 [ [[REM]], [[SW_BB1]] ], [ [[DIV]], [[SW_BB]] ], [ 0, [[ENTRY:%.*]] ]
+; CHECK-NEXT: ret i64 [[RETVAL_0]]
+;
+entry:
+ switch i64 %a, label %return [
+ i64 0, label %sw.bb
+ i64 1, label %sw.bb1
+ ]
+
+sw.bb: ; preds = %entry
+ %div = udiv i64 %b, %c
+ br label %return
+
+sw.bb1: ; preds = %entry
+ %rem = urem i64 %b, %c
+ br label %return
+
+return: ; preds = %entry, %sw.bb1, %sw.bb
+ %retval.0 = phi i64 [ %rem, %sw.bb1 ], [ %div, %sw.bb ], [ 0, %entry ]
+ ret i64 %retval.0
+}
+
+define i64 @diamond_div_no_unique_predecessor(i64 %a, i64 %b, i64 %c) {
+; CHECK-LABEL: @diamond_div_no_unique_predecessor(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i64 [[A:%.*]], -1
+; CHECK-NEXT: br i1 [[CMP]], label [[FOO:%.*]], label [[IF_END:%.*]]
+; CHECK: if.end:
+; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i64 [[A]], 1
+; CHECK-NEXT: br i1 [[CMP1]], label [[BAR:%.*]], label [[BAZ:%.*]]
+; CHECK: baz:
+; CHECK-NEXT: [[DIV:%.*]] = udiv i64 [[B:%.*]], [[C:%.*]]
+; CHECK-NEXT: br label [[RETURN:%.*]]
+; CHECK: foo:
+; CHECK-NEXT: br label [[BAZ]]
+; CHECK: bar:
+; CHECK-NEXT: [[REM:%.*]] = urem i64 [[B]], [[C]]
+; CHECK-NEXT: br label [[RETURN]]
+; CHECK: return:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i64 [ [[DIV]], [[BAZ]] ], [ [[REM]], [[BAR]] ]
+; CHECK-NEXT: ret i64 [[RETVAL_0]]
+;
+entry:
+ %cmp = icmp slt i64 %a, -1
+ br i1 %cmp, label %foo, label %if.end
+
+if.end:
+ %cmp1 = icmp sgt i64 %a, 1
+ br i1 %cmp1, label %bar, label %baz
+
+baz:
+ %div = udiv i64 %b, %c
+ br label %return
+
+foo:
+ br label %baz
+
+bar:
+ %rem = urem i64 %b, %c
+ br label %return
+
+return:
+ %retval.0 = phi i64 [ %div, %baz ], [ %rem, %bar ]
+ ret i64 %retval.0
+}
+
+define i64 @diamond_rem_no_unique_predecessor(i64 %a, i64 %b, i64 %c) {
+; CHECK-LABEL: @diamond_rem_no_unique_predecessor(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = icmp slt i64 [[A:%.*]], -1
+; CHECK-NEXT: br i1 [[CMP]], label [[FOO:%.*]], label [[IF_END:%.*]]
+; CHECK: if.end:
+; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i64 [[A]], 1
+; CHECK-NEXT: br i1 [[CMP1]], label [[BAR:%.*]], label [[BAZ:%.*]]
+; CHECK: baz:
+; CHECK-NEXT: [[REM:%.*]] = urem i64 [[B:%.*]], [[C:%.*]]
+; CHECK-NEXT: br label [[RETURN:%.*]]
+; CHECK: foo:
+; CHECK-NEXT: br label [[BAZ]]
+; CHECK: bar:
+; CHECK-NEXT: [[DIV:%.*]] = udiv i64 [[B]], [[C]]
+; CHECK-NEXT: br label [[RETURN]]
+; CHECK: return:
+; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i64 [ [[REM]], [[BAZ]] ], [ [[DIV]], [[BAR]] ]
+; CHECK-NEXT: ret i64 [[RETVAL_0]]
+;
+entry:
+ %cmp = icmp slt i64 %a, -1
+ br i1 %cmp, label %foo, label %if.end
+
+if.end:
+ %cmp1 = icmp sgt i64 %a, 1
+ br i1 %cmp1, label %bar, label %baz
+
+baz:
+ %rem = urem i64 %b, %c
+ br label %return
+
+foo:
+ br label %baz
+
+bar:
+ %div = udiv i64 %b, %c
+ br label %return
+
+return:
+ %retval.0 = phi i64 [ %rem, %baz ], [ %div, %bar ]
+ ret i64 %retval.0
+}
+
+declare void @dummy()
+
+define i32 @invoke_not_willreturn(i32 %a, i32 %b) personality ptr null {
+; CHECK-LABEL: @invoke_not_willreturn(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: invoke void @dummy()
+; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
+; CHECK: cont:
+; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: ret i32 [[DIV]]
+; CHECK: lpad:
+; CHECK-NEXT: [[TMP0:%.*]] = landingpad { ptr, i32 }
+; CHECK-NEXT: cleanup
+; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
+; CHECK-NEXT: ret i32 [[REM]]
+;
+entry:
+ invoke void @dummy()
+ to label %cont unwind label %lpad
+
+cont:
+ %div = sdiv i32 %a, %b
+ ret i32 %div
+
+lpad:
+ landingpad { ptr, i32 }
+ cleanup
+ %rem = srem i32 %a, %b
+ ret i32 %rem
+}
+
+define i32 @invoke_willreturn(i32 %a, i32 %b) personality ptr null {
+; CHECK-LABEL: @invoke_willreturn(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
+; CHECK-NEXT: invoke void @dummy() #[[ATTR0:[0-9]+]]
+; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
+; CHECK: cont:
+; CHECK-NEXT: ret i32 [[DIV]]
+; CHECK: lpad:
+; CHECK-NEXT: [[TMP0:%.*]] = landingpad { ptr, i32 }
+; CHECK-NEXT: cleanup
+; CHECK-NEXT: ret i32 [[REM]]
+;
+entry:
+ invoke void @dummy() willreturn
+ to label %cont unwind label %lpad
+
+cont:
+ %div = sdiv i32 %a, %b
+ ret i32 %div
+
+lpad:
+ landingpad { ptr, i32 }
+ cleanup
+ %rem = srem i32 %a, %b
+ ret i32 %rem
+}
+
+; Use this personality function so that catchpad is guaranteed to transfer.
+declare void @ProcessCLRException()
+
+define i32 @catchswitch(i32 %a, i32 %b) personality ptr @ProcessCLRException {
+; CHECK-LABEL: @catchswitch(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: invoke void @dummy() #[[ATTR0]]
+; CHECK-NEXT: to label [[CONT:%.*]] unwind label [[LPAD:%.*]]
+; CHECK: cont:
+; CHECK-NEXT: ret i32 0
+; CHECK: lpad:
+; CHECK-NEXT: [[CS:%.*]] = catchswitch within none [label %cp] unwind label [[LPAD_END:%.*]]
+; CHECK: cp:
+; CHECK-NEXT: [[TMP0:%.*]] = catchpad within [[CS]] []
+; CHECK-NEXT: [[DIV:%.*]] = sdiv i32 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT: ret i32 [[DIV]]
+; CHECK: lpad.end:
+; CHECK-NEXT: [[TMP1:%.*]] = cleanuppad within none []
+; CHECK-NEXT: [[REM:%.*]] = srem i32 [[A]], [[B]]
+; CHECK-NEXT: ret i32 [[REM]]
+;
+entry:
+ invoke void @dummy() willreturn
+ to label %cont unwind label %lpad
+
+cont:
+ ret i32 0
+
+lpad:
+ %cs = catchswitch within none [label %cp] unwind label %lpad.end
+
+cp:
+ catchpad within %cs []
+ %div = sdiv i32 %a, %b
+ ret i32 %div
+
+lpad.end:
+ cleanuppad within none []
+ %rem = srem i32 %a, %b
+ ret i32 %rem
+}
More information about the llvm-commits
mailing list