Re: suspect code in fold-const.c

[Kenneth Zadeck <zadeck at naturalbridge dot com>]

This patch fixes a number of places where the mode bitsize had been used 
but the mode precision should have been used.  The tree level is 
somewhat sloppy about this - some places use the mode precision and some 
use the mode bitsize.   It seems that the mode precision is the proper 
choice since it does the correct thing if the underlying mode is a 
partial int mode.

This code has been tested on x86-64 with no regressions.   Ok to commit?



2013-11-15 Kenneth Zadeck <zadeck@naturalbridge.com>
    * tree.c (int_fits_type_p): Change GET_MODE_BITSIZE to
    GET_MODE_PRECISION.
    * fold-const.c (fold_single_bit_test_into_sign_test,
    fold_binary_loc):  Change GET_MODE_BITSIZE to
    GET_MODE_PRECISION.

Kenny


On 11/15/2013 08:32 AM, Kenneth Zadeck wrote:
> On 11/15/2013 04:07 AM, Eric Botcazou wrote:
> > > this code from fold-const.c starts on line 13811.
> > >
> > >           else if (TREE_INT_CST_HIGH (arg1) == signed_max_hi
> > >                && TREE_INT_CST_LOW (arg1) == signed_max_lo
> > >                && TYPE_UNSIGNED (arg1_type)
> > >                /* We will flip the signedness of the comparison 
> > > operator
> > >               associated with the mode of arg1, so the sign bit is
> > >               specified by this mode.  Check that arg1 is the signed
> > >               max associated with this sign bit.  */
> > >                && width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type))
> > >                /* signed_type does not work on pointer types. */
> > >                && INTEGRAL_TYPE_P (arg1_type))
> > with width defined as:
> >
> >     unsigned int width = TYPE_PRECISION (arg1_type);
> >
> > > it seems that the check on bitsize should really be a check on the
> > > precision of the variable.   If this seems right, i will correct 
> > > this on
> > > the trunk and make the appropriate changes to the wide-int branch.
> > Do you mean
> >
> >    && width == GET_MODE_PRECISION (TYPE_MODE (arg1_type))
> >
> > instead?  If so, that would probably make sense, but there are a few 
> > other
> > places with the same TYPE_PRECISION/GET_MODE_BITSIZE check, in 
> > particular the
> > very similar transformation done in fold_single_bit_test_into_sign_test.
> yes.  I understand the need to do this check on the mode rather than 
> the precision of the type itself.
> The point is that if the mode under this type happens to be a partial 
> int mode, then that sign bit may not even be where the bitsize points to.
>
> However, having just done a few greps, it looks like this case was 
> just the one that i found while doing the wide-int work, there may be 
> several more of these cases.   Just in fold-const, there are a couple 
> in fold_binary_loc.   The one in tree.c:int_fits_type_p looks 
> particularly wrong.
>
> I think that there are also several in tree-vect-patterns.c.
>
> Kenny

Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c	(revision 204842)
+++ gcc/fold-const.c	(working copy)
@@ -6593,7 +6593,7 @@ fold_single_bit_test_into_sign_test (loc
 	  /* This is only a win if casting to a signed type is cheap,
 	     i.e. when arg00's type is not a partial mode.  */
 	  && TYPE_PRECISION (TREE_TYPE (arg00))
-	     == GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (arg00))))
+	     == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (arg00))))
 	{
 	  tree stype = signed_type_for (TREE_TYPE (arg00));
 	  return fold_build2_loc (loc, code == EQ_EXPR ? GE_EXPR : LT_EXPR,
@@ -12050,7 +12050,7 @@ fold_binary_loc (location_t loc,
 	    zerobits = ((((unsigned HOST_WIDE_INT) 1) << shiftc) - 1);
 	  else if (TREE_CODE (arg0) == RSHIFT_EXPR
 		   && TYPE_PRECISION (TREE_TYPE (arg0))
-		      == GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (arg0))))
+		      == GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (arg0))))
 	    {
 	      prec = TYPE_PRECISION (TREE_TYPE (arg0));
 	      tree arg00 = TREE_OPERAND (arg0, 0);
@@ -12061,7 +12061,7 @@ fold_binary_loc (location_t loc,
 		{
 		  tree inner_type = TREE_TYPE (TREE_OPERAND (arg00, 0));
 		  if (TYPE_PRECISION (inner_type)
-		      == GET_MODE_BITSIZE (TYPE_MODE (inner_type))
+		      == GET_MODE_PRECISION (TYPE_MODE (inner_type))
 		      && TYPE_PRECISION (inner_type) < prec)
 		    {
 		      prec = TYPE_PRECISION (inner_type);
@@ -13816,7 +13816,7 @@ fold_binary_loc (location_t loc,
 			associated with the mode of arg1, so the sign bit is
 			specified by this mode.  Check that arg1 is the signed
 			max associated with this sign bit.  */
-		     && width == GET_MODE_BITSIZE (TYPE_MODE (arg1_type))
+		     && width == GET_MODE_PRECISION (TYPE_MODE (arg1_type))
 		     /* signed_type does not work on pointer types.  */
 		     && INTEGRAL_TYPE_P (arg1_type))
 	      {
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 204842)
+++ gcc/tree.c	(working copy)
@@ -8614,7 +8614,7 @@ retry:
   /* Third, unsigned integers with top bit set never fit signed types.  */
   if (! TYPE_UNSIGNED (type) && unsc)
     {
-      int prec = GET_MODE_BITSIZE (TYPE_MODE (TREE_TYPE (c))) - 1;
+      int prec = GET_MODE_PRECISION (TYPE_MODE (TREE_TYPE (c))) - 1;
       if (prec < HOST_BITS_PER_WIDE_INT)
 	{
 	  if (((((unsigned HOST_WIDE_INT) 1) << prec) & dc.low) != 0)