Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Dec 6, 2023

Operator allows the phi operand to be a ConstantExpr. A ConstantExpr is a valid operand to a phi, but a ConstantExpr is never going to be a recurrence. We can only match a BinaryOperator so use that instead.

…urrence.

Operator allows the phi operand to be a ConstantExpr. A ConstantExpr
is a valid operand to a phi, but a ConstantExpr is never going to be a
recurrence. So I think we should use Instruction.
@topperc topperc requested a review from preames December 6, 2023 23:47
@topperc topperc requested a review from nikic as a code owner December 6, 2023 23:47
@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Dec 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-llvm-analysis

Author: Craig Topper (topperc)

Changes

Operator allows the phi operand to be a ConstantExpr. A ConstantExpr is a valid operand to a phi, but a ConstantExpr is never going to be a recurrence. So I think we should use Instruction.


Full diff: https://github.com/llvm/llvm-project/pull/74678.diff

1 Files Affected:

  • (modified) llvm/lib/Analysis/ValueTracking.cpp (+1-1)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index ee4f97f3bf5e0f..78b791d1d3b331 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -8024,7 +8024,7 @@ 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);
-    Operator *LU = dyn_cast<Operator>(L);
+    auto *LU = dyn_cast<Instruction>(L);
     if (!LU)
       continue;
     unsigned Opcode = LU->getOpcode();

@topperc topperc changed the title [ValueTracking] Use Instruction instead of Operator in matchSimpleRecurrence. [ValueTracking] Use BinaryOperator instead of Operator in matchSimpleRecurrence. Dec 7, 2023
Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Collaborator

@preames preames left a comment

Choose a reason for hiding this comment

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

LGTM

@topperc topperc merged commit 32ec5fb into llvm:main Dec 7, 2023
@topperc topperc deleted the pr/match-simple branch December 7, 2023 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants