[PATCH] D70564: [GlobalISel] LegalizationArtifactCombiner: Fix a bug in tryCombineMerges

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:162
+  if (Register::isPhysicalRegister(DstReg) ||
+      Register::isPhysicalRegister(SrcReg))
+    return false;
----------------
You can do .isPhysical()


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:171-172
+  // Give up if the types don't match.
+  if (DstTy != SrcTy)
+    return false;
+
----------------
This still looks redundant to me.  (DstTy.isValid() != SrcTy.isValid()) is implied by DstTy != SrcTy


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:175-178
+  const RegisterBank *DstBank = MRI.getRegBankOrNull(DstReg);
+  const RegisterBank *SrcBank = MRI.getRegBankOrNull(SrcReg);
+  const TargetRegisterClass *DstRC = MRI.getRegClassOrNull(DstReg);
+  const TargetRegisterClass *SrcRC = MRI.getRegClassOrNull(SrcReg);
----------------
Just check getRegClassOrRegBank(Src) == getRegClassOrRegBank(Dst)?


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:183-185
+  // Replace if DstReg has no constraints.
+  if (!DstBank && !DstRC)
+    return true;
----------------
Check these first and short circuit the source checks?


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

https://reviews.llvm.org/D70564