[PATCH] Add field to InstAlias used to instruct AsmMatcherEmitter to ignore alias

[Jim Grosbach]

On Jul 19, 2013, at 1:40 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:

> Thank you for your quick response.
> 
> Could you be more specific? I am not sure if I understand what you mean when you say there is a bigger problem. 
> 
> - Do you mean it is a mistake to have two aliases with the same .s format in the .td file, and neither AsmWriterEmitter nor AsmMatcherEmitter should have to deal with two aliases?

This is correct. If you’re finding it necessary to define ambiguous aliases, that implies there’s something deeper in how the assembly syntax is modeled that should be looked at(1).

-Jim

(1) I’ll grant that MIPS has lots of really strange historical legacy syntax, assembler pseudo instructions, etc. that it needs to live with that could make this hard.

> 
> - Or we should leave InstAlias intact and define another class specifically for AsmWriterEmitter or instPrinter? This way, we can define as many aliases as we wish for instPrinter.
> 
> 
> 
> On Fri, Jul 19, 2013 at 1:12 PM, Jim Grosbach <grosbach at apple.com> wrote:
> This should be solved differently. InstAliases are for the parser and should never be ignored, so ambiguous aliases is an indication of a bigger problem.
> 
> -Jim
> 
> 
> On Jul 19, 2013, at 12:51 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
> 
> > The attached patch (instalias1.patch) adds field "Assemble", which AsmMatcherEmitter can use to decide whether it should ignore the alias.
> >
> > This is needed when there are two InstAlias patterns with the same AsmString. For example, in MipsInstrInfo.td, there are two patterns which have the .s format, "move $dst, $src":
> >
> > def : InstAlias<"move $dst, $src",
> >                 (ADDu CPURegsOpnd:$dst, CPURegsOpnd:$src,ZERO), 1>,
> >       Requires<[NotMips64]>;
> > def : InstAlias<"move $dst, $src",
> >                 (OR CPURegsOpnd:$dst, CPURegsOpnd:$src,ZERO), 1>,
> >
> > We need two aliases here because we want to have instPrinter print a "move" instruction instead of an "addu" or "or" when one of the operands is $zero:
> >
> > "addu $2, $3, $zero" or "or $2, $3, $zero" =>  "move $2, $3"
> >
> > However, this causes tablegen to place two entries in array MatchTable in MipsGenAsmMatcher.inc:
> >
> >   { 1364 /* move */, Mips::OR, Convert__CPURegsAsm1_0__CPURegsAsm1_1__regZERO, Feature_NotMips64, { MCK_CPURegsAsm, MCK_CPURegsAsm }, 0},
> >   { 1364 /* move */, Mips::ADDu, Convert__CPURegsAsm1_0__CPURegsAsm1_1__regZERO, Feature_NotMips64, { MCK_CPURegsAsm, MCK_CPURegsAsm }, 0},
> >
> > The order in which these entries appear in the table seems arbitrary, as I discovered when  I made completely unrelated changes in other places. We want to have the assembler generate an "ADDu" instruction and ignore the pattern with "OR".
> >
> > Please review.
> >
> > <instalias1.patch><instalias-mips.patch>_______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130719/ab378911/attachment.html>