[InstCombine] Compare icmp inttoptr, inttoptr values directly

[citymarina]

InstCombine already has some rules for icmp ptrtoint, ptrtoint to drop the casts and compare the source values. This change adds the same for the reverse case with inttoptr.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Marina Taylor (citymarina)

Changes

InstCombine already has some rules for icmp ptrtoint, ptrtoint to drop the casts and compare the source values. This change adds the same for the reverse case with inttoptr.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+21-5)
• (modified) llvm/test/Transforms/InstCombine/icmp.ll (+19)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index 8e8d472a5df1d3..af60b1886fc72e 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -6081,12 +6081,12 @@ Instruction *InstCombinerImpl::foldICmpWithCastOp(ICmpInst &ICmp) {
 
   // Turn icmp (ptrtoint x), (ptrtoint/c) into a compare of the input if the
   // integer type is the same size as the pointer type.
-  auto CompatibleSizes = [&](Type *SrcTy, Type *DestTy) {
-    if (isa<VectorType>(SrcTy)) {
-      SrcTy = cast<VectorType>(SrcTy)->getElementType();
-      DestTy = cast<VectorType>(DestTy)->getElementType();
+  auto CompatibleSizes = [&](Type *PtrTy, Type *IntTy) {
+    if (isa<VectorType>(PtrTy)) {
+      PtrTy = cast<VectorType>(PtrTy)->getElementType();
+      IntTy = cast<VectorType>(IntTy)->getElementType();
     }
-    return DL.getPointerTypeSizeInBits(SrcTy) == DestTy->getIntegerBitWidth();
+    return DL.getPointerTypeSizeInBits(PtrTy) == IntTy->getIntegerBitWidth();
   };
   if (CastOp0->getOpcode() == Instruction::PtrToInt &&
       CompatibleSizes(SrcTy, DestTy)) {
@@ -6103,6 +6103,22 @@ Instruction *InstCombinerImpl::foldICmpWithCastOp(ICmpInst &ICmp) {
       return new ICmpInst(ICmp.getPredicate(), Op0Src, NewOp1);
   }
 
+  // Do the same in the other direction for icmp (inttoptr x), (inttoptr/c).
+  if (CastOp0->getOpcode() == Instruction::IntToPtr &&
+      CompatibleSizes(DestTy, SrcTy)) {
+    Value *NewOp1 = nullptr;
+    if (auto *IntToPtrOp1 = dyn_cast<IntToPtrInst>(ICmp.getOperand(1))) {
+      Value *IntSrc = IntToPtrOp1->getOperand(0);
+      if (IntSrc->getType() == Op0Src->getType())
+        NewOp1 = IntToPtrOp1->getOperand(0);
+    } else if (auto *RHSC = dyn_cast<Constant>(ICmp.getOperand(1))) {
+      NewOp1 = ConstantExpr::getPtrToInt(RHSC, SrcTy);
+    }
+
+    if (NewOp1)
+      return new ICmpInst(ICmp.getPredicate(), Op0Src, NewOp1);
+  }
+
   if (Instruction *R = foldICmpWithTrunc(ICmp))
     return R;
 
diff --git a/llvm/test/Transforms/InstCombine/icmp.ll b/llvm/test/Transforms/InstCombine/icmp.ll
index e492055fea8b8d..b3cce3548a949e 100644
--- a/llvm/test/Transforms/InstCombine/icmp.ll
+++ b/llvm/test/Transforms/InstCombine/icmp.ll
@@ -5326,3 +5326,22 @@ define i1 @pr94897(i32 range(i32 -2147483648, 0) %x) {
   %cmp = icmp ugt i32 %shl, -50331648
   ret i1 %cmp
 }
+
+define i1 @test_icmp_inttoptr(i64 %x, i64 %y) {
+; CHECK-LABEL: @test_icmp_inttoptr(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+  %xptr = inttoptr i64 %x to ptr
+  %yptr = inttoptr i64 %y to ptr
+  %cmp = icmp eq ptr %xptr, %yptr
+  ret i1 %cmp
+}
+
+define i1 @test_icmp_inttoptr_constant(i64 %x) {
+; CHECK-LABEL: @test_icmp_inttoptr_constant(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i64 [[X:%.*]], 42
+; CHECK-NEXT:    ret i1 [[CMP]]
+  %xptr = inttoptr i64 %x to ptr
+  %cmp = icmp eq ptr %xptr, inttoptr (i64 42 to ptr)
+  ret i1 %cmp
+}

[nikic]

Please see https://llvm.org/docs/InstCombineContributorGuide.html, esp. the bit about pre-committing tests.

[citymarina]

Oh wow, that contributor guide looks really thorough. Thank you for writing it!

[dtcxzyw]

My last attempt #77832 broke hwasan buildbot. @vitalybuka Can you check this patch?

[citymarina]

Oh wow, that contributor guide looks really thorough. Thank you for writing it!

I've read through the guide carefully and hopefully this revision meets the expectations better. I appreciate the clear instructions!

[citymarina]

Ping

[nikic]

I think the open question here is:

My last attempt #77832 broke hwasan buildbot. @vitalybuka Can you check this patch?

But maybe we can just land this and see what happens.

[citymarina]

I think the open question here is:

My last attempt #77832 broke hwasan buildbot. @vitalybuka Can you check this patch?

But maybe we can just land this and see what happens.

I interpreted @vitalybuka's removing themself from the review to indicate no plans to test this, but if that was incorrect let me know.

[XChy]

Look reasonable to me

[dtcxzyw]

I think the open question here is:

My last attempt #77832 broke hwasan buildbot. @vitalybuka Can you check this patch?

But maybe we can just land this and see what happens.

Agree.

[dtcxzyw]

LG

[nikic]

Looks like the hwasan build bot with this commit passed: https://lab.llvm.org/buildbot/#/builders/55/builds/2229

[vitalybuka]

My last attempt #77832 broke hwasan buildbot. @vitalybuka Can you check this patch?

Sorry, I don't expert in this code, so I removed myself, assuming I am accidentally here. I didn't noticed testing request.
In general if you confident, it's fine to land and wait, because even for owner of the bot testing there is some hassle.