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

Duncan P. N. Exon Smith dexonsmith at apple.com
Mon Feb 17 20:59:28 PST 2014


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?



More information about the llvm-commits mailing list