[PATCH] D27246: Bug fix and a proposed improvement in integer division expansion

[Dmitri Shtilman via Phabricator via llvm-commits]

dshtilman created this revision.
dshtilman added a reviewer: vsk.
dshtilman added subscribers: llvm-commits, qcolombet.
dshtilman set the repository for this revision to rL LLVM.

Another improvement in integer division sw expansion and a bug fix.

The special case code in udiv expansion is currently broken, namely the check for divisor==1 :

  // ; special-cases:
  // ;   %ret0_1      = icmp eq i32 %divisor, 0
  // ;   %ret0_2      = icmp eq i32 %dividend, 0
  // ;   %ret0_3      = or i1 %ret0_1, %ret0_2
  // ;   %tmp0        = tail call i32 @llvm.ctlz.i32(i32 %divisor, i1 true)
  // ;   %tmp1        = tail call i32 @llvm.ctlz.i32(i32 %dividend, i1 true)
  // ;   %sr          = sub nsw i32 %tmp0, %tmp1
  // ;   %ret0_4      = icmp ugt i32 %sr, 31
  // ;   %ret0        = or i1 %ret0_3, %ret0_4
  // ;   %retDividend = icmp eq i32 **%sr**, 31
  // ;   %retVal      = select i1 %ret0, i32 0, i32 %dividend
  // ;   %earlyRet    = or i1 %ret0, %retDividend
  // ;   br i1 %earlyRet, label %end, label %bb1

The intention must have been to use **%tmp0**, not **%sr**:

  // ;   %retDividend = icmp eq i32 **%tmp0**, 31

The following patch fixes this special case and also extends it to cover divisions by a power-of-2 which will be routed through a simple shift-right instead of going through the full division loop. Division by 1 will also go through the shift-right, yielding the dividend [shifted by 0]. Power-of-2 divisors are identified through adding trailing zeroes to the already calculated leading zeroes to verify that only one bit is set.

Please review.
Dmitri


Repository:
  rL LLVM

https://reviews.llvm.org/D27246

Files:
  lib/Transforms/Utils/IntegerDivision.cpp
  unittests/Transforms/Utils/IntegerDivision.cpp


Index: unittests/Transforms/Utils/IntegerDivision.cpp
===================================================================
--- unittests/Transforms/Utils/IntegerDivision.cpp
+++ unittests/Transforms/Utils/IntegerDivision.cpp
@@ -78,6 +78,12 @@
 
   Instruction* Quotient = dyn_cast<Instruction>(cast<User>(Ret)->getOperand(0));
   EXPECT_TRUE(Quotient && Quotient->getOpcode() == Instruction::PHI);
+
+  SelectInst* EarlyRetVal = dyn_cast<SelectInst>(cast<User>(Quotient)->getOperand(1));
+  EXPECT_TRUE(EarlyRetVal);
+
+  Instruction* ShrDividend = dyn_cast<Instruction>(EarlyRetVal->getFalseValue());
+  EXPECT_TRUE(ShrDividend && ShrDividend->getOpcode() == Instruction::LShr);
 }
 
 TEST(IntegerDivision, SRem) {
Index: lib/Transforms/Utils/IntegerDivision.cpp
===================================================================
--- lib/Transforms/Utils/IntegerDivision.cpp
+++ lib/Transforms/Utils/IntegerDivision.cpp
@@ -182,6 +182,8 @@
   Function *F = IBB->getParent();
   Function *CTLZ = Intrinsic::getDeclaration(F->getParent(), Intrinsic::ctlz,
                                              DivTy);
+  Function *CTTZ = Intrinsic::getDeclaration(F->getParent(), Intrinsic::cttz,
+                                             DivTy);
 
   // Our CFG is going to look like:
   // +---------------------+
@@ -234,7 +236,7 @@
   // Same instructions are generated for both i32 (msb 31) and i64 (msb 63).
 
   // First off, check for special cases: dividend or divisor is zero, divisor
-  // is greater than dividend, and divisor is 1.
+  // is greater than dividend, and divisor is a power of 2.
   // ; special-cases:
   // ;   %ret0_1      = icmp eq i32 %divisor, 0
   // ;   %ret0_2      = icmp eq i32 %dividend, 0
@@ -244,8 +246,11 @@
   // ;   %sr          = sub nsw i32 %tmp0, %tmp1
   // ;   %ret0_4      = icmp ugt i32 %sr, 31
   // ;   %ret0        = or i1 %ret0_3, %ret0_4
-  // ;   %retDividend = icmp eq i32 %sr, 31
-  // ;   %retVal      = select i1 %ret0, i32 0, i32 %dividend
+  // ;   %tmp14       = tail call i32 @llvm.cttz.i32(i32 %divisor, i1 true)
+  // ;   %tmp15       = add i32 %tmp0, %tmp14
+  // ;   %retDividend = icmp eq i32 %tmp15, 31
+  // ;   %shrDividend = lshr i32 %dividend, %tmp14
+  // ;   %retVal      = select i1 %ret0, i32 0, i32 %shrDividend
   // ;   %earlyRet    = or i1 %ret0, %retDividend
   // ;   br i1 %earlyRet, label %end, label %bb1
   Builder.SetInsertPoint(SpecialCases);
@@ -257,8 +262,11 @@
   Value *SR          = Builder.CreateSub(Tmp0, Tmp1);
   Value *Ret0_4      = Builder.CreateICmpUGT(SR, MSB);
   Value *Ret0        = Builder.CreateOr(Ret0_3, Ret0_4);
-  Value *RetDividend = Builder.CreateICmpEQ(SR, MSB);
-  Value *RetVal      = Builder.CreateSelect(Ret0, Zero, Dividend);
+  Value *Tmp14       = Builder.CreateCall(CTTZ, {Divisor, True});
+  Value *Tmp15       = Builder.CreateAdd(Tmp0, Tmp14);
+  Value *RetDividend = Builder.CreateICmpEQ(Tmp15, MSB);
+  Value *ShrDividend = Builder.CreateLShr(Dividend, Tmp14);
+  Value *RetVal      = Builder.CreateSelect(Ret0, Zero, ShrDividend);
   Value *EarlyRet    = Builder.CreateOr(Ret0, RetDividend);
   Builder.CreateCondBr(EarlyRet, End, BB1);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D27246.79695.patch
Type: text/x-patch
Size: 3163 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161130/a598d803/attachment.bin>