[BypassSlowDivision] Handle division by constant numerators properly.

[jlebar]

Summary

We don't do BypassSlowDivision when the denominator is a constant, but
we do do it when the numerator is a constant.

This patch makes two related changes to BypassSlowDivision when the
numerator is a constant:

• If the numerator is too large to fit into the bypass width, don't bypass slow division (because we'll never run the smaller-width code).
• If we bypass slow division where the numerator is a constant, don't OR together the numerator and denominator when determining whether both operands fit within the bypass width. We need to check only the denominator.

[tra]

llvm/lib/Transforms/Utils/BypassSlowDivision.cpp

86–87 ↗

(On Diff #78074)

This condition appears to be tautological.
AFAICT, it's equivalent to just if(isa<ConstantInt>(Divisor)).

160 ↗

(On Diff #78074)

This does not parse.
Perhaps it was meant to be "We ensured above" or "We bailed out above if divisor *was* constant".

163 ↗

(On Diff #78074)

Given that we do bail out on all constant divisors, this assert seems to be a bit redundant.

[jlebar]

llvm/lib/Transforms/Utils/BypassSlowDivision.cpp

86–87 ↗

(On Diff #78074)

lol, if (A || (A && B)).

I will fix this in a separate patch.

160 ↗

(On Diff #78074)

Fixed, thanks.

163 ↗

(On Diff #78074)

Well, yes, that's why the assertion is safe? :) The intent is to ensure that the behavior above doesn't change without the logic down here also changing.

[jlebar]

Fix comment

[tra]

Comment nit. OK otherwise.

llvm/lib/Transforms/Utils/BypassSlowDivision.cpp

160 ↗

(On Diff #78074)

Now it parses, but still does not seem to match the code.
We bail out if divisor *is* constant.

llvm/trunk/lib/Transforms/Utils/BypassSlowDivision.cpp

static bool insertFastDiv(Instruction *I, IntegerType *BypassType,
   Value *Divisor = I->getOperand(1);
 
   if (isa<ConstantInt>(Divisor)) {
     // Division by a constant should have been been solved and replaced earlier
     // in the pipeline.
     return false;
   }
 
+  // If the numerator is a constant, bail if it doesn't fit into BypassType.
+  if (ConstantInt *ConstDividend = dyn_cast<ConstantInt>(Dividend))
+    if (ConstDividend->getValue().getActiveBits() > BypassType->getBitWidth())
+      return false;
+
   // Basic Block is split before divide
   BasicBlock *MainBB = &*I->getParent();
   BasicBlock *SuccessorBB = MainBB->splitBasicBlock(I);
 
   // Add new basic block for slow divide operation
   BasicBlock *SlowBB =
       BasicBlock::Create(F->getContext(), "", MainBB->getParent(), SuccessorBB);
   SlowBB->moveBefore(SuccessorBB);
if (UseDivOp)
     I->replaceAllUsesWith(QuoPhi);
   else
     I->replaceAllUsesWith(RemPhi);
   I->eraseFromParent();
 
   // Combine operands into a single value with OR for value testing below
   MainBB->getInstList().back().eraseFromParent();
   IRBuilder<> MainBuilder(MainBB, MainBB->end());
-  Value *OrV = MainBuilder.CreateOr(Dividend, Divisor);
+
+  // We should have bailed out above if the divisor is a constant, but the
+  // dividend may still be a constant.  Set OrV to our non-constant operands
+  // OR'ed together.
+  assert(!isa<ConstantInt>(Divisor));
+
+  Value *OrV;
+  if (!isa<ConstantInt>(Dividend))
+    OrV = MainBuilder.CreateOr(Dividend, Divisor);
+  else
+    OrV = Divisor;
 
   // BitMask is inverted to check if the operands are
   // larger than the bypass type
   uint64_t BitMask = ~BypassType->getBitMask();
   Value *AndV = MainBuilder.CreateAnd(OrV, BitMask);
 
   // Compare operand values and branch
   Value *ZeroV = ConstantInt::getSigned(Dividend->getType(), 0);

llvm/trunk/test/Transforms/CodeGenPrepare/NVPTX/bypass-slow-div-constant-numerator.ll

+; RUN: opt -S -codegenprepare < %s | FileCheck %s
+
+target datalayout = "e-i64:64-v16:16-v32:32-n16:32:64"
+target triple = "nvptx64-nvidia-cuda"
+
+; When we bypass slow div with a constant numerator which fits into the bypass
+; width, we still emit the bypass code, but we don't 'or' the numerator with
+; the denominator.
+; CHECK-LABEL: @small_constant_numer
+define i64 @small_constant_numer(i64 %a) {
+  ; CHECK: [[AND:%[0-9]+]] = and i64 %a, -4294967296
+  ; CHECK: icmp eq i64 [[AND]], 0
+
+  ; CHECK: [[TRUNC:%[0-9]+]] = trunc i64 %a to i32
+  ; CHECK: udiv i32 -1, [[TRUNC]]
+  %d = sdiv i64 4294967295, %a  ; 0xffff'ffff
+  ret i64 %d
+}
+
+; When we try to bypass slow div with a constant numerator which *doesn't* fit
+; into the bypass width, leave it as a plain 64-bit div with no bypass.
+; CHECK-LABEL: @large_constant_numer
+define i64 @large_constant_numer(i64 %a) {
+  ; CHECK-NOT: udiv i32
+  %d = sdiv i64 4294967296, %a  ; 0x1'0000'0000
+  ret i64 %d
+}
+
+; For good measure, try a value larger than 2^32.
+; CHECK-LABEL: @larger_constant_numer
+define i64 @larger_constant_numer(i64 %a) {
+  ; CHECK-NOT: udiv i32
+  %d = sdiv i64 5000000000, %a
+  ret i64 %d
+}