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

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/LegalizationArtifactCombiner.h:277
+                                    GISelObserverWrapper &Observer) {
+    if (llvm::canReplaceReg(DstReg, SrcReg, MRI)) {
+      SmallVector<MachineInstr *, 4> UseMIs;
----------------
I think we can simplify the logic a bit here:

```
if (!llvm::canReplaceReg(DstReg, SrcReg, MRI) {
   Builder.buildCopy(DstReg, SrcReg);
   return;
}

SmallVector<MachineInstr *, 4> UseMIs;
// ...
```


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:165
+
+  // Give up the types don't match.
+  LLT DstTy = MRI.getType(DstReg);
----------------
Redundant comment?


================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:172
+  // Give up if the types don't match.
+  if (DstTy.isValid() && SrcTy.isValid() && DstTy != SrcTy)
+    return false;
----------------
Don't need to check both `DstTy.isValid()` and `SrcTy.isValid()`, since `DstTy.isValid() == SrcTy.isValid()` at this point.


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

https://reviews.llvm.org/D70564