public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH]: middle-end, fix bug in fold_builtin_cexp using COMPLEX_FLOAT_TYPE_P
@ 2009-10-22  6:18 Kaveh R. GHAZI
  2009-10-22  9:59 ` Richard Guenther
  0 siblings, 1 reply; 3+ messages in thread
From: Kaveh R. GHAZI @ 2009-10-22  6:18 UTC (permalink / raw)
  To: gcc-patches

While reading the source code, I noticed a latent bug in builtins.c
fold_builtin_cexp.  I say "latent" because these are sanity checks and I
don't know of a way to trigger it just now.  The argument check does
something like this pseudo-code:

	if (! (type == COMPLEX_TYPE) && subtype == REAL_TYPE)
	  return NULL_TREE;

whereas it really should do:

	if (! (type == COMPLEX_TYPE && subtype == REAL_TYPE))
	  return NULL_TREE;

I.e. the '!' should cover both sides of the &&.

I decided to convert builtins.c to use the more compact and readable
COMPLEX_FLOAT_TYPE_P() macro which will hopefully avoid these kinds of
errors.  While doing it, I noticed that builtin cimag() was missing the
subtype check, so by doing this change that's fixed as well.

Note some of the places check the TREE_CODE manually, while others use
validate_arg(). They're essentially the same thing when considering
COMPLEX_TYPE.

Since these changes touch code using MPC, I tested it with MPC svn
revision 706, mpc-0.7 and without MPC.  No regressions.

Okay for mainline?

		Thanks,
		--Kaveh


2009-10-21  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* builtins.c (fold_builtin_cabs): Use COMPLEX_FLOAT_TYPE_P to
	check for a complex float type.
	(fold_builtin_ccos): Likewise.
	(fold_builtin_cexp): Likewise.
	(fold_builtin_carg): Likewise.
	(fold_builtin_1): Likewise.
	(fold_builtin_2): Likewise.

diff -rup orig/egcc-SVN20091021/gcc/builtins.c egcc-SVN20091021/gcc/builtins.c
--- orig/egcc-SVN20091021/gcc/builtins.c	2009-10-14 03:30:12.000000000 +0200
+++ egcc-SVN20091021/gcc/builtins.c	2009-10-21 22:41:22.000000000 +0200
@@ -7194,8 +7194,7 @@ fold_builtin_cabs (location_t loc, tree
 {
   tree res;

-  if (TREE_CODE (TREE_TYPE (arg)) != COMPLEX_TYPE
-      || TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != REAL_TYPE)
+  if (! COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
     return NULL_TREE;

   /* Calculate the result when the argument is a constant.  */
@@ -7484,8 +7483,7 @@ fold_builtin_ccos (location_t loc,
 		   tree arg, tree type ATTRIBUTE_UNUSED, tree fndecl,
 		   bool hyper ATTRIBUTE_UNUSED)
 {
-  if (validate_arg (arg, COMPLEX_TYPE)
-      && TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == REAL_TYPE)
+  if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
     {
       tree tmp;

@@ -7582,8 +7580,7 @@ fold_builtin_cexp (location_t loc, tree
   tree res;
 #endif

-  if (!validate_arg (arg0, COMPLEX_TYPE)
-      && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+  if (! COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
     return NULL_TREE;

 #ifdef HAVE_mpc
@@ -9363,8 +9360,7 @@ fold_builtin_fmin_fmax (location_t loc,
 static tree
 fold_builtin_carg (location_t loc, tree arg, tree type)
 {
-  if (validate_arg (arg, COMPLEX_TYPE)
-      && TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == REAL_TYPE)
+  if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
     {
       tree atan2_fn = mathfn_built_in (type, BUILT_IN_ATAN2);

@@ -10005,19 +10001,17 @@ fold_builtin_1 (location_t loc, tree fnd
       return fold_builtin_abs (loc, arg0, type);

     CASE_FLT_FN (BUILT_IN_CONJ):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	&& TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return fold_build1_loc (loc, CONJ_EXPR, type, arg0);
     break;

     CASE_FLT_FN (BUILT_IN_CREAL):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	&& TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return non_lvalue_loc (loc, fold_build1_loc (loc, REALPART_EXPR, type, arg0));;
     break;

     CASE_FLT_FN (BUILT_IN_CIMAG):
-      if (validate_arg (arg0, COMPLEX_TYPE))
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return non_lvalue_loc (loc, fold_build1_loc (loc, IMAGPART_EXPR, type, arg0));
     break;

@@ -10029,75 +10023,63 @@ fold_builtin_1 (location_t loc, tree fnd

 #ifdef HAVE_mpc
     CASE_FLT_FN (BUILT_IN_CSIN):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_sin);
     break;

     CASE_FLT_FN (BUILT_IN_CSINH):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_sinh);
     break;

     CASE_FLT_FN (BUILT_IN_CTAN):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_tan);
     break;

     CASE_FLT_FN (BUILT_IN_CTANH):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_tanh);
     break;

     CASE_FLT_FN (BUILT_IN_CLOG):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_log);
     break;

     CASE_FLT_FN (BUILT_IN_CSQRT):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_sqrt);
     break;

 #ifdef HAVE_mpc_arc
     CASE_FLT_FN (BUILT_IN_CASIN):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_asin);
     break;

     CASE_FLT_FN (BUILT_IN_CACOS):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_acos);
     break;

     CASE_FLT_FN (BUILT_IN_CATAN):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_atan);
     break;

     CASE_FLT_FN (BUILT_IN_CASINH):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_asinh);
     break;

     CASE_FLT_FN (BUILT_IN_CACOSH):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_acosh);
     break;

     CASE_FLT_FN (BUILT_IN_CATANH):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
 	return do_mpc_arg1 (arg0, type, mpc_atanh);
     break;
 #endif /* HAVE_mpc_arc */
@@ -10421,10 +10403,8 @@ fold_builtin_2 (location_t loc, tree fnd

 #ifdef HAVE_mpc_pow
     CASE_FLT_FN (BUILT_IN_CPOW):
-      if (validate_arg (arg0, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE
-	  && validate_arg (arg1, COMPLEX_TYPE)
-	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg1))) == REAL_TYPE)
+      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0))
+	  && COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg1)))
 	return do_mpc_arg2 (arg0, arg1, type, /*do_nonfinite=*/ 0, mpc_pow);
     break;
 #endif

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH]: middle-end, fix bug in fold_builtin_cexp using   COMPLEX_FLOAT_TYPE_P
  2009-10-22  6:18 [PATCH]: middle-end, fix bug in fold_builtin_cexp using COMPLEX_FLOAT_TYPE_P Kaveh R. GHAZI
@ 2009-10-22  9:59 ` Richard Guenther
  2009-10-23  9:09   ` Kaveh R. GHAZI
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Guenther @ 2009-10-22  9:59 UTC (permalink / raw)
  To: Kaveh R. GHAZI; +Cc: gcc-patches

On Thu, Oct 22, 2009 at 7:29 AM, Kaveh R. GHAZI <ghazi@caip.rutgers.edu> wrote:
> While reading the source code, I noticed a latent bug in builtins.c
> fold_builtin_cexp.  I say "latent" because these are sanity checks and I
> don't know of a way to trigger it just now.  The argument check does
> something like this pseudo-code:
>
>        if (! (type == COMPLEX_TYPE) && subtype == REAL_TYPE)
>          return NULL_TREE;
>
> whereas it really should do:
>
>        if (! (type == COMPLEX_TYPE && subtype == REAL_TYPE))
>          return NULL_TREE;
>
> I.e. the '!' should cover both sides of the &&.
>
> I decided to convert builtins.c to use the more compact and readable
> COMPLEX_FLOAT_TYPE_P() macro which will hopefully avoid these kinds of
> errors.  While doing it, I noticed that builtin cimag() was missing the
> subtype check, so by doing this change that's fixed as well.
>
> Note some of the places check the TREE_CODE manually, while others use
> validate_arg(). They're essentially the same thing when considering
> COMPLEX_TYPE.
>
> Since these changes touch code using MPC, I tested it with MPC svn
> revision 706, mpc-0.7 and without MPC.  No regressions.
>
> Okay for mainline?

I think you can now ICE the compiler with

  double cimag();
  double foo()
  {
     return cimag();
  }

because only validate_arg checks for the argument being NULL.

Thus I think you should switch all places to use validate_arg
instead.

Ok with that change.

Thanks,
Richard.

>                Thanks,
>                --Kaveh
>
>
> 2009-10-21  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
>
>        * builtins.c (fold_builtin_cabs): Use COMPLEX_FLOAT_TYPE_P to
>        check for a complex float type.
>        (fold_builtin_ccos): Likewise.
>        (fold_builtin_cexp): Likewise.
>        (fold_builtin_carg): Likewise.
>        (fold_builtin_1): Likewise.
>        (fold_builtin_2): Likewise.
>
> diff -rup orig/egcc-SVN20091021/gcc/builtins.c egcc-SVN20091021/gcc/builtins.c
> --- orig/egcc-SVN20091021/gcc/builtins.c        2009-10-14 03:30:12.000000000 +0200
> +++ egcc-SVN20091021/gcc/builtins.c     2009-10-21 22:41:22.000000000 +0200
> @@ -7194,8 +7194,7 @@ fold_builtin_cabs (location_t loc, tree
>  {
>   tree res;
>
> -  if (TREE_CODE (TREE_TYPE (arg)) != COMPLEX_TYPE
> -      || TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != REAL_TYPE)
> +  if (! COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
>     return NULL_TREE;
>
>   /* Calculate the result when the argument is a constant.  */
> @@ -7484,8 +7483,7 @@ fold_builtin_ccos (location_t loc,
>                   tree arg, tree type ATTRIBUTE_UNUSED, tree fndecl,
>                   bool hyper ATTRIBUTE_UNUSED)
>  {
> -  if (validate_arg (arg, COMPLEX_TYPE)
> -      && TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == REAL_TYPE)
> +  if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
>     {
>       tree tmp;
>
> @@ -7582,8 +7580,7 @@ fold_builtin_cexp (location_t loc, tree
>   tree res;
>  #endif
>
> -  if (!validate_arg (arg0, COMPLEX_TYPE)
> -      && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +  if (! COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>     return NULL_TREE;
>
>  #ifdef HAVE_mpc
> @@ -9363,8 +9360,7 @@ fold_builtin_fmin_fmax (location_t loc,
>  static tree
>  fold_builtin_carg (location_t loc, tree arg, tree type)
>  {
> -  if (validate_arg (arg, COMPLEX_TYPE)
> -      && TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) == REAL_TYPE)
> +  if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg)))
>     {
>       tree atan2_fn = mathfn_built_in (type, BUILT_IN_ATAN2);
>
> @@ -10005,19 +10001,17 @@ fold_builtin_1 (location_t loc, tree fnd
>       return fold_builtin_abs (loc, arg0, type);
>
>     CASE_FLT_FN (BUILT_IN_CONJ):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -       && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return fold_build1_loc (loc, CONJ_EXPR, type, arg0);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CREAL):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -       && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return non_lvalue_loc (loc, fold_build1_loc (loc, REALPART_EXPR, type, arg0));;
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CIMAG):
> -      if (validate_arg (arg0, COMPLEX_TYPE))
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return non_lvalue_loc (loc, fold_build1_loc (loc, IMAGPART_EXPR, type, arg0));
>     break;
>
> @@ -10029,75 +10023,63 @@ fold_builtin_1 (location_t loc, tree fnd
>
>  #ifdef HAVE_mpc
>     CASE_FLT_FN (BUILT_IN_CSIN):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_sin);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CSINH):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_sinh);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CTAN):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_tan);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CTANH):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_tanh);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CLOG):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_log);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CSQRT):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_sqrt);
>     break;
>
>  #ifdef HAVE_mpc_arc
>     CASE_FLT_FN (BUILT_IN_CASIN):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_asin);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CACOS):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_acos);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CATAN):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_atan);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CASINH):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_asinh);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CACOSH):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_acosh);
>     break;
>
>     CASE_FLT_FN (BUILT_IN_CATANH):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0)))
>        return do_mpc_arg1 (arg0, type, mpc_atanh);
>     break;
>  #endif /* HAVE_mpc_arc */
> @@ -10421,10 +10403,8 @@ fold_builtin_2 (location_t loc, tree fnd
>
>  #ifdef HAVE_mpc_pow
>     CASE_FLT_FN (BUILT_IN_CPOW):
> -      if (validate_arg (arg0, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE
> -         && validate_arg (arg1, COMPLEX_TYPE)
> -         && TREE_CODE (TREE_TYPE (TREE_TYPE (arg1))) == REAL_TYPE)
> +      if (COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg0))
> +         && COMPLEX_FLOAT_TYPE_P (TREE_TYPE (arg1)))
>        return do_mpc_arg2 (arg0, arg1, type, /*do_nonfinite=*/ 0, mpc_pow);
>     break;
>  #endif
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH]: middle-end, fix bug in fold_builtin_cexp using    COMPLEX_FLOAT_TYPE_P
  2009-10-22  9:59 ` Richard Guenther
@ 2009-10-23  9:09   ` Kaveh R. GHAZI
  0 siblings, 0 replies; 3+ messages in thread
From: Kaveh R. GHAZI @ 2009-10-23  9:09 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Thu, 22 Oct 2009, Richard Guenther wrote:

> > Okay for mainline?
>
> I think you can now ICE the compiler with
>
>   double cimag();
>   double foo()
>   {
>      return cimag();
>   }
>
> because only validate_arg checks for the argument being NULL.
>
> Thus I think you should switch all places to use validate_arg
> instead.
>
> Ok with that change.
>

Hmm, I checked whether code such as the above crashes and by chance it
doesn't.  The reason is that builtin function calls with zero arguments go
through fold_builtin_0 and miss the actual handlers, so the possible crash
is hidden.  Nevertheless, you do raise a valid issue in that my original
patch would substitute one latent bug for many more new ones ...

So per your request, here's what I installed after testing.

		--Kaveh



2009-10-22  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* builtins.c (fold_builtin_cabs): Use validate_arg().
	(fold_builtin_cexp): Fix if-logic.
	(fold_builtin_1): Check subtype for BUILT_IN_CIMAG.

diff -rup orig/egcc-SVN20091022/gcc/builtins.c egcc-SVN20091022/gcc/builtins.c
--- orig/egcc-SVN20091022/gcc/builtins.c	2009-10-14 03:24:05.000000000 +0200
+++ egcc-SVN20091022/gcc/builtins.c	2009-10-22 18:21:12.000000000 +0200
@@ -7194,7 +7194,7 @@ fold_builtin_cabs (location_t loc, tree
 {
   tree res;

-  if (TREE_CODE (TREE_TYPE (arg)) != COMPLEX_TYPE
+  if (!validate_arg (arg, COMPLEX_TYPE)
       || TREE_CODE (TREE_TYPE (TREE_TYPE (arg))) != REAL_TYPE)
     return NULL_TREE;

@@ -7583,7 +7583,7 @@ fold_builtin_cexp (location_t loc, tree
 #endif

   if (!validate_arg (arg0, COMPLEX_TYPE)
-      && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
+      || TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) != REAL_TYPE)
     return NULL_TREE;

 #ifdef HAVE_mpc
@@ -10017,7 +10017,8 @@ fold_builtin_1 (location_t loc, tree fnd
     break;

     CASE_FLT_FN (BUILT_IN_CIMAG):
-      if (validate_arg (arg0, COMPLEX_TYPE))
+      if (validate_arg (arg0, COMPLEX_TYPE)
+	  && TREE_CODE (TREE_TYPE (TREE_TYPE (arg0))) == REAL_TYPE)
 	return non_lvalue_loc (loc, fold_build1_loc (loc, IMAGPART_EXPR, type, arg0));
     break;

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-10-23  6:32 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-22  6:18 [PATCH]: middle-end, fix bug in fold_builtin_cexp using COMPLEX_FLOAT_TYPE_P Kaveh R. GHAZI
2009-10-22  9:59 ` Richard Guenther
2009-10-23  9:09   ` Kaveh R. GHAZI

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).