Help with subtarget features and context-dependent asm parsers

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Help with subtarget features and context-dependent asm parsers

Richard Sandiford
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



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

check-features.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Help with subtarget features and context-dependent asm parsers

Tim Northover-2
> /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.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Help with subtarget features and context-dependent asm parsers

Richard Sandiford
Tim Northover <[hidden email]> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Help with subtarget features and context-dependent asm parsers

Jim Grosbach

On Jul 17, 2013, at 10:26 AM, Richard Sandiford <[hidden email]> wrote:

> Tim Northover <[hidden email]> 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
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Help with subtarget features and context-dependent asm parsers

Tim Northover-2
> 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.
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Help with subtarget features and context-dependent asm parsers

Jim Grosbach

On Jul 17, 2013, at 11:14 AM, Tim Northover <[hidden email]> wrote:

>> 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?
>

No objection. I vaguely recall there were some encoding differences for the same input strings between the sub targets, which is what motivated the split. If that's a faulty recollection, then yeah, merging them sounds great.


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


Cool.

-Jim
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev