[PATCH] D158463: [AMDGPU] Add IR-level pass to rewrite away address space 7

[Piotr Sobczak via Phabricator via llvm-commits]

piotr added a comment.

I have done the first round of testing of the patch on graphics content, with LLPC PatchBufferOp disabled and small modification in LLPC to use ptr addrspace (7) in lieu of lgc.buffer.desc.to.ptr.

Found 5 different issues, although 4 of them are in the isel and look related to the mentioned TODO ("break up large loads and stores into smaller parts"). The 5th one is in the pass itself (llvm/lib/IR/Value.cpp:507: void llvm::Value::doRAUW(llvm::Value *, llvm::Value::ReplaceMetadataUses): Assertion `!contains(New, this) && "this->replaceAllUsesWith(expr(this)) is NOT valid!"' failed.).
I will send you all reproducers offline.

I also noticed that s_buffer_load instruction is not generated anymore. In PatchBufferOp we have a logic that understands llvm.invariant.start intrinsic and generates s_buffer_load intrinsic accordingly. This is how the front-end marks a load from constant memory (side note: s_buffer_load can still end up being buffer_load during isel if offset is divergent, see SITargetLowering::lowerSBuffer). Do you have any plans to handle that use case in the new lowering? This is an important case for Vulkan applications for graphics uniform blocks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158463/new/

https://reviews.llvm.org/D158463