Skip to content

[IRTranslator] Handle ptrtoaddr #139601

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: users/arichardson/spr/main.irtranslator-handle-ptrtoaddr
Choose a base branch
from
Open

[IRTranslator] Handle ptrtoaddr #139601

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
49f7574
[𝘀𝗽𝗿] initial version
arichardson May 12, 2025
01acf4a
rebase
arichardson May 13, 2025
daa8dc9
rebase
arichardson Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,7 @@ class IRTranslator : public MachineFunctionPass {
bool translatePtrToInt(const User &U, MachineIRBuilder &MIRBuilder) {
return translateCast(TargetOpcode::G_PTRTOINT, U, MIRBuilder);
}
bool translatePtrToAddr(const User &U, MachineIRBuilder &MIRBuilder) {
return translatePtrToInt(U, MIRBuilder);
}
bool translatePtrToAddr(const User &U, MachineIRBuilder &MIRBuilder);
bool translateTrunc(const User &U, MachineIRBuilder &MIRBuilder) {
return translateCast(TargetOpcode::G_TRUNC, U, MIRBuilder);
}
Expand Down
20 changes: 20 additions & 0 deletions llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1583,6 +1583,26 @@ bool IRTranslator::translateCast(unsigned Opcode, const User &U,
return true;
}

bool IRTranslator::translatePtrToAddr(const User &U,
MachineIRBuilder &MIRBuilder) {
if (containsBF16Type(U))
return false;

uint32_t Flags = 0;
if (const Instruction *I = dyn_cast<Instruction>(&U))
Flags = MachineInstr::copyFlagsFromInstruction(*I);

Register Op = getOrCreateVReg(*U.getOperand(0));
Type *PtrTy = U.getOperand(0)->getType();
LLT AddrTy = getLLTForType(*DL->getIndexType(PtrTy), *DL);
auto IntPtrTy = getLLTForType(*DL->getIntPtrType(PtrTy), *DL);
auto PtrToInt = MIRBuilder.buildPtrToInt(IntPtrTy, Op);
Copy link
Contributor

@arsenm arsenm May 12, 2025

Choose a reason for hiding this comment

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

Why not introduce a G_PTRTOADDR?

Copy link
Member Author

@arichardson arichardson May 12, 2025

Choose a reason for hiding this comment

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

I would be perfectly happy with that, not familar with GlobalIsel so not sure what the threshold is for adding a new operation. Happy to do that if it's cleaner?

Copy link
Contributor

@arsenm arsenm May 12, 2025

Choose a reason for hiding this comment

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

In general I expect a 1:1 mapping with IR opcodes

Copy link
Member Author

@arichardson arichardson May 16, 2025

Choose a reason for hiding this comment

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

I tried this in #140300 but it seems to me that adding this node introduces a lot of complexity for no real gain. I don't think we track provenance at the MIR level, so having the dedicated node probably doesn't enable much optimization over G_PTRTOINT+G_TRUNC?

Copy link
Contributor

@arsenm arsenm May 22, 2025

Choose a reason for hiding this comment

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

Maybe, but what if some legalization wants to introduce a ptrtoaddr? Are there any cases where that's useful?

Copy link
Collaborator

@jrtc27 jrtc27 Jun 10, 2025

Choose a reason for hiding this comment

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

We'd need a G_PTRTOADDR for CHERI given we can't do ptrtoint as it's defined upstream

Copy link
Member Author

@arichardson arichardson Jun 10, 2025

Choose a reason for hiding this comment

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

Well, considering ptrtoint does not return any of the hidden state, CHERI targets can lower a i128 ptrtoint as gethigh+shift+or. In cases where only the address bits are requested that trivially optimizes to just a read of the low bits.
Since ptrtoaddr is lowered as a ptrtoint truncating to address width, it would always be a trivial copy of the low register bits.

Adding the node is possible, but it will definitely need some more updates to GISelValueTracking.cpp to make sure we don't miscompile it.

PtrToInt->setFlags(Flags);
auto Addr = MIRBuilder.buildTrunc(AddrTy, PtrToInt.getReg(0));
MIRBuilder.buildZExtOrTrunc(getOrCreateVReg(U), Addr.getReg(0));
return true;
}

bool IRTranslator::translateGetElementPtr(const User &U,
MachineIRBuilder &MIRBuilder) {
Value &Op0 = *U.getOperand(0);
Expand Down
187 changes: 187 additions & 0 deletions llvm/test/CodeGen/AMDGPU/GlobalISel/ptrtoint-ptrtoaddr-p8.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,187 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
; RUN: llc -mtriple=amdgcn -global-isel -verify-machineinstrs --print-changed --debug < %s | FileCheck %s --check-prefixes=CHECK,GISEL
; RUN: llc -mtriple=amdgcn -verify-machineinstrs < %s | FileCheck %s --check-prefixes=CHECK,SDAG
;; Check that we can lower ptrtoaddr differently from ptrtoint.
;; Includes an ignored argument so the registers actually need to be written

define i128 @ptrtoint(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
; GISEL-LABEL: ptrtoint:
; GISEL: ; %bb.0:
; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GISEL-NEXT: v_mov_b32_e32 v0, v4
; GISEL-NEXT: v_mov_b32_e32 v1, v5
; GISEL-NEXT: v_mov_b32_e32 v2, v6
; GISEL-NEXT: v_mov_b32_e32 v3, v7
; GISEL-NEXT: s_setpc_b64 s[30:31]
;
; SDAG-LABEL: ptrtoint:
; SDAG: ; %bb.0:
; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SDAG-NEXT: v_mov_b32_e32 v3, v7
; SDAG-NEXT: v_mov_b32_e32 v2, v6
; SDAG-NEXT: v_mov_b32_e32 v1, v5
; SDAG-NEXT: v_mov_b32_e32 v0, v4
; SDAG-NEXT: s_setpc_b64 s[30:31]
%ret = ptrtoint ptr addrspace(8) %ptr to i128
ret i128 %ret
}

define i48 @ptrtoaddr(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
; GISEL-LABEL: ptrtoaddr:
; GISEL: ; %bb.0:
; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GISEL-NEXT: v_mov_b32_e32 v0, v4
; GISEL-NEXT: v_mov_b32_e32 v1, v5
; GISEL-NEXT: s_setpc_b64 s[30:31]
;
; SDAG-LABEL: ptrtoaddr:
; SDAG: ; %bb.0:
; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SDAG-NEXT: v_mov_b32_e32 v1, v5
; SDAG-NEXT: v_mov_b32_e32 v0, v4
; SDAG-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) {
; GISEL-LABEL: ptrtoint_vec:
; GISEL: ; %bb.0:
; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GISEL-NEXT: v_mov_b32_e32 v0, v4
; GISEL-NEXT: v_mov_b32_e32 v1, v5
; GISEL-NEXT: v_mov_b32_e32 v2, v6
; GISEL-NEXT: v_mov_b32_e32 v3, v7
; GISEL-NEXT: v_mov_b32_e32 v4, v8
; GISEL-NEXT: v_mov_b32_e32 v5, v9
; GISEL-NEXT: v_mov_b32_e32 v6, v10
; GISEL-NEXT: v_mov_b32_e32 v7, v11
; GISEL-NEXT: s_setpc_b64 s[30:31]
;
; SDAG-LABEL: ptrtoint_vec:
; SDAG: ; %bb.0:
; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SDAG-NEXT: v_mov_b32_e32 v3, v7
; SDAG-NEXT: v_mov_b32_e32 v2, v6
; SDAG-NEXT: v_mov_b32_e32 v1, v5
; SDAG-NEXT: v_mov_b32_e32 v0, v4
; SDAG-NEXT: v_mov_b32_e32 v4, v8
; SDAG-NEXT: v_mov_b32_e32 v5, v9
; SDAG-NEXT: v_mov_b32_e32 v6, v10
; SDAG-NEXT: v_mov_b32_e32 v7, v11
; SDAG-NEXT: s_setpc_b64 s[30:31]
%ret = ptrtoint <2 x ptr addrspace(8)> %ptr to <2 x i128>
ret <2 x i128> %ret
}

define <2 x i64> @ptrtoaddr_vec(ptr addrspace(8) %ignored, <2 x ptr addrspace(8)> %ptr) {
; GISEL-LABEL: ptrtoaddr_vec:
; GISEL: ; %bb.0:
; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GISEL-NEXT: v_mov_b32_e32 v0, v4
; GISEL-NEXT: v_mov_b32_e32 v1, v5
; GISEL-NEXT: v_mov_b32_e32 v2, v8
; GISEL-NEXT: v_mov_b32_e32 v3, v9
; GISEL-NEXT: s_setpc_b64 s[30:31]
;
; SDAG-LABEL: ptrtoaddr_vec:
; SDAG: ; %bb.0:
; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SDAG-NEXT: v_mov_b32_e32 v2, v8
; SDAG-NEXT: v_and_b32_e32 v1, 0xffff, v5
; SDAG-NEXT: v_and_b32_e32 v3, 0xffff, v9
; SDAG-NEXT: v_mov_b32_e32 v0, v4
; SDAG-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 i256 @ptrtoint_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
; GISEL-LABEL: ptrtoint_ext:
; GISEL: ; %bb.0:
; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GISEL-NEXT: v_mov_b32_e32 v0, v4
; GISEL-NEXT: v_mov_b32_e32 v1, v5
; GISEL-NEXT: v_mov_b32_e32 v2, v6
; GISEL-NEXT: v_mov_b32_e32 v3, v7
; GISEL-NEXT: v_mov_b32_e32 v4, 0
; GISEL-NEXT: v_mov_b32_e32 v5, 0
; GISEL-NEXT: v_mov_b32_e32 v6, 0
; GISEL-NEXT: v_mov_b32_e32 v7, 0
; GISEL-NEXT: s_setpc_b64 s[30:31]
;
; SDAG-LABEL: ptrtoint_ext:
; SDAG: ; %bb.0:
; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SDAG-NEXT: v_mov_b32_e32 v3, v7
; SDAG-NEXT: v_mov_b32_e32 v2, v6
; SDAG-NEXT: v_mov_b32_e32 v1, v5
; SDAG-NEXT: v_mov_b32_e32 v0, v4
; SDAG-NEXT: v_mov_b32_e32 v4, 0
; SDAG-NEXT: v_mov_b32_e32 v5, 0
; SDAG-NEXT: v_mov_b32_e32 v6, 0
; SDAG-NEXT: v_mov_b32_e32 v7, 0
; SDAG-NEXT: s_setpc_b64 s[30:31]
%ret = ptrtoint ptr addrspace(8) %ptr to i256
ret i256 %ret
}

;; FIXME: this is wrong for the GlobalISel case, we are removing the trunc:
;; https://github.com/llvm/llvm-project/issues/139598
define i256 @ptrtoaddr_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
; GISEL-LABEL: ptrtoaddr_ext:
; GISEL: ; %bb.0:
; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GISEL-NEXT: v_mov_b32_e32 v0, v4
; GISEL-NEXT: v_mov_b32_e32 v1, v5
; GISEL-NEXT: v_mov_b32_e32 v2, v6
; GISEL-NEXT: v_mov_b32_e32 v3, v7
; GISEL-NEXT: v_mov_b32_e32 v4, 0
; GISEL-NEXT: v_mov_b32_e32 v5, 0
; GISEL-NEXT: v_mov_b32_e32 v6, 0
; GISEL-NEXT: v_mov_b32_e32 v7, 0
; GISEL-NEXT: s_setpc_b64 s[30:31]
;
; SDAG-LABEL: ptrtoaddr_ext:
; SDAG: ; %bb.0:
; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SDAG-NEXT: v_mov_b32_e32 v0, v4
; SDAG-NEXT: v_and_b32_e32 v1, 0xffff, v5
; SDAG-NEXT: v_mov_b32_e32 v2, 0
; SDAG-NEXT: v_mov_b32_e32 v3, 0
; SDAG-NEXT: v_mov_b32_e32 v4, 0
; SDAG-NEXT: v_mov_b32_e32 v5, 0
; SDAG-NEXT: v_mov_b32_e32 v6, 0
; SDAG-NEXT: v_mov_b32_e32 v7, 0
; SDAG-NEXT: s_setpc_b64 s[30:31]
%ret = ptrtoaddr ptr addrspace(8) %ptr to i256
ret i256 %ret
}

define i64 @ptrtoint_trunc(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
; GISEL-LABEL: ptrtoint_trunc:
; GISEL: ; %bb.0:
; GISEL-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; GISEL-NEXT: v_mov_b32_e32 v0, v4
; GISEL-NEXT: v_mov_b32_e32 v1, v5
; GISEL-NEXT: s_setpc_b64 s[30:31]
;
; SDAG-LABEL: ptrtoint_trunc:
; SDAG: ; %bb.0:
; SDAG-NEXT: s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
; SDAG-NEXT: v_mov_b32_e32 v1, v5
; SDAG-NEXT: v_mov_b32_e32 v0, v4
; SDAG-NEXT: s_setpc_b64 s[30:31]
%ret = ptrtoint ptr addrspace(8) %ptr to i64
ret i64 %ret
}

define 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_mov_b32_e32 v0, v4
; CHECK-NEXT: s_setpc_b64 s[30:31]
%ret = ptrtoaddr ptr addrspace(8) %ptr to i16
ret i16 %ret
}
Loading
You are viewing a condensed version of this merge commit. You can view the full changes here.