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 all commits
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
5 changes: 1 addition & 4 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,10 +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) {
// FIXME: this is not correct for pointers with addr width != pointer width
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
14 changes: 14 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,20 @@ bool IRTranslator::translateCast(unsigned Opcode, const User &U,
return true;
}

bool IRTranslator::translatePtrToAddr(const User &U,
MachineIRBuilder &MIRBuilder) {
Register Op = getOrCreateVReg(*U.getOperand(0));
Type *PtrTy = U.getOperand(0)->getType();
LLT AddrTy = getLLTForType(*DL->getAddressType(PtrTy), *DL);
auto IntPtrTy = getLLTForType(*DL->getIntPtrType(PtrTy), *DL);
auto PtrToInt = MIRBuilder.buildPtrToInt(IntPtrTy, Op);
Copy link
Contributor

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

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

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

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

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

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

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.

auto Addr = PtrToInt;
if (AddrTy != IntPtrTy)
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
11 changes: 5 additions & 6 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
Expand Up @@ -52,9 +52,9 @@ define <2 x i64> @ptrtoaddr_vec(ptr addrspace(8) %ignored, <2 x ptr addrspace(8)
; 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_and_b32_e32 v1, 0xffff, v5
; CHECK-NEXT: v_and_b32_e32 v3, 0xffff, v9
; 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
Expand All @@ -78,15 +78,14 @@ define zeroext i256 @ptrtoint_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %p
}

;; 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_and_b32_e32 v1, 0xffff, v5
; CHECK-NEXT: v_mov_b32_e32 v2, 0
; CHECK-NEXT: v_mov_b32_e32 v3, 0
; CHECK-NEXT: v_mov_b32_e32 v4, 0
; CHECK-NEXT: v_mov_b32_e32 v5, 0
; CHECK-NEXT: v_mov_b32_e32 v6, 0
Expand Down
Loading