[PATCH] D115945: [AMDGPU][GlobalISel] Eliminate cross regbank copies of constants

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

Could use a dedicated mir test



================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCombine.td:111
+  (defs root:$const, copybank_matchdata:$matchinfo),
+  (match (wip_match_opcode G_CONSTANT):$const,
+         [{ return RegBankHelper.matchConstVgpr(*${const}, ${matchinfo}); }]),
----------------
Should also apply to G_FCONSTANT?


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp:360-361
+  Register Dst = MI.getOperand(0).getReg();
+  if (MRI.use_nodbg_empty(Dst))
+    return false; // Dead instruction
+
----------------
Shouldn't really need to specially check this in an individual combine


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp:371
+
+  // Check if all uses of constant are copies to same regbank
+  for (MachineInstr &Use : MRI.use_nodbg_instructions(Dst)) {
----------------
This level of heuristic could have/should have been done by greedy regbankselect.

However, I think we want something a bit smarter here. We should rematerialize all uses that are inline constants. For literal constants, we can make a code size tradeoff based on the number of uses and if other operands are scalars


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegBankCombiner.cpp:405
+  // Gather original uses of Dst
+  llvm::SmallVector<MachineInstr *> Copies;
+  for (MachineInstr &Use : MRI.use_nodbg_instructions(Dst)) {
----------------
Don't need llvm::


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

https://reviews.llvm.org/D115945