public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][RFC] Fix sizetype "sign" checks
@ 2011-08-18 13:10 Richard Guenther
  2011-08-19  7:10 ` Eric Botcazou
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Guenther @ 2011-08-18 13:10 UTC (permalink / raw)
  To: gcc-patches; +Cc: ebotcazou


The following fixes a few places that think that using
host_integerp (t, 1) with t being a sizetype constant to
verify the value is "positive" is a good idea.  sizetype
is an unsigned type (yes, still sign-extended), so
testing for a positive value is at least weird.

The patch below adds tree_int_cst_sign_bit tests in
addition to the existing host_integerp checks.  The
varasm.c hunk removes a few diagnostic regressions
when you make sizetypes no longer sign-extended,
the second solves Ada bootstrap problems in that case.

Looking at the Ada case I believe this happens because
Ada has negative DECL_FIELD_OFFSET values (but that's
again in sizetype, not ssizetype)?  Other host_integerp
uses in Ada operate on sizes where I hope those are
never negative ;)

Eric, any better way of fixing this or would you be fine
with this patch?

Thanks,
Richard.

2011-08-18  Richard Guenther  <rguenther@suse.de>

	* varasm.c (assemble_variable): Fix declaration size check.

	ada/
	* gcc-interface/utils.c (rest_of_record_type_compilation):
	Fix sizetype "sign" checks.


Index: trunk/gcc/ada/gcc-interface/utils.c
===================================================================
*** trunk.orig/gcc/ada/gcc-interface/utils.c	2011-08-18 12:58:17.000000000 +0200
--- trunk/gcc/ada/gcc-interface/utils.c	2011-08-18 13:18:39.000000000 +0200
*************** rest_of_record_type_compilation (tree re
*** 948,954 ****
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
--- 948,955 ----
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1)
! 	      && !tree_int_cst_sign_bit (TREE_OPERAND (curpos, 1)))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
*************** rest_of_record_type_compilation (tree re
*** 960,966 ****
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && TREE_INT_CST_HIGH (TREE_OPERAND (offset, 1)) < 0)
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
--- 961,967 ----
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && tree_int_cst_sign_bit (TREE_OPERAND (offset, 1)))
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
*************** rest_of_record_type_compilation (tree re
*** 975,982 ****
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1),
! 				     1))
  	    {
  	      align
  		= tree_low_cst
--- 976,984 ----
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1), 1)
! 		   && !tree_int_cst_sign_bit (TREE_OPERAND
! 					      (TREE_OPERAND (curpos, 0), 1), 1))
  	    {
  	      align
  		= tree_low_cst
Index: trunk/gcc/varasm.c
===================================================================
*** trunk.orig/gcc/varasm.c	2011-08-18 12:58:17.000000000 +0200
--- trunk/gcc/varasm.c	2011-08-18 13:18:17.000000000 +0200
*************** assemble_variable (tree decl, int top_le
*** 1980,1986 ****
      return;
  
    if (! dont_output_data
!       && ! host_integerp (DECL_SIZE_UNIT (decl), 1))
      {
        error ("size of variable %q+D is too large", decl);
        return;
--- 1980,1989 ----
      return;
  
    if (! dont_output_data
!       && (! host_integerp (DECL_SIZE_UNIT (decl), 1)
! 	  /* Restrict sizes of variables to half the address-space by
! 	     making sure the msb of the size is not set.  */
! 	  || tree_int_cst_sign_bit (DECL_SIZE_UNIT (decl)) != 0))
      {
        error ("size of variable %q+D is too large", decl);
        return;

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

* Re: [PATCH][RFC] Fix sizetype "sign" checks
  2011-08-18 13:10 [PATCH][RFC] Fix sizetype "sign" checks Richard Guenther
@ 2011-08-19  7:10 ` Eric Botcazou
  2011-08-19 10:06   ` Richard Guenther
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Botcazou @ 2011-08-19  7:10 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

> Looking at the Ada case I believe this happens because
> Ada has negative DECL_FIELD_OFFSET values (but that's
> again in sizetype, not ssizetype)?  Other host_integerp
> uses in Ada operate on sizes where I hope those are
> never negative ;)

Yes, the Ada compiler uses negative offsets for some peculiar constructs.
Nothing to do with the language per se, but with mechanisms implemented in 
gigi to support some features of the language.

> Eric, any better way of fixing this or would you be fine with this patch?

Hard to say without seeing the complete patch and playing a little with it.

-- 
Eric Botcazou

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

* Re: [PATCH][RFC] Fix sizetype "sign" checks
  2011-08-19  7:10 ` Eric Botcazou
@ 2011-08-19 10:06   ` Richard Guenther
  2011-08-24 16:05     ` Richard Guenther
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Guenther @ 2011-08-19 10:06 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Fri, 19 Aug 2011, Eric Botcazou wrote:

> > Looking at the Ada case I believe this happens because
> > Ada has negative DECL_FIELD_OFFSET values (but that's
> > again in sizetype, not ssizetype)?  Other host_integerp
> > uses in Ada operate on sizes where I hope those are
> > never negative ;)
> 
> Yes, the Ada compiler uses negative offsets for some peculiar constructs.
> Nothing to do with the language per se, but with mechanisms implemented in 
> gigi to support some features of the language.
> 
> > Eric, any better way of fixing this or would you be fine with this patch?
> 
> Hard to say without seeing the complete patch and playing a little with it.

This is the "complete" patch I am playing with currently, Ada bootstrap
still fails for me unfortunately.  Bootstrap for all other languages
succeeds, but there are some regressions, mostly warning-related.

Any help with pinpointing the Ada problem is welcome.

Thanks,
Richard.

2011-06-16  Richard Guenther  <rguenther@suse.de>

	* fold-const.c (div_if_zero_remainder): sizetypes no longer
	sign-extend.
	* stor-layout.c (initialize_sizetypes): Likewise.
	* tree-ssa-ccp.c (bit_value_unop_1): Likewise.
	(bit_value_binop_1): Likewise.
	* tree.c (double_int_to_tree): Likewise.
	(double_int_fits_to_tree_p): Likewise.
	(force_fit_type_double): Likewise.
	(host_integerp): Likewise.
	(int_fits_type_p): Likewise.
	* tree-cfg.c (verify_types_in_gimple_reference): Do not compare
	sizes by pointer.

Index: trunk/gcc/fold-const.c
===================================================================
*** trunk.orig/gcc/fold-const.c	2011-08-18 13:41:00.000000000 +0200
--- trunk/gcc/fold-const.c	2011-08-18 14:38:31.000000000 +0200
*************** div_if_zero_remainder (enum tree_code co
*** 194,202 ****
       does the correct thing for POINTER_PLUS_EXPR where we want
       a signed division.  */
    uns = TYPE_UNSIGNED (TREE_TYPE (arg2));
-   if (TREE_CODE (TREE_TYPE (arg2)) == INTEGER_TYPE
-       && TYPE_IS_SIZETYPE (TREE_TYPE (arg2)))
-     uns = false;
  
    quo = double_int_divmod (tree_to_double_int (arg1),
  			   tree_to_double_int (arg2),
--- 194,199 ----
*************** int_binop_types_match_p (enum tree_code
*** 938,945 ****
     to produce a new constant.  Return NULL_TREE if we don't know how
     to evaluate CODE at compile-time.  */
  
! tree
! int_const_binop (enum tree_code code, const_tree arg1, const_tree arg2)
  {
    double_int op1, op2, res, tmp;
    tree t;
--- 935,943 ----
     to produce a new constant.  Return NULL_TREE if we don't know how
     to evaluate CODE at compile-time.  */
  
! static tree
! int_const_binop_1 (enum tree_code code, const_tree arg1, const_tree arg2,
! 		   int overflowable)
  {
    double_int op1, op2, res, tmp;
    tree t;
*************** int_const_binop (enum tree_code code, co
*** 1081,1093 ****
        return NULL_TREE;
      }
  
!   t = force_fit_type_double (TREE_TYPE (arg1), res, 1,
  			     ((!uns || is_sizetype) && overflow)
  			     | TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2));
  
    return t;
  }
  
  /* Combine two constants ARG1 and ARG2 under operation CODE to produce a new
     constant.  We assume ARG1 and ARG2 have the same data type, or at least
     are the same kind of constant and the same machine mode.  Return zero if
--- 1079,1097 ----
        return NULL_TREE;
      }
  
!   t = force_fit_type_double (TREE_TYPE (arg1), res, overflowable,
  			     ((!uns || is_sizetype) && overflow)
  			     | TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2));
  
    return t;
  }
  
+ tree
+ int_const_binop (enum tree_code code, const_tree arg1, const_tree arg2)
+ {
+   return int_const_binop_1 (code, arg1, arg2, 1);
+ }
+ 
  /* Combine two constants ARG1 and ARG2 under operation CODE to produce a new
     constant.  We assume ARG1 and ARG2 have the same data type, or at least
     are the same kind of constant and the same machine mode.  Return zero if
*************** size_binop_loc (location_t loc, enum tre
*** 1448,1455 ****
  	    return arg1;
  	}
  
!       /* Handle general case of two integer constants.  */
!       return int_const_binop (code, arg0, arg1);
      }
  
    return fold_build2_loc (loc, code, type, arg0, arg1);
--- 1452,1461 ----
  	    return arg1;
  	}
  
!       /* Handle general case of two integer constants.  For sizetype
!          constant calculations we always want to know about overflow,
! 	 even in the unsigned case.  */
!       return int_const_binop_1 (code, arg0, arg1, -1);
      }
  
    return fold_build2_loc (loc, code, type, arg0, arg1);
Index: trunk/gcc/stor-layout.c
===================================================================
*** trunk.orig/gcc/stor-layout.c	2011-08-18 13:40:55.000000000 +0200
--- trunk/gcc/stor-layout.c	2011-08-18 13:43:39.000000000 +0200
*************** initialize_sizetypes (void)
*** 2234,2244 ****
    TYPE_SIZE_UNIT (sizetype) = size_int (GET_MODE_SIZE (TYPE_MODE (sizetype)));
    set_min_and_max_values_for_integral_type (sizetype, precision,
  					    /*is_unsigned=*/true);
-   /* sizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is
-      sign-extended in a way consistent with force_fit_type.  */
-   TYPE_MAX_VALUE (sizetype)
-     = double_int_to_tree (sizetype,
- 			  tree_to_double_int (TYPE_MAX_VALUE (sizetype)));
  
    SET_TYPE_MODE (bitsizetype, smallest_mode_for_size (bprecision, MODE_INT));
    TYPE_ALIGN (bitsizetype) = GET_MODE_ALIGNMENT (TYPE_MODE (bitsizetype));
--- 2234,2239 ----
*************** initialize_sizetypes (void)
*** 2247,2257 ****
      = size_int (GET_MODE_SIZE (TYPE_MODE (bitsizetype)));
    set_min_and_max_values_for_integral_type (bitsizetype, bprecision,
  					    /*is_unsigned=*/true);
-   /* bitsizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is
-      sign-extended in a way consistent with force_fit_type.  */
-   TYPE_MAX_VALUE (bitsizetype)
-     = double_int_to_tree (bitsizetype,
- 			  tree_to_double_int (TYPE_MAX_VALUE (bitsizetype)));
  
    /* Create the signed variants of *sizetype.  */
    ssizetype = make_signed_type (TYPE_PRECISION (sizetype));
--- 2242,2247 ----
Index: trunk/gcc/tree-ssa-ccp.c
===================================================================
*** trunk.orig/gcc/tree-ssa-ccp.c	2011-08-18 13:40:55.000000000 +0200
--- trunk/gcc/tree-ssa-ccp.c	2011-08-18 13:43:39.000000000 +0200
*************** bit_value_unop_1 (enum tree_code code, t
*** 1099,1112 ****
  	bool uns;
  
  	/* First extend mask and value according to the original type.  */
! 	uns = (TREE_CODE (rtype) == INTEGER_TYPE && TYPE_IS_SIZETYPE (rtype)
! 	       ? 0 : TYPE_UNSIGNED (rtype));
  	*mask = double_int_ext (rmask, TYPE_PRECISION (rtype), uns);
  	*val = double_int_ext (rval, TYPE_PRECISION (rtype), uns);
  
  	/* Then extend mask and value according to the target type.  */
! 	uns = (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type)
! 	       ? 0 : TYPE_UNSIGNED (type));
  	*mask = double_int_ext (*mask, TYPE_PRECISION (type), uns);
  	*val = double_int_ext (*val, TYPE_PRECISION (type), uns);
  	break;
--- 1099,1110 ----
  	bool uns;
  
  	/* First extend mask and value according to the original type.  */
! 	uns = TYPE_UNSIGNED (rtype);
  	*mask = double_int_ext (rmask, TYPE_PRECISION (rtype), uns);
  	*val = double_int_ext (rval, TYPE_PRECISION (rtype), uns);
  
  	/* Then extend mask and value according to the target type.  */
! 	uns = TYPE_UNSIGNED (type);
  	*mask = double_int_ext (*mask, TYPE_PRECISION (type), uns);
  	*val = double_int_ext (*val, TYPE_PRECISION (type), uns);
  	break;
*************** bit_value_binop_1 (enum tree_code code,
*** 1128,1135 ****
  		   tree r1type, double_int r1val, double_int r1mask,
  		   tree r2type, double_int r2val, double_int r2mask)
  {
!   bool uns = (TREE_CODE (type) == INTEGER_TYPE
! 	      && TYPE_IS_SIZETYPE (type) ? 0 : TYPE_UNSIGNED (type));
    /* Assume we'll get a constant result.  Use an initial varying value,
       we fall back to varying in the end if necessary.  */
    *mask = double_int_minus_one;
--- 1126,1132 ----
  		   tree r1type, double_int r1val, double_int r1mask,
  		   tree r2type, double_int r2val, double_int r2mask)
  {
!   bool uns = TYPE_UNSIGNED (type);
    /* Assume we'll get a constant result.  Use an initial varying value,
       we fall back to varying in the end if necessary.  */
    *mask = double_int_minus_one;
*************** bit_value_binop_1 (enum tree_code code,
*** 1196,1208 ****
  	    }
  	  else if (shift < 0)
  	    {
- 	      /* ???  We can have sizetype related inconsistencies in
- 		 the IL.  */
- 	      if ((TREE_CODE (r1type) == INTEGER_TYPE
- 		   && (TYPE_IS_SIZETYPE (r1type)
- 		       ? 0 : TYPE_UNSIGNED (r1type))) != uns)
- 		break;
- 
  	      shift = -shift;
  	      *mask = double_int_rshift (r1mask, shift,
  					 TYPE_PRECISION (type), !uns);
--- 1193,1198 ----
*************** bit_value_binop_1 (enum tree_code code,
*** 1314,1325 ****
  	  break;
  
  	/* For comparisons the signedness is in the comparison operands.  */
! 	uns = (TREE_CODE (r1type) == INTEGER_TYPE
! 	       && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type));
! 	/* ???  We can have sizetype related inconsistencies in the IL.  */
! 	if ((TREE_CODE (r2type) == INTEGER_TYPE
! 	     && TYPE_IS_SIZETYPE (r2type) ? 0 : TYPE_UNSIGNED (r2type)) != uns)
! 	  break;
  
  	/* If we know the most significant bits we know the values
  	   value ranges by means of treating varying bits as zero
--- 1304,1310 ----
  	  break;
  
  	/* For comparisons the signedness is in the comparison operands.  */
! 	uns = TYPE_UNSIGNED (r1type);
  
  	/* If we know the most significant bits we know the values
  	   value ranges by means of treating varying bits as zero
Index: trunk/gcc/tree.c
===================================================================
*** trunk.orig/gcc/tree.c	2011-08-18 13:40:55.000000000 +0200
--- trunk/gcc/tree.c	2011-08-18 13:43:39.000000000 +0200
*************** tree
*** 1059,1067 ****
  double_int_to_tree (tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = (!TYPE_UNSIGNED (type)
! 			     || (TREE_CODE (type) == INTEGER_TYPE
! 				 && TYPE_IS_SIZETYPE (type)));
  
    cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
  
--- 1059,1065 ----
  double_int_to_tree (tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = !TYPE_UNSIGNED (type);
  
    cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
  
*************** bool
*** 1075,1083 ****
  double_int_fits_to_tree_p (const_tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = (!TYPE_UNSIGNED (type)
! 			     || (TREE_CODE (type) == INTEGER_TYPE
! 				 && TYPE_IS_SIZETYPE (type)));
  
    double_int ext
      = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
--- 1073,1079 ----
  double_int_fits_to_tree_p (const_tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = !TYPE_UNSIGNED (type);
  
    double_int ext
      = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
*************** force_fit_type_double (tree type, double
*** 1107,1115 ****
    bool sign_extended_type;
  
    /* Size types *are* sign extended.  */
!   sign_extended_type = (!TYPE_UNSIGNED (type)
!                         || (TREE_CODE (type) == INTEGER_TYPE
!                             && TYPE_IS_SIZETYPE (type)));
  
    /* If we need to set overflow flags, return a new unshared node.  */
    if (overflowed || !double_int_fits_to_tree_p(type, cst))
--- 1103,1109 ----
    bool sign_extended_type;
  
    /* Size types *are* sign extended.  */
!   sign_extended_type = !TYPE_UNSIGNED (type);
  
    /* If we need to set overflow flags, return a new unshared node.  */
    if (overflowed || !double_int_fits_to_tree_p(type, cst))
*************** host_integerp (const_tree t, int pos)
*** 6496,6504 ****
  	       && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) >= 0)
  	      || (! pos && TREE_INT_CST_HIGH (t) == -1
  		  && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) < 0
! 		  && (!TYPE_UNSIGNED (TREE_TYPE (t))
! 		      || (TREE_CODE (TREE_TYPE (t)) == INTEGER_TYPE
! 			  && TYPE_IS_SIZETYPE (TREE_TYPE (t)))))
  	      || (pos && TREE_INT_CST_HIGH (t) == 0)));
  }
  
--- 6490,6496 ----
  	       && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) >= 0)
  	      || (! pos && TREE_INT_CST_HIGH (t) == -1
  		  && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) < 0
! 		  && !TYPE_UNSIGNED (TREE_TYPE (t)))
  	      || (pos && TREE_INT_CST_HIGH (t) == 0)));
  }
  
*************** int_fits_type_p (const_tree c, const_tre
*** 8188,8205 ****
    dc = tree_to_double_int (c);
    unsc = TYPE_UNSIGNED (TREE_TYPE (c));
  
-   if (TREE_CODE (TREE_TYPE (c)) == INTEGER_TYPE
-       && TYPE_IS_SIZETYPE (TREE_TYPE (c))
-       && unsc)
-     /* So c is an unsigned integer whose type is sizetype and type is not.
-        sizetype'd integers are sign extended even though they are
-        unsigned. If the integer value fits in the lower end word of c,
-        and if the higher end word has all its bits set to 1, that
-        means the higher end bits are set to 1 only for sign extension.
-        So let's convert c into an equivalent zero extended unsigned
-        integer.  */
-     dc = double_int_zext (dc, TYPE_PRECISION (TREE_TYPE (c)));
- 
  retry:
    type_low_bound = TYPE_MIN_VALUE (type);
    type_high_bound = TYPE_MAX_VALUE (type);
--- 8180,8185 ----
*************** retry:
*** 8218,8227 ****
    if (type_low_bound && TREE_CODE (type_low_bound) == INTEGER_CST)
      {
        dd = tree_to_double_int (type_low_bound);
-       if (TREE_CODE (type) == INTEGER_TYPE
- 	  && TYPE_IS_SIZETYPE (type)
- 	  && TYPE_UNSIGNED (type))
- 	dd = double_int_zext (dd, TYPE_PRECISION (type));
        if (unsc != TYPE_UNSIGNED (TREE_TYPE (type_low_bound)))
  	{
  	  int c_neg = (!unsc && double_int_negative_p (dc));
--- 8198,8203 ----
*************** retry:
*** 8243,8252 ****
    if (type_high_bound && TREE_CODE (type_high_bound) == INTEGER_CST)
      {
        dd = tree_to_double_int (type_high_bound);
-       if (TREE_CODE (type) == INTEGER_TYPE
- 	  && TYPE_IS_SIZETYPE (type)
- 	  && TYPE_UNSIGNED (type))
- 	dd = double_int_zext (dd, TYPE_PRECISION (type));
        if (unsc != TYPE_UNSIGNED (TREE_TYPE (type_high_bound)))
  	{
  	  int c_neg = (!unsc && double_int_negative_p (dc));
--- 8219,8224 ----
Index: trunk/gcc/ada/gcc-interface/utils.c
===================================================================
*** trunk.orig/gcc/ada/gcc-interface/utils.c	2011-08-18 13:40:55.000000000 +0200
--- trunk/gcc/ada/gcc-interface/utils.c	2011-08-18 13:43:39.000000000 +0200
*************** rest_of_record_type_compilation (tree re
*** 948,954 ****
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
--- 948,955 ----
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1)
! 	      && !tree_int_cst_sign_bit (TREE_OPERAND (curpos, 1)))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
*************** rest_of_record_type_compilation (tree re
*** 960,966 ****
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && TREE_INT_CST_HIGH (TREE_OPERAND (offset, 1)) < 0)
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
--- 961,967 ----
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && tree_int_cst_sign_bit (TREE_OPERAND (offset, 1)))
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
*************** rest_of_record_type_compilation (tree re
*** 975,982 ****
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1),
! 				     1))
  	    {
  	      align
  		= tree_low_cst
--- 976,984 ----
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1), 1)
! 		   && !tree_int_cst_sign_bit (TREE_OPERAND
! 					      (TREE_OPERAND (curpos, 0), 1), 1))
  	    {
  	      align
  		= tree_low_cst
Index: trunk/gcc/varasm.c
===================================================================
*** trunk.orig/gcc/varasm.c	2011-08-18 13:40:55.000000000 +0200
--- trunk/gcc/varasm.c	2011-08-18 13:43:39.000000000 +0200
*************** assemble_variable (tree decl, int top_le
*** 1980,1986 ****
      return;
  
    if (! dont_output_data
!       && ! host_integerp (DECL_SIZE_UNIT (decl), 1))
      {
        error ("size of variable %q+D is too large", decl);
        return;
--- 1980,1989 ----
      return;
  
    if (! dont_output_data
!       && (! host_integerp (DECL_SIZE_UNIT (decl), 1)
! 	  /* Restrict sizes of variables to half the address-space by
! 	     making sure the msb of the size is not set.  */
! 	  || tree_int_cst_sign_bit (DECL_SIZE_UNIT (decl)) != 0))
      {
        error ("size of variable %q+D is too large", decl);
        return;

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

* Re: [PATCH][RFC] Fix sizetype "sign" checks
  2011-08-19 10:06   ` Richard Guenther
@ 2011-08-24 16:05     ` Richard Guenther
  2011-08-25 10:31       ` Richard Guenther
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Guenther @ 2011-08-24 16:05 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Fri, 19 Aug 2011, Richard Guenther wrote:

> On Fri, 19 Aug 2011, Eric Botcazou wrote:
> 
> > > Looking at the Ada case I believe this happens because
> > > Ada has negative DECL_FIELD_OFFSET values (but that's
> > > again in sizetype, not ssizetype)?  Other host_integerp
> > > uses in Ada operate on sizes where I hope those are
> > > never negative ;)
> > 
> > Yes, the Ada compiler uses negative offsets for some peculiar constructs.
> > Nothing to do with the language per se, but with mechanisms implemented in 
> > gigi to support some features of the language.
> > 
> > > Eric, any better way of fixing this or would you be fine with this patch?
> > 
> > Hard to say without seeing the complete patch and playing a little with it.
> 
> This is the "complete" patch I am playing with currently, Ada bootstrap
> still fails for me unfortunately.  Bootstrap for all other languages
> succeeds, but there are some regressions, mostly warning-related.
> 
> Any help with pinpointing the Ada problem is welcome.

I have narrowed it down a bit.  Building Ada + libada unpatched,
patching the tree and then running the gnat.dg testsuite reveals

FAIL: gnat.dg/align_max.adb execution test
FAIL: gnat.dg/array11.adb  (test for warnings, line 12)
FAIL: gnat.dg/loop_optimization3.adb (test for excess errors)
WARNING: gnat.dg/loop_optimization3.adb compilation failed to produce 
executable
FAIL: gnat.dg/object_overflow.adb  (test for warnings, line 8)
FAIL: gnat.dg/thin_pointer2.adb execution test
FAIL: gnat.dg/unc_memfree.adb execution test

I just looked at gnat.dg/thin_pointer2.adb for now and we run into

utils2.c:build_simple_component_ref ()

  /* If the field's offset has overflowed, do not attempt to access it
     as doing so may trigger sanity checks deeper in the back-end.
     Note that we don't need to warn since this will be done on trying
     to declare the object.  */
  if (TREE_CODE (DECL_FIELD_OFFSET (field)) == INTEGER_CST
      && TREE_OVERFLOW (DECL_FIELD_OFFSET (field)))
    return NULL_TREE;

for fields with "negative" offset:

(gdb) call debug_tree (field)
 <field_decl 0x7ffff7ef7688 BOUNDS
    type <record_type 0x7ffff7f0f150 string___XUB readonly DI
        size <integer_cst 0x7ffff7ed3ec0 constant visited 64>
        unit size <integer_cst 0x7ffff7ed3ee0 constant visited 8>
        align 32 symtab 0 alias set -1 canonical type 0x7ffff7f0f150
        fields <field_decl 0x7ffff7ef7558 LB0 type <integer_type 
0x7ffff7ee35e8 integer>
            visited nonaddressable SI file <built-in> line 0 col 0
            size <integer_cst 0x7ffff7ee6200 constant visited 32>
            unit size <integer_cst 0x7ffff7ee6220 constant visited 4>
            align 32 offset_align 128
            offset <integer_cst 0x7ffff7ed3f00 constant visited 0>
            bit offset <integer_cst 0x7ffff7ed3f40 constant 0> context 
<record_type 0x7ffff7f0f150 string___XUB> chain <field_decl 0x7ffff7ef75f0 
UB0>> Ada size <integer_cst 0x7ffff7ed3ec0 64>
        pointer_to_this <pointer_type 0x7ffff7f0f1f8> chain <type_decl 
0x7ffff7ef48a0 string___XUB>>
    readonly DI file <built-in> line 0 col 0 size <integer_cst 
0x7ffff7ed3ec0 64> unit size <integer_cst 0x7ffff7ed3ee0 8>
    align 32 offset_align 128
    offset <integer_cst 0x7ffff7ee6e80 type <integer_type 0x7ffff7ee3000 
sizetype> constant public overflow 18446744073709551608> bit offset 
<integer_cst 0x7ffff7ed3f40 0> context <record_type 0x7ffff7f0fa80 
string___XUT> chain <field_decl 0x7ffff7ef7720 ARRAY>>

which is computed by shift_unc_components_for_thin_pointers doing

  DECL_FIELD_OFFSET (bounds_field)
    = size_binop (MINUS_EXPR, size_zero_node, byte_position (array_field));

which (at least) after my change to always make size_binop report
overflow (even for unsigned sizetype, to match what the code was
doing previously for sign_extended_types) results in 0 - 8
overflowing.

The above is probably not always an INTEGER_CST(?), but the following
should "fix" this particular issue.

Index: trunk/gcc/ada/gcc-interface/utils.c
===================================================================
--- trunk.orig/gcc/ada/gcc-interface/utils.c    2011-08-24 
16:06:21.000000000 +0200
+++ trunk/gcc/ada/gcc-interface/utils.c 2011-08-24 16:05:53.000000000 
+0200
@@ -3413,6 +3413,12 @@ shift_unc_components_for_thin_pointers (
 
   DECL_FIELD_OFFSET (bounds_field)
     = size_binop (MINUS_EXPR, size_zero_node, byte_position 
(array_field));
+  if (TREE_CODE (DECL_FIELD_OFFSET (bounds_field)) == INTEGER_CST
+      && TREE_OVERFLOW (DECL_FIELD_OFFSET (bounds_field)))
+    DECL_FIELD_OFFSET (bounds_field)
+      = build_int_cst_wide (sizetype,
+                           TREE_INT_CST_LOW (DECL_FIELD_OFFSET 
(bounds_field)),
+                           TREE_INT_CST_HIGH (DECL_FIELD_OFFSET 
(bounds_field)));
 
   DECL_FIELD_OFFSET (array_field) = size_zero_node;
   DECL_FIELD_BIT_OFFSET (array_field) = bitsize_zero_node;

Updated patch follows.  I hope the above makes me go past
building libada with the patch (maybe even bootstrapping ...).

ACATS fails w/o the above are (x86_64-linux)

                === acats tests ===
FAIL:   c36204d
FAIL:   c41107a
FAIL:   c43204a
FAIL:   c43204c
FAIL:   c43204e
FAIL:   c43204f
FAIL:   c43204g
FAIL:   c43204h
FAIL:   c43204i
FAIL:   c43205i
FAIL:   c52102a
FAIL:   c52102b
FAIL:   c52102c
FAIL:   c52102d
FAIL:   c95087a
FAIL:   cc1311a
FAIL:   cc3106b
FAIL:   cc3224a
FAIL:   cxh1001

Richard.

2011-06-16  Richard Guenther  <rguenther@suse.de>

	* fold-const.c (div_if_zero_remainder): sizetypes no longer
	sign-extend.
	* stor-layout.c (initialize_sizetypes): Likewise.
	* tree-ssa-ccp.c (bit_value_unop_1): Likewise.
	(bit_value_binop_1): Likewise.
	* tree.c (double_int_to_tree): Likewise.
	(double_int_fits_to_tree_p): Likewise.
	(force_fit_type_double): Likewise.
	(host_integerp): Likewise.
	(int_fits_type_p): Likewise.
	* tree-cfg.c (verify_types_in_gimple_reference): Do not compare
	sizes by pointer.

Index: trunk/gcc/fold-const.c
===================================================================
*** trunk.orig/gcc/fold-const.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/fold-const.c	2011-08-24 15:36:22.000000000 +0200
*************** div_if_zero_remainder (enum tree_code co
*** 194,202 ****
       does the correct thing for POINTER_PLUS_EXPR where we want
       a signed division.  */
    uns = TYPE_UNSIGNED (TREE_TYPE (arg2));
-   if (TREE_CODE (TREE_TYPE (arg2)) == INTEGER_TYPE
-       && TYPE_IS_SIZETYPE (TREE_TYPE (arg2)))
-     uns = false;
  
    quo = double_int_divmod (tree_to_double_int (arg1),
  			   tree_to_double_int (arg2),
--- 194,199 ----
*************** int_binop_types_match_p (enum tree_code
*** 938,945 ****
     to produce a new constant.  Return NULL_TREE if we don't know how
     to evaluate CODE at compile-time.  */
  
! tree
! int_const_binop (enum tree_code code, const_tree arg1, const_tree arg2)
  {
    double_int op1, op2, res, tmp;
    tree t;
--- 935,943 ----
     to produce a new constant.  Return NULL_TREE if we don't know how
     to evaluate CODE at compile-time.  */
  
! static tree
! int_const_binop_1 (enum tree_code code, const_tree arg1, const_tree arg2,
! 		   int overflowable)
  {
    double_int op1, op2, res, tmp;
    tree t;
*************** int_const_binop (enum tree_code code, co
*** 1081,1093 ****
        return NULL_TREE;
      }
  
!   t = force_fit_type_double (TREE_TYPE (arg1), res, 1,
  			     ((!uns || is_sizetype) && overflow)
  			     | TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2));
  
    return t;
  }
  
  /* Combine two constants ARG1 and ARG2 under operation CODE to produce a new
     constant.  We assume ARG1 and ARG2 have the same data type, or at least
     are the same kind of constant and the same machine mode.  Return zero if
--- 1079,1097 ----
        return NULL_TREE;
      }
  
!   t = force_fit_type_double (TREE_TYPE (arg1), res, overflowable,
  			     ((!uns || is_sizetype) && overflow)
  			     | TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2));
  
    return t;
  }
  
+ tree
+ int_const_binop (enum tree_code code, const_tree arg1, const_tree arg2)
+ {
+   return int_const_binop_1 (code, arg1, arg2, 1);
+ }
+ 
  /* Combine two constants ARG1 and ARG2 under operation CODE to produce a new
     constant.  We assume ARG1 and ARG2 have the same data type, or at least
     are the same kind of constant and the same machine mode.  Return zero if
*************** size_binop_loc (location_t loc, enum tre
*** 1448,1455 ****
  	    return arg1;
  	}
  
!       /* Handle general case of two integer constants.  */
!       return int_const_binop (code, arg0, arg1);
      }
  
    return fold_build2_loc (loc, code, type, arg0, arg1);
--- 1452,1461 ----
  	    return arg1;
  	}
  
!       /* Handle general case of two integer constants.  For sizetype
!          constant calculations we always want to know about overflow,
! 	 even in the unsigned case.  */
!       return int_const_binop_1 (code, arg0, arg1, -1);
      }
  
    return fold_build2_loc (loc, code, type, arg0, arg1);
Index: trunk/gcc/stor-layout.c
===================================================================
*** trunk.orig/gcc/stor-layout.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/stor-layout.c	2011-08-24 15:36:22.000000000 +0200
*************** initialize_sizetypes (void)
*** 2235,2245 ****
    TYPE_SIZE_UNIT (sizetype) = size_int (GET_MODE_SIZE (TYPE_MODE (sizetype)));
    set_min_and_max_values_for_integral_type (sizetype, precision,
  					    /*is_unsigned=*/true);
-   /* sizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is
-      sign-extended in a way consistent with force_fit_type.  */
-   TYPE_MAX_VALUE (sizetype)
-     = double_int_to_tree (sizetype,
- 			  tree_to_double_int (TYPE_MAX_VALUE (sizetype)));
  
    SET_TYPE_MODE (bitsizetype, smallest_mode_for_size (bprecision, MODE_INT));
    TYPE_ALIGN (bitsizetype) = GET_MODE_ALIGNMENT (TYPE_MODE (bitsizetype));
--- 2235,2240 ----
*************** initialize_sizetypes (void)
*** 2248,2258 ****
      = size_int (GET_MODE_SIZE (TYPE_MODE (bitsizetype)));
    set_min_and_max_values_for_integral_type (bitsizetype, bprecision,
  					    /*is_unsigned=*/true);
-   /* bitsizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is
-      sign-extended in a way consistent with force_fit_type.  */
-   TYPE_MAX_VALUE (bitsizetype)
-     = double_int_to_tree (bitsizetype,
- 			  tree_to_double_int (TYPE_MAX_VALUE (bitsizetype)));
  
    /* Create the signed variants of *sizetype.  */
    ssizetype = make_signed_type (TYPE_PRECISION (sizetype));
--- 2243,2248 ----
Index: trunk/gcc/tree-ssa-ccp.c
===================================================================
*** trunk.orig/gcc/tree-ssa-ccp.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/tree-ssa-ccp.c	2011-08-24 15:36:22.000000000 +0200
*************** bit_value_unop_1 (enum tree_code code, t
*** 1099,1112 ****
  	bool uns;
  
  	/* First extend mask and value according to the original type.  */
! 	uns = (TREE_CODE (rtype) == INTEGER_TYPE && TYPE_IS_SIZETYPE (rtype)
! 	       ? 0 : TYPE_UNSIGNED (rtype));
  	*mask = double_int_ext (rmask, TYPE_PRECISION (rtype), uns);
  	*val = double_int_ext (rval, TYPE_PRECISION (rtype), uns);
  
  	/* Then extend mask and value according to the target type.  */
! 	uns = (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type)
! 	       ? 0 : TYPE_UNSIGNED (type));
  	*mask = double_int_ext (*mask, TYPE_PRECISION (type), uns);
  	*val = double_int_ext (*val, TYPE_PRECISION (type), uns);
  	break;
--- 1099,1110 ----
  	bool uns;
  
  	/* First extend mask and value according to the original type.  */
! 	uns = TYPE_UNSIGNED (rtype);
  	*mask = double_int_ext (rmask, TYPE_PRECISION (rtype), uns);
  	*val = double_int_ext (rval, TYPE_PRECISION (rtype), uns);
  
  	/* Then extend mask and value according to the target type.  */
! 	uns = TYPE_UNSIGNED (type);
  	*mask = double_int_ext (*mask, TYPE_PRECISION (type), uns);
  	*val = double_int_ext (*val, TYPE_PRECISION (type), uns);
  	break;
*************** bit_value_binop_1 (enum tree_code code,
*** 1128,1135 ****
  		   tree r1type, double_int r1val, double_int r1mask,
  		   tree r2type, double_int r2val, double_int r2mask)
  {
!   bool uns = (TREE_CODE (type) == INTEGER_TYPE
! 	      && TYPE_IS_SIZETYPE (type) ? 0 : TYPE_UNSIGNED (type));
    /* Assume we'll get a constant result.  Use an initial varying value,
       we fall back to varying in the end if necessary.  */
    *mask = double_int_minus_one;
--- 1126,1132 ----
  		   tree r1type, double_int r1val, double_int r1mask,
  		   tree r2type, double_int r2val, double_int r2mask)
  {
!   bool uns = TYPE_UNSIGNED (type);
    /* Assume we'll get a constant result.  Use an initial varying value,
       we fall back to varying in the end if necessary.  */
    *mask = double_int_minus_one;
*************** bit_value_binop_1 (enum tree_code code,
*** 1196,1208 ****
  	    }
  	  else if (shift < 0)
  	    {
- 	      /* ???  We can have sizetype related inconsistencies in
- 		 the IL.  */
- 	      if ((TREE_CODE (r1type) == INTEGER_TYPE
- 		   && (TYPE_IS_SIZETYPE (r1type)
- 		       ? 0 : TYPE_UNSIGNED (r1type))) != uns)
- 		break;
- 
  	      shift = -shift;
  	      *mask = double_int_rshift (r1mask, shift,
  					 TYPE_PRECISION (type), !uns);
--- 1193,1198 ----
*************** bit_value_binop_1 (enum tree_code code,
*** 1314,1325 ****
  	  break;
  
  	/* For comparisons the signedness is in the comparison operands.  */
! 	uns = (TREE_CODE (r1type) == INTEGER_TYPE
! 	       && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type));
! 	/* ???  We can have sizetype related inconsistencies in the IL.  */
! 	if ((TREE_CODE (r2type) == INTEGER_TYPE
! 	     && TYPE_IS_SIZETYPE (r2type) ? 0 : TYPE_UNSIGNED (r2type)) != uns)
! 	  break;
  
  	/* If we know the most significant bits we know the values
  	   value ranges by means of treating varying bits as zero
--- 1304,1310 ----
  	  break;
  
  	/* For comparisons the signedness is in the comparison operands.  */
! 	uns = TYPE_UNSIGNED (r1type);
  
  	/* If we know the most significant bits we know the values
  	   value ranges by means of treating varying bits as zero
Index: trunk/gcc/tree.c
===================================================================
*** trunk.orig/gcc/tree.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/tree.c	2011-08-24 15:36:22.000000000 +0200
*************** tree
*** 1059,1067 ****
  double_int_to_tree (tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = (!TYPE_UNSIGNED (type)
! 			     || (TREE_CODE (type) == INTEGER_TYPE
! 				 && TYPE_IS_SIZETYPE (type)));
  
    cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
  
--- 1059,1065 ----
  double_int_to_tree (tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = !TYPE_UNSIGNED (type);
  
    cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
  
*************** bool
*** 1075,1083 ****
  double_int_fits_to_tree_p (const_tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = (!TYPE_UNSIGNED (type)
! 			     || (TREE_CODE (type) == INTEGER_TYPE
! 				 && TYPE_IS_SIZETYPE (type)));
  
    double_int ext
      = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
--- 1073,1079 ----
  double_int_fits_to_tree_p (const_tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = !TYPE_UNSIGNED (type);
  
    double_int ext
      = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
*************** force_fit_type_double (tree type, double
*** 1107,1115 ****
    bool sign_extended_type;
  
    /* Size types *are* sign extended.  */
!   sign_extended_type = (!TYPE_UNSIGNED (type)
!                         || (TREE_CODE (type) == INTEGER_TYPE
!                             && TYPE_IS_SIZETYPE (type)));
  
    /* If we need to set overflow flags, return a new unshared node.  */
    if (overflowed || !double_int_fits_to_tree_p(type, cst))
--- 1103,1109 ----
    bool sign_extended_type;
  
    /* Size types *are* sign extended.  */
!   sign_extended_type = !TYPE_UNSIGNED (type);
  
    /* If we need to set overflow flags, return a new unshared node.  */
    if (overflowed || !double_int_fits_to_tree_p(type, cst))
*************** host_integerp (const_tree t, int pos)
*** 6496,6504 ****
  	       && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) >= 0)
  	      || (! pos && TREE_INT_CST_HIGH (t) == -1
  		  && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) < 0
! 		  && (!TYPE_UNSIGNED (TREE_TYPE (t))
! 		      || (TREE_CODE (TREE_TYPE (t)) == INTEGER_TYPE
! 			  && TYPE_IS_SIZETYPE (TREE_TYPE (t)))))
  	      || (pos && TREE_INT_CST_HIGH (t) == 0)));
  }
  
--- 6490,6496 ----
  	       && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) >= 0)
  	      || (! pos && TREE_INT_CST_HIGH (t) == -1
  		  && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) < 0
! 		  && !TYPE_UNSIGNED (TREE_TYPE (t)))
  	      || (pos && TREE_INT_CST_HIGH (t) == 0)));
  }
  
*************** int_fits_type_p (const_tree c, const_tre
*** 8190,8207 ****
    dc = tree_to_double_int (c);
    unsc = TYPE_UNSIGNED (TREE_TYPE (c));
  
-   if (TREE_CODE (TREE_TYPE (c)) == INTEGER_TYPE
-       && TYPE_IS_SIZETYPE (TREE_TYPE (c))
-       && unsc)
-     /* So c is an unsigned integer whose type is sizetype and type is not.
-        sizetype'd integers are sign extended even though they are
-        unsigned. If the integer value fits in the lower end word of c,
-        and if the higher end word has all its bits set to 1, that
-        means the higher end bits are set to 1 only for sign extension.
-        So let's convert c into an equivalent zero extended unsigned
-        integer.  */
-     dc = double_int_zext (dc, TYPE_PRECISION (TREE_TYPE (c)));
- 
  retry:
    type_low_bound = TYPE_MIN_VALUE (type);
    type_high_bound = TYPE_MAX_VALUE (type);
--- 8182,8187 ----
*************** retry:
*** 8220,8229 ****
    if (type_low_bound && TREE_CODE (type_low_bound) == INTEGER_CST)
      {
        dd = tree_to_double_int (type_low_bound);
-       if (TREE_CODE (type) == INTEGER_TYPE
- 	  && TYPE_IS_SIZETYPE (type)
- 	  && TYPE_UNSIGNED (type))
- 	dd = double_int_zext (dd, TYPE_PRECISION (type));
        if (unsc != TYPE_UNSIGNED (TREE_TYPE (type_low_bound)))
  	{
  	  int c_neg = (!unsc && double_int_negative_p (dc));
--- 8200,8205 ----
*************** retry:
*** 8245,8254 ****
    if (type_high_bound && TREE_CODE (type_high_bound) == INTEGER_CST)
      {
        dd = tree_to_double_int (type_high_bound);
-       if (TREE_CODE (type) == INTEGER_TYPE
- 	  && TYPE_IS_SIZETYPE (type)
- 	  && TYPE_UNSIGNED (type))
- 	dd = double_int_zext (dd, TYPE_PRECISION (type));
        if (unsc != TYPE_UNSIGNED (TREE_TYPE (type_high_bound)))
  	{
  	  int c_neg = (!unsc && double_int_negative_p (dc));
--- 8221,8226 ----
Index: trunk/gcc/ada/gcc-interface/utils.c
===================================================================
*** trunk.orig/gcc/ada/gcc-interface/utils.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/ada/gcc-interface/utils.c	2011-08-24 16:05:53.000000000 +0200
*************** rest_of_record_type_compilation (tree re
*** 948,954 ****
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
--- 948,955 ----
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1)
! 	      && !tree_int_cst_sign_bit (TREE_OPERAND (curpos, 1)))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
*************** rest_of_record_type_compilation (tree re
*** 960,966 ****
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && TREE_INT_CST_HIGH (TREE_OPERAND (offset, 1)) < 0)
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
--- 961,967 ----
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && tree_int_cst_sign_bit (TREE_OPERAND (offset, 1)))
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
*************** rest_of_record_type_compilation (tree re
*** 975,982 ****
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1),
! 				     1))
  	    {
  	      align
  		= tree_low_cst
--- 976,984 ----
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1), 1)
! 		   && !tree_int_cst_sign_bit (TREE_OPERAND
! 					      (TREE_OPERAND (curpos, 0), 1)))
  	    {
  	      align
  		= tree_low_cst
*************** shift_unc_components_for_thin_pointers (
*** 3411,3416 ****
--- 3413,3424 ----
  
    DECL_FIELD_OFFSET (bounds_field)
      = size_binop (MINUS_EXPR, size_zero_node, byte_position (array_field));
+   if (TREE_CODE (DECL_FIELD_OFFSET (bounds_field)) == INTEGER_CST
+       && TREE_OVERFLOW (DECL_FIELD_OFFSET (bounds_field)))
+     DECL_FIELD_OFFSET (bounds_field)
+       = build_int_cst_wide (sizetype,
+ 			    TREE_INT_CST_LOW (DECL_FIELD_OFFSET (bounds_field)),
+ 			    TREE_INT_CST_HIGH (DECL_FIELD_OFFSET (bounds_field)));
  
    DECL_FIELD_OFFSET (array_field) = size_zero_node;
    DECL_FIELD_BIT_OFFSET (array_field) = bitsize_zero_node;
Index: trunk/gcc/varasm.c
===================================================================
*** trunk.orig/gcc/varasm.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/varasm.c	2011-08-24 15:36:23.000000000 +0200
*************** assemble_variable (tree decl, int top_le
*** 1980,1986 ****
      return;
  
    if (! dont_output_data
!       && ! host_integerp (DECL_SIZE_UNIT (decl), 1))
      {
        error ("size of variable %q+D is too large", decl);
        return;
--- 1980,1989 ----
      return;
  
    if (! dont_output_data
!       && (! host_integerp (DECL_SIZE_UNIT (decl), 1)
! 	  /* Restrict sizes of variables to half the address-space by
! 	     making sure the msb of the size is not set.  */
! 	  || tree_int_cst_sign_bit (DECL_SIZE_UNIT (decl)) != 0))
      {
        error ("size of variable %q+D is too large", decl);
        return;
Index: trunk/gcc/c-family/c-common.c
===================================================================
*** trunk.orig/gcc/c-family/c-common.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/c-family/c-common.c	2011-08-24 15:36:23.000000000 +0200
*************** pointer_int_sum (location_t loc, enum tr
*** 3790,3819 ****
        intop = convert (int_type, TREE_OPERAND (intop, 0));
      }
  
!   /* Convert the integer argument to a type the same size as sizetype
       so the multiply won't overflow spuriously.  */
    if (TYPE_PRECISION (TREE_TYPE (intop)) != TYPE_PRECISION (sizetype)
!       || TYPE_UNSIGNED (TREE_TYPE (intop)) != TYPE_UNSIGNED (sizetype))
      intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype),
! 					     TYPE_UNSIGNED (sizetype)), intop);
  
!   /* Replace the integer argument with a suitable product by the object size.
!      Do this multiplication as signed, then convert to the appropriate type
!      for the pointer operation and disregard an overflow that occured only
!      because of the sign-extension change in the latter conversion.  */
!   {
!     tree t = build_binary_op (loc,
! 			      MULT_EXPR, intop,
! 			      convert (TREE_TYPE (intop), size_exp), 1);
!     intop = convert (sizetype, t);
!     if (TREE_OVERFLOW_P (intop) && !TREE_OVERFLOW (t))
!       intop = build_int_cst_wide (TREE_TYPE (intop), TREE_INT_CST_LOW (intop),
! 				  TREE_INT_CST_HIGH (intop));
!   }
  
    /* Create the sum or difference.  */
    if (resultcode == MINUS_EXPR)
!     intop = fold_build1_loc (loc, NEGATE_EXPR, sizetype, intop);
  
    ret = fold_build_pointer_plus_loc (loc, ptrop, intop);
  
--- 3790,3811 ----
        intop = convert (int_type, TREE_OPERAND (intop, 0));
      }
  
!   /* Convert the integer argument to a signed type the same size as sizetype
       so the multiply won't overflow spuriously.  */
    if (TYPE_PRECISION (TREE_TYPE (intop)) != TYPE_PRECISION (sizetype)
!       || TYPE_UNSIGNED (TREE_TYPE (intop)))
      intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype),
! 					     false), intop);
  
!   /* Replace the integer argument with a suitable product by the
!      object size.  */
!   intop = build_binary_op (loc,
! 			   MULT_EXPR, intop,
! 			   convert (TREE_TYPE (intop), size_exp), 1);
  
    /* Create the sum or difference.  */
    if (resultcode == MINUS_EXPR)
!     intop = fold_build1_loc (loc, NEGATE_EXPR, TREE_TYPE (intop), intop);
  
    ret = fold_build_pointer_plus_loc (loc, ptrop, intop);
  

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

* Re: [PATCH][RFC] Fix sizetype "sign" checks
  2011-08-24 16:05     ` Richard Guenther
@ 2011-08-25 10:31       ` Richard Guenther
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Guenther @ 2011-08-25 10:31 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches

On Wed, 24 Aug 2011, Richard Guenther wrote:

> On Fri, 19 Aug 2011, Richard Guenther wrote:
> 
> > On Fri, 19 Aug 2011, Eric Botcazou wrote:
> > 
> > > > Looking at the Ada case I believe this happens because
> > > > Ada has negative DECL_FIELD_OFFSET values (but that's
> > > > again in sizetype, not ssizetype)?  Other host_integerp
> > > > uses in Ada operate on sizes where I hope those are
> > > > never negative ;)
> > > 
> > > Yes, the Ada compiler uses negative offsets for some peculiar constructs.
> > > Nothing to do with the language per se, but with mechanisms implemented in 
> > > gigi to support some features of the language.
> > > 
> > > > Eric, any better way of fixing this or would you be fine with this patch?
> > > 
> > > Hard to say without seeing the complete patch and playing a little with it.
> > 
> > This is the "complete" patch I am playing with currently, Ada bootstrap
> > still fails for me unfortunately.  Bootstrap for all other languages
> > succeeds, but there are some regressions, mostly warning-related.
> > 
> > Any help with pinpointing the Ada problem is welcome.

Another patch that makes the gnat.dg and acats testsuites clean
on x86_64-linux apart from diagnostic regressions is

Index: varasm.c
===================================================================
--- varasm.c    (revision 178035)
+++ varasm.c    (working copy)
@@ -4740,8 +4743,15 @@ output_constructor_regular_field (oc_loc
 
   if (local->index != NULL_TREE)
     {
-      double_int idx = double_int_sub (tree_to_double_int (local->index),
-                                      tree_to_double_int 
(local->min_index));
+      /* ???  Ada has negative DECL_FIELD_OFFSETs but we are using an
+         unsigned sizetype so make sure to sign-extend the indices before
+        subtracting them.  */
+      unsigned prec = TYPE_PRECISION (sizetype);
+      double_int idx
+       = double_int_sub (double_int_sext (tree_to_double_int 
(local->index),
+                                          prec),
+                         double_int_sext (tree_to_double_int
+                                            (local->min_index), prec));
       gcc_assert (double_int_fits_in_shwi_p (idx));
       fieldpos = (tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (local->val)), 
1)
                  * idx.low);

but I still fail to build Ada without bootstrapping as the RTS is
still miscompiled which results in gnatmake segfaulting like

Starting program: /home/abuild/rguenther/obj/gcc/gnatmake -c -b -I../rts 
-I. -I/space/rguenther/src/svn/trunk2/gcc/ada --GNATBIND=../../gnatbind 
--GCC=../../xgcc\ -B../../\ -g\ -O2\ \ -gnatpg\ -gnata gnatchop gnatcmd 
gnatkr gnatls gnatprep gnatxref gnatfind gnatname gnatclean -bargs 
-I../rts -I. -I/space/rguenther/src/svn/trunk2/gcc/ada -static -x

Program received signal SIGSEGV, Segmentation fault.
system.secondary_stack.ss_mark () at ../rts/s-secsta.adb:465
465              return (Sstk => Sstk, Sptr => To_Stack_Ptr (Sstk).Top);
(gdb) p Sstk
$1 = (system.address) 0x0

Are there any other places where GIGI would expect sign-extended
sizetype values?  Any hint where to look for the above failure?

system.soft_links.get_sec_stack_addr_nt doesn't seem to get called
at all.  Where is the machinery to eventually set it up located?
Grepping for stack or secondary doesn't reveal too much useful 
information.

Updated patch below.

Thanks,
Richard.

2011-06-16  Richard Guenther  <rguenther@suse.de>

	* fold-const.c (div_if_zero_remainder): sizetypes no longer
	sign-extend.
	* stor-layout.c (initialize_sizetypes): Likewise.
	* tree-ssa-ccp.c (bit_value_unop_1): Likewise.
	(bit_value_binop_1): Likewise.
	* tree.c (double_int_to_tree): Likewise.
	(double_int_fits_to_tree_p): Likewise.
	(force_fit_type_double): Likewise.
	(host_integerp): Likewise.
	(int_fits_type_p): Likewise.
	* tree-cfg.c (verify_types_in_gimple_reference): Do not compare
	sizes by pointer.

Index: trunk/gcc/fold-const.c
===================================================================
*** trunk.orig/gcc/fold-const.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/fold-const.c	2011-08-24 15:36:22.000000000 +0200
*************** div_if_zero_remainder (enum tree_code co
*** 194,202 ****
       does the correct thing for POINTER_PLUS_EXPR where we want
       a signed division.  */
    uns = TYPE_UNSIGNED (TREE_TYPE (arg2));
-   if (TREE_CODE (TREE_TYPE (arg2)) == INTEGER_TYPE
-       && TYPE_IS_SIZETYPE (TREE_TYPE (arg2)))
-     uns = false;
  
    quo = double_int_divmod (tree_to_double_int (arg1),
  			   tree_to_double_int (arg2),
--- 194,199 ----
*************** int_binop_types_match_p (enum tree_code
*** 938,945 ****
     to produce a new constant.  Return NULL_TREE if we don't know how
     to evaluate CODE at compile-time.  */
  
! tree
! int_const_binop (enum tree_code code, const_tree arg1, const_tree arg2)
  {
    double_int op1, op2, res, tmp;
    tree t;
--- 935,943 ----
     to produce a new constant.  Return NULL_TREE if we don't know how
     to evaluate CODE at compile-time.  */
  
! static tree
! int_const_binop_1 (enum tree_code code, const_tree arg1, const_tree arg2,
! 		   int overflowable)
  {
    double_int op1, op2, res, tmp;
    tree t;
*************** int_const_binop (enum tree_code code, co
*** 1081,1093 ****
        return NULL_TREE;
      }
  
!   t = force_fit_type_double (TREE_TYPE (arg1), res, 1,
  			     ((!uns || is_sizetype) && overflow)
  			     | TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2));
  
    return t;
  }
  
  /* Combine two constants ARG1 and ARG2 under operation CODE to produce a new
     constant.  We assume ARG1 and ARG2 have the same data type, or at least
     are the same kind of constant and the same machine mode.  Return zero if
--- 1079,1097 ----
        return NULL_TREE;
      }
  
!   t = force_fit_type_double (TREE_TYPE (arg1), res, overflowable,
  			     ((!uns || is_sizetype) && overflow)
  			     | TREE_OVERFLOW (arg1) | TREE_OVERFLOW (arg2));
  
    return t;
  }
  
+ tree
+ int_const_binop (enum tree_code code, const_tree arg1, const_tree arg2)
+ {
+   return int_const_binop_1 (code, arg1, arg2, 1);
+ }
+ 
  /* Combine two constants ARG1 and ARG2 under operation CODE to produce a new
     constant.  We assume ARG1 and ARG2 have the same data type, or at least
     are the same kind of constant and the same machine mode.  Return zero if
*************** size_binop_loc (location_t loc, enum tre
*** 1448,1455 ****
  	    return arg1;
  	}
  
!       /* Handle general case of two integer constants.  */
!       return int_const_binop (code, arg0, arg1);
      }
  
    return fold_build2_loc (loc, code, type, arg0, arg1);
--- 1452,1461 ----
  	    return arg1;
  	}
  
!       /* Handle general case of two integer constants.  For sizetype
!          constant calculations we always want to know about overflow,
! 	 even in the unsigned case.  */
!       return int_const_binop_1 (code, arg0, arg1, -1);
      }
  
    return fold_build2_loc (loc, code, type, arg0, arg1);
Index: trunk/gcc/stor-layout.c
===================================================================
*** trunk.orig/gcc/stor-layout.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/stor-layout.c	2011-08-24 15:36:22.000000000 +0200
*************** initialize_sizetypes (void)
*** 2235,2245 ****
    TYPE_SIZE_UNIT (sizetype) = size_int (GET_MODE_SIZE (TYPE_MODE (sizetype)));
    set_min_and_max_values_for_integral_type (sizetype, precision,
  					    /*is_unsigned=*/true);
-   /* sizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is
-      sign-extended in a way consistent with force_fit_type.  */
-   TYPE_MAX_VALUE (sizetype)
-     = double_int_to_tree (sizetype,
- 			  tree_to_double_int (TYPE_MAX_VALUE (sizetype)));
  
    SET_TYPE_MODE (bitsizetype, smallest_mode_for_size (bprecision, MODE_INT));
    TYPE_ALIGN (bitsizetype) = GET_MODE_ALIGNMENT (TYPE_MODE (bitsizetype));
--- 2235,2240 ----
*************** initialize_sizetypes (void)
*** 2248,2258 ****
      = size_int (GET_MODE_SIZE (TYPE_MODE (bitsizetype)));
    set_min_and_max_values_for_integral_type (bitsizetype, bprecision,
  					    /*is_unsigned=*/true);
-   /* bitsizetype is unsigned but we need to fix TYPE_MAX_VALUE so that it is
-      sign-extended in a way consistent with force_fit_type.  */
-   TYPE_MAX_VALUE (bitsizetype)
-     = double_int_to_tree (bitsizetype,
- 			  tree_to_double_int (TYPE_MAX_VALUE (bitsizetype)));
  
    /* Create the signed variants of *sizetype.  */
    ssizetype = make_signed_type (TYPE_PRECISION (sizetype));
--- 2243,2248 ----
Index: trunk/gcc/tree-ssa-ccp.c
===================================================================
*** trunk.orig/gcc/tree-ssa-ccp.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/tree-ssa-ccp.c	2011-08-24 15:36:22.000000000 +0200
*************** bit_value_unop_1 (enum tree_code code, t
*** 1099,1112 ****
  	bool uns;
  
  	/* First extend mask and value according to the original type.  */
! 	uns = (TREE_CODE (rtype) == INTEGER_TYPE && TYPE_IS_SIZETYPE (rtype)
! 	       ? 0 : TYPE_UNSIGNED (rtype));
  	*mask = double_int_ext (rmask, TYPE_PRECISION (rtype), uns);
  	*val = double_int_ext (rval, TYPE_PRECISION (rtype), uns);
  
  	/* Then extend mask and value according to the target type.  */
! 	uns = (TREE_CODE (type) == INTEGER_TYPE && TYPE_IS_SIZETYPE (type)
! 	       ? 0 : TYPE_UNSIGNED (type));
  	*mask = double_int_ext (*mask, TYPE_PRECISION (type), uns);
  	*val = double_int_ext (*val, TYPE_PRECISION (type), uns);
  	break;
--- 1099,1110 ----
  	bool uns;
  
  	/* First extend mask and value according to the original type.  */
! 	uns = TYPE_UNSIGNED (rtype);
  	*mask = double_int_ext (rmask, TYPE_PRECISION (rtype), uns);
  	*val = double_int_ext (rval, TYPE_PRECISION (rtype), uns);
  
  	/* Then extend mask and value according to the target type.  */
! 	uns = TYPE_UNSIGNED (type);
  	*mask = double_int_ext (*mask, TYPE_PRECISION (type), uns);
  	*val = double_int_ext (*val, TYPE_PRECISION (type), uns);
  	break;
*************** bit_value_binop_1 (enum tree_code code,
*** 1128,1135 ****
  		   tree r1type, double_int r1val, double_int r1mask,
  		   tree r2type, double_int r2val, double_int r2mask)
  {
!   bool uns = (TREE_CODE (type) == INTEGER_TYPE
! 	      && TYPE_IS_SIZETYPE (type) ? 0 : TYPE_UNSIGNED (type));
    /* Assume we'll get a constant result.  Use an initial varying value,
       we fall back to varying in the end if necessary.  */
    *mask = double_int_minus_one;
--- 1126,1132 ----
  		   tree r1type, double_int r1val, double_int r1mask,
  		   tree r2type, double_int r2val, double_int r2mask)
  {
!   bool uns = TYPE_UNSIGNED (type);
    /* Assume we'll get a constant result.  Use an initial varying value,
       we fall back to varying in the end if necessary.  */
    *mask = double_int_minus_one;
*************** bit_value_binop_1 (enum tree_code code,
*** 1196,1208 ****
  	    }
  	  else if (shift < 0)
  	    {
- 	      /* ???  We can have sizetype related inconsistencies in
- 		 the IL.  */
- 	      if ((TREE_CODE (r1type) == INTEGER_TYPE
- 		   && (TYPE_IS_SIZETYPE (r1type)
- 		       ? 0 : TYPE_UNSIGNED (r1type))) != uns)
- 		break;
- 
  	      shift = -shift;
  	      *mask = double_int_rshift (r1mask, shift,
  					 TYPE_PRECISION (type), !uns);
--- 1193,1198 ----
*************** bit_value_binop_1 (enum tree_code code,
*** 1314,1325 ****
  	  break;
  
  	/* For comparisons the signedness is in the comparison operands.  */
! 	uns = (TREE_CODE (r1type) == INTEGER_TYPE
! 	       && TYPE_IS_SIZETYPE (r1type) ? 0 : TYPE_UNSIGNED (r1type));
! 	/* ???  We can have sizetype related inconsistencies in the IL.  */
! 	if ((TREE_CODE (r2type) == INTEGER_TYPE
! 	     && TYPE_IS_SIZETYPE (r2type) ? 0 : TYPE_UNSIGNED (r2type)) != uns)
! 	  break;
  
  	/* If we know the most significant bits we know the values
  	   value ranges by means of treating varying bits as zero
--- 1304,1310 ----
  	  break;
  
  	/* For comparisons the signedness is in the comparison operands.  */
! 	uns = TYPE_UNSIGNED (r1type);
  
  	/* If we know the most significant bits we know the values
  	   value ranges by means of treating varying bits as zero
Index: trunk/gcc/tree.c
===================================================================
*** trunk.orig/gcc/tree.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/tree.c	2011-08-24 15:36:22.000000000 +0200
*************** tree
*** 1059,1067 ****
  double_int_to_tree (tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = (!TYPE_UNSIGNED (type)
! 			     || (TREE_CODE (type) == INTEGER_TYPE
! 				 && TYPE_IS_SIZETYPE (type)));
  
    cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
  
--- 1059,1065 ----
  double_int_to_tree (tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = !TYPE_UNSIGNED (type);
  
    cst = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
  
*************** bool
*** 1075,1083 ****
  double_int_fits_to_tree_p (const_tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = (!TYPE_UNSIGNED (type)
! 			     || (TREE_CODE (type) == INTEGER_TYPE
! 				 && TYPE_IS_SIZETYPE (type)));
  
    double_int ext
      = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
--- 1073,1079 ----
  double_int_fits_to_tree_p (const_tree type, double_int cst)
  {
    /* Size types *are* sign extended.  */
!   bool sign_extended_type = !TYPE_UNSIGNED (type);
  
    double_int ext
      = double_int_ext (cst, TYPE_PRECISION (type), !sign_extended_type);
*************** force_fit_type_double (tree type, double
*** 1107,1115 ****
    bool sign_extended_type;
  
    /* Size types *are* sign extended.  */
!   sign_extended_type = (!TYPE_UNSIGNED (type)
!                         || (TREE_CODE (type) == INTEGER_TYPE
!                             && TYPE_IS_SIZETYPE (type)));
  
    /* If we need to set overflow flags, return a new unshared node.  */
    if (overflowed || !double_int_fits_to_tree_p(type, cst))
--- 1103,1109 ----
    bool sign_extended_type;
  
    /* Size types *are* sign extended.  */
!   sign_extended_type = !TYPE_UNSIGNED (type);
  
    /* If we need to set overflow flags, return a new unshared node.  */
    if (overflowed || !double_int_fits_to_tree_p(type, cst))
*************** host_integerp (const_tree t, int pos)
*** 6496,6504 ****
  	       && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) >= 0)
  	      || (! pos && TREE_INT_CST_HIGH (t) == -1
  		  && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) < 0
! 		  && (!TYPE_UNSIGNED (TREE_TYPE (t))
! 		      || (TREE_CODE (TREE_TYPE (t)) == INTEGER_TYPE
! 			  && TYPE_IS_SIZETYPE (TREE_TYPE (t)))))
  	      || (pos && TREE_INT_CST_HIGH (t) == 0)));
  }
  
--- 6490,6496 ----
  	       && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) >= 0)
  	      || (! pos && TREE_INT_CST_HIGH (t) == -1
  		  && (HOST_WIDE_INT) TREE_INT_CST_LOW (t) < 0
! 		  && !TYPE_UNSIGNED (TREE_TYPE (t)))
  	      || (pos && TREE_INT_CST_HIGH (t) == 0)));
  }
  
*************** int_fits_type_p (const_tree c, const_tre
*** 8190,8207 ****
    dc = tree_to_double_int (c);
    unsc = TYPE_UNSIGNED (TREE_TYPE (c));
  
-   if (TREE_CODE (TREE_TYPE (c)) == INTEGER_TYPE
-       && TYPE_IS_SIZETYPE (TREE_TYPE (c))
-       && unsc)
-     /* So c is an unsigned integer whose type is sizetype and type is not.
-        sizetype'd integers are sign extended even though they are
-        unsigned. If the integer value fits in the lower end word of c,
-        and if the higher end word has all its bits set to 1, that
-        means the higher end bits are set to 1 only for sign extension.
-        So let's convert c into an equivalent zero extended unsigned
-        integer.  */
-     dc = double_int_zext (dc, TYPE_PRECISION (TREE_TYPE (c)));
- 
  retry:
    type_low_bound = TYPE_MIN_VALUE (type);
    type_high_bound = TYPE_MAX_VALUE (type);
--- 8182,8187 ----
*************** retry:
*** 8220,8229 ****
    if (type_low_bound && TREE_CODE (type_low_bound) == INTEGER_CST)
      {
        dd = tree_to_double_int (type_low_bound);
-       if (TREE_CODE (type) == INTEGER_TYPE
- 	  && TYPE_IS_SIZETYPE (type)
- 	  && TYPE_UNSIGNED (type))
- 	dd = double_int_zext (dd, TYPE_PRECISION (type));
        if (unsc != TYPE_UNSIGNED (TREE_TYPE (type_low_bound)))
  	{
  	  int c_neg = (!unsc && double_int_negative_p (dc));
--- 8200,8205 ----
*************** retry:
*** 8245,8254 ****
    if (type_high_bound && TREE_CODE (type_high_bound) == INTEGER_CST)
      {
        dd = tree_to_double_int (type_high_bound);
-       if (TREE_CODE (type) == INTEGER_TYPE
- 	  && TYPE_IS_SIZETYPE (type)
- 	  && TYPE_UNSIGNED (type))
- 	dd = double_int_zext (dd, TYPE_PRECISION (type));
        if (unsc != TYPE_UNSIGNED (TREE_TYPE (type_high_bound)))
  	{
  	  int c_neg = (!unsc && double_int_negative_p (dc));
--- 8221,8226 ----
Index: trunk/gcc/ada/gcc-interface/utils.c
===================================================================
*** trunk.orig/gcc/ada/gcc-interface/utils.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/ada/gcc-interface/utils.c	2011-08-24 16:05:53.000000000 +0200
*************** rest_of_record_type_compilation (tree re
*** 948,954 ****
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
--- 948,955 ----
  	    pos = compute_related_constant (curpos, last_pos);
  
  	  if (!pos && TREE_CODE (curpos) == MULT_EXPR
! 	      && host_integerp (TREE_OPERAND (curpos, 1), 1)
! 	      && !tree_int_cst_sign_bit (TREE_OPERAND (curpos, 1)))
  	    {
  	      tree offset = TREE_OPERAND (curpos, 0);
  	      align = tree_low_cst (TREE_OPERAND (curpos, 1), 1);
*************** rest_of_record_type_compilation (tree re
*** 960,966 ****
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && TREE_INT_CST_HIGH (TREE_OPERAND (offset, 1)) < 0)
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
--- 961,967 ----
  	      offset = remove_conversions (offset, true);
  	      if (TREE_CODE (offset) == BIT_AND_EXPR
  		  && host_integerp (TREE_OPERAND (offset, 1), 0)
! 		  && tree_int_cst_sign_bit (TREE_OPERAND (offset, 1)))
  		{
  		  unsigned int pow
  		    = - tree_low_cst (TREE_OPERAND (offset, 1), 0);
*************** rest_of_record_type_compilation (tree re
*** 975,982 ****
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1),
! 				     1))
  	    {
  	      align
  		= tree_low_cst
--- 976,984 ----
  		   && TREE_CODE (TREE_OPERAND (curpos, 1)) == INTEGER_CST
  		   && TREE_CODE (TREE_OPERAND (curpos, 0)) == MULT_EXPR
  		   && host_integerp (TREE_OPERAND
! 				     (TREE_OPERAND (curpos, 0), 1), 1)
! 		   && !tree_int_cst_sign_bit (TREE_OPERAND
! 					      (TREE_OPERAND (curpos, 0), 1)))
  	    {
  	      align
  		= tree_low_cst
*************** shift_unc_components_for_thin_pointers (
*** 3411,3416 ****
--- 3413,3424 ----
  
    DECL_FIELD_OFFSET (bounds_field)
      = size_binop (MINUS_EXPR, size_zero_node, byte_position (array_field));
+   if (TREE_CODE (DECL_FIELD_OFFSET (bounds_field)) == INTEGER_CST
+       && TREE_OVERFLOW (DECL_FIELD_OFFSET (bounds_field)))
+     DECL_FIELD_OFFSET (bounds_field)
+       = build_int_cst_wide (sizetype,
+ 			    TREE_INT_CST_LOW (DECL_FIELD_OFFSET (bounds_field)),
+ 			    TREE_INT_CST_HIGH (DECL_FIELD_OFFSET (bounds_field)));
  
    DECL_FIELD_OFFSET (array_field) = size_zero_node;
    DECL_FIELD_BIT_OFFSET (array_field) = bitsize_zero_node;
Index: trunk/gcc/varasm.c
===================================================================
*** trunk.orig/gcc/varasm.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/varasm.c	2011-08-25 09:51:16.000000000 +0200
*************** assemble_variable (tree decl, int top_le
*** 1980,1986 ****
      return;
  
    if (! dont_output_data
!       && ! host_integerp (DECL_SIZE_UNIT (decl), 1))
      {
        error ("size of variable %q+D is too large", decl);
        return;
--- 1980,1989 ----
      return;
  
    if (! dont_output_data
!       && (! host_integerp (DECL_SIZE_UNIT (decl), 1)
! 	  /* Restrict sizes of variables to half the address-space by
! 	     making sure the msb of the size is not set.  */
! 	  || tree_int_cst_sign_bit (DECL_SIZE_UNIT (decl)) != 0))
      {
        error ("size of variable %q+D is too large", decl);
        return;
*************** output_constructor_regular_field (oc_loc
*** 4740,4747 ****
  
    if (local->index != NULL_TREE)
      {
!       double_int idx = double_int_sub (tree_to_double_int (local->index),
! 				       tree_to_double_int (local->min_index));
        gcc_assert (double_int_fits_in_shwi_p (idx));
        fieldpos = (tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (local->val)), 1)
  		  * idx.low);
--- 4743,4757 ----
  
    if (local->index != NULL_TREE)
      {
!       /* ???  Ada has negative DECL_FIELD_OFFSETs but we are using an
!          unsigned sizetype so make sure to sign-extend the indices before
! 	 subtracting them.  */
!       unsigned prec = TYPE_PRECISION (sizetype);
!       double_int idx
! 	= double_int_sub (double_int_sext (tree_to_double_int (local->index),
! 					   prec),
! 			  double_int_sext (tree_to_double_int
! 					     (local->min_index), prec));
        gcc_assert (double_int_fits_in_shwi_p (idx));
        fieldpos = (tree_low_cst (TYPE_SIZE_UNIT (TREE_TYPE (local->val)), 1)
  		  * idx.low);
Index: trunk/gcc/c-family/c-common.c
===================================================================
*** trunk.orig/gcc/c-family/c-common.c	2011-08-24 15:34:13.000000000 +0200
--- trunk/gcc/c-family/c-common.c	2011-08-24 15:36:23.000000000 +0200
*************** pointer_int_sum (location_t loc, enum tr
*** 3790,3819 ****
        intop = convert (int_type, TREE_OPERAND (intop, 0));
      }
  
!   /* Convert the integer argument to a type the same size as sizetype
       so the multiply won't overflow spuriously.  */
    if (TYPE_PRECISION (TREE_TYPE (intop)) != TYPE_PRECISION (sizetype)
!       || TYPE_UNSIGNED (TREE_TYPE (intop)) != TYPE_UNSIGNED (sizetype))
      intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype),
! 					     TYPE_UNSIGNED (sizetype)), intop);
  
!   /* Replace the integer argument with a suitable product by the object size.
!      Do this multiplication as signed, then convert to the appropriate type
!      for the pointer operation and disregard an overflow that occured only
!      because of the sign-extension change in the latter conversion.  */
!   {
!     tree t = build_binary_op (loc,
! 			      MULT_EXPR, intop,
! 			      convert (TREE_TYPE (intop), size_exp), 1);
!     intop = convert (sizetype, t);
!     if (TREE_OVERFLOW_P (intop) && !TREE_OVERFLOW (t))
!       intop = build_int_cst_wide (TREE_TYPE (intop), TREE_INT_CST_LOW (intop),
! 				  TREE_INT_CST_HIGH (intop));
!   }
  
    /* Create the sum or difference.  */
    if (resultcode == MINUS_EXPR)
!     intop = fold_build1_loc (loc, NEGATE_EXPR, sizetype, intop);
  
    ret = fold_build_pointer_plus_loc (loc, ptrop, intop);
  
--- 3790,3811 ----
        intop = convert (int_type, TREE_OPERAND (intop, 0));
      }
  
!   /* Convert the integer argument to a signed type the same size as sizetype
       so the multiply won't overflow spuriously.  */
    if (TYPE_PRECISION (TREE_TYPE (intop)) != TYPE_PRECISION (sizetype)
!       || TYPE_UNSIGNED (TREE_TYPE (intop)))
      intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype),
! 					     false), intop);
  
!   /* Replace the integer argument with a suitable product by the
!      object size.  */
!   intop = build_binary_op (loc,
! 			   MULT_EXPR, intop,
! 			   convert (TREE_TYPE (intop), size_exp), 1);
  
    /* Create the sum or difference.  */
    if (resultcode == MINUS_EXPR)
!     intop = fold_build1_loc (loc, NEGATE_EXPR, TREE_TYPE (intop), intop);
  
    ret = fold_build_pointer_plus_loc (loc, ptrop, intop);
  

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

end of thread, other threads:[~2011-08-25  8:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-18 13:10 [PATCH][RFC] Fix sizetype "sign" checks Richard Guenther
2011-08-19  7:10 ` Eric Botcazou
2011-08-19 10:06   ` Richard Guenther
2011-08-24 16:05     ` Richard Guenther
2011-08-25 10:31       ` Richard Guenther

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