[PATCH] D142192: [AMDGPU] Run unmerge combines post regbankselect

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1767-1775
+    // This combine may run after RegBankSelect, so we need to be aware of
+    // register banks.
+    if (MRI.getRegBankOrNull(DstReg) != MRI.getRegBankOrNull(SrcReg)) {
+      SrcReg = Builder.buildCopy(MRI.getType(SrcReg), SrcReg).getReg(0);
+      if (const RegisterBank *DstRB = MRI.getRegBankOrNull(DstReg))
+        MRI.setRegBank(SrcReg, *DstRB);
+      if (const TargetRegisterClass *RC = MRI.getRegClassOrNull(SrcReg))
----------------
I'm pretty sure we have a helper for this already (at least the artifact combiner handles this already)


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1771-1774
+      if (const RegisterBank *DstRB = MRI.getRegBankOrNull(DstReg))
+        MRI.setRegBank(SrcReg, *DstRB);
+      if (const TargetRegisterClass *RC = MRI.getRegClassOrNull(SrcReg))
+        MRI.setRegClass(SrcReg, RC);
----------------
Should go through one pair of getRegClassOrRegBank calls


================
Comment at: llvm/lib/Target/AMDGPU/SIInstructions.td:2050-2052
+  (V_BFI_B32_e64 (COPY_TO_REGCLASS VSrc_b32:$x, VGPR_32),
+                 (COPY_TO_REGCLASS VSrc_b32:$y, VGPR_32),
+                 (COPY_TO_REGCLASS VSrc_b32:$z, VGPR_32))
----------------
I think this needs to go off the a predicate. If we have to generate so many copies it's potentially worse than matching the pattern


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142192