[llvm] 8bca60f - [SelectionDAG][GlobalISel] Don't use UnsignedDivisionByConstantInfo for divisor of 1.
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 4 10:02:09 PST 2023
Author: Craig Topper
Date: 2023-01-04T10:01:15-08:00
New Revision: 8bca60fb0aa5f82d32e619d75e65d0dea2472722
URL: https://github.com/llvm/llvm-project/commit/8bca60fb0aa5f82d32e619d75e65d0dea2472722
DIFF: https://github.com/llvm/llvm-project/commit/8bca60fb0aa5f82d32e619d75e65d0dea2472722.diff
LOG: [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
Added:
Modified:
llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
llvm/lib/Support/DivisionByConstantInfo.cpp
llvm/unittests/Support/DivisionByConstantTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
index 89ebcdccdf173..9899875e47c56 100644
--- 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 e9ef26a138ad1..310fe14c378e0 100644
--- 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 b795c09c2257c..a40f49ba69bb3 100644
--- 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 90ae9180c50f9..43e44c9b81c4d 100644
--- 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) {
More information about the llvm-commits
mailing list