llvm dev - Jul 2013 - [LLVMdev] Help with subtarget features and context-dependent asm parsers

Tim Northover <t.p.northover at gmail.com> writes:
>> /tmp/foo.s:1:2: error: instruction requires: distinct-ops
>>         sllk    %r2,%r3,1
>>         ^
>
> That seems like it would be a good improvement for all targets.
Thanks, sounds like it might be more acceptable than I thought :-)
>> ARM seems to rely on the current MatchOperandParserImpl() behaviour,
>> so I'm not going to suggest changing it unconditionally.
>
> Presumably you switched it and looked at what fell over; do you
> remember what kind of problems ARM had? Perhaps we can fix ARM so that
> your change works there too.
Yeah, there were two new MC failures.  The first was:

/home/richards/llvm/build/Debug+Asserts/bin/llvm-mc -triple=thumbv7-apple-darwin
-mcpu=cortex-a8 -show-encoding < /home/richards/llvm/src/test/MC/ARM/basic
-thumb2-instructions.s | /home/richards/llvm/build/Debug+Asserts/bin/FileCheck
/home/richards/llvm/src/test/MC/ARM/basic-thumb2-instructions.s
--
Exit Code: 1
Command Output (stderr): 
--
<stdin>:1356:9: error: instruction requires: armv7m
        mrs  r8, apsr
        ^
<stdin>:1357:9: error: instruction requires: armv7m
        mrs  r8, cpsr
        ^
<stdin>:1358:9: error: instruction requires: armv7m
        mrs  r8, spsr
        ^

and the second was the same for basic-arm-instructions.s.  The problem seems
to be that the MSRMask parser is then always used, even for non-M-class.

Richard

On Jul 17, 2013, at 10:26 AM, Richard Sandiford <rsandifo at
linux.vnet.ibm.com> wrote:
> Tim Northover <t.p.northover at gmail.com> writes:
>>> /tmp/foo.s:1:2: error: instruction requires: distinct-ops
>>>        sllk    %r2,%r3,1
>>>        ^
>> 
>> That seems like it would be a good improvement for all targets.
> 
> Thanks, sounds like it might be more acceptable than I thought :-)
FWIW, I'm the guy to blame for the current implementation and I like the
idea. Getting it right may be marginally tricky, but the direction is good.

Better diagnostics from the assemblers is a very good thing.
> 
>>> ARM seems to rely on the current MatchOperandParserImpl()
behaviour,
>>> so I'm not going to suggest changing it unconditionally.
>> 
>> Presumably you switched it and looked at what fell over; do you
>> remember what kind of problems ARM had? Perhaps we can fix ARM so that
>> your change works there too.
> 
> Yeah, there were two new MC failures.  The first was:
> 
> /home/richards/llvm/build/Debug+Asserts/bin/llvm-mc
-triple=thumbv7-apple-darwin -mcpu=cortex-a8 -show-encoding <
/home/richards/llvm/src/test/MC/ARM/basic
> -thumb2-instructions.s |
/home/richards/llvm/build/Debug+Asserts/bin/FileCheck
/home/richards/llvm/src/test/MC/ARM/basic-thumb2-instructions.s
> --
> Exit Code: 1
> Command Output (stderr): 
> --
> <stdin>:1356:9: error: instruction requires: armv7m
>        mrs  r8, apsr
>        ^
> <stdin>:1357:9: error: instruction requires: armv7m
>        mrs  r8, cpsr
>        ^
> <stdin>:1358:9: error: instruction requires: armv7m
>        mrs  r8, spsr
>        ^
> 
> and the second was the same for basic-arm-instructions.s.  The problem
seems
> to be that the MSRMask parser is then always used, even for non-M-class.
This seems fixable. The custom parsers that are only valid for certain sub
targets could easily have an explicit early-exit if the active sub target
isn't what it's looking for. Would that be sufficient here?

-Jim

> This seems fixable. The custom parsers that are only valid for certain sub
targets could easily have an explicit early-exit if the active sub target
isn't what it's looking for. Would that be sufficient here?
It doesn't look like there are different encodings for the same string
so I don't think even that would be necessary. How about partitioning
by MRS_APSR and MRS_OtherM instead of the current MRS_M and MRS_AR?

I'll see if I can put a patch together.

Tim.