diff mbox series

[2/2] RISC-V: Allow uninitialized preferred_else_value for RVV

Message ID 20240711121208.45328-2-syq@gcc.gnu.org
State New
Headers show
Series [1/2] Add allow_uninitialized to tree_base.u.bits for VAR_DECL | expand

Commit Message

YunQiang Su July 11, 2024, 12:12 p.m. UTC 
From: YunQiang Su <yunqiang@isrc.iscas.ac.cn>

PR target/115840.

In riscv_preferred_else_value, we create an uninitialized tmp var
for else value, instead of the 0 (as default_preferred_else_value)
or the pre-exists VAR (as aarch64 does), so that we can use agnostic
policy.

The problem is that `warn_uninit` will emit a warning:
  ({anonymous})’ may be used uninitialized

Let's mark this tmp var as "allow_uninitialized".

This problem is found when I try to build glibc with V extension.

gcc
	PR target/115840.
	* config/riscv/riscv.cc(riscv_preferred_else_value): Mark
	tmp_var as allow_unitialized.

gcc/testsuite
	* gcc.dg/vect/pr115840.c: New testcase.
---
 gcc/config/riscv/riscv.cc            |  6 +++++-
 gcc/testsuite/gcc.dg/vect/pr115840.c | 11 +++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/vect/pr115840.c

Comments

Richard Biener July 11, 2024, 12:20 p.m. UTC | #1 
On Thu, Jul 11, 2024 at 2:13 PM YunQiang Su <syq@gcc.gnu.org> wrote:
>
> From: YunQiang Su <yunqiang@isrc.iscas.ac.cn>
>
> PR target/115840.
>
> In riscv_preferred_else_value, we create an uninitialized tmp var
> for else value, instead of the 0 (as default_preferred_else_value)
> or the pre-exists VAR (as aarch64 does), so that we can use agnostic
> policy.
>
> The problem is that `warn_uninit` will emit a warning:
>   ({anonymous})’ may be used uninitialized
>
> Let's mark this tmp var as "allow_uninitialized".
>
> This problem is found when I try to build glibc with V extension.
>
> gcc
>         PR target/115840.
>         * config/riscv/riscv.cc(riscv_preferred_else_value): Mark
>         tmp_var as allow_unitialized.
>
> gcc/testsuite
>         * gcc.dg/vect/pr115840.c: New testcase.
> ---
>  gcc/config/riscv/riscv.cc            |  6 +++++-
>  gcc/testsuite/gcc.dg/vect/pr115840.c | 11 +++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/vect/pr115840.c
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 61fa74e9322..08159d7cbbc 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -11431,7 +11431,11 @@ riscv_preferred_else_value (unsigned ifn, tree vectype, unsigned int nops,
>                             tree *ops)
>  {
>    if (riscv_v_ext_mode_p (TYPE_MODE (vectype)))
> -    return get_or_create_ssa_default_def (cfun, create_tmp_var (vectype));
> +    {
> +      tree tmp_var = create_tmp_var (vectype);
> +      TREE_ALLOW_UNINITIALIZED (tmp_var) = 1;

Does it work when you do

 TREE_NO_WARNING (tmp_var) = 1;

?

> +      return get_or_create_ssa_default_def (cfun, tmp_var);
> +    }
>
>    return default_preferred_else_value (ifn, vectype, nops, ops);
>  }
> diff --git a/gcc/testsuite/gcc.dg/vect/pr115840.c b/gcc/testsuite/gcc.dg/vect/pr115840.c
> new file mode 100644
> index 00000000000..09dc9e4eb7c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr115840.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-Wall -Werror" } */
> +
> +double loads[16];
> +
> +void
> +foo (double loadavg[], int count)
> +{
> +  for (int i = 0; i < count; i++)
> +    loadavg[i] = loads[i] / 1.5;
> +}
> --
> 2.45.1
>
YunQiang Su July 11, 2024, 12:45 p.m. UTC | #2 
Richard Biener <richard.guenther@gmail.com> 于2024年7月11日周四 20:21写道：
>
> On Thu, Jul 11, 2024 at 2:13 PM YunQiang Su <syq@gcc.gnu.org> wrote:
> >
> > From: YunQiang Su <yunqiang@isrc.iscas.ac.cn>
> >
> > PR target/115840.
> >
> > In riscv_preferred_else_value, we create an uninitialized tmp var
> > for else value, instead of the 0 (as default_preferred_else_value)
> > or the pre-exists VAR (as aarch64 does), so that we can use agnostic
> > policy.
> >
> > The problem is that `warn_uninit` will emit a warning:
> >   ({anonymous})’ may be used uninitialized
> >
> > Let's mark this tmp var as "allow_uninitialized".
> >
> > This problem is found when I try to build glibc with V extension.
> >
> > gcc
> >         PR target/115840.
> >         * config/riscv/riscv.cc(riscv_preferred_else_value): Mark
> >         tmp_var as allow_unitialized.
> >
> > gcc/testsuite
> >         * gcc.dg/vect/pr115840.c: New testcase.
> > ---
> >  gcc/config/riscv/riscv.cc            |  6 +++++-
> >  gcc/testsuite/gcc.dg/vect/pr115840.c | 11 +++++++++++
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/vect/pr115840.c
> >
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 61fa74e9322..08159d7cbbc 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -11431,7 +11431,11 @@ riscv_preferred_else_value (unsigned ifn, tree vectype, unsigned int nops,
> >                             tree *ops)
> >  {
> >    if (riscv_v_ext_mode_p (TYPE_MODE (vectype)))
> > -    return get_or_create_ssa_default_def (cfun, create_tmp_var (vectype));
> > +    {
> > +      tree tmp_var = create_tmp_var (vectype);
> > +      TREE_ALLOW_UNINITIALIZED (tmp_var) = 1;
>
> Does it work when you do
>
>  TREE_NO_WARNING (tmp_var) = 1;
>

Thanks.  It works.  I did notice it, while I worried that there may be
some other
warnings, that TREE_NO_WARNING may cover them.

> ?
>
> > +      return get_or_create_ssa_default_def (cfun, tmp_var);
> > +    }
> >
> >    return default_preferred_else_value (ifn, vectype, nops, ops);
> >  }
> > diff --git a/gcc/testsuite/gcc.dg/vect/pr115840.c b/gcc/testsuite/gcc.dg/vect/pr115840.c
> > new file mode 100644
> > index 00000000000..09dc9e4eb7c
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/vect/pr115840.c
> > @@ -0,0 +1,11 @@
> > +/* { dg-do compile } */
> > +/* { dg-additional-options "-Wall -Werror" } */
> > +
> > +double loads[16];
> > +
> > +void
> > +foo (double loadavg[], int count)
> > +{
> > +  for (int i = 0; i < count; i++)
> > +    loadavg[i] = loads[i] / 1.5;
> > +}
> > --
> > 2.45.1
> >
Richard Biener July 11, 2024, 1 p.m. UTC | #3 
On Thu, Jul 11, 2024 at 2:45 PM YunQiang Su <syq@gcc.gnu.org> wrote:
>
> Richard Biener <richard.guenther@gmail.com> 于2024年7月11日周四 20:21写道：
> >
> > On Thu, Jul 11, 2024 at 2:13 PM YunQiang Su <syq@gcc.gnu.org> wrote:
> > >
> > > From: YunQiang Su <yunqiang@isrc.iscas.ac.cn>
> > >
> > > PR target/115840.
> > >
> > > In riscv_preferred_else_value, we create an uninitialized tmp var
> > > for else value, instead of the 0 (as default_preferred_else_value)
> > > or the pre-exists VAR (as aarch64 does), so that we can use agnostic
> > > policy.
> > >
> > > The problem is that `warn_uninit` will emit a warning:
> > >   ({anonymous})’ may be used uninitialized
> > >
> > > Let's mark this tmp var as "allow_uninitialized".
> > >
> > > This problem is found when I try to build glibc with V extension.
> > >
> > > gcc
> > >         PR target/115840.
> > >         * config/riscv/riscv.cc(riscv_preferred_else_value): Mark
> > >         tmp_var as allow_unitialized.
> > >
> > > gcc/testsuite
> > >         * gcc.dg/vect/pr115840.c: New testcase.
> > > ---
> > >  gcc/config/riscv/riscv.cc            |  6 +++++-
> > >  gcc/testsuite/gcc.dg/vect/pr115840.c | 11 +++++++++++
> > >  2 files changed, 16 insertions(+), 1 deletion(-)
> > >  create mode 100644 gcc/testsuite/gcc.dg/vect/pr115840.c
> > >
> > > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > > index 61fa74e9322..08159d7cbbc 100644
> > > --- a/gcc/config/riscv/riscv.cc
> > > +++ b/gcc/config/riscv/riscv.cc
> > > @@ -11431,7 +11431,11 @@ riscv_preferred_else_value (unsigned ifn, tree vectype, unsigned int nops,
> > >                             tree *ops)
> > >  {
> > >    if (riscv_v_ext_mode_p (TYPE_MODE (vectype)))
> > > -    return get_or_create_ssa_default_def (cfun, create_tmp_var (vectype));
> > > +    {
> > > +      tree tmp_var = create_tmp_var (vectype);
> > > +      TREE_ALLOW_UNINITIALIZED (tmp_var) = 1;
> >
> > Does it work when you do
> >
> >  TREE_NO_WARNING (tmp_var) = 1;
> >
>
> Thanks.  It works.  I did notice it, while I worried that there may be
> some other
> warnings, that TREE_NO_WARNING may cover them.

That's quite unlikely in this case but yes, TREE_NO_WARNING is a big hammer.

> > ?
> >
> > > +      return get_or_create_ssa_default_def (cfun, tmp_var);
> > > +    }
> > >
> > >    return default_preferred_else_value (ifn, vectype, nops, ops);
> > >  }
> > > diff --git a/gcc/testsuite/gcc.dg/vect/pr115840.c b/gcc/testsuite/gcc.dg/vect/pr115840.c
> > > new file mode 100644
> > > index 00000000000..09dc9e4eb7c
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/vect/pr115840.c
> > > @@ -0,0 +1,11 @@
> > > +/* { dg-do compile } */
> > > +/* { dg-additional-options "-Wall -Werror" } */
> > > +
> > > +double loads[16];
> > > +
> > > +void
> > > +foo (double loadavg[], int count)
> > > +{
> > > +  for (int i = 0; i < count; i++)
> > > +    loadavg[i] = loads[i] / 1.5;
> > > +}
> > > --
> > > 2.45.1
> > >
diff mbox series

Patch

diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 61fa74e9322..08159d7cbbc 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -11431,7 +11431,11 @@  riscv_preferred_else_value (unsigned ifn, tree vectype, unsigned int nops,
 			    tree *ops)
 {
   if (riscv_v_ext_mode_p (TYPE_MODE (vectype)))
-    return get_or_create_ssa_default_def (cfun, create_tmp_var (vectype));
+    {
+      tree tmp_var = create_tmp_var (vectype);
+      TREE_ALLOW_UNINITIALIZED (tmp_var) = 1;
+      return get_or_create_ssa_default_def (cfun, tmp_var);
+    }
 
   return default_preferred_else_value (ifn, vectype, nops, ops);
 }
diff --git a/gcc/testsuite/gcc.dg/vect/pr115840.c b/gcc/testsuite/gcc.dg/vect/pr115840.c
new file mode 100644
index 00000000000..09dc9e4eb7c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr115840.c
@@ -0,0 +1,11 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-Wall -Werror" } */
+
+double loads[16];
+
+void
+foo (double loadavg[], int count)
+{
+  for (int i = 0; i < count; i++)
+    loadavg[i] = loads[i] / 1.5;
+}