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

[Jim Grosbach]

FWIW, it's also possible to directly recognize these sorts of things in the printer via C++ code. ARM does that for a few canonical aliases. It's not an optimal solution, but might be helpful.

-Jim

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

> Okay, then I guess I have to remove the second InstAlias (the one with OR). 
> 
> This will make llvm's disassembler slightly less user-friendly than gnu's disassembler, but probably that won't be a big problem.
> 
> 
> 
> On Fri, Jul 19, 2013 at 1:44 PM, Jim Grosbach <grosbach at apple.com> wrote:
> 
> 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/fc27a0f2/attachment.html>