Skip to content

Conversation

@nikic
Copy link
Contributor

@nikic nikic commented Dec 11, 2025

There is a generic fold for recursively calling simplifyICmpInst with the ptrtoint cast stripped:

// Turn icmp (ptrtoint x), (ptrtoint/constant) into a compare of the input
// if the integer type is the same size as the pointer type.
if (MaxRecurse && isa<PtrToIntInst>(LI) &&
Q.DL.getTypeSizeInBits(SrcTy) == DstTy->getPrimitiveSizeInBits()) {
if (Constant *RHSC = dyn_cast<Constant>(RHS)) {
// Transfer the cast to the constant.
if (Value *V = simplifyICmpInst(Pred, SrcOp,
ConstantExpr::getIntToPtr(RHSC, SrcTy),
Q, MaxRecurse - 1))
return V;
} else if (PtrToIntInst *RI = dyn_cast<PtrToIntInst>(RHS)) {
if (RI->getOperand(0)->getType() == SrcTy)
// Compare without the cast.
if (Value *V = simplifyICmpInst(Pred, SrcOp, RI->getOperand(0), Q,
MaxRecurse - 1))
return V;
}
}

As such, we shouldn't have to explicitly do this for the computePointerICmp() fold.

This is not strictly NFC because the recursion limit applies to the generic fold, though I wouldn't expect this to matter in practice.

There is a generic fold for recursively calling simplifyICmpInst
with the ptrtoint cast stripped:
https://github.com/llvm/llvm-project/blob/9b6b52b534ba592d7d6b5863e3d9a240119a74d9/llvm/lib/Analysis/InstructionSimplify.cpp#L3850-L3867

As such, we shouldn't have to explicitly do this for the
computePointerICmp() fold.

This is not strictly NFC because the recursion limit applies to
the generic fold, though I wouldn't expect this to matter in
practice.
@nikic nikic requested a review from dtcxzyw December 11, 2025 11:37
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:analysis Includes value tracking, cost tables and constant folding labels Dec 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Nikita Popov (nikic)

Changes

There is a generic fold for recursively calling simplifyICmpInst with the ptrtoint cast stripped:

// Turn icmp (ptrtoint x), (ptrtoint/constant) into a compare of the input
// if the integer type is the same size as the pointer type.
if (MaxRecurse && isa<PtrToIntInst>(LI) &&
Q.DL.getTypeSizeInBits(SrcTy) == DstTy->getPrimitiveSizeInBits()) {
if (Constant *RHSC = dyn_cast<Constant>(RHS)) {
// Transfer the cast to the constant.
if (Value *V = simplifyICmpInst(Pred, SrcOp,
ConstantExpr::getIntToPtr(RHSC, SrcTy),
Q, MaxRecurse - 1))
return V;
} else if (PtrToIntInst *RI = dyn_cast<PtrToIntInst>(RHS)) {
if (RI->getOperand(0)->getType() == SrcTy)
// Compare without the cast.
if (Value *V = simplifyICmpInst(Pred, SrcOp, RI->getOperand(0), Q,
MaxRecurse - 1))
return V;
}
}

As such, we shouldn't have to explicitly do this for the computePointerICmp() fold.

This is not strictly NFC because the recursion limit applies to the generic fold, though I wouldn't expect this to matter in practice.


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

1 Files Affected:

  • (modified) llvm/lib/Analysis/InstructionSimplify.cpp (-8)
diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
index 1d82515cd84f9..63930617071f3 100644
--- a/llvm/lib/Analysis/InstructionSimplify.cpp
+++ b/llvm/lib/Analysis/InstructionSimplify.cpp
@@ -4077,14 +4077,6 @@ static Value *simplifyICmpInst(CmpPredicate Pred, Value *LHS, Value *RHS,
   if (LHS->getType()->isPointerTy())
     if (auto *C = computePointerICmp(Pred, LHS, RHS, Q))
       return C;
-  if (auto *CLHS = dyn_cast<PtrToIntOperator>(LHS))
-    if (auto *CRHS = dyn_cast<PtrToIntOperator>(RHS))
-      if (CLHS->getPointerOperandType() == CRHS->getPointerOperandType() &&
-          Q.DL.getTypeSizeInBits(CLHS->getPointerOperandType()) ==
-              Q.DL.getTypeSizeInBits(CLHS->getType()))
-        if (auto *C = computePointerICmp(Pred, CLHS->getPointerOperand(),
-                                         CRHS->getPointerOperand(), Q))
-          return C;
 
   // If the comparison is with the result of a select instruction, check whether
   // comparing with either branch of the select always yields the same value.

@nikic
Copy link
Contributor Author

nikic commented Dec 11, 2025

No changes on llvm-opt-benchmark.

@nikic nikic merged commit d0d8359 into llvm:main Dec 12, 2025
13 checks passed
@nikic nikic deleted the instsimplify-redundant branch December 12, 2025 07:55
anonymouspc pushed a commit to anonymouspc/llvm that referenced this pull request Dec 15, 2025
There is a generic fold for recursively calling simplifyICmpInst with
the ptrtoint cast stripped:

https://github.com/llvm/llvm-project/blob/9b6b52b534ba592d7d6b5863e3d9a240119a74d9/llvm/lib/Analysis/InstructionSimplify.cpp#L3850-L3867

As such, we shouldn't have to explicitly do this for the
computePointerICmp() fold.

This is not strictly NFC because the recursion limit applies to the
generic fold, though I wouldn't expect this to matter in practice.
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 llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants