[PATCH] D111506: [GlobalISel][Tablegen] Fix SameOperandMatcher's isIdentical check

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/test/TableGen/GlobalISelEmitterMatchTableOptimizerSameOperand-invalid.td:16
+// CHECK-NEXT:        GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
+// CHECK-NEXT:        GIM_CheckIsSameOperand, /*MI*/0, /*OpIdx*/3, /*OtherMI*/2, /*OtherOpIdx*/2,
+// CHECK-NEXT:        GIM_Try, /*On fail goto*//*Label 2*/ 108, // Rule ID 1 //
----------------
kschwarz wrote:
> Even with the fix for the `isSameOperand` predicate, we can still have a situation where hoisting the check is not correct.
> 
> In this case, the optimizer finds that the operand checks are indeed identical, and hoists the check into a common block.
> However, the check will unconditionally access operand index 2 of `/*OtherMI*/2`.
> If the input GMIR does not have three operands in `/*OtherMI*/2`, this will crash.
> 
> The safest way would be to forbid hoisting `SameOperandMatcher`s altogether.
> Anyone have a better idea?
Just to make sure I understand correctly: what you're saying is this patch fixes one problem but there are others that need to be fixed as well, but this is orthogonal to this patch.

Is that right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111506