-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[InstComb] Fold inttoptr (add (ptrtoint %B), %O) -> GEP for ICMP users. #153421
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
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesFull diff: https://github.com/llvm/llvm-project/pull/153421.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
index a43a6ee1f58b0..2edabcec2aecf 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
@@ -2073,6 +2073,22 @@ Instruction *InstCombinerImpl::visitIntToPtr(IntToPtrInst &CI) {
return new IntToPtrInst(P, CI.getType());
}
+ // Replace (inttoptr (add (ptrtoint %Base), %Offset)) with
+ //(getelementptr i8, %Base, %Offset) if all users are ICmps.
+ Value *Base;
+ Value *Offset;
+ if (match(CI.getOperand(0),
+ m_Add(m_PtrToInt(m_Value(Base)), m_Value(Offset))) &&
+ all_of(CI.users(), IsaPred<ICmpInst>)) {
+ Type *BasePtrTy = Base->getType();
+ if (CI.getType()->getPointerAddressSpace() ==
+ BasePtrTy->getPointerAddressSpace() &&
+ DL.getTypeSizeInBits(BasePtrTy) ==
+ DL.getTypeSizeInBits(CI.getSrcTy())) {
+ return GetElementPtrInst::Create(Builder.getInt8Ty(), Base, Offset);
+ }
+ }
+
if (Instruction *I = commonCastTransforms(CI))
return I;
diff --git a/llvm/test/Transforms/InstCombine/fold-bin-operand.ll b/llvm/test/Transforms/InstCombine/fold-bin-operand.ll
index f28262b2a77e0..f596bc5d226be 100644
--- a/llvm/test/Transforms/InstCombine/fold-bin-operand.ll
+++ b/llvm/test/Transforms/InstCombine/fold-bin-operand.ll
@@ -1,6 +1,6 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -passes=instcombine -S | FileCheck %s
-target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
+target datalayout = "E-p:64:64:64-p1:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
define i1 @f(i1 %x) {
; CHECK-LABEL: @f(
@@ -30,6 +30,156 @@ define i32 @g(i32 %x) {
ret i32 %b
}
+define i1 @inttoptr_add_ptrtoint_used_by_single_icmp(ptr %src, ptr %p2) {
+; CHECK-LABEL: @inttoptr_add_ptrtoint_used_by_single_icmp(
+; CHECK-NEXT: [[P2:%.*]] = getelementptr i8, ptr [[SRC:%.*]], i64 10
+; CHECK-NEXT: [[C:%.*]] = icmp eq ptr [[P2]], [[P:%.*]]
+; CHECK-NEXT: ret i1 [[C]]
+;
+ %i = ptrtoint ptr %src to i64
+ %a = add i64 %i, 10
+ %p = inttoptr i64 %a to ptr
+ %c = icmp eq ptr %p, %p2
+ ret i1 %c
+}
+
+define i1 @inttoptr_add_ptrtoint_used_by_single_icmp_different_src_address_spaces(ptr addrspace(1) %src, ptr %p2) {
+; CHECK-LABEL: @inttoptr_add_ptrtoint_used_by_single_icmp_different_src_address_spaces(
+; CHECK-NEXT: [[I:%.*]] = ptrtoint ptr addrspace(1) [[SRC:%.*]] to i64
+; CHECK-NEXT: [[A:%.*]] = add i64 [[I]], 10
+; CHECK-NEXT: [[P:%.*]] = inttoptr i64 [[A]] to ptr
+; CHECK-NEXT: [[C:%.*]] = icmp eq ptr [[P2:%.*]], [[P]]
+; CHECK-NEXT: ret i1 [[C]]
+;
+ %i = ptrtoint ptr addrspace(1) %src to i64
+ %a = add i64 %i, 10
+ %p = inttoptr i64 %a to ptr
+ %c = icmp eq ptr %p, %p2
+ ret i1 %c
+}
+
+define i1 @inttoptr_add_ptrtoint_used_by_single_icmp_different_dst_address_spaces(ptr %src, ptr addrspace(1) %p2) {
+; CHECK-LABEL: @inttoptr_add_ptrtoint_used_by_single_icmp_different_dst_address_spaces(
+; CHECK-NEXT: [[I:%.*]] = ptrtoint ptr [[SRC:%.*]] to i64
+; CHECK-NEXT: [[A:%.*]] = add i64 [[I]], 10
+; CHECK-NEXT: [[P:%.*]] = inttoptr i64 [[A]] to ptr addrspace(1)
+; CHECK-NEXT: [[C:%.*]] = icmp eq ptr addrspace(1) [[P2:%.*]], [[P]]
+; CHECK-NEXT: ret i1 [[C]]
+;
+ %i = ptrtoint ptr %src to i64
+ %a = add i64 %i, 10
+ %p = inttoptr i64 %a to ptr addrspace(1)
+ %c = icmp eq ptr addrspace(1) %p, %p2
+ ret i1 %c
+}
+
+define i1 @inttoptr_add_ptrtoint_used_by_single_icmp_int_type_does_not_match_ptr_ty(ptr %src, ptr %p2) {
+; CHECK-LABEL: @inttoptr_add_ptrtoint_used_by_single_icmp_int_type_does_not_match_ptr_ty(
+; CHECK-NEXT: [[TMP1:%.*]] = ptrtoint ptr [[SRC:%.*]] to i64
+; CHECK-NEXT: [[I:%.*]] = trunc i64 [[TMP1]] to i8
+; CHECK-NEXT: [[A:%.*]] = add i8 [[I]], 10
+; CHECK-NEXT: [[TMP2:%.*]] = zext i8 [[A]] to i64
+; CHECK-NEXT: [[P:%.*]] = inttoptr i64 [[TMP2]] to ptr
+; CHECK-NEXT: [[C:%.*]] = icmp eq ptr [[P2:%.*]], [[P]]
+; CHECK-NEXT: ret i1 [[C]]
+;
+ %i = ptrtoint ptr %src to i8
+ %a = add i8 %i, 10
+ %p = inttoptr i8 %a to ptr
+ %c = icmp eq ptr %p, %p2
+ ret i1 %c
+}
+
+define i1 @multiple_inttoptr_add_ptrtoint_used_by_single_icmp(ptr %src) {
+; CHECK-LABEL: @multiple_inttoptr_add_ptrtoint_used_by_single_icmp(
+; CHECK-NEXT: ret i1 false
+;
+ %i = ptrtoint ptr %src to i64
+ %a = add i64 %i, 10
+ %p = inttoptr i64 %a to ptr
+ %a.2 = add i64 %i, 11
+ %p.2 = inttoptr i64 %a.2 to ptr
+ %c = icmp eq ptr %p, %p.2
+ ret i1 %c
+}
+
+define i1 @multiple_inttoptr_add_ptrtoint_used_by_single_icmp_non_constant_offset(ptr %src, i64 %off.1, i64 %off.2) {
+; CHECK-LABEL: @multiple_inttoptr_add_ptrtoint_used_by_single_icmp_non_constant_offset(
+; CHECK-NEXT: [[C:%.*]] = icmp eq i64 [[OFF_1:%.*]], [[OFF_2:%.*]]
+; CHECK-NEXT: ret i1 [[C]]
+;
+ %i = ptrtoint ptr %src to i64
+ %a = add i64 %i, %off.1
+ %p = inttoptr i64 %a to ptr
+ %a.2 = add i64 %i, %off.2
+ %p.2 = inttoptr i64 %a.2 to ptr
+ %c = icmp eq ptr %p, %p.2
+ ret i1 %c
+}
+
+define i1 @inttoptr_add_ptrtoint_used_by_single_icmp_in_different_bb(i1 %bc, ptr %src, ptr %p2) {
+; CHECK-LABEL: @inttoptr_add_ptrtoint_used_by_single_icmp_in_different_bb(
+; CHECK-NEXT: br i1 [[BC:%.*]], label [[THEN:%.*]], label [[ELSE:%.*]]
+; CHECK: then:
+; CHECK-NEXT: [[P2:%.*]] = getelementptr i8, ptr [[SRC:%.*]], i64 10
+; CHECK-NEXT: [[C:%.*]] = icmp eq ptr [[P2]], [[P:%.*]]
+; CHECK-NEXT: ret i1 [[C]]
+; CHECK: else:
+; CHECK-NEXT: ret i1 false
+;
+ %i = ptrtoint ptr %src to i64
+ %a = add i64 %i, 10
+ %p = inttoptr i64 %a to ptr
+ br i1 %bc, label %then, label %else
+
+then:
+ %c = icmp eq ptr %p, %p2
+ ret i1 %c
+
+else:
+ ret i1 false
+}
+
+define i1 @inttoptr_add_ptrtoint_used_by_multiple_icmps(ptr %src, ptr %p2, ptr %p3) {
+; CHECK-LABEL: @inttoptr_add_ptrtoint_used_by_multiple_icmps(
+; CHECK-NEXT: [[P2:%.*]] = getelementptr i8, ptr [[SRC:%.*]], i64 10
+; CHECK-NEXT: [[C_1:%.*]] = icmp eq ptr [[P2]], [[P:%.*]]
+; CHECK-NEXT: [[C_2:%.*]] = icmp eq ptr [[P2]], [[P3:%.*]]
+; CHECK-NEXT: [[XOR:%.*]] = xor i1 [[C_1]], [[C_2]]
+; CHECK-NEXT: ret i1 [[XOR]]
+;
+ %i = ptrtoint ptr %src to i64
+ %a = add i64 %i, 10
+ %p = inttoptr i64 %a to ptr
+ %c.1 = icmp eq ptr %p, %p2
+ %c.2 = icmp eq ptr %p, %p3
+ %xor = xor i1 %c.1, %c.2
+ ret i1 %xor
+}
+
+declare void @foo(ptr)
+
+define i1 @inttoptr_add_ptrtoint_used_by_multiple_icmps_and_other_user(ptr %src, ptr %p2, ptr %p3) {
+; CHECK-LABEL: @inttoptr_add_ptrtoint_used_by_multiple_icmps_and_other_user(
+; CHECK-NEXT: [[TMP1:%.*]] = ptrtoint ptr [[SRC:%.*]] to i64
+; CHECK-NEXT: [[TMP2:%.*]] = add i64 [[TMP1]], 10
+; CHECK-NEXT: [[P:%.*]] = inttoptr i64 [[TMP2]] to ptr
+; CHECK-NEXT: [[C_1:%.*]] = icmp eq ptr [[P2:%.*]], [[P]]
+; CHECK-NEXT: [[C_2:%.*]] = icmp eq ptr [[P3:%.*]], [[P]]
+; CHECK-NEXT: [[XOR:%.*]] = xor i1 [[C_1]], [[C_2]]
+; CHECK-NEXT: call void @foo(ptr [[P]])
+; CHECK-NEXT: ret i1 [[XOR]]
+;
+ %i = ptrtoint ptr %src to i64
+ %a = add i64 %i, 10
+ %p = inttoptr i64 %a to ptr
+ %c.1 = icmp eq ptr %p, %p2
+ %c.2 = icmp eq ptr %p, %p3
+ %xor = xor i1 %c.1, %c.2
+ call void @foo(ptr %p)
+ ret i1 %xor
+}
+
define i32 @h(i1 %A, i32 %B) {
; CHECK-LABEL: @h(
; CHECK-NEXT: EntryBlock:
|
dtcxzyw
left a comment
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.
Can you share the original IR for libc++ or other projects? I'd expect that %B and the other operand of icmp share the same underlying object. If so, it should be handled by foldGEPICmp.
BTW, the title is misleading. It works only if the provenance doesn't matter: https://alive2.llvm.org/ce/z/9GK5la
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 m_PtrToIntSameSize: https://alive2.llvm.org/ce/z/6fTkhq
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.
Updated to use m_PtrToIntSameSize and removed the size check below, thanks!
It isn't always true. The base pointers may be loaded from a data structure (e.g., |
fhahn
left a comment
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.
BTW, the title is misleading. It works only if the provenance doesn't matter: https://alive2.llvm.org/ce/z/9GK5la
Hmm, Try should imply that it is not always possible. Do you have any suggestions on how to clarify the title?
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.
Updated to use m_PtrToIntSameSize and removed the size check below, thanks!
"Try to convert inttoptr(add(ptrtoint)) to GEP when provenance doesn't matter"? |
I generally try to keep the length <= 72 chars, to avoid truncation in some clients. Updated to |
|
For this to be really useful we need to allow a few more cases where the pointer is only used as integer: #153566 This allows vectorizing std::find in some cases. |
|
ping :) |
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.
| m_Add(m_PtrToIntSameSize(DL, m_Value(Base)), m_Value(Offset))) && | |
| m_c_Add(m_PtrToIntSameSize(DL, m_Value(Base)), m_Value(Offset))) && |
Please add some commuted tests.
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.
Done thanks
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.
Should this have a one use check on the add? Otherwise we can end up with both the add and the gep.
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.
yeah added, thanks
Replace inttoptr (add (ptrtoint %B), %O) with (getelementptr i8, %B, %o) if all users are ICmp instruction, which in turn means only the address value is compared. We should be able to do this, if the src pointer, the integer type and the destination pointer types have the same bitwidth and address space. A common source of such (inttoptr (add (ptrtoint %B), %O)) is from various iterations in libc++. In practice this triggers in a number of files in Clang and various open source projects, including cppcheck, diamond, llama and more. Alive2 Proof with constant offset: https://alive2.llvm.org/ce/z/K_5N_B
…), %Offset)). Precommit tests for llvm/llvm-project#153421.
nikic
left a comment
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.
LGTM
…r ICMP users. (#153421) Replace inttoptr (add (ptrtoint %B), %O) with (getelementptr i8, %B, %o) if all users are ICmp instruction, which in turn means only the address value is compared. We should be able to do this, if the src pointer, the integer type and the destination pointer types have the same bitwidth and address space. A common source of such (inttoptr (add (ptrtoint %B), %O)) is from various iterations in libc++. In practice this triggers in a number of files in Clang and various open source projects, including cppcheck, diamond, llama and more. Alive2 Proof with constant offset: https://alive2.llvm.org/ce/z/K_5N_B PR: llvm/llvm-project#153421
Generalize the logic from llvm#153421 to support additional cases where the pointer is only used as integer. Alive2 Proof: https://alive2.llvm.org/ce/z/po58pP This enables vectorizing std::find for some cases, if additional assumptions are provided: https://godbolt.org/z/94oq3576E Depends on llvm#153421 (included in the PR).
…rm. (#153566) Generalize the logic from #153421 to support additional cases where the pointer is only used as integer. Alive2 Proof: https://alive2.llvm.org/ce/z/po58pP This enables vectorizing std::find for some cases, if additional assumptions are provided: https://godbolt.org/z/94oq3576E Depends on #15342. PR: #153566
…GEP transform. (#153566) Generalize the logic from llvm/llvm-project#153421 to support additional cases where the pointer is only used as integer. Alive2 Proof: https://alive2.llvm.org/ce/z/po58pP This enables vectorizing std::find for some cases, if additional assumptions are provided: https://godbolt.org/z/94oq3576E Depends on llvm/llvm-project#15342. PR: llvm/llvm-project#153566
…s. (llvm#153421) Replace inttoptr (add (ptrtoint %B), %O) with (getelementptr i8, %B, %o) if all users are ICmp instruction, which in turn means only the address value is compared. We should be able to do this, if the src pointer, the integer type and the destination pointer types have the same bitwidth and address space. A common source of such (inttoptr (add (ptrtoint %B), %O)) is from various iterations in libc++. In practice this triggers in a number of files in Clang and various open source projects, including cppcheck, diamond, llama and more. Alive2 Proof with constant offset: https://alive2.llvm.org/ce/z/K_5N_B PR: llvm#153421 (cherry picked from commit 1b0b59a)
…rm. (llvm#153566) Generalize the logic from llvm#153421 to support additional cases where the pointer is only used as integer. Alive2 Proof: https://alive2.llvm.org/ce/z/po58pP This enables vectorizing std::find for some cases, if additional assumptions are provided: https://godbolt.org/z/94oq3576E Depends on llvm#15342. PR: llvm#153566 (cherry picked from commit 8bc038d)
Replace inttoptr (add (ptrtoint %B), %O) with (getelementptr i8, %B, %o)
if all users are ICmp instruction, which in turn means only the address
value is compared. We should be able to do this, if the src pointer,
the integer type and the destination pointer types have the same
bitwidth and address space.
A common source of such (inttoptr (add (ptrtoint %B), %O)) is from
various iterations in libc++.
In practice this triggers in a number of files in Clang and various open
source projects, including cppcheck, diamond, llama and more.
Alive2 Proof with constant offset: https://alive2.llvm.org/ce/z/K_5N_B