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

Akira Hatanaka ahatanak at gmail.com
Fri Jul 19 13:40:25 PDT 2013


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?

- 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/059455e6/attachment.html>


More information about the llvm-commits mailing list