[PATCH] D95071: [AMDGPU] Fix the inconsistency in soffset for MUBUF stack accesses.

Wed Jan 20 15:03:02 PST 2021

scott.linder added a comment.

In D95071#2511105 <https://reviews.llvm.org/D95071#2511105>, @arsenm wrote:

> In D95071#2511070 <https://reviews.llvm.org/D95071#2511070>, @scott.linder wrote:
>
>> Thank you for the patch, I definitely think this is a step in the right direction! However, I think we are still broken in common cases, the first of which can be seen by simply removing `amdgpu_kernel` from `llvm/test/CodeGen/AMDGPU/stack-pointer-offset-relative-frameindex.ll` and noticing that the `soffset` of the `buffer_load_dword`s in `%bb.1` do not correctly refer to the frame pointer, they are constant `0` just like in the kernel case:
>
> The frame index being separated from the memory operation is fine. 0 is always an acceptable soffset, kernel or not. The vaddr will be interpreted as the absolute address. If the frame index is materialized by a move, it will be expanded such that the address placed in the mubuf vaddr should work

OK, that makes sense, sorry for the noise!

I'm not sure why I didn't consider the code right above the `MUBUF` handling in `eliminateFrameIndex` before assuming this wasn't correct. I'm also not sure why I didn't just read the ISA for the function case more carefully.

I'm satisfied that starting with `0` for the frame-index case and requiring that be the true in `eliminateFrameIndex` is a sound approach, then.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95071