[PATCH] D122286: [AMDGPU] Add missing testcase for SGPR to AGPR copy

Jay Foad via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 23 03:23:00 PDT 2022


foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:554
                                Register ImpUseSuperReg = Register()) {
-  const SIRegisterInfo &RI = TII.getRegisterInfo();
+  // FIXME: Add a check hasGFX908Insts() and appropriately replace below assert
+  // check?
----------------
Either do this or don't. Adding another FIXME is just more clutter. Anyway I think it is more of a TODO than a FIXME since it is not really broken.


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:608
 
+  // FIXME: When we have reserved VGPR for this indirect copy, why do we need
+  //        scavenger here?
----------------
I don't think you need a FIXME comment for this. It looks like the code is using the scavenger to try to find a low-numbered vgpr, and only using the reserved one if that fails.


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