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