[PATCH] D84391: [AMDGPU] Fix incorrect arch assert while setting up FlatScratchInit

[Scott Linder via Phabricator via llvm-commits]

scott.linder added a comment.

In D84391#2170741 <https://reviews.llvm.org/D84391#2170741>, @scott.linder wrote:

> Can we just add a predicate for whatever necessitates the SETREG for gfx10+? No idea on a good name, but if it were `flatScratchIsHwreg()` and it implied `flatScratchIsPointer()` this becomes:
>
>   if (ST.flatScratchIsHwreg()) {
>     // pointer add then S_SETREG
>   } else if (ST.flatScratchIsPointer()) (
>     // pointer add
>   } else {
>     // size and offset dance
>   }
>   llvm_unreachable();
>
>
> I agree that the existing assert here doesn't really mean anything, not sure why I wrote it.


Rather, the `llvm_unreachable` isn't needed if we consider the offset/add case as the "default", but maybe we should predicate it too? It would be nice if we could do this all in a way that was statically known to cover all the targets we have.

Could we combine the various predicate functions into one which returns an enumeration instead? Similar to Madhur's proposal, but without the direct reference to GFX#? I.e.

  switch (ST.getFlatScratchSetupKind()) {
  AMDGPU::PointerInHwreg:
    // ...
    break;
  AMDGPU::PointerInSGPR:
    // ...
    break;
  AMDGPU::SizeAndOffsetInSGPR:
    // ...
    break;
  }

Then when we add a new variant the compiler points us to everywhere that cares.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84391