[RFC,wide-int] Fix some build errors on arm in wide-int branch and report ICE

Kyrill Tkachov <kyrylo.tkachov@arm.com> writes:
> With that patch bootstrap now still fails at dwarf2out.c with the same
> message. I'm attaching a gzipped dwarf2out.ii

Thanks.  This is a nice proof of why clz_zero and ctz_zero were as bogus
as claimed.  It meant that the behaviour of floor_log2 depended on the
target and would return the wrong value if clz (0) was anything other
than the precision.

This patch makes the wide-int functions behave like the double_int
ones and pushes the target dependency back to the callers that care,
which is where it belongs.  The "new" *_DEFINED_VALUE_AT_ZERO checks
are really reinstating what's already on trunk.  There are other tree
uses of ctz that I think relied on the double_int behaviour.

Tests still ongoing, but could you check what the arm results are like
with this?

Thanks,
Richard

ok to commit.

kenny
On 04/28/2014 11:42 AM, Richard Sandiford wrote:
> Kyrill Tkachov <kyrylo.tkachov@arm.com> writes:
>> With that patch bootstrap now still fails at dwarf2out.c with the same
>> message. I'm attaching a gzipped dwarf2out.ii
> Thanks.  This is a nice proof of why clz_zero and ctz_zero were as bogus
> as claimed.  It meant that the behaviour of floor_log2 depended on the
> target and would return the wrong value if clz (0) was anything other
> than the precision.
>
> This patch makes the wide-int functions behave like the double_int
> ones and pushes the target dependency back to the callers that care,
> which is where it belongs.  The "new" *_DEFINED_VALUE_AT_ZERO checks
> are really reinstating what's already on trunk.  There are other tree
> uses of ctz that I think relied on the double_int behaviour.
>
> Tests still ongoing, but could you check what the arm results are like
> with this?
>
> Thanks,
> Richard
>
>
> Index: gcc/builtins.c
> ===================================================================
> --- gcc/builtins.c	2014-04-28 16:30:59.939239843 +0100
> +++ gcc/builtins.c	2014-04-28 16:31:00.252238996 +0100
> @@ -8080,6 +8080,7 @@ fold_builtin_bitop (tree fndecl, tree ar
>     /* Optimize for constant argument.  */
>     if (TREE_CODE (arg) == INTEGER_CST && !TREE_OVERFLOW (arg))
>       {
> +      tree type = TREE_TYPE (arg);
>         int result;
>   
>         switch (DECL_FUNCTION_CODE (fndecl))
> @@ -8089,11 +8090,17 @@ fold_builtin_bitop (tree fndecl, tree ar
>   	  break;
>   
>   	CASE_INT_FN (BUILT_IN_CLZ):
> -	  result = wi::clz (arg);
> +	  if (wi::ne_p (arg, 0))
> +	    result = wi::clz (arg);
> +	  else if (! CLZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (type), result))
> +	    result = TYPE_PRECISION (type);
>   	  break;
>   
>   	CASE_INT_FN (BUILT_IN_CTZ):
> -	  result = wi::ctz (arg);
> +	  if (wi::ne_p (arg, 0))
> +	    result = wi::ctz (arg);
> +	  else if (! CTZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (type), result))
> +	    result = TYPE_PRECISION (type);
>   	  break;
>   
>   	CASE_INT_FN (BUILT_IN_CLRSB):
> Index: gcc/simplify-rtx.c
> ===================================================================
> --- gcc/simplify-rtx.c	2014-04-28 16:30:59.941239838 +0100
> +++ gcc/simplify-rtx.c	2014-04-28 16:31:00.254238990 +0100
> @@ -1656,6 +1656,7 @@ simplify_const_unary_operation (enum rtx
>         wide_int result;
>         enum machine_mode imode = op_mode == VOIDmode ? mode : op_mode;
>         rtx_mode_t op0 = std::make_pair (op, imode);
> +      int int_value;
>   
>   #if TARGET_SUPPORTS_WIDE_INT == 0
>         /* This assert keeps the simplification from producing a result
> @@ -1686,7 +1687,11 @@ simplify_const_unary_operation (enum rtx
>   	  break;
>   
>   	case CLZ:
> -	  result = wi::shwi (wi::clz (op0), mode);
> +	  if (wi::ne_p (op0, 0))
> +	    int_value = wi::clz (op0);
> +	  else if (! CLZ_DEFINED_VALUE_AT_ZERO (mode, int_value))
> +	    int_value = GET_MODE_PRECISION (mode);
> +	  result = wi::shwi (int_value, mode);
>   	  break;
>   
>   	case CLRSB:
> @@ -1694,7 +1699,11 @@ simplify_const_unary_operation (enum rtx
>   	  break;
>   
>   	case CTZ:
> -	  result = wi::shwi (wi::ctz (op0), mode);
> +	  if (wi::ne_p (op0, 0))
> +	    int_value = wi::ctz (op0);
> +	  else if (! CTZ_DEFINED_VALUE_AT_ZERO (mode, int_value))
> +	    int_value = GET_MODE_PRECISION (mode);
> +	  result = wi::shwi (int_value, mode);
>   	  break;
>   
>   	case POPCOUNT:
> Index: gcc/wide-int.cc
> ===================================================================
> --- gcc/wide-int.cc	2014-04-28 16:30:59.941239838 +0100
> +++ gcc/wide-int.cc	2014-04-28 16:31:00.254238990 +0100
> @@ -1137,46 +1137,6 @@ wi::add_large (HOST_WIDE_INT *val, const
>     return canonize (val, len, prec);
>   }
>   
> -/* This is bogus.  We should always return the precision and leave the
> -   caller to handle target dependencies.  */
> -static int
> -clz_zero (unsigned int precision)
> -{
> -  unsigned int count;
> -
> -  enum machine_mode mode = mode_for_size (precision, MODE_INT, 0);
> -  if (mode == BLKmode)
> -    mode_for_size (precision, MODE_PARTIAL_INT, 0);
> -
> -  /* Even if the value at zero is undefined, we have to come up
> -     with some replacement.  Seems good enough.  */
> -  if (mode == BLKmode)
> -    count = precision;
> -  else if (!CLZ_DEFINED_VALUE_AT_ZERO (mode, count))
> -    count = precision;
> -  return count;
> -}
> -
> -/* This is bogus.  We should always return the precision and leave the
> -   caller to handle target dependencies.  */
> -static int
> -ctz_zero (unsigned int precision)
> -{
> -  unsigned int count;
> -
> -  enum machine_mode mode = mode_for_size (precision, MODE_INT, 0);
> -  if (mode == BLKmode)
> -    mode_for_size (precision, MODE_PARTIAL_INT, 0);
> -
> -  /* Even if the value at zero is undefined, we have to come up
> -     with some replacement.  Seems good enough.  */
> -  if (mode == BLKmode)
> -    count = precision;
> -  else if (!CTZ_DEFINED_VALUE_AT_ZERO (mode, count))
> -    count = precision;
> -  return count;
> -}
> -
>   /* Subroutines of the multiplication and division operations.  Unpack
>      the first IN_LEN HOST_WIDE_INTs in INPUT into 2 * IN_LEN
>      HOST_HALF_WIDE_INTs of RESULT.  The rest of RESULT is filled by
> @@ -2002,10 +1962,6 @@ wi::clz (const wide_int_ref &x)
>       /* The upper bit is set, so there are no leading zeros.  */
>       return 0;
>   
> -  /* Check whether the value is zero.  */
> -  if (high == 0 && x.len == 1)
> -    return clz_zero (x.precision);
> -
>     /* We don't need to look below HIGH.  Either HIGH is nonzero,
>        or the top bit of the block below is nonzero; clz_hwi is
>        HOST_BITS_PER_WIDE_INT in the latter case.  */
> @@ -2047,7 +2003,7 @@ int
>   wi::ctz (const wide_int_ref &x)
>   {
>     if (x.len == 1 && x.ulow () == 0)
> -    return ctz_zero (x.precision);
> +    return x.precision;
>   
>     /* Having dealt with the zero case, there must be a block with a
>        nonzero bit.  We don't care about the bits above the first 1.  */
>