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

I'm trying to add some instructions that are only available on certain
processors.  These instructions use context-dependent parsers.  Everything
works fine for the valid cases, but if you try to use an instruction on
processors that don't support it, the asm parser says:

/tmp/foo.s:1:2: error: invalid operands for instruction
        sllk    %r2,%r3,1
                ^
rather than:

/tmp/foo.s:1:2: error: instruction requires: distinct-ops
        sllk    %r2,%r3,1
        ^
This is because MatchOperandParserImpl() skips custom parsers if the
subtarget feature isn't enabled, so the instruction is parsed using
the default operand parser instead.  Then MatchInstructionImpl() only
returns Match_MissingFeature if an otherwise good match is found,
which in my case requires the custom parser to be used.

ARM seems to rely on the current MatchOperandParserImpl() behaviour,
so I'm not going to suggest changing it unconditionally.  But on SystemZ
there aren't any cases where the choice of parse routine depends on the
enabled features.  It'd be better just to parse in the same way
regardless and check for errors at the end.

The patch below does that by adding an optional argument to
MatchOperandParserImpl().  It seems really ugly though.  Does anyone have
any better suggestions?

Thanks,
Richard


-------------- next part --------------
A non-text attachment was scrubbed...
Name: check-features.diff
Type: text/x-patch
Size: 1704 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20130717/2d51d97c/attachment.bin>

> /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.
> 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.

Don't worry if not, I can try poking it myself based on your patch and
see what happens.

Cheers.

Tim.

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