[PATCH] D109870: [AMDGPU] Enable the pass "amdgpu-replace-lds-use-with-pointer"

[Mahesha S via Phabricator via llvm-commits]

hsmhsm added a comment.

In D109870#3012287 <https://reviews.llvm.org/D109870#3012287>, @JonChesterfield wrote:

> Hi Roman, Nicolai,
>
> This is an IR transform which assumes alloca are contiguous within the entry block. Mahesha is presenting your replies to the linked thread as community approval for miscompiling code where that assumption does not hold. Somewhat indirectly, this pass will replace load/store of a addrspace(3) global variable with load/store from an uninitialised address if there happens to be a call to a function using said variable in the middle of the alloca in the entry block.
>
> I won't speak for either of you here. I will say that I consider using your replies to a tangentially related question as authority to fast track a known broken IR pass to be inconceivable behaviour.

Hi Jon,

Small correction: I am not claiming that "alloca are contiguous within the entry block". please re-read my replies.

Hi Roman, Nicolai,

Here is my understanding and claim about static allocas (not dynamic allocas) - All static allocas should appear in the entry basic block before any function call for better optimization opportunities. If there are interleaved static allocas with function call in-between, such an ir is considered broken, even though the ir is valid from correctness perspective. And if any pass is not adhering to the requirement that all static allocas should be placed in the entry block before any function call, then such a pass is considered broken since it may leads to surprising result in general.

Let me know if my above understanding is correct or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109870