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

diff --git a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
index fe7fdcd..3039516 100644 (file)
--- 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 (file)
index 0000000..dafa1b5
--- /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 a35a634..904189d 100644 (file)
--- 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 2c7e327..08095c8 100644 (file)
--- 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 (file)
index 0000000..5ee788d
--- /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 7d135b3..9a13d8b 100644 (file)
--- 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 86bf3b2..8bf75bb 100644 (file)
--- 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
+}