Skip to content

[ValueTracking] Support GEPs in matchSimpleRecurrence. #123518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion llvm/include/llvm/Analysis/ValueTracking.h
Original file line number Diff line number Diff line change
Expand Up @@ -1245,7 +1245,11 @@ bool matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO, Value *&Start,
Value *&Step);

/// Analogous to the above, but starting from the binary operator
bool matchSimpleRecurrence(const BinaryOperator *I, PHINode *&P, Value *&Start,
bool matchSimpleRecurrence(const Instruction *I, PHINode *&P, Value *&Start,
Value *&Step);

/// Analogous to the above, but also supporting non-binary operators.
bool matchSimpleRecurrence(const PHINode *P, Instruction *&BO, Value *&Start,
Value *&Step);

/// Return true if RHS is known to be implied true by LHS. Return false if
Expand Down
48 changes: 37 additions & 11 deletions llvm/lib/Analysis/ValueTracking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1524,7 +1524,7 @@ static void computeKnownBitsFromOperator(const Operator *I,
}
case Instruction::PHI: {
const PHINode *P = cast<PHINode>(I);
BinaryOperator *BO = nullptr;
Instruction *BO = nullptr;
Copy link
Contributor

@goldsteinn goldsteinn Jan 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think BO should be renamed. That's probably not worth it given the large amount of unrelated diffs it will create.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could rename it as NFC after the change lands?

Value *R = nullptr, *L = nullptr;
if (matchSimpleRecurrence(P, BO, R, L)) {
// Handle the case of a simple two-predecessor recurrence PHI.
Expand Down Expand Up @@ -1588,6 +1588,7 @@ static void computeKnownBitsFromOperator(const Operator *I,
case Instruction::Sub:
case Instruction::And:
case Instruction::Or:
case Instruction::GetElementPtr:
case Instruction::Mul: {
// Change the context instruction to the "edge" that flows into the
// phi. This is important because that is where the value is actually
Expand All @@ -1606,6 +1607,11 @@ static void computeKnownBitsFromOperator(const Operator *I,

// We need to take the minimum number of known bits
KnownBits Known3(BitWidth);
if (BitWidth != getBitWidth(L->getType(), Q.DL)) {
assert(isa<GetElementPtrInst>(BO) &&
"Bitwidth should only be different for GEPs.");
break;
}
RecQ.CxtI = LInst;
computeKnownBits(L, DemandedElts, Known3, Depth + 1, RecQ);

Expand Down Expand Up @@ -1768,6 +1774,7 @@ static void computeKnownBitsFromOperator(const Operator *I,
Known.resetAll();
}
}

if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
switch (II->getIntrinsicID()) {
default:
Expand Down Expand Up @@ -2301,7 +2308,7 @@ void computeKnownBits(const Value *V, const APInt &DemandedElts,
/// always a power of two (or zero).
static bool isPowerOfTwoRecurrence(const PHINode *PN, bool OrZero,
unsigned Depth, SimplifyQuery &Q) {
BinaryOperator *BO = nullptr;
Instruction *BO = nullptr;
Value *Start = nullptr, *Step = nullptr;
if (!matchSimpleRecurrence(PN, BO, Start, Step))
return false;
Expand Down Expand Up @@ -2339,7 +2346,7 @@ static bool isPowerOfTwoRecurrence(const PHINode *PN, bool OrZero,
// Divisor must be a power of two.
// If OrZero is false, cannot guarantee induction variable is non-zero after
// division, same for Shr, unless it is exact division.
return (OrZero || Q.IIQ.isExact(BO)) &&
return (OrZero || Q.IIQ.isExact(cast<BinaryOperator>(BO))) &&
isKnownToBeAPowerOfTwo(Step, false, Depth, Q);
case Instruction::Shl:
return OrZero || Q.IIQ.hasNoUnsignedWrap(BO) || Q.IIQ.hasNoSignedWrap(BO);
Expand All @@ -2348,7 +2355,7 @@ static bool isPowerOfTwoRecurrence(const PHINode *PN, bool OrZero,
return false;
[[fallthrough]];
case Instruction::LShr:
return OrZero || Q.IIQ.isExact(BO);
return OrZero || Q.IIQ.isExact(cast<BinaryOperator>(BO));
default:
return false;
}
Expand Down Expand Up @@ -2760,7 +2767,7 @@ static bool rangeMetadataExcludesValue(const MDNode* Ranges, const APInt& Value)
/// Try to detect a recurrence that monotonically increases/decreases from a
/// non-zero starting value. These are common as induction variables.
static bool isNonZeroRecurrence(const PHINode *PN) {
BinaryOperator *BO = nullptr;
Instruction *BO = nullptr;
Value *Start = nullptr, *Step = nullptr;
const APInt *StartC, *StepC;
if (!matchSimpleRecurrence(PN, BO, Start, Step) ||
Expand Down Expand Up @@ -3593,9 +3600,9 @@ getInvertibleOperands(const Operator *Op1,
// If PN1 and PN2 are both recurrences, can we prove the entire recurrences
// are a single invertible function of the start values? Note that repeated
// application of an invertible function is also invertible
BinaryOperator *BO1 = nullptr;
Instruction *BO1 = nullptr;
Value *Start1 = nullptr, *Step1 = nullptr;
BinaryOperator *BO2 = nullptr;
Instruction *BO2 = nullptr;
Value *Start2 = nullptr, *Step2 = nullptr;
if (PN1->getParent() != PN2->getParent() ||
!matchSimpleRecurrence(PN1, BO1, Start1, Step1) ||
Expand Down Expand Up @@ -9215,6 +9222,16 @@ llvm::canConvertToMinOrMaxIntrinsic(ArrayRef<Value *> VL) {

bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
Value *&Start, Value *&Step) {
Instruction *I;
if (matchSimpleRecurrence(P, I, Start, Step)) {
BO = dyn_cast<BinaryOperator>(I);
return BO;
}
return false;
}

bool llvm::matchSimpleRecurrence(const PHINode *P, Instruction *&BO,
Value *&Start, Value *&Step) {
// Handle the case of a simple two-predecessor recurrence PHI.
// There's a lot more that could theoretically be done here, but
// this is sufficient to catch some interesting cases.
Expand All @@ -9224,15 +9241,15 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
for (unsigned i = 0; i != 2; ++i) {
Value *L = P->getIncomingValue(i);
Value *R = P->getIncomingValue(!i);
auto *LU = dyn_cast<BinaryOperator>(L);
auto *LU = dyn_cast<Instruction>(L);
if (!LU)
continue;
unsigned Opcode = LU->getOpcode();

switch (Opcode) {
default:
continue;
// TODO: Expand list -- xor, gep, uadd.sat etc.
// TODO: Expand list -- xor, uadd.sat etc.
case Instruction::LShr:
case Instruction::AShr:
case Instruction::Shl:
Expand All @@ -9256,6 +9273,15 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,

break; // Match!
}
case Instruction::GetElementPtr: {
Value *LR;
if (match(L, m_PtrAdd(m_Specific(P), m_Value(LR)))) {
// Found a match
L = LR;
break;
}
continue;
}
};

// We have matched a recurrence of the form:
Expand All @@ -9272,9 +9298,9 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
return false;
}

bool llvm::matchSimpleRecurrence(const BinaryOperator *I, PHINode *&P,
bool llvm::matchSimpleRecurrence(const Instruction *I, PHINode *&P,
Value *&Start, Value *&Step) {
BinaryOperator *BO = nullptr;
Instruction *BO = nullptr;
P = dyn_cast<PHINode>(I->getOperand(0));
if (!P)
P = dyn_cast<PHINode>(I->getOperand(1));
Expand Down
Loading