| Message ID | 20240111085043.1246942-1-pan2.li@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] LOOP-UNROLL: Leverage HAS_SIGNED_ZERO for var expansion | expand |
On Thu, Jan 11, 2024 at 9:50 AM <pan2.li@intel.com> wrote: > > From: Pan Li <pan2.li@intel.com> > > The insert_var_expansion_initialization depends on the > HONOR_SIGNED_ZEROS to initialize the unrolling variables > to +0.0f when -0.0f and no-signed-option. Unfortunately, > we should always keep the -0.0f here because: > > * The -0.0f is always the correct initial value. > * We need to support the target that always honor signed zero. > > Thus, we need to leverage MODE_HAS_SIGNED_ZEROS when initialize > instead of HONOR_SIGNED_ZEROS. Then the target/backend can > decide to honor the no-signed-zero or not. > > We also removed the testcase pr30957-1.c, as it makes undefined behavior > whether the return value is positive or negative. > > The below tests are passed for this patch: > > * The riscv regression tests. > * The aarch64 regression tests. > * The x86 bootstrap and regression tests. OK > gcc/ChangeLog: > > * loop-unroll.cc (insert_var_expansion_initialization): Leverage > MODE_HAS_SIGNED_ZEROS for expansion variable initialization. > > gcc/testsuite/ChangeLog: > > * gcc.dg/pr30957-1.c: Remove. > > Signed-off-by: Pan Li <pan2.li@intel.com> > --- > gcc/loop-unroll.cc | 4 ++-- > gcc/testsuite/gcc.dg/pr30957-1.c | 36 -------------------------------- > 2 files changed, 2 insertions(+), 38 deletions(-) > delete mode 100644 gcc/testsuite/gcc.dg/pr30957-1.c > > diff --git a/gcc/loop-unroll.cc b/gcc/loop-unroll.cc > index 4176a21e308..bfdfe6c2bb7 100644 > --- a/gcc/loop-unroll.cc > +++ b/gcc/loop-unroll.cc > @@ -1855,7 +1855,7 @@ insert_var_expansion_initialization (struct var_to_expand *ve, > rtx var, zero_init; > unsigned i; > machine_mode mode = GET_MODE (ve->reg); > - bool honor_signed_zero_p = HONOR_SIGNED_ZEROS (mode); > + bool has_signed_zero_p = MODE_HAS_SIGNED_ZEROS (mode); > > if (ve->var_expansions.length () == 0) > return; > @@ -1869,7 +1869,7 @@ insert_var_expansion_initialization (struct var_to_expand *ve, > case MINUS: > FOR_EACH_VEC_ELT (ve->var_expansions, i, var) > { > - if (honor_signed_zero_p) > + if (has_signed_zero_p) > zero_init = simplify_gen_unary (NEG, mode, CONST0_RTX (mode), mode); > else > zero_init = CONST0_RTX (mode); > diff --git a/gcc/testsuite/gcc.dg/pr30957-1.c b/gcc/testsuite/gcc.dg/pr30957-1.c > deleted file mode 100644 > index 564410913ab..00000000000 > --- a/gcc/testsuite/gcc.dg/pr30957-1.c > +++ /dev/null > @@ -1,36 +0,0 @@ > -/* { dg-do run { xfail { mmix-*-* } } } */ > -/* We don't (and don't want to) perform this optimisation on soft-float targets, > - where each addition is a library call. / > -/* { dg-require-effective-target hard_float } */ > -/* -fassociative-math requires -fno-trapping-math and -fno-signed-zeros. */ > -/* { dg-options "-O2 -funroll-loops -fassociative-math -fno-trapping-math -fno-signed-zeros -fvariable-expansion-in-unroller -fdump-rtl-loop2_unroll" } */ > - > -extern void abort (void); > -extern void exit (int); > - > -float __attribute__((noinline)) > -foo (float d, int n) > -{ > - unsigned i; > - float accum = d; > - > - for (i = 0; i < n; i++) > - accum += d; > - > - return accum; > -} > - > -int > -main () > -{ > - /* When compiling standard compliant we expect foo to return -0.0. But the > - variable expansion during unrolling optimization (for this testcase enabled > - by non-compliant -fassociative-math) instantiates copy(s) of the > - accumulator which it initializes with +0.0. Hence we expect that foo > - returns +0.0. */ > - if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0) > - abort (); > - exit (0); > -} > - > -/* { dg-final { scan-rtl-dump "Expanding Accumulator" "loop2_unroll" { xfail mmix-*-* } } } */ > -- > 2.34.1 >
Committed, thanks Richard. Pan -----Original Message----- From: Richard Biener <richard.guenther@gmail.com> Sent: Thursday, January 11, 2024 5:22 PM To: Li, Pan2 <pan2.li@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; jeffreyalaw@gmail.com Subject: Re: [PATCH v5] LOOP-UNROLL: Leverage HAS_SIGNED_ZERO for var expansion On Thu, Jan 11, 2024 at 9:50 AM <pan2.li@intel.com> wrote: > > From: Pan Li <pan2.li@intel.com> > > The insert_var_expansion_initialization depends on the > HONOR_SIGNED_ZEROS to initialize the unrolling variables > to +0.0f when -0.0f and no-signed-option. Unfortunately, > we should always keep the -0.0f here because: > > * The -0.0f is always the correct initial value. > * We need to support the target that always honor signed zero. > > Thus, we need to leverage MODE_HAS_SIGNED_ZEROS when initialize > instead of HONOR_SIGNED_ZEROS. Then the target/backend can > decide to honor the no-signed-zero or not. > > We also removed the testcase pr30957-1.c, as it makes undefined behavior > whether the return value is positive or negative. > > The below tests are passed for this patch: > > * The riscv regression tests. > * The aarch64 regression tests. > * The x86 bootstrap and regression tests. OK > gcc/ChangeLog: > > * loop-unroll.cc (insert_var_expansion_initialization): Leverage > MODE_HAS_SIGNED_ZEROS for expansion variable initialization. > > gcc/testsuite/ChangeLog: > > * gcc.dg/pr30957-1.c: Remove. > > Signed-off-by: Pan Li <pan2.li@intel.com> > --- > gcc/loop-unroll.cc | 4 ++-- > gcc/testsuite/gcc.dg/pr30957-1.c | 36 -------------------------------- > 2 files changed, 2 insertions(+), 38 deletions(-) > delete mode 100644 gcc/testsuite/gcc.dg/pr30957-1.c > > diff --git a/gcc/loop-unroll.cc b/gcc/loop-unroll.cc > index 4176a21e308..bfdfe6c2bb7 100644 > --- a/gcc/loop-unroll.cc > +++ b/gcc/loop-unroll.cc > @@ -1855,7 +1855,7 @@ insert_var_expansion_initialization (struct var_to_expand *ve, > rtx var, zero_init; > unsigned i; > machine_mode mode = GET_MODE (ve->reg); > - bool honor_signed_zero_p = HONOR_SIGNED_ZEROS (mode); > + bool has_signed_zero_p = MODE_HAS_SIGNED_ZEROS (mode); > > if (ve->var_expansions.length () == 0) > return; > @@ -1869,7 +1869,7 @@ insert_var_expansion_initialization (struct var_to_expand *ve, > case MINUS: > FOR_EACH_VEC_ELT (ve->var_expansions, i, var) > { > - if (honor_signed_zero_p) > + if (has_signed_zero_p) > zero_init = simplify_gen_unary (NEG, mode, CONST0_RTX (mode), mode); > else > zero_init = CONST0_RTX (mode); > diff --git a/gcc/testsuite/gcc.dg/pr30957-1.c b/gcc/testsuite/gcc.dg/pr30957-1.c > deleted file mode 100644 > index 564410913ab..00000000000 > --- a/gcc/testsuite/gcc.dg/pr30957-1.c > +++ /dev/null > @@ -1,36 +0,0 @@ > -/* { dg-do run { xfail { mmix-*-* } } } */ > -/* We don't (and don't want to) perform this optimisation on soft-float targets, > - where each addition is a library call. / > -/* { dg-require-effective-target hard_float } */ > -/* -fassociative-math requires -fno-trapping-math and -fno-signed-zeros. */ > -/* { dg-options "-O2 -funroll-loops -fassociative-math -fno-trapping-math -fno-signed-zeros -fvariable-expansion-in-unroller -fdump-rtl-loop2_unroll" } */ > - > -extern void abort (void); > -extern void exit (int); > - > -float __attribute__((noinline)) > -foo (float d, int n) > -{ > - unsigned i; > - float accum = d; > - > - for (i = 0; i < n; i++) > - accum += d; > - > - return accum; > -} > - > -int > -main () > -{ > - /* When compiling standard compliant we expect foo to return -0.0. But the > - variable expansion during unrolling optimization (for this testcase enabled > - by non-compliant -fassociative-math) instantiates copy(s) of the > - accumulator which it initializes with +0.0. Hence we expect that foo > - returns +0.0. */ > - if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0) > - abort (); > - exit (0); > -} > - > -/* { dg-final { scan-rtl-dump "Expanding Accumulator" "loop2_unroll" { xfail mmix-*-* } } } */ > -- > 2.34.1 >
diff --git a/gcc/loop-unroll.cc b/gcc/loop-unroll.cc index 4176a21e308..bfdfe6c2bb7 100644 --- a/gcc/loop-unroll.cc +++ b/gcc/loop-unroll.cc @@ -1855,7 +1855,7 @@ insert_var_expansion_initialization (struct var_to_expand *ve, rtx var, zero_init; unsigned i; machine_mode mode = GET_MODE (ve->reg); - bool honor_signed_zero_p = HONOR_SIGNED_ZEROS (mode); + bool has_signed_zero_p = MODE_HAS_SIGNED_ZEROS (mode); if (ve->var_expansions.length () == 0) return; @@ -1869,7 +1869,7 @@ insert_var_expansion_initialization (struct var_to_expand *ve, case MINUS: FOR_EACH_VEC_ELT (ve->var_expansions, i, var) { - if (honor_signed_zero_p) + if (has_signed_zero_p) zero_init = simplify_gen_unary (NEG, mode, CONST0_RTX (mode), mode); else zero_init = CONST0_RTX (mode); diff --git a/gcc/testsuite/gcc.dg/pr30957-1.c b/gcc/testsuite/gcc.dg/pr30957-1.c deleted file mode 100644 index 564410913ab..00000000000 --- a/gcc/testsuite/gcc.dg/pr30957-1.c +++ /dev/null @@ -1,36 +0,0 @@ -/* { dg-do run { xfail { mmix-*-* } } } */ -/* We don't (and don't want to) perform this optimisation on soft-float targets, - where each addition is a library call. / -/* { dg-require-effective-target hard_float } */ -/* -fassociative-math requires -fno-trapping-math and -fno-signed-zeros. */ -/* { dg-options "-O2 -funroll-loops -fassociative-math -fno-trapping-math -fno-signed-zeros -fvariable-expansion-in-unroller -fdump-rtl-loop2_unroll" } */ - -extern void abort (void); -extern void exit (int); - -float __attribute__((noinline)) -foo (float d, int n) -{ - unsigned i; - float accum = d; - - for (i = 0; i < n; i++) - accum += d; - - return accum; -} - -int -main () -{ - /* When compiling standard compliant we expect foo to return -0.0. But the - variable expansion during unrolling optimization (for this testcase enabled - by non-compliant -fassociative-math) instantiates copy(s) of the - accumulator which it initializes with +0.0. Hence we expect that foo - returns +0.0. */ - if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0) - abort (); - exit (0); -} - -/* { dg-final { scan-rtl-dump "Expanding Accumulator" "loop2_unroll" { xfail mmix-*-* } } } */