Bug 91734 - gcc skip an if statement with "-O1 -ffast-math"
Summary: gcc skip an if statement with "-O1 -ffast-math"
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 9.2.0
: P3 normal
Target Milestone: ---
Assignee: Jakub Jelinek
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-09-11 08:44 UTC by Chinoune
Modified: 2019-10-21 11:48 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2019-09-11 00:00:00


Attachments
gcc10-pr91734.patch (1.23 KB, patch)
2019-09-11 15:35 UTC, Jakub Jelinek
Details | Diff
gcc10-pr91734.patch (1.82 KB, patch)
2019-09-12 09:26 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chinoune 2019-09-11 08:44:30 UTC
Building this program with gcc [8,9] "-O1 -ffast-math" makes the result overflows.

#include <stdio.h>
#include <float.h>
#include <math.h>
#include <complex.h>

void foo(float complex z, float *y){
    float const sqrt_flt_min = sqrtf( FLT_MIN );
    float az;
    az = cabsf(z);
    //az = cabsf(z/sqrt_flt_min) * sqrt_flt_min;
    if( az<FLT_MIN ) az = cabsf(z/sqrt_flt_min) * sqrt_flt_min;
    printf("az = %.8e\n",az);
    *y = 1.f/az;
}

int main(){
    float complex z;
    float y;

    z = 1.e-19f + 0.*I;
    foo(z,&y);
    printf("y = %.7e\n",y);

    return 0;
}

gcc-9 -O1 -fno-math-errno -funsafe-math-optimizations -ffinite-math-only -fdump-tree-optimized cbug_skip_if.c -o test.x -lm
./test.x
az = 0.00000000e+00
y = inf

cbug_skip_if.c.231t.optimized

;; Function foo (foo, funcdef_no=32, decl_uid=4150, cgraph_uid=33, symbol_order=32)

foo (complex float z, float * y)
{
  double _3;
  float _4;
  float cabs_17;
  float cabs_18;
  float cabs_19;
  float cabs_20;
  float cabs_21;
  float powroot_22;

  <bb 2> [local count: 1073741824]:
  cabs_17 = REALPART_EXPR <z_6(D)>;
  cabs_18 = cabs_17 * cabs_17;
  cabs_19 = IMAGPART_EXPR <z_6(D)>;
  cabs_20 = cabs_19 * cabs_19;
  cabs_21 = cabs_18 + cabs_20;
  powroot_22 = __builtin_sqrtf (cabs_21);
  _3 = (double) powroot_22;
  __printf_chk (1, "az = %.8e\n", _3);
  _4 = 1.0e+0 / powroot_22;
  *y_10(D) = _4;
  return;

}

;; Function main (main, funcdef_no=33, decl_uid=4154, cgraph_uid=34, symbol_order=33) (executed once)

main ()
{
  float y;
  float y.0_1;
  double _2;

  <bb 2> [local count: 1073741824]:
  foo (__complex__ (9.9999996826552253889678874634872052240552875446155667305e-20, 0.0), &y);
  y.0_1 = y;
  _2 = (double) y.0_1;
  __printf_chk (1, "y = %.7e\n", _2);
  y ={v} {CLOBBER};
  return 0;

}

There is no if statement!
Comment 1 Andrew Pinski 2019-09-11 09:41:24 UTC
So two things:
-funsafe-math-optimizations
assumes there are no denormals (subnormals) or they are flushed to zero.

-ffinite-math-only
assumes that infinite and nans don't exists (IIRC).
Comment 2 Chinoune 2019-09-11 11:13:24 UTC
Replacing `if( az<FLT_MIN)` with `if( az==0.f )` gives a correct result.
Comment 3 Jakub Jelinek 2019-09-11 13:57:38 UTC
This is:
        /* sqrt(x) < c is the same as x < c*c, if we ignore NaNs.  */
        (if (! HONOR_NANS (@0))
         (cmp @0 { build_real (TREE_TYPE (@0), c2); })
         /* sqrt(x) < c is the same as x >= 0 && x < c*c.  */
         (if (GENERIC)
          (truth_andif
           (ge @0 { build_real (TREE_TYPE (@0), dconst0); })
           (cmp @0 { build_real (TREE_TYPE (@0), c2); })))))))))
c here is 1.17549435082228750796873653722224567781866555677208752151e-38f,
so c2 is 0.0f because the product is smaller than smallest positive subnormal float.  Obviously, in that case we can't use < c2, but need to use <= c2.

Maybe we probably should compute c2 with rounding towards positive infinity, then we could keep using < c2.
Comment 4 Jakub Jelinek 2019-09-11 14:23:18 UTC
Likely introduced in r64619.
Comment 5 Jakub Jelinek 2019-09-11 14:52:06 UTC
Even in the default rounding mode, cases where c2 is equal to zero are clearly problematic as this testcase shows, but also cases where c is subnormal.
E.g.
sqrtf (x) < 0x1.2dd3d0p-65f is true for x 0x1.63dbc0p-130f, because sqrtf (0x1.63dbc0p-130f) == 0x1.2dd3cep-65.  But c2 is 0x1.63dbc0p-130f and so
0x1.63dbc0p-130f < 0x1.63dbc0p-130f is false.

So, 1) shall we somehow guard some of these optimizations on !HONOR_SIGN_DEPENDENT_ROUNDING (or is flag_unsafe_math_optimizations it is guarded on incompatible with -frounding-math)?
2) after computing the c2, try to real_sqrt it again and if cmp is LT_EXPR or GE_EXPR, adjust cmp depending on how the boundary value compares?
Comment 6 Jakub Jelinek 2019-09-11 15:35:44 UTC
Created attachment 46871 [details]
gcc10-pr91734.patch

Untested patch that does the 2), though for LE_EXPR only.
Comment 7 Richard Biener 2019-09-12 08:26:58 UTC
flag_unsafe_math_optimizations "trumps" everything to some extent, so yes,
I'd say -funsafe-math-optimizations -frounding-math doesn't make much sense.
Comment 8 Jakub Jelinek 2019-09-12 09:26:31 UTC
Created attachment 46874 [details]
gcc10-pr91734.patch

Updated patch that also handles GE_EXPR comparisons.
Comment 9 Jakub Jelinek 2019-10-05 07:36:41 UTC
Author: jakub
Date: Sat Oct  5 07:36:09 2019
New Revision: 276621

URL: https://gcc.gnu.org/viewcvs?rev=276621&root=gcc&view=rev
Log:
	PR tree-optimization/91734
	* generic-match-head.c: Include fold-const-call.h.
	* match.pd (sqrt(x) cmp c): Check the boundary value and
	in case inexact computation of c*c affects comparison of the boundary,
	turn LT_EXPR into LE_EXPR, GE_EXPR into GT_EXPR, LE_EXPR into LT_EXPR
	or GT_EXPR into GE_EXPR.  Punt for sqrt comparisons against NaN and
	for -frounding-math.  For c2, try the next smaller or larger floating
	point constant depending on comparison code and if it has the same
	sqrt as c2, use it instead of c2.

	* gcc.dg/pr91734.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr91734.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/generic-match-head.c
    trunk/gcc/match.pd
    trunk/gcc/testsuite/ChangeLog
Comment 10 Chinoune 2019-10-07 18:01:35 UTC
Thanks for fixing.
Comment 11 Jakub Jelinek 2019-10-21 11:48:32 UTC
Author: jakub
Date: Mon Oct 21 11:48:00 2019
New Revision: 277257

URL: https://gcc.gnu.org/viewcvs?rev=277257&root=gcc&view=rev
Log:
	Backported from mainline
	2019-10-05  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/91734
	* generic-match-head.c: Include fold-const-call.h.
	* match.pd (sqrt(x) cmp c): Check the boundary value and
	in case inexact computation of c*c affects comparison of the boundary,
	turn LT_EXPR into LE_EXPR, GE_EXPR into GT_EXPR, LE_EXPR into LT_EXPR
	or GT_EXPR into GE_EXPR.  Punt for sqrt comparisons against NaN and
	for -frounding-math.  For c2, try the next smaller or larger floating
	point constant depending on comparison code and if it has the same
	sqrt as c2, use it instead of c2.

	* gcc.dg/pr91734.c: New test.

Added:
    branches/gcc-9-branch/gcc/testsuite/gcc.dg/pr91734.c
Modified:
    branches/gcc-9-branch/gcc/ChangeLog
    branches/gcc-9-branch/gcc/generic-match-head.c
    branches/gcc-9-branch/gcc/match.pd
    branches/gcc-9-branch/gcc/testsuite/ChangeLog