diff mbox series

arm: [MVE intrinsics] Avoid warnings when floating-point is not supported [PR 117814]

Message ID 20241202112100.3083428-1-christophe.lyon@linaro.org
State New
Headers show
Series arm: [MVE intrinsics] Avoid warnings when floating-point is not supported [PR 117814] | expand

Commit Message

Christophe Lyon Dec. 2, 2024, 11:21 a.m. UTC 
If the target does not support floating-point, we register FP vector
types as 'void' (see register_vector_type).

The leads to warnings about 'pure attribute on function returning
void' when we declare the various load intrinsics because their
call_properties say CP_READ_MEMORY (thus giving them the 'pure'
attribute), but their return type is void.

To avoid such warnings, pretend the call_properties are empty when FP
is disabled and the function would return an FP value.  If such
functions are incorrectly used in user code, a proper error is
emitted:
unknown type name ‘float16x8_t'; did you mean ‘int16x8_t’?

gcc/ChangeLog:

	PR target/117814
	* config/arm/arm-mve-builtins-base.cc (vld1_impl): Fix
	call_properties.
	(vld24_impl): Likewise.
	* config/arm/arm-mve-builtins-functions.h (load_extending):
	Likewise.
---
 gcc/config/arm/arm-mve-builtins-base.cc     | 22 +++++++++++++++++++--
 gcc/config/arm/arm-mve-builtins-functions.h | 11 ++++++++++-
 2 files changed, 30 insertions(+), 3 deletions(-)

Comments

Richard Earnshaw (lists) Dec. 2, 2024, 11:43 a.m. UTC | #1 
On 02/12/2024 11:21, Christophe Lyon wrote:
> If the target does not support floating-point, we register FP vector
> types as 'void' (see register_vector_type).
> 
> The leads to warnings about 'pure attribute on function returning
> void' when we declare the various load intrinsics because their
> call_properties say CP_READ_MEMORY (thus giving them the 'pure'
> attribute), but their return type is void.
> 
> To avoid such warnings, pretend the call_properties are empty when FP
> is disabled and the function would return an FP value.  If such
> functions are incorrectly used in user code, a proper error is
> emitted:
> unknown type name ‘float16x8_t'; did you mean ‘int16x8_t’?
> 
> gcc/ChangeLog:
> 
> 	PR target/117814
> 	* config/arm/arm-mve-builtins-base.cc (vld1_impl): Fix
> 	call_properties.
> 	(vld24_impl): Likewise.
> 	* config/arm/arm-mve-builtins-functions.h (load_extending):
> 	Likewise.

Won't this lead to problems if the code is something like

#include "arm_mve.h"

#pragma gcc target ("arch=armv8.1-m.main+mve.fp")

// Some use of an affected intrinsic

and then compile with "-march=armv8.1-m.main+mve -mfpu=auto -mfloat-abi=softfp"?

R.

> ---
>  gcc/config/arm/arm-mve-builtins-base.cc     | 22 +++++++++++++++++++--
>  gcc/config/arm/arm-mve-builtins-functions.h | 11 ++++++++++-
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/config/arm/arm-mve-builtins-base.cc b/gcc/config/arm/arm-mve-builtins-base.cc
> index 723004b53d7..a322730eca8 100644
> --- a/gcc/config/arm/arm-mve-builtins-base.cc
> +++ b/gcc/config/arm/arm-mve-builtins-base.cc
> @@ -141,8 +141,17 @@ class vld1_impl : public full_width_access
>  {
>  public:
>    unsigned int
> -  call_properties (const function_instance &) const override
> +  call_properties (const function_instance &instance) const override
>    {
> +    /* If the target does not support floating-point, we register FP vector
> +       types as 'void'.  In this case, pretend we do not access memory to avoid
> +       warnings about 'pure attribute on function returning void' when we
> +       declare the intrinsics.  Such uses in user code are properly
> +       diagnosed.  */
> +    if (!TARGET_HAVE_MVE_FLOAT
> +	&& instance.type_suffix (0).float_p)
> +      return 0;
> +
>      return CP_READ_MEMORY;
>    }
>  
> @@ -1141,8 +1150,17 @@ public:
>    using full_width_access::full_width_access;
>  
>    unsigned int
> -  call_properties (const function_instance &) const override
> +  call_properties (const function_instance &instance) const override
>    {
> +    /* If the target does not support floating-point, we register FP vector
> +       types as 'void'.  In this case, pretend we do not access memory to avoid
> +       warnings about 'pure attribute on function returning void' when we
> +       declare the intrinsics.  Such uses in user code are properly
> +       diagnosed.  */
> +    if (!TARGET_HAVE_MVE_FLOAT
> +	&& instance.type_suffix (0).float_p)
> +      return 0;
> +
>      return CP_READ_MEMORY;
>    }
>  
> diff --git a/gcc/config/arm/arm-mve-builtins-functions.h b/gcc/config/arm/arm-mve-builtins-functions.h
> index 0ade2157e4a..1a9a347805c 100644
> --- a/gcc/config/arm/arm-mve-builtins-functions.h
> +++ b/gcc/config/arm/arm-mve-builtins-functions.h
> @@ -986,8 +986,17 @@ public:
>        m_float_memory_type (NUM_TYPE_SUFFIXES)
>    {}
>  
> -  unsigned int call_properties (const function_instance &) const override
> +  unsigned int call_properties (const function_instance &instance) const override
>    {
> +    /* If the target does not support floating-point, we register FP vector
> +       types as 'void'.  In this case, pretend we do not access memory to avoid
> +       warnings about 'pure attribute on function returning void' when we
> +       declare the intrinsics.  Such uses in user code are properly
> +       diagnosed.  */
> +    if (!TARGET_HAVE_MVE_FLOAT
> +	&& instance.type_suffix (0).float_p)
> +      return 0;
> +
>      return CP_READ_MEMORY;
>    }
>
Christophe Lyon Dec. 2, 2024, 1:23 p.m. UTC | #2 
On Mon, 2 Dec 2024 at 12:44, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
>
> On 02/12/2024 11:21, Christophe Lyon wrote:
> > If the target does not support floating-point, we register FP vector
> > types as 'void' (see register_vector_type).
> >
> > The leads to warnings about 'pure attribute on function returning
> > void' when we declare the various load intrinsics because their
> > call_properties say CP_READ_MEMORY (thus giving them the 'pure'
> > attribute), but their return type is void.
> >
> > To avoid such warnings, pretend the call_properties are empty when FP
> > is disabled and the function would return an FP value.  If such
> > functions are incorrectly used in user code, a proper error is
> > emitted:
> > unknown type name ‘float16x8_t'; did you mean ‘int16x8_t’?
> >
> > gcc/ChangeLog:
> >
> >       PR target/117814
> >       * config/arm/arm-mve-builtins-base.cc (vld1_impl): Fix
> >       call_properties.
> >       (vld24_impl): Likewise.
> >       * config/arm/arm-mve-builtins-functions.h (load_extending):
> >       Likewise.
>
> Won't this lead to problems if the code is something like
>
> #include "arm_mve.h"
>
> #pragma gcc target ("arch=armv8.1-m.main+mve.fp")
>
> // Some use of an affected intrinsic
>
> and then compile with "-march=armv8.1-m.main+mve -mfpu=auto -mfloat-abi=softfp"?
>
Indeed.... actually the real cause is precisely that without FPU we do
not register FP types (and map them to "void" instead).
I had started to look at fixing that but it looked more invasive.

Actually if you take any intrinsic test (say vaddq_f16.c), remove
"_fp" from dg-require-effective-target and dg-add-options, and add the
pragma above, you'll get the same error:
vaddq_f16.c:20:1: error: unknown type name 'float16x8_t'; did you mean
'int16x8_t'?

Let me look at fixing this.

Thanks,

Christophe

> R.
>
> > ---
> >  gcc/config/arm/arm-mve-builtins-base.cc     | 22 +++++++++++++++++++--
> >  gcc/config/arm/arm-mve-builtins-functions.h | 11 ++++++++++-
> >  2 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/config/arm/arm-mve-builtins-base.cc b/gcc/config/arm/arm-mve-builtins-base.cc
> > index 723004b53d7..a322730eca8 100644
> > --- a/gcc/config/arm/arm-mve-builtins-base.cc
> > +++ b/gcc/config/arm/arm-mve-builtins-base.cc
> > @@ -141,8 +141,17 @@ class vld1_impl : public full_width_access
> >  {
> >  public:
> >    unsigned int
> > -  call_properties (const function_instance &) const override
> > +  call_properties (const function_instance &instance) const override
> >    {
> > +    /* If the target does not support floating-point, we register FP vector
> > +       types as 'void'.  In this case, pretend we do not access memory to avoid
> > +       warnings about 'pure attribute on function returning void' when we
> > +       declare the intrinsics.  Such uses in user code are properly
> > +       diagnosed.  */
> > +    if (!TARGET_HAVE_MVE_FLOAT
> > +     && instance.type_suffix (0).float_p)
> > +      return 0;
> > +
> >      return CP_READ_MEMORY;
> >    }
> >
> > @@ -1141,8 +1150,17 @@ public:
> >    using full_width_access::full_width_access;
> >
> >    unsigned int
> > -  call_properties (const function_instance &) const override
> > +  call_properties (const function_instance &instance) const override
> >    {
> > +    /* If the target does not support floating-point, we register FP vector
> > +       types as 'void'.  In this case, pretend we do not access memory to avoid
> > +       warnings about 'pure attribute on function returning void' when we
> > +       declare the intrinsics.  Such uses in user code are properly
> > +       diagnosed.  */
> > +    if (!TARGET_HAVE_MVE_FLOAT
> > +     && instance.type_suffix (0).float_p)
> > +      return 0;
> > +
> >      return CP_READ_MEMORY;
> >    }
> >
> > diff --git a/gcc/config/arm/arm-mve-builtins-functions.h b/gcc/config/arm/arm-mve-builtins-functions.h
> > index 0ade2157e4a..1a9a347805c 100644
> > --- a/gcc/config/arm/arm-mve-builtins-functions.h
> > +++ b/gcc/config/arm/arm-mve-builtins-functions.h
> > @@ -986,8 +986,17 @@ public:
> >        m_float_memory_type (NUM_TYPE_SUFFIXES)
> >    {}
> >
> > -  unsigned int call_properties (const function_instance &) const override
> > +  unsigned int call_properties (const function_instance &instance) const override
> >    {
> > +    /* If the target does not support floating-point, we register FP vector
> > +       types as 'void'.  In this case, pretend we do not access memory to avoid
> > +       warnings about 'pure attribute on function returning void' when we
> > +       declare the intrinsics.  Such uses in user code are properly
> > +       diagnosed.  */
> > +    if (!TARGET_HAVE_MVE_FLOAT
> > +     && instance.type_suffix (0).float_p)
> > +      return 0;
> > +
> >      return CP_READ_MEMORY;
> >    }
> >
>
diff mbox series

Patch

diff --git a/gcc/config/arm/arm-mve-builtins-base.cc b/gcc/config/arm/arm-mve-builtins-base.cc
index 723004b53d7..a322730eca8 100644
--- a/gcc/config/arm/arm-mve-builtins-base.cc
+++ b/gcc/config/arm/arm-mve-builtins-base.cc
@@ -141,8 +141,17 @@  class vld1_impl : public full_width_access
 {
 public:
   unsigned int
-  call_properties (const function_instance &) const override
+  call_properties (const function_instance &instance) const override
   {
+    /* If the target does not support floating-point, we register FP vector
+       types as 'void'.  In this case, pretend we do not access memory to avoid
+       warnings about 'pure attribute on function returning void' when we
+       declare the intrinsics.  Such uses in user code are properly
+       diagnosed.  */
+    if (!TARGET_HAVE_MVE_FLOAT
+	&& instance.type_suffix (0).float_p)
+      return 0;
+
     return CP_READ_MEMORY;
   }
 
@@ -1141,8 +1150,17 @@  public:
   using full_width_access::full_width_access;
 
   unsigned int
-  call_properties (const function_instance &) const override
+  call_properties (const function_instance &instance) const override
   {
+    /* If the target does not support floating-point, we register FP vector
+       types as 'void'.  In this case, pretend we do not access memory to avoid
+       warnings about 'pure attribute on function returning void' when we
+       declare the intrinsics.  Such uses in user code are properly
+       diagnosed.  */
+    if (!TARGET_HAVE_MVE_FLOAT
+	&& instance.type_suffix (0).float_p)
+      return 0;
+
     return CP_READ_MEMORY;
   }
 
diff --git a/gcc/config/arm/arm-mve-builtins-functions.h b/gcc/config/arm/arm-mve-builtins-functions.h
index 0ade2157e4a..1a9a347805c 100644
--- a/gcc/config/arm/arm-mve-builtins-functions.h
+++ b/gcc/config/arm/arm-mve-builtins-functions.h
@@ -986,8 +986,17 @@  public:
       m_float_memory_type (NUM_TYPE_SUFFIXES)
   {}
 
-  unsigned int call_properties (const function_instance &) const override
+  unsigned int call_properties (const function_instance &instance) const override
   {
+    /* If the target does not support floating-point, we register FP vector
+       types as 'void'.  In this case, pretend we do not access memory to avoid
+       warnings about 'pure attribute on function returning void' when we
+       declare the intrinsics.  Such uses in user code are properly
+       diagnosed.  */
+    if (!TARGET_HAVE_MVE_FLOAT
+	&& instance.type_suffix (0).float_p)
+      return 0;
+
     return CP_READ_MEMORY;
   }