Skip to content

Conversation

arichardson
Copy link
Member

@arichardson arichardson commented Jun 11, 2025

We currently use the same code path as ptrtoint, which happens to be
correct since ptrtoaddr IR always has to truncate to address width, but
we should probably be handling ptrtoaddr explicitly instead of relying on
ptrtoint lowering being correct.

Created using spr 1.3.6-beta.1
@llvmbot
Copy link
Member

llvmbot commented Jun 11, 2025

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-globalisel

Author: Alexander Richardson (arichardson)

Changes

This is currently not correct as it uses the same code path as ptrtoint.


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

2 Files Affected:

  • (added) llvm/test/CodeGen/AMDGPU/GlobalISel/ptrtoint-ptrtoaddr-p8.ll (+118)
  • (added) llvm/test/CodeGen/AMDGPU/ptrtoint-ptrtoaddr-p8.ll (+118)
diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/ptrtoint-ptrtoaddr-p8.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/ptrtoint-ptrtoaddr-p8.ll
new file mode 100644
index 0000000000000..8f7867c8030d9
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/ptrtoint-ptrtoaddr-p8.ll
@@ -0,0 +1,118 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=amdgcn -global-isel -verify-machineinstrs < %s | FileCheck %s
+;; Check that we can lower ptrtoaddr differently from ptrtoint.
+;; Includes an ignored argument so the registers actually need to be written
+;; Also all functions are zeroext to show that non-address bits are masked off
+;; instead of filling them with arbitrary data
+
+define zeroext i128 @ptrtoint(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoint:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint ptr addrspace(8) %ptr to i128
+  ret i128 %ret
+}
+
+define zeroext i48 @ptrtoaddr(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoaddr:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_and_b32_e32 v1, 0xffff, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr ptr addrspace(8) %ptr to i48
+  ret i48 %ret
+}
+
+define <2 x i128> @ptrtoint_vec(ptr addrspace(8) %ignored, <2 x ptr addrspace(8)> %ptr) {
+; CHECK-LABEL: ptrtoint_vec:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    v_mov_b32_e32 v4, v8
+; CHECK-NEXT:    v_mov_b32_e32 v5, v9
+; CHECK-NEXT:    v_mov_b32_e32 v6, v10
+; CHECK-NEXT:    v_mov_b32_e32 v7, v11
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint <2 x ptr addrspace(8)> %ptr to <2 x i128>
+  ret <2 x i128> %ret
+}
+
+;; Note: needs to be 2 x i64 instead of 2 x i48 since the latter is not supported.
+define <2 x i64> @ptrtoaddr_vec(ptr addrspace(8) %ignored, <2 x ptr addrspace(8)> %ptr) {
+; CHECK-LABEL: ptrtoaddr_vec:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v2, v8
+; CHECK-NEXT:    v_mov_b32_e32 v3, v9
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr <2 x ptr addrspace(8)> %ptr to <2 x i64>
+  ret <2 x i64> %ret
+}
+
+define zeroext i256 @ptrtoint_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoint_ext:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    v_mov_b32_e32 v4, 0
+; CHECK-NEXT:    v_mov_b32_e32 v5, 0
+; CHECK-NEXT:    v_mov_b32_e32 v6, 0
+; CHECK-NEXT:    v_mov_b32_e32 v7, 0
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint ptr addrspace(8) %ptr to i256
+  ret i256 %ret
+}
+
+;; Check that we extend the offset to i256 instead of reinterpreting all bits.
+;; FIXME: this is wrong, we are removing the trunc to i48:
+define zeroext i256 @ptrtoaddr_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoaddr_ext:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    v_mov_b32_e32 v4, 0
+; CHECK-NEXT:    v_mov_b32_e32 v5, 0
+; CHECK-NEXT:    v_mov_b32_e32 v6, 0
+; CHECK-NEXT:    v_mov_b32_e32 v7, 0
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr ptr addrspace(8) %ptr to i256
+  ret i256 %ret
+}
+
+define zeroext i64 @ptrtoint_trunc(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoint_trunc:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint ptr addrspace(8) %ptr to i64
+  ret i64 %ret
+}
+
+define zeroext i16 @ptrtoaddr_trunc(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoaddr_trunc:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_and_b32_e32 v0, 0xffff, v4
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr ptr addrspace(8) %ptr to i16
+  ret i16 %ret
+}
diff --git a/llvm/test/CodeGen/AMDGPU/ptrtoint-ptrtoaddr-p8.ll b/llvm/test/CodeGen/AMDGPU/ptrtoint-ptrtoaddr-p8.ll
new file mode 100644
index 0000000000000..49d91e59dff2d
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/ptrtoint-ptrtoaddr-p8.ll
@@ -0,0 +1,118 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple=amdgcn -verify-machineinstrs < %s | FileCheck %s
+;; Check that we can lower ptrtoaddr differently from ptrtoint.
+;; Includes an ignored argument so the registers actually need to be written
+;; Also all functions are zeroext to show that non-address bits are masked off
+;; instead of filling them with arbitrary data
+
+define zeroext i128 @ptrtoint(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoint:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint ptr addrspace(8) %ptr to i128
+  ret i128 %ret
+}
+
+define zeroext i48 @ptrtoaddr(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoaddr:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_and_b32_e32 v1, 0xffff, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr ptr addrspace(8) %ptr to i48
+  ret i48 %ret
+}
+
+define <2 x i128> @ptrtoint_vec(ptr addrspace(8) %ignored, <2 x ptr addrspace(8)> %ptr) {
+; CHECK-LABEL: ptrtoint_vec:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v4, v8
+; CHECK-NEXT:    v_mov_b32_e32 v5, v9
+; CHECK-NEXT:    v_mov_b32_e32 v6, v10
+; CHECK-NEXT:    v_mov_b32_e32 v7, v11
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint <2 x ptr addrspace(8)> %ptr to <2 x i128>
+  ret <2 x i128> %ret
+}
+
+;; Note: needs to be 2 x i64 instead of 2 x i48 since the latter is not supported.
+define <2 x i64> @ptrtoaddr_vec(ptr addrspace(8) %ignored, <2 x ptr addrspace(8)> %ptr) {
+; CHECK-LABEL: ptrtoaddr_vec:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v3, v9
+; CHECK-NEXT:    v_mov_b32_e32 v2, v8
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr <2 x ptr addrspace(8)> %ptr to <2 x i64>
+  ret <2 x i64> %ret
+}
+
+define zeroext i256 @ptrtoint_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoint_ext:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v4, 0
+; CHECK-NEXT:    v_mov_b32_e32 v5, 0
+; CHECK-NEXT:    v_mov_b32_e32 v6, 0
+; CHECK-NEXT:    v_mov_b32_e32 v7, 0
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint ptr addrspace(8) %ptr to i256
+  ret i256 %ret
+}
+
+;; Check that we extend the offset to i256 instead of reinterpreting all bits.
+;; FIXME: this is wrong, we are removing the trunc to i48:
+define zeroext i256 @ptrtoaddr_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoaddr_ext:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v3, v7
+; CHECK-NEXT:    v_mov_b32_e32 v2, v6
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    v_mov_b32_e32 v4, 0
+; CHECK-NEXT:    v_mov_b32_e32 v5, 0
+; CHECK-NEXT:    v_mov_b32_e32 v6, 0
+; CHECK-NEXT:    v_mov_b32_e32 v7, 0
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr ptr addrspace(8) %ptr to i256
+  ret i256 %ret
+}
+
+define zeroext i64 @ptrtoint_trunc(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoint_trunc:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_mov_b32_e32 v1, v5
+; CHECK-NEXT:    v_mov_b32_e32 v0, v4
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoint ptr addrspace(8) %ptr to i64
+  ret i64 %ret
+}
+
+define zeroext i16 @ptrtoaddr_trunc(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
+; CHECK-LABEL: ptrtoaddr_trunc:
+; CHECK:       ; %bb.0:
+; CHECK-NEXT:    s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
+; CHECK-NEXT:    v_and_b32_e32 v0, 0xffff, v4
+; CHECK-NEXT:    s_setpc_b64 s[30:31]
+  %ret = ptrtoaddr ptr addrspace(8) %ptr to i16
+  ret i16 %ret
+}

@@ -0,0 +1,118 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
; RUN: llc -mtriple=amdgcn -global-isel -verify-machineinstrs < %s | FileCheck %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
; RUN: llc -mtriple=amdgcn -global-isel -verify-machineinstrs < %s | FileCheck %s
; RUN: llc -global-isel -mtriple=amdgcn-amd-amdpal -mcpu=gfx900 < %s | FileCheck %s

Avoid using the default subtarget

; CHECK-NEXT: s_setpc_b64 s[30:31]
%ret = ptrtoaddr ptr addrspace(8) %ptr to i16
ret i16 %ret
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should have SGPR variants too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a stupid question but I am not sure how to do this. I tried using addrspace(1), addrspace(4), and addrspace(8) but I get pretty much the same code generation?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The type is orthogonal to the register kind. To get an SGPR argument, you use an inreg argument. For the use of the value, it's probably easiest to use

call void asm sideeffect "; use $0", "s(i32 %use)

@arichardson arichardson changed the title [AMDGPU] Baseline test for ptrtoaddr code generation [AMDGPU] Add test for ptrtoaddr code generation Aug 11, 2025
@arichardson
Copy link
Member Author

Thanks to the change in ptrtoaddr semantics that the result type always has to be the address width, the code generation is now correct instead of buggy. I believe we should probably handle it explicitly anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants