Re: [PATCH, ARM 2/7] Add support for ARMv8-M Mainline without security extensions

["Richard Earnshaw (lists)" <Richard dot Earnshaw at arm dot com>]

On 23/12/15 06:47, Thomas Preud'homme wrote:
> Hi Richard,
> 
>> From: binutils-owner@sourceware.org [mailto:binutils-
>> owner@sourceware.org] On Behalf Of Richard Earnshaw (lists)
>> Sent: Friday, December 18, 2015 9:26 PM
>>
>> On 17/12/15 02:25, Thomas Preud'homme wrote:
>>
>> This is mostly OK, but:
>>
>>> @@ -25545,6 +25570,12 @@ aeabi_set_public_attributes (void)
>>>        && ARM_CPU_HAS_FEATURE (flags, arm_ext_v6_dsp))
>>>      arch = 13;
>>>
>>> +  /* In cpu_arch_ver ARMv8-A is before ARMv8-M for atomics to be
>> detected as
>>> +     coming from ARMv8-A.  However, since ARMv8-A has more
>> instructions than
>>> +     ARMv8-M, -march=all must be detected as ARMv8-A.  */
>>> +  if (arch == 17 && ARM_FEATURE_CORE_EQUAL (selected_cpu,
>> arm_arch_any))
>>> +    arch = 14;
>>> +
>>
>> You should use TAG_CPU_ARCH_... here rather than manifest constants.
>> And can you also fix up the other cases above as well, please.
>>
>>
>>> index
>> c64e7886e9266b4084e11a04b2e60636dda62bd5..d0ec5d2a1ccff17ba9e09
>> d564d408ff6be6a33e4 100644
>>> --- a/include/elf/arm.h
>>> +++ b/include/elf/arm.h
>>> @@ -105,10 +105,11 @@
>>>  #define TAG_CPU_ARCH_V6S_M	12
>>>  #define TAG_CPU_ARCH_V7E_M	13
>>>  #define TAG_CPU_ARCH_V8		14
>>> -#define MAX_TAG_CPU_ARCH	14
>>> +#define TAG_CPU_ARCH_V8M_MAIN	17
>>> +#define MAX_TAG_CPU_ARCH	TAG_CPU_ARCH_V8M_MAIN
>>>  /* Pseudo-architecture to allow objects to be compatible with the
>> subset of
>>>     armv4t and armv6-m.  This value should never be stored in object
>> files.  */
>>> -#define TAG_CPU_ARCH_V4T_PLUS_V6_M (MAX_TAG_CPU_ARCH +
>> 1)
>>> +#define TAG_CPU_ARCH_V4T_PLUS_V6_M 15
>>
>> No, I think this should remain as MAX_TAG...+1.  You shouldn't assume
>> that gaps in the numbering will remain and making the definition special
>> highlights that this is an internal artefact.
> 
> I wanted to avoid holes in the numbers but it was indeed a bad idea since tag_cpu_arch_combine depends on it being the highest number.
> 
>>
>>> diff --git a/include/opcode/arm.h b/include/opcode/arm.h
>>> index
>> abbe6a82206450aa525b36f3679379d538fe6e1e..91df5d5a7075c62fb99281
>> bf79158c7b91c016b3 100644
>>> --- a/include/opcode/arm.h
>>> +++ b/include/opcode/arm.h
>>> @@ -34,7 +34,7 @@
>>>  #define ARM_EXT_V6       0x00001000     /* ARM V6.                 */
>>>  #define ARM_EXT_V6K      0x00002000     /* ARM V6K.                */
>>>  /*			 0x00004000	   Was ARM V6Z.            */
>>> -#define ARM_EXT_V8	 0x00004000     /* is now ARMv8.           */
>>> +#define ARM_EXT_V8	 0x00004000     /* is now ARMv8 w/o
>> atomics.  */
>>
>> I think the "Was ARM V6Z" comment was intended only to highlight that
>> there was a spare bit here.  Better to remove that and then drop the 'is
>> now'.
>>
>> OK with those changes.
> 
> Please find updated ChangeLog entries:
> 
> *** bfd/ChangeLog ***
> 
> 2015-12-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         (tag_cpu_arch_combine): Adjust v4t_plus_v6_m and comb array to account
>         for new TAG_CPU_ARCH_V4T_PLUS_V6_M value.  Deal with NULL values in
>         comb array.
> 
> 
> *** binutils/ChangeLog ***
> 
> 2015-12-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * readelf.c (arm_attr_tag_CPU_arch): Add ARMv8-M Mainline Tag_CPU_arch
>         value.
>         (arm_attr_tag_THUMB_ISA_use): Add ARMv8-M Mainline Tag_THUMB_ISA_use
>         value.
> 
> 
> *** gas/ChangeLog ***
> 
> 2015-12-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/tc-arm.c (arm_ext_m): Include ARMv8-M.
>         (arm_ext_v8m): New feature for ARMv8-M.
>         (arm_ext_atomics): New feature for ARMv8 atomics.
>         (do_tt): New encoding function for TT* instructions.
>         (insns): Add new entries for ARMv8-M specific instructions and
>         reorganize the ones shared by ARMv8-M Mainline and ARMv8-A.
>         (arm_archs): Define armv8-m.main architecture.
>         (cpu_arch_ver): Define ARM_ARCH_V8M_MAIN architecture version and
>         clarify the ordering rule.
>         (aeabi_set_public_attributes): Use TAG_CPU_ARCH_* macro to refer to
>         Tag_CPU_arch values for ARMv7e-M detection.  Add logic to keep setting
>         Tag_CPU_arch to ARMv8-A for -march=all.  Also set Tag_CPU_arch_profile
>         to 'A' if extension bit for atomic instructions is set, unless it is
>         ARMv8-M.  Set Tag_THUMB_ISA_use to 3 for ARMv8-M.  Set Tag_DIV_use to 0
>         for ARMv8-M Mainline.
> 
> 
> *** gas/testsuite/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * gas/arm/archv8m.s: New file. 
>         * gas/arm/archv8m-main.d: Likewise.
>         * gas/arm/attr-march-armv8m.main.d: Likewise.
>         * gas/arm/any-armv8m.s: Likewise.
>         * gas/arm/any-armv8m.d: Likewise.
> 
> 
> *** include/elf/ChangeLog ***
> 
> 2015-12-11  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm.h (TAG_CPU_ARCH_V8M_MAIN): Declare.
>         (MAX_TAG_CPU_ARCH): Define to TAG_CPU_ARCH_V8M_MAIN.
>         (TAG_CPU_ARCH_V4T_PLUS_V6_M): Define to unused value 15.
> 
> 
> *** include/opcode/ChangeLog ***
> 
> 2015-12-22  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm.h (ARM_EXT2_ATOMICS): New extension bit.
>         (ARM_EXT2_V8M): Likewise.
>         (ARM_EXT_V8): Adjust comment with regards to atomics and remove
>         mention of legacy use for that bit.
>         (ARM_AEXT2_V8_1A): New architecture extension bitfield.
>         (ARM_AEXT2_V8_2A): Likewise.
>         (ARM_AEXT_V8M_MAIN): Likewise.
>         (ARM_AEXT2_V8M): Likewise.
>         (ARM_ARCH_V8A): Use ARM_EXT2_ATOMICS for features in second bitfield.
>         (ARM_ARCH_V8_1A): Likewise with ARM_AEXT2_V8_1A.
>         (ARM_ARCH_V8_2A): Likewise with ARM_AEXT2_V8_2A.
>         (ARM_ARCH_V8M_MAIN): New architecture feature bitfield.
>         (ARM_ARCH_V8A_FP): Use ARM_EXT2_ATOMICS for features in second bitfield
>         and reindent.
>         (ARM_ARCH_V8A_SIMD): Likewise.
>         (ARM_ARCH_V8A_CRYPTOV1): Likewise.
>         (ARM_ARCH_V8_1A_FP): Use ARM_AEXT2_V8_1A to set second bitfield of
>         feature bits.
>         (ARM_ARCH_V8_1A_SIMD): Likewise.
>         (ARM_ARCH_V8_1A_CRYPTOV1): Likewise.
> 
> 
> *** opcodes/ChangeLog ***
> 
> 2015-12-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * arm-dis.c (arm_opcodes): Guard lda, ldab, ldaex, ldaexb, ldaexh, stl,
>         stlb, stlh, stlex, stlexb and stlexh by ARM_EXT2_ATOMICS instead of
>         ARM_EXT_V8.
>         (thumb32_opcodes): Add entries for wide ARMv8-M instructions.
> 

OK.

R.