public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* RE: [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
@ 2016-01-12  9:25 Thomas Preud'homme
  2016-05-17  9:58 ` [PATCH, libgcc/ARM 1/6, ping1] " Thomas Preudhomme
  2016-06-01  9:01 ` [PATCH, libgcc/ARM 1/6] " Ramana Radhakrishnan
  0 siblings, 2 replies; 15+ messages in thread
From: Thomas Preud'homme @ 2016-01-12  9:25 UTC (permalink / raw)
  To: gcc-patches

> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> Sent: Thursday, December 17, 2015 1:58 PM
> 
> Hi,
> 
> We decided to apply the following patch to the ARM embedded 5 branch.
> This is *not* intended for trunk for now. We will send a separate email
> for trunk.

And now a rebased patch on top of trunk.


> 
> This patch is part of a patch series to add support for ARMv8-M[1] to GCC.
> This specific patch fixes some assumptions related to M profile
> architectures. Currently GCC (mostly libgcc) contains several assumptions
> that the only ARM architecture with Thumb-1 only instructions is ARMv6-
> M and the only one with Thumb-2 only instructions is ARMv7-M. ARMv8-
> M [1] make this wrong since ARMv8-M baseline is also (mostly) Thumb-1
> only and ARMv8-M mainline is also Thumb-2 only. This patch replace
> checks for __ARM_ARCH_*__ for checks against
> __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM instead. For
> instance, Thumb-1 only can be checked with
> #if !defined(__ARM_ARCH_ISA_ARM) && (__ARM_ARCH_ISA_THUMB
> == 1). It also fixes the guard for DIV code to not apply to ARMv8-M
> Baseline since it uses Thumb-2 instructions.
> 
> [1] For a quick overview of ARMv8-M please refer to the initial cover
> letter.
> 
> ChangeLog entries are as follow:
> 
> 

*** gcc/ChangeLog ***

2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
        decide whether to prevent some libgcc routines being included for some
        multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
        link between this condition and the one in
        libgcc/config/arm/lib1func.S.
        * config/arm/arm.h (TARGET_ARM_V6M): Add check to TARGET_ARM_ARCH.
        (TARGET_ARM_V7M): Likewise.


*** gcc/testsuite/ChangeLog ***

2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
        __ARM_ARCH_ISA_ARM to test for Cortex-M devices.


*** libgcc/ChangeLog ***

2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1 rather
        than ARMv6-M.
        * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
        for all Thumb-1 only targets.
        (__only_thumb1__): Define for all Thumb-1 only targets.
        (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
        (EQUIV): Likewise.
        (ARM_FUNC_ALIAS): Likewise.
        (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
        (modsi3): Likewise.
        (HAVE_ARM_CLZ): Remove block defining it.
        (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
        check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
        (clzdi2): Likewise.
        (ctzsi2): Likewise.
        (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
        __ARM_ARCH_6M__ in guard for checking whether it is defined.
        (final includes): Test for __only_thumb1__ rather than
        __ARM_ARCH_6M__ and add comment to indicate the connection between
        this condition and the one in gcc/config/arm/elf.h.
        * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
        __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
        * config/arm/t-softfp: Likewise.


diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index fd999dd..0d23f39 100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2182,8 +2182,10 @@ extern int making_const_table;
 #define TARGET_ARM_ARCH	\
   (arm_base_arch)	\
 
-#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
-#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
+#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm \
+			&& !arm_arch_thumb2)
+#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm \
+			&& arm_arch_thumb2)
 
 /* The highest Thumb instruction set version supported by the chip.  */
 #define TARGET_ARM_ARCH_ISA_THUMB 		\
diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
index 3795728..579a580 100644
--- a/gcc/config/arm/elf.h
+++ b/gcc/config/arm/elf.h
@@ -148,8 +148,9 @@
   while (0)
 
 /* Horrible hack: We want to prevent some libgcc routines being included
-   for some multilibs.  */
-#ifndef __ARM_ARCH_6M__
+   for some multilibs.  The condition should match the one in
+   libgcc/config/arm/lib1funcs.S.  */
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 #undef L_fixdfsi
 #undef L_fixunsdfsi
 #undef L_truncdfsf2
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 4e349e9..3f96826 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3221,10 +3221,8 @@ proc check_effective_target_arm_cortex_m { } {
 	return 0
     }
     return [check_no_compiler_messages arm_cortex_m assembly {
-	#if !defined(__ARM_ARCH_7M__) \
-            && !defined (__ARM_ARCH_7EM__) \
-            && !defined (__ARM_ARCH_6M__)
-	#error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
+	#if defined(__ARM_ARCH_ISA_ARM)
+	#error __ARM_ARCH_ISA_ARM is defined
 	#endif
 	int i;
     } "-mthumb"]
diff --git a/libgcc/config/arm/bpabi-v6m.S b/libgcc/config/arm/bpabi-v6m.S
index a1e1640..9ae0bb8 100644
--- a/libgcc/config/arm/bpabi-v6m.S
+++ b/libgcc/config/arm/bpabi-v6m.S
@@ -1,4 +1,4 @@
-/* Miscellaneous BPABI functions.  ARMv6M implementation
+/* Miscellaneous BPABI functions.  Thumb-1 only implementation
 
    Copyright (C) 2006-2015 Free Software Foundation, Inc.
    Contributed by CodeSourcery.
diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index 252efcb..95a1f80 100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
      && !defined(__thumb2__)		\
      && (!defined(__THUMB_INTERWORK__)	\
 	 || defined (__OPTIMIZE_SIZE__)	\
-	 || defined(__ARM_ARCH_6M__)))
+	 || !__ARM_ARCH_ISA_ARM))
 # define __prefer_thumb__
 #endif
 
@@ -305,7 +305,7 @@ LSYM(Lend_fde):
 
 #ifdef __ARM_EABI__
 .macro THUMB_LDIV0 name signed
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 	.ifc \signed, unsigned
 	cmp	r0, #0
 	beq	1f
@@ -478,7 +478,7 @@ _L__\name:
 
 #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
 
-#ifdef __ARM_ARCH_6M__
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 #define EQUIV .thumb_set
 #else
 .macro	ARM_FUNC_START name sp_section=
@@ -510,7 +510,7 @@ SYM (__\name):
 #endif
 .endm
 
-#ifndef __ARM_ARCH_6M__
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 .macro	ARM_FUNC_ALIAS new old
 	.globl	SYM (__\new)
 	EQUIV	SYM (__\new), SYM (__\old)
@@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod
 /* ------------------------------------------------------------------------ */
 #ifdef L_umodsi3
 
-#ifdef __ARM_ARCH_EXT_IDIV__
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START umodsi3
 
@@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod
 /* ------------------------------------------------------------------------ */
 #ifdef L_modsi3
 
-#if defined(__ARM_ARCH_EXT_IDIV__)
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START modsi3
 
@@ -1508,14 +1508,8 @@ LSYM(Lover12):
 
 #endif /* __symbian__ */
 
-#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
-    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
-    || defined(__ARM_ARCH_5TEJ__)
-#define HAVE_ARM_CLZ 1
-#endif
-
 #ifdef L_clzsi2
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START clzsi2
 	mov	r1, #28
 	mov	r3, #1
@@ -1544,7 +1538,7 @@ FUNC_START clzsi2
 	FUNC_END clzsi2
 #else
 ARM_FUNC_START clzsi2
-# if defined(HAVE_ARM_CLZ)
+# if defined(__ARM_FEATURE_CLZ)
 	clz	r0, r0
 	RET
 # else
@@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
 .align 2
 1:
 .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
-# endif /* !HAVE_ARM_CLZ */
+# endif /* !__ARM_FEATURE_CLZ */
 	FUNC_END clzsi2
 #endif
 #endif /* L_clzsi2 */
 
 #ifdef L_clzdi2
-#if !defined(HAVE_ARM_CLZ)
+#if !defined(__ARM_FEATURE_CLZ)
 
-# if defined(__ARM_ARCH_6M__)
+# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START clzdi2
 	push	{r4, lr}
 # else
@@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2
 	bl	__clzsi2
 # endif
 2:
-# if defined(__ARM_ARCH_6M__)
+# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 	pop	{r4, pc}
 # else
 	RETLDM	r4
 # endif
 	FUNC_END clzdi2
 
-#else /* HAVE_ARM_CLZ */
+#else /* __ARM_FEATURE_CLZ */
 
 ARM_FUNC_START clzdi2
 	cmp	xxh, #0
@@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2
 #endif /* L_clzdi2 */
 
 #ifdef L_ctzsi2
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START ctzsi2
 	neg	r1, r0
 	and	r0, r0, r1
@@ -1656,7 +1650,7 @@ FUNC_START ctzsi2
 ARM_FUNC_START ctzsi2
 	rsb	r1, r0, #0
 	and	r0, r0, r1
-# if defined(HAVE_ARM_CLZ)
+# if defined(__ARM_FEATURE_CLZ)
 	clz	r0, r0
 	rsb	r0, r0, #31
 	RET
@@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2
 .align 2
 1:
 .byte	27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31
-# endif /* !HAVE_ARM_CLZ */
+# endif /* !__ARM_FEATURE_CLZ */
 	FUNC_END ctzsi2
 #endif
 #endif /* L_clzsi2 */
@@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2
 
 /* Don't bother with the old interworking routines for Thumb-2.  */
 /* ??? Maybe only omit these on "m" variants.  */
-#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
+#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
 
 #if defined L_interwork_call_via_rX
 
@@ -1983,11 +1977,12 @@ LSYM(Lchange_\register):
 .endm
 
 #ifndef __symbian__
-#ifndef __ARM_ARCH_6M__
+/* The condition here must match the one in gcc/config/arm/elf.h.  */
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 #include "ieee754-df.S"
 #include "ieee754-sf.S"
 #include "bpabi.S"
-#else /* __ARM_ARCH_6M__ */
+#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
 #include "bpabi-v6m.S"
-#endif /* __ARM_ARCH_6M__ */
+#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
 #endif /* !__symbian__ */
diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
index cac1022..393ec8a 100644
--- a/libgcc/config/arm/libunwind.S
+++ b/libgcc/config/arm/libunwind.S
@@ -58,7 +58,7 @@
 #endif
 #endif
 
-#ifdef __ARM_ARCH_6M__
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#else /* !__ARM_ARCH_6M__ */
+#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#endif /* !__ARM_ARCH_6M__ */
+#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 UNWIND_WRAPPER _Unwind_RaiseException 1
 UNWIND_WRAPPER _Unwind_Resume 1
diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
index 4ede438..554ec9b 100644
--- a/libgcc/config/arm/t-softfp
+++ b/libgcc/config/arm/t-softfp
@@ -1,2 +1,2 @@
-softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
+softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1'
 softfp_wrap_end := '\#endif'


Tested by building libgcc and comparing it before and
after with objdump -D and readelf -A for all permutations of:

  Architectures:
    armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
    armv6z armv6kz armv6t2 armv6s-m armv7 armv7-a armv7ve
    armv7-r armv7-m armv7e-m armv8-a armv8-a+crc iwmmxt
    iwmmxt2

  ISAs:
    thumb arm

  Optimization Levels:
    Os O2

  Excluding:
    armv6s-m -marm
    armv7-m -marm
    armv7e-m -marm
    iwmmxt -mthumb
    iwmmxt2 -mthumb

as being rejected by the compiler or assembler. ARMv6-m was not tested because compilation fails due to svc instruction.

Best regards,

Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-01-12  9:25 [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Thomas Preud'homme
@ 2016-05-17  9:58 ` Thomas Preudhomme
  2016-05-19 16:42   ` Kyrill Tkachov
  2016-06-01  9:01 ` [PATCH, libgcc/ARM 1/6] " Ramana Radhakrishnan
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2016-05-17  9:58 UTC (permalink / raw)
  To: kyrylo.tkachov, ramana.radhakrishnan, richard.earnshaw, gcc-patches

Ping?

*** gcc/ChangeLog ***

2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
        decide whether to prevent some libgcc routines being included for some
        multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
        link between this condition and the one in
        libgcc/config/arm/lib1func.S.
        * config/arm/arm.h (TARGET_ARM_V6M): Add check to TARGET_ARM_ARCH.
        (TARGET_ARM_V7M): Likewise.


*** gcc/testsuite/ChangeLog ***

2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
        __ARM_ARCH_ISA_ARM to test for Cortex-M devices.


*** libgcc/ChangeLog ***

2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1 rather
        than ARMv6-M.
        * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
        for all Thumb-1 only targets.
        (__only_thumb1__): Define for all Thumb-1 only targets.
        (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
        (EQUIV): Likewise.
        (ARM_FUNC_ALIAS): Likewise.
        (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
        (modsi3): Likewise.
        (HAVE_ARM_CLZ): Remove block defining it.
        (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
        check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
        (clzdi2): Likewise.
        (ctzsi2): Likewise.
        (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
        __ARM_ARCH_6M__ in guard for checking whether it is defined.
        (final includes): Test for __only_thumb1__ rather than
        __ARM_ARCH_6M__ and add comment to indicate the connection between
        this condition and the one in gcc/config/arm/elf.h.
        * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
        __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
        * config/arm/t-softfp: Likewise.


diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
index 
5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9fe66611c19 
100644
--- a/gcc/config/arm/arm.h
+++ b/gcc/config/arm/arm.h
@@ -2188,8 +2188,10 @@ extern int making_const_table;
 #define TARGET_ARM_ARCH	\
   (arm_base_arch)	\
 
-#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
-#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
+#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm \
+			&& !arm_arch_thumb2)
+#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm \
+			&& arm_arch_thumb2)
 
 /* The highest Thumb instruction set version supported by the chip.  */
 #define TARGET_ARM_ARCH_ISA_THUMB 		\
diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
index 
77f30554d5286bd83aeab0c8dc308cfd44e732dc..246de5492665ba2a0292736a9c53fbaaef184d72 
100644
--- a/gcc/config/arm/elf.h
+++ b/gcc/config/arm/elf.h
@@ -148,8 +148,9 @@
   while (0)
 
 /* Horrible hack: We want to prevent some libgcc routines being included
-   for some multilibs.  */
-#ifndef __ARM_ARCH_6M__
+   for some multilibs.  The condition should match the one in
+   libgcc/config/arm/lib1funcs.S.  */
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 #undef L_fixdfsi
 #undef L_fixunsdfsi
 #undef L_truncdfsf2
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-
supports.exp
index 
04ca17656f2f26dda710e8a0f9ca77dd963ab39b..38151375c29cd007f1cc34ead3aa495606224061 
100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3320,10 +3320,8 @@ proc check_effective_target_arm_cortex_m { } {
 	return 0
     }
     return [check_no_compiler_messages arm_cortex_m assembly {
-	#if !defined(__ARM_ARCH_7M__) \
-            && !defined (__ARM_ARCH_7EM__) \
-            && !defined (__ARM_ARCH_6M__)
-	#error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
+	#if defined(__ARM_ARCH_ISA_ARM)
+	#error __ARM_ARCH_ISA_ARM is defined
 	#endif
 	int i;
     } "-mthumb"]
diff --git a/libgcc/config/arm/bpabi-v6m.S b/libgcc/config/arm/bpabi-v6m.S
index 
5d35aa6afca224613c94cf923f8a2ee8dac949f2..b1d375dfb88efb899ce6213013205b85e531a884 
100644
--- a/libgcc/config/arm/bpabi-v6m.S
+++ b/libgcc/config/arm/bpabi-v6m.S
@@ -1,4 +1,4 @@
-/* Miscellaneous BPABI functions.  ARMv6M implementation
+/* Miscellaneous BPABI functions.  Thumb-1 only implementation
 
    Copyright (C) 2006-2016 Free Software Foundation, Inc.
    Contributed by CodeSourcery.
diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index 
375a5135110895faa44267ebee045fd315515027..7268deb37f2c4ce4e43eeac06c418235071ebcd2 
100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  
If not, see
      && !defined(__thumb2__)		\
      && (!defined(__THUMB_INTERWORK__)	\
 	 || defined (__OPTIMIZE_SIZE__)	\
-	 || defined(__ARM_ARCH_6M__)))
+	 || !__ARM_ARCH_ISA_ARM))
 # define __prefer_thumb__
 #endif
 
@@ -305,7 +305,7 @@ LSYM(Lend_fde):
 
 #ifdef __ARM_EABI__
 .macro THUMB_LDIV0 name signed
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 	.ifc \signed, unsigned
 	cmp	r0, #0
 	beq	1f
@@ -478,7 +478,7 @@ _L__\name:
 
 #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
 
-#ifdef __ARM_ARCH_6M__
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 #define EQUIV .thumb_set
 #else
 .macro	ARM_FUNC_START name sp_section=
@@ -510,7 +510,7 @@ SYM (__\name):
 #endif
 .endm
 
-#ifndef __ARM_ARCH_6M__
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 .macro	ARM_FUNC_ALIAS new old
 	.globl	SYM (__\new)
 	EQUIV	SYM (__\new), SYM (__\old)
@@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod
 /* ------------------------------------------------------------------------ 
*/
 #ifdef L_umodsi3
 
-#ifdef __ARM_ARCH_EXT_IDIV__
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START umodsi3
 
@@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod
 /* ------------------------------------------------------------------------ 
*/
 #ifdef L_modsi3
 
-#if defined(__ARM_ARCH_EXT_IDIV__)
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START modsi3
 
@@ -1508,14 +1508,8 @@ LSYM(Lover12):
 
 #endif /* __symbian__ */
 
-#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
-    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
-    || defined(__ARM_ARCH_5TEJ__)
-#define HAVE_ARM_CLZ 1
-#endif
-
 #ifdef L_clzsi2
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START clzsi2
 	mov	r1, #28
 	mov	r3, #1
@@ -1544,7 +1538,7 @@ FUNC_START clzsi2
 	FUNC_END clzsi2
 #else
 ARM_FUNC_START clzsi2
-# if defined(HAVE_ARM_CLZ)
+# if defined(__ARM_FEATURE_CLZ)
 	clz	r0, r0
 	RET
 # else
@@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
 .align 2
 1:
 .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
-# endif /* !HAVE_ARM_CLZ */
+# endif /* !__ARM_FEATURE_CLZ */
 	FUNC_END clzsi2
 #endif
 #endif /* L_clzsi2 */
 
 #ifdef L_clzdi2
-#if !defined(HAVE_ARM_CLZ)
+#if !defined(__ARM_FEATURE_CLZ)
 
-# if defined(__ARM_ARCH_6M__)
+# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START clzdi2
 	push	{r4, lr}
 # else
@@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2
 	bl	__clzsi2
 # endif
 2:
-# if defined(__ARM_ARCH_6M__)
+# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 	pop	{r4, pc}
 # else
 	RETLDM	r4
 # endif
 	FUNC_END clzdi2
 
-#else /* HAVE_ARM_CLZ */
+#else /* __ARM_FEATURE_CLZ */
 
 ARM_FUNC_START clzdi2
 	cmp	xxh, #0
@@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2
 #endif /* L_clzdi2 */
 
 #ifdef L_ctzsi2
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START ctzsi2
 	neg	r1, r0
 	and	r0, r0, r1
@@ -1656,7 +1650,7 @@ FUNC_START ctzsi2
 ARM_FUNC_START ctzsi2
 	rsb	r1, r0, #0
 	and	r0, r0, r1
-# if defined(HAVE_ARM_CLZ)
+# if defined(__ARM_FEATURE_CLZ)
 	clz	r0, r0
 	rsb	r0, r0, #31
 	RET
@@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2
 .align 2
 1:
 .byte	27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31
-# endif /* !HAVE_ARM_CLZ */
+# endif /* !__ARM_FEATURE_CLZ */
 	FUNC_END ctzsi2
 #endif
 #endif /* L_clzsi2 */
@@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2
 
 /* Don't bother with the old interworking routines for Thumb-2.  */
 /* ??? Maybe only omit these on "m" variants.  */
-#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
+#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
 
 #if defined L_interwork_call_via_rX
 
@@ -1983,11 +1977,12 @@ LSYM(Lchange_\register):
 .endm
 
 #ifndef __symbian__
-#ifndef __ARM_ARCH_6M__
+/* The condition here must match the one in gcc/config/arm/elf.h.  */
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 #include "ieee754-df.S"
 #include "ieee754-sf.S"
 #include "bpabi.S"
-#else /* __ARM_ARCH_6M__ */
+#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
 #include "bpabi-v6m.S"
-#endif /* __ARM_ARCH_6M__ */
+#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
 #endif /* !__symbian__ */
diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
index 
a68b10ddce93d230c504f3747dcad3832d9f753c..3d7e70181fa80fe53a4903c96bb0f90480feee21 
100644
--- a/libgcc/config/arm/libunwind.S
+++ b/libgcc/config/arm/libunwind.S
@@ -58,7 +58,7 @@
 #endif
 #endif
 
-#ifdef __ARM_ARCH_6M__
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#else /* !__ARM_ARCH_6M__ */
+#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#endif /* !__ARM_ARCH_6M__ */
+#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 UNWIND_WRAPPER _Unwind_RaiseException 1
 UNWIND_WRAPPER _Unwind_Resume 1
diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
index 
4ede438baf6a297737e52db00395f6c3a359f681..554ec9bc47b04445e79e84b1f957bf88680c08d1 
100644
--- a/libgcc/config/arm/t-softfp
+++ b/libgcc/config/arm/t-softfp
@@ -1,2 +1,2 @@
-softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
+softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1'
 softfp_wrap_end := '\#endif'


Tested by building libgcc and comparing it before and
after with objdump -D and readelf -A for all permutations of:

  Architectures:
    armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
    armv6z armv6kz armv6t2 armv6s-m armv7 armv7-a armv7ve
    armv7-r armv7-m armv7e-m armv8-a armv8-a+crc iwmmxt
    iwmmxt2

  ISAs:
    thumb arm

  Optimization Levels:
    Os O2

  Excluding:
    armv6s-m -marm
    armv7-m -marm
    armv7e-m -marm
    iwmmxt -mthumb
    iwmmxt2 -mthumb

as being rejected by the compiler or assembler. ARMv6-m was not tested because 
compilation fails due to svc instruction.


Best regards,

Thomas

On Tuesday 12 January 2016 17:25:17 Thomas Preud'homme wrote:
> > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> > owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> > Sent: Thursday, December 17, 2015 1:58 PM
> > 
> > Hi,
> > 
> > We decided to apply the following patch to the ARM embedded 5 branch.
> > This is *not* intended for trunk for now. We will send a separate email
> > for trunk.
> 
> And now a rebased patch on top of trunk.
> 
> > This patch is part of a patch series to add support for ARMv8-M[1] to GCC.
> > This specific patch fixes some assumptions related to M profile
> > architectures. Currently GCC (mostly libgcc) contains several assumptions
> > that the only ARM architecture with Thumb-1 only instructions is ARMv6-
> > M and the only one with Thumb-2 only instructions is ARMv7-M. ARMv8-
> > M [1] make this wrong since ARMv8-M baseline is also (mostly) Thumb-1
> > only and ARMv8-M mainline is also Thumb-2 only. This patch replace
> > checks for __ARM_ARCH_*__ for checks against
> > __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM instead. For
> > instance, Thumb-1 only can be checked with
> > #if !defined(__ARM_ARCH_ISA_ARM) && (__ARM_ARCH_ISA_THUMB
> > == 1). It also fixes the guard for DIV code to not apply to ARMv8-M
> > Baseline since it uses Thumb-2 instructions.
> > 
> > [1] For a quick overview of ARMv8-M please refer to the initial cover
> > letter.
> 
> > ChangeLog entries are as follow:
> *** gcc/ChangeLog ***
> 
> 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM
> to decide whether to prevent some libgcc routines being included for some
> multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the link
> between this condition and the one in
>         libgcc/config/arm/lib1func.S.
>         * config/arm/arm.h (TARGET_ARM_V6M): Add check to TARGET_ARM_ARCH.
>         (TARGET_ARM_V7M): Likewise.
> 
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
> __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
> 
> 
> *** libgcc/ChangeLog ***
> 
> 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1
> rather than ARMv6-M.
>         * config/arm/lib1funcs.S (__prefer_thumb__): Define among other
> cases for all Thumb-1 only targets.
>         (__only_thumb1__): Define for all Thumb-1 only targets.
>         (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
> (EQUIV): Likewise.
>         (ARM_FUNC_ALIAS): Likewise.
>         (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
>         (modsi3): Likewise.
>         (HAVE_ARM_CLZ): Remove block defining it.
>         (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
>         check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
>         (clzdi2): Likewise.
>         (ctzsi2): Likewise.
>         (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
>         __ARM_ARCH_6M__ in guard for checking whether it is defined.
>         (final includes): Test for __only_thumb1__ rather than
>         __ARM_ARCH_6M__ and add comment to indicate the connection between
>         this condition and the one in gcc/config/arm/elf.h.
>         * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>         __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>         * config/arm/t-softfp: Likewise.
> 
> 
> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> index fd999dd..0d23f39 100644
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -2182,8 +2182,10 @@ extern int making_const_table;
>  #define TARGET_ARM_ARCH	\
>    (arm_base_arch)	\
> 
> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm \
> +			&& !arm_arch_thumb2)
> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm \
> +			&& arm_arch_thumb2)
> 
>  /* The highest Thumb instruction set version supported by the chip.  */
>  #define TARGET_ARM_ARCH_ISA_THUMB 		\
> diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
> index 3795728..579a580 100644
> --- a/gcc/config/arm/elf.h
> +++ b/gcc/config/arm/elf.h
> @@ -148,8 +148,9 @@
>    while (0)
> 
>  /* Horrible hack: We want to prevent some libgcc routines being included
> -   for some multilibs.  */
> -#ifndef __ARM_ARCH_6M__
> +   for some multilibs.  The condition should match the one in
> +   libgcc/config/arm/lib1funcs.S.  */
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>  #undef L_fixdfsi
>  #undef L_fixunsdfsi
>  #undef L_truncdfsf2
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp index 4e349e9..3f96826 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -3221,10 +3221,8 @@ proc check_effective_target_arm_cortex_m { } {
>  	return 0
>      }
>      return [check_no_compiler_messages arm_cortex_m assembly {
> -	#if !defined(__ARM_ARCH_7M__) \
> -            && !defined (__ARM_ARCH_7EM__) \
> -            && !defined (__ARM_ARCH_6M__)
> -	#error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
> +	#if defined(__ARM_ARCH_ISA_ARM)
> +	#error __ARM_ARCH_ISA_ARM is defined
>  	#endif
>  	int i;
>      } "-mthumb"]
> diff --git a/libgcc/config/arm/bpabi-v6m.S b/libgcc/config/arm/bpabi-v6m.S
> index a1e1640..9ae0bb8 100644
> --- a/libgcc/config/arm/bpabi-v6m.S
> +++ b/libgcc/config/arm/bpabi-v6m.S
> @@ -1,4 +1,4 @@
> -/* Miscellaneous BPABI functions.  ARMv6M implementation
> +/* Miscellaneous BPABI functions.  Thumb-1 only implementation
> 
>     Copyright (C) 2006-2015 Free Software Foundation, Inc.
>     Contributed by CodeSourcery.
> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
> index 252efcb..95a1f80 100644
> --- a/libgcc/config/arm/lib1funcs.S
> +++ b/libgcc/config/arm/lib1funcs.S
> @@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
>  If not, see && !defined(__thumb2__)		\
>       && (!defined(__THUMB_INTERWORK__)	\
> 
>  	 || defined (__OPTIMIZE_SIZE__)	\
> 
> -	 || defined(__ARM_ARCH_6M__)))
> +	 || !__ARM_ARCH_ISA_ARM))
>  # define __prefer_thumb__
>  #endif
> 
> @@ -305,7 +305,7 @@ LSYM(Lend_fde):
> 
>  #ifdef __ARM_EABI__
>  .macro THUMB_LDIV0 name signed
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  	.ifc \signed, unsigned
>  	cmp	r0, #0
>  	beq	1f
> @@ -478,7 +478,7 @@ _L__\name:
> 
>  #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
> 
> -#ifdef __ARM_ARCH_6M__
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  #define EQUIV .thumb_set
>  #else
>  .macro	ARM_FUNC_START name sp_section=
> @@ -510,7 +510,7 @@ SYM (__\name):
>  #endif
>  .endm
> 
> -#ifndef __ARM_ARCH_6M__
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>  .macro	ARM_FUNC_ALIAS new old
>  	.globl	SYM (__\new)
>  	EQUIV	SYM (__\new), SYM (__\old)
> @@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod
>  /* ------------------------------------------------------------------------
> */ #ifdef L_umodsi3
> 
> -#ifdef __ARM_ARCH_EXT_IDIV__
> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
> 
>  	ARM_FUNC_START umodsi3
> 
> @@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod
>  /* ------------------------------------------------------------------------
> */ #ifdef L_modsi3
> 
> -#if defined(__ARM_ARCH_EXT_IDIV__)
> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
> 
>  	ARM_FUNC_START modsi3
> 
> @@ -1508,14 +1508,8 @@ LSYM(Lover12):
> 
>  #endif /* __symbian__ */
> 
> -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
> -    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
> -    || defined(__ARM_ARCH_5TEJ__)
> -#define HAVE_ARM_CLZ 1
> -#endif
> -
>  #ifdef L_clzsi2
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  FUNC_START clzsi2
>  	mov	r1, #28
>  	mov	r3, #1
> @@ -1544,7 +1538,7 @@ FUNC_START clzsi2
>  	FUNC_END clzsi2
>  #else
>  ARM_FUNC_START clzsi2
> -# if defined(HAVE_ARM_CLZ)
> +# if defined(__ARM_FEATURE_CLZ)
>  	clz	r0, r0
>  	RET
>  # else
> @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
>  .align 2
>  1:
>  .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
> -# endif /* !HAVE_ARM_CLZ */
> +# endif /* !__ARM_FEATURE_CLZ */
>  	FUNC_END clzsi2
>  #endif
>  #endif /* L_clzsi2 */
> 
>  #ifdef L_clzdi2
> -#if !defined(HAVE_ARM_CLZ)
> +#if !defined(__ARM_FEATURE_CLZ)
> 
> -# if defined(__ARM_ARCH_6M__)
> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  FUNC_START clzdi2
>  	push	{r4, lr}
>  # else
> @@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2
>  	bl	__clzsi2
>  # endif
>  2:
> -# if defined(__ARM_ARCH_6M__)
> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  	pop	{r4, pc}
>  # else
>  	RETLDM	r4
>  # endif
>  	FUNC_END clzdi2
> 
> -#else /* HAVE_ARM_CLZ */
> +#else /* __ARM_FEATURE_CLZ */
> 
>  ARM_FUNC_START clzdi2
>  	cmp	xxh, #0
> @@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2
>  #endif /* L_clzdi2 */
> 
>  #ifdef L_ctzsi2
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  FUNC_START ctzsi2
>  	neg	r1, r0
>  	and	r0, r0, r1
> @@ -1656,7 +1650,7 @@ FUNC_START ctzsi2
>  ARM_FUNC_START ctzsi2
>  	rsb	r1, r0, #0
>  	and	r0, r0, r1
> -# if defined(HAVE_ARM_CLZ)
> +# if defined(__ARM_FEATURE_CLZ)
>  	clz	r0, r0
>  	rsb	r0, r0, #31
>  	RET
> @@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2
>  .align 2
>  1:
>  .byte	27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31
> -# endif /* !HAVE_ARM_CLZ */
> +# endif /* !__ARM_FEATURE_CLZ */
>  	FUNC_END ctzsi2
>  #endif
>  #endif /* L_clzsi2 */
> @@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2
> 
>  /* Don't bother with the old interworking routines for Thumb-2.  */
>  /* ??? Maybe only omit these on "m" variants.  */
> -#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
> +#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
> 
>  #if defined L_interwork_call_via_rX
> 
> @@ -1983,11 +1977,12 @@ LSYM(Lchange_\register):
>  .endm
> 
>  #ifndef __symbian__
> -#ifndef __ARM_ARCH_6M__
> +/* The condition here must match the one in gcc/config/arm/elf.h.  */
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>  #include "ieee754-df.S"
>  #include "ieee754-sf.S"
>  #include "bpabi.S"
> -#else /* __ARM_ARCH_6M__ */
> +#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>  #include "bpabi-v6m.S"
> -#endif /* __ARM_ARCH_6M__ */
> +#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>  #endif /* !__symbian__ */
> diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
> index cac1022..393ec8a 100644
> --- a/libgcc/config/arm/libunwind.S
> +++ b/libgcc/config/arm/libunwind.S
> @@ -58,7 +58,7 @@
>  #endif
>  #endif
> 
> -#ifdef __ARM_ARCH_6M__
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
> 
>  /* r0 points to a 16-word block.  Upload these values to the actual core
>     state.  */
> @@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
>  	UNPREFIX \name
>  .endm
> 
> -#else /* !__ARM_ARCH_6M__ */
> +#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
> 
>  /* r0 points to a 16-word block.  Upload these values to the actual core
>     state.  */
> @@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
>  	UNPREFIX \name
>  .endm
> 
> -#endif /* !__ARM_ARCH_6M__ */
> +#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
> 
>  UNWIND_WRAPPER _Unwind_RaiseException 1
>  UNWIND_WRAPPER _Unwind_Resume 1
> diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
> index 4ede438..554ec9b 100644
> --- a/libgcc/config/arm/t-softfp
> +++ b/libgcc/config/arm/t-softfp
> @@ -1,2 +1,2 @@
> -softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
> +softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB ==
> 1' softfp_wrap_end := '\#endif'
> 
> 
> Tested by building libgcc and comparing it before and
> after with objdump -D and readelf -A for all permutations of:
> 
>   Architectures:
>     armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
>     armv6z armv6kz armv6t2 armv6s-m armv7 armv7-a armv7ve
>     armv7-r armv7-m armv7e-m armv8-a armv8-a+crc iwmmxt
>     iwmmxt2
> 
>   ISAs:
>     thumb arm
> 
>   Optimization Levels:
>     Os O2
> 
>   Excluding:
>     armv6s-m -marm
>     armv7-m -marm
>     armv7e-m -marm
>     iwmmxt -mthumb
>     iwmmxt2 -mthumb
> 
> as being rejected by the compiler or assembler. ARMv6-m was not tested
> because compilation fails due to svc instruction.
> 
> Best regards,
> 
> Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-05-17  9:58 ` [PATCH, libgcc/ARM 1/6, ping1] " Thomas Preudhomme
@ 2016-05-19 16:42   ` Kyrill Tkachov
  2016-05-19 16:56     ` Thomas Preudhomme
  0 siblings, 1 reply; 15+ messages in thread
From: Kyrill Tkachov @ 2016-05-19 16:42 UTC (permalink / raw)
  To: Thomas Preudhomme, ramana.radhakrishnan, richard.earnshaw, gcc-patches

Hi Thomas,

I'm not very familiar with the libgcc machinery, but I have a comment on an arm.h hunk inline.

On 17/05/16 10:58, Thomas Preudhomme wrote:
> Ping?
>
> *** gcc/ChangeLog ***
>
> 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
>          decide whether to prevent some libgcc routines being included for some
>          multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
>          link between this condition and the one in
>          libgcc/config/arm/lib1func.S.
>          * config/arm/arm.h (TARGET_ARM_V6M): Add check to TARGET_ARM_ARCH.
>          (TARGET_ARM_V7M): Likewise.
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
>          __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>
>
> *** libgcc/ChangeLog ***
>
> 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1 rather
>          than ARMv6-M.
>          * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
>          for all Thumb-1 only targets.
>          (__only_thumb1__): Define for all Thumb-1 only targets.
>          (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
>          (EQUIV): Likewise.
>          (ARM_FUNC_ALIAS): Likewise.
>          (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
>          (modsi3): Likewise.
>          (HAVE_ARM_CLZ): Remove block defining it.
>          (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
>          check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
>          (clzdi2): Likewise.
>          (ctzsi2): Likewise.
>          (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
>          __ARM_ARCH_6M__ in guard for checking whether it is defined.
>          (final includes): Test for __only_thumb1__ rather than
>          __ARM_ARCH_6M__ and add comment to indicate the connection between
>          this condition and the one in gcc/config/arm/elf.h.
>          * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>          __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>          * config/arm/t-softfp: Likewise.
>
>
> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> index
> 5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9fe66611c19
> 100644
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -2188,8 +2188,10 @@ extern int making_const_table;
>   #define TARGET_ARM_ARCH	\
>     (arm_base_arch)	\
>   
> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm \
> +			&& !arm_arch_thumb2)
> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm \
> +			&& arm_arch_thumb2)
>   

I think now that you're checking TARGET_ARM_ARCH you don't need
the "!arm_arch_notm && !arm_arch_thumb2" && "!arm_arch_notm && arm_arch_thumb2".

Kyrill

>   /* The highest Thumb instruction set version supported by the chip.  */
>   #define TARGET_ARM_ARCH_ISA_THUMB 		\
> diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
> index
> 77f30554d5286bd83aeab0c8dc308cfd44e732dc..246de5492665ba2a0292736a9c53fbaaef184d72
> 100644
> --- a/gcc/config/arm/elf.h
> +++ b/gcc/config/arm/elf.h
> @@ -148,8 +148,9 @@
>     while (0)
>   
>   /* Horrible hack: We want to prevent some libgcc routines being included
> -   for some multilibs.  */
> -#ifndef __ARM_ARCH_6M__
> +   for some multilibs.  The condition should match the one in
> +   libgcc/config/arm/lib1funcs.S.  */
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>   #undef L_fixdfsi
>   #undef L_fixunsdfsi
>   #undef L_truncdfsf2
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-
> supports.exp
> index
> 04ca17656f2f26dda710e8a0f9ca77dd963ab39b..38151375c29cd007f1cc34ead3aa495606224061
> 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -3320,10 +3320,8 @@ proc check_effective_target_arm_cortex_m { } {
>   	return 0
>       }
>       return [check_no_compiler_messages arm_cortex_m assembly {
> -	#if !defined(__ARM_ARCH_7M__) \
> -            && !defined (__ARM_ARCH_7EM__) \
> -            && !defined (__ARM_ARCH_6M__)
> -	#error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
> +	#if defined(__ARM_ARCH_ISA_ARM)
> +	#error __ARM_ARCH_ISA_ARM is defined
>   	#endif
>   	int i;
>       } "-mthumb"]
> diff --git a/libgcc/config/arm/bpabi-v6m.S b/libgcc/config/arm/bpabi-v6m.S
> index
> 5d35aa6afca224613c94cf923f8a2ee8dac949f2..b1d375dfb88efb899ce6213013205b85e531a884
> 100644
> --- a/libgcc/config/arm/bpabi-v6m.S
> +++ b/libgcc/config/arm/bpabi-v6m.S
> @@ -1,4 +1,4 @@
> -/* Miscellaneous BPABI functions.  ARMv6M implementation
> +/* Miscellaneous BPABI functions.  Thumb-1 only implementation
>   
>      Copyright (C) 2006-2016 Free Software Foundation, Inc.
>      Contributed by CodeSourcery.
> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
> index
> 375a5135110895faa44267ebee045fd315515027..7268deb37f2c4ce4e43eeac06c418235071ebcd2
> 100644
> --- a/libgcc/config/arm/lib1funcs.S
> +++ b/libgcc/config/arm/lib1funcs.S
> @@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
>        && !defined(__thumb2__)		\
>        && (!defined(__THUMB_INTERWORK__)	\
>   	 || defined (__OPTIMIZE_SIZE__)	\
> -	 || defined(__ARM_ARCH_6M__)))
> +	 || !__ARM_ARCH_ISA_ARM))
>   # define __prefer_thumb__
>   #endif
>   
> @@ -305,7 +305,7 @@ LSYM(Lend_fde):
>   
>   #ifdef __ARM_EABI__
>   .macro THUMB_LDIV0 name signed
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>   	.ifc \signed, unsigned
>   	cmp	r0, #0
>   	beq	1f
> @@ -478,7 +478,7 @@ _L__\name:
>   
>   #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
>   
> -#ifdef __ARM_ARCH_6M__
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>   #define EQUIV .thumb_set
>   #else
>   .macro	ARM_FUNC_START name sp_section=
> @@ -510,7 +510,7 @@ SYM (__\name):
>   #endif
>   .endm
>   
> -#ifndef __ARM_ARCH_6M__
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>   .macro	ARM_FUNC_ALIAS new old
>   	.globl	SYM (__\new)
>   	EQUIV	SYM (__\new), SYM (__\old)
> @@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod
>   /* ------------------------------------------------------------------------
> */
>   #ifdef L_umodsi3
>   
> -#ifdef __ARM_ARCH_EXT_IDIV__
> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
>   
>   	ARM_FUNC_START umodsi3
>   
> @@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod
>   /* ------------------------------------------------------------------------
> */
>   #ifdef L_modsi3
>   
> -#if defined(__ARM_ARCH_EXT_IDIV__)
> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
>   
>   	ARM_FUNC_START modsi3
>   
> @@ -1508,14 +1508,8 @@ LSYM(Lover12):
>   
>   #endif /* __symbian__ */
>   
> -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
> -    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
> -    || defined(__ARM_ARCH_5TEJ__)
> -#define HAVE_ARM_CLZ 1
> -#endif
> -
>   #ifdef L_clzsi2
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>   FUNC_START clzsi2
>   	mov	r1, #28
>   	mov	r3, #1
> @@ -1544,7 +1538,7 @@ FUNC_START clzsi2
>   	FUNC_END clzsi2
>   #else
>   ARM_FUNC_START clzsi2
> -# if defined(HAVE_ARM_CLZ)
> +# if defined(__ARM_FEATURE_CLZ)
>   	clz	r0, r0
>   	RET
>   # else
> @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
>   .align 2
>   1:
>   .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
> -# endif /* !HAVE_ARM_CLZ */
> +# endif /* !__ARM_FEATURE_CLZ */
>   	FUNC_END clzsi2
>   #endif
>   #endif /* L_clzsi2 */
>   
>   #ifdef L_clzdi2
> -#if !defined(HAVE_ARM_CLZ)
> +#if !defined(__ARM_FEATURE_CLZ)
>   
> -# if defined(__ARM_ARCH_6M__)
> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>   FUNC_START clzdi2
>   	push	{r4, lr}
>   # else
> @@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2
>   	bl	__clzsi2
>   # endif
>   2:
> -# if defined(__ARM_ARCH_6M__)
> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>   	pop	{r4, pc}
>   # else
>   	RETLDM	r4
>   # endif
>   	FUNC_END clzdi2
>   
> -#else /* HAVE_ARM_CLZ */
> +#else /* __ARM_FEATURE_CLZ */
>   
>   ARM_FUNC_START clzdi2
>   	cmp	xxh, #0
> @@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2
>   #endif /* L_clzdi2 */
>   
>   #ifdef L_ctzsi2
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>   FUNC_START ctzsi2
>   	neg	r1, r0
>   	and	r0, r0, r1
> @@ -1656,7 +1650,7 @@ FUNC_START ctzsi2
>   ARM_FUNC_START ctzsi2
>   	rsb	r1, r0, #0
>   	and	r0, r0, r1
> -# if defined(HAVE_ARM_CLZ)
> +# if defined(__ARM_FEATURE_CLZ)
>   	clz	r0, r0
>   	rsb	r0, r0, #31
>   	RET
> @@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2
>   .align 2
>   1:
>   .byte	27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31
> -# endif /* !HAVE_ARM_CLZ */
> +# endif /* !__ARM_FEATURE_CLZ */
>   	FUNC_END ctzsi2
>   #endif
>   #endif /* L_clzsi2 */
> @@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2
>   
>   /* Don't bother with the old interworking routines for Thumb-2.  */
>   /* ??? Maybe only omit these on "m" variants.  */
> -#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
> +#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
>   
>   #if defined L_interwork_call_via_rX
>   
> @@ -1983,11 +1977,12 @@ LSYM(Lchange_\register):
>   .endm
>   
>   #ifndef __symbian__
> -#ifndef __ARM_ARCH_6M__
> +/* The condition here must match the one in gcc/config/arm/elf.h.  */
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>   #include "ieee754-df.S"
>   #include "ieee754-sf.S"
>   #include "bpabi.S"
> -#else /* __ARM_ARCH_6M__ */
> +#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>   #include "bpabi-v6m.S"
> -#endif /* __ARM_ARCH_6M__ */
> +#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>   #endif /* !__symbian__ */
> diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
> index
> a68b10ddce93d230c504f3747dcad3832d9f753c..3d7e70181fa80fe53a4903c96bb0f90480feee21
> 100644
> --- a/libgcc/config/arm/libunwind.S
> +++ b/libgcc/config/arm/libunwind.S
> @@ -58,7 +58,7 @@
>   #endif
>   #endif
>   
> -#ifdef __ARM_ARCH_6M__
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>   
>   /* r0 points to a 16-word block.  Upload these values to the actual core
>      state.  */
> @@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
>   	UNPREFIX \name
>   .endm
>   
> -#else /* !__ARM_ARCH_6M__ */
> +#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
>   
>   /* r0 points to a 16-word block.  Upload these values to the actual core
>      state.  */
> @@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
>   	UNPREFIX \name
>   .endm
>   
> -#endif /* !__ARM_ARCH_6M__ */
> +#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
>   
>   UNWIND_WRAPPER _Unwind_RaiseException 1
>   UNWIND_WRAPPER _Unwind_Resume 1
> diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
> index
> 4ede438baf6a297737e52db00395f6c3a359f681..554ec9bc47b04445e79e84b1f957bf88680c08d1
> 100644
> --- a/libgcc/config/arm/t-softfp
> +++ b/libgcc/config/arm/t-softfp
> @@ -1,2 +1,2 @@
> -softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
> +softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1'
>   softfp_wrap_end := '\#endif'
>
>
> Tested by building libgcc and comparing it before and
> after with objdump -D and readelf -A for all permutations of:
>
>    Architectures:
>      armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
>      armv6z armv6kz armv6t2 armv6s-m armv7 armv7-a armv7ve
>      armv7-r armv7-m armv7e-m armv8-a armv8-a+crc iwmmxt
>      iwmmxt2
>
>    ISAs:
>      thumb arm
>
>    Optimization Levels:
>      Os O2
>
>    Excluding:
>      armv6s-m -marm
>      armv7-m -marm
>      armv7e-m -marm
>      iwmmxt -mthumb
>      iwmmxt2 -mthumb
>
> as being rejected by the compiler or assembler. ARMv6-m was not tested because
> compilation fails due to svc instruction.
>
>
> Best regards,
>
> Thomas
>
> On Tuesday 12 January 2016 17:25:17 Thomas Preud'homme wrote:
>>> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
>>> owner@gcc.gnu.org] On Behalf Of Thomas Preud'homme
>>> Sent: Thursday, December 17, 2015 1:58 PM
>>>
>>> Hi,
>>>
>>> We decided to apply the following patch to the ARM embedded 5 branch.
>>> This is *not* intended for trunk for now. We will send a separate email
>>> for trunk.
>> And now a rebased patch on top of trunk.
>>
>>> This patch is part of a patch series to add support for ARMv8-M[1] to GCC.
>>> This specific patch fixes some assumptions related to M profile
>>> architectures. Currently GCC (mostly libgcc) contains several assumptions
>>> that the only ARM architecture with Thumb-1 only instructions is ARMv6-
>>> M and the only one with Thumb-2 only instructions is ARMv7-M. ARMv8-
>>> M [1] make this wrong since ARMv8-M baseline is also (mostly) Thumb-1
>>> only and ARMv8-M mainline is also Thumb-2 only. This patch replace
>>> checks for __ARM_ARCH_*__ for checks against
>>> __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM instead. For
>>> instance, Thumb-1 only can be checked with
>>> #if !defined(__ARM_ARCH_ISA_ARM) && (__ARM_ARCH_ISA_THUMB
>>> == 1). It also fixes the guard for DIV code to not apply to ARMv8-M
>>> Baseline since it uses Thumb-2 instructions.
>>>
>>> [1] For a quick overview of ARMv8-M please refer to the initial cover
>>> letter.
>>> ChangeLog entries are as follow:
>> *** gcc/ChangeLog ***
>>
>> 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM
>> to decide whether to prevent some libgcc routines being included for some
>> multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the link
>> between this condition and the one in
>>          libgcc/config/arm/lib1func.S.
>>          * config/arm/arm.h (TARGET_ARM_V6M): Add check to TARGET_ARM_ARCH.
>>          (TARGET_ARM_V7M): Likewise.
>>
>>
>> *** gcc/testsuite/ChangeLog ***
>>
>> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
>> __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>>
>>
>> *** libgcc/ChangeLog ***
>>
>> 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>
>>          * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1
>> rather than ARMv6-M.
>>          * config/arm/lib1funcs.S (__prefer_thumb__): Define among other
>> cases for all Thumb-1 only targets.
>>          (__only_thumb1__): Define for all Thumb-1 only targets.
>>          (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
>> (EQUIV): Likewise.
>>          (ARM_FUNC_ALIAS): Likewise.
>>          (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
>>          (modsi3): Likewise.
>>          (HAVE_ARM_CLZ): Remove block defining it.
>>          (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
>>          check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
>>          (clzdi2): Likewise.
>>          (ctzsi2): Likewise.
>>          (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
>>          __ARM_ARCH_6M__ in guard for checking whether it is defined.
>>          (final includes): Test for __only_thumb1__ rather than
>>          __ARM_ARCH_6M__ and add comment to indicate the connection between
>>          this condition and the one in gcc/config/arm/elf.h.
>>          * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>>          __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>>          * config/arm/t-softfp: Likewise.
>>
>>
>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
>> index fd999dd..0d23f39 100644
>> --- a/gcc/config/arm/arm.h
>> +++ b/gcc/config/arm/arm.h
>> @@ -2182,8 +2182,10 @@ extern int making_const_table;
>>   #define TARGET_ARM_ARCH	\
>>     (arm_base_arch)	\
>>
>> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
>> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
>> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm \
>> +			&& !arm_arch_thumb2)
>> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm \
>> +			&& arm_arch_thumb2)
>>
>>   /* The highest Thumb instruction set version supported by the chip.  */
>>   #define TARGET_ARM_ARCH_ISA_THUMB 		\
>> diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
>> index 3795728..579a580 100644
>> --- a/gcc/config/arm/elf.h
>> +++ b/gcc/config/arm/elf.h
>> @@ -148,8 +148,9 @@
>>     while (0)
>>
>>   /* Horrible hack: We want to prevent some libgcc routines being included
>> -   for some multilibs.  */
>> -#ifndef __ARM_ARCH_6M__
>> +   for some multilibs.  The condition should match the one in
>> +   libgcc/config/arm/lib1funcs.S.  */
>> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>>   #undef L_fixdfsi
>>   #undef L_fixunsdfsi
>>   #undef L_truncdfsf2
>> diff --git a/gcc/testsuite/lib/target-supports.exp
>> b/gcc/testsuite/lib/target-supports.exp index 4e349e9..3f96826 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -3221,10 +3221,8 @@ proc check_effective_target_arm_cortex_m { } {
>>   	return 0
>>       }
>>       return [check_no_compiler_messages arm_cortex_m assembly {
>> -	#if !defined(__ARM_ARCH_7M__) \
>> -            && !defined (__ARM_ARCH_7EM__) \
>> -            && !defined (__ARM_ARCH_6M__)
>> -	#error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
>> +	#if defined(__ARM_ARCH_ISA_ARM)
>> +	#error __ARM_ARCH_ISA_ARM is defined
>>   	#endif
>>   	int i;
>>       } "-mthumb"]
>> diff --git a/libgcc/config/arm/bpabi-v6m.S b/libgcc/config/arm/bpabi-v6m.S
>> index a1e1640..9ae0bb8 100644
>> --- a/libgcc/config/arm/bpabi-v6m.S
>> +++ b/libgcc/config/arm/bpabi-v6m.S
>> @@ -1,4 +1,4 @@
>> -/* Miscellaneous BPABI functions.  ARMv6M implementation
>> +/* Miscellaneous BPABI functions.  Thumb-1 only implementation
>>
>>      Copyright (C) 2006-2015 Free Software Foundation, Inc.
>>      Contributed by CodeSourcery.
>> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
>> index 252efcb..95a1f80 100644
>> --- a/libgcc/config/arm/lib1funcs.S
>> +++ b/libgcc/config/arm/lib1funcs.S
>> @@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
>>   If not, see && !defined(__thumb2__)		\
>>        && (!defined(__THUMB_INTERWORK__)	\
>>
>>   	 || defined (__OPTIMIZE_SIZE__)	\
>>
>> -	 || defined(__ARM_ARCH_6M__)))
>> +	 || !__ARM_ARCH_ISA_ARM))
>>   # define __prefer_thumb__
>>   #endif
>>
>> @@ -305,7 +305,7 @@ LSYM(Lend_fde):
>>
>>   #ifdef __ARM_EABI__
>>   .macro THUMB_LDIV0 name signed
>> -#if defined(__ARM_ARCH_6M__)
>> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>>   	.ifc \signed, unsigned
>>   	cmp	r0, #0
>>   	beq	1f
>> @@ -478,7 +478,7 @@ _L__\name:
>>
>>   #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
>>
>> -#ifdef __ARM_ARCH_6M__
>> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>>   #define EQUIV .thumb_set
>>   #else
>>   .macro	ARM_FUNC_START name sp_section=
>> @@ -510,7 +510,7 @@ SYM (__\name):
>>   #endif
>>   .endm
>>
>> -#ifndef __ARM_ARCH_6M__
>> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>>   .macro	ARM_FUNC_ALIAS new old
>>   	.globl	SYM (__\new)
>>   	EQUIV	SYM (__\new), SYM (__\old)
>> @@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod
>>   /* ------------------------------------------------------------------------
>> */ #ifdef L_umodsi3
>>
>> -#ifdef __ARM_ARCH_EXT_IDIV__
>> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
>>
>>   	ARM_FUNC_START umodsi3
>>
>> @@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod
>>   /* ------------------------------------------------------------------------
>> */ #ifdef L_modsi3
>>
>> -#if defined(__ARM_ARCH_EXT_IDIV__)
>> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
>>
>>   	ARM_FUNC_START modsi3
>>
>> @@ -1508,14 +1508,8 @@ LSYM(Lover12):
>>
>>   #endif /* __symbian__ */
>>
>> -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
>> -    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
>> -    || defined(__ARM_ARCH_5TEJ__)
>> -#define HAVE_ARM_CLZ 1
>> -#endif
>> -
>>   #ifdef L_clzsi2
>> -#if defined(__ARM_ARCH_6M__)
>> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>>   FUNC_START clzsi2
>>   	mov	r1, #28
>>   	mov	r3, #1
>> @@ -1544,7 +1538,7 @@ FUNC_START clzsi2
>>   	FUNC_END clzsi2
>>   #else
>>   ARM_FUNC_START clzsi2
>> -# if defined(HAVE_ARM_CLZ)
>> +# if defined(__ARM_FEATURE_CLZ)
>>   	clz	r0, r0
>>   	RET
>>   # else
>> @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
>>   .align 2
>>   1:
>>   .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
>> -# endif /* !HAVE_ARM_CLZ */
>> +# endif /* !__ARM_FEATURE_CLZ */
>>   	FUNC_END clzsi2
>>   #endif
>>   #endif /* L_clzsi2 */
>>
>>   #ifdef L_clzdi2
>> -#if !defined(HAVE_ARM_CLZ)
>> +#if !defined(__ARM_FEATURE_CLZ)
>>
>> -# if defined(__ARM_ARCH_6M__)
>> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>>   FUNC_START clzdi2
>>   	push	{r4, lr}
>>   # else
>> @@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2
>>   	bl	__clzsi2
>>   # endif
>>   2:
>> -# if defined(__ARM_ARCH_6M__)
>> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>>   	pop	{r4, pc}
>>   # else
>>   	RETLDM	r4
>>   # endif
>>   	FUNC_END clzdi2
>>
>> -#else /* HAVE_ARM_CLZ */
>> +#else /* __ARM_FEATURE_CLZ */
>>
>>   ARM_FUNC_START clzdi2
>>   	cmp	xxh, #0
>> @@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2
>>   #endif /* L_clzdi2 */
>>
>>   #ifdef L_ctzsi2
>> -#if defined(__ARM_ARCH_6M__)
>> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>>   FUNC_START ctzsi2
>>   	neg	r1, r0
>>   	and	r0, r0, r1
>> @@ -1656,7 +1650,7 @@ FUNC_START ctzsi2
>>   ARM_FUNC_START ctzsi2
>>   	rsb	r1, r0, #0
>>   	and	r0, r0, r1
>> -# if defined(HAVE_ARM_CLZ)
>> +# if defined(__ARM_FEATURE_CLZ)
>>   	clz	r0, r0
>>   	rsb	r0, r0, #31
>>   	RET
>> @@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2
>>   .align 2
>>   1:
>>   .byte	27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31
>> -# endif /* !HAVE_ARM_CLZ */
>> +# endif /* !__ARM_FEATURE_CLZ */
>>   	FUNC_END ctzsi2
>>   #endif
>>   #endif /* L_clzsi2 */
>> @@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2
>>
>>   /* Don't bother with the old interworking routines for Thumb-2.  */
>>   /* ??? Maybe only omit these on "m" variants.  */
>> -#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
>> +#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
>>
>>   #if defined L_interwork_call_via_rX
>>
>> @@ -1983,11 +1977,12 @@ LSYM(Lchange_\register):
>>   .endm
>>
>>   #ifndef __symbian__
>> -#ifndef __ARM_ARCH_6M__
>> +/* The condition here must match the one in gcc/config/arm/elf.h.  */
>> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>>   #include "ieee754-df.S"
>>   #include "ieee754-sf.S"
>>   #include "bpabi.S"
>> -#else /* __ARM_ARCH_6M__ */
>> +#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>>   #include "bpabi-v6m.S"
>> -#endif /* __ARM_ARCH_6M__ */
>> +#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>>   #endif /* !__symbian__ */
>> diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
>> index cac1022..393ec8a 100644
>> --- a/libgcc/config/arm/libunwind.S
>> +++ b/libgcc/config/arm/libunwind.S
>> @@ -58,7 +58,7 @@
>>   #endif
>>   #endif
>>
>> -#ifdef __ARM_ARCH_6M__
>> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>>
>>   /* r0 points to a 16-word block.  Upload these values to the actual core
>>      state.  */
>> @@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
>>   	UNPREFIX \name
>>   .endm
>>
>> -#else /* !__ARM_ARCH_6M__ */
>> +#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
>>
>>   /* r0 points to a 16-word block.  Upload these values to the actual core
>>      state.  */
>> @@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
>>   	UNPREFIX \name
>>   .endm
>>
>> -#endif /* !__ARM_ARCH_6M__ */
>> +#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
>>
>>   UNWIND_WRAPPER _Unwind_RaiseException 1
>>   UNWIND_WRAPPER _Unwind_Resume 1
>> diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
>> index 4ede438..554ec9b 100644
>> --- a/libgcc/config/arm/t-softfp
>> +++ b/libgcc/config/arm/t-softfp
>> @@ -1,2 +1,2 @@
>> -softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
>> +softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB ==
>> 1' softfp_wrap_end := '\#endif'
>>
>>
>> Tested by building libgcc and comparing it before and
>> after with objdump -D and readelf -A for all permutations of:
>>
>>    Architectures:
>>      armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
>>      armv6z armv6kz armv6t2 armv6s-m armv7 armv7-a armv7ve
>>      armv7-r armv7-m armv7e-m armv8-a armv8-a+crc iwmmxt
>>      iwmmxt2
>>
>>    ISAs:
>>      thumb arm
>>
>>    Optimization Levels:
>>      Os O2
>>
>>    Excluding:
>>      armv6s-m -marm
>>      armv7-m -marm
>>      armv7e-m -marm
>>      iwmmxt -mthumb
>>      iwmmxt2 -mthumb
>>
>> as being rejected by the compiler or assembler. ARMv6-m was not tested
>> because compilation fails due to svc instruction.
>>
>> Best regards,
>>
>> Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-05-19 16:42   ` Kyrill Tkachov
@ 2016-05-19 16:56     ` Thomas Preudhomme
  2016-05-19 17:01       ` Kyrill Tkachov
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2016-05-19 16:56 UTC (permalink / raw)
  To: Kyrill Tkachov; +Cc: ramana.radhakrishnan, richard.earnshaw, gcc-patches

On Thursday 19 May 2016 17:42:26 Kyrill Tkachov wrote:
> Hi Thomas,
> 
> I'm not very familiar with the libgcc machinery, but I have a comment on an
> arm.h hunk inline.
> On 17/05/16 10:58, Thomas Preudhomme wrote:
> > Ping?
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and
> >          __ARM_ARCH_ISA_ARM to
> >          decide whether to prevent some libgcc routines being included for
> >          some
> >          multilibs rather than __ARM_ARCH_6M__ and add comment to indicate
> >          the
> >          link between this condition and the one in
> >          libgcc/config/arm/lib1func.S.
> >          * config/arm/arm.h (TARGET_ARM_V6M): Add check to
> >          TARGET_ARM_ARCH.
> >          (TARGET_ARM_V7M): Likewise.
> > 
> > *** gcc/testsuite/ChangeLog ***
> > 
> > 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * lib/target-supports.exp (check_effective_target_arm_cortex_m):
> >          Use
> >          __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
> > 
> > *** libgcc/ChangeLog ***
> > 
> > 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >          * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1
> >          rather
> >          than ARMv6-M.
> >          * config/arm/lib1funcs.S (__prefer_thumb__): Define among other
> >          cases
> >          for all Thumb-1 only targets.
> >          (__only_thumb1__): Define for all Thumb-1 only targets.
> >          (THUMB_LDIV0): Test for __only_thumb1__ rather than
> >          __ARM_ARCH_6M__.
> >          (EQUIV): Likewise.
> >          (ARM_FUNC_ALIAS): Likewise.
> >          (umodsi3): Add check to __only_thumb1__ to guard the idiv
> >          version.
> >          (modsi3): Likewise.
> >          (HAVE_ARM_CLZ): Remove block defining it.
> >          (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__
> >          and
> >          check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
> >          (clzdi2): Likewise.
> >          (ctzsi2): Likewise.
> >          (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather
> >          than
> >          __ARM_ARCH_6M__ in guard for checking whether it is defined.
> >          (final includes): Test for __only_thumb1__ rather than
> >          __ARM_ARCH_6M__ and add comment to indicate the connection
> >          between
> >          this condition and the one in gcc/config/arm/elf.h.
> >          * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
> >          __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
> >          * config/arm/t-softfp: Likewise.
> > 
> > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> > index
> > 5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9fe
> > 66611c19 100644
> > --- a/gcc/config/arm/arm.h
> > +++ b/gcc/config/arm/arm.h
> > @@ -2188,8 +2188,10 @@ extern int making_const_table;
> > 
> >   #define TARGET_ARM_ARCH	\
> >   
> >     (arm_base_arch)	\
> > 
> > -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
> > -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
> > +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm
> > \ +			&& !arm_arch_thumb2)
> > +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm
> > \ +			&& arm_arch_thumb2)
> 
> I think now that you're checking TARGET_ARM_ARCH you don't need
> the "!arm_arch_notm && !arm_arch_thumb2" && "!arm_arch_notm &&
> arm_arch_thumb2".

% git --no-pager grep TARGET_ARM_V.M config/arm
config/arm/arm.h:#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
config/arm/arm.h:#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)

What about just removing those? I kept them because I wasn't sure of their 
purpose but I think we should just remove them.

Best regards,

Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-05-19 16:56     ` Thomas Preudhomme
@ 2016-05-19 17:01       ` Kyrill Tkachov
  2016-05-25 13:56         ` Thomas Preudhomme
  0 siblings, 1 reply; 15+ messages in thread
From: Kyrill Tkachov @ 2016-05-19 17:01 UTC (permalink / raw)
  To: Thomas Preudhomme; +Cc: ramana.radhakrishnan, richard.earnshaw, gcc-patches


On 19/05/16 17:55, Thomas Preudhomme wrote:
> On Thursday 19 May 2016 17:42:26 Kyrill Tkachov wrote:
>> Hi Thomas,
>>
>> I'm not very familiar with the libgcc machinery, but I have a comment on an
>> arm.h hunk inline.
>> On 17/05/16 10:58, Thomas Preudhomme wrote:
>>> Ping?
>>>
>>> *** gcc/ChangeLog ***
>>>
>>> 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>           * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and
>>>           __ARM_ARCH_ISA_ARM to
>>>           decide whether to prevent some libgcc routines being included for
>>>           some
>>>           multilibs rather than __ARM_ARCH_6M__ and add comment to indicate
>>>           the
>>>           link between this condition and the one in
>>>           libgcc/config/arm/lib1func.S.
>>>           * config/arm/arm.h (TARGET_ARM_V6M): Add check to
>>>           TARGET_ARM_ARCH.
>>>           (TARGET_ARM_V7M): Likewise.
>>>
>>> *** gcc/testsuite/ChangeLog ***
>>>
>>> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>           * lib/target-supports.exp (check_effective_target_arm_cortex_m):
>>>           Use
>>>           __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>>>
>>> *** libgcc/ChangeLog ***
>>>
>>> 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>
>>>           * config/arm/bpabi-v6m.S: Fix header comment to mention Thumb-1
>>>           rather
>>>           than ARMv6-M.
>>>           * config/arm/lib1funcs.S (__prefer_thumb__): Define among other
>>>           cases
>>>           for all Thumb-1 only targets.
>>>           (__only_thumb1__): Define for all Thumb-1 only targets.
>>>           (THUMB_LDIV0): Test for __only_thumb1__ rather than
>>>           __ARM_ARCH_6M__.
>>>           (EQUIV): Likewise.
>>>           (ARM_FUNC_ALIAS): Likewise.
>>>           (umodsi3): Add check to __only_thumb1__ to guard the idiv
>>>           version.
>>>           (modsi3): Likewise.
>>>           (HAVE_ARM_CLZ): Remove block defining it.
>>>           (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__
>>>           and
>>>           check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
>>>           (clzdi2): Likewise.
>>>           (ctzsi2): Likewise.
>>>           (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather
>>>           than
>>>           __ARM_ARCH_6M__ in guard for checking whether it is defined.
>>>           (final includes): Test for __only_thumb1__ rather than
>>>           __ARM_ARCH_6M__ and add comment to indicate the connection
>>>           between
>>>           this condition and the one in gcc/config/arm/elf.h.
>>>           * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>>>           __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>>>           * config/arm/t-softfp: Likewise.
>>>
>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
>>> index
>>> 5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9fe
>>> 66611c19 100644
>>> --- a/gcc/config/arm/arm.h
>>> +++ b/gcc/config/arm/arm.h
>>> @@ -2188,8 +2188,10 @@ extern int making_const_table;
>>>
>>>    #define TARGET_ARM_ARCH	\
>>>    
>>>      (arm_base_arch)	\
>>>
>>> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
>>> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
>>> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm
>>> \ +			&& !arm_arch_thumb2)
>>> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm
>>> \ +			&& arm_arch_thumb2)
>> I think now that you're checking TARGET_ARM_ARCH you don't need
>> the "!arm_arch_notm && !arm_arch_thumb2" && "!arm_arch_notm &&
>> arm_arch_thumb2".
> % git --no-pager grep TARGET_ARM_V.M config/arm
> config/arm/arm.h:#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
> config/arm/arm.h:#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
>
> What about just removing those? I kept them because I wasn't sure of their
> purpose but I think we should just remove them.

That's fine with me.
Then you'd also want to remove the TARGET_ARM_V8M definition from your second patch
that I flagged up in that review.

Kyrill

> Best regards,
>
> Thomas
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-05-19 17:01       ` Kyrill Tkachov
@ 2016-05-25 13:56         ` Thomas Preudhomme
  2016-05-25 15:30           ` Kyrill Tkachov
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2016-05-25 13:56 UTC (permalink / raw)
  To: Kyrill Tkachov; +Cc: ramana.radhakrishnan, richard.earnshaw, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 6960 bytes --]

Hi Kyrill,

Please find an updated version below. Note that I also:

* removed the change to bpabi-v6m.S because that actually accurately describe 
the implementation (using instructions from ARMv6-M) and does not suggest it 
is not compatible with other architecture (it does not say ARMv6-M only)
* kept the TARGET_ARM_V*M unchanged, this is now dealt in a separate patch


ChangeLog entries are now as follow:


*** gcc/ChangeLog ***

2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
        decide whether to prevent some libgcc routines being included for some
        multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
        link between this condition and the one in
        libgcc/config/arm/lib1func.S.


*** gcc/testsuite/ChangeLog ***

2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
        __ARM_ARCH_ISA_ARM to test for Cortex-M devices.


*** libgcc/ChangeLog ***

2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
        for all Thumb-1 only targets.
        (__only_thumb1__): Define for all Thumb-1 only targets.
        (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
        (EQUIV): Likewise.
        (ARM_FUNC_ALIAS): Likewise.
        (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
        (modsi3): Likewise.
        (HAVE_ARM_CLZ): Remove block defining it.
        (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
        check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
        (clzdi2): Likewise.
        (ctzsi2): Likewise.
        (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
        __ARM_ARCH_6M__ in guard for checking whether it is defined.
        (final includes): Test for __only_thumb1__ rather than
        __ARM_ARCH_6M__ and add comment to indicate the connection between
        this condition and the one in gcc/config/arm/elf.h.
        * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
        __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
        * config/arm/t-softfp: Likewise.


Updated patch is attached to this mail.

Best regards,

Thomas

On Thursday 19 May 2016 18:01:16 Kyrill Tkachov wrote:
> On 19/05/16 17:55, Thomas Preudhomme wrote:
> > On Thursday 19 May 2016 17:42:26 Kyrill Tkachov wrote:
> >> Hi Thomas,
> >> 
> >> I'm not very familiar with the libgcc machinery, but I have a comment on
> >> an
> >> arm.h hunk inline.
> >> 
> >> On 17/05/16 10:58, Thomas Preudhomme wrote:
> >>> Ping?
> >>> 
> >>> *** gcc/ChangeLog ***
> >>> 
> >>> 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >>> 
> >>>           * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and
> >>>           __ARM_ARCH_ISA_ARM to
> >>>           decide whether to prevent some libgcc routines being included
> >>>           for
> >>>           some
> >>>           multilibs rather than __ARM_ARCH_6M__ and add comment to
> >>>           indicate
> >>>           the
> >>>           link between this condition and the one in
> >>>           libgcc/config/arm/lib1func.S.
> >>>           * config/arm/arm.h (TARGET_ARM_V6M): Add check to
> >>>           TARGET_ARM_ARCH.
> >>>           (TARGET_ARM_V7M): Likewise.
> >>> 
> >>> *** gcc/testsuite/ChangeLog ***
> >>> 
> >>> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >>> 
> >>>           * lib/target-supports.exp
> >>>           (check_effective_target_arm_cortex_m):
> >>>           Use
> >>>           __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
> >>> 
> >>> *** libgcc/ChangeLog ***
> >>> 
> >>> 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> >>> 
> >>>           * config/arm/bpabi-v6m.S: Fix header comment to mention
> >>>           Thumb-1
> >>>           rather
> >>>           than ARMv6-M.
> >>>           * config/arm/lib1funcs.S (__prefer_thumb__): Define among
> >>>           other
> >>>           cases
> >>>           for all Thumb-1 only targets.
> >>>           (__only_thumb1__): Define for all Thumb-1 only targets.
> >>>           (THUMB_LDIV0): Test for __only_thumb1__ rather than
> >>>           __ARM_ARCH_6M__.
> >>>           (EQUIV): Likewise.
> >>>           (ARM_FUNC_ALIAS): Likewise.
> >>>           (umodsi3): Add check to __only_thumb1__ to guard the idiv
> >>>           version.
> >>>           (modsi3): Likewise.
> >>>           (HAVE_ARM_CLZ): Remove block defining it.
> >>>           (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__
> >>>           and
> >>>           check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
> >>>           (clzdi2): Likewise.
> >>>           (ctzsi2): Likewise.
> >>>           (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather
> >>>           than
> >>>           __ARM_ARCH_6M__ in guard for checking whether it is defined.
> >>>           (final includes): Test for __only_thumb1__ rather than
> >>>           __ARM_ARCH_6M__ and add comment to indicate the connection
> >>>           between
> >>>           this condition and the one in gcc/config/arm/elf.h.
> >>>           * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
> >>>           __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
> >>>           * config/arm/t-softfp: Likewise.
> >>> 
> >>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> >>> index
> >>> 5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9
> >>> fe
> >>> 66611c19 100644
> >>> --- a/gcc/config/arm/arm.h
> >>> +++ b/gcc/config/arm/arm.h
> >>> @@ -2188,8 +2188,10 @@ extern int making_const_table;
> >>> 
> >>>    #define TARGET_ARM_ARCH	\
> >>>    
> >>>      (arm_base_arch)	\
> >>> 
> >>> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
> >>> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
> >>> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M &&
> >>> !arm_arch_notm
> >>> \ +			&& !arm_arch_thumb2)
> >>> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M &&
> >>> !arm_arch_notm
> >>> \ +			&& arm_arch_thumb2)
> >> 
> >> I think now that you're checking TARGET_ARM_ARCH you don't need
> >> the "!arm_arch_notm && !arm_arch_thumb2" && "!arm_arch_notm &&
> >> arm_arch_thumb2".
> > 
> > % git --no-pager grep TARGET_ARM_V.M config/arm
> > config/arm/arm.h:#define TARGET_ARM_V6M (!arm_arch_notm &&
> > !arm_arch_thumb2) config/arm/arm.h:#define TARGET_ARM_V7M (!arm_arch_notm
> > && arm_arch_thumb2)
> > 
> > What about just removing those? I kept them because I wasn't sure of their
> > purpose but I think we should just remove them.
> 
> That's fine with me.
> Then you'd also want to remove the TARGET_ARM_V8M definition from your
> second patch that I flagged up in that review.
> 
> Kyrill
> 
> > Best regards,
> > 
> > Thomas

[-- Attachment #2: fix_armv6m_armv7m_assumptions.patch --]
[-- Type: text/x-patch, Size: 7284 bytes --]

diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
index 77f30554d5286bd83aeab0c8dc308cfd44e732dc..246de5492665ba2a0292736a9c53fbaaef184d72 100644
--- a/gcc/config/arm/elf.h
+++ b/gcc/config/arm/elf.h
@@ -148,8 +148,9 @@
   while (0)
 
 /* Horrible hack: We want to prevent some libgcc routines being included
-   for some multilibs.  */
-#ifndef __ARM_ARCH_6M__
+   for some multilibs.  The condition should match the one in
+   libgcc/config/arm/lib1funcs.S.  */
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 #undef L_fixdfsi
 #undef L_fixunsdfsi
 #undef L_truncdfsf2
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 04ca17656f2f26dda710e8a0f9ca77dd963ab39b..38151375c29cd007f1cc34ead3aa495606224061 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3320,10 +3320,8 @@ proc check_effective_target_arm_cortex_m { } {
 	return 0
     }
     return [check_no_compiler_messages arm_cortex_m assembly {
-	#if !defined(__ARM_ARCH_7M__) \
-            && !defined (__ARM_ARCH_7EM__) \
-            && !defined (__ARM_ARCH_6M__)
-	#error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
+	#if defined(__ARM_ARCH_ISA_ARM)
+	#error __ARM_ARCH_ISA_ARM is defined
 	#endif
 	int i;
     } "-mthumb"]
diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index 375a5135110895faa44267ebee045fd315515027..7268deb37f2c4ce4e43eeac06c418235071ebcd2 100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
      && !defined(__thumb2__)		\
      && (!defined(__THUMB_INTERWORK__)	\
 	 || defined (__OPTIMIZE_SIZE__)	\
-	 || defined(__ARM_ARCH_6M__)))
+	 || !__ARM_ARCH_ISA_ARM))
 # define __prefer_thumb__
 #endif
 
@@ -305,7 +305,7 @@ LSYM(Lend_fde):
 
 #ifdef __ARM_EABI__
 .macro THUMB_LDIV0 name signed
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 	.ifc \signed, unsigned
 	cmp	r0, #0
 	beq	1f
@@ -478,7 +478,7 @@ _L__\name:
 
 #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
 
-#ifdef __ARM_ARCH_6M__
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 #define EQUIV .thumb_set
 #else
 .macro	ARM_FUNC_START name sp_section=
@@ -510,7 +510,7 @@ SYM (__\name):
 #endif
 .endm
 
-#ifndef __ARM_ARCH_6M__
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 .macro	ARM_FUNC_ALIAS new old
 	.globl	SYM (__\new)
 	EQUIV	SYM (__\new), SYM (__\old)
@@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod
 /* ------------------------------------------------------------------------ */
 #ifdef L_umodsi3
 
-#ifdef __ARM_ARCH_EXT_IDIV__
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START umodsi3
 
@@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod
 /* ------------------------------------------------------------------------ */
 #ifdef L_modsi3
 
-#if defined(__ARM_ARCH_EXT_IDIV__)
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START modsi3
 
@@ -1508,14 +1508,8 @@ LSYM(Lover12):
 
 #endif /* __symbian__ */
 
-#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
-    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
-    || defined(__ARM_ARCH_5TEJ__)
-#define HAVE_ARM_CLZ 1
-#endif
-
 #ifdef L_clzsi2
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START clzsi2
 	mov	r1, #28
 	mov	r3, #1
@@ -1544,7 +1538,7 @@ FUNC_START clzsi2
 	FUNC_END clzsi2
 #else
 ARM_FUNC_START clzsi2
-# if defined(HAVE_ARM_CLZ)
+# if defined(__ARM_FEATURE_CLZ)
 	clz	r0, r0
 	RET
 # else
@@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
 .align 2
 1:
 .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
-# endif /* !HAVE_ARM_CLZ */
+# endif /* !__ARM_FEATURE_CLZ */
 	FUNC_END clzsi2
 #endif
 #endif /* L_clzsi2 */
 
 #ifdef L_clzdi2
-#if !defined(HAVE_ARM_CLZ)
+#if !defined(__ARM_FEATURE_CLZ)
 
-# if defined(__ARM_ARCH_6M__)
+# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START clzdi2
 	push	{r4, lr}
 # else
@@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2
 	bl	__clzsi2
 # endif
 2:
-# if defined(__ARM_ARCH_6M__)
+# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 	pop	{r4, pc}
 # else
 	RETLDM	r4
 # endif
 	FUNC_END clzdi2
 
-#else /* HAVE_ARM_CLZ */
+#else /* __ARM_FEATURE_CLZ */
 
 ARM_FUNC_START clzdi2
 	cmp	xxh, #0
@@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2
 #endif /* L_clzdi2 */
 
 #ifdef L_ctzsi2
-#if defined(__ARM_ARCH_6M__)
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 FUNC_START ctzsi2
 	neg	r1, r0
 	and	r0, r0, r1
@@ -1656,7 +1650,7 @@ FUNC_START ctzsi2
 ARM_FUNC_START ctzsi2
 	rsb	r1, r0, #0
 	and	r0, r0, r1
-# if defined(HAVE_ARM_CLZ)
+# if defined(__ARM_FEATURE_CLZ)
 	clz	r0, r0
 	rsb	r0, r0, #31
 	RET
@@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2
 .align 2
 1:
 .byte	27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31
-# endif /* !HAVE_ARM_CLZ */
+# endif /* !__ARM_FEATURE_CLZ */
 	FUNC_END ctzsi2
 #endif
 #endif /* L_clzsi2 */
@@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2
 
 /* Don't bother with the old interworking routines for Thumb-2.  */
 /* ??? Maybe only omit these on "m" variants.  */
-#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
+#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
 
 #if defined L_interwork_call_via_rX
 
@@ -1983,11 +1977,12 @@ LSYM(Lchange_\register):
 .endm
 
 #ifndef __symbian__
-#ifndef __ARM_ARCH_6M__
+/* The condition here must match the one in gcc/config/arm/elf.h.  */
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 #include "ieee754-df.S"
 #include "ieee754-sf.S"
 #include "bpabi.S"
-#else /* __ARM_ARCH_6M__ */
+#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
 #include "bpabi-v6m.S"
-#endif /* __ARM_ARCH_6M__ */
+#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
 #endif /* !__symbian__ */
diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
index a68b10ddce93d230c504f3747dcad3832d9f753c..3d7e70181fa80fe53a4903c96bb0f90480feee21 100644
--- a/libgcc/config/arm/libunwind.S
+++ b/libgcc/config/arm/libunwind.S
@@ -58,7 +58,7 @@
 #endif
 #endif
 
-#ifdef __ARM_ARCH_6M__
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#else /* !__ARM_ARCH_6M__ */
+#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#endif /* !__ARM_ARCH_6M__ */
+#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 UNWIND_WRAPPER _Unwind_RaiseException 1
 UNWIND_WRAPPER _Unwind_Resume 1
diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
index 4ede438baf6a297737e52db00395f6c3a359f681..554ec9bc47b04445e79e84b1f957bf88680c08d1 100644
--- a/libgcc/config/arm/t-softfp
+++ b/libgcc/config/arm/t-softfp
@@ -1,2 +1,2 @@
-softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
+softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1'
 softfp_wrap_end := '\#endif'

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6, ping1] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-05-25 13:56         ` Thomas Preudhomme
@ 2016-05-25 15:30           ` Kyrill Tkachov
  0 siblings, 0 replies; 15+ messages in thread
From: Kyrill Tkachov @ 2016-05-25 15:30 UTC (permalink / raw)
  To: Thomas Preudhomme; +Cc: ramana.radhakrishnan, richard.earnshaw, gcc-patches

Hi Thomas,

On 25/05/16 14:22, Thomas Preudhomme wrote:
> Hi Kyrill,
>
> Please find an updated version below. Note that I also:
>
> * removed the change to bpabi-v6m.S because that actually accurately describe
> the implementation (using instructions from ARMv6-M) and does not suggest it
> is not compatible with other architecture (it does not say ARMv6-M only)
> * kept the TARGET_ARM_V*M unchanged, this is now dealt in a separate patch
>
>
> ChangeLog entries are now as follow:
>
>
> *** gcc/ChangeLog ***
>
> 2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
>          decide whether to prevent some libgcc routines being included for some
>          multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
>          link between this condition and the one in
>          libgcc/config/arm/lib1func.S.
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
>          __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>
>
> *** libgcc/ChangeLog ***
>
> 2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>          * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
>          for all Thumb-1 only targets.
>          (__only_thumb1__): Define for all Thumb-1 only targets.
>          (THUMB_LDIV0): Test for __only_thumb1__ rather than __ARM_ARCH_6M__.
>          (EQUIV): Likewise.
>          (ARM_FUNC_ALIAS): Likewise.
>          (umodsi3): Add check to __only_thumb1__ to guard the idiv version.
>          (modsi3): Likewise.
>          (HAVE_ARM_CLZ): Remove block defining it.
>          (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__ and
>          check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
>          (clzdi2): Likewise.
>          (ctzsi2): Likewise.
>          (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
>          __ARM_ARCH_6M__ in guard for checking whether it is defined.
>          (final includes): Test for __only_thumb1__ rather than
>          __ARM_ARCH_6M__ and add comment to indicate the connection between
>          this condition and the one in gcc/config/arm/elf.h.
>          * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>          __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>          * config/arm/t-softfp: Likewise.
>
>
> Updated patch is attached to this mail.

Thanks, the gcc/ changes look ok to me.
The libgcc/ changes look sensible to me too, but I don't know libgcc
very well so there could be something I'm missing.
So please wait for an ok from an arm maintainer on this.

Kyrill

> Best regards,
>
> Thomas
>
> On Thursday 19 May 2016 18:01:16 Kyrill Tkachov wrote:
>> On 19/05/16 17:55, Thomas Preudhomme wrote:
>>> On Thursday 19 May 2016 17:42:26 Kyrill Tkachov wrote:
>>>> Hi Thomas,
>>>>
>>>> I'm not very familiar with the libgcc machinery, but I have a comment on
>>>> an
>>>> arm.h hunk inline.
>>>>
>>>> On 17/05/16 10:58, Thomas Preudhomme wrote:
>>>>> Ping?
>>>>>
>>>>> *** gcc/ChangeLog ***
>>>>>
>>>>> 2015-11-13  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>>
>>>>>            * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and
>>>>>            __ARM_ARCH_ISA_ARM to
>>>>>            decide whether to prevent some libgcc routines being included
>>>>>            for
>>>>>            some
>>>>>            multilibs rather than __ARM_ARCH_6M__ and add comment to
>>>>>            indicate
>>>>>            the
>>>>>            link between this condition and the one in
>>>>>            libgcc/config/arm/lib1func.S.
>>>>>            * config/arm/arm.h (TARGET_ARM_V6M): Add check to
>>>>>            TARGET_ARM_ARCH.
>>>>>            (TARGET_ARM_V7M): Likewise.
>>>>>
>>>>> *** gcc/testsuite/ChangeLog ***
>>>>>
>>>>> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>>
>>>>>            * lib/target-supports.exp
>>>>>            (check_effective_target_arm_cortex_m):
>>>>>            Use
>>>>>            __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>>>>>
>>>>> *** libgcc/ChangeLog ***
>>>>>
>>>>> 2015-12-17  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>>>>>
>>>>>            * config/arm/bpabi-v6m.S: Fix header comment to mention
>>>>>            Thumb-1
>>>>>            rather
>>>>>            than ARMv6-M.
>>>>>            * config/arm/lib1funcs.S (__prefer_thumb__): Define among
>>>>>            other
>>>>>            cases
>>>>>            for all Thumb-1 only targets.
>>>>>            (__only_thumb1__): Define for all Thumb-1 only targets.
>>>>>            (THUMB_LDIV0): Test for __only_thumb1__ rather than
>>>>>            __ARM_ARCH_6M__.
>>>>>            (EQUIV): Likewise.
>>>>>            (ARM_FUNC_ALIAS): Likewise.
>>>>>            (umodsi3): Add check to __only_thumb1__ to guard the idiv
>>>>>            version.
>>>>>            (modsi3): Likewise.
>>>>>            (HAVE_ARM_CLZ): Remove block defining it.
>>>>>            (clzsi2): Test for __only_thumb1__ rather than __ARM_ARCH_6M__
>>>>>            and
>>>>>            check __ARM_FEATURE_CLZ instead of HAVE_ARM_CLZ.
>>>>>            (clzdi2): Likewise.
>>>>>            (ctzsi2): Likewise.
>>>>>            (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather
>>>>>            than
>>>>>            __ARM_ARCH_6M__ in guard for checking whether it is defined.
>>>>>            (final includes): Test for __only_thumb1__ rather than
>>>>>            __ARM_ARCH_6M__ and add comment to indicate the connection
>>>>>            between
>>>>>            this condition and the one in gcc/config/arm/elf.h.
>>>>>            * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>>>>>            __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>>>>>            * config/arm/t-softfp: Likewise.
>>>>>
>>>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
>>>>> index
>>>>> 5b1a03080d0a00fc1ef6934f6bce552e65230640..7eb11d920944c693700d80bb3fb3f9
>>>>> fe
>>>>> 66611c19 100644
>>>>> --- a/gcc/config/arm/arm.h
>>>>> +++ b/gcc/config/arm/arm.h
>>>>> @@ -2188,8 +2188,10 @@ extern int making_const_table;
>>>>>
>>>>>     #define TARGET_ARM_ARCH	\
>>>>>     
>>>>>       (arm_base_arch)	\
>>>>>
>>>>> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
>>>>> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
>>>>> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M &&
>>>>> !arm_arch_notm
>>>>> \ +			&& !arm_arch_thumb2)
>>>>> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M &&
>>>>> !arm_arch_notm
>>>>> \ +			&& arm_arch_thumb2)
>>>> I think now that you're checking TARGET_ARM_ARCH you don't need
>>>> the "!arm_arch_notm && !arm_arch_thumb2" && "!arm_arch_notm &&
>>>> arm_arch_thumb2".
>>> % git --no-pager grep TARGET_ARM_V.M config/arm
>>> config/arm/arm.h:#define TARGET_ARM_V6M (!arm_arch_notm &&
>>> !arm_arch_thumb2) config/arm/arm.h:#define TARGET_ARM_V7M (!arm_arch_notm
>>> && arm_arch_thumb2)
>>>
>>> What about just removing those? I kept them because I wasn't sure of their
>>> purpose but I think we should just remove them.
>> That's fine with me.
>> Then you'd also want to remove the TARGET_ARM_V8M definition from your
>> second patch that I flagged up in that review.
>>
>> Kyrill
>>
>>> Best regards,
>>>
>>> Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-01-12  9:25 [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Thomas Preud'homme
  2016-05-17  9:58 ` [PATCH, libgcc/ARM 1/6, ping1] " Thomas Preudhomme
@ 2016-06-01  9:01 ` Ramana Radhakrishnan
  2016-06-17 17:22   ` [PATCH, libgcc/ARM 1a/6] " Thomas Preudhomme
  2016-06-27 17:22   ` [PATCH, libgcc/ARM 1/6] " Thomas Preudhomme
  1 sibling, 2 replies; 15+ messages in thread
From: Ramana Radhakrishnan @ 2016-06-01  9:01 UTC (permalink / raw)
  To: Thomas Preud'homme; +Cc: gcc-patches

>
> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> index fd999dd..0d23f39 100644
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -2182,8 +2182,10 @@ extern int making_const_table;
>  #define TARGET_ARM_ARCH        \
>    (arm_base_arch)      \
>
> -#define TARGET_ARM_V6M (!arm_arch_notm && !arm_arch_thumb2)
> -#define TARGET_ARM_V7M (!arm_arch_notm && arm_arch_thumb2)
> +#define TARGET_ARM_V6M (TARGET_ARM_ARCH == BASE_ARCH_6M && !arm_arch_notm \
> +                       && !arm_arch_thumb2)
> +#define TARGET_ARM_V7M (TARGET_ARM_ARCH == BASE_ARCH_7M && !arm_arch_notm \
> +                       && arm_arch_thumb2)
>
>  /* The highest Thumb instruction set version supported by the chip.  */
>  #define TARGET_ARM_ARCH_ISA_THUMB              \
> diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
> index 3795728..579a580 100644
> --- a/gcc/config/arm/elf.h
> +++ b/gcc/config/arm/elf.h
> @@ -148,8 +148,9 @@
>    while (0)
>
>  /* Horrible hack: We want to prevent some libgcc routines being included
> -   for some multilibs.  */
> -#ifndef __ARM_ARCH_6M__
> +   for some multilibs.  The condition should match the one in
> +   libgcc/config/arm/lib1funcs.S.  */
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>  #undef L_fixdfsi
>  #undef L_fixunsdfsi
>  #undef L_truncdfsf2
> diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
> index 4e349e9..3f96826 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -3221,10 +3221,8 @@ proc check_effective_target_arm_cortex_m { } {
>         return 0
>      }
>      return [check_no_compiler_messages arm_cortex_m assembly {
> -       #if !defined(__ARM_ARCH_7M__) \
> -            && !defined (__ARM_ARCH_7EM__) \
> -            && !defined (__ARM_ARCH_6M__)
> -       #error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
> +       #if defined(__ARM_ARCH_ISA_ARM)
> +       #error __ARM_ARCH_ISA_ARM is defined
>         #endif
>         int i;
>      } "-mthumb"]
> diff --git a/libgcc/config/arm/bpabi-v6m.S b/libgcc/config/arm/bpabi-v6m.S
> index a1e1640..9ae0bb8 100644
> --- a/libgcc/config/arm/bpabi-v6m.S
> +++ b/libgcc/config/arm/bpabi-v6m.S
> @@ -1,4 +1,4 @@
> -/* Miscellaneous BPABI functions.  ARMv6M implementation
> +/* Miscellaneous BPABI functions.  Thumb-1 only implementation

This is for Thumb-1 as in v4t, v6m and v8m.baseline like ISA variants
would be a better comment.

>
>     Copyright (C) 2006-2015 Free Software Foundation, Inc.
>     Contributed by CodeSourcery.
> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
> index 252efcb..95a1f80 100644
> --- a/libgcc/config/arm/lib1funcs.S
> +++ b/libgcc/config/arm/lib1funcs.S
> @@ -124,7 +124,7 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
>       && !defined(__thumb2__)           \
>       && (!defined(__THUMB_INTERWORK__) \
>          || defined (__OPTIMIZE_SIZE__) \
> -        || defined(__ARM_ARCH_6M__)))
> +        || !__ARM_ARCH_ISA_ARM))
>  # define __prefer_thumb__
>  #endif

This is ok.

>
> @@ -305,7 +305,7 @@ LSYM(Lend_fde):
>
>  #ifdef __ARM_EABI__
>  .macro THUMB_LDIV0 name signed
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1

I would prefer a separate pre-processor macro that is set when this
condition is true so that we don't have to check this in every single
instance.

#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
#define NOT_ISA_TARGET_32BIT
#endif



>         .ifc \signed, unsigned
>         cmp     r0, #0
>         beq     1f
> @@ -478,7 +478,7 @@ _L__\name:
>
>  #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
>
> -#ifdef __ARM_ARCH_6M__
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  #define EQUIV .thumb_set
>  #else
>  .macro ARM_FUNC_START name sp_section=
> @@ -510,7 +510,7 @@ SYM (__\name):
>  #endif
>  .endm
>
> -#ifndef __ARM_ARCH_6M__
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>  .macro ARM_FUNC_ALIAS new old
>         .globl  SYM (__\new)
>         EQUIV   SYM (__\new), SYM (__\old)
> @@ -1054,7 +1054,7 @@ ARM_FUNC_START aeabi_uidivmod
>  /* ------------------------------------------------------------------------ */
>  #ifdef L_umodsi3
>
> -#ifdef __ARM_ARCH_EXT_IDIV__
> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
>
>         ARM_FUNC_START umodsi3
>
> @@ -1240,7 +1240,7 @@ ARM_FUNC_START aeabi_idivmod
>  /* ------------------------------------------------------------------------ */
>  #ifdef L_modsi3
>
> -#if defined(__ARM_ARCH_EXT_IDIV__)
> +#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
>
>         ARM_FUNC_START modsi3
>
> @@ -1508,14 +1508,8 @@ LSYM(Lover12):
>
>  #endif /* __symbian__ */
>


From here down to ....

> -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
> -    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
> -    || defined(__ARM_ARCH_5TEJ__)
> -#define HAVE_ARM_CLZ 1
> -#endif
> -
>  #ifdef L_clzsi2
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  FUNC_START clzsi2
>         mov     r1, #28
>         mov     r3, #1
> @@ -1544,7 +1538,7 @@ FUNC_START clzsi2
>         FUNC_END clzsi2
>  #else
>  ARM_FUNC_START clzsi2
> -# if defined(HAVE_ARM_CLZ)
> +# if defined(__ARM_FEATURE_CLZ)
>         clz     r0, r0
>         RET
>  # else
> @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
>  .align 2
>  1:
>  .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
> -# endif /* !HAVE_ARM_CLZ */
> +# endif /* !__ARM_FEATURE_CLZ */
>         FUNC_END clzsi2
>  #endif
>  #endif /* L_clzsi2 */
>
>  #ifdef L_clzdi2
> -#if !defined(HAVE_ARM_CLZ)
> +#if !defined(__ARM_FEATURE_CLZ)

here should be it's own little patchlet and can  go in separately.



>
> -# if defined(__ARM_ARCH_6M__)
> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  FUNC_START clzdi2
>         push    {r4, lr}
>  # else
> @@ -1601,14 +1595,14 @@ ARM_FUNC_START clzdi2
>         bl      __clzsi2
>  # endif
>  2:
> -# if defined(__ARM_ARCH_6M__)
> +# if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>         pop     {r4, pc}
>  # else
>         RETLDM  r4
>  # endif
>         FUNC_END clzdi2
>
> -#else /* HAVE_ARM_CLZ */
> +#else /* __ARM_FEATURE_CLZ */
>
>  ARM_FUNC_START clzdi2
>         cmp     xxh, #0
> @@ -1623,7 +1617,7 @@ ARM_FUNC_START clzdi2
>  #endif /* L_clzdi2 */
>
>  #ifdef L_ctzsi2
> -#if defined(__ARM_ARCH_6M__)
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>  FUNC_START ctzsi2
>         neg     r1, r0
>         and     r0, r0, r1
> @@ -1656,7 +1650,7 @@ FUNC_START ctzsi2
>  ARM_FUNC_START ctzsi2
>         rsb     r1, r0, #0
>         and     r0, r0, r1


> -# if defined(HAVE_ARM_CLZ)
> +# if defined(__ARM_FEATURE_CLZ)
>         clz     r0, r0
>         rsb     r0, r0, #31
>         RET

This can go with the other __ARM_FEATURE_CLZ hunk too.

> @@ -1681,7 +1675,7 @@ ARM_FUNC_START ctzsi2
>  .align 2
>  1:
>  .byte  27, 28, 29, 29, 30, 30, 30, 30, 31, 31, 31, 31, 31, 31, 31, 31
> -# endif /* !HAVE_ARM_CLZ */
> +# endif /* !__ARM_FEATURE_CLZ */
>         FUNC_END ctzsi2
>  #endif
>  #endif /* L_clzsi2 */
> @@ -1738,7 +1732,7 @@ ARM_FUNC_START ctzsi2
>
>  /* Don't bother with the old interworking routines for Thumb-2.  */
>  /* ??? Maybe only omit these on "m" variants.  */
> -#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
> +#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
>
>  #if defined L_interwork_call_via_rX
>
> @@ -1983,11 +1977,12 @@ LSYM(Lchange_\register):
>  .endm
>
>  #ifndef __symbian__
> -#ifndef __ARM_ARCH_6M__
> +/* The condition here must match the one in gcc/config/arm/elf.h.  */
> +#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
>  #include "ieee754-df.S"
>  #include "ieee754-sf.S"
>  #include "bpabi.S"
> -#else /* __ARM_ARCH_6M__ */
> +#else /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>  #include "bpabi-v6m.S"
> -#endif /* __ARM_ARCH_6M__ */
> +#endif /* !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1 */
>  #endif /* !__symbian__ */
> diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
> index cac1022..393ec8a 100644
> --- a/libgcc/config/arm/libunwind.S
> +++ b/libgcc/config/arm/libunwind.S
> @@ -58,7 +58,7 @@
>  #endif
>  #endif
>
> -#ifdef __ARM_ARCH_6M__
> +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>
>  /* r0 points to a 16-word block.  Upload these values to the actual core
>     state.  */
> @@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
>         UNPREFIX \name
>  .endm
>
> -#else /* !__ARM_ARCH_6M__ */
> +#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
>
>  /* r0 points to a 16-word block.  Upload these values to the actual core
>     state.  */
> @@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
>         UNPREFIX \name
>  .endm
>
> -#endif /* !__ARM_ARCH_6M__ */
> +#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
>
>  UNWIND_WRAPPER _Unwind_RaiseException 1
>  UNWIND_WRAPPER _Unwind_Resume 1
> diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
> index 4ede438..554ec9b 100644
> --- a/libgcc/config/arm/t-softfp
> +++ b/libgcc/config/arm/t-softfp
> @@ -1,2 +1,2 @@
> -softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
> +softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1'
>  softfp_wrap_end := '\#endif'
>
>
> Tested by building libgcc and comparing it before and
> after with objdump -D and readelf -A for all permutations of:
>
>   Architectures:
>     armv4 armv4t armv5 armv5t armv5te armv6 armv6j armv6k
>     armv6z armv6kz armv6t2 armv6s-m armv7 armv7-a armv7ve
>     armv7-r armv7-m armv7e-m armv8-a armv8-a+crc iwmmxt
>     iwmmxt2
>
>   ISAs:
>     thumb arm
>
>   Optimization Levels:
>     Os O2
>
>   Excluding:
>     armv6s-m -marm
>     armv7-m -marm
>     armv7e-m -marm
>     iwmmxt -mthumb
>     iwmmxt2 -mthumb
>
> as being rejected by the compiler or assembler. ARMv6-m was not tested because compilation fails due to svc instruction.

Please fix up the macros, post back and redo the test. Otherwise this
is ok from a quick read.

Thanks for your patience and sorry about the delay in reviewing.

regards
Ramana

>
> Best regards,
>
> Thomas
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1a/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-06-01  9:01 ` [PATCH, libgcc/ARM 1/6] " Ramana Radhakrishnan
@ 2016-06-17 17:22   ` Thomas Preudhomme
  2016-06-27 16:26     ` [PATCH, libgcc/ARM 1a/6, ping] " Thomas Preudhomme
  2016-07-06 16:21     ` [PATCH, libgcc/ARM 1a/6] " Ramana Radhakrishnan
  2016-06-27 17:22   ` [PATCH, libgcc/ARM 1/6] " Thomas Preudhomme
  1 sibling, 2 replies; 15+ messages in thread
From: Thomas Preudhomme @ 2016-06-17 17:22 UTC (permalink / raw)
  To: Ramana Radhakrishnan; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2713 bytes --]

On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote:
> Please fix up the macros, post back and redo the test. Otherwise this
> is ok from a quick read.

What about the updated patch in attachment? As for the original patch, I've 
checked that code generation does not change for a number of combinations of 
ISAs (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, 
armv4t, armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, 
armv6t2, armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve, 
armv8-a, armv8-a+crc, iwmmxt and iwmmxt2).

Note, I renumbered this patch 1a to not make the numbering of other patches 
look strange. The CLZ part is now in patch 1b/7.

ChangeLog entries are now as follow:


*** gcc/ChangeLog ***

2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
        decide whether to prevent some libgcc routines being included for some
        multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
        link between this condition and the one in
        libgcc/config/arm/lib1func.S.


*** gcc/testsuite/ChangeLog ***

2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
        __ARM_ARCH_ISA_ARM to test for Cortex-M devices.


*** libgcc/ChangeLog ***

2016-06-01  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/bpabi-v6m.S: Clarify what architectures is the
        implementation suitable for.
        * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
        for all Thumb-1 only targets.
        (NOT_ISA_TARGET_32BIT): Define for Thumb-1 only targets.
        (THUMB_LDIV0): Test for NOT_ISA_TARGET_32BIT rather than
        __ARM_ARCH_6M__.
        (EQUIV): Likewise.
        (ARM_FUNC_ALIAS): Likewise.
        (umodsi3): Add check to __ARM_ARCH_ISA_THUMB != 1 to guard the idiv
        version.
        (modsi3): Likewise.
        (clzsi2): Test for NOT_ISA_TARGET_32BIT rather than __ARM_ARCH_6M__.
        (clzdi2): Likewise.
        (ctzsi2): Likewise.
        (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
        __ARM_ARCH_6M__ in guard for checking whether it is defined.
        (final includes): Test for NOT_ISA_TARGET_32BIT rather than
        __ARM_ARCH_6M__ and add comment to indicate the connection between
        this condition and the one in gcc/config/arm/elf.h.
        * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
        __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
        * config/arm/t-softfp: Likewise.


Best regards,

Thomas

[-- Attachment #2: remove_thumb1_v6m_assumptions.diff --]
[-- Type: text/x-patch, Size: 6642 bytes --]

diff --git a/gcc/config/arm/elf.h b/gcc/config/arm/elf.h
index 77f30554d5286bd83aeab0c8dc308cfd44e732dc..246de5492665ba2a0292736a9c53fbaaef184d72 100644
--- a/gcc/config/arm/elf.h
+++ b/gcc/config/arm/elf.h
@@ -148,8 +148,9 @@
   while (0)
 
 /* Horrible hack: We want to prevent some libgcc routines being included
-   for some multilibs.  */
-#ifndef __ARM_ARCH_6M__
+   for some multilibs.  The condition should match the one in
+   libgcc/config/arm/lib1funcs.S.  */
+#if __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1
 #undef L_fixdfsi
 #undef L_fixunsdfsi
 #undef L_truncdfsf2
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 04ca17656f2f26dda710e8a0f9ca77dd963ab39b..38151375c29cd007f1cc34ead3aa495606224061 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -3320,10 +3320,8 @@ proc check_effective_target_arm_cortex_m { } {
 	return 0
     }
     return [check_no_compiler_messages arm_cortex_m assembly {
-	#if !defined(__ARM_ARCH_7M__) \
-            && !defined (__ARM_ARCH_7EM__) \
-            && !defined (__ARM_ARCH_6M__)
-	#error !__ARM_ARCH_7M__ && !__ARM_ARCH_7EM__ && !__ARM_ARCH_6M__
+	#if defined(__ARM_ARCH_ISA_ARM)
+	#error __ARM_ARCH_ISA_ARM is defined
 	#endif
 	int i;
     } "-mthumb"]
diff --git a/libgcc/config/arm/bpabi-v6m.S b/libgcc/config/arm/bpabi-v6m.S
index 5d35aa6afca224613c94cf923f8a2ee8dac949f2..27f33a4e8ced2cb2da8e38f5d78501954ee7363b 100644
--- a/libgcc/config/arm/bpabi-v6m.S
+++ b/libgcc/config/arm/bpabi-v6m.S
@@ -1,4 +1,5 @@
-/* Miscellaneous BPABI functions.  ARMv6M implementation
+/* Miscellaneous BPABI functions.  Thumb-1 implementation, suitable for ARMv4T,
+   ARMv6-M and ARMv8-M Baseline like ISA variants.
 
    Copyright (C) 2006-2016 Free Software Foundation, Inc.
    Contributed by CodeSourcery.
diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index 375a5135110895faa44267ebee045fd315515027..951dcda1c3bf7f323423a3e2813bdf0501653016 100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -124,10 +124,14 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
      && !defined(__thumb2__)		\
      && (!defined(__THUMB_INTERWORK__)	\
 	 || defined (__OPTIMIZE_SIZE__)	\
-	 || defined(__ARM_ARCH_6M__)))
+	 || !__ARM_ARCH_ISA_ARM))
 # define __prefer_thumb__
 #endif
 
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
+#define NOT_ISA_TARGET_32BIT 1
+#endif
+
 /* How to return from a function call depends on the architecture variant.  */
 
 #if (__ARM_ARCH__ > 4) || defined(__ARM_ARCH_4T__)
@@ -305,7 +309,7 @@ LSYM(Lend_fde):
 
 #ifdef __ARM_EABI__
 .macro THUMB_LDIV0 name signed
-#if defined(__ARM_ARCH_6M__)
+#ifdef NOT_ISA_TARGET_32BIT
 	.ifc \signed, unsigned
 	cmp	r0, #0
 	beq	1f
@@ -478,7 +482,7 @@ _L__\name:
 
 #else /* !(__INTERWORKING_STUBS__ || __thumb2__) */
 
-#ifdef __ARM_ARCH_6M__
+#ifdef NOT_ISA_TARGET_32BIT
 #define EQUIV .thumb_set
 #else
 .macro	ARM_FUNC_START name sp_section=
@@ -510,7 +514,7 @@ SYM (__\name):
 #endif
 .endm
 
-#ifndef __ARM_ARCH_6M__
+#ifndef NOT_ISA_TARGET_32BIT
 .macro	ARM_FUNC_ALIAS new old
 	.globl	SYM (__\new)
 	EQUIV	SYM (__\new), SYM (__\old)
@@ -1054,7 +1058,7 @@ ARM_FUNC_START aeabi_uidivmod
 /* ------------------------------------------------------------------------ */
 #ifdef L_umodsi3
 
-#ifdef __ARM_ARCH_EXT_IDIV__
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START umodsi3
 
@@ -1240,7 +1244,7 @@ ARM_FUNC_START aeabi_idivmod
 /* ------------------------------------------------------------------------ */
 #ifdef L_modsi3
 
-#if defined(__ARM_ARCH_EXT_IDIV__)
+#if defined(__ARM_ARCH_EXT_IDIV__) && __ARM_ARCH_ISA_THUMB != 1
 
 	ARM_FUNC_START modsi3
 
@@ -1515,7 +1519,7 @@ LSYM(Lover12):
 #endif
 
 #ifdef L_clzsi2
-#if defined(__ARM_ARCH_6M__)
+#ifdef NOT_ISA_TARGET_32BIT
 FUNC_START clzsi2
 	mov	r1, #28
 	mov	r3, #1
@@ -1576,7 +1580,7 @@ ARM_FUNC_START clzsi2
 #ifdef L_clzdi2
 #if !defined(HAVE_ARM_CLZ)
 
-# if defined(__ARM_ARCH_6M__)
+# ifdef NOT_ISA_TARGET_32BIT
 FUNC_START clzdi2
 	push	{r4, lr}
 # else
@@ -1601,7 +1605,7 @@ ARM_FUNC_START clzdi2
 	bl	__clzsi2
 # endif
 2:
-# if defined(__ARM_ARCH_6M__)
+# ifdef NOT_ISA_TARGET_32BIT
 	pop	{r4, pc}
 # else
 	RETLDM	r4
@@ -1623,7 +1627,7 @@ ARM_FUNC_START clzdi2
 #endif /* L_clzdi2 */
 
 #ifdef L_ctzsi2
-#if defined(__ARM_ARCH_6M__)
+#ifdef NOT_ISA_TARGET_32BIT
 FUNC_START ctzsi2
 	neg	r1, r0
 	and	r0, r0, r1
@@ -1738,7 +1742,7 @@ ARM_FUNC_START ctzsi2
 
 /* Don't bother with the old interworking routines for Thumb-2.  */
 /* ??? Maybe only omit these on "m" variants.  */
-#if !defined(__thumb2__) && !defined(__ARM_ARCH_6M__)
+#if !defined(__thumb2__) && __ARM_ARCH_ISA_ARM
 
 #if defined L_interwork_call_via_rX
 
@@ -1983,11 +1987,12 @@ LSYM(Lchange_\register):
 .endm
 
 #ifndef __symbian__
-#ifndef __ARM_ARCH_6M__
+/* The condition here must match the one in gcc/config/arm/elf.h.  */
+#ifndef NOT_ISA_TARGET_32BIT
 #include "ieee754-df.S"
 #include "ieee754-sf.S"
 #include "bpabi.S"
-#else /* __ARM_ARCH_6M__ */
+#else /* NOT_ISA_TARGET_32BIT */
 #include "bpabi-v6m.S"
-#endif /* __ARM_ARCH_6M__ */
+#endif /* NOT_ISA_TARGET_32BIT */
 #endif /* !__symbian__ */
diff --git a/libgcc/config/arm/libunwind.S b/libgcc/config/arm/libunwind.S
index a68b10ddce93d230c504f3747dcad3832d9f753c..3d7e70181fa80fe53a4903c96bb0f90480feee21 100644
--- a/libgcc/config/arm/libunwind.S
+++ b/libgcc/config/arm/libunwind.S
@@ -58,7 +58,7 @@
 #endif
 #endif
 
-#ifdef __ARM_ARCH_6M__
+#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -169,7 +169,7 @@ FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#else /* !__ARM_ARCH_6M__ */
+#else /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 /* r0 points to a 16-word block.  Upload these values to the actual core
    state.  */
@@ -351,7 +351,7 @@ ARM_FUNC_START gnu_Unwind_Save_WMMXC
 	UNPREFIX \name
 .endm
 
-#endif /* !__ARM_ARCH_6M__ */
+#endif /* __ARM_ARCH_ISA_ARM || __ARM_ARCH_ISA_THUMB != 1 */
 
 UNWIND_WRAPPER _Unwind_RaiseException 1
 UNWIND_WRAPPER _Unwind_Resume 1
diff --git a/libgcc/config/arm/t-softfp b/libgcc/config/arm/t-softfp
index 4ede438baf6a297737e52db00395f6c3a359f681..554ec9bc47b04445e79e84b1f957bf88680c08d1 100644
--- a/libgcc/config/arm/t-softfp
+++ b/libgcc/config/arm/t-softfp
@@ -1,2 +1,2 @@
-softfp_wrap_start := '\#ifdef __ARM_ARCH_6M__'
+softfp_wrap_start := '\#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1'
 softfp_wrap_end := '\#endif'

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1a/6, ping] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-06-17 17:22   ` [PATCH, libgcc/ARM 1a/6] " Thomas Preudhomme
@ 2016-06-27 16:26     ` Thomas Preudhomme
  2016-07-04 10:26       ` [PATCH, libgcc/ARM 1a/6, ping2] " Thomas Preudhomme
  2016-07-06 16:21     ` [PATCH, libgcc/ARM 1a/6] " Ramana Radhakrishnan
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Preudhomme @ 2016-06-27 16:26 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ramana Radhakrishnan

Ping?

Best regards,

Thomas

On Friday 17 June 2016 18:21:44 Thomas Preudhomme wrote:
> On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote:
> > Please fix up the macros, post back and redo the test. Otherwise this
> > is ok from a quick read.
> 
> What about the updated patch in attachment? As for the original patch, I've
> checked that code generation does not change for a number of combinations of
> ISAs (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4,
> armv4t, armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz,
> armv6t2, armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r,
> armv7ve, armv8-a, armv8-a+crc, iwmmxt and iwmmxt2).
> 
> Note, I renumbered this patch 1a to not make the numbering of other patches
> look strange. The CLZ part is now in patch 1b/7.
> 
> ChangeLog entries are now as follow:
> 
> 
> *** gcc/ChangeLog ***
> 
> 2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM
> to decide whether to prevent some libgcc routines being included for some
> multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the link
> between this condition and the one in
>         libgcc/config/arm/lib1func.S.
> 
> 
> *** gcc/testsuite/ChangeLog ***
> 
> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
> __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
> 
> 
> *** libgcc/ChangeLog ***
> 
> 2016-06-01  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/arm/bpabi-v6m.S: Clarify what architectures is the
>         implementation suitable for.
>         * config/arm/lib1funcs.S (__prefer_thumb__): Define among other
> cases for all Thumb-1 only targets.
>         (NOT_ISA_TARGET_32BIT): Define for Thumb-1 only targets.
>         (THUMB_LDIV0): Test for NOT_ISA_TARGET_32BIT rather than
>         __ARM_ARCH_6M__.
>         (EQUIV): Likewise.
>         (ARM_FUNC_ALIAS): Likewise.
>         (umodsi3): Add check to __ARM_ARCH_ISA_THUMB != 1 to guard the idiv
>         version.
>         (modsi3): Likewise.
>         (clzsi2): Test for NOT_ISA_TARGET_32BIT rather than __ARM_ARCH_6M__.
> (clzdi2): Likewise.
>         (ctzsi2): Likewise.
>         (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
>         __ARM_ARCH_6M__ in guard for checking whether it is defined.
>         (final includes): Test for NOT_ISA_TARGET_32BIT rather than
>         __ARM_ARCH_6M__ and add comment to indicate the connection between
>         this condition and the one in gcc/config/arm/elf.h.
>         * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>         __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>         * config/arm/t-softfp: Likewise.
> 
> 
> Best regards,
> 
> Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-06-01  9:01 ` [PATCH, libgcc/ARM 1/6] " Ramana Radhakrishnan
  2016-06-17 17:22   ` [PATCH, libgcc/ARM 1a/6] " Thomas Preudhomme
@ 2016-06-27 17:22   ` Thomas Preudhomme
  2016-07-05  9:30     ` [PATCH, libgcc/ARM 1b/7] Check CLZ availability with ISA support and architecture level macros Thomas Preudhomme
  2016-07-06 16:20     ` [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Ramana Radhakrishnan
  1 sibling, 2 replies; 15+ messages in thread
From: Thomas Preudhomme @ 2016-06-27 17:22 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ramana Radhakrishnan

[-- Attachment #1: Type: text/plain, Size: 2935 bytes --]

Hi Ramana,

On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote:
> 
> From here down to ....
> 
> > -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
> > -    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
> > -    || defined(__ARM_ARCH_5TEJ__)
> > -#define HAVE_ARM_CLZ 1
> > -#endif
> > -
> > 
> >  #ifdef L_clzsi2
> > 
> > -#if defined(__ARM_ARCH_6M__)
> > +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
> > 
> >  FUNC_START clzsi2
> >  
> >         mov     r1, #28
> >         mov     r3, #1
> > 
> > @@ -1544,7 +1538,7 @@ FUNC_START clzsi2
> > 
> >         FUNC_END clzsi2
> >  
> >  #else
> >  ARM_FUNC_START clzsi2
> > 
> > -# if defined(HAVE_ARM_CLZ)
> > +# if defined(__ARM_FEATURE_CLZ)
> > 
> >         clz     r0, r0
> >         RET
> >  
> >  # else
> > 
> > @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
> > 
> >  .align 2
> >  1:
> >  .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
> > 
> > -# endif /* !HAVE_ARM_CLZ */
> > +# endif /* !__ARM_FEATURE_CLZ */
> > 
> >         FUNC_END clzsi2
> >  
> >  #endif
> >  #endif /* L_clzsi2 */
> >  
> >  #ifdef L_clzdi2
> > 
> > -#if !defined(HAVE_ARM_CLZ)
> > +#if !defined(__ARM_FEATURE_CLZ)
> 
> here should be it's own little patchlet and can  go in separately.

The patch in attachment changes the CLZ availability check in libgcc to test 
ISA supported and architecture version rather than encode a specific list of 
architectures. __ARM_FEATURE_CLZ is not used because its value depends on what 
mode the user is targeting but only the architecture support matters in this 
case. Indeed, the code using CLZ is written in assembler and uses mnemonics 
available both in ARM and Thumb mode so only CLZ availability in one of the 
mode matters.

This change was split out from [PATCH, GCC, ARM 1/7] Fix Thumb-1 only == 
ARMv6-M & Thumb-2 only == ARMv7-M assumptions.

ChangeLog entry is as follows:

*** libgcc/ChangeLog ***

2016-06-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>

        * config/arm/lib1funcs.S (HAVE_ARM_CLZ): Define for ARMv6* or later
        and ARMv5t* rather than for a fixed list of architectures.

Looking for code generation change accross a number of combinations of ISAs 
(ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, armv4t, 
armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, armv6t2, 
armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve, armv8-a, 
armv8-a+crc, iwmmxt and iwmmxt2) shows that only ARMv5T is impacted (uses CLZ 
now). This is expected because currently HAVE_ARM_CLZ is not defined for this 
architecture while the ARMv7-a/ARMv7-R Architecture Reference Manual [1] 
states that all ARMv5T* architectures have CLZ. ARMv5E should also be impacted 
(not using CLZ anymore) but testing it is difficult since current binutils does 
not support ARMv5E.

[1] Document ARM DDI0406C in http://infocenter.arm.com

Best regards,

Thomas

[-- Attachment #2: isa_clz_check.patch --]
[-- Type: text/x-patch, Size: 649 bytes --]

diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
index 951dcda1c3bf7f323423a3e2813bdf0501653016..c4f061f8196d243159903cac4eb0291d1bf0b1ad 100644
--- a/libgcc/config/arm/lib1funcs.S
+++ b/libgcc/config/arm/lib1funcs.S
@@ -1512,9 +1512,10 @@ LSYM(Lover12):
 
 #endif /* __symbian__ */
 
-#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
-    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
-    || defined(__ARM_ARCH_5TEJ__)
+#if (__ARM_ARCH_ISA_THUMB == 2	\
+     || (__ARM_ARCH_ISA_ARM	\
+	 && (__ARM_ARCH__ > 5	\
+	     || (__ARM_ARCH__ == 5 && __ARM_ARCH_ISA_THUMB))))
 #define HAVE_ARM_CLZ 1
 #endif
 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1a/6, ping2] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-06-27 16:26     ` [PATCH, libgcc/ARM 1a/6, ping] " Thomas Preudhomme
@ 2016-07-04 10:26       ` Thomas Preudhomme
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Preudhomme @ 2016-07-04 10:26 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ramana Radhakrishnan

Ping?

Best regards,

Thomas

On Monday 27 June 2016 16:52:50 Thomas Preudhomme wrote:
> Ping?
> 
> Best regards,
> 
> Thomas
> 
> On Friday 17 June 2016 18:21:44 Thomas Preudhomme wrote:
> > On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote:
> > > Please fix up the macros, post back and redo the test. Otherwise this
> > > is ok from a quick read.
> > 
> > What about the updated patch in attachment? As for the original patch,
> > I've
> > checked that code generation does not change for a number of combinations
> > of ISAs (ARM/Thumb), optimization levels (Os/O2), and architectures
> > (armv4, armv4t, armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m,
> > armv6kz, armv6t2, armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m,
> > armv7-r, armv7ve, armv8-a, armv8-a+crc, iwmmxt and iwmmxt2).
> > 
> > Note, I renumbered this patch 1a to not make the numbering of other
> > patches
> > look strange. The CLZ part is now in patch 1b/7.
> > 
> > ChangeLog entries are now as follow:
> > 
> > 
> > *** gcc/ChangeLog ***
> > 
> > 2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >         * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and
> >         __ARM_ARCH_ISA_ARM
> > 
> > to decide whether to prevent some libgcc routines being included for some
> > multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the link
> > between this condition and the one in
> > 
> >         libgcc/config/arm/lib1func.S.
> > 
> > *** gcc/testsuite/ChangeLog ***
> > 
> > 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >         * lib/target-supports.exp (check_effective_target_arm_cortex_m):
> >         Use
> > 
> > __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
> > 
> > 
> > *** libgcc/ChangeLog ***
> > 
> > 2016-06-01  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> > 
> >         * config/arm/bpabi-v6m.S: Clarify what architectures is the
> >         implementation suitable for.
> >         * config/arm/lib1funcs.S (__prefer_thumb__): Define among other
> > 
> > cases for all Thumb-1 only targets.
> > 
> >         (NOT_ISA_TARGET_32BIT): Define for Thumb-1 only targets.
> >         (THUMB_LDIV0): Test for NOT_ISA_TARGET_32BIT rather than
> >         __ARM_ARCH_6M__.
> >         (EQUIV): Likewise.
> >         (ARM_FUNC_ALIAS): Likewise.
> >         (umodsi3): Add check to __ARM_ARCH_ISA_THUMB != 1 to guard the
> >         idiv
> >         version.
> >         (modsi3): Likewise.
> >         (clzsi2): Test for NOT_ISA_TARGET_32BIT rather than
> >         __ARM_ARCH_6M__.
> > 
> > (clzdi2): Likewise.
> > 
> >         (ctzsi2): Likewise.
> >         (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
> >         __ARM_ARCH_6M__ in guard for checking whether it is defined.
> >         (final includes): Test for NOT_ISA_TARGET_32BIT rather than
> >         __ARM_ARCH_6M__ and add comment to indicate the connection between
> >         this condition and the one in gcc/config/arm/elf.h.
> >         * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
> >         __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
> >         * config/arm/t-softfp: Likewise.
> > 
> > Best regards,
> > 
> > Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1b/7] Check CLZ availability with ISA support and architecture level macros
  2016-06-27 17:22   ` [PATCH, libgcc/ARM 1/6] " Thomas Preudhomme
@ 2016-07-05  9:30     ` Thomas Preudhomme
  2016-07-06 16:20     ` [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Ramana Radhakrishnan
  1 sibling, 0 replies; 15+ messages in thread
From: Thomas Preudhomme @ 2016-07-05  9:30 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ramana Radhakrishnan

[Fixed subject to reflect patch]

Ping?

Best regards,

Thomas

On Monday 27 June 2016 17:51:34 Thomas Preudhomme wrote:
> Hi Ramana,
> 
> On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote:
> > From here down to ....
> > 
> > > -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
> > > -    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
> > > -    || defined(__ARM_ARCH_5TEJ__)
> > > -#define HAVE_ARM_CLZ 1
> > > -#endif
> > > -
> > > 
> > >  #ifdef L_clzsi2
> > > 
> > > -#if defined(__ARM_ARCH_6M__)
> > > +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
> > > 
> > >  FUNC_START clzsi2
> > >  
> > >         mov     r1, #28
> > >         mov     r3, #1
> > > 
> > > @@ -1544,7 +1538,7 @@ FUNC_START clzsi2
> > > 
> > >         FUNC_END clzsi2
> > >  
> > >  #else
> > >  ARM_FUNC_START clzsi2
> > > 
> > > -# if defined(HAVE_ARM_CLZ)
> > > +# if defined(__ARM_FEATURE_CLZ)
> > > 
> > >         clz     r0, r0
> > >         RET
> > >  
> > >  # else
> > > 
> > > @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
> > > 
> > >  .align 2
> > >  1:
> > >  .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
> > > 
> > > -# endif /* !HAVE_ARM_CLZ */
> > > +# endif /* !__ARM_FEATURE_CLZ */
> > > 
> > >         FUNC_END clzsi2
> > >  
> > >  #endif
> > >  #endif /* L_clzsi2 */
> > >  
> > >  #ifdef L_clzdi2
> > > 
> > > -#if !defined(HAVE_ARM_CLZ)
> > > +#if !defined(__ARM_FEATURE_CLZ)
> > 
> > here should be it's own little patchlet and can  go in separately.
> 
> The patch in attachment changes the CLZ availability check in libgcc to test
> ISA supported and architecture version rather than encode a specific list
> of architectures. __ARM_FEATURE_CLZ is not used because its value depends
> on what mode the user is targeting but only the architecture support
> matters in this case. Indeed, the code using CLZ is written in assembler
> and uses mnemonics available both in ARM and Thumb mode so only CLZ
> availability in one of the mode matters.
> 
> This change was split out from [PATCH, GCC, ARM 1/7] Fix Thumb-1 only ==
> ARMv6-M & Thumb-2 only == ARMv7-M assumptions.
> 
> ChangeLog entry is as follows:
> 
> *** libgcc/ChangeLog ***
> 
> 2016-06-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
> 
>         * config/arm/lib1funcs.S (HAVE_ARM_CLZ): Define for ARMv6* or later
>         and ARMv5t* rather than for a fixed list of architectures.
> 
> Looking for code generation change accross a number of combinations of ISAs
> (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, armv4t,
> armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, armv6t2,
> armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve,
> armv8-a, armv8-a+crc, iwmmxt and iwmmxt2) shows that only ARMv5T is
> impacted (uses CLZ now). This is expected because currently HAVE_ARM_CLZ is
> not defined for this architecture while the ARMv7-a/ARMv7-R Architecture
> Reference Manual [1] states that all ARMv5T* architectures have CLZ. ARMv5E
> should also be impacted (not using CLZ anymore) but testing it is difficult
> since current binutils does not support ARMv5E.
> 
> [1] Document ARM DDI0406C in http://infocenter.arm.com
> 
> Best regards,
> 
> Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-06-27 17:22   ` [PATCH, libgcc/ARM 1/6] " Thomas Preudhomme
  2016-07-05  9:30     ` [PATCH, libgcc/ARM 1b/7] Check CLZ availability with ISA support and architecture level macros Thomas Preudhomme
@ 2016-07-06 16:20     ` Ramana Radhakrishnan
  1 sibling, 0 replies; 15+ messages in thread
From: Ramana Radhakrishnan @ 2016-07-06 16:20 UTC (permalink / raw)
  To: Thomas Preudhomme; +Cc: gcc-patches

On Mon, Jun 27, 2016 at 5:51 PM, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> Hi Ramana,
>
> On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote:
>>
>> From here down to ....
>>
>> > -#if ((__ARM_ARCH__ > 5) && !defined(__ARM_ARCH_6M__)) \
>> > -    || defined(__ARM_ARCH_5E__) || defined(__ARM_ARCH_5TE__) \
>> > -    || defined(__ARM_ARCH_5TEJ__)
>> > -#define HAVE_ARM_CLZ 1
>> > -#endif
>> > -
>> >
>> >  #ifdef L_clzsi2
>> >
>> > -#if defined(__ARM_ARCH_6M__)
>> > +#if !__ARM_ARCH_ISA_ARM && __ARM_ARCH_ISA_THUMB == 1
>> >
>> >  FUNC_START clzsi2
>> >
>> >         mov     r1, #28
>> >         mov     r3, #1
>> >
>> > @@ -1544,7 +1538,7 @@ FUNC_START clzsi2
>> >
>> >         FUNC_END clzsi2
>> >
>> >  #else
>> >  ARM_FUNC_START clzsi2
>> >
>> > -# if defined(HAVE_ARM_CLZ)
>> > +# if defined(__ARM_FEATURE_CLZ)
>> >
>> >         clz     r0, r0
>> >         RET
>> >
>> >  # else
>> >
>> > @@ -1568,15 +1562,15 @@ ARM_FUNC_START clzsi2
>> >
>> >  .align 2
>> >  1:
>> >  .byte 4, 3, 2, 2, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0
>> >
>> > -# endif /* !HAVE_ARM_CLZ */
>> > +# endif /* !__ARM_FEATURE_CLZ */
>> >
>> >         FUNC_END clzsi2
>> >
>> >  #endif
>> >  #endif /* L_clzsi2 */
>> >
>> >  #ifdef L_clzdi2
>> >
>> > -#if !defined(HAVE_ARM_CLZ)
>> > +#if !defined(__ARM_FEATURE_CLZ)
>>
>> here should be it's own little patchlet and can  go in separately.
>
> The patch in attachment changes the CLZ availability check in libgcc to test
> ISA supported and architecture version rather than encode a specific list of
> architectures. __ARM_FEATURE_CLZ is not used because its value depends on what
> mode the user is targeting but only the architecture support matters in this
> case. Indeed, the code using CLZ is written in assembler and uses mnemonics
> available both in ARM and Thumb mode so only CLZ availability in one of the
> mode matters.
>
> This change was split out from [PATCH, GCC, ARM 1/7] Fix Thumb-1 only ==
> ARMv6-M & Thumb-2 only == ARMv7-M assumptions.
>
> ChangeLog entry is as follows:
>
> *** libgcc/ChangeLog ***
>
> 2016-06-16  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * config/arm/lib1funcs.S (HAVE_ARM_CLZ): Define for ARMv6* or later
>         and ARMv5t* rather than for a fixed list of architectures.
>
> Looking for code generation change accross a number of combinations of ISAs
> (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4, armv4t,
> armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz, armv6t2,
> armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve, armv8-a,
> armv8-a+crc, iwmmxt and iwmmxt2) shows that only ARMv5T is impacted (uses CLZ
> now). This is expected because currently HAVE_ARM_CLZ is not defined for this
> architecture while the ARMv7-a/ARMv7-R Architecture Reference Manual [1]
> states that all ARMv5T* architectures have CLZ. ARMv5E should also be impacted
> (not using CLZ anymore) but testing it is difficult since current binutils does
> not support ARMv5E.
>
> [1] Document ARM DDI0406C in http://infocenter.arm.com
>
> Best regards,
>
> Thomas



OK.

Ramana

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH, libgcc/ARM 1a/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions
  2016-06-17 17:22   ` [PATCH, libgcc/ARM 1a/6] " Thomas Preudhomme
  2016-06-27 16:26     ` [PATCH, libgcc/ARM 1a/6, ping] " Thomas Preudhomme
@ 2016-07-06 16:21     ` Ramana Radhakrishnan
  1 sibling, 0 replies; 15+ messages in thread
From: Ramana Radhakrishnan @ 2016-07-06 16:21 UTC (permalink / raw)
  To: Thomas Preudhomme; +Cc: gcc-patches

On Fri, Jun 17, 2016 at 6:21 PM, Thomas Preudhomme
<thomas.preudhomme@foss.arm.com> wrote:
> On Wednesday 01 June 2016 10:00:52 Ramana Radhakrishnan wrote:
>> Please fix up the macros, post back and redo the test. Otherwise this
>> is ok from a quick read.
>
> What about the updated patch in attachment? As for the original patch, I've
> checked that code generation does not change for a number of combinations of
> ISAs (ARM/Thumb), optimization levels (Os/O2), and architectures (armv4,
> armv4t, armv5, armv5t, armv5te, armv6, armv6j, armv6k, armv6s-m, armv6kz,
> armv6t2, armv6z, armv6zk, armv7, armv7-a, armv7e-m, armv7-m, armv7-r, armv7ve,
> armv8-a, armv8-a+crc, iwmmxt and iwmmxt2).
>
> Note, I renumbered this patch 1a to not make the numbering of other patches
> look strange. The CLZ part is now in patch 1b/7.
>
> ChangeLog entries are now as follow:
>
>
> *** gcc/ChangeLog ***
>
> 2016-05-23  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * config/arm/elf.h: Use __ARM_ARCH_ISA_THUMB and __ARM_ARCH_ISA_ARM to
>         decide whether to prevent some libgcc routines being included for some
>         multilibs rather than __ARM_ARCH_6M__ and add comment to indicate the
>         link between this condition and the one in
>         libgcc/config/arm/lib1func.S.
>
>
> *** gcc/testsuite/ChangeLog ***
>
> 2015-11-10  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * lib/target-supports.exp (check_effective_target_arm_cortex_m): Use
>         __ARM_ARCH_ISA_ARM to test for Cortex-M devices.
>
>
> *** libgcc/ChangeLog ***
>
> 2016-06-01  Thomas Preud'homme  <thomas.preudhomme@arm.com>
>
>         * config/arm/bpabi-v6m.S: Clarify what architectures is the
>         implementation suitable for.
>         * config/arm/lib1funcs.S (__prefer_thumb__): Define among other cases
>         for all Thumb-1 only targets.
>         (NOT_ISA_TARGET_32BIT): Define for Thumb-1 only targets.
>         (THUMB_LDIV0): Test for NOT_ISA_TARGET_32BIT rather than
>         __ARM_ARCH_6M__.
>         (EQUIV): Likewise.
>         (ARM_FUNC_ALIAS): Likewise.
>         (umodsi3): Add check to __ARM_ARCH_ISA_THUMB != 1 to guard the idiv
>         version.
>         (modsi3): Likewise.
>         (clzsi2): Test for NOT_ISA_TARGET_32BIT rather than __ARM_ARCH_6M__.
>         (clzdi2): Likewise.
>         (ctzsi2): Likewise.
>         (L_interwork_call_via_rX): Test for __ARM_ARCH_ISA_ARM rather than
>         __ARM_ARCH_6M__ in guard for checking whether it is defined.
>         (final includes): Test for NOT_ISA_TARGET_32BIT rather than
>         __ARM_ARCH_6M__ and add comment to indicate the connection between
>         this condition and the one in gcc/config/arm/elf.h.
>         * config/arm/libunwind.S: Test for __ARM_ARCH_ISA_THUMB and
>         __ARM_ARCH_ISA_ARM rather than __ARM_ARCH_6M__.
>         * config/arm/t-softfp: Likewise.
>
>
> Best regards,
>
> Thomas

OK - thanks for your patience and sorry about the delay.

Ramana

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2016-07-06 16:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-12  9:25 [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Thomas Preud'homme
2016-05-17  9:58 ` [PATCH, libgcc/ARM 1/6, ping1] " Thomas Preudhomme
2016-05-19 16:42   ` Kyrill Tkachov
2016-05-19 16:56     ` Thomas Preudhomme
2016-05-19 17:01       ` Kyrill Tkachov
2016-05-25 13:56         ` Thomas Preudhomme
2016-05-25 15:30           ` Kyrill Tkachov
2016-06-01  9:01 ` [PATCH, libgcc/ARM 1/6] " Ramana Radhakrishnan
2016-06-17 17:22   ` [PATCH, libgcc/ARM 1a/6] " Thomas Preudhomme
2016-06-27 16:26     ` [PATCH, libgcc/ARM 1a/6, ping] " Thomas Preudhomme
2016-07-04 10:26       ` [PATCH, libgcc/ARM 1a/6, ping2] " Thomas Preudhomme
2016-07-06 16:21     ` [PATCH, libgcc/ARM 1a/6] " Ramana Radhakrishnan
2016-06-27 17:22   ` [PATCH, libgcc/ARM 1/6] " Thomas Preudhomme
2016-07-05  9:30     ` [PATCH, libgcc/ARM 1b/7] Check CLZ availability with ISA support and architecture level macros Thomas Preudhomme
2016-07-06 16:20     ` [PATCH, libgcc/ARM 1/6] Fix Thumb-1 only == ARMv6-M & Thumb-2 only == ARMv7-M assumptions Ramana Radhakrishnan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).