[PATCH] Fix treatment of ARM unallocated hint instructions

[Mihail Popa]

Jim,

I will address your concerns in a subsequent patch.
You are correct - the same issue is present in the Thumb encoding space as
well.
I have left that alone simply because ARM encodings are of higher priority
at the moment, but will fix in the near future.

Mihai



On Wed, Apr 17, 2013 at 11:52 PM, Jim Grosbach <grosbach at apple.com> wrote:

>
> Thanks, much clearer now. This is a nice tightening up of things.
>
> Also, thanks for reviewing this Quentin. I just a have a few additional
> comments on some of the details.
>
> +def Imm0_4AsmOperand : ImmAsmOperand { let Name = "Imm0_4"; }
>
> This should have a custom DiagnosticType as well to help get better error
> messages for out of range values. We don’t use this facility as much as we
> should in the ARM backend, but it’s worth doing. Imm0_15AsmOperand has an
> example of how this works.
>
>
> -def HINT : AI<(outs), (ins imm0_255:$imm), MiscFrm, NoItinerary,
> +def HINT : AI<(outs), (ins imm0_4:$imm), MiscFrm, NoItinerary,
>                "hint", "\t$imm", []>, Requires<[IsARM, HasV6]> {
> -  bits<8> imm;
> -  let Inst{27-8} = 0b00110010000011110000;
> -  let Inst{7-0} = imm;
> +  bits<3> imm;
> +  let Inst{27-3} = 0b0011001000001111000000000;
> +  let Inst{2-0} = imm;
>  }
>
> This is only for the ARM mode of the instruction. Don’t restrictions like
> this also apply to the Thumb2 encodings?
>
>  @ CHECK: yield                          @ encoding: [0x01,0xf0,0x20,0xe3]
>  @ CHECK: yieldne                        @ encoding: [0x01,0xf0,0x20,0x13]
> -@ CHECK: hint #5                      @ encoding: [0x05,0xf0,0x20,0xe3]
> +@ CHECK-NOT: hint #5
>  @ CHECK: sev
>
> A better way to handle this is that “hint #5” should be removed entirely
> from this test file since it’s not a valid encoding. It should be replaced
> with test cases in diagnostics.s and thumb-diagnostics.s which check that
> the expected diagnostic is produced. As is, for example, we get "error:
> instruction requires: thumb2” which isn’t correct, since this instruction
> shouldn’t be valid for thumb2 either. We should explicitly check for the
> new diagnostic we add via DiagnosticType.
>
> Jim
>
>
> On Apr 17, 2013, at 3:10 AM, Mihail Popa <mihail.popa at gmail.com> wrote:
>
> Hi.
>
> I have amended the patch by removing white space trimming and replacing
> grep with FileCheck.
>
> Regards,
> Mihai
>
>
> On Tue, Apr 16, 2013 at 7:47 PM, Quentin Colombet <qcolombet at apple.com>
> wrote:
>
>> Oh yes, that’s true. I’ve forgotten about that!
>>
>> -Quentin
>>
>> On Apr 16, 2013, at 11:36 AM, Tim Northover <t.p.northover at gmail.com>
>> wrote:
>>
>> Maybe I’m missing something but you’ve defined a new isImm0_4 method but
>> you’re never using it.
>>
>>
>> That's automatically used by TableGen based on the Name member in the
>> AsmOperandClass, isn't it?
>>
>> Tim.
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
> <LLVM-713.hints.patch>_______________________________________________
>
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130418/fd99037a/attachment.html>