Message ID | 87mwf5sbob.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com |
---|---|
State | New |
Headers | show |
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. */ >
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. */