diff mbox

[AArch64] Fix PR71727

Message ID CO2PR07MB2694F7DD93ED87C63C279DAD83A60@CO2PR07MB2694.namprd07.prod.outlook.com
State New
Headers show

Commit Message

Hurugalawadi, Naveen Nov. 8, 2016, 9:57 a.m. UTC
Hi Kyrill,

Thanks for the review and suggestions.

>> It's a good idea to CC the AArch64 maintainers and reviewers
>> on aarch64 patches, or at least

Thanks for CCing the maintainers. Added [AArch64] in the subject line.

>> New functions need a function comment describing their arguments and their result.

Done.

>> Some more information about why the current behaviour is wrong
>> and how the patch fixes it would be useful in reviewing.

support_vector_misalignment target hook is incorrect when 
STRICT_ALIGNMENT is true for AArch64. 
The patch implements the hook and rectifies the behavior.

Please find attached the modified patch as per suggestions.

Thanks,
Naveen

Comments

James Greenhalgh Dec. 6, 2016, 5:44 p.m. UTC | #1
On Tue, Nov 08, 2016 at 09:57:29AM +0000, Hurugalawadi, Naveen wrote:
> Hi Kyrill,
> 
> Thanks for the review and suggestions.
> 
> >> It's a good idea to CC the AArch64 maintainers and reviewers
> >> on aarch64 patches, or at least
> 
> Thanks for CCing the maintainers. Added [AArch64] in the subject line.
> 
> >> New functions need a function comment describing their arguments and their result.
> 
> Done.
> 
> >> Some more information about why the current behaviour is wrong
> >> and how the patch fixes it would be useful in reviewing.
> 
> support_vector_misalignment target hook is incorrect when 
> STRICT_ALIGNMENT is true for AArch64. 
> The patch implements the hook and rectifies the behavior.
> 
> Please find attached the modified patch as per suggestions.

OK.

Thanks,
James

> 
> Thanks,
> Naveen

> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index b7d4640..5a0eff5 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -141,6 +141,10 @@ static bool aarch64_vector_mode_supported_p (machine_mode);
>  static bool aarch64_vectorize_vec_perm_const_ok (machine_mode vmode,
>  						 const unsigned char *sel);
>  static int aarch64_address_cost (rtx, machine_mode, addr_space_t, bool);
> +static bool aarch64_builtin_support_vector_misalignment (machine_mode mode,
> +							 const_tree type,
> +							 int misalignment,
> +							 bool is_packed);
>  
>  /* Major revision number of the ARM Architecture implemented by the target.  */
>  unsigned aarch64_architecture_version;
> @@ -11148,6 +11152,37 @@ aarch64_simd_vector_alignment_reachable (const_tree type, bool is_packed)
>    return true;
>  }
>  
> +/* Return true if the vector misalignment factor is supported by the
> +   target.  */
> +static bool
> +aarch64_builtin_support_vector_misalignment (machine_mode mode,
> +					     const_tree type, int misalignment,
> +					     bool is_packed)
> +{
> +  if (TARGET_SIMD && STRICT_ALIGNMENT)
> +    {
> +      /* Return if movmisalign pattern is not supported for this mode.  */
> +      if (optab_handler (movmisalign_optab, mode) == CODE_FOR_nothing)
> +        return false;
> +
> +      if (misalignment == -1)
> +	{
> +	  /* Misalignment factor is unknown at compile time but we know
> +	     it's word aligned.  */
> +	  if (aarch64_simd_vector_alignment_reachable (type, is_packed))
> +            {
> +              int element_size = TREE_INT_CST_LOW (TYPE_SIZE (type));
> +
> +              if (element_size != 64)
> +                return true;
> +            }
> +	  return false;
> +	}
> +    }
> +  return default_builtin_support_vector_misalignment (mode, type, misalignment,
> +						      is_packed);
> +}
> +
>  /* If VALS is a vector constant that can be loaded into a register
>     using DUP, generate instructions to do so and return an RTX to
>     assign to the register.  Otherwise return NULL_RTX.  */
> @@ -14398,6 +14433,10 @@ aarch64_optab_supported_p (int op, machine_mode mode1, machine_mode,
>  #undef TARGET_VECTOR_MODE_SUPPORTED_P
>  #define TARGET_VECTOR_MODE_SUPPORTED_P aarch64_vector_mode_supported_p
>  
> +#undef TARGET_VECTORIZE_SUPPORT_VECTOR_MISALIGNMENT
> +#define TARGET_VECTORIZE_SUPPORT_VECTOR_MISALIGNMENT \
> +  aarch64_builtin_support_vector_misalignment
> +
>  #undef TARGET_ARRAY_MODE_SUPPORTED_P
>  #define TARGET_ARRAY_MODE_SUPPORTED_P aarch64_array_mode_supported_p
>  
> diff --git a/gcc/testsuite/gcc.target/aarch64/pr71727.c b/gcc/testsuite/gcc.target/aarch64/pr71727.c
> new file mode 100644
> index 0000000..05eef3e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/pr71727.c
> @@ -0,0 +1,33 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mstrict-align -O3" } */
> +
> +struct test_struct_s
> +{
> +  long a;
> +  long b;
> +  long c;
> +  long d;
> +  unsigned long e;
> +};
> +
> +
> +char _a;
> +struct test_struct_s xarray[128];
> +
> +void
> +_start (void)
> +{
> +  struct test_struct_s *new_entry;
> +
> +  new_entry = &xarray[0];
> +  new_entry->a = 1;
> +  new_entry->b = 2;
> +  new_entry->c = 3;
> +  new_entry->d = 4;
> +  new_entry->e = 5;
> +
> +  return;
> +}
> +
> +/* { dg-final { scan-assembler-times "mov\tx" 5 {target lp64} } } */
> +/* { dg-final { scan-assembler-not "add\tx0, x0, :" {target lp64} } } */
diff mbox

Patch

diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index b7d4640..5a0eff5 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -141,6 +141,10 @@  static bool aarch64_vector_mode_supported_p (machine_mode);
 static bool aarch64_vectorize_vec_perm_const_ok (machine_mode vmode,
 						 const unsigned char *sel);
 static int aarch64_address_cost (rtx, machine_mode, addr_space_t, bool);
+static bool aarch64_builtin_support_vector_misalignment (machine_mode mode,
+							 const_tree type,
+							 int misalignment,
+							 bool is_packed);
 
 /* Major revision number of the ARM Architecture implemented by the target.  */
 unsigned aarch64_architecture_version;
@@ -11148,6 +11152,37 @@  aarch64_simd_vector_alignment_reachable (const_tree type, bool is_packed)
   return true;
 }
 
+/* Return true if the vector misalignment factor is supported by the
+   target.  */
+static bool
+aarch64_builtin_support_vector_misalignment (machine_mode mode,
+					     const_tree type, int misalignment,
+					     bool is_packed)
+{
+  if (TARGET_SIMD && STRICT_ALIGNMENT)
+    {
+      /* Return if movmisalign pattern is not supported for this mode.  */
+      if (optab_handler (movmisalign_optab, mode) == CODE_FOR_nothing)
+        return false;
+
+      if (misalignment == -1)
+	{
+	  /* Misalignment factor is unknown at compile time but we know
+	     it's word aligned.  */
+	  if (aarch64_simd_vector_alignment_reachable (type, is_packed))
+            {
+              int element_size = TREE_INT_CST_LOW (TYPE_SIZE (type));
+
+              if (element_size != 64)
+                return true;
+            }
+	  return false;
+	}
+    }
+  return default_builtin_support_vector_misalignment (mode, type, misalignment,
+						      is_packed);
+}
+
 /* If VALS is a vector constant that can be loaded into a register
    using DUP, generate instructions to do so and return an RTX to
    assign to the register.  Otherwise return NULL_RTX.  */
@@ -14398,6 +14433,10 @@  aarch64_optab_supported_p (int op, machine_mode mode1, machine_mode,
 #undef TARGET_VECTOR_MODE_SUPPORTED_P
 #define TARGET_VECTOR_MODE_SUPPORTED_P aarch64_vector_mode_supported_p
 
+#undef TARGET_VECTORIZE_SUPPORT_VECTOR_MISALIGNMENT
+#define TARGET_VECTORIZE_SUPPORT_VECTOR_MISALIGNMENT \
+  aarch64_builtin_support_vector_misalignment
+
 #undef TARGET_ARRAY_MODE_SUPPORTED_P
 #define TARGET_ARRAY_MODE_SUPPORTED_P aarch64_array_mode_supported_p
 
diff --git a/gcc/testsuite/gcc.target/aarch64/pr71727.c b/gcc/testsuite/gcc.target/aarch64/pr71727.c
new file mode 100644
index 0000000..05eef3e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/pr71727.c
@@ -0,0 +1,33 @@ 
+/* { dg-do compile } */
+/* { dg-options "-mstrict-align -O3" } */
+
+struct test_struct_s
+{
+  long a;
+  long b;
+  long c;
+  long d;
+  unsigned long e;
+};
+
+
+char _a;
+struct test_struct_s xarray[128];
+
+void
+_start (void)
+{
+  struct test_struct_s *new_entry;
+
+  new_entry = &xarray[0];
+  new_entry->a = 1;
+  new_entry->b = 2;
+  new_entry->c = 3;
+  new_entry->d = 4;
+  new_entry->e = 5;
+
+  return;
+}
+
+/* { dg-final { scan-assembler-times "mov\tx" 5 {target lp64} } } */
+/* { dg-final { scan-assembler-not "add\tx0, x0, :" {target lp64} } } */