diff --git a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp --- a/llvm/lib/Transforms/Scalar/DivRemPairs.cpp +++ b/llvm/lib/Transforms/Scalar/DivRemPairs.cpp @@ -238,8 +238,52 @@ if (!DivDominates && !DT.dominates(RemInst, DivInst)) { // We have matching div-rem pair, but they are in two different 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 --- a/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll +++ b/llvm/test/Transforms/DivRemPairs/X86/div-expanded-rem-pair.ll @@ -204,3 +204,328 @@ %ret = phi i32 [ %t2, %if.then ], [ %t3, %if.else ] ret i32 %ret } + +; Test for hoisting a udiv to dominate a urem to allow udivrem. +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: store i64 [[REM]], i64* [[RP]], align 4 +; CHECK-NEXT: br label [[END]] +; CHECK: end: +; CHECK-NEXT: ret i64 [[DIV]] +; +entry: + %cmp = icmp ne i64* %rp, null + br i1 %cmp, label %if.then, label %end + +if.then: + %rem = urem i64 %a, %b + store i64 %rem, i64* %rp + br label %end + +end: + %div = udiv i64 %a, %b + ret i64 %div +} + +; Test for hoisting a udiv to dominate a urem to allow the urem to be expanded +; into mul+sub. +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: [[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: ret i128 [[DIV]] +; +entry: + %cmp = icmp ne i128* %rp, null + br i1 %cmp, label %if.then, label %end + +if.then: + %rem = urem i128 %a, %b + store i128 %rem, i128* %rp + br label %end + +end: + %div = udiv i128 %a, %b + ret i128 %div +} + +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: store i64 [[REM]], i64* [[RP:%.*]], align 4 +; CHECK-NEXT: br label [[SW_DEFAULT]] +; CHECK: sw.default: +; CHECK-NEXT: ret i64 [[DIV]] +; +entry: + switch i64 %c, label %sw.default [ + i64 0, label %sw.bb + i64 2, label %sw.bb + ] + +sw.bb: ; preds = %entry, %entry + %rem = urem i64 %a, %b + store i64 %rem, i64* %rp + br label %sw.default + +sw.default: ; preds = %entry, %sw.bb + %div = udiv i64 %a, %b + ret i64 %div +} + +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: store i64 [[REM]], i64* [[RP:%.*]], align 4 +; CHECK-NEXT: br label [[SW_BB]] +; CHECK: sw.bb: +; CHECK-NEXT: ret i64 [[DIV]] +; +entry: + switch i64 %c, label %sw.default [ + i64 0, label %sw.bb + i64 2, label %sw.bb + ] + +sw.default: ; preds = %entry, %entry + %rem = urem i64 %a, %b + store i64 %rem, i64* %rp + br label %sw.bb + +sw.bb: ; preds = %entry, %sw.default + %div = udiv i64 %a, %b + ret i64 %div +} + +declare void @maythrow() + +; Negative test. make sure we don't transform if there are instructions before +; the rem that might throw. +define i64 @remainder_triangle_i64_maythrow_rem(i64 %a, i64 %b, i64* %rp) { +; CHECK-LABEL: @remainder_triangle_i64_maythrow_rem( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64* [[RP:%.*]], null +; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[END:%.*]] +; CHECK: if.then: +; CHECK-NEXT: call void @maythrow() +; 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: + %cmp = icmp ne i64* %rp, null + br i1 %cmp, label %if.then, label %end + +if.then: + call void @maythrow() + %rem = urem i64 %a, %b + store i64 %rem, i64* %rp + br label %end + +end: + %div = udiv i64 %a, %b + ret i64 %div +} + +; Negative test. make sure we don't transform if there are instructions before +; the div that might throw. +define i64 @remainder_triangle_i64_maythrow_div(i64 %a, i64 %b, i64* %rp) { +; CHECK-LABEL: @remainder_triangle_i64_maythrow_div( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i64* [[RP:%.*]], null +; 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: call void @maythrow() +; CHECK-NEXT: [[DIV:%.*]] = udiv i64 [[A]], [[B]] +; CHECK-NEXT: ret i64 [[DIV]] +; +entry: + %cmp = icmp ne i64* %rp, null + br i1 %cmp, label %if.then, label %end + +if.then: + %rem = urem i64 %a, %b + store i64 %rem, i64* %rp + br label %end + +end: + call void @maythrow() + %div = udiv i64 %a, %b + ret i64 %div +} + +; Negative test, Make sure we don't transform if there are instructions before +; the rem that might throw. +define i128 @remainder_triangle_i128_maythrow_rem(i128 %a, i128 %b, i128* %rp) { +; CHECK-LABEL: @remainder_triangle_i128_maythrow_rem( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i128* [[RP:%.*]], null +; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[END:%.*]] +; CHECK: if.then: +; CHECK-NEXT: call void @maythrow() +; CHECK-NEXT: [[REM:%.*]] = urem i128 [[A:%.*]], [[B:%.*]] +; CHECK-NEXT: store i128 [[REM]], i128* [[RP]], align 4 +; CHECK-NEXT: br label [[END]] +; CHECK: end: +; CHECK-NEXT: [[DIV:%.*]] = udiv i128 [[A]], [[B]] +; CHECK-NEXT: ret i128 [[DIV]] +; +entry: + %cmp = icmp ne i128* %rp, null + br i1 %cmp, label %if.then, label %end + +if.then: + call void @maythrow() + %rem = urem i128 %a, %b + store i128 %rem, i128* %rp + br label %end + +end: + %div = udiv i128 %a, %b + ret i128 %div +} + +; Negative test. Make sure we don't transform if there are instructions before +; the div that might throw. +define i128 @remainder_triangle_i128_maythrow_div(i128 %a, i128 %b, i128* %rp) { +; CHECK-LABEL: @remainder_triangle_i128_maythrow_div( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[CMP:%.*]] = icmp ne i128* [[RP:%.*]], null +; 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: br label [[END]] +; CHECK: end: +; CHECK-NEXT: call void @maythrow() +; CHECK-NEXT: [[DIV:%.*]] = udiv i128 [[A]], [[B]] +; CHECK-NEXT: ret i128 [[DIV]] +; +entry: + %cmp = icmp ne i128* %rp, null + br i1 %cmp, label %if.then, label %end + +if.then: + %rem = urem i128 %a, %b + store i128 %rem, i128* %rp + br label %end + +end: + call void @maythrow() + %div = udiv i128 %a, %b + ret i128 %div +} + +; Negative test. The common predecessor has another successor so we can't hoist +; the udiv to the common predecessor. +define i64 @remainder_not_triangle_i32(i64 %a, i64 %b, i64 %c, i64*%rp) { +; CHECK-LABEL: @remainder_not_triangle_i32( +; CHECK-NEXT: entry: +; CHECK-NEXT: switch i64 [[C:%.*]], label [[RETURN:%.*]] [ +; CHECK-NEXT: i64 0, label [[SW_BB:%.*]] +; CHECK-NEXT: i64 1, label [[SW_BB1:%.*]] +; 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_BB1]] +; CHECK: sw.bb1: +; CHECK-NEXT: [[DIV:%.*]] = udiv i64 [[A]], [[B]] +; CHECK-NEXT: br label [[RETURN]] +; CHECK: return: +; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i64 [ [[DIV]], [[SW_BB1]] ], [ 0, [[ENTRY:%.*]] ] +; CHECK-NEXT: ret i64 [[RETVAL_0]] +; +entry: + switch i64 %c, label %return [ + i64 0, label %sw.bb + i64 1, label %sw.bb1 + ] + +sw.bb: ; preds = %entry + %rem = urem i64 %a, %b + store i64 %rem, i64* %rp + br label %sw.bb1 + +sw.bb1: ; preds = %entry, %sw.bb + %div = udiv i64 %a, %b + br label %return + +return: ; preds = %entry, %sw.bb1 + %retval.0 = phi i64 [ %div, %sw.bb1 ], [ 0, %entry ] + ret i64 %retval.0 +} + +; Negative test. The urem block has a successor that isn't udiv. +define i64 @remainder_not_triangle_i32_2(i64 %a, i64 %b, i64 %c, i64* %rp) { +; CHECK-LABEL: @remainder_not_triangle_i32_2( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[TOBOOL_NOT:%.*]] = icmp eq i64* [[RP:%.*]], null +; CHECK-NEXT: br i1 [[TOBOOL_NOT]], label [[IF_END3:%.*]], label [[IF_THEN:%.*]] +; CHECK: if.then: +; CHECK-NEXT: [[REM:%.*]] = urem i64 [[A:%.*]], [[B:%.*]] +; CHECK-NEXT: store i64 [[REM]], i64* [[RP]], align 4 +; CHECK-NEXT: [[TOBOOL1_NOT:%.*]] = icmp eq i64 [[C:%.*]], 0 +; CHECK-NEXT: br i1 [[TOBOOL1_NOT]], label [[IF_END3]], label [[RETURN:%.*]] +; CHECK: if.end3: +; CHECK-NEXT: [[DIV:%.*]] = udiv i64 [[A]], [[B]] +; CHECK-NEXT: br label [[RETURN]] +; CHECK: return: +; CHECK-NEXT: [[RETVAL_0:%.*]] = phi i64 [ [[DIV]], [[IF_END3]] ], [ 0, [[IF_THEN]] ] +; CHECK-NEXT: ret i64 [[RETVAL_0]] +; +entry: + %tobool.not = icmp eq i64* %rp, null + br i1 %tobool.not, label %if.end3, label %if.then + +if.then: ; preds = %entry + %rem = urem i64 %a, %b + store i64 %rem, i64* %rp + %tobool1.not = icmp eq i64 %c, 0 + br i1 %tobool1.not, label %if.end3, label %return + +if.end3: ; preds = %if.then, %entry + %div = udiv i64 %a, %b + br label %return + +return: ; preds = %if.then, %if.end3 + %retval.0 = phi i64 [ %div, %if.end3 ], [ 0, %if.then ] + ret i64 %retval.0 +}