Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[GISelValueTracking] Use representation size for G_PTRTOINT src width #139608

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

Open
wants to merge 2 commits into
base: users/arichardson/spr/main.giselvaluetracking-use-representation-size-for-g_ptrtoint-src-width
Choose a base branch
Loading
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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: 3 additions & 1 deletion 4 llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -483,8 +483,10 @@ void GISelValueTracking::computeKnownBitsImpl(Register R, KnownBits &Known,
if (Opcode == TargetOpcode::G_ASSERT_ZEXT)
SrcBitWidth = MI.getOperand(2).getImm();
else {
// For G_PTRTOINT all representation bits are returned even though only
// the address bits can be reasoned about generically.
SrcBitWidth = SrcTy.isPointer()
? DL.getIndexSizeInBits(SrcTy.getAddressSpace())
? DL.getPointerSizeInBits(SrcTy.getAddressSpace())
Comment on lines +486 to +489
Copy link
Contributor

Choose a reason for hiding this comment

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

Sticking G_INTTOPTR and G_PTRTOINT with these totally unrelated cases makes this a lot harder to follow

: SrcTy.getSizeInBits();
}
assert(SrcBitWidth && "SrcBitWidth can't be zero");
Expand Down
41 changes: 14 additions & 27 deletions 41 llvm/test/CodeGen/AMDGPU/GlobalISel/ptrtoint-ptrtoaddr-p8.ll
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ define <2 x i64> @ptrtoaddr_vec(ptr addrspace(8) %ignored, <2 x ptr addrspace(8)
; 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_and_b32_e32 v1, 0xffff, v5
; GISEL-NEXT: v_and_b32_e32 v3, 0xffff, v9
Comment on lines +82 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

The changed tests don't look specifically intended to catch mismatched sizes

; 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:
Expand Down Expand Up @@ -129,31 +129,18 @@ define i256 @ptrtoint_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
;; FIXME: this is wrong for the GlobalISel case, we are removing the trunc:
;; https://github.com/llvm/llvm-project/issues/139598
Comment on lines 129 to 130
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the fixme?

define i256 @ptrtoaddr_ext(ptr addrspace(8) %ignored, ptr addrspace(8) %ptr) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Test this in the vector case?

; 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]
; 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_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
; 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
}
Expand Down
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.