[InstCombine] Handle ptrtoaddr in gep of pointer sub fold #164818
Merged
+76
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This extends the `ptradd x, ptrtoint(y) - ptrtoint(x)` to `y` fold to support ptrtoaddr. In the case where x and y have the same underlying object, this is handled by InstSimplify already. If the underlying object may differ, the replacement can only be performed if provenance does not matter. For pointers with non-address bits we need to be careful here, because the pattern will return a pointer with the non-address bits of x and the address bits of y. As such, uses in ptrtoaddr are safe to replace, but uses in ptrtoint are not. Whether uses in icmp are safe to replace depends on the outcome of the pending discussion on icmp semantics.
llvmbot
added
llvm:instcombine
|
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesThis extends the For pointers with non-address bits we need to be careful here, because the pattern will return a pointer with the non-address bits of x and the address bits of y. As such, uses in ptrtoaddr are safe to replace, but uses in ptrtoint are not. Whether uses in icmp are safe to replace depends on the outcome of the pending discussion on icmp semantics (I'll adjust this in #163936 if/when that lands). Full diff: https://github.com/llvm/llvm-project/pull/164818.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 67e2aae4eda63..cca277d73f5fd 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3315,21 +3315,21 @@ Instruction *InstCombinerImpl::visitGetElementPtrInst(GetElementPtrInst &GEP) {
if (TyAllocSize == 1) {
// Canonicalize (gep i8* X, (ptrtoint Y)-(ptrtoint X)) to (bitcast Y),
- // but only if the result pointer is only used as if it were an integer,
- // or both point to the same underlying object (otherwise provenance is
- // not necessarily retained).
+ // but only if the result pointer is only used as if it were an integer.
+ // (The case where the underlying object is the same is handled by
+ // InstSimplify.)
Value *X = GEP.getPointerOperand();
Value *Y;
- if (match(GEP.getOperand(1),
- m_Sub(m_PtrToInt(m_Value(Y)), m_PtrToInt(m_Specific(X)))) &&
+ if (match(GEP.getOperand(1), m_Sub(m_PtrToIntOrAddr(m_Value(Y)),
+ m_PtrToIntOrAddr(m_Specific(X)))) &&
GEPType == Y->getType()) {
- bool HasSameUnderlyingObject =
- getUnderlyingObject(X) == getUnderlyingObject(Y);
+ bool HasNonAddressBits =
+ DL.getAddressSizeInBits(AS) != DL.getPointerSizeInBits(AS);
bool Changed = false;
GEP.replaceUsesWithIf(Y, [&](Use &U) {
- bool ShouldReplace = HasSameUnderlyingObject ||
- isa<ICmpInst>(U.getUser()) ||
- isa<PtrToIntInst>(U.getUser());
+ bool ShouldReplace = isa<PtrToAddrInst>(U.getUser()) ||
+ (!HasNonAddressBits &&
+ isa<ICmpInst, PtrToIntInst>(U.getUser()));
Changed |= ShouldReplace;
return ShouldReplace;
});
diff --git a/llvm/test/Transforms/InstCombine/ptrtoaddr.ll b/llvm/test/Transforms/InstCombine/ptrtoaddr.ll
index f19cca8f808c8..bc69fa3a0c5c7 100644
--- a/llvm/test/Transforms/InstCombine/ptrtoaddr.ll
+++ b/llvm/test/Transforms/InstCombine/ptrtoaddr.ll
@@ -4,6 +4,10 @@
; The ptrtoaddr folds are also valid for pointers that have external state.
target datalayout = "pe1:64:64:64:32"
+declare void @use.i1(i1)
+declare void @use.i32(i32)
+declare void @use.i64(i64)
+
; ptrtoaddr result type is fixed, and can't be combined with integer cast.
define i32 @ptrtoaddr_trunc(ptr %p) {
; CHECK-LABEL: define i32 @ptrtoaddr_trunc(
@@ -171,3 +175,60 @@ define i128 @sub_zext_ptrtoint_ptrtoaddr_addrsize(ptr addrspace(1) %p, i32 %offs
%sub = sub i128 %p2.addr.ext, %p.int.ext
ret i128 %sub
}
+
+define ptr @gep_sub_ptrtoaddr_different_obj(ptr %p, ptr %p2, ptr %p3) {
+; CHECK-LABEL: define ptr @gep_sub_ptrtoaddr_different_obj(
+; CHECK-SAME: ptr [[P:%.*]], ptr [[P2:%.*]], ptr [[P3:%.*]]) {
+; CHECK-NEXT: [[P_ADDR:%.*]] = ptrtoaddr ptr [[P]] to i64
+; CHECK-NEXT: [[P2_ADDR:%.*]] = ptrtoaddr ptr [[P2]] to i64
+; CHECK-NEXT: [[SUB:%.*]] = sub i64 [[P2_ADDR]], [[P_ADDR]]
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr [[P]], i64 [[SUB]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P2]], [[P3]]
+; CHECK-NEXT: call void @use.i1(i1 [[CMP]])
+; CHECK-NEXT: [[INT:%.*]] = ptrtoint ptr [[P2]] to i64
+; CHECK-NEXT: call void @use.i64(i64 [[INT]])
+; CHECK-NEXT: [[ADDR:%.*]] = ptrtoaddr ptr [[P2]] to i64
+; CHECK-NEXT: call void @use.i64(i64 [[ADDR]])
+; CHECK-NEXT: ret ptr [[GEP]]
+;
+ %p.addr = ptrtoaddr ptr %p to i64
+ %p2.addr = ptrtoaddr ptr %p2 to i64
+ %sub = sub i64 %p2.addr, %p.addr
+ %gep = getelementptr i8, ptr %p, i64 %sub
+ %cmp = icmp eq ptr %gep, %p3
+ call void @use.i1(i1 %cmp)
+ %int = ptrtoint ptr %gep to i64
+ call void @use.i64(i64 %int)
+ %addr = ptrtoaddr ptr %gep to i64
+ call void @use.i64(i64 %addr)
+ ret ptr %gep
+}
+
+define ptr addrspace(1) @gep_sub_ptrtoaddr_different_obj_addrsize(ptr addrspace(1) %p, ptr addrspace(1) %p2, ptr addrspace(1) %p3) {
+; CHECK-LABEL: define ptr addrspace(1) @gep_sub_ptrtoaddr_different_obj_addrsize(
+; CHECK-SAME: ptr addrspace(1) [[P:%.*]], ptr addrspace(1) [[P2:%.*]], ptr addrspace(1) [[P3:%.*]]) {
+; CHECK-NEXT: [[P_ADDR:%.*]] = ptrtoaddr ptr addrspace(1) [[P]] to i32
+; CHECK-NEXT: [[P2_ADDR:%.*]] = ptrtoaddr ptr addrspace(1) [[P2]] to i32
+; CHECK-NEXT: [[SUB:%.*]] = sub i32 [[P2_ADDR]], [[P_ADDR]]
+; CHECK-NEXT: [[GEP:%.*]] = getelementptr i8, ptr addrspace(1) [[P]], i32 [[SUB]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr addrspace(1) [[GEP]], [[P3]]
+; CHECK-NEXT: call void @use.i1(i1 [[CMP]])
+; CHECK-NEXT: [[TMP1:%.*]] = ptrtoint ptr addrspace(1) [[GEP]] to i64
+; CHECK-NEXT: [[INT:%.*]] = trunc i64 [[TMP1]] to i32
+; CHECK-NEXT: call void @use.i32(i32 [[INT]])
+; CHECK-NEXT: [[ADDR:%.*]] = ptrtoaddr ptr addrspace(1) [[P2]] to i32
+; CHECK-NEXT: call void @use.i32(i32 [[ADDR]])
+; CHECK-NEXT: ret ptr addrspace(1) [[GEP]]
+;
+ %p.addr = ptrtoaddr ptr addrspace(1) %p to i32
+ %p2.addr = ptrtoaddr ptr addrspace(1) %p2 to i32
+ %sub = sub i32 %p2.addr, %p.addr
+ %gep = getelementptr i8, ptr addrspace(1) %p, i32 %sub
+ %cmp = icmp eq ptr addrspace(1) %gep, %p3
+ call void @use.i1(i1 %cmp)
+ %int = ptrtoint ptr addrspace(1) %gep to i32
+ call void @use.i32(i32 %int)
+ %addr = ptrtoaddr ptr addrspace(1) %gep to i32
+ call void @use.i32(i32 %addr)
+ ret ptr addrspace(1) %gep
+}
|
nikic
commented
Oct 23, 2025
| m_PtrToIntOrAddr(m_Specific(X)))) && | ||
| GEPType == Y->getType()) { | ||
| bool HasSameUnderlyingObject = | ||
| getUnderlyingObject(X) == getUnderlyingObject(Y); |
There was a problem hiding this comment.
See
for the InstSimplify transform that handles the same underlying object case, which is why I dropped the handling here.
arichardson
approved these changes
Oct 24, 2025
| ret ptr %gep | ||
| } | ||
|
|
||
| define ptr addrspace(1) @gep_sub_ptrtoaddr_different_obj_addrsize(ptr addrspace(1) %p, ptr addrspace(1) %p2, ptr addrspace(1) %p3) { |
There was a problem hiding this comment.
Maybe add a comment above this function that ptrtoint should not be folded just to make it easier to understand what is being tested? I initially had to look at this a few times since only the file check variable name differs.
dtcxzyw
approved these changes
Oct 24, 2025
arichardson
approved these changes
Oct 24, 2025
dvbuka
pushed a commit
to dvbuka/llvm-project
that referenced
this pull request
Oct 27, 2025
This extends the `ptradd x, ptrtoint(y) - ptrtoint(x)` to `y` InstCombine fold to support ptrtoaddr. In the case where x and y have the same underlying object, this is handled by InstSimplify already. If the underlying object may differ, the replacement can only be performed if provenance does not matter. For pointers with non-address bits we need to be careful here, because the pattern will return a pointer with the non-address bits of x and the address bits of y. As such, uses in ptrtoaddr are safe to replace, but uses in ptrtoint are not. Whether uses in icmp are safe to replace depends on the outcome of the pending discussion on icmp semantics (I'll adjust this in llvm#163936 if/when that lands).
Lukacma
pushed a commit
to Lukacma/llvm-project
that referenced
this pull request
Oct 29, 2025
This extends the `ptradd x, ptrtoint(y) - ptrtoint(x)` to `y` InstCombine fold to support ptrtoaddr. In the case where x and y have the same underlying object, this is handled by InstSimplify already. If the underlying object may differ, the replacement can only be performed if provenance does not matter. For pointers with non-address bits we need to be careful here, because the pattern will return a pointer with the non-address bits of x and the address bits of y. As such, uses in ptrtoaddr are safe to replace, but uses in ptrtoint are not. Whether uses in icmp are safe to replace depends on the outcome of the pending discussion on icmp semantics (I'll adjust this in llvm#163936 if/when that lands).
aokblast
pushed a commit
to aokblast/llvm-project
that referenced
this pull request
Oct 30, 2025
This extends the `ptradd x, ptrtoint(y) - ptrtoint(x)` to `y` InstCombine fold to support ptrtoaddr. In the case where x and y have the same underlying object, this is handled by InstSimplify already. If the underlying object may differ, the replacement can only be performed if provenance does not matter. For pointers with non-address bits we need to be careful here, because the pattern will return a pointer with the non-address bits of x and the address bits of y. As such, uses in ptrtoaddr are safe to replace, but uses in ptrtoint are not. Whether uses in icmp are safe to replace depends on the outcome of the pending discussion on icmp semantics (I'll adjust this in llvm#163936 if/when that lands).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This extends the
ptradd x, ptrtoint(y) - ptrtoint(x)toyInstCombine fold to support ptrtoaddr. In the case where x and y have the same underlying object, this is handled by InstSimplify already. If the underlying object may differ, the replacement can only be performed if provenance does not matter.For pointers with non-address bits we need to be careful here, because the pattern will return a pointer with the non-address bits of x and the address bits of y. As such, uses in ptrtoaddr are safe to replace, but uses in ptrtoint are not. Whether uses in icmp are safe to replace depends on the outcome of the pending discussion on icmp semantics (I'll adjust this in #163936 if/when that lands).