[PATCH] D144099: [WIP][AMDGPU] Fold more AGPR copies/PHIs in SIFoldOperands

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 15 10:30:56 PST 2023


arsenm added a comment.

Should have a MIR test. Also feels like it's more complicated than it needs to be to solve this



================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:1631
 
-// Try to hoist an AGPR to VGPR copy out of the loop across a LCSSA PHI.
+static Register tryFindExistingCopy(MachineRegisterInfo &MRI,
+                                    MachineInstr &Begin, Register FromReg,
----------------
If SIFoldOperands worked like PeepholeOpt, you would have a map of these already. I'd rather avoid a linear scan backwards for every copy


================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:1692
+    MachineOperand &MO = PHI.getOperand(K);
+    if (!MO.getReg())
+      return false;
----------------
This can't happen


================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:1696
+    Register PhiIn = MO.getReg();
+    if (MO.getSubReg() || !TRI->isVGPR(*MRI, PhiIn))
+      return false;
----------------
I'm somewhat surprised we allow subregs on phi inputs but the verifier doesn't seem to check this 


================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:1768
+    // TODO: Is using the PHI's DebugLoc okay here?
+    const unsigned RegState = (MRI->hasOneNonDBGUse(Reg) ? RegState::Kill : 0);
+    Register NewReg = MRI->createVirtualRegister(ARC);
----------------
Don't worry about setting kills, they're semi-deprecated and will be recomputed fully later anyway. Pre-regalloc you only need to worry about unsetting ones you may have had before


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144099



More information about the llvm-commits mailing list