[PATCH] D36618: [globalisel][tablegen] Simplify named operand/operator lookups and fix a wrong-code bug this revealed.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: include/llvm/CodeGen/GlobalISel/InstructionSelector.h:158
+  /// - OtherOpIdx - Other operand index
+  GIM_CheckTie,
+
----------------
dsanders wrote:
> qcolombet wrote:
> > Food for thought: I wonder if we should use a different name like CheckIsSame, because when I see tie, it makes me think of the tie operand in the MCDesc and that's not what this is.
> > I believe I won't be the only one confused by this at first :).
> I see your point, I'll rename it. The reason I went with Tie was that the use of the opcode is triggered by using the same name multiple times like the MC layer.
I also renamed related functions like isTied().


================
Comment at: utils/TableGen/GlobalISelEmitter.cpp:2092
+      if (!OM.isTied())
+        OM.addPredicate<LiteralIntOperandMatcher>(SrcIntInit->getValue());
     } else
----------------
dsanders wrote:
> qcolombet wrote:
> > Could we have this check pushed into ::addPredicate?
> > That way we wouldn't have to spread it all around.
> I think so, I'll give it a go
I managed to make this work. addPredicate() now returns an optional and for the OperandMatcher will avoid adding more predicates when isSameAsAnotherOperand() is true.

The importer will now give up on a rule if isSameAsAnotherOperand() is true and it's asked to create a sub-instruction matcher. As far as I know, this situation never occurred but if it did it wouldn't have checked they were the same operand and would therefore have matched cases it shouldn't. It's technically possible to import this case but I don't think we should. It would occur for something like (add $src1:(sub $a, $b), $src1:(sub $a, $b)) but this only works if both $src1 sub-patterns are identical. If they differ, then matching is impossible and we ought to warn about that. However, rather than verify that the sub-patterns are identical, I think it makes more sense to encourage writing it as (add ($src1:(sub $a, $b), $src1). Without the redundancy, impossible rules can't occur.


Repository:
  rL LLVM

https://reviews.llvm.org/D36618