This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Committed] Relax sizetype constraints in size_binop (take 2)


The following patch is a revised version of my earlier submission
http://gcc.gnu.org/ml/gcc-patches/2006-11/msg00142.html that implements
Richard Guenther's suggestion of breaking the new "invariant" predicate
out into it's own function.  The name of this new *middle-end private*
function is int_binop_types_match_p, reflecting its role as the expected
(required) type equality in calls to int_const_binop, and for integer
operations to fold*.

Missing from the following commit is the hunk:

Index: fold-const.c
===================================================================
*** fold-const.c        (revision 118681)
--- fold-const.c        (working copy)
*************** int_const_binop (enum tree_code code, tr
*** 1393,1398 ****
--- 1422,1433 ----
      = (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type));
    int overflow = 0;

+ #if 0
+   /* Temporarily disabled whilst the remaining type mismatches are fixed.  */
+   gcc_assert (int_binop_types_match_p (code, TREE_TYPE (arg1),
+                                        TREE_TYPE (arg2)));
+ #endif
+
    int1l = TREE_INT_CST_LOW (arg1);
    int1h = TREE_INT_CST_HIGH (arg1);
    int2l = TREE_INT_CST_LOW (arg2);


which is the invariant that the middle-end expects to see hold between
operands passed to const_int_binop.  Notice that const_int_binop doesn't
take a type argument, so selects the operation type from the type of its
arguments, arbitrarily arg1.  If the signedness of arg1 and arg2
differ, it's not unlikely that these functions don't perform the
operation that the caller intended.

Naturally, the reason this hunk is omitted is that we currently pass
completely bogus operands to fold* as revealed by Andrew Pinski's PR 22368
meta-bug.  So although richi's comments suggested we were mysteriously
weakening the middle-end's type system, in actuality the aim is to
significantly strengthen it.  Asserting "type1 == type2" is incompatible
with the tree-ssa/gimple notion of "useless_type_conversion", so we need
something else.  Historically, matching modes was probably sufficient,
but I suspect there are numerous latent bugs caused by type mismatching.

Included in this commit are several more middle-end type correctness
tweaks designed to get us closer to this goal of type safety.


The following revised patch has been tested on x86_64-unknown-linux-gnu,
with a full "make bootstrap", all default languages, and regression tested
with a top-level "make -k check" with no new failures.

Committed to mainline as revision 118718.



2006-11-11  Roger Sayle  <roger@eyesopen.com>

	* fold-const.c (int_binop_types_match_p): New function.
	(size_binop): Relax constraint that both arguments must both have
	exactly the same sizetype type.  Instead use int_binop_types_match_p.
	(size_diffop): Likewise.

	(make_range): Use build_int_cst instead of fold_convert.
	(fold_cond_expr_with_comparison): Use build_int_cst to construct
	integer constants of the correct type.
	(fold_div_compare): Likewise.
	(fold_single_bit_test): Likewise.
	(fold_binary): Likewise.
	* stor-layout.c (layout_type) <VECTOR_TYPE>: Ensure that TYPE_SIZE
	has type bitsizetype and TYPE_SIZE_UNIT has type sizetype.


Index: fold-const.c
===================================================================
*** fold-const.c	(revision 118681)
--- fold-const.c	(working copy)
*************** associate_trees (tree t1, tree t2, enum
*** 1371,1376 ****
--- 1371,1405 ----
  		      fold_convert (type, t2));
  }

+ /* Check whether TYPE1 and TYPE2 are equivalent integer types, suitable
+    for use in int_const_binop, size_binop and size_diffop.  */
+
+ static bool
+ int_binop_types_match_p (enum tree_code code, tree type1, tree type2)
+ {
+   if (TREE_CODE (type1) != INTEGER_TYPE && !POINTER_TYPE_P (type1))
+     return false;
+   if (TREE_CODE (type2) != INTEGER_TYPE && !POINTER_TYPE_P (type2))
+     return false;
+
+   switch (code)
+     {
+     case LSHIFT_EXPR:
+     case RSHIFT_EXPR:
+     case LROTATE_EXPR:
+     case RROTATE_EXPR:
+       return true;
+
+     default:
+       break;
+     }
+
+   return TYPE_UNSIGNED (type1) == TYPE_UNSIGNED (type2)
+ 	 && TYPE_PRECISION (type1) == TYPE_PRECISION (type2)
+ 	 && TYPE_MODE (type1) == TYPE_MODE (type2);
+ }
+
+
  /* Combine two integer constants ARG1 and ARG2 under operation CODE
     to produce a new constant.  Return NULL_TREE if we don't know how
     to evaluate CODE at compile-time.
*************** size_int_kind (HOST_WIDE_INT number, enu
*** 1732,1738 ****

  /* Combine operands OP1 and OP2 with arithmetic operation CODE.  CODE
     is a tree code.  The type of the result is taken from the operands.
!    Both must be the same type integer type and it must be a size type.
     If the operands are constant, so is the result.  */

  tree
--- 1767,1773 ----

  /* Combine operands OP1 and OP2 with arithmetic operation CODE.  CODE
     is a tree code.  The type of the result is taken from the operands.
!    Both must be equivalent integer types, ala int_binop_types_match_p.
     If the operands are constant, so is the result.  */

  tree
*************** size_binop (enum tree_code code, tree ar
*** 1743,1750 ****
    if (arg0 == error_mark_node || arg1 == error_mark_node)
      return error_mark_node;

!   gcc_assert (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type)
! 	      && type == TREE_TYPE (arg1));

    /* Handle the special case of two integer constants faster.  */
    if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
--- 1778,1785 ----
    if (arg0 == error_mark_node || arg1 == error_mark_node)
      return error_mark_node;

!   gcc_assert (int_binop_types_match_p (code, TREE_TYPE (arg0),
!                                        TREE_TYPE (arg1)));

    /* Handle the special case of two integer constants faster.  */
    if (TREE_CODE (arg0) == INTEGER_CST && TREE_CODE (arg1) == INTEGER_CST)
*************** size_diffop (tree arg0, tree arg1)
*** 1775,1788 ****
    tree type = TREE_TYPE (arg0);
    tree ctype;

!   gcc_assert (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type)
! 	      && type == TREE_TYPE (arg1));

    /* If the type is already signed, just do the simple thing.  */
    if (!TYPE_UNSIGNED (type))
      return size_binop (MINUS_EXPR, arg0, arg1);

!   ctype = type == bitsizetype ? sbitsizetype : ssizetype;

    /* If either operand is not a constant, do the conversions to the signed
       type and subtract.  The hardware will do the right thing with any
--- 1810,1828 ----
    tree type = TREE_TYPE (arg0);
    tree ctype;

!   gcc_assert (int_binop_types_match_p (MINUS_EXPR, TREE_TYPE (arg0),
! 				       TREE_TYPE (arg1)));

    /* If the type is already signed, just do the simple thing.  */
    if (!TYPE_UNSIGNED (type))
      return size_binop (MINUS_EXPR, arg0, arg1);

!   if (type == sizetype)
!     ctype = ssizetype;
!   else if (type == bitsizetype)
!     ctype = sbitsizetype;
!   else
!     ctype = lang_hooks.types.signed_type (type);

    /* If either operand is not a constant, do the conversions to the signed
       type and subtract.  The hardware will do the right thing with any
*************** make_range (tree exp, int *pin_p, tree *
*** 4000,4007 ****
  		high_positive = fold_build2 (RSHIFT_EXPR, arg0_type,
  					     fold_convert (arg0_type,
  							   high_positive),
! 					     fold_convert (arg0_type,
! 							   integer_one_node));

  	      /* If the low bound is specified, "and" the range with the
  		 range for which the original unsigned value will be
--- 4040,4046 ----
  		high_positive = fold_build2 (RSHIFT_EXPR, arg0_type,
  					     fold_convert (arg0_type,
  							   high_positive),
! 					     build_int_cst (arg0_type, 1));

  	      /* If the low bound is specified, "and" the range with the
  		 range for which the original unsigned value will be
*************** fold_cond_expr_with_comparison (tree typ
*** 4647,4653 ****
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (PLUS_EXPR, arg2,
! 					     integer_one_node, 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MIN_EXPR,
  						   type, arg1, arg2));
--- 4686,4692 ----
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (PLUS_EXPR, arg2,
! 					     build_int_cst (type, 1), 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MIN_EXPR,
  						   type, arg1, arg2));
*************** fold_cond_expr_with_comparison (tree typ
*** 4659,4665 ****
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (MINUS_EXPR, arg2,
! 					     integer_one_node, 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MIN_EXPR,
  						   type, arg1, arg2));
--- 4698,4704 ----
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (MINUS_EXPR, arg2,
! 					     build_int_cst (type, 1), 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MIN_EXPR,
  						   type, arg1, arg2));
*************** fold_cond_expr_with_comparison (tree typ
*** 4671,4677 ****
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (MINUS_EXPR, arg2,
! 					     integer_one_node, 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MAX_EXPR,
  						   type, arg1, arg2));
--- 4710,4716 ----
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (MINUS_EXPR, arg2,
! 					     build_int_cst (type, 1), 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MAX_EXPR,
  						   type, arg1, arg2));
*************** fold_cond_expr_with_comparison (tree typ
*** 4683,4689 ****
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (PLUS_EXPR, arg2,
! 					     integer_one_node, 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MAX_EXPR,
  						   type, arg1, arg2));
--- 4722,4728 ----
  			       OEP_ONLY_CONST)
  	    && operand_equal_p (arg01,
  				const_binop (PLUS_EXPR, arg2,
! 					     build_int_cst (type, 1), 0),
  				OEP_ONLY_CONST))
  	  return pedantic_non_lvalue (fold_build2 (MAX_EXPR,
  						   type, arg1, arg2));
*************** fold_div_compare (enum tree_code code, t
*** 6106,6112 ****

    if (unsigned_p)
      {
!       tmp = int_const_binop (MINUS_EXPR, arg01, integer_one_node, 0);
        lo = prod;

        /* Likewise hi = int_const_binop (PLUS_EXPR, prod, tmp, 0).  */
--- 6145,6152 ----

    if (unsigned_p)
      {
!       tmp = int_const_binop (MINUS_EXPR, arg01,
!                              build_int_cst (TREE_TYPE (arg01), 1), 0);
        lo = prod;

        /* Likewise hi = int_const_binop (PLUS_EXPR, prod, tmp, 0).  */
*************** fold_div_compare (enum tree_code code, t
*** 6121,6127 ****
      }
    else if (tree_int_cst_sgn (arg01) >= 0)
      {
!       tmp = int_const_binop (MINUS_EXPR, arg01, integer_one_node, 0);
        switch (tree_int_cst_sgn (arg1))
  	{
  	case -1:
--- 6161,6168 ----
      }
    else if (tree_int_cst_sgn (arg01) >= 0)
      {
!       tmp = int_const_binop (MINUS_EXPR, arg01,
! 			     build_int_cst (TREE_TYPE (arg01), 1), 0);
        switch (tree_int_cst_sgn (arg1))
  	{
  	case -1:
*************** fold_div_compare (enum tree_code code, t
*** 6149,6155 ****
        /* A negative divisor reverses the relational operators.  */
        code = swap_tree_comparison (code);

!       tmp = int_const_binop (PLUS_EXPR, arg01, integer_one_node, 0);
        switch (tree_int_cst_sgn (arg1))
  	{
  	case -1:
--- 6190,6197 ----
        /* A negative divisor reverses the relational operators.  */
        code = swap_tree_comparison (code);

!       tmp = int_const_binop (PLUS_EXPR, arg01,
! 			     build_int_cst (TREE_TYPE (arg01), 1), 0);
        switch (tree_int_cst_sgn (arg1))
  	{
  	case -1:
*************** fold_single_bit_test (enum tree_code cod
*** 6287,6293 ****
        enum machine_mode operand_mode = TYPE_MODE (type);
        int ops_unsigned;
        tree signed_type, unsigned_type, intermediate_type;
!       tree tem;

        /* First, see if we can fold the single bit test into a sign-bit
  	 test.  */
--- 6329,6335 ----
        enum machine_mode operand_mode = TYPE_MODE (type);
        int ops_unsigned;
        tree signed_type, unsigned_type, intermediate_type;
!       tree tem, one;

        /* First, see if we can fold the single bit test into a sign-bit
  	 test.  */
*************** fold_single_bit_test (enum tree_code cod
*** 6332,6344 ****
  	inner = build2 (RSHIFT_EXPR, intermediate_type,
  			inner, size_int (bitnum));

        if (code == EQ_EXPR)
! 	inner = fold_build2 (BIT_XOR_EXPR, intermediate_type,
! 			     inner, integer_one_node);

        /* Put the AND last so it can combine with more things.  */
!       inner = build2 (BIT_AND_EXPR, intermediate_type,
! 		      inner, integer_one_node);

        /* Make sure to return the proper type.  */
        inner = fold_convert (result_type, inner);
--- 6374,6386 ----
  	inner = build2 (RSHIFT_EXPR, intermediate_type,
  			inner, size_int (bitnum));

+       one = build_int_cst (intermediate_type, 1);
+
        if (code == EQ_EXPR)
! 	inner = fold_build2 (BIT_XOR_EXPR, intermediate_type, inner, one);

        /* Put the AND last so it can combine with more things.  */
!       inner = build2 (BIT_AND_EXPR, intermediate_type, inner, one);

        /* Make sure to return the proper type.  */
        inner = fold_convert (result_type, inner);
*************** fold_binary (enum tree_code code, tree t
*** 10060,10067 ****

  	  if (integer_pow2p (c) && tree_int_cst_sgn (c) > 0)
  	    {
! 	      tree mask = fold_build2 (MINUS_EXPR, TREE_TYPE (arg1),
! 				       arg1, integer_one_node);
  	      return fold_build2 (BIT_AND_EXPR, type,
  				  fold_convert (type, arg0),
  				  fold_convert (type, mask));
--- 10102,10109 ----

  	  if (integer_pow2p (c) && tree_int_cst_sgn (c) > 0)
  	    {
! 	      tree mask = fold_build2 (MINUS_EXPR, TREE_TYPE (arg1), arg1,
! 				       build_int_cst (TREE_TYPE (arg1), 1));
  	      return fold_build2 (BIT_AND_EXPR, type,
  				  fold_convert (type, arg0),
  				  fold_convert (type, mask));
*************** fold_binary (enum tree_code code, tree t
*** 10173,10181 ****
  	 RROTATE_EXPR by a new constant.  */
        if (code == LROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST)
  	{
! 	  tree tem = build_int_cst (NULL_TREE,
  				    GET_MODE_BITSIZE (TYPE_MODE (type)));
- 	  tem = fold_convert (TREE_TYPE (arg1), tem);
  	  tem = const_binop (MINUS_EXPR, tem, arg1, 0);
  	  return fold_build2 (RROTATE_EXPR, type, arg0, tem);
  	}
--- 10215,10222 ----
  	 RROTATE_EXPR by a new constant.  */
        if (code == LROTATE_EXPR && TREE_CODE (arg1) == INTEGER_CST)
  	{
! 	  tree tem = build_int_cst (TREE_TYPE (arg1),
  				    GET_MODE_BITSIZE (TYPE_MODE (type)));
  	  tem = const_binop (MINUS_EXPR, tem, arg1, 0);
  	  return fold_build2 (RROTATE_EXPR, type, arg0, tem);
  	}
*************** fold_binary (enum tree_code code, tree t
*** 10995,11004 ****
  	      switch (code)
  		{
  		case GT_EXPR:
! 		  arg1 = const_binop (PLUS_EXPR, arg1, integer_one_node, 0);
  		  return fold_build2 (EQ_EXPR, type, arg0, arg1);
  		case LE_EXPR:
! 		  arg1 = const_binop (PLUS_EXPR, arg1, integer_one_node, 0);
  		  return fold_build2 (NE_EXPR, type, arg0, arg1);
  		default:
  		  break;
--- 11036,11047 ----
  	      switch (code)
  		{
  		case GT_EXPR:
! 		  arg1 = const_binop (PLUS_EXPR, arg1,
! 				      build_int_cst (TREE_TYPE (arg1), 1), 0);
  		  return fold_build2 (EQ_EXPR, type, arg0, arg1);
  		case LE_EXPR:
! 		  arg1 = const_binop (PLUS_EXPR, arg1,
! 				      build_int_cst (TREE_TYPE (arg1), 1), 0);
  		  return fold_build2 (NE_EXPR, type, arg0, arg1);
  		default:
  		  break;
Index: stor-layout.c
===================================================================
*** stor-layout.c	(revision 118681)
--- stor-layout.c	(working copy)
*************** layout_type (tree type)
*** 1617,1623 ****
      case VECTOR_TYPE:
        {
  	int nunits = TYPE_VECTOR_SUBPARTS (type);
- 	tree nunits_tree = build_int_cst (NULL_TREE, nunits);
  	tree innertype = TREE_TYPE (type);

  	gcc_assert (!(nunits & (nunits - 1)));
--- 1617,1622 ----
*************** layout_type (tree type)
*** 1655,1663 ****
          TYPE_UNSIGNED (type) = TYPE_UNSIGNED (TREE_TYPE (type));
  	TYPE_SIZE_UNIT (type) = int_const_binop (MULT_EXPR,
  					         TYPE_SIZE_UNIT (innertype),
! 					         nunits_tree, 0);
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
! 					    nunits_tree, 0);

  	/* Always naturally align vectors.  This prevents ABI changes
  	   depending on whether or not native vector modes are supported.  */
--- 1654,1662 ----
          TYPE_UNSIGNED (type) = TYPE_UNSIGNED (TREE_TYPE (type));
  	TYPE_SIZE_UNIT (type) = int_const_binop (MULT_EXPR,
  					         TYPE_SIZE_UNIT (innertype),
! 					         size_int (nunits), 0);
  	TYPE_SIZE (type) = int_const_binop (MULT_EXPR, TYPE_SIZE (innertype),
! 					    bitsize_int (nunits), 0);

  	/* Always naturally align vectors.  This prevents ABI changes
  	   depending on whether or not native vector modes are supported.  */


Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]