[SelectionDAG][GlobalISel] Don't use UnsignedDivisionByConstantInfo for divisor of 1.

The magic algorithm sets IsAdd indication for division by 1 that
the caller had to ignore.

I considered folding the ignore into UnsignedDivisionByConstantInfo,
but we only allow 1 for vectors of mixed visiors. And really what we
want to end up with is undef. Currently, we get to undef via
DemandedElts optimizations using the select instruction. We could
directly emit undef.

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

diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 89ebcdc..9899875 100644 (file)
--- a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
@@ -4948,26 +4948,35 @@ MachineInstr *CombinerHelper::buildUDivUsingMul(MachineInstr &MI) {
   auto BuildUDIVPattern = [&](const Constant *C) {
     auto *CI = cast<ConstantInt>(C);
     const APInt &Divisor = CI->getValue();
-    UnsignedDivisionByConstantInfo magics =
-        UnsignedDivisionByConstantInfo::get(Divisor);
+
+    bool SelNPQ = false;
+    APInt Magic(Divisor.getBitWidth(), 0);
     unsigned PreShift = 0, PostShift = 0;
 
-    unsigned SelNPQ;
-    if (!magics.IsAdd || Divisor.isOneValue()) {
-      assert(magics.ShiftAmount < Divisor.getBitWidth() &&
-             "We shouldn't generate an undefined shift!");
-      PostShift = magics.ShiftAmount;
-      PreShift = magics.PreShift;
-      SelNPQ = false;
-    } else {
-      assert(magics.PreShift == 0 && "Unexpected pre-shift");
-      PostShift = magics.ShiftAmount - 1;
-      SelNPQ = true;
+    // Magic algorithm doesn't work for division by 1. We need to emit a select
+    // at the end.
+    // TODO: Use undef values for divisor of 1.
+    if (!Divisor.isOneValue()) {
+      UnsignedDivisionByConstantInfo magics =
+          UnsignedDivisionByConstantInfo::get(Divisor);
+
+      Magic = std::move(magics.Magic);
+
+      if (!magics.IsAdd) {
+        assert(magics.ShiftAmount < Divisor.getBitWidth() &&
+               "We shouldn't generate an undefined shift!");
+        PostShift = magics.ShiftAmount;
+        PreShift = magics.PreShift;
+      } else {
+        assert(magics.PreShift == 0 && "Unexpected pre-shift");
+        PostShift = magics.ShiftAmount - 1;
+        SelNPQ = true;
+      }
     }
 
     PreShifts.push_back(
         MIB.buildConstant(ScalarShiftAmtTy, PreShift).getReg(0));
-    MagicFactors.push_back(MIB.buildConstant(ScalarTy, magics.Magic).getReg(0));
+    MagicFactors.push_back(MIB.buildConstant(ScalarTy, Magic).getReg(0));
     NPQFactors.push_back(
         MIB.buildConstant(ScalarTy,
                           SelNPQ ? APInt::getOneBitSet(EltBits, EltBits - 1)

diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index e9ef26a..310fe14 100644 (file)
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -6042,25 +6042,34 @@ SDValue TargetLowering::BuildUDIV(SDNode *N, SelectionDAG &DAG,
     // FIXME: We should use a narrower constant when the upper
     // bits are known to be zero.
     const APInt& Divisor = C->getAPIntValue();
-    UnsignedDivisionByConstantInfo magics =
-        UnsignedDivisionByConstantInfo::get(Divisor, LeadingZeros);
+
+    bool SelNPQ = false;
+    APInt Magic(Divisor.getBitWidth(), 0);
     unsigned PreShift = 0, PostShift = 0;
 
-    unsigned SelNPQ;
-    if (!magics.IsAdd || Divisor.isOne()) {
-      assert(magics.ShiftAmount < Divisor.getBitWidth() &&
-             "We shouldn't generate an undefined shift!");
-      PostShift = magics.ShiftAmount;
-      PreShift = magics.PreShift;
-      SelNPQ = false;
-    } else {
-      assert(magics.PreShift == 0 && "Unexpected pre-shift");
-      PostShift = magics.ShiftAmount - 1;
-      SelNPQ = true;
+    // Magic algorithm doesn't work for division by 1. We need to emit a select
+    // at the end.
+    // TODO: Use undef values for divisor of 1.
+    if (!Divisor.isOne()) {
+      UnsignedDivisionByConstantInfo magics =
+          UnsignedDivisionByConstantInfo::get(Divisor, LeadingZeros);
+
+      Magic = std::move(magics.Magic);
+
+      if (!magics.IsAdd) {
+        assert(magics.ShiftAmount < Divisor.getBitWidth() &&
+               "We shouldn't generate an undefined shift!");
+        PostShift = magics.ShiftAmount;
+        PreShift = magics.PreShift;
+      } else {
+        assert(magics.PreShift == 0 && "Unexpected pre-shift");
+        PostShift = magics.ShiftAmount - 1;
+        SelNPQ = true;
+      }
     }
 
     PreShifts.push_back(DAG.getConstant(PreShift, dl, ShSVT));
-    MagicFactors.push_back(DAG.getConstant(magics.Magic, dl, SVT));
+    MagicFactors.push_back(DAG.getConstant(Magic, dl, SVT));
     NPQFactors.push_back(
         DAG.getConstant(SelNPQ ? APInt::getOneBitSet(EltBits, EltBits - 1)
                                : APInt::getZero(EltBits),

diff --git a/llvm/lib/Support/DivisionByConstantInfo.cpp b/llvm/lib/Support/DivisionByConstantInfo.cpp
index b795c09..a40f49b 100644 (file)
--- a/llvm/lib/Support/DivisionByConstantInfo.cpp
+++ b/llvm/lib/Support/DivisionByConstantInfo.cpp
@@ -73,7 +73,7 @@ SignedDivisionByConstantInfo SignedDivisionByConstantInfo::get(const APInt &D) {
 UnsignedDivisionByConstantInfo
 UnsignedDivisionByConstantInfo::get(const APInt &D, unsigned LeadingZeros,
                                     bool AllowEvenDivisorOptimization) {
-  assert(!D.isZero() && "Precondition violation.");
+  assert(!D.isZero() && !D.isOne() && "Precondition violation.");
   assert(D.getBitWidth() > 1 && "Does not work at smaller bitwidths.");
 
   APInt Delta;

diff --git a/llvm/unittests/Support/DivisionByConstantTest.cpp b/llvm/unittests/Support/DivisionByConstantTest.cpp
index 90ae918..43e44c9 100644 (file)
--- a/llvm/unittests/Support/DivisionByConstantTest.cpp
+++ b/llvm/unittests/Support/DivisionByConstantTest.cpp
@@ -101,9 +101,11 @@ APInt UnsignedDivideUsingMagic(const APInt &Numerator, const APInt &Divisor,
                                bool LZOptimization,
                                bool AllowEvenDivisorOptimization, bool ForceNPQ,
                                UnsignedDivisionByConstantInfo Magics) {
+  assert(!Divisor.isOne() && "Division by 1 is not supported using Magic.");
+
   unsigned Bits = Numerator.getBitWidth();
 
-  if (LZOptimization && !Divisor.isOne()) {
+  if (LZOptimization) {
     unsigned LeadingZeros = Numerator.countLeadingZeros();
     // Clip to the number of leading zeros in the divisor.
     LeadingZeros = std::min(LeadingZeros, Divisor.countLeadingZeros());
@@ -117,7 +119,7 @@ APInt UnsignedDivideUsingMagic(const APInt &Numerator, const APInt &Divisor,
   unsigned PreShift = 0;
   unsigned PostShift = 0;
   bool UseNPQ = false;
-  if (!Magics.IsAdd || Divisor.isOne()) {
+  if (!Magics.IsAdd) {
     assert(Magics.ShiftAmount < Divisor.getBitWidth() &&
            "We shouldn't generate an undefined shift!");
     PreShift = Magics.PreShift;
@@ -154,7 +156,7 @@ APInt UnsignedDivideUsingMagic(const APInt &Numerator, const APInt &Divisor,
 
   Q = Q.lshr(PostShift);
 
-  return Divisor.isOne() ? Numerator : Q;
+  return Q;
 }
 
 TEST(UnsignedDivisionByConstantTest, Test) {
@@ -166,6 +168,9 @@ TEST(UnsignedDivisionByConstantTest, Test) {
     EnumerateAPInts(Bits, [Bits](const APInt &Divisor) {
       if (Divisor.isZero())
         return; // Division by zero is undefined behavior.
+      if (Divisor.isOne())
+        return; // Division by one is the numerator.
+
       const UnsignedDivisionByConstantInfo Magics =
           UnsignedDivisionByConstantInfo::get(Divisor);
       EnumerateAPInts(Bits, [Divisor, Magics, Bits](const APInt &Numerator) {