[PATCH] D42293: [TableGen][AsmMatcherEmitter] Fix tied-constraint checking for InstAliases

[Oliver Stannard via Phabricator via llvm-commits]

olista01 added a comment.

> Well, this patch doesn't change the actual conversion to MCInst, it just encodes more information into the conversion-function by using an indirection through the 'TiedAsmOperandTable'. It will still create the MCInst in the same way as it did before.

Ah, yep, I misread the patch. It confused me a bit that CVT_Tied is being used for these two different cases (if i've understood it correctly this time):

- One parsed operand which generates two identical MCInst operands
- Two parsed operands which must bet the same, and generate two identical MCInst operand

> That might be possible, but I worry that it might lead to unmatched instructions and/or vague error messages if the register-operands don't match up. At the moment it just tries to match the whole instruction based on the operand classes, and then checks that the operands should be the same, already knowing they are of the right operand class, so we already know the instruction should otherwise match. What would be the benefit of doing it during matching of the operand classes?

I think the way it is being done now will result in the situation where a diagnostic says "operand must be a register in some range", the user fixes that, then the diagnostic changes to "operand must be the same as this other operand". That's something I've been trying to avoid, always emitting diagnostics that will result in a valid instruction if followed. My thought here was that it would be better to do all of the operand checks at once, so that we can favour the more precise diagnostics (the tied-operand ones) over the looser ones (operand class).

However, now that I've thought about it a bit more, I'm not sure that would work any better. In particular, in the case where the left-most of a pair of tied operands isn't in the correct register class, we can't emit the tied-operand diagnostic because we haven't matched the right-most operand yet. I also can't think of a good way to word diagnostics that say "_this_ operand is valid, but _that one_ needs to match it". Maybe it would be better to relax that rule, giving slightly less precise, but easier to understand diagnostics.


https://reviews.llvm.org/D42293