[PATCH] D103431: [AMDGPU] Fix missing lowering of LDS used in global scope.

Mahesha S via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 1 07:36:57 PDT 2021


hsmhsm added a comment.

In D103431#2791017 <https://reviews.llvm.org/D103431#2791017>, @JonChesterfield wrote:

> For that test, I note that commenting out the llvm.used statement results in the variable being lowered. I.e. the following works fine:
>
>   @lds = addrspace(3) global float undef, align 8
>   @gptr = addrspace(1) global i64* addrspacecast (float addrspace(3)* @lds to i64*), align 8
>   
>   define void @f0() {
>     %ld = load i64*, i64* addrspace(1)* @gptr
>     ret void
>   }
>
> It's therefore likely that the missed handling is related to skipping uses from the .used list. This change seems to be a complete rewrite of the algorithm in shouldLowerLDSToStruct. I'm not sure we have the test coverage to be confident that a different algorithm will work, can we fix the .used oversight directly instead?

If the current logic of shouldLowerLDSToStruct() is more robust compere to earlier ones, then, why cannot we use it, instead of patching-up earlier ones?

I spent dedicated time to understand the shortcomings of shouldLowerLDSToStruct() and tried to overcome it in this patch. Please take a look at newly added tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103431



More information about the llvm-commits mailing list