[PATCH] TableGen: AsmMatcherEmitter: Don't use custom converters for instruction aliases

[Tom Stellard]

In http://reviews.llvm.org/D9083#161432, @ab wrote:

> I can see this being useful, but I don't know these parts very well, so, just to make sure I understand this correctly:  someone might need to have an InstAlias-specific AsmMatchConverter, someday, right?  And they could restore the current behavior - which I can see being useful as well - by setting that to the same converter as the aliasee?  Also, when you say the "converter supplied with the alias", that would be the tblgen-erated converter, right?
>
> If that makes no sense, I'm not qualified to review;  if it does, LGTM ;)


I think I understand what you are saying about an InstAlias-specific AsmMatchConverter, but let me explain the issue to make sure we are both on the same page.

There are three ways to specify an AsmMatchConverter (the function that converts from OperandVector to MCInst):

1. Do nothing, tablegen will generate one for you.
2. Set Instruction::AsmMatchConverter to the name of a function (e.g. cvtFoo) and then implement that function in your target's AsmParser
3. Define an InstAlias, which will cause TableGen to generate a custom AsmMatchConverter based on the InstAlias.

This patch attempts to fix what I consider to be a bug, which is that if you do #2, then you can never use that instruction in an InstAlias, because TableGen will generate a call to your custom AsmMatchConverter (e.g. cvtFoo) rather than to the AsmMatchConverter that is generated based on the InstAlias.

In your comment, you point out the fact that someone may want to use the custom AsmMatchConverter (e.g. cvtFoo) for an InstAlias.  I had originally thought this would never be useful, but after reconsidering, I think it would potentially be useful in the case where you have an InstAlias where the mnemonic and MCInst are different e.g.

InstAlias <(shl $dst, $src0, 1), (mul GPR:$dst, GPR:$src0, 2)>;

So, it turns out you are qualified to review the patch ;)

I think the correct solution would be to add a bit to the InstAlias class that is used to determine which AsmMatchConverter to use (#2 or #3).  I will make that change to this patch.


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D9083

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/