ValueTracking: refactor recurrence-matching (NFC) #109659
Conversation
|
@llvm/pr-subscribers-llvm-analysis Author: Ramkumar Ramachandra (artagnon) ChangesFull diff: https://github.com/llvm/llvm-project/pull/109659.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 56eb3f99b39d2c..af3420b6280a39 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1419,9 +1419,12 @@ static void computeKnownBitsFromOperator(const Operator *I,
// If this is a shift recurrence, we know the bits being shifted in.
// We can combine that with information about the start value of the
// recurrence to conclude facts about the result.
- if ((Opcode == Instruction::LShr || Opcode == Instruction::AShr ||
- Opcode == Instruction::Shl) &&
- BO->getOperand(0) == I) {
+ switch (Opcode) {
+ case Instruction::LShr:
+ case Instruction::AShr:
+ case Instruction::Shl: {
+ if (BO->getOperand(0) != I)
+ break;
// We have matched a recurrence of the form:
// %iv = [R, %entry], [%iv.next, %backedge]
@@ -1449,17 +1452,18 @@ static void computeKnownBitsFromOperator(const Operator *I,
Known.Zero.setHighBits(Known2.countMinLeadingZeros());
Known.One.setHighBits(Known2.countMinLeadingOnes());
break;
- };
+ }
+ break;
}
// Check for operations that have the property that if
// both their operands have low zero bits, the result
// will have low zero bits.
- if (Opcode == Instruction::Add ||
- Opcode == Instruction::Sub ||
- Opcode == Instruction::And ||
- Opcode == Instruction::Or ||
- Opcode == Instruction::Mul) {
+ case Instruction::Add:
+ case Instruction::Sub:
+ case Instruction::And:
+ case Instruction::Or:
+ 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
// "evaluated" even though it is used later somewhere else. (see also
@@ -1484,38 +1488,51 @@ static void computeKnownBitsFromOperator(const Operator *I,
Known3.countMinTrailingZeros()));
auto *OverflowOp = dyn_cast<OverflowingBinaryOperator>(BO);
- if (OverflowOp && Q.IIQ.hasNoSignedWrap(OverflowOp)) {
- // If initial value of recurrence is nonnegative, and we are adding
- // a nonnegative number with nsw, the result can only be nonnegative
- // or poison value regardless of the number of times we execute the
- // add in phi recurrence. If initial value is negative and we are
- // adding a negative number with nsw, the result can only be
- // negative or poison value. Similar arguments apply to sub and mul.
- //
- // (add non-negative, non-negative) --> non-negative
- // (add negative, negative) --> negative
- if (Opcode == Instruction::Add) {
- if (Known2.isNonNegative() && Known3.isNonNegative())
- Known.makeNonNegative();
- else if (Known2.isNegative() && Known3.isNegative())
- Known.makeNegative();
- }
+ if (!OverflowOp || !Q.IIQ.hasNoSignedWrap(OverflowOp))
+ break;
- // (sub nsw non-negative, negative) --> non-negative
- // (sub nsw negative, non-negative) --> negative
- else if (Opcode == Instruction::Sub && BO->getOperand(0) == I) {
- if (Known2.isNonNegative() && Known3.isNegative())
- Known.makeNonNegative();
- else if (Known2.isNegative() && Known3.isNonNegative())
- Known.makeNegative();
- }
+ switch (Opcode) {
+ // If initial value of recurrence is nonnegative, and we are adding
+ // a nonnegative number with nsw, the result can only be nonnegative
+ // or poison value regardless of the number of times we execute the
+ // add in phi recurrence. If initial value is negative and we are
+ // adding a negative number with nsw, the result can only be
+ // negative or poison value. Similar arguments apply to sub and mul.
+ //
+ // (add non-negative, non-negative) --> non-negative
+ // (add negative, negative) --> negative
+ case Instruction::Add: {
+ if (Known2.isNonNegative() && Known3.isNonNegative())
+ Known.makeNonNegative();
+ else if (Known2.isNegative() && Known3.isNegative())
+ Known.makeNegative();
+ break;
+ }
- // (mul nsw non-negative, non-negative) --> non-negative
- else if (Opcode == Instruction::Mul && Known2.isNonNegative() &&
- Known3.isNonNegative())
+ // (sub nsw non-negative, negative) --> non-negative
+ // (sub nsw negative, non-negative) --> negative
+ case Instruction::Sub: {
+ if (BO->getOperand(0) != I)
+ break;
+ if (Known2.isNonNegative() && Known3.isNegative())
Known.makeNonNegative();
+ else if (Known2.isNegative() && Known3.isNonNegative())
+ Known.makeNegative();
+ break;
}
+ // (mul nsw non-negative, non-negative) --> non-negative
+ case Instruction::Mul:
+ if (Known2.isNonNegative() && Known3.isNonNegative())
+ Known.makeNonNegative();
+ break;
+
+ default:
+ break;
+ }
+ break;
+ }
+ default:
break;
}
}
|
|
LG As an aside, can you format your PR titles per the style guide ('[] <msg...>" as opposed to ": <msg...>") If you have doing this to avoid |
Is it specified in the style guide? I looked it up, but couldn't find a guideline saying this. I've been doing this for all my patches to save one valuable character in the title: it's a habit I picked up while contributing to upstream git. FWIW, there are some commits in the project that horribly overflow in both the title and body. |
I guess it isn't, just the norm.
Indeed, although that seems to be within the norms of the community :P |
|
Sorry, tests are failing, due to what appears to be an underlying bug. Kindly see #109794. |
|
All tests passing now. Could we kindly verify that the patch is an improvement? |
No description provided.