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

[Petar Avramovic via Phabricator via llvm-commits]

Petar.Avramovic added a comment.

With removed xfail from tests, this looks good to me. I would like to hear if other reviewers have comments.



================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:273
+  /// depending on the register constraints.
+  static void replaceRegOrBuildCopy(Register DstReg, Register SrcReg,
+                                    MachineRegisterInfo &MRI,
----------------
Where will we use replaceRegOrBuildCopy so it has to be static?
I think that eventually Observer should end up as field in LegalizationArtifactCombiner like in LegalizerHelper. There are a few `LLVM_DEBUG(dbgs() << ".. Combine MI: " << MI;);` around and Observer could handle them.


================
Comment at: llvm/test/CodeGen/Mips/GlobalISel/regbankselect/long_ambiguous_chain_s32.mir:3-6
+
+; XFAIL: *
+; MipsRegisterBankInfo is unable to handle COPY instructions.
+
----------------
With updated patch there are no changes in regbankselect tests, you can remove XFAIL. Register we are replacing in combine has no constraints.


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

https://reviews.llvm.org/D70564