patch for Bug 18833 - ARMAsmParser fails to recognize .req directive alias name in capital letters

[Saleem Abdulrasool]

On Mon, Feb 17, 2014 at 8:59 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> On 2014 Feb 17, at 20:30, lin zuojian <manjian2006 at gmail.com> wrote:
>
> > patch v2:
> >
> > Index: test/MC/ARM/dot-req-capital.s
> > ===================================================================
> > --- test/MC/ARM/dot-req-capital.s (revision 0)
> > +++ test/MC/ARM/dot-req-capital.s (working copy)
> > @@ -0,0 +1,14 @@
> > +@ RUN: llvm-mc -triple=arm-linux-androideabi < %s | FileCheck %s
> > + .syntax unified
> > +_foo:
> > +
> > + OBJECT .req r2
> > + mov r4, OBJECT
> > + mov r4, oBjEcT
> > + .unreq OBJECT
> > +
> > +_foo2:
> > + OBJECT .req r5
> > + mov r4, OBJECT
> > + .unreq OBJECT
>
> Awesome, almost there.
>
> 1. Add a couple of CHECK lines to confirm the output is correct.  Probably
> along
>    the lines of:
>
>         @ CHECK-LABEL: _foo:
>         @ CHECK: mov r4, r2
>         @ CHECK: mov r4, r2
>
>     and
>
>         @ CHECK-LABEL: _foo2:
>         @ CHECK: mov r4, r5
>
>     unless I’ve misunderstood what .req does.
>
> 2. Add a comment to the testcase (usually after the RUN line) to document
> why
>    this testcase is important.  Often people will reference the bug here.
>    E.g.,
>
>         @ PR18833: Confirm that .req and .unreq are case-insensitive.
>
> > Index: lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> > ===================================================================
> > --- lib/Target/ARM/AsmParser/ARMAsmParser.cpp (revision 201500)
> > +++ lib/Target/ARM/AsmParser/ARMAsmParser.cpp (working copy)
> > @@ -8243,7 +8243,7 @@
> > Error(L, "unexpected input in .unreq directive.");
> > return false;
> > }
> > - RegisterReqs.erase(Parser.getTok().getIdentifier());
> > + RegisterReqs.erase(Parser.getTok().getIdentifier().lower());
> > Parser.Lex(); // Eat the identifier.
> > return false;
> > }
>
> The code looks correct to me, but I haven’t worked with the AsmParsers
> before.
>
> Anyone else see a problem with it?
>

Whilst this is conceptually correct, I dont think it is sufficient.  This
will fix the `.unreq' directive but will not handle the use of the alias.
 Both of them should be addressed simultaneously imo.


> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>

-- 
Saleem Abdulrasool
compnerd (at) compnerd (dot) org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140217/a3a577e6/attachment.html>