[PATCH] D108971: [AMDGPU] Split entry basic block after alloca instructions.

Wed Sep 1 20:39:41 PDT 2021

hsmhsm added a comment.

In D108971#2977372 <https://reviews.llvm.org/D108971#2977372>, @arsenm wrote:

> In D108971#2976989 <https://reviews.llvm.org/D108971#2976989>, @JonChesterfield wrote:
>
>>> Only such possibility for breaking is - there is a call to non-kernel function foo() before alloca, and foo() uses LDS. But, the assumption here is that usually won't happen, because allocas are usually put at the beggining of the entry block.
>>
>> I've seen alloca in blocks other than the entry block after inlining. I think I've seen a function call followed by an alloca as well. I can't think of a reason why that would be invalid IR, and I think it would be possible to set up a series of passes that create it. Could you add a (handwritten) test case with the pattern that is miscompiled?
>>
>> In such a case we could move the alloca to the start of the basic block. We might actually want to move alloca to the (start of the) entry block in general for amdgpu as (I think, it's been a few months) we can only lower them in the entry block.
>
> This needs to work correctly. Alloca can legally be placed anywhere

I think we discussed these items in our last Monday's internal weekly meeting. I am recapping it here again:

- First,  the entry block splitting here should happen after all the allocas which are inserted at the beginning of the block before any other non-alloca instructions which actually make sense. Note down the word  `beginning of the block` here.  But, this will not guarentee that split is always happened after alloca. As Jon mentioned, there could be a call to function foo(), and foo() has allocas, and which is inlined so that these allocas do appear after split and which is perfectly legal. But, AMDGPU back-end is not handling it correctly at the moment which is a bug in AMDGPU back-end.
- Second, root-cause why AMDGPU back-end is not able to handle allocas which are not inserted at the beginning of the block.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108971