public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Is this a bug in the folder?
@ 2004-09-21 15:55 Diego Novillo
  2004-09-21 18:26 ` Roger Sayle
  0 siblings, 1 reply; 3+ messages in thread
From: Diego Novillo @ 2004-09-21 15:55 UTC (permalink / raw)
  To: gcc; +Cc: Roger Sayle, Richard Henderson


With a local patch, I am triggering an ice in NUMERICAL_TYPE_CHECK
because it is trying to get the lower bound on a VECTOR_TYPE.

      /* If we are widening one operand of an integer comparison,
         see if the other operand is similarly being widened.  Perhaps we
         can do the comparison in the narrower type.  */
      else if (TREE_CODE (TREE_TYPE (arg0)) == INTEGER_TYPE
               && TREE_CODE (arg0) == NOP_EXPR
               && (tem = get_unwidened (arg0, NULL_TREE)) != arg0
               && (code == EQ_EXPR || code == NE_EXPR
                   || TYPE_UNSIGNED (TREE_TYPE (arg0))
                      == TYPE_UNSIGNED (TREE_TYPE (tem)))
              && (t1 = get_unwidened (arg1, TREE_TYPE (tem))) != 0
               && (TREE_TYPE (t1) == TREE_TYPE (tem)
                   || (TREE_CODE (t1) == INTEGER_CST
                       && int_fits_type_p (t1, TREE_TYPE (tem)))))

The failure occurs when we try to fold the following expression
(testsuite/gcc.dg/20020531-1.c):

(long long unsigned int) D.1201 != 1122334455667788

The type of D.1201 is

 <vector_type 0xf6f8a32c
    type <integer_type 0xf6fda244 signed char public QI
        size <integer_cst 0xf6fd71c8 constant invariant 8>
        unit size <integer_cst 0xf6fd71e0 constant invariant 1>
        align 8 symtab 0 alias set -1 precision 8 min <integer_cst 0xf6fd7198 -128> max <integer_cst 0xf6fd71b0 127>
        pointer_to_this <pointer_type 0xf6f890e8>>
    V8QI
    size <integer_cst 0xf6fd75e8 type <integer_type 0xf6fda1d0 bit_size_type> constant invariant 64>
    unit size <integer_cst 0xf6fd7600 type <integer_type 0xf6fda15c unsigned int> constant invariant 8>
    align 64 symtab 0 alias set -1 nunits 8>


The code fragment I quoted from fold-const.c has

	arg0	-> (long long unsigned int)D.1201
	tem	-> D.1201

So, when we call int_fits_type_p() we are passing a VECTOR_TYPE to it,
which triggers the ICE.

You can trigger this by compiling testsuite/gcc.dg/20020531-1.c after
applying this patchlet to tree-cfg.c:

Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.55
diff -d -u -p -r2.55 tree-cfg.c
--- tree-cfg.c  17 Sep 2004 21:54:37 -0000      2.55
+++ tree-cfg.c  21 Sep 2004 15:28:32 -0000
@@ -2003,20 +2002,8 @@ find_taken_edge (basic_block bb, tree va
   /* If VAL is a predicate of the form N RELOP N, where N is an
      SSA_NAME, we can always determine its truth value (except when
      doing floating point comparisons that may involve NaNs).  */
-  if (val
-      && COMPARISON_CLASS_P (val)
-      && TREE_OPERAND (val, 0) == TREE_OPERAND (val, 1)
-      && TREE_CODE (TREE_OPERAND (val, 0)) == SSA_NAME
-      && (TREE_CODE (TREE_TYPE (TREE_OPERAND (val, 0))) != REAL_TYPE
-         || !HONOR_NANS (TYPE_MODE (TREE_TYPE (TREE_OPERAND (val, 0))))))
-    {
-      enum tree_code code = TREE_CODE (val);
-
-      if (code == EQ_EXPR || code == LE_EXPR || code == GE_EXPR)
-       val = boolean_true_node;
-      else if (code == LT_EXPR || code == GT_EXPR || code == NE_EXPR)
-       val = boolean_false_node;
-    }
+  if (val && COMPARISON_CLASS_P (val))
+    val = fold (val);

   /* If VAL is not a constant, we can't determine which edge might
      be taken.  */


Any ideas?


Thanks.  Diego.

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

* Re: Is this a bug in the folder?
  2004-09-21 15:55 Is this a bug in the folder? Diego Novillo
@ 2004-09-21 18:26 ` Roger Sayle
  2004-09-22  8:45   ` Diego Novillo
  0 siblings, 1 reply; 3+ messages in thread
From: Roger Sayle @ 2004-09-21 18:26 UTC (permalink / raw)
  To: Diego Novillo; +Cc: gcc, Richard Henderson


On Tue, 21 Sep 2004, Diego Novillo wrote:
> With a local patch, I am triggering an ice in NUMERICAL_TYPE_CHECK
> because it is trying to get the lower bound on a VECTOR_TYPE.
>
>       /* If we are widening one operand of an integer comparison,
>          see if the other operand is similarly being widened.  Perhaps we
>          can do the comparison in the narrower type.  */
>       else if (TREE_CODE (TREE_TYPE (arg0)) == INTEGER_TYPE
>                && TREE_CODE (arg0) == NOP_EXPR
>                && (tem = get_unwidened (arg0, NULL_TREE)) != arg0
>                && (code == EQ_EXPR || code == NE_EXPR
>                    || TYPE_UNSIGNED (TREE_TYPE (arg0))
>                       == TYPE_UNSIGNED (TREE_TYPE (tem)))
>               && (t1 = get_unwidened (arg1, TREE_TYPE (tem))) != 0
>                && (TREE_TYPE (t1) == TREE_TYPE (tem)
>                    || (TREE_CODE (t1) == INTEGER_CST
>                        && int_fits_type_p (t1, TREE_TYPE (tem)))))


Clearly this is a bug in the above logic.  Possible solutions are:

(1) Check that TREE_CODE (TREE_TYPE (tem)) == INTEGER_TYPE after
    the assignment of tem.

(2) Check that TREE_CODE (TREE_TYPE (tem)) == INTEGER_TYPE in
    the final condition immediately before the call to int_fits_type_p.

or slightly more effort

(3) Fix get_unwidened such that t1 is a VECTOR_CST of the approriate
    type and not an INTEGER_CST.  Unfortunately, fold_convert doesn't
    yet know how to convert an integer_cst into a vector_cst for values
    other than zero, however it would be useful functionality to have.
    See http://gcc.gnu.org/ml/gcc-patches/2004-09/msg00773.html

I suspect (2) is the safest short term solution to get your code working,
but ultimately (3) [and the reciprocal vector_cst to integer_cst] would
allow for better optimizations, for example, to enable constant folding
of your example comparison at compile-time.

Roger
--

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

* Re: Is this a bug in the folder?
  2004-09-21 18:26 ` Roger Sayle
@ 2004-09-22  8:45   ` Diego Novillo
  0 siblings, 0 replies; 3+ messages in thread
From: Diego Novillo @ 2004-09-22  8:45 UTC (permalink / raw)
  To: Roger Sayle; +Cc: gcc, Richard Henderson

On Tue, 2004-09-21 at 11:27, Roger Sayle wrote:

> (1) Check that TREE_CODE (TREE_TYPE (tem)) == INTEGER_TYPE after
>     the assignment of tem.
> 
> (2) Check that TREE_CODE (TREE_TYPE (tem)) == INTEGER_TYPE in
>     the final condition immediately before the call to int_fits_type_p.
> 
> or slightly more effort
> 
> (3) Fix get_unwidened such that t1 is a VECTOR_CST of the approriate
>     type and not an INTEGER_CST.  Unfortunately, fold_convert doesn't
>     yet know how to convert an integer_cst into a vector_cst for values
>     other than zero, however it would be useful functionality to have.
>     See http://gcc.gnu.org/ml/gcc-patches/2004-09/msg00773.html
> 
> I suspect (2) is the safest short term solution to get your code working,
> but ultimately (3) [and the reciprocal vector_cst to integer_cst] would
> allow for better optimizations, for example, to enable constant folding
> of your example comparison at compile-time.
> 
I went with #2 in the interest of minimal intrusion for 4.0.  The fix to
fold() allowed me to simplify the folding of COND_EXPR predicates
without all the twisty logic we had before.

OK for mainline?  (no new tests are necessary, the second part of the
patch produces 3 new failures in gcc.c-torture without the fold patch).


Thanks.  Diego.

2004-09-21  Diego Novillo  <dnovillo@redhat.com>

	* fold-const.c (fold): Avoid non INTEGER_TYPEs when widening
	operands in an integer comparison.
	* tree-cfg.c (find_taken_edge): Call fold() to determine
	whether the predicate is known.

Index: fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.461
diff -d -c -p -u -r1.461 fold-const.c
--- fold-const.c	17 Sep 2004 21:54:32 -0000	1.461
+++ fold-const.c	22 Sep 2004 01:11:30 -0000
@@ -8290,6 +8290,7 @@ fold (tree expr)
 	       && (t1 = get_unwidened (arg1, TREE_TYPE (tem))) != 0
 	       && (TREE_TYPE (t1) == TREE_TYPE (tem)
 		   || (TREE_CODE (t1) == INTEGER_CST
+		       && TREE_CODE (TREE_TYPE (tem)) == INTEGER_TYPE
 		       && int_fits_type_p (t1, TREE_TYPE (tem)))))
 	return fold (build2 (code, type, tem,
 			     fold_convert (TREE_TYPE (tem), t1)));
Index: tree-cfg.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-cfg.c,v
retrieving revision 2.56
diff -d -c -p -u -r2.56 tree-cfg.c
--- tree-cfg.c	18 Sep 2004 21:54:53 -0000	2.56
+++ tree-cfg.c	22 Sep 2004 01:11:30 -0000
@@ -2001,22 +2001,9 @@ find_taken_edge (basic_block bb, tree va
   gcc_assert (is_ctrl_stmt (stmt));
 
   /* If VAL is a predicate of the form N RELOP N, where N is an
-     SSA_NAME, we can always determine its truth value (except when
-     doing floating point comparisons that may involve NaNs).  */
-  if (val
-      && COMPARISON_CLASS_P (val)
-      && TREE_OPERAND (val, 0) == TREE_OPERAND (val, 1)
-      && TREE_CODE (TREE_OPERAND (val, 0)) == SSA_NAME
-      && (TREE_CODE (TREE_TYPE (TREE_OPERAND (val, 0))) != REAL_TYPE
-	  || !HONOR_NANS (TYPE_MODE (TREE_TYPE (TREE_OPERAND (val, 0))))))
-    {
-      enum tree_code code = TREE_CODE (val);
-
-      if (code == EQ_EXPR || code == LE_EXPR || code == GE_EXPR)
-	val = boolean_true_node;
-      else if (code == LT_EXPR || code == GT_EXPR || code == NE_EXPR)
-	val = boolean_false_node;
-    }
+     SSA_NAME, we can usually determine its truth value.  */
+  if (val && COMPARISON_CLASS_P (val))
+    val = fold (val);
 
   /* If VAL is not a constant, we can't determine which edge might
      be taken.  */

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

end of thread, other threads:[~2004-09-22  1:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-09-21 15:55 Is this a bug in the folder? Diego Novillo
2004-09-21 18:26 ` Roger Sayle
2004-09-22  8:45   ` Diego Novillo

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).