[llvm] e38b7e8 - [DivRemPairs] Add an initial case for hoisting to a common predecessor.

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Sun Jul 11 10:08:19 PDT 2021


Author: Craig Topper
Date: 2021-07-11T10:03:07-07:00
New Revision: e38b7e894808ec2a0c976ab01e44364f167508d3

URL: https://github.com/llvm/llvm-project/commit/e38b7e894808ec2a0c976ab01e44364f167508d3
DIFF: https://github.com/llvm/llvm-project/commit/e38b7e894808ec2a0c976ab01e44364f167508d3.diff

LOG: [DivRemPairs] Add an initial case for hoisting to a common predecessor.

This patch adds support for hoisting the division and maybe the
remainder for control flow graphs like this.

```
PredBB
  |  \
  |  Rem
  |  /
 Div
```

If we have DivRem we'll hoist both to PredBB. If not we'll just
hoist Div and expand Rem using the Div.

This improves our codegen for something like this

```
__uint128_t udivmodti4(__uint128_t dividend, __uint128_t divisor, __uint128_t *remainder) {
    if (remainder != 0)
        *remainder = dividend % divisor;
    return dividend / divisor;
}
```

Reviewed By: spatel, lebedev.ri

Differential Revision: https://reviews.llvm.org/D87555

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 574640d978a90..c77769368ede1 100644
--- a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
+++ b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp
@@ -238,8 +238,52 @@ static bool optimizeDivRem(Function &F, const TargetTransformInfo &TTI,
     if (!DivDominates && !DT.dominates(RemInst, DivInst)) {
       // We have matching div-rem pair, but they are in two 
diff erent blocks,
       // neither of which dominates one another.
-      // FIXME: We could hoist both ops to the common predecessor block?
-      continue;
+
+      BasicBlock *PredBB = nullptr;
+      BasicBlock *DivBB = DivInst->getParent();
+      BasicBlock *RemBB = RemInst->getParent();
+
+      // It's only safe to hoist if every instruction before the Div/Rem in the
+      // basic block is guaranteed to transfer execution.
+      auto IsSafeToHoist = [](Instruction *DivOrRem, BasicBlock *ParentBB) {
+        for (auto I = ParentBB->begin(), E = DivOrRem->getIterator(); I != E;
+             ++I)
+          if (!isGuaranteedToTransferExecutionToSuccessor(&*I))
+            return false;
+
+        return true;
+      };
+
+      // Look for something like this
+      // PredBB
+      //   |  \
+      //   |  Rem
+      //   |  /
+      //  Div
+      //
+      // If the Rem block has a single predecessor and successor, 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. 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)
+        PredBB = RemBB->getUniquePredecessor();
+
+      if (PredBB && IsSafeToHoist(RemInst, RemBB) &&
+          IsSafeToHoist(DivInst, DivBB) &&
+          llvm::all_of(successors(PredBB), [&](BasicBlock *BB) {
+            return BB == DivBB || BB == RemBB;
+          })) {
+        DivDominates = true;
+        DivInst->moveBefore(PredBB->getTerminator());
+        Changed = true;
+        if (HasDivRemOp) {
+          RemInst->moveBefore(PredBB->getTerminator());
+          continue;
+        }
+      } else
+        continue;
     }
 
     // The target does not have a single div/rem operation,

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 f8ecf77392418..e93556c0c6844 100644
--- a/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll
+++ b/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll
@@ -210,13 +210,13 @@ define i64 @remainder_triangle_i64(i64 %a, i64 %b, i64* %rp) {
 ; CHECK-LABEL: @remainder_triangle_i64(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i64* [[RP:%.*]], null
+; CHECK-NEXT:    [[DIV:%.*]] = udiv i64 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[REM:%.*]] = urem i64 [[A]], [[B]]
 ; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[END:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    [[REM:%.*]] = urem i64 [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    store i64 [[REM]], i64* [[RP]], align 4
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[DIV:%.*]] = udiv i64 [[A]], [[B]]
 ; CHECK-NEXT:    ret i64 [[DIV]]
 ;
 entry:
@@ -239,13 +239,16 @@ define i128 @remainder_triangle_i128(i128 %a, i128 %b, i128* %rp) {
 ; CHECK-LABEL: @remainder_triangle_i128(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i128* [[RP:%.*]], null
+; CHECK-NEXT:    [[A_FROZEN:%.*]] = freeze i128 [[A:%.*]]
+; CHECK-NEXT:    [[B_FROZEN:%.*]] = freeze i128 [[B:%.*]]
+; CHECK-NEXT:    [[DIV:%.*]] = udiv i128 [[A_FROZEN]], [[B_FROZEN]]
 ; CHECK-NEXT:    br i1 [[CMP]], label [[IF_THEN:%.*]], label [[END:%.*]]
 ; CHECK:       if.then:
-; CHECK-NEXT:    [[REM:%.*]] = urem i128 [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT:    store i128 [[REM]], i128* [[RP]], align 4
+; CHECK-NEXT:    [[TMP0:%.*]] = mul i128 [[DIV]], [[B_FROZEN]]
+; CHECK-NEXT:    [[REM_DECOMPOSED:%.*]] = sub i128 [[A_FROZEN]], [[TMP0]]
+; CHECK-NEXT:    store i128 [[REM_DECOMPOSED]], i128* [[RP]], align 4
 ; CHECK-NEXT:    br label [[END]]
 ; CHECK:       end:
-; CHECK-NEXT:    [[DIV:%.*]] = udiv i128 [[A]], [[B]]
 ; CHECK-NEXT:    ret i128 [[DIV]]
 ;
 entry:
@@ -265,16 +268,16 @@ end:
 define i64 @remainder_triangle_i64_multiple_rem_edges(i64 %a, i64 %b, i64 %c, i64* %rp) {
 ; CHECK-LABEL: @remainder_triangle_i64_multiple_rem_edges(
 ; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[DIV:%.*]] = udiv i64 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[REM:%.*]] = urem i64 [[A]], [[B]]
 ; CHECK-NEXT:    switch i64 [[C:%.*]], label [[SW_DEFAULT:%.*]] [
 ; CHECK-NEXT:    i64 0, label [[SW_BB:%.*]]
 ; CHECK-NEXT:    i64 2, label [[SW_BB]]
 ; CHECK-NEXT:    ]
 ; CHECK:       sw.bb:
-; CHECK-NEXT:    [[REM:%.*]] = urem i64 [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    store i64 [[REM]], i64* [[RP:%.*]], align 4
 ; CHECK-NEXT:    br label [[SW_DEFAULT]]
 ; CHECK:       sw.default:
-; CHECK-NEXT:    [[DIV:%.*]] = udiv i64 [[A]], [[B]]
 ; CHECK-NEXT:    ret i64 [[DIV]]
 ;
 entry:
@@ -296,16 +299,16 @@ sw.default:                                       ; preds = %entry, %sw.bb
 define i64 @remainder_triangle_i64_multiple_div_edges(i64 %a, i64 %b, i64 %c, i64* %rp) {
 ; CHECK-LABEL: @remainder_triangle_i64_multiple_div_edges(
 ; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[DIV:%.*]] = udiv i64 [[A:%.*]], [[B:%.*]]
+; CHECK-NEXT:    [[REM:%.*]] = urem i64 [[A]], [[B]]
 ; CHECK-NEXT:    switch i64 [[C:%.*]], label [[SW_DEFAULT:%.*]] [
 ; CHECK-NEXT:    i64 0, label [[SW_BB:%.*]]
 ; CHECK-NEXT:    i64 2, label [[SW_BB]]
 ; CHECK-NEXT:    ]
 ; CHECK:       sw.default:
-; CHECK-NEXT:    [[REM:%.*]] = urem i64 [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    store i64 [[REM]], i64* [[RP:%.*]], align 4
 ; CHECK-NEXT:    br label [[SW_BB]]
 ; CHECK:       sw.bb:
-; CHECK-NEXT:    [[DIV:%.*]] = udiv i64 [[A]], [[B]]
 ; CHECK-NEXT:    ret i64 [[DIV]]
 ;
 entry:


        


More information about the llvm-commits mailing list