This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Combine is.shared/is.private of null/undef
ClosedPublic

Authored by arsenm on Dec 13 2021, 1:17 PM.

Details

Reviewers
rampitec

Diff Detail

Unit TestsFailed

Event Timeline

arsenm created this revision.Dec 13 2021, 1:17 PM
arsenm requested review of this revision.Dec 13 2021, 1:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 13 2021, 1:17 PM
Herald added a subscriber: wdng. · View Herald Transcript
rampitec added inline comments.Dec 13 2021, 1:49 PM
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
900

Why cannot we have a shared or private null pointer?

arsenm added inline comments.Dec 13 2021, 1:55 PM
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
900

The query is for whether the particular address points into the aperture. The null pointer doesn't point at anything, so therefore it doesn't point into the shared/private segment

rampitec added inline comments.Dec 13 2021, 2:07 PM
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
900

You can have a flat pointer pointing exactly into the address zero of LDS or private. Aren't these addresses also null?

rampitec added inline comments.Dec 13 2021, 2:17 PM
llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
900

I believe this is not correct and clearly change in behavior:

define i1 @test(i32 addrspace(1)* %res) {
  %cast = addrspacecast i8 addrspace(5)* null to i8*
  %is_private = tail call i1 @llvm.amdgcn.is.private(i8* %cast)
  ret i1 %is_private
}

declare i1 @llvm.amdgcn.is.private(i8* nocapture)

Right now it is true:

s_waitcnt vmcnt(0) expcnt(0) lgkmcnt(0)
v_mov_b32_e32 v0, 1
s_setpc_b64 s[30:31]
rampitec accepted this revision.Dec 13 2021, 2:22 PM

LGTM

llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp
900

Ah, OK. Checked definition of is_private and is_shared. They only take flat pointers, and flat null is not the same as private or shared null.

This revision is now accepted and ready to land.Dec 13 2021, 2:22 PM