[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