[PATCH, ARM] PR target/68617 Fix armv6 unaligned_access with attribute thumb

[PATCH, ARM] PR target/68617 Fix armv6 unaligned_access with attribute thumb

[Christian Bruel]

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

Hi,

This patches fixes the PR my making the unaligned_access flag sensitive 
to the attribute target, since some armv6 might use unaligned loads 
depending on the TARGET_32BIT flag.

OK for stage3 ?


[-- Attachment #2: v6-attr-unaligned.patch --]
[-- Type: text/x-patch, Size: 5448 bytes --]

2015-11-30  Christian Bruel  <christian.bruel@st.com>

	PR target/68617
	* config/arm/arm.opt (unaligned_access): Save.
	* config/arm/arm-c.c (__ARM_FEATURE_UNALIGNED): Conditionally define.
	* config/arm/arm.c (arm_option_override): Move unaligned_access setting
	(arm_option_override_internal): ... here.
	* config/arm/arm.h (TARGET_32BIT_P): New macro.

2015-11-30  Christian Bruel  <christian.bruel@st.com>

	PR target/68617
	* gcc.target/arm/attr-unaligned-load-ice.c: New test.

Index: gcc/config/arm/arm-c.c
===================================================================
--- gcc/config/arm/arm-c.c	(revision 231101)
+++ gcc/config/arm/arm-c.c	(working copy)
@@ -66,8 +66,8 @@ arm_cpu_builtins (struct cpp_reader* pfi
   def_or_undef_macro (pfile, "__ARM_FEATURE_SAT", TARGET_ARM_SAT);
   def_or_undef_macro (pfile, "__ARM_FEATURE_CRYPTO", TARGET_CRYPTO);
 
-  if (unaligned_access)
-    builtin_define ("__ARM_FEATURE_UNALIGNED");
+  def_or_undef_macro (pfile, "__ARM_FEATURE_UNALIGNED", unaligned_access);
+
   if (TARGET_CRC32)
     builtin_define ("__ARM_FEATURE_CRC32");
 
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 231101)
+++ gcc/config/arm/arm.c	(working copy)
@@ -2876,6 +2876,28 @@ arm_option_override_internal (struct gcc
   if (!TARGET_THUMB2_P (opts->x_target_flags))
     opts->x_arm_restrict_it = 0;
 
+  /* Enable -munaligned-access by default for
+     - all ARMv6 architecture-based processors when compiling for a 32-bit ISA
+     i.e. Thumb2 and ARM state only.
+     - ARMv7-A, ARMv7-R, and ARMv7-M architecture-based processors.
+     - ARMv8 architecture-base processors.
+
+     Disable -munaligned-access by default for
+     - all pre-ARMv6 architecture-based processors
+     - ARMv6-M architecture-based processors.  */
+
+  if (! opts_set->x_unaligned_access)
+    {
+      opts->x_unaligned_access = (TARGET_32BIT_P (opts->x_target_flags)
+			  && arm_arch6 && (arm_arch_notm || arm_arch7));
+    }
+  else if (opts->x_unaligned_access == 1
+	   && !(arm_arch6 && (arm_arch_notm || arm_arch7)))
+    {
+      warning (0, "target CPU does not support unaligned accesses");
+     opts->x_unaligned_access = 0;
+    }
+
   /* Don't warn since it's on by default in -O2.  */
   if (TARGET_THUMB1_P (opts->x_target_flags))
     opts->x_flag_schedule_insns = 0;
@@ -3281,30 +3303,6 @@ arm_option_override (void)
 	fix_cm3_ldrd = 0;
     }
 
-  /* Enable -munaligned-access by default for
-     - all ARMv6 architecture-based processors when compiling for a 32-bit ISA
-     i.e. Thumb2 and ARM state only.
-     - ARMv7-A, ARMv7-R, and ARMv7-M architecture-based processors.
-     - ARMv8 architecture-base processors.
-
-     Disable -munaligned-access by default for
-     - all pre-ARMv6 architecture-based processors
-     - ARMv6-M architecture-based processors.  */
-
-  if (unaligned_access == 2)
-    {
-      if (TARGET_32BIT && arm_arch6 && (arm_arch_notm || arm_arch7))
-	unaligned_access = 1;
-      else
-	unaligned_access = 0;
-    }
-  else if (unaligned_access == 1
-	   && !(arm_arch6 && (arm_arch_notm || arm_arch7)))
-    {
-      warning (0, "target CPU does not support unaligned accesses");
-      unaligned_access = 0;
-    }
-
   /* Hot/Cold partitioning is not currently supported, since we can't
      handle literal pool placement in that case.  */
   if (flag_reorder_blocks_and_partition)
Index: gcc/config/arm/arm.h
===================================================================
--- gcc/config/arm/arm.h	(revision 231101)
+++ gcc/config/arm/arm.h	(working copy)
@@ -131,6 +131,7 @@ extern void (*arm_lang_output_object_att
 #define TARGET_ARM_P(flags)    (!TARGET_THUMB_P (flags))
 #define TARGET_THUMB1_P(flags) (TARGET_THUMB_P (flags) && !arm_arch_thumb2)
 #define TARGET_THUMB2_P(flags) (TARGET_THUMB_P (flags) && arm_arch_thumb2)
+#define TARGET_32BIT_P(flags)  (TARGET_ARM_P (flags) || TARGET_THUMB2_P (flags))
 
 /* Run-time Target Specification.  */
 #define TARGET_SOFT_FLOAT		(arm_float_abi == ARM_FLOAT_ABI_SOFT)
Index: gcc/config/arm/arm.opt
===================================================================
--- gcc/config/arm/arm.opt	(revision 231101)
+++ gcc/config/arm/arm.opt	(working copy)
@@ -267,7 +267,7 @@ Avoid overlapping destination and addres
 that may trigger Cortex-M3 errata.
 
 munaligned-access
-Target Report Var(unaligned_access) Init(2)
+Target Report Var(unaligned_access) Init(2) Save
 Enable unaligned word and halfword accesses to packed data.
 
 mneon-for-64bits
Index: gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c
===================================================================
--- gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c	(working copy)
@@ -0,0 +1,19 @@
+/* PR target/68617
+   Verify that unaligned_access is correctly with attribute target.  */
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6" } } */
+/* { dg-options "-Os -mfloat-abi=softfp -mtp=soft" } */
+/* { dg-add-options arm_arch_v6 } */
+
+long __attribute__((target("thumb")))
+foo (char *s, long size, int unsigned_p)
+{
+  long x;
+  unsigned char *p = (unsigned char *) s;
+  switch (size)
+    {
+    case 4:
+      x = ((long) p[3] << 24) | ((long) p[2] << 16) | (p[1] << 8) | p[0];
+      return x;
+    }
+}

Re: [PATCH, ARM] PR target/68617 Fix armv6 unaligned_access with attribute thumb

[Kyrill Tkachov]

Hi Christian,

On 01/12/15 09:18, Christian Bruel wrote:
> Hi,
>
> This patches fixes the PR my making the unaligned_access flag sensitive to the attribute target, since some armv6 might use unaligned loads depending on the TARGET_32BIT flag.
>
> OK for stage3 ?
>

Index: gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c
===================================================================
--- gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c	(revision 0)
+++ gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c	(working copy)
@@ -0,0 +1,19 @@
+/* PR target/68617
+   Verify that unaligned_access is correctly with attribute target.  */
+/* { dg-do compile } */
+/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6" } } */
+/* { dg-options "-Os -mfloat-abi=softfp -mtp=soft" } */
+/* { dg-add-options arm_arch_v6 } */


Do you need the -mtp=soft ?

This is ok for trunk.
Thanks for picking this up.

Kyrill

Re: [PATCH, ARM] PR target/68617 Fix armv6 unaligned_access with attribute thumb

[Kyrill Tkachov]

On 01/12/15 12:28, Christian Bruel wrote:
>
>
> On 12/01/2015 12:57 PM, Kyrill Tkachov wrote:
>> Hi Christian,
>>
>> On 01/12/15 09:18, Christian Bruel wrote:
>>> Hi,
>>>
>>> This patches fixes the PR my making the unaligned_access flag sensitive to the attribute target, since some armv6 might use unaligned loads depending on the TARGET_32BIT flag.
>>>
>>> OK for stage3 ?
>>>
>>
>> Index: gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c
>> ===================================================================
>> --- gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c (revision 0)
>> +++ gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c (working copy)
>> @@ -0,0 +1,19 @@
>> +/* PR target/68617
>> +   Verify that unaligned_access is correctly with attribute target.  */
>> +/* { dg-do compile } */
>> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6" } } */
>> +/* { dg-options "-Os -mfloat-abi=softfp -mtp=soft" } */
>> +/* { dg-add-options arm_arch_v6 } */
>>
>>
>> Do you need the -mtp=soft ?
>
> I think so. When auto, the TP mode is "TP_SOFT" for arm and "TP_CP15" for thumb, which cannot be thumb1. To avoid this kind of discrepancy I prefer to force it.
>
> this is guarded by the lines @arm.c:2759:
>
>   if (TARGET_HARD_TP && TARGET_THUMB1_P (flags))
>     error ("can not use -mtp=cp15 with 16-bit Thumb");


Ok, thanks, I was just curious.
Kyrill


>>
>> This is ok for trunk.
>> Thanks for picking this up.
>>
>> Kyrill
>>
>

Re: [PATCH, ARM] PR target/68617 Fix armv6 unaligned_access with attribute thumb

[Christian Bruel]

On 12/01/2015 12:57 PM, Kyrill Tkachov wrote:
> Hi Christian,
>
> On 01/12/15 09:18, Christian Bruel wrote:
>> Hi,
>>
>> This patches fixes the PR my making the unaligned_access flag sensitive to the attribute target, since some armv6 might use unaligned loads depending on the TARGET_32BIT flag.
>>
>> OK for stage3 ?
>>
>
> Index: gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c
> ===================================================================
> --- gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c	(revision 0)
> +++ gcc/testsuite/gcc.target/arm/attr-unaligned-load-ice.c	(working copy)
> @@ -0,0 +1,19 @@
> +/* PR target/68617
> +   Verify that unaligned_access is correctly with attribute target.  */
> +/* { dg-do compile } */
> +/* { dg-skip-if "avoid conflicting multilib options" { *-*-* } { "-march=*" } { "-march=armv6" } } */
> +/* { dg-options "-Os -mfloat-abi=softfp -mtp=soft" } */
> +/* { dg-add-options arm_arch_v6 } */
>
>
> Do you need the -mtp=soft ?

I think so. When auto, the TP mode is "TP_SOFT" for arm and "TP_CP15" 
for thumb, which cannot be thumb1. To avoid this kind of discrepancy I 
prefer to force it.

this is guarded by the lines @arm.c:2759:

   if (TARGET_HARD_TP && TARGET_THUMB1_P (flags))
     error ("can not use -mtp=cp15 with 16-bit Thumb");
>
> This is ok for trunk.
> Thanks for picking this up.
>
> Kyrill
>