This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On 17/12/15 02:25, Thomas Preud'homme wrote:
> Hi,
> 
> This patch is part of a patch series to add support for ARMv8-M[1] to binutils. This specific patch adds support for ARMv8-M Mainline without security extensions instructions: it allows armv8-m.main to be recognized as well as all instructions added in ARMv8-M Mainline over ARMv7-M, except for security extensions ones.
> 
> [1] For a quick overview of ARMv8-M please refer to the initial cover letter.
> 
> 
> ChangeLog entries are as follow:
> 
> 
> *** 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-16  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): 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-16  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.
>         (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.
> 
> 

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..d0ec5d2a1ccff17ba9e09d564d408ff6be6a33e4 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.

> diff --git a/include/opcode/arm.h b/include/opcode/arm.h
> index abbe6a82206450aa525b36f3679379d538fe6e1e..91df5d5a7075c62fb99281bf79158c7b91c016b3 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.

R.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]