From: Pan Li <pan2...@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. The below tests are passed for this patch: * The riscv regression tests. * The aarch64 regression tests. * The x86 bootstrap and regression tests. 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: Adjust tests cases for different scenarios. Signed-off-by: Pan Li <pan2...@intel.com> --- gcc/loop-unroll.cc | 4 +-- gcc/testsuite/gcc.dg/pr30957-1.c | 48 ++++++++++++++++++++++++++++---- 2 files changed, 44 insertions(+), 8 deletions(-) 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 index 564410913ab..6a9d3d87932 100644 --- a/gcc/testsuite/gcc.dg/pr30957-1.c +++ b/gcc/testsuite/gcc.dg/pr30957-1.c @@ -20,16 +20,52 @@ foo (float d, int n) return accum; } +float __attribute__((noinline)) +get_minus_zero() +{ + return 0.0 / -5.0; +} + 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) + /* The variable expansion in unroll requires option unsafe-math-optimizations + (aka -fno-signed-zeros, -fno-trapping-math, -fassociative-math + and -freciprocal-math). + + When loop like above will have expansion after unrolling as below: + + accum_1 += d_1; + accum_2 += d_2; + accum_3 += d_3; + ... + + The accum_1, accum_2 and accum_3 need to be initialized. Given the + floating-point we have + +0.0f + -0.0f = +0.0f. + + Thus, we should initialize the accum_* to -0.0 for correctness. But + the things become more complicated when no-signed-zeros, as well as VLA + vectorizer mode which doesn't trigger variable expansion. Then we have: + + Case 1: Trigger variable expansion but target doesn't honor no-signed-zero. + minus_zero will be -0.0f and foo (minus_zero, 10) will be -0.0f. + Case 2: Trigger variable expansion but target does honor no-signed-zero. + minus_zero will be +0.0f and foo (minus_zero, 10) will be +0.0f. + Case 3: No variable expansion but target doesn't honor no-signed-zero. + minus_zero will be -0.0f and foo (minus_zero, 10) will be -0.0f. + Case 4: No variable expansion but target does honor no-signed-zero. + minus_zero will be +0.0f and foo (minus_zero, 10) will be +0.0f. + + The test case covers above 4 cases for running. + */ + float minus_zero = get_minus_zero (); + float a = __builtin_copysignf (1.0, minus_zero); + float b = __builtin_copysignf (1.0, foo (minus_zero, 10)); + + if (a != b) abort (); + exit (0); } -- 2.34.1