This is an archive of the discontinued LLVM Phabricator instance.

[amdgpu] Teach one more case for assumed global pointers.
AbandonedPublic

Authored by hliao on Dec 1 2020, 7:34 AM.

Details

Summary
  • If a generic pointer is loaded from a readonly and noalias kernel pointer argument, it could be assumed as a global one. + readonly prevents the possible modifications from the device side, and + noalias ensures that pointer won't alias to any other objects in the whole kernel execution lifetime. + Taking them together, it's safe to assume that memory object could only be modified on the host side and contains global pointers only.

Diff Detail

Event Timeline

hliao created this revision.Dec 1 2020, 7:34 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 1 2020, 7:34 AM
hliao requested review of this revision.Dec 1 2020, 7:34 AM
arsenm added inline comments.Dec 4 2020, 2:08 PM
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
553

Readonly is incorrect here, as it only indicates the function does not write to the memory and does not indicate the memory may be changed by another function/thread. I also don't think noalias is really the correct check for the owned object

hliao added inline comments.Dec 4 2020, 6:44 PM
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
553

That readonly atttribute is marked in function attribute deduction pass by checking every use of a pointer argument, including if it's passed into another sub-func. It's only marked as readonly when it's safe. It's safe to assume there's no write though a pointer argument marked with readonly.
noalias here is to ensure that there's no other path within the kernel function execution to modify that memory pointed by that pointer argument.
By checking both, we could ensure that memory is only prepared by the host.

arsenm requested changes to this revision.Mar 30 2021, 3:57 PM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
553

Yes, it infers that *this* function did not modify the memory. It's a step further to conclude that no function exists that's modifying the memory. A different kernel could have access to the same memory which it is writing to

This revision now requires changes to proceed.Mar 30 2021, 3:57 PM
hliao abandoned this revision.Mar 30 2021, 9:05 PM

the case is no longer valid considering concurrent kernel execution.

llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
553

yeah, by allowing concurrent kernel execution, the assumption is no longer held.