[PATCH] D107362: GlobalISel: Fix matchEqualDefs for instructions with multiple defs

[Jay Foad via Phabricator via llvm-commits]

foad added a reviewer: rampitec.
foad added a comment.

The bug fix looks good to me. Inline comments are just about optimization.



================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2671-2676
-  // %0:_(s64), %1:_(s64) = G_UNMERGE_VALUES %2:_(<2 x s64>)
-  //
-  // Even though %0 and %1 are produced by the same instruction they are not
-  // the same values.
-  if (I1 == I2)
-    return MOP1.getReg() == MOP2.getReg();
----------------
I'm not sure if it's a good idea to remove this code.
- I think it's correct(?)
- It looks like a useful speed optimization if I1 and I2 happen to be the same instruction.
- It allows returning true if I1 and I2 refer to the same load or store, otherwise we'd return false below.
Thoughts?
Adding Stas who wrote this code.


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:2732
+    // %1 and %6 are same, %1 and %7 are not the same value.
+    return I1->findRegisterDefOperandIdx(InstAndDef1->Reg) ==
+           I2->findRegisterDefOperandIdx(InstAndDef2->Reg);
----------------
If findRegisterDefOperandIdx is slow then it might be worth adding a fast case like: "if I1 only has 1 def, return true". But I'm not sure if it's worth it.


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

https://reviews.llvm.org/D107362