[llvm] r314482 - Revert "[BypassSlowDivision] Improve our handling of divisions by constants"

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 28 17:54:16 PDT 2017


Author: sanjoy
Date: Thu Sep 28 17:54:16 2017
New Revision: 314482

URL: http://llvm.org/viewvc/llvm-project?rev=314482&view=rev
Log:
Revert "[BypassSlowDivision] Improve our handling of divisions by constants"

This reverts commit r314253.  It causes a miscompile on P100 in an internal
benchmark.  Reverting while I investigate.

Modified:
    llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp
    llvm/trunk/test/Transforms/CodeGenPrepare/NVPTX/bypass-slow-div.ll

Modified: llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp?rev=314482&r1=314481&r2=314482&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp Thu Sep 28 17:54:16 2017
@@ -339,6 +339,11 @@ Optional<QuotRemPair> FastDivInsertionTa
   Value *Dividend = SlowDivOrRem->getOperand(0);
   Value *Divisor = SlowDivOrRem->getOperand(1);
 
+  if (isa<ConstantInt>(Divisor)) {
+    // Keep division by a constant for DAGCombiner.
+    return None;
+  }
+
   VisitedSetTy SetL;
   ValueRange DividendRange = getValueRange(Dividend, SetL);
   if (DividendRange == VALRNG_LIKELY_LONG)
@@ -354,9 +359,7 @@ Optional<QuotRemPair> FastDivInsertionTa
 
   if (DividendShort && DivisorShort) {
     // If both operands are known to be short then just replace the long
-    // division with a short one in-place.  Since we're not introducing control
-    // flow in this case, narrowing the division is always a win, even if the
-    // divisor is a constant (and will later get replaced by a multiplication).
+    // division with a short one in-place.
 
     IRBuilder<> Builder(SlowDivOrRem);
     Value *TruncDividend = Builder.CreateTrunc(Dividend, BypassType);
@@ -366,16 +369,7 @@ Optional<QuotRemPair> FastDivInsertionTa
     Value *ExtDiv = Builder.CreateZExt(TruncDiv, getSlowType());
     Value *ExtRem = Builder.CreateZExt(TruncRem, getSlowType());
     return QuotRemPair(ExtDiv, ExtRem);
-  }
-
-  if (isa<ConstantInt>(Divisor)) {
-    // If the divisor is not a constant, DAGCombiner will convert it to a
-    // multiplication by a magic constant.  It isn't clear if it is worth
-    // introducing control flow to get a narrower multiply.
-    return None;
-  }
-
-  if (DividendShort && !isSignedOp()) {
+  } else if (DividendShort && !isSignedOp()) {
     // If the division is unsigned and Dividend is known to be short, then
     // either
     // 1) Divisor is less or equal to Dividend, and the result can be computed

Modified: llvm/trunk/test/Transforms/CodeGenPrepare/NVPTX/bypass-slow-div.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/CodeGenPrepare/NVPTX/bypass-slow-div.ll?rev=314482&r1=314481&r2=314482&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/CodeGenPrepare/NVPTX/bypass-slow-div.ll (original)
+++ llvm/trunk/test/Transforms/CodeGenPrepare/NVPTX/bypass-slow-div.ll Thu Sep 28 17:54:16 2017
@@ -27,80 +27,3 @@ define void @rem_only(i64 %a, i64 %b, i6
   store i64 %d, i64* %retptr
   ret void
 }
-
-; CHECK-LABEL: @udiv_by_constant(
-define i64 @udiv_by_constant(i32 %a) {
-; CHECK-NEXT:    [[A_ZEXT:%.*]] = zext i32 [[A:%.*]] to i64
-; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[A_ZEXT]] to i32
-; CHECK-NEXT:    [[TMP2:%.*]] = udiv i32 [[TMP1]], 50
-; CHECK-NEXT:    [[TMP3:%.*]] = zext i32 [[TMP2]] to i64
-; CHECK-NEXT:    ret i64 [[TMP3]]
-
-  %a.zext = zext i32 %a to i64
-  %wide.div = udiv i64 %a.zext, 50
-  ret i64 %wide.div
-}
-
-; CHECK-LABEL: @urem_by_constant(
-define i64 @urem_by_constant(i32 %a) {
-; CHECK-NEXT:    [[A_ZEXT:%.*]] = zext i32 [[A:%.*]] to i64
-; CHECK-NEXT:    [[TMP1:%.*]] = trunc i64 [[A_ZEXT]] to i32
-; CHECK-NEXT:    [[TMP2:%.*]] = urem i32 [[TMP1]], 50
-; CHECK-NEXT:    [[TMP3:%.*]] = zext i32 [[TMP2]] to i64
-; CHECK-NEXT:    ret i64 [[TMP3]]
-
-  %a.zext = zext i32 %a to i64
-  %wide.div = urem i64 %a.zext, 50
-  ret i64 %wide.div
-}
-
-; Negative test: instead of emitting a runtime check on %a, we prefer to let the
-; DAGCombiner transform this division by constant into a multiplication (with a
-; "magic constant").
-;
-; CHECK-LABEL: @udiv_by_constant_negative_0(
-define i64 @udiv_by_constant_negative_0(i64 %a) {
-; CHECK-NEXT:    [[WIDE_DIV:%.*]] = udiv i64 [[A:%.*]], 50
-; CHECK-NEXT:    ret i64 [[WIDE_DIV]]
-
-  %wide.div = udiv i64 %a, 50
-  ret i64 %wide.div
-}
-
-; Negative test: while we know the dividend is short, the divisor isn't.  This
-; test is here for completeness, but instcombine will optimize this to return 0.
-;
-; CHECK-LABEL: @udiv_by_constant_negative_1(
-define i64 @udiv_by_constant_negative_1(i32 %a) {
-; CHECK-NEXT:    [[A_ZEXT:%.*]] = zext i32 [[A:%.*]] to i64
-; CHECK-NEXT:    [[WIDE_DIV:%.*]] = udiv i64 [[A_ZEXT]], 8589934592
-; CHECK-NEXT:    ret i64 [[WIDE_DIV]]
-
-  %a.zext = zext i32 %a to i64
-  %wide.div = udiv i64 %a.zext, 8589934592 ;; == 1 << 33
-  ret i64 %wide.div
-}
-
-; URem version of udiv_by_constant_negative_0
-;
-; CHECK-LABEL: @urem_by_constant_negative_0(
-define i64 @urem_by_constant_negative_0(i64 %a) {
-; CHECK-NEXT:    [[WIDE_DIV:%.*]] = urem i64 [[A:%.*]], 50
-; CHECK-NEXT:    ret i64 [[WIDE_DIV]]
-
-  %wide.div = urem i64 %a, 50
-  ret i64 %wide.div
-}
-
-; URem version of udiv_by_constant_negative_1
-;
-; CHECK-LABEL: @urem_by_constant_negative_1(
-define i64 @urem_by_constant_negative_1(i32 %a) {
-; CHECK-NEXT:    [[A_ZEXT:%.*]] = zext i32 [[A:%.*]] to i64
-; CHECK-NEXT:    [[WIDE_DIV:%.*]] = urem i64 [[A_ZEXT]], 8589934592
-; CHECK-NEXT:    ret i64 [[WIDE_DIV]]
-
-  %a.zext = zext i32 %a to i64
-  %wide.div = urem i64 %a.zext, 8589934592 ;; == 1 << 33
-  ret i64 %wide.div
-}




More information about the llvm-commits mailing list