Patchwork Fix folding of logb (-Inf) (PR tree-optimization/57066)

login
register
mail settings
Submitter Marek Polacek
Date April 25, 2013, 3:16 p.m.
Message ID <20130425151630.GO13346@redhat.com>
Download mbox | patch
Permalink /patch/239542/
State New
Headers show

Comments

Marek Polacek - April 25, 2013, 3:16 p.m.
On Thu, Apr 25, 2013 at 03:44:08PM +0200, Richard Biener wrote:
> On Thu, Apr 25, 2013 at 3:23 PM, Marek Polacek <polacek@redhat.com> wrote:
> > This is an attempt to fix PR57066, where when folding logb call,
> > we returned -Inf for logb(-Inf), which is not correct.
> >
> > I had to adjust one testcase, because it checked for a wrong value.
> >
> > What I don't know yet is what we should return for -Nan, it should be
> > in C9X standard, but I don't have it by hand.
> >
> > Does this look sane?  So far regtested/bootstrapped on x86_64-linux.
> >
> > 2013-04-25  Marek Polacek  <polacek@redhat.com>
> >
> >         * builtins.c (fold_builtin_logb): Return +Inf for -Inf.
> >
> >         * gcc.dg/torture/builtin-logb-1.c: Adjust testcase.
> >
> > --- gcc/builtins.c.mp   2013-04-25 12:52:37.463451032 +0200
> > +++ gcc/builtins.c      2013-04-25 14:26:33.963099851 +0200
> > @@ -9698,7 +9698,12 @@ fold_builtin_logb (location_t loc, tree
> >        case rvc_inf:
> >         /* If arg is Inf or NaN and we're logb, return it.  */
> >         if (TREE_CODE (rettype) == REAL_TYPE)
> > -         return fold_convert_loc (loc, rettype, arg);
> > +         {
> > +           /* For logb(-Inf) we have to return +Inf.  */
> > +           if (value->cl == rvc_inf && !tree_expr_nonnegative_p (arg))
> > +             real_inf (TREE_REAL_CST_PTR (arg));
> 
> Please don't modify arg in-place but simply use
> 
>              REAL_VALUE_TYPE tem;
>              real_inf (&tem);
>              return build_real (rettype, tem);
> 

Okay, patch updated.  Ok now (which branches, all active?)?
Regtested/bootstrapped again on x86_64-linux.

2013-04-25  Marek Polacek  <polacek@redhat.com>

        * builtins.c (fold_builtin_logb): Return +Inf for -Inf.

        * gcc.dg/torture/builtin-logb-1.c: Adjust testcase.


	Marek

Patch

--- gcc/builtins.c.mp	2013-04-25 12:52:37.463451032 +0200
+++ gcc/builtins.c	2013-04-25 16:29:59.054615841 +0200
@@ -9698,7 +9698,16 @@  fold_builtin_logb (location_t loc, tree
       case rvc_inf:
 	/* If arg is Inf or NaN and we're logb, return it.  */
 	if (TREE_CODE (rettype) == REAL_TYPE)
-	  return fold_convert_loc (loc, rettype, arg);
+	  {
+	    /* For logb(-Inf) we have to return +Inf.  */
+	    if (value->cl == rvc_inf && !tree_expr_nonnegative_p (arg))
+	      {
+		REAL_VALUE_TYPE tem;
+		real_inf (&tem);
+		return build_real (rettype, tem);
+	      }
+	    return fold_convert_loc (loc, rettype, arg);
+	  }
 	/* Fall through... */
       case rvc_zero:
 	/* Zero may set errno and/or raise an exception for logb, also
--- gcc/testsuite/gcc.dg/torture/builtin-logb-1.c.mp	2013-04-25 13:23:18.408224450 +0200
+++ gcc/testsuite/gcc.dg/torture/builtin-logb-1.c	2013-04-25 16:29:19.842490916 +0200
@@ -48,19 +48,19 @@  extern void link_error(int);
 /* Test if FUNCRES(FUNC(NEG FUNCARG(ARGARG))) is false.  Check the
    sign as well.  */
 #ifndef __SPU__
-#define TESTIT3(FUNC,NEG,FUNCARG,ARGARG,FUNCRES) do { \
+#define TESTIT3(FUNC,NEG,FUNCARG,ARGARG,FUNCRES,NEG2) do { \
   if (!__builtin_##FUNCRES##f(__builtin_##FUNC(NEG __builtin_##FUNCARG##f(ARGARG))) \
-      || CKSGN_F(__builtin_##FUNC##f(NEG __builtin_##FUNCARG##f(ARGARG)), NEG __builtin_##FUNCARG##f(ARGARG))) \
+      || CKSGN_F(__builtin_##FUNC##f(NEG __builtin_##FUNCARG##f(ARGARG)), NEG2 __builtin_##FUNCARG##f(ARGARG))) \
     link_error(__LINE__); \
   if (!__builtin_##FUNCRES(__builtin_##FUNC(NEG __builtin_##FUNCARG(ARGARG))) \
-      || CKSGN(__builtin_##FUNC(NEG __builtin_##FUNCARG(ARGARG)), NEG __builtin_##FUNCARG(ARGARG))) \
+      || CKSGN(__builtin_##FUNC(NEG __builtin_##FUNCARG(ARGARG)), NEG2 __builtin_##FUNCARG(ARGARG))) \
     link_error(__LINE__); \
   if (!__builtin_##FUNCRES##l(__builtin_##FUNC##l(NEG __builtin_##FUNCARG##l(ARGARG))) \
-      || CKSGN_L(__builtin_##FUNC##l(NEG __builtin_##FUNCARG##l(ARGARG)), NEG __builtin_##FUNCARG##l(ARGARG))) \
+      || CKSGN_L(__builtin_##FUNC##l(NEG __builtin_##FUNCARG##l(ARGARG)), NEG2 __builtin_##FUNCARG##l(ARGARG))) \
     link_error(__LINE__); \
   } while (0)
 #else
-#define TESTIT3(FUNC,NEG,FUNCARG,ARGARG,FUNCRES) do { \
+#define TESTIT3(FUNC,NEG,FUNCARG,ARGARG,FUNCRES,NEG2) do { \
   /* SPU single-precision floating point format does not support Inf or Nan.  */ \
   if (!__builtin_##FUNCRES(__builtin_##FUNC(NEG __builtin_##FUNCARG(ARGARG))) \
       || CKSGN(__builtin_##FUNC(NEG __builtin_##FUNCARG(ARGARG)), NEG __builtin_##FUNCARG(ARGARG))) \
@@ -173,15 +173,15 @@  foo(void)
 
   /* Test for f(+-Inf) -> +-Inf and f(+-NaN) -> +-NaN, regardless of
      the radix.  */
-  TESTIT3 (logb, ,inf, , isinf);
-  TESTIT3 (logb, - ,inf, , isinf);
-  TESTIT3 (logb,  ,nan, "", isnan);
-  TESTIT3 (logb, - ,nan, "", isnan);
-
-  TESTIT3 (significand, ,inf, , isinf);
-  TESTIT3 (significand, - ,inf, , isinf);
-  TESTIT3 (significand,  ,nan, "", isnan);
-  TESTIT3 (significand, - ,nan, "", isnan);
+  TESTIT3 (logb, ,inf, , isinf, );
+  TESTIT3 (logb, - ,inf, , isinf, );
+  TESTIT3 (logb,  ,nan, "", isnan, );
+  TESTIT3 (logb, - ,nan, "", isnan, -);
+
+  TESTIT3 (significand, ,inf, , isinf, );
+  TESTIT3 (significand, - ,inf, , isinf, -);
+  TESTIT3 (significand,  ,nan, "", isnan, );
+  TESTIT3 (significand, - ,nan, "", isnan, -);
 }
 
 int main()