[PATCH] arm: [MVE intrinsics] Fix support for loads [PR target/114323]

[PATCH] arm: [MVE intrinsics] Fix support for loads [PR target/114323]

[Christophe Lyon]

The testcase in this PR shows that we would load from an uninitialized
location, because the vld1 instrinsics are reported as "const". This
is because function_instance::reads_global_state_p() does not take
CP_READ_MEMORY into account.  Fixing this gives vld1 the "pure"
attribute instead, and solves the problem.

2024-03-15  Christophe Lyon  <christophe.lyon@linaro.org>

	PR target/114323
	gcc/
	* config/arm/arm-mve-builtins.cc
	(function_instance::reads_global_state_p): Take CP_READ_MEMORY
	into account.

	gcc/testsuite/
	* gcc.target/arm/mve/pr114323.c: New.
---
 gcc/config/arm/arm-mve-builtins.cc          |  2 +-
 gcc/testsuite/gcc.target/arm/mve/pr114323.c | 22 +++++++++++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/arm/mve/pr114323.c

diff --git a/gcc/config/arm/arm-mve-builtins.cc b/gcc/config/arm/arm-mve-builtins.cc
index 2f2c0f4a02a..6a5775c67e5 100644
--- a/gcc/config/arm/arm-mve-builtins.cc
+++ b/gcc/config/arm/arm-mve-builtins.cc
@@ -746,7 +746,7 @@ function_instance::reads_global_state_p () const
   if (flags & CP_READ_FPCR)
     return true;
 
-  return false;
+  return flags & CP_READ_MEMORY;
 }
 
 /* Return true if calls to the function could modify some form of
diff --git a/gcc/testsuite/gcc.target/arm/mve/pr114323.c b/gcc/testsuite/gcc.target/arm/mve/pr114323.c
new file mode 100644
index 00000000000..bd9127b886a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/mve/pr114323.c
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-require-effective-target arm_mve_hw } */
+/* { dg-options "-O2" } */
+/* { dg-add-options arm_v8_1m_mve_fp } */
+
+#include <arm_mve.h>
+
+__attribute__((noipa))
+uint32x4_t foo (void) {
+  uint32x4_t V0 = vld1q_u32(((const uint32_t[4]){1, 2, 3, 4}));
+  return V0;
+}
+
+int main(void)
+{
+  uint32_t buf[4];
+ vst1q_u32 (buf, foo());
+
+  for (int i = 0; i < 4; i++)
+    if (buf[i] != i+1)
+      __builtin_abort ();
+}
-- 
2.34.1

Re: [PATCH] arm: [MVE intrinsics] Fix support for loads [PR target/114323]

[Richard Earnshaw (lists)]

On 15/03/2024 20:08, Christophe Lyon wrote:
> The testcase in this PR shows that we would load from an uninitialized
> location, because the vld1 instrinsics are reported as "const". This
> is because function_instance::reads_global_state_p() does not take
> CP_READ_MEMORY into account.  Fixing this gives vld1 the "pure"
> attribute instead, and solves the problem.
> 
> 2024-03-15  Christophe Lyon  <christophe.lyon@linaro.org>
> 
> 	PR target/114323
> 	gcc/
> 	* config/arm/arm-mve-builtins.cc
> 	(function_instance::reads_global_state_p): Take CP_READ_MEMORY
> 	into account.
> 
> 	gcc/testsuite/
> 	* gcc.target/arm/mve/pr114323.c: New.

OK.

R.

> ---
>   gcc/config/arm/arm-mve-builtins.cc          |  2 +-
>   gcc/testsuite/gcc.target/arm/mve/pr114323.c | 22 +++++++++++++++++++++
>   2 files changed, 23 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/gcc.target/arm/mve/pr114323.c
> 
> diff --git a/gcc/config/arm/arm-mve-builtins.cc b/gcc/config/arm/arm-mve-builtins.cc
> index 2f2c0f4a02a..6a5775c67e5 100644
> --- a/gcc/config/arm/arm-mve-builtins.cc
> +++ b/gcc/config/arm/arm-mve-builtins.cc
> @@ -746,7 +746,7 @@ function_instance::reads_global_state_p () const
>     if (flags & CP_READ_FPCR)
>       return true;
>   
> -  return false;
> +  return flags & CP_READ_MEMORY;
>   }
>   
>   /* Return true if calls to the function could modify some form of
> diff --git a/gcc/testsuite/gcc.target/arm/mve/pr114323.c b/gcc/testsuite/gcc.target/arm/mve/pr114323.c
> new file mode 100644
> index 00000000000..bd9127b886a
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/mve/pr114323.c
> @@ -0,0 +1,22 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target arm_mve_hw } */
> +/* { dg-options "-O2" } */
> +/* { dg-add-options arm_v8_1m_mve_fp } */
> +
> +#include <arm_mve.h>
> +
> +__attribute__((noipa))
> +uint32x4_t foo (void) {
> +  uint32x4_t V0 = vld1q_u32(((const uint32_t[4]){1, 2, 3, 4}));
> +  return V0;
> +}
> +
> +int main(void)
> +{
> +  uint32_t buf[4];
> + vst1q_u32 (buf, foo());
> +
> +  for (int i = 0; i < 4; i++)
> +    if (buf[i] != i+1)
> +      __builtin_abort ();
> +}