Re: [PATCH] [AArch64] Fix PR71727
On Tue, Nov 08, 2016 at 09:57:29AM +, 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 000..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} } } */
[PING] [PATCH] [AArch64] Fix PR71727
Hi, Please consider this as a personal reminder to review the patch at following link and let me know your comments on the same. https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00697.html Thanks, Naveen
Re: [PATCH] [AArch64] Fix PR71727
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,
Naveendiff --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 000..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} } } */
