[PATCH] D122286: [AMDGPU] VGPR need to be reserved for AGPR copy *only* on subtarget GFX908

Mahesha S via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 23 02:45:30 PDT 2022


hsmhsm added a comment.

In D122286#3401911 <https://reviews.llvm.org/D122286#3401911>, @foad wrote:

> Can you simplify the patch? I think all it is doing is (a) adding a new test and (b) simplifying the "if" on line 609 because the condition is always true. All the other changes make it harder to see what the patch is really doing.

For sure, I need to change review message now based on my latest update where I lately realized that "hasGFX90AInsts() is true for both GFX90A and GFX940 sub-targets".

However, the whole intention of this patch is to make sure that the function indirectCopyToAGPR() is meaningful only in case of GFX908, and should be called only in case of GFX908(). Hence updated indirectCopyToAGPR() accordingly. I am not sure what else can be simplified here apart from meaningfully updating the review message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122286



More information about the llvm-commits mailing list