-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[InstSimplify] Avoid use of ConstantExpr::getICmp. NFC #67873
Conversation
@llvm/pr-subscribers-llvm-analysis ChangesAs I understand ICmp ConstantExpr are planned to be removed. Full diff: https://github.com/llvm/llvm-project/pull/67873.diff 1 Files Affected:
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 40a8f6a155b23d2..5bdff48f79b4657 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3916,12 +3916,12 @@ static Value *simplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS,
// is non-negative then LHS <s RHS.
case ICmpInst::ICMP_SGT:
case ICmpInst::ICMP_SGE:
- return ConstantExpr::getICmp(ICmpInst::ICMP_SLT, C,
- Constant::getNullValue(C->getType()));
+ return ConstantFoldCompareInstruction(
+ ICmpInst::ICMP_SLT, C, Constant::getNullValue(C->getType()));
case ICmpInst::ICMP_SLT:
case ICmpInst::ICMP_SLE:
- return ConstantExpr::getICmp(ICmpInst::ICMP_SGE, C,
- Constant::getNullValue(C->getType()));
+ return ConstantFoldCompareInstruction(
+ ICmpInst::ICMP_SGE, C, Constant::getNullValue(C->getType()));
}
}
}
|
@@ -3916,12 +3916,12 @@ static Value *simplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS, | |||
// is non-negative then LHS <s RHS. | |||
case ICmpInst::ICMP_SGT: | |||
case ICmpInst::ICMP_SGE: | |||
return ConstantExpr::getICmp(ICmpInst::ICMP_SLT, C, | |||
Constant::getNullValue(C->getType())); | |||
return ConstantFoldCompareInstruction( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ConstantFoldCompareInstOperands instead, which is DataLayout-aware.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that matter here? C
came from m_ImmConstant and we're comparing it to null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes no functional difference here, but you should never use the functions declared in llvm/IR/ConstantFold.h, only those in llvm/Analysis/ConstantFolding.h, which is the public API. We should maybe move the former into the detail
namespace to make this clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or possibly we can move the ConstantFolder implementation out-of-line (if it causes no compile-time impact), because that's the only reason ConstantFold.h is a public header at all.
As I understand ICmp ConstantExpr are planned to be removed.
d887d75
to
7cb5b6e
Compare
As I understand ICmp ConstantExpr are planned to be removed.