Re: [PATCH] gas/arm: Misses deprecated IT instruction warning for ARMv8

[Richard Earnshaw <rearnsha at arm dot com>]

On 17/06/14 09:34, Jiong Wang wrote:
> 
> On 16/06/14 16:31, Jiong Wang wrote:
>> On 16/06/14 15:41, Richard Earnshaw wrote:
>>> On 13/06/14 16:55, Nicholas Clifton wrote:
>>>> Hi Jiong,
>>>>
>>>>> given code:
>>>>>
>>>>> .thumb
>>>>> .syntax unified
>>>>>
>>>>> it eq
>>>>> addeq sp, sp, #12
>>>>>
>>>>> assembled with -march=armv8-a -mthumb, gas fails to warn
>>>>> that this is a deprecated form.
>>>> Where is this deprecation specified ?   Please can you give a reference
>>>> so that I can check it ?
>>>>
>>>> Cheers
>>>>      Nick
>>>>
>>>>
>>> It's in section F6.2 of the v8 ARM ARM:
>>>
>>> Registered users can get a copy from here:
>>>
>>> http://arminfo.emea.arm.com/help/topic/com.arm.doc.ddi0487a.b/DDI0487A_b_armv8_arm.pdf
>>>
>>> But this doesn't look right, since the instructions you need to match
>>> don't follow this pattern:
>>>
>>> +  { 0x00dd, 0x7fff, N_("ADD/SUB sp, sp #imm") },
>>>
>>> Specifically SUB SP is 0b101100001xxxxxxx
>>> and ADD SP is 0b101100000xxxxxxx.
>>>
>>> Iff your mask is right, it needs a comment to explain why.
>> yes, the mask confuse people at the first glance.
>>
>> the reason to put "0x00dd" there is because arm assembler is using the
>> pseudo "tvalue" which is from the table "T16_32_TAB", instead of the
>> original encoding when that checking happen.
>>
>> the real encoding will be converted back at later stage in "md_apply_fix".
>>
>> OK for trunk?
> 
>    Sorry, updated the patch, it's better to put the explain as NOTE 
> comment in the code.
> 

Thanks, this is OK.

R.

>    Please review.
> 
>    Thanks.
> 
>>
>> Thanks
>>
>>> R.
>>>
>>>
>>
>>
>>
>>
> 
> 
> inc-spv2.patch
> 
> 
> diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
> index 590855c..6262929 100644
> --- a/gas/config/tc-arm.c
> +++ b/gas/config/tc-arm.c
> @@ -17265,6 +17265,9 @@ static const struct depr_insn_mask depr_it_insns[] = {
>    { 0x4800, 0xf800, N_("Literal loads") },
>    { 0x4478, 0xf478, N_("Hi-register ADD, MOV, CMP, BX, BLX using pc") },
>    { 0x4487, 0xfc87, N_("Hi-register ADD, MOV, CMP using pc") },
> +  /* NOTE: 0x00dd is not the real encoding, instead, it is the 'tvalue'
> +     field in asm_opcode. 'tvalue' is used at the stage this check happen.  */
> +  { 0x00dd, 0x7fff, N_("ADD/SUB sp, sp #imm") },
>    { 0, 0, NULL }
>  };
>  
> diff --git a/gas/testsuite/gas/arm/armv8-a-it-bad.l b/gas/testsuite/gas/arm/armv8-a-it-bad.l
> index e3a5971..282216e 100644
> --- a/gas/testsuite/gas/arm/armv8-a-it-bad.l
> +++ b/gas/testsuite/gas/arm/armv8-a-it-bad.l
> @@ -12,3 +12,7 @@
>  .*:55: Error: r15 not allowed here -- `addeq r0,pc,pc'
>  .*:58: Warning: IT blocks containing 16-bit Thumb instructions of the following class are deprecated in ARMv8: Short branches, Undefined, SVC, LDM/STM
>  .*:58: Error: r15 not allowed here -- `addeq pc,r0,r0'
> +.*:61: Warning: IT blocks containing 16-bit Thumb instructions of the following class are deprecated in ARMv8: ADD/SUB sp, sp #imm
> +.*:65: Warning: IT blocks containing 16-bit Thumb instructions of the following class are deprecated in ARMv8: ADD/SUB sp, sp #imm
> +.*:68: Warning: IT blocks containing 16-bit Thumb instructions of the following class are deprecated in ARMv8: ADD/SUB sp, sp #imm
> +.*:72: Warning: IT blocks containing 16-bit Thumb instructions of the following class are deprecated in ARMv8: ADD/SUB sp, sp #imm
> diff --git a/gas/testsuite/gas/arm/armv8-a-it-bad.s b/gas/testsuite/gas/arm/armv8-a-it-bad.s
> index 42f2b86..d1bc07a 100644
> --- a/gas/testsuite/gas/arm/armv8-a-it-bad.s
> +++ b/gas/testsuite/gas/arm/armv8-a-it-bad.s
> @@ -56,3 +56,17 @@ addeq r0, pc, pc
>  
>  it eq
>  addeq pc, r0, r0
> +
> +it eq
> +addeq sp, sp, #12
> +
> +@ Misaligned immediate.
> +it eq
> +addeq sp, sp, #3
> +
> +it eq
> +subeq sp, sp, #12
> +
> +@ Misaligned immediate.
> +it eq
> +subeq sp, sp, #3
>