[PATCH]: Address PR middle-end/35509 by adding a separate isinf_sign builtin

["Kaveh R. GHAZI" <ghazi at caip dot rutgers dot edu>]

This patch addresses PR middle-end/35509, which was a report about the
sign of the return value from builtin isinf with -Inf.  The PR's
resolution was that since C99 makes no guarantees about the non-zero
return value from isinf, the PR was closed as invalid.  While I agree with
this sentiment, I imagine this is unsatisfying to libc implementors who's
interfaces document -Inf yields a negative result.

So I am attempting to resolve this by creating a separate builtin named
isinf_sign() which results in the sign dependent behavior.  Libc
implementors can then chose whether to #define isinf(x) to
__builtin_isinf(x) or to __builtin_isinf_sign(x).

E.g. GCC on solaris10 could continue to use __builtin_isinf(), but I
suspect glibc could now use __builtin_isinf_sign() if desired.

This new builtin folds itself into the following code:

	isinf_sign(x) -> isinf(x) ? (signbit(x) ? -1 : 1) : 0

This idiom was suggested in the PR by Dmitry initially because it is
efficient (fast) in the common finite case returning 0.

However the interesting part is that in a boolean context, the inner
conditional expression using signbit folds to 1.  So if you say:

	if (__builtin_isinf_sign(x))

this will resolve to simply:

	if (isinf(x) ? 1 : 0)

which becomes:

	if (isinf(x))

And of course this takes advantage of the obtabs used for isinf. There are
more examples where bool-context folding can happen in the testcase I
created.

Thus in many (most?) situations isinf_sign is just as fast and compact as
plain isinf, and in the (rare?) cases where sign matters, the integer
context will preserve the sign value.

In the process of doing this, I had to teach operand_equal_p() how to
match COND_EXPRs so the testcase would pass.  I also added a sentence
about plain isinf and isnan to the docs since it was missing. Finally, I
needed the ability to select a builtin (signbit) from the *explicit*
builtin decl array (as opposed to the implicit one) so I modified
mathfn_built_in() to accept a parameter specifying implicit (or not).

Bootstrapped on x86_64-unknown-linux-gnu, no regressions and the new
testcases all pass.

Okay for mainline?

		Thanks,
		--Kaveh


2008-05-18  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	PR middle-end/35509

	* builtins.c (mathfn_built_in_1): Renamed from mathfn_built_in.
	Add `implicit' parameter.  Handle BUILT_IN_SIGNBIT.
	(mathfn_built_in): Rewrite in terms of mathfn_built_in_1.
	(fold_builtin_classify): Handle BUILT_IN_ISINF_SIGN.
	(fold_builtin_1): Likewise.
	* builtins.def (BUILT_IN_ISINF_SIGN): New.
	c-common.c (check_builtin_function_arguments): Handle
	BUILT_IN_ISINF_SIGN.
	* doc/extend.texi: Document __builtin_isinf_sign.
	* fold-const.c (operand_equal_p): Handle COND_EXPR.

testsuite:
	* gcc.dg/builtins-error.c: Test __builtin_isinf_sign.
	* gcc.dg/tg-tests.h: Likewise.  Mark variables volatile.
	* gcc.dg/torture/builtin-isinf_sign-1.c: New test.

diff -rup orig/egcc-SVN20080517/gcc/builtins.c egcc-SVN20080517/gcc/builtins.c
--- orig/egcc-SVN20080517/gcc/builtins.c	2008-05-15 02:02:41.000000000 +0200
+++ egcc-SVN20080517/gcc/builtins.c	2008-05-18 00:37:29.000000000 +0200
@@ -1669,10 +1669,15 @@ expand_builtin_classify_type (tree exp)
   fcodel = BUILT_IN_MATHFN##L_R ; break;

 /* Return mathematic function equivalent to FN but operating directly
-   on TYPE, if available.  If we can't do the conversion, return zero.  */
-tree
-mathfn_built_in (tree type, enum built_in_function fn)
+   on TYPE, if available.  If IMPLICIT is true find the function in
+   implicit_built_in_decls[], otherwise use built_in_decls[].  If we
+   can't do the conversion, return zero.  */
+
+static tree
+mathfn_built_in_1 (tree type, enum built_in_function fn, bool implicit)
 {
+  tree const *const fn_arr
+    = implicit ? implicit_built_in_decls : built_in_decls;
   enum built_in_function fcode, fcodef, fcodel;

   switch (fn)
@@ -1747,6 +1752,7 @@ mathfn_built_in (tree type, enum built_i
       CASE_MATHFN (BUILT_IN_SCALB)
       CASE_MATHFN (BUILT_IN_SCALBLN)
       CASE_MATHFN (BUILT_IN_SCALBN)
+      CASE_MATHFN (BUILT_IN_SIGNBIT)
       CASE_MATHFN (BUILT_IN_SIGNIFICAND)
       CASE_MATHFN (BUILT_IN_SIN)
       CASE_MATHFN (BUILT_IN_SINCOS)
@@ -1765,15 +1771,23 @@ mathfn_built_in (tree type, enum built_i
       }

   if (TYPE_MAIN_VARIANT (type) == double_type_node)
-    return implicit_built_in_decls[fcode];
+    return fn_arr[fcode];
   else if (TYPE_MAIN_VARIANT (type) == float_type_node)
-    return implicit_built_in_decls[fcodef];
+    return fn_arr[fcodef];
   else if (TYPE_MAIN_VARIANT (type) == long_double_type_node)
-    return implicit_built_in_decls[fcodel];
+    return fn_arr[fcodel];
   else
     return NULL_TREE;
 }

+/* Like mathfn_built_in_1(), but always use the implicit array.  */
+
+tree
+mathfn_built_in (tree type, enum built_in_function fn)
+{
+  return mathfn_built_in_1 (type, fn, /*implicit=*/ 1);
+}
+
 /* If errno must be maintained, expand the RTL to check if the result,
    TARGET, of a built-in function call, EXP, is NaN, and if so set
    errno to EDOM.  */
@@ -9668,6 +9682,37 @@ fold_builtin_classify (tree fndecl, tree

       return NULL_TREE;

+    case BUILT_IN_ISINF_SIGN:
+      {
+	/* isinf_sign(x) -> isinf(x) ? (signbit(x) ? -1 : 1) : 0 */
+	/* In a boolean context, GCC will fold the inner COND_EXPR to
+	   1.  So e.g. "if (isinf_sign(x))" would be folded to just
+	   "if (isinf(x) ? 1 : 0)" which becomes "if (isinf(x))". */
+	tree signbit_fn = mathfn_built_in_1 (TREE_TYPE (arg), BUILT_IN_SIGNBIT, 0);
+	tree isinf_fn = built_in_decls[BUILT_IN_ISINF];
+	tree tmp = NULL_TREE;
+
+	arg = builtin_save_expr (arg);
+
+	if (signbit_fn && isinf_fn)
+	  {
+	    tree signbit_call = build_call_expr (signbit_fn, 1, arg);
+	    tree isinf_call = build_call_expr (isinf_fn, 1, arg);
+
+	    signbit_call = fold_build2 (NE_EXPR, integer_type_node,
+					signbit_call, integer_zero_node);
+	    isinf_call = fold_build2 (NE_EXPR, integer_type_node,
+				      isinf_call, integer_zero_node);
+
+	    tmp = fold_build3 (COND_EXPR, integer_type_node, signbit_call,
+			       integer_minus_one_node, integer_one_node);
+	    tmp = fold_build3 (COND_EXPR, integer_type_node, isinf_call, tmp,
+			       integer_zero_node);
+	  }
+
+	return tmp;
+      }
+
     case BUILT_IN_ISFINITE:
       if (!HONOR_NANS (TYPE_MODE (TREE_TYPE (arg)))
 	  && !HONOR_INFINITIES (TYPE_MODE (TREE_TYPE (arg))))
@@ -10074,6 +10119,9 @@ fold_builtin_1 (tree fndecl, tree arg0,
     case BUILT_IN_ISINFD128:
       return fold_builtin_classify (fndecl, arg0, BUILT_IN_ISINF);

+    case BUILT_IN_ISINF_SIGN:
+      return fold_builtin_classify (fndecl, arg0, BUILT_IN_ISINF_SIGN);
+
     CASE_FLT_FN (BUILT_IN_ISNAN):
     case BUILT_IN_ISNAND32:
     case BUILT_IN_ISNAND64:
diff -rup orig/egcc-SVN20080517/gcc/builtins.def egcc-SVN20080517/gcc/builtins.def
--- orig/egcc-SVN20080517/gcc/builtins.def	2008-04-28 02:02:19.000000000 +0200
+++ egcc-SVN20080517/gcc/builtins.def	2008-05-18 00:37:29.000000000 +0200
@@ -655,6 +655,7 @@ DEF_EXT_LIB_BUILTIN    (BUILT_IN_FINITED
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_FINITED64, "finited64", BT_FN_INT_DFLOAT64, ATTR_CONST_NOTHROW_LIST)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_FINITED128, "finited128", BT_FN_INT_DFLOAT128, ATTR_CONST_NOTHROW_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_ISFINITE, "isfinite", BT_FN_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC)
+DEF_GCC_BUILTIN        (BUILT_IN_ISINF_SIGN, "isinf_sign", BT_FN_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC)
 DEF_C99_C90RES_BUILTIN (BUILT_IN_ISINF, "isinf", BT_FN_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_ISINFF, "isinff", BT_FN_INT_FLOAT, ATTR_CONST_NOTHROW_LIST)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_ISINFL, "isinfl", BT_FN_INT_LONGDOUBLE, ATTR_CONST_NOTHROW_LIST)
diff -rup orig/egcc-SVN20080517/gcc/c-common.c egcc-SVN20080517/gcc/c-common.c
--- orig/egcc-SVN20080517/gcc/c-common.c	2008-05-10 02:02:39.000000000 +0200
+++ egcc-SVN20080517/gcc/c-common.c	2008-05-18 00:37:29.000000000 +0200
@@ -6674,6 +6674,7 @@ check_builtin_function_arguments (tree f

     case BUILT_IN_ISFINITE:
     case BUILT_IN_ISINF:
+    case BUILT_IN_ISINF_SIGN:
     case BUILT_IN_ISNAN:
     case BUILT_IN_ISNORMAL:
       if (validate_nargs (fndecl, nargs, 1))
diff -rup orig/egcc-SVN20080517/gcc/doc/extend.texi egcc-SVN20080517/gcc/doc/extend.texi
--- orig/egcc-SVN20080517/gcc/doc/extend.texi	2008-05-03 02:02:29.000000000 +0200
+++ egcc-SVN20080517/gcc/doc/extend.texi	2008-05-18 05:54:18.000000000 +0200
@@ -5768,6 +5768,7 @@ should be called and the @var{flag} argu
 @findex __builtin_isnormal
 @findex __builtin_isgreater
 @findex __builtin_isgreaterequal
+@findex __builtin_isinf_sign
 @findex __builtin_isless
 @findex __builtin_islessequal
 @findex __builtin_islessgreater
@@ -6294,8 +6295,10 @@ the same names as the standard macros (
 @code{islessgreater}, and @code{isunordered}) , with @code{__builtin_}
 prefixed.  We intend for a library implementor to be able to simply
 @code{#define} each standard macro to its built-in equivalent.
-In the same fashion, GCC provides @code{isfinite} and @code{isnormal}
-built-ins used with @code{__builtin_} prefixed.
+In the same fashion, GCC provides @code{isfinite}, @code{isinf_sign}
+and @code{isnormal} built-ins used with @code{__builtin_} prefixed.
+The @code{isinf} and @code{isnan} builtins appear both with and
+without the @code{__builtin_} prefix.

 @deftypefn {Built-in Function} int __builtin_types_compatible_p (@var{type1}, @var{type2})

diff -rup orig/egcc-SVN20080517/gcc/fold-const.c egcc-SVN20080517/gcc/fold-const.c
--- orig/egcc-SVN20080517/gcc/fold-const.c	2008-05-14 02:02:29.000000000 +0200
+++ egcc-SVN20080517/gcc/fold-const.c	2008-05-18 00:37:29.000000000 +0200
@@ -3258,6 +3258,9 @@ operand_equal_p (const_tree arg0, const_
 		  && operand_equal_p (TREE_OPERAND (arg0, 1),
 				      TREE_OPERAND (arg1, 0), flags));

+	case COND_EXPR:
+	  return OP_SAME (0) && OP_SAME (1) && OP_SAME (2);
+
 	default:
 	  return 0;
 	}
diff -rup orig/egcc-SVN20080517/gcc/testsuite/gcc.dg/builtins-error.c egcc-SVN20080517/gcc/testsuite/gcc.dg/builtins-error.c
--- orig/egcc-SVN20080517/gcc/testsuite/gcc.dg/builtins-error.c	2008-04-25 02:01:52.000000000 +0200
+++ egcc-SVN20080517/gcc/testsuite/gcc.dg/builtins-error.c	2008-05-18 00:37:29.000000000 +0200
@@ -16,3 +16,8 @@ int test3(double x)
 {
   return __builtin_isinf(x, x); /* { dg-error "too many arguments" } */
 }
+
+int test4(double x)
+{
+  return __builtin_isinf_sign(x, x); /* { dg-error "too many arguments" } */
+}
diff -rup orig/egcc-SVN20080517/gcc/testsuite/gcc.dg/tg-tests.h egcc-SVN20080517/gcc/testsuite/gcc.dg/tg-tests.h
--- orig/egcc-SVN20080517/gcc/testsuite/gcc.dg/tg-tests.h	2008-03-14 00:38:59.000000000 +0100
+++ egcc-SVN20080517/gcc/testsuite/gcc.dg/tg-tests.h	2008-05-18 00:37:29.000000000 +0200
@@ -3,7 +3,7 @@
 void __attribute__ ((__noinline__))
 foo_1 (float f, double d, long double ld,
        int res_unord, int res_isnan, int res_isinf,
-       int res_isfin, int res_isnorm)
+       int res_isinf_sign, int res_isfin, int res_isnorm)
 {
   if (__builtin_isunordered (f, 0) != res_unord)
     __builtin_abort ();
@@ -40,6 +40,13 @@ foo_1 (float f, double d, long double ld
   if (__builtin_isinfl (ld) != res_isinf)
     __builtin_abort ();

+  if (__builtin_isinf_sign (f) != res_isinf_sign)
+    __builtin_abort ();
+  if (__builtin_isinf_sign (d) != res_isinf_sign)
+    __builtin_abort ();
+  if (__builtin_isinf_sign (ld) != res_isinf_sign)
+    __builtin_abort ();
+
   if (__builtin_isnormal (f) != res_isnorm)
     __builtin_abort ();
   if (__builtin_isnormal (d) != res_isnorm)
@@ -71,17 +78,17 @@ foo (float f, double d, long double ld,
      int res_unord, int res_isnan, int res_isinf,
      int res_isfin, int res_isnorm)
 {
-  foo_1 (f, d, ld, res_unord, res_isnan, res_isinf, res_isfin, res_isnorm);
+  foo_1 (f, d, ld, res_unord, res_isnan, res_isinf, res_isinf, res_isfin, res_isnorm);
   /* Try all the values negated as well.  */
-  foo_1 (-f, -d, -ld, res_unord, res_isnan, res_isinf, res_isfin, res_isnorm);
+  foo_1 (-f, -d, -ld, res_unord, res_isnan, res_isinf, -res_isinf, res_isfin, res_isnorm);
 }

 int __attribute__ ((__noinline__))
 main_tests (void)
 {
-  float f;
-  double d;
-  long double ld;
+  volatile float f;
+  volatile double d;
+  volatile long double ld;

   /* Test NaN.  */
   f = __builtin_nanf(""); d = __builtin_nan(""); ld = __builtin_nanl("");
diff -rup orig/egcc-SVN20080517/gcc/testsuite/gcc.dg/torture/builtin-isinf_sign-1.c egcc-SVN20080517/gcc/testsuite/gcc.dg/torture/builtin-isinf_sign-1.c
--- orig/egcc-SVN20080517/gcc/testsuite/gcc.dg/torture/builtin-isinf_sign-1.c	2008-05-18 05:37:45.000000000 +0200
+++ egcc-SVN20080517/gcc/testsuite/gcc.dg/torture/builtin-isinf_sign-1.c	2008-05-18 06:03:23.000000000 +0200
@@ -0,0 +1,56 @@
+/* Copyright (C) 2008  Free Software Foundation.
+
+   Verify that __builtin_isinf_sign folds correctly.
+
+   Origin: Kaveh R. Ghazi,  May 17, 2008.  */
+
+/* { dg-do link } */
+
+/* All references to link_error should go away at compile-time.  */
+extern void link_error(int);
+
+void __attribute__ ((__noinline__))
+foo (float f, double d, long double ld)
+{
+  /* Test the generic expansion of isinf_sign.  */
+
+  if (__builtin_isinf_sign(f)
+      != (__builtin_isinf(f) ? (__builtin_signbitf(f) ? -1 : 1) : 0))
+    link_error (__LINE__);
+  if (__builtin_isinf_sign(d)
+      != (__builtin_isinf(d) ? (__builtin_signbit(d) ? -1 : 1) : 0))
+    link_error (__LINE__);
+  if (__builtin_isinf_sign(ld)
+      != (__builtin_isinf(ld) ? (__builtin_signbitl(ld) ? -1 : 1) : 0))
+    link_error (__LINE__);
+
+  /* In boolean contexts, GCC will fold the inner conditional
+     expression to 1.  So isinf_sign folds to plain isinf.  */
+
+  if ((_Bool)__builtin_isinf_sign(f) != (__builtin_isinf(f) != 0))
+    link_error (__LINE__);
+  if ((_Bool)__builtin_isinf_sign(d) != (__builtin_isinf(d) != 0))
+    link_error (__LINE__);
+  if ((_Bool)__builtin_isinf_sign(ld) != (__builtin_isinf(ld) != 0))
+    link_error (__LINE__);
+
+  if ((__builtin_isinf_sign(f) != 0) != (__builtin_isinf(f) != 0))
+    link_error (__LINE__);
+  if ((__builtin_isinf_sign(d) != 0) != (__builtin_isinf(d) != 0))
+    link_error (__LINE__);
+  if ((__builtin_isinf_sign(ld) != 0) != (__builtin_isinf(ld) != 0))
+    link_error (__LINE__);
+
+  if ((__builtin_isinf_sign(f) ? 5 : 6) != (__builtin_isinf(f) ? 5 : 6))
+    link_error (__LINE__);
+  if ((__builtin_isinf_sign(d) ? 5 : 6) != (__builtin_isinf(d) ? 5 : 6))
+    link_error (__LINE__);
+  if ((__builtin_isinf_sign(ld) ? 5 : 6) != (__builtin_isinf(ld) ? 5 : 6))
+    link_error (__LINE__);
+}
+
+int main (void)
+{
+  foo (1, 2, 3);
+  return 0;
+}