[PATCH] Fix treatment of ARM unallocated hint instructions

Mihail Popa mihail.popa at gmail.com
Thu Apr 25 07:21:52 PDT 2013


I have submitted the patch on a different thread.

Mihai


On Wed, Apr 24, 2013 at 10:07 AM, Mihail Popa <mihail.popa at gmail.com> wrote:

> Sure - sorry for delaying - it'll be in by tomorrow.
>
>
> On Tue, Apr 23, 2013 at 8:34 PM, Jim Grosbach <grosbach at apple.com> wrote:
>
>> Ping. Please fix this sooner rather than later. As-is, the test case is
>> fundamentally wrong and the diagnostic is bogus. The end result is that the
>> behavior after this patch is overall worse than before.
>>
>> -Jim
>>
>> On Apr 18, 2013, at 5:19 AM, Mihail Popa <mihail.popa at gmail.com> wrote:
>>
>> 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/20130425/f05704f0/attachment.html>


More information about the llvm-commits mailing list