* [wide-int] Handle zero-precision INTEGER_CSTs again
@ 2014-05-02 19:19 Richard Sandiford
2014-05-03 16:42 ` Kenneth Zadeck
2014-05-05 8:31 ` Richard Biener
0 siblings, 2 replies; 15+ messages in thread
From: Richard Sandiford @ 2014-05-02 19:19 UTC (permalink / raw)
To: gcc-patches; +Cc: zadeck, mikestump
I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
Richard's patch to remove min amd max values from zero-width bitfields,
but a boostrap-ubsan showed otherwise. One source is in:
null_pointer_node = build_int_cst (build_pointer_type (void_type_node), 0);
if no_target, since the precision of the type comes from ptr_mode,
which remains the default VOIDmode. Maybe that's a bug, but setting
it to an arbitrary nonzero value would also be dangerous.
The other use is in the C/C++ void_zero_node, which is specifically
defined as a VOID_TYPE, zero-precision INTEGER_CST. This is used by the
ubsan code in some ?: tests, as well as by other places in the frontend
proper. At least the ubsan use persists until gimple.
This patch therefore restores the wide-int handling for zero precision,
for which the length must be 1 and the single HWI must be zero.
I've tried to wrap up most of the dependencies in two new functions,
wi::blocks_needed (a function version of the .cc BLOCKS_NEEDED, now also
used in the header) and wi::excess_bits, so that it'll be easier to
remove the handling again if zero precision ever goes away.
There are some remaining, easily-greppable cases that check directly
for a precision of 0 though.
The combination of this and the other patches allows boostrap-ubsan to
complete. There are a lot of extra testsuite failures compared to a
normal bootstrap, but they don't look related to wide-int.
Tested on x86_64-linux-gnu and powerpc64-linux-gnu. OK to install?
Thanks,
Richard
Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc 2014-05-02 16:28:07.657847935 +0100
+++ gcc/wide-int.cc 2014-05-02 16:28:09.560842845 +0100
@@ -48,8 +48,6 @@ static const HOST_WIDE_INT zeros[WIDE_IN
#define HALF_INT_MASK (((HOST_WIDE_INT) 1 << HOST_BITS_PER_HALF_WIDE_INT) - 1)
#define BLOCK_OF(TARGET) ((TARGET) / HOST_BITS_PER_WIDE_INT)
-#define BLOCKS_NEEDED(PREC) \
- (PREC ? (((PREC) + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT) : 1)
#define SIGN_MASK(X) ((HOST_WIDE_INT) (X) < 0 ? -1 : 0)
/* Return the value a VAL[I] if I < LEN, otherwise, return 0 or -1
@@ -69,20 +67,20 @@ safe_uhwi (const HOST_WIDE_INT *val, uns
static unsigned int
canonize (HOST_WIDE_INT *val, unsigned int len, unsigned int precision)
{
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
HOST_WIDE_INT top;
int i;
if (len > blocks_needed)
len = blocks_needed;
+ if (wi::excess_bits (len, precision) > 0)
+ val[len - 1] = sext_hwi (val[len - 1], precision % HOST_BITS_PER_WIDE_INT);
if (len == 1)
return len;
top = val[len - 1];
- if (len * HOST_BITS_PER_WIDE_INT > precision)
- val[len - 1] = top = sext_hwi (top, precision % HOST_BITS_PER_WIDE_INT);
- if (top != 0 && top != (HOST_WIDE_INT)-1)
+ if (top != 0 && top != (HOST_WIDE_INT) -1)
return len;
/* At this point we know that the top is either 0 or -1. Find the
@@ -134,7 +132,7 @@ wi::from_buffer (const unsigned char *bu
/* We have to clear all the bits ourself, as we merely or in values
below. */
- unsigned int len = BLOCKS_NEEDED (precision);
+ unsigned int len = wi::blocks_needed (precision);
HOST_WIDE_INT *val = result.write_val ();
for (unsigned int i = 0; i < len; ++i)
val[i] = 0;
@@ -180,7 +178,7 @@ wi::to_mpz (const wide_int_ref &x, mpz_t
{
int len = x.get_len ();
const HOST_WIDE_INT *v = x.get_val ();
- int excess = len * HOST_BITS_PER_WIDE_INT - x.get_precision ();
+ unsigned int excess = wi::excess_bits (len, x.get_precision ());
if (wi::neg_p (x, sgn))
{
@@ -306,7 +304,8 @@ wi::force_to_size (HOST_WIDE_INT *val, c
unsigned int xlen, unsigned int xprecision,
unsigned int precision, signop sgn)
{
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
+ unsigned int xblocks_needed = wi::blocks_needed (xprecision);
unsigned int len = blocks_needed < xlen ? blocks_needed : xlen;
for (unsigned i = 0; i < len; i++)
val[i] = xval[i];
@@ -318,11 +317,11 @@ wi::force_to_size (HOST_WIDE_INT *val, c
/* Expanding. */
if (sgn == UNSIGNED)
{
- if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
+ if (small_xprecision && len == xblocks_needed)
val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
else if (val[len - 1] < 0)
{
- while (len < BLOCKS_NEEDED (xprecision))
+ while (len < xblocks_needed)
val[len++] = -1;
if (small_xprecision)
val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
@@ -332,7 +331,7 @@ wi::force_to_size (HOST_WIDE_INT *val, c
}
else
{
- if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
+ if (small_xprecision && len == xblocks_needed)
val[len - 1] = sext_hwi (val[len - 1], small_xprecision);
}
}
@@ -372,10 +371,8 @@ selt (const HOST_WIDE_INT *a, unsigned i
static inline HOST_WIDE_INT
top_bit_of (const HOST_WIDE_INT *a, unsigned int len, unsigned int prec)
{
- int excess = len * HOST_BITS_PER_WIDE_INT - prec;
unsigned HOST_WIDE_INT val = a[len - 1];
- if (excess > 0)
- val <<= excess;
+ val <<= wi::excess_bits (len, prec);
return val >> (HOST_BITS_PER_WIDE_INT - 1);
}
@@ -391,28 +388,16 @@ wi::eq_p_large (const HOST_WIDE_INT *op0
const HOST_WIDE_INT *op1, unsigned int op1len,
unsigned int prec)
{
- int l0 = op0len - 1;
- unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
-
if (op0len != op1len)
return false;
- if (op0len == BLOCKS_NEEDED (prec) && small_prec)
- {
- /* It does not matter if we zext or sext here, we just have to
- do both the same way. */
- if (zext_hwi (op0 [l0], small_prec) != zext_hwi (op1 [l0], small_prec))
- return false;
- l0--;
- }
-
- while (l0 >= 0)
- if (op0[l0] != op1[l0])
+ for (unsigned int i = 0; i < op0len - 1; i++)
+ if (op0[i] != op1[i])
return false;
- else
- l0--;
- return true;
+ unsigned HOST_WIDE_INT top0 = op0[op0len - 1];
+ unsigned HOST_WIDE_INT top1 = op1[op1len - 1];
+ return ((top0 ^ top1) << wi::excess_bits (op0len, prec)) == 0;
}
/* Return true if OP0 < OP1 using signed comparisons. */
@@ -423,7 +408,7 @@ wi::lts_p_large (const HOST_WIDE_INT *op
{
HOST_WIDE_INT s0, s1;
unsigned HOST_WIDE_INT u0, u1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -461,7 +446,7 @@ wi::cmps_large (const HOST_WIDE_INT *op0
{
HOST_WIDE_INT s0, s1;
unsigned HOST_WIDE_INT u0, u1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -498,7 +483,7 @@ wi::ltu_p_large (const HOST_WIDE_INT *op
{
unsigned HOST_WIDE_INT x0;
unsigned HOST_WIDE_INT x1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -525,7 +510,7 @@ wi::cmpu_large (const HOST_WIDE_INT *op0
{
unsigned HOST_WIDE_INT x0;
unsigned HOST_WIDE_INT x1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -673,7 +658,7 @@ wide_int_storage::bswap () const
{
wide_int result = wide_int::create (precision);
unsigned int i, s;
- unsigned int len = BLOCKS_NEEDED (precision);
+ unsigned int len = wi::blocks_needed (precision);
unsigned int xlen = get_len ();
const HOST_WIDE_INT *xval = get_val ();
HOST_WIDE_INT *val = result.write_val ();
@@ -1149,7 +1134,7 @@ wi_unpack (unsigned HOST_HALF_WIDE_INT *
unsigned int i;
unsigned int j = 0;
unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
- unsigned int blocks_needed = BLOCKS_NEEDED (prec);
+ unsigned int blocks_needed = wi::blocks_needed (prec);
HOST_WIDE_INT mask;
if (sgn == SIGNED)
@@ -1222,7 +1207,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
unsigned HOST_WIDE_INT o0, o1, k, t;
unsigned int i;
unsigned int j;
- unsigned int blocks_needed = BLOCKS_NEEDED (prec);
+ unsigned int blocks_needed = wi::blocks_needed (prec);
unsigned int half_blocks_needed = blocks_needed * 2;
/* The sizes here are scaled to support a 2x largest mode by 2x
largest mode yielding a 4x largest mode result. This is what is
@@ -1426,6 +1411,9 @@ wi::popcount (const wide_int_ref &x)
unsigned int i;
int count;
+ if (x.precision == 0)
+ return 0;
+
/* The high order block is special if it is the last block and the
precision is not an even multiple of HOST_BITS_PER_WIDE_INT. We
have to clear out any ones above the precision before doing
@@ -1645,8 +1633,8 @@ wi::divmod_internal (HOST_WIDE_INT *quot
unsigned int divisor_prec, signop sgn,
bool *oflow)
{
- unsigned int dividend_blocks_needed = 2 * BLOCKS_NEEDED (dividend_prec);
- unsigned int divisor_blocks_needed = 2 * BLOCKS_NEEDED (divisor_prec);
+ unsigned int dividend_blocks_needed = 2 * wi::blocks_needed (dividend_prec);
+ unsigned int divisor_blocks_needed = 2 * wi::blocks_needed (divisor_prec);
unsigned HOST_HALF_WIDE_INT
b_quotient[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
unsigned HOST_HALF_WIDE_INT
@@ -1671,7 +1659,7 @@ wi::divmod_internal (HOST_WIDE_INT *quot
/* The smallest signed number / -1 causes overflow. The dividend_len
check is for speed rather than correctness. */
if (sgn == SIGNED
- && dividend_len == BLOCKS_NEEDED (dividend_prec)
+ && dividend_len == wi::blocks_needed (dividend_prec)
&& divisor == -1
&& wi::only_sign_bit_p (dividend))
overflow = true;
@@ -1838,7 +1826,7 @@ wi::lshift_large (HOST_WIDE_INT *val, co
unsigned int small_shift = shift % HOST_BITS_PER_WIDE_INT;
/* The whole-block shift fills with zeros. */
- unsigned int len = BLOCKS_NEEDED (precision);
+ unsigned int len = wi::blocks_needed (precision);
for (unsigned int i = 0; i < skip; ++i)
val[i] = 0;
@@ -1876,7 +1864,7 @@ rshift_large_common (HOST_WIDE_INT *val,
/* Work out how many blocks are needed to store the significant bits
(excluding the upper zeros or signs). */
- unsigned int len = BLOCKS_NEEDED (xprecision - shift);
+ unsigned int len = wi::blocks_needed (xprecision - shift);
/* It's easier to handle the simple block case specially. */
if (small_shift == 0)
@@ -1949,6 +1937,9 @@ wi::arshift_large (HOST_WIDE_INT *val, c
int
wi::clz (const wide_int_ref &x)
{
+ if (x.precision == 0)
+ return 0;
+
/* Calculate how many bits there above the highest represented block. */
int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
@@ -1973,6 +1964,9 @@ wi::clz (const wide_int_ref &x)
int
wi::clrsb (const wide_int_ref &x)
{
+ if (x.precision == 0)
+ return 0;
+
/* Calculate how many bits there above the highest represented block. */
int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
@@ -2017,6 +2011,9 @@ wi::ctz (const wide_int_ref &x)
int
wi::exact_log2 (const wide_int_ref &x)
{
+ if (x.precision == 0)
+ return -1;
+
/* Reject cases where there are implicit -1 blocks above HIGH. */
if (x.len * HOST_BITS_PER_WIDE_INT < x.precision && x.sign_mask () < 0)
return -1;
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h 2014-05-02 16:28:07.657847935 +0100
+++ gcc/wide-int.h 2014-05-02 16:28:09.561842842 +0100
@@ -430,6 +430,9 @@ #define WIDE_INT_REF_FOR(T) \
/* Public functions for querying and operating on integers. */
namespace wi
{
+ unsigned int excess_bits (unsigned int, unsigned int);
+ unsigned int blocks_needed (unsigned int);
+
template <typename T>
unsigned int get_precision (const T &);
@@ -740,7 +743,7 @@ inline generic_wide_int <storage>::gener
inline HOST_WIDE_INT
generic_wide_int <storage>::to_shwi (unsigned int precision) const
{
- if (precision < HOST_BITS_PER_WIDE_INT)
+ if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
return sext_hwi (this->get_val ()[0], precision);
else
return this->get_val ()[0];
@@ -764,7 +767,7 @@ generic_wide_int <storage>::to_shwi () c
inline unsigned HOST_WIDE_INT
generic_wide_int <storage>::to_uhwi (unsigned int precision) const
{
- if (precision < HOST_BITS_PER_WIDE_INT)
+ if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
return zext_hwi (this->get_val ()[0], precision);
else
return this->get_val ()[0];
@@ -797,12 +800,7 @@ generic_wide_int <storage>::sign_mask ()
unsigned int len = this->get_len ();
unsigned HOST_WIDE_INT high = this->get_val ()[len - 1];
if (!is_sign_extended)
- {
- unsigned int precision = this->get_precision ();
- int excess = len * HOST_BITS_PER_WIDE_INT - precision;
- if (excess > 0)
- high <<= excess;
- }
+ high <<= wi::excess_bits (len, this->get_precision ());
return (HOST_WIDE_INT) (high) < 0 ? -1 : 0;
}
@@ -1068,7 +1066,7 @@ wide_int_storage::write_val ()
wide_int_storage::set_len (unsigned int l, bool is_sign_extended)
{
len = l;
- if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > precision)
+ if (!is_sign_extended && wi::excess_bits (len, precision) > 0)
val[len - 1] = sext_hwi (val[len - 1],
precision % HOST_BITS_PER_WIDE_INT);
}
@@ -1347,7 +1345,7 @@ trailing_wide_int_storage::write_val ()
trailing_wide_int_storage::set_len (unsigned int len, bool is_sign_extended)
{
*m_len = len;
- if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > m_precision)
+ if (!is_sign_extended && wi::excess_bits (len, m_precision) > 0)
m_val[len - 1] = sext_hwi (m_val[len - 1],
m_precision % HOST_BITS_PER_WIDE_INT);
}
@@ -1368,8 +1366,7 @@ trailing_wide_int_storage::operator = (c
trailing_wide_ints <N>::set_precision (unsigned int precision)
{
m_precision = precision;
- m_max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
- / HOST_BITS_PER_WIDE_INT);
+ m_max_len = wi::blocks_needed (precision);
}
/* Return a reference to element INDEX. */
@@ -1387,9 +1384,7 @@ trailing_wide_ints <N>::operator [] (uns
inline size_t
trailing_wide_ints <N>::extra_size (unsigned int precision)
{
- unsigned int max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
- / HOST_BITS_PER_WIDE_INT);
- return (N * max_len - 1) * sizeof (HOST_WIDE_INT);
+ return (N * wi::blocks_needed (precision) - 1) * sizeof (HOST_WIDE_INT);
}
/* This macro is used in structures that end with a trailing_wide_ints field
@@ -1621,6 +1616,26 @@ decompose (HOST_WIDE_INT *scratch, unsig
signop, bool *);
}
+/* If a value of length LEN blocks has more than PRECISION bits, return
+ the number of excess bits, otherwise return 0. For the special case
+ of PRECISION being zero, the single HWI must have the value zero and
+ there are no excess bits. Handling zero precision this way means
+ that the result is always a valid shift amount. */
+inline unsigned int
+wi::excess_bits (unsigned int len, unsigned int precision)
+{
+ unsigned int excess = len * HOST_BITS_PER_WIDE_INT - precision;
+ return excess < HOST_BITS_PER_WIDE_INT ? excess : 0;
+}
+
+/* Return the number of blocks needed for precision PRECISION. */
+inline unsigned int
+wi::blocks_needed (unsigned int precision)
+{
+ return precision == 0 ? 1 : ((precision + HOST_BITS_PER_WIDE_INT - 1)
+ / HOST_BITS_PER_WIDE_INT);
+}
+
/* Return the number of bits that integer X can hold. */
template <typename T>
inline unsigned int
@@ -1729,9 +1744,7 @@ wi::eq_p (const T1 &x, const T2 &y)
return xi.val[0] == 0;
/* Otherwise flush out any excess bits first. */
unsigned HOST_WIDE_INT diff = xi.val[0] ^ yi.val[0];
- int excess = HOST_BITS_PER_WIDE_INT - precision;
- if (excess > 0)
- diff <<= excess;
+ diff <<= wi::excess_bits (1, precision);
return diff == 0;
}
return eq_p_large (xi.val, xi.len, yi.val, yi.len, precision);
@@ -2323,7 +2336,9 @@ wi::add (const T1 &x, const T2 &y, signo
unsigned HOST_WIDE_INT xl = xi.ulow ();
unsigned HOST_WIDE_INT yl = yi.ulow ();
unsigned HOST_WIDE_INT resultl = xl + yl;
- if (sgn == SIGNED)
+ if (precision == 0)
+ *overflow = false;
+ else if (sgn == SIGNED)
*overflow = (((resultl ^ xl) & (resultl ^ yl))
>> (precision - 1)) & 1;
else
@@ -2396,7 +2411,9 @@ wi::sub (const T1 &x, const T2 &y, signo
unsigned HOST_WIDE_INT xl = xi.ulow ();
unsigned HOST_WIDE_INT yl = yi.ulow ();
unsigned HOST_WIDE_INT resultl = xl - yl;
- if (sgn == SIGNED)
+ if (precision == 0)
+ *overflow = false;
+ else if (sgn == SIGNED)
*overflow = (((xl ^ yl) & (resultl ^ xl)) >> (precision - 1)) & 1;
else
*overflow = ((resultl << (HOST_BITS_PER_WIDE_INT - precision))
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-02 19:19 [wide-int] Handle zero-precision INTEGER_CSTs again Richard Sandiford
@ 2014-05-03 16:42 ` Kenneth Zadeck
2014-05-03 18:59 ` Richard Sandiford
2014-05-05 8:31 ` Richard Biener
1 sibling, 1 reply; 15+ messages in thread
From: Kenneth Zadeck @ 2014-05-03 16:42 UTC (permalink / raw)
To: gcc-patches, mikestump, rdsandiford
The doc at wide-int.h:136 needs work. The doc currently says that the
precision and length are always greater than 0. This is now not
correct. It also says that the bits above the precision are defined
to be the sign extension if the precision does not cover that block.
I do not know if i believe this completely.
I noticed that in the eq_p_large code, you removed a block of code left
over from the days when this was not true, but there is still so of this
code remaining that does not assume this.
I worry about this because we have moved back and forth on this issue
many times and i still see code at rtl.h:1440 which assumes that BImode
constants are stored differently on some platforms. It is possible that
i am reading that code incorrectly but at first reading it looks
questionable. so code comparing a bimode 1 with a 1 of a different
precision would not work.
aside from that the patch is fine.
kenny
On 05/02/2014 03:19 PM, Richard Sandiford wrote:
> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
> Richard's patch to remove min amd max values from zero-width bitfields,
> but a boostrap-ubsan showed otherwise. One source is in:
>
> null_pointer_node = build_int_cst (build_pointer_type (void_type_node), 0);
>
> if no_target, since the precision of the type comes from ptr_mode,
> which remains the default VOIDmode. Maybe that's a bug, but setting
> it to an arbitrary nonzero value would also be dangerous.
>
> The other use is in the C/C++ void_zero_node, which is specifically
> defined as a VOID_TYPE, zero-precision INTEGER_CST. This is used by the
> ubsan code in some ?: tests, as well as by other places in the frontend
> proper. At least the ubsan use persists until gimple.
>
> This patch therefore restores the wide-int handling for zero precision,
> for which the length must be 1 and the single HWI must be zero.
> I've tried to wrap up most of the dependencies in two new functions,
> wi::blocks_needed (a function version of the .cc BLOCKS_NEEDED, now also
> used in the header) and wi::excess_bits, so that it'll be easier to
> remove the handling again if zero precision ever goes away.
> There are some remaining, easily-greppable cases that check directly
> for a precision of 0 though.
>
> The combination of this and the other patches allows boostrap-ubsan to
> complete. There are a lot of extra testsuite failures compared to a
> normal bootstrap, but they don't look related to wide-int.
>
> Tested on x86_64-linux-gnu and powerpc64-linux-gnu. OK to install?
>
> Thanks,
> Richard
>
>
> Index: gcc/wide-int.cc
> ===================================================================
> --- gcc/wide-int.cc 2014-05-02 16:28:07.657847935 +0100
> +++ gcc/wide-int.cc 2014-05-02 16:28:09.560842845 +0100
> @@ -48,8 +48,6 @@ static const HOST_WIDE_INT zeros[WIDE_IN
> #define HALF_INT_MASK (((HOST_WIDE_INT) 1 << HOST_BITS_PER_HALF_WIDE_INT) - 1)
>
> #define BLOCK_OF(TARGET) ((TARGET) / HOST_BITS_PER_WIDE_INT)
> -#define BLOCKS_NEEDED(PREC) \
> - (PREC ? (((PREC) + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT) : 1)
> #define SIGN_MASK(X) ((HOST_WIDE_INT) (X) < 0 ? -1 : 0)
>
> /* Return the value a VAL[I] if I < LEN, otherwise, return 0 or -1
> @@ -69,20 +67,20 @@ safe_uhwi (const HOST_WIDE_INT *val, uns
> static unsigned int
> canonize (HOST_WIDE_INT *val, unsigned int len, unsigned int precision)
> {
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> HOST_WIDE_INT top;
> int i;
>
> if (len > blocks_needed)
> len = blocks_needed;
>
> + if (wi::excess_bits (len, precision) > 0)
> + val[len - 1] = sext_hwi (val[len - 1], precision % HOST_BITS_PER_WIDE_INT);
> if (len == 1)
> return len;
>
> top = val[len - 1];
> - if (len * HOST_BITS_PER_WIDE_INT > precision)
> - val[len - 1] = top = sext_hwi (top, precision % HOST_BITS_PER_WIDE_INT);
> - if (top != 0 && top != (HOST_WIDE_INT)-1)
> + if (top != 0 && top != (HOST_WIDE_INT) -1)
> return len;
>
> /* At this point we know that the top is either 0 or -1. Find the
> @@ -134,7 +132,7 @@ wi::from_buffer (const unsigned char *bu
>
> /* We have to clear all the bits ourself, as we merely or in values
> below. */
> - unsigned int len = BLOCKS_NEEDED (precision);
> + unsigned int len = wi::blocks_needed (precision);
> HOST_WIDE_INT *val = result.write_val ();
> for (unsigned int i = 0; i < len; ++i)
> val[i] = 0;
> @@ -180,7 +178,7 @@ wi::to_mpz (const wide_int_ref &x, mpz_t
> {
> int len = x.get_len ();
> const HOST_WIDE_INT *v = x.get_val ();
> - int excess = len * HOST_BITS_PER_WIDE_INT - x.get_precision ();
> + unsigned int excess = wi::excess_bits (len, x.get_precision ());
>
> if (wi::neg_p (x, sgn))
> {
> @@ -306,7 +304,8 @@ wi::force_to_size (HOST_WIDE_INT *val, c
> unsigned int xlen, unsigned int xprecision,
> unsigned int precision, signop sgn)
> {
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> + unsigned int xblocks_needed = wi::blocks_needed (xprecision);
> unsigned int len = blocks_needed < xlen ? blocks_needed : xlen;
> for (unsigned i = 0; i < len; i++)
> val[i] = xval[i];
> @@ -318,11 +317,11 @@ wi::force_to_size (HOST_WIDE_INT *val, c
> /* Expanding. */
> if (sgn == UNSIGNED)
> {
> - if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
> + if (small_xprecision && len == xblocks_needed)
> val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
> else if (val[len - 1] < 0)
> {
> - while (len < BLOCKS_NEEDED (xprecision))
> + while (len < xblocks_needed)
> val[len++] = -1;
> if (small_xprecision)
> val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
> @@ -332,7 +331,7 @@ wi::force_to_size (HOST_WIDE_INT *val, c
> }
> else
> {
> - if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
> + if (small_xprecision && len == xblocks_needed)
> val[len - 1] = sext_hwi (val[len - 1], small_xprecision);
> }
> }
> @@ -372,10 +371,8 @@ selt (const HOST_WIDE_INT *a, unsigned i
> static inline HOST_WIDE_INT
> top_bit_of (const HOST_WIDE_INT *a, unsigned int len, unsigned int prec)
> {
> - int excess = len * HOST_BITS_PER_WIDE_INT - prec;
> unsigned HOST_WIDE_INT val = a[len - 1];
> - if (excess > 0)
> - val <<= excess;
> + val <<= wi::excess_bits (len, prec);
> return val >> (HOST_BITS_PER_WIDE_INT - 1);
> }
>
> @@ -391,28 +388,16 @@ wi::eq_p_large (const HOST_WIDE_INT *op0
> const HOST_WIDE_INT *op1, unsigned int op1len,
> unsigned int prec)
> {
> - int l0 = op0len - 1;
> - unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
> -
> if (op0len != op1len)
> return false;
>
> - if (op0len == BLOCKS_NEEDED (prec) && small_prec)
> - {
> - /* It does not matter if we zext or sext here, we just have to
> - do both the same way. */
> - if (zext_hwi (op0 [l0], small_prec) != zext_hwi (op1 [l0], small_prec))
> - return false;
> - l0--;
> - }
> -
> - while (l0 >= 0)
> - if (op0[l0] != op1[l0])
> + for (unsigned int i = 0; i < op0len - 1; i++)
> + if (op0[i] != op1[i])
> return false;
> - else
> - l0--;
>
> - return true;
> + unsigned HOST_WIDE_INT top0 = op0[op0len - 1];
> + unsigned HOST_WIDE_INT top1 = op1[op1len - 1];
> + return ((top0 ^ top1) << wi::excess_bits (op0len, prec)) == 0;
> }
>
> /* Return true if OP0 < OP1 using signed comparisons. */
> @@ -423,7 +408,7 @@ wi::lts_p_large (const HOST_WIDE_INT *op
> {
> HOST_WIDE_INT s0, s1;
> unsigned HOST_WIDE_INT u0, u1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -461,7 +446,7 @@ wi::cmps_large (const HOST_WIDE_INT *op0
> {
> HOST_WIDE_INT s0, s1;
> unsigned HOST_WIDE_INT u0, u1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -498,7 +483,7 @@ wi::ltu_p_large (const HOST_WIDE_INT *op
> {
> unsigned HOST_WIDE_INT x0;
> unsigned HOST_WIDE_INT x1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -525,7 +510,7 @@ wi::cmpu_large (const HOST_WIDE_INT *op0
> {
> unsigned HOST_WIDE_INT x0;
> unsigned HOST_WIDE_INT x1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -673,7 +658,7 @@ wide_int_storage::bswap () const
> {
> wide_int result = wide_int::create (precision);
> unsigned int i, s;
> - unsigned int len = BLOCKS_NEEDED (precision);
> + unsigned int len = wi::blocks_needed (precision);
> unsigned int xlen = get_len ();
> const HOST_WIDE_INT *xval = get_val ();
> HOST_WIDE_INT *val = result.write_val ();
> @@ -1149,7 +1134,7 @@ wi_unpack (unsigned HOST_HALF_WIDE_INT *
> unsigned int i;
> unsigned int j = 0;
> unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
> - unsigned int blocks_needed = BLOCKS_NEEDED (prec);
> + unsigned int blocks_needed = wi::blocks_needed (prec);
> HOST_WIDE_INT mask;
>
> if (sgn == SIGNED)
> @@ -1222,7 +1207,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
> unsigned HOST_WIDE_INT o0, o1, k, t;
> unsigned int i;
> unsigned int j;
> - unsigned int blocks_needed = BLOCKS_NEEDED (prec);
> + unsigned int blocks_needed = wi::blocks_needed (prec);
> unsigned int half_blocks_needed = blocks_needed * 2;
> /* The sizes here are scaled to support a 2x largest mode by 2x
> largest mode yielding a 4x largest mode result. This is what is
> @@ -1426,6 +1411,9 @@ wi::popcount (const wide_int_ref &x)
> unsigned int i;
> int count;
>
> + if (x.precision == 0)
> + return 0;
> +
> /* The high order block is special if it is the last block and the
> precision is not an even multiple of HOST_BITS_PER_WIDE_INT. We
> have to clear out any ones above the precision before doing
> @@ -1645,8 +1633,8 @@ wi::divmod_internal (HOST_WIDE_INT *quot
> unsigned int divisor_prec, signop sgn,
> bool *oflow)
> {
> - unsigned int dividend_blocks_needed = 2 * BLOCKS_NEEDED (dividend_prec);
> - unsigned int divisor_blocks_needed = 2 * BLOCKS_NEEDED (divisor_prec);
> + unsigned int dividend_blocks_needed = 2 * wi::blocks_needed (dividend_prec);
> + unsigned int divisor_blocks_needed = 2 * wi::blocks_needed (divisor_prec);
> unsigned HOST_HALF_WIDE_INT
> b_quotient[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
> unsigned HOST_HALF_WIDE_INT
> @@ -1671,7 +1659,7 @@ wi::divmod_internal (HOST_WIDE_INT *quot
> /* The smallest signed number / -1 causes overflow. The dividend_len
> check is for speed rather than correctness. */
> if (sgn == SIGNED
> - && dividend_len == BLOCKS_NEEDED (dividend_prec)
> + && dividend_len == wi::blocks_needed (dividend_prec)
> && divisor == -1
> && wi::only_sign_bit_p (dividend))
> overflow = true;
> @@ -1838,7 +1826,7 @@ wi::lshift_large (HOST_WIDE_INT *val, co
> unsigned int small_shift = shift % HOST_BITS_PER_WIDE_INT;
>
> /* The whole-block shift fills with zeros. */
> - unsigned int len = BLOCKS_NEEDED (precision);
> + unsigned int len = wi::blocks_needed (precision);
> for (unsigned int i = 0; i < skip; ++i)
> val[i] = 0;
>
> @@ -1876,7 +1864,7 @@ rshift_large_common (HOST_WIDE_INT *val,
>
> /* Work out how many blocks are needed to store the significant bits
> (excluding the upper zeros or signs). */
> - unsigned int len = BLOCKS_NEEDED (xprecision - shift);
> + unsigned int len = wi::blocks_needed (xprecision - shift);
>
> /* It's easier to handle the simple block case specially. */
> if (small_shift == 0)
> @@ -1949,6 +1937,9 @@ wi::arshift_large (HOST_WIDE_INT *val, c
> int
> wi::clz (const wide_int_ref &x)
> {
> + if (x.precision == 0)
> + return 0;
> +
> /* Calculate how many bits there above the highest represented block. */
> int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
>
> @@ -1973,6 +1964,9 @@ wi::clz (const wide_int_ref &x)
> int
> wi::clrsb (const wide_int_ref &x)
> {
> + if (x.precision == 0)
> + return 0;
> +
> /* Calculate how many bits there above the highest represented block. */
> int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
>
> @@ -2017,6 +2011,9 @@ wi::ctz (const wide_int_ref &x)
> int
> wi::exact_log2 (const wide_int_ref &x)
> {
> + if (x.precision == 0)
> + return -1;
> +
> /* Reject cases where there are implicit -1 blocks above HIGH. */
> if (x.len * HOST_BITS_PER_WIDE_INT < x.precision && x.sign_mask () < 0)
> return -1;
> Index: gcc/wide-int.h
> ===================================================================
> --- gcc/wide-int.h 2014-05-02 16:28:07.657847935 +0100
> +++ gcc/wide-int.h 2014-05-02 16:28:09.561842842 +0100
> @@ -430,6 +430,9 @@ #define WIDE_INT_REF_FOR(T) \
> /* Public functions for querying and operating on integers. */
> namespace wi
> {
> + unsigned int excess_bits (unsigned int, unsigned int);
> + unsigned int blocks_needed (unsigned int);
> +
> template <typename T>
> unsigned int get_precision (const T &);
>
> @@ -740,7 +743,7 @@ inline generic_wide_int <storage>::gener
> inline HOST_WIDE_INT
> generic_wide_int <storage>::to_shwi (unsigned int precision) const
> {
> - if (precision < HOST_BITS_PER_WIDE_INT)
> + if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
> return sext_hwi (this->get_val ()[0], precision);
> else
> return this->get_val ()[0];
> @@ -764,7 +767,7 @@ generic_wide_int <storage>::to_shwi () c
> inline unsigned HOST_WIDE_INT
> generic_wide_int <storage>::to_uhwi (unsigned int precision) const
> {
> - if (precision < HOST_BITS_PER_WIDE_INT)
> + if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
> return zext_hwi (this->get_val ()[0], precision);
> else
> return this->get_val ()[0];
> @@ -797,12 +800,7 @@ generic_wide_int <storage>::sign_mask ()
> unsigned int len = this->get_len ();
> unsigned HOST_WIDE_INT high = this->get_val ()[len - 1];
> if (!is_sign_extended)
> - {
> - unsigned int precision = this->get_precision ();
> - int excess = len * HOST_BITS_PER_WIDE_INT - precision;
> - if (excess > 0)
> - high <<= excess;
> - }
> + high <<= wi::excess_bits (len, this->get_precision ());
> return (HOST_WIDE_INT) (high) < 0 ? -1 : 0;
> }
>
> @@ -1068,7 +1066,7 @@ wide_int_storage::write_val ()
> wide_int_storage::set_len (unsigned int l, bool is_sign_extended)
> {
> len = l;
> - if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > precision)
> + if (!is_sign_extended && wi::excess_bits (len, precision) > 0)
> val[len - 1] = sext_hwi (val[len - 1],
> precision % HOST_BITS_PER_WIDE_INT);
> }
> @@ -1347,7 +1345,7 @@ trailing_wide_int_storage::write_val ()
> trailing_wide_int_storage::set_len (unsigned int len, bool is_sign_extended)
> {
> *m_len = len;
> - if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > m_precision)
> + if (!is_sign_extended && wi::excess_bits (len, m_precision) > 0)
> m_val[len - 1] = sext_hwi (m_val[len - 1],
> m_precision % HOST_BITS_PER_WIDE_INT);
> }
> @@ -1368,8 +1366,7 @@ trailing_wide_int_storage::operator = (c
> trailing_wide_ints <N>::set_precision (unsigned int precision)
> {
> m_precision = precision;
> - m_max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
> - / HOST_BITS_PER_WIDE_INT);
> + m_max_len = wi::blocks_needed (precision);
> }
>
> /* Return a reference to element INDEX. */
> @@ -1387,9 +1384,7 @@ trailing_wide_ints <N>::operator [] (uns
> inline size_t
> trailing_wide_ints <N>::extra_size (unsigned int precision)
> {
> - unsigned int max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
> - / HOST_BITS_PER_WIDE_INT);
> - return (N * max_len - 1) * sizeof (HOST_WIDE_INT);
> + return (N * wi::blocks_needed (precision) - 1) * sizeof (HOST_WIDE_INT);
> }
>
> /* This macro is used in structures that end with a trailing_wide_ints field
> @@ -1621,6 +1616,26 @@ decompose (HOST_WIDE_INT *scratch, unsig
> signop, bool *);
> }
>
> +/* If a value of length LEN blocks has more than PRECISION bits, return
> + the number of excess bits, otherwise return 0. For the special case
> + of PRECISION being zero, the single HWI must have the value zero and
> + there are no excess bits. Handling zero precision this way means
> + that the result is always a valid shift amount. */
> +inline unsigned int
> +wi::excess_bits (unsigned int len, unsigned int precision)
> +{
> + unsigned int excess = len * HOST_BITS_PER_WIDE_INT - precision;
> + return excess < HOST_BITS_PER_WIDE_INT ? excess : 0;
> +}
> +
> +/* Return the number of blocks needed for precision PRECISION. */
> +inline unsigned int
> +wi::blocks_needed (unsigned int precision)
> +{
> + return precision == 0 ? 1 : ((precision + HOST_BITS_PER_WIDE_INT - 1)
> + / HOST_BITS_PER_WIDE_INT);
> +}
> +
> /* Return the number of bits that integer X can hold. */
> template <typename T>
> inline unsigned int
> @@ -1729,9 +1744,7 @@ wi::eq_p (const T1 &x, const T2 &y)
> return xi.val[0] == 0;
> /* Otherwise flush out any excess bits first. */
> unsigned HOST_WIDE_INT diff = xi.val[0] ^ yi.val[0];
> - int excess = HOST_BITS_PER_WIDE_INT - precision;
> - if (excess > 0)
> - diff <<= excess;
> + diff <<= wi::excess_bits (1, precision);
> return diff == 0;
> }
> return eq_p_large (xi.val, xi.len, yi.val, yi.len, precision);
> @@ -2323,7 +2336,9 @@ wi::add (const T1 &x, const T2 &y, signo
> unsigned HOST_WIDE_INT xl = xi.ulow ();
> unsigned HOST_WIDE_INT yl = yi.ulow ();
> unsigned HOST_WIDE_INT resultl = xl + yl;
> - if (sgn == SIGNED)
> + if (precision == 0)
> + *overflow = false;
> + else if (sgn == SIGNED)
> *overflow = (((resultl ^ xl) & (resultl ^ yl))
> >> (precision - 1)) & 1;
> else
> @@ -2396,7 +2411,9 @@ wi::sub (const T1 &x, const T2 &y, signo
> unsigned HOST_WIDE_INT xl = xi.ulow ();
> unsigned HOST_WIDE_INT yl = yi.ulow ();
> unsigned HOST_WIDE_INT resultl = xl - yl;
> - if (sgn == SIGNED)
> + if (precision == 0)
> + *overflow = false;
> + else if (sgn == SIGNED)
> *overflow = (((xl ^ yl) & (resultl ^ xl)) >> (precision - 1)) & 1;
> else
> *overflow = ((resultl << (HOST_BITS_PER_WIDE_INT - precision))
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-03 16:42 ` Kenneth Zadeck
@ 2014-05-03 18:59 ` Richard Sandiford
2014-05-03 19:53 ` Kenneth Zadeck
0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2014-05-03 18:59 UTC (permalink / raw)
To: Kenneth Zadeck; +Cc: gcc-patches, mikestump
Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> The doc at wide-int.h:136 needs work. The doc currently says that the
> precision and length are always greater than 0. This is now not
> correct. It also says that the bits above the precision are defined
> to be the sign extension if the precision does not cover that block.
I agree that the comment needs work.
Reordering slightly:
> I do not know if i believe this completely.
This is now covered by the compile-time is_sign_extended property.
Trees aren't extended in that way because we want to be able to access
them quickly in wider precisions, with the extension following the
signedness of the underlying type. So an 8-bit all-ones INTEGER_CST
will be stored as -1 if the type is signed and as 0xff otherwise.
RTL constants aren't sign-extended because of the case you note:
> I worry about this because we have moved back and forth on this issue
> many times and i still see code at rtl.h:1440 which assumes that BImode
> constants are stored differently on some platforms. It is possible that
> i am reading that code incorrectly but at first reading it looks
> questionable. so code comparing a bimode 1 with a 1 of a different
> precision would not work.
I hope to get rid of the special BImode case after the merge and set
rtl's is_sign_extended back to true.
wide_int itself is always sign-extended. The situation never occurs
for offset_int and widest_int.
> I noticed that in the eq_p_large code, you removed a block of code left
> over from the days when this was not true, but there is still so of this
> code remaining that does not assume this.
The *_large functions have to be conservative and assume that the inputs
are not sign-extended. My eq_p_large change doesn't change that,
it just represents it differently. The idea is that rather than
extending each HWI individually from small_prec and then comparing them,
we can shift the XOR of the two values left by the number of excess bits,
so that all undefined bits disappear from the value. The shifted-in bits
are all zeros so the shifted XOR value will be zero iff the meaningful
bits are the same and nonzero iff the meaningful bits are different,
regardless of whether the input is sign-extended or not. The same
technique is already used in the inline version.
Thanks,
Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-03 18:59 ` Richard Sandiford
@ 2014-05-03 19:53 ` Kenneth Zadeck
2014-05-04 19:56 ` Richard Sandiford
0 siblings, 1 reply; 15+ messages in thread
From: Kenneth Zadeck @ 2014-05-03 19:53 UTC (permalink / raw)
To: gcc-patches, mikestump, rdsandiford
Then with a fixed comment, this patch is fine.
kenny
On 05/03/2014 02:59 PM, Richard Sandiford wrote:
> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>> The doc at wide-int.h:136 needs work. The doc currently says that the
>> precision and length are always greater than 0. This is now not
>> correct. It also says that the bits above the precision are defined
>> to be the sign extension if the precision does not cover that block.
> I agree that the comment needs work.
>
> Reordering slightly:
>
>> I do not know if i believe this completely.
> This is now covered by the compile-time is_sign_extended property.
>
> Trees aren't extended in that way because we want to be able to access
> them quickly in wider precisions, with the extension following the
> signedness of the underlying type. So an 8-bit all-ones INTEGER_CST
> will be stored as -1 if the type is signed and as 0xff otherwise.
>
> RTL constants aren't sign-extended because of the case you note:
>
>> I worry about this because we have moved back and forth on this issue
>> many times and i still see code at rtl.h:1440 which assumes that BImode
>> constants are stored differently on some platforms. It is possible that
>> i am reading that code incorrectly but at first reading it looks
>> questionable. so code comparing a bimode 1 with a 1 of a different
>> precision would not work.
> I hope to get rid of the special BImode case after the merge and set
> rtl's is_sign_extended back to true.
>
> wide_int itself is always sign-extended. The situation never occurs
> for offset_int and widest_int.
>
>> I noticed that in the eq_p_large code, you removed a block of code left
>> over from the days when this was not true, but there is still so of this
>> code remaining that does not assume this.
> The *_large functions have to be conservative and assume that the inputs
> are not sign-extended. My eq_p_large change doesn't change that,
> it just represents it differently. The idea is that rather than
> extending each HWI individually from small_prec and then comparing them,
> we can shift the XOR of the two values left by the number of excess bits,
> so that all undefined bits disappear from the value. The shifted-in bits
> are all zeros so the shifted XOR value will be zero iff the meaningful
> bits are the same and nonzero iff the meaningful bits are different,
> regardless of whether the input is sign-extended or not. The same
> technique is already used in the inline version.
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-03 19:53 ` Kenneth Zadeck
@ 2014-05-04 19:56 ` Richard Sandiford
0 siblings, 0 replies; 15+ messages in thread
From: Richard Sandiford @ 2014-05-04 19:56 UTC (permalink / raw)
To: Kenneth Zadeck; +Cc: gcc-patches, mikestump
Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> Then with a fixed comment, this patch is fine.
OK, here's what I committed.
Richard
Index: gcc/wide-int.cc
===================================================================
--- gcc/wide-int.cc 2014-05-03 07:59:36.274750108 +0100
+++ gcc/wide-int.cc 2014-05-04 20:48:44.939829293 +0100
@@ -48,8 +48,6 @@ static const HOST_WIDE_INT zeros[WIDE_IN
#define HALF_INT_MASK (((HOST_WIDE_INT) 1 << HOST_BITS_PER_HALF_WIDE_INT) - 1)
#define BLOCK_OF(TARGET) ((TARGET) / HOST_BITS_PER_WIDE_INT)
-#define BLOCKS_NEEDED(PREC) \
- (PREC ? (((PREC) + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT) : 1)
#define SIGN_MASK(X) ((HOST_WIDE_INT) (X) < 0 ? -1 : 0)
/* Return the value a VAL[I] if I < LEN, otherwise, return 0 or -1
@@ -69,20 +67,20 @@ safe_uhwi (const HOST_WIDE_INT *val, uns
static unsigned int
canonize (HOST_WIDE_INT *val, unsigned int len, unsigned int precision)
{
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
HOST_WIDE_INT top;
int i;
if (len > blocks_needed)
len = blocks_needed;
+ if (wi::excess_bits (len, precision) > 0)
+ val[len - 1] = sext_hwi (val[len - 1], precision % HOST_BITS_PER_WIDE_INT);
if (len == 1)
return len;
top = val[len - 1];
- if (len * HOST_BITS_PER_WIDE_INT > precision)
- val[len - 1] = top = sext_hwi (top, precision % HOST_BITS_PER_WIDE_INT);
- if (top != 0 && top != (HOST_WIDE_INT)-1)
+ if (top != 0 && top != (HOST_WIDE_INT) -1)
return len;
/* At this point we know that the top is either 0 or -1. Find the
@@ -134,7 +132,7 @@ wi::from_buffer (const unsigned char *bu
/* We have to clear all the bits ourself, as we merely or in values
below. */
- unsigned int len = BLOCKS_NEEDED (precision);
+ unsigned int len = wi::blocks_needed (precision);
HOST_WIDE_INT *val = result.write_val ();
for (unsigned int i = 0; i < len; ++i)
val[i] = 0;
@@ -180,7 +178,7 @@ wi::to_mpz (const wide_int_ref &x, mpz_t
{
int len = x.get_len ();
const HOST_WIDE_INT *v = x.get_val ();
- int excess = len * HOST_BITS_PER_WIDE_INT - x.get_precision ();
+ unsigned int excess = wi::excess_bits (len, x.get_precision ());
if (wi::neg_p (x, sgn))
{
@@ -306,7 +304,8 @@ wi::force_to_size (HOST_WIDE_INT *val, c
unsigned int xlen, unsigned int xprecision,
unsigned int precision, signop sgn)
{
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
+ unsigned int xblocks_needed = wi::blocks_needed (xprecision);
unsigned int len = blocks_needed < xlen ? blocks_needed : xlen;
for (unsigned i = 0; i < len; i++)
val[i] = xval[i];
@@ -318,11 +317,11 @@ wi::force_to_size (HOST_WIDE_INT *val, c
/* Expanding. */
if (sgn == UNSIGNED)
{
- if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
+ if (small_xprecision && len == xblocks_needed)
val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
else if (val[len - 1] < 0)
{
- while (len < BLOCKS_NEEDED (xprecision))
+ while (len < xblocks_needed)
val[len++] = -1;
if (small_xprecision)
val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
@@ -332,7 +331,7 @@ wi::force_to_size (HOST_WIDE_INT *val, c
}
else
{
- if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
+ if (small_xprecision && len == xblocks_needed)
val[len - 1] = sext_hwi (val[len - 1], small_xprecision);
}
}
@@ -372,10 +371,8 @@ selt (const HOST_WIDE_INT *a, unsigned i
static inline HOST_WIDE_INT
top_bit_of (const HOST_WIDE_INT *a, unsigned int len, unsigned int prec)
{
- int excess = len * HOST_BITS_PER_WIDE_INT - prec;
unsigned HOST_WIDE_INT val = a[len - 1];
- if (excess > 0)
- val <<= excess;
+ val <<= wi::excess_bits (len, prec);
return val >> (HOST_BITS_PER_WIDE_INT - 1);
}
@@ -391,28 +388,16 @@ wi::eq_p_large (const HOST_WIDE_INT *op0
const HOST_WIDE_INT *op1, unsigned int op1len,
unsigned int prec)
{
- int l0 = op0len - 1;
- unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
-
if (op0len != op1len)
return false;
- if (op0len == BLOCKS_NEEDED (prec) && small_prec)
- {
- /* It does not matter if we zext or sext here, we just have to
- do both the same way. */
- if (zext_hwi (op0 [l0], small_prec) != zext_hwi (op1 [l0], small_prec))
- return false;
- l0--;
- }
-
- while (l0 >= 0)
- if (op0[l0] != op1[l0])
+ for (unsigned int i = 0; i < op0len - 1; i++)
+ if (op0[i] != op1[i])
return false;
- else
- l0--;
- return true;
+ unsigned HOST_WIDE_INT top0 = op0[op0len - 1];
+ unsigned HOST_WIDE_INT top1 = op1[op1len - 1];
+ return ((top0 ^ top1) << wi::excess_bits (op0len, prec)) == 0;
}
/* Return true if OP0 < OP1 using signed comparisons. */
@@ -423,7 +408,7 @@ wi::lts_p_large (const HOST_WIDE_INT *op
{
HOST_WIDE_INT s0, s1;
unsigned HOST_WIDE_INT u0, u1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -461,7 +446,7 @@ wi::cmps_large (const HOST_WIDE_INT *op0
{
HOST_WIDE_INT s0, s1;
unsigned HOST_WIDE_INT u0, u1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -498,7 +483,7 @@ wi::ltu_p_large (const HOST_WIDE_INT *op
{
unsigned HOST_WIDE_INT x0;
unsigned HOST_WIDE_INT x1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -525,7 +510,7 @@ wi::cmpu_large (const HOST_WIDE_INT *op0
{
unsigned HOST_WIDE_INT x0;
unsigned HOST_WIDE_INT x1;
- unsigned int blocks_needed = BLOCKS_NEEDED (precision);
+ unsigned int blocks_needed = wi::blocks_needed (precision);
unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
int l = MAX (op0len - 1, op1len - 1);
@@ -673,7 +658,7 @@ wide_int_storage::bswap () const
{
wide_int result = wide_int::create (precision);
unsigned int i, s;
- unsigned int len = BLOCKS_NEEDED (precision);
+ unsigned int len = wi::blocks_needed (precision);
unsigned int xlen = get_len ();
const HOST_WIDE_INT *xval = get_val ();
HOST_WIDE_INT *val = result.write_val ();
@@ -1148,7 +1133,7 @@ wi_unpack (unsigned HOST_HALF_WIDE_INT *
unsigned int i;
unsigned int j = 0;
unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
- unsigned int blocks_needed = BLOCKS_NEEDED (prec);
+ unsigned int blocks_needed = wi::blocks_needed (prec);
HOST_WIDE_INT mask;
if (sgn == SIGNED)
@@ -1225,7 +1210,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
unsigned HOST_WIDE_INT o0, o1, k, t;
unsigned int i;
unsigned int j;
- unsigned int blocks_needed = BLOCKS_NEEDED (prec);
+ unsigned int blocks_needed = wi::blocks_needed (prec);
unsigned int half_blocks_needed = blocks_needed * 2;
/* The sizes here are scaled to support a 2x largest mode by 2x
largest mode yielding a 4x largest mode result. This is what is
@@ -1427,6 +1412,9 @@ wi::popcount (const wide_int_ref &x)
unsigned int i;
int count;
+ if (x.precision == 0)
+ return 0;
+
/* The high order block is special if it is the last block and the
precision is not an even multiple of HOST_BITS_PER_WIDE_INT. We
have to clear out any ones above the precision before doing
@@ -1647,8 +1635,8 @@ wi::divmod_internal (HOST_WIDE_INT *quot
unsigned int divisor_prec, signop sgn,
bool *oflow)
{
- unsigned int dividend_blocks_needed = 2 * BLOCKS_NEEDED (dividend_prec);
- unsigned int divisor_blocks_needed = 2 * BLOCKS_NEEDED (divisor_prec);
+ unsigned int dividend_blocks_needed = 2 * wi::blocks_needed (dividend_prec);
+ unsigned int divisor_blocks_needed = 2 * wi::blocks_needed (divisor_prec);
unsigned HOST_HALF_WIDE_INT
b_quotient[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
unsigned HOST_HALF_WIDE_INT
@@ -1673,7 +1661,7 @@ wi::divmod_internal (HOST_WIDE_INT *quot
/* The smallest signed number / -1 causes overflow. The dividend_len
check is for speed rather than correctness. */
if (sgn == SIGNED
- && dividend_len == BLOCKS_NEEDED (dividend_prec)
+ && dividend_len == wi::blocks_needed (dividend_prec)
&& divisor == -1
&& wi::only_sign_bit_p (dividend))
overflow = true;
@@ -1831,7 +1819,7 @@ wi::lshift_large (HOST_WIDE_INT *val, co
unsigned int small_shift = shift % HOST_BITS_PER_WIDE_INT;
/* The whole-block shift fills with zeros. */
- unsigned int len = BLOCKS_NEEDED (precision);
+ unsigned int len = wi::blocks_needed (precision);
for (unsigned int i = 0; i < skip; ++i)
val[i] = 0;
@@ -1869,7 +1857,7 @@ rshift_large_common (HOST_WIDE_INT *val,
/* Work out how many blocks are needed to store the significant bits
(excluding the upper zeros or signs). */
- unsigned int len = BLOCKS_NEEDED (xprecision - shift);
+ unsigned int len = wi::blocks_needed (xprecision - shift);
/* It's easier to handle the simple block case specially. */
if (small_shift == 0)
@@ -1942,6 +1930,9 @@ wi::arshift_large (HOST_WIDE_INT *val, c
int
wi::clz (const wide_int_ref &x)
{
+ if (x.precision == 0)
+ return 0;
+
/* Calculate how many bits there above the highest represented block. */
int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
@@ -1966,6 +1957,9 @@ wi::clz (const wide_int_ref &x)
int
wi::clrsb (const wide_int_ref &x)
{
+ if (x.precision == 0)
+ return 0;
+
/* Calculate how many bits there above the highest represented block. */
int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
@@ -2010,6 +2004,9 @@ wi::ctz (const wide_int_ref &x)
int
wi::exact_log2 (const wide_int_ref &x)
{
+ if (x.precision == 0)
+ return -1;
+
/* Reject cases where there are implicit -1 blocks above HIGH. */
if (x.len * HOST_BITS_PER_WIDE_INT < x.precision && x.sign_mask () < 0)
return -1;
Index: gcc/wide-int.h
===================================================================
--- gcc/wide-int.h 2014-05-03 07:58:04.005175780 +0100
+++ gcc/wide-int.h 2014-05-04 20:53:28.131085933 +0100
@@ -127,13 +127,16 @@ #define WIDE_INT_H
Since most integers used in a compiler are small values, it is
generally profitable to use a representation of the value that is
as small as possible. LEN is used to indicate the number of
- elements of the vector that are in use. The numbers are stored as
- sign extended numbers as a means of compression. Leading
- HOST_WIDE_INTs that contain strings of either -1 or 0 are removed
- as long as they can be reconstructed from the top bit that is being
- represented.
+ elements of the vector that are in use, which is always at least one.
+ The numbers are stored as sign extended numbers as a means of
+ compression. Leading HOST_WIDE_INTs that contain strings of either
+ -1 or 0 are removed as long as they can be reconstructed from the top
+ bit that is being represented.
+
+ wide_ints can have a precision of 0. This is needed to handle things
+ like void_zero_node in the C and C++ front ends. In this case the
+ stored value is always 0 and the length is always 1.
- The precision and length of a wide_int are always greater than 0.
Any bits in a wide_int above the precision are sign-extended from the
most significant bit. For example, a 4-bit value 0x8 is represented as
VAL = { 0xf...fff8 }. However, as an optimization, we allow other integer
@@ -430,6 +433,9 @@ #define WIDE_INT_REF_FOR(T) \
/* Public functions for querying and operating on integers. */
namespace wi
{
+ unsigned int excess_bits (unsigned int, unsigned int);
+ unsigned int blocks_needed (unsigned int);
+
template <typename T>
unsigned int get_precision (const T &);
@@ -740,7 +746,7 @@ inline generic_wide_int <storage>::gener
inline HOST_WIDE_INT
generic_wide_int <storage>::to_shwi (unsigned int precision) const
{
- if (precision < HOST_BITS_PER_WIDE_INT)
+ if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
return sext_hwi (this->get_val ()[0], precision);
else
return this->get_val ()[0];
@@ -764,7 +770,7 @@ generic_wide_int <storage>::to_shwi () c
inline unsigned HOST_WIDE_INT
generic_wide_int <storage>::to_uhwi (unsigned int precision) const
{
- if (precision < HOST_BITS_PER_WIDE_INT)
+ if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
return zext_hwi (this->get_val ()[0], precision);
else
return this->get_val ()[0];
@@ -797,12 +803,7 @@ generic_wide_int <storage>::sign_mask ()
unsigned int len = this->get_len ();
unsigned HOST_WIDE_INT high = this->get_val ()[len - 1];
if (!is_sign_extended)
- {
- unsigned int precision = this->get_precision ();
- int excess = len * HOST_BITS_PER_WIDE_INT - precision;
- if (excess > 0)
- high <<= excess;
- }
+ high <<= wi::excess_bits (len, this->get_precision ());
return (HOST_WIDE_INT) (high) < 0 ? -1 : 0;
}
@@ -1068,7 +1069,7 @@ wide_int_storage::write_val ()
wide_int_storage::set_len (unsigned int l, bool is_sign_extended)
{
len = l;
- if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > precision)
+ if (!is_sign_extended && wi::excess_bits (len, precision) > 0)
val[len - 1] = sext_hwi (val[len - 1],
precision % HOST_BITS_PER_WIDE_INT);
}
@@ -1347,7 +1348,7 @@ trailing_wide_int_storage::write_val ()
trailing_wide_int_storage::set_len (unsigned int len, bool is_sign_extended)
{
*m_len = len;
- if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > m_precision)
+ if (!is_sign_extended && wi::excess_bits (len, m_precision) > 0)
m_val[len - 1] = sext_hwi (m_val[len - 1],
m_precision % HOST_BITS_PER_WIDE_INT);
}
@@ -1368,8 +1369,7 @@ trailing_wide_int_storage::operator = (c
trailing_wide_ints <N>::set_precision (unsigned int precision)
{
m_precision = precision;
- m_max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
- / HOST_BITS_PER_WIDE_INT);
+ m_max_len = wi::blocks_needed (precision);
}
/* Return a reference to element INDEX. */
@@ -1387,9 +1387,7 @@ trailing_wide_ints <N>::operator [] (uns
inline size_t
trailing_wide_ints <N>::extra_size (unsigned int precision)
{
- unsigned int max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
- / HOST_BITS_PER_WIDE_INT);
- return (N * max_len - 1) * sizeof (HOST_WIDE_INT);
+ return (N * wi::blocks_needed (precision) - 1) * sizeof (HOST_WIDE_INT);
}
/* This macro is used in structures that end with a trailing_wide_ints field
@@ -1621,6 +1619,26 @@ decompose (HOST_WIDE_INT *scratch, unsig
signop, bool *);
}
+/* If a value of length LEN blocks has more than PRECISION bits, return
+ the number of excess bits, otherwise return 0. For the special case
+ of PRECISION being zero, the single HWI must have the value zero and
+ there are no excess bits. Handling zero precision this way means
+ that the result is always a valid shift amount. */
+inline unsigned int
+wi::excess_bits (unsigned int len, unsigned int precision)
+{
+ unsigned int excess = len * HOST_BITS_PER_WIDE_INT - precision;
+ return excess < HOST_BITS_PER_WIDE_INT ? excess : 0;
+}
+
+/* Return the number of blocks needed for precision PRECISION. */
+inline unsigned int
+wi::blocks_needed (unsigned int precision)
+{
+ return precision == 0 ? 1 : ((precision + HOST_BITS_PER_WIDE_INT - 1)
+ / HOST_BITS_PER_WIDE_INT);
+}
+
/* Return the number of bits that integer X can hold. */
template <typename T>
inline unsigned int
@@ -1729,9 +1747,7 @@ wi::eq_p (const T1 &x, const T2 &y)
return xi.val[0] == 0;
/* Otherwise flush out any excess bits first. */
unsigned HOST_WIDE_INT diff = xi.val[0] ^ yi.val[0];
- int excess = HOST_BITS_PER_WIDE_INT - precision;
- if (excess > 0)
- diff <<= excess;
+ diff <<= wi::excess_bits (1, precision);
return diff == 0;
}
return eq_p_large (xi.val, xi.len, yi.val, yi.len, precision);
@@ -2323,7 +2339,9 @@ wi::add (const T1 &x, const T2 &y, signo
unsigned HOST_WIDE_INT xl = xi.ulow ();
unsigned HOST_WIDE_INT yl = yi.ulow ();
unsigned HOST_WIDE_INT resultl = xl + yl;
- if (sgn == SIGNED)
+ if (precision == 0)
+ *overflow = false;
+ else if (sgn == SIGNED)
*overflow = (((resultl ^ xl) & (resultl ^ yl))
>> (precision - 1)) & 1;
else
@@ -2396,7 +2414,9 @@ wi::sub (const T1 &x, const T2 &y, signo
unsigned HOST_WIDE_INT xl = xi.ulow ();
unsigned HOST_WIDE_INT yl = yi.ulow ();
unsigned HOST_WIDE_INT resultl = xl - yl;
- if (sgn == SIGNED)
+ if (precision == 0)
+ *overflow = false;
+ else if (sgn == SIGNED)
*overflow = (((xl ^ yl) & (resultl ^ xl)) >> (precision - 1)) & 1;
else
*overflow = ((resultl << (HOST_BITS_PER_WIDE_INT - precision))
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-02 19:19 [wide-int] Handle zero-precision INTEGER_CSTs again Richard Sandiford
2014-05-03 16:42 ` Kenneth Zadeck
@ 2014-05-05 8:31 ` Richard Biener
2014-05-05 10:54 ` Richard Sandiford
1 sibling, 1 reply; 15+ messages in thread
From: Richard Biener @ 2014-05-05 8:31 UTC (permalink / raw)
To: GCC Patches, Kenneth Zadeck, Mike Stump, Richard Sandiford
On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
> Richard's patch to remove min amd max values from zero-width bitfields,
> but a boostrap-ubsan showed otherwise. One source is in:
>
> null_pointer_node = build_int_cst (build_pointer_type (void_type_node), 0);
>
> if no_target, since the precision of the type comes from ptr_mode,
> which remains the default VOIDmode. Maybe that's a bug, but setting
> it to an arbitrary nonzero value would also be dangerous.
Can you explain what 'no_target' should be? ptr_mode should never be
VOIDmode. So I don't think we want this patch. Instead stor-layout should
ICE on zero-precision integer/pointer types.
Richard.
>
> The other use is in the C/C++ void_zero_node, which is specifically
> defined as a VOID_TYPE, zero-precision INTEGER_CST. This is used by the
> ubsan code in some ?: tests, as well as by other places in the frontend
> proper. At least the ubsan use persists until gimple.
>
> This patch therefore restores the wide-int handling for zero precision,
> for which the length must be 1 and the single HWI must be zero.
> I've tried to wrap up most of the dependencies in two new functions,
> wi::blocks_needed (a function version of the .cc BLOCKS_NEEDED, now also
> used in the header) and wi::excess_bits, so that it'll be easier to
> remove the handling again if zero precision ever goes away.
> There are some remaining, easily-greppable cases that check directly
> for a precision of 0 though.
>
> The combination of this and the other patches allows boostrap-ubsan to
> complete. There are a lot of extra testsuite failures compared to a
> normal bootstrap, but they don't look related to wide-int.
>
> Tested on x86_64-linux-gnu and powerpc64-linux-gnu. OK to install?
>
> Thanks,
> Richard
>
>
> Index: gcc/wide-int.cc
> ===================================================================
> --- gcc/wide-int.cc 2014-05-02 16:28:07.657847935 +0100
> +++ gcc/wide-int.cc 2014-05-02 16:28:09.560842845 +0100
> @@ -48,8 +48,6 @@ static const HOST_WIDE_INT zeros[WIDE_IN
> #define HALF_INT_MASK (((HOST_WIDE_INT) 1 << HOST_BITS_PER_HALF_WIDE_INT) - 1)
>
> #define BLOCK_OF(TARGET) ((TARGET) / HOST_BITS_PER_WIDE_INT)
> -#define BLOCKS_NEEDED(PREC) \
> - (PREC ? (((PREC) + HOST_BITS_PER_WIDE_INT - 1) / HOST_BITS_PER_WIDE_INT) : 1)
> #define SIGN_MASK(X) ((HOST_WIDE_INT) (X) < 0 ? -1 : 0)
>
> /* Return the value a VAL[I] if I < LEN, otherwise, return 0 or -1
> @@ -69,20 +67,20 @@ safe_uhwi (const HOST_WIDE_INT *val, uns
> static unsigned int
> canonize (HOST_WIDE_INT *val, unsigned int len, unsigned int precision)
> {
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> HOST_WIDE_INT top;
> int i;
>
> if (len > blocks_needed)
> len = blocks_needed;
>
> + if (wi::excess_bits (len, precision) > 0)
> + val[len - 1] = sext_hwi (val[len - 1], precision % HOST_BITS_PER_WIDE_INT);
> if (len == 1)
> return len;
>
> top = val[len - 1];
> - if (len * HOST_BITS_PER_WIDE_INT > precision)
> - val[len - 1] = top = sext_hwi (top, precision % HOST_BITS_PER_WIDE_INT);
> - if (top != 0 && top != (HOST_WIDE_INT)-1)
> + if (top != 0 && top != (HOST_WIDE_INT) -1)
> return len;
>
> /* At this point we know that the top is either 0 or -1. Find the
> @@ -134,7 +132,7 @@ wi::from_buffer (const unsigned char *bu
>
> /* We have to clear all the bits ourself, as we merely or in values
> below. */
> - unsigned int len = BLOCKS_NEEDED (precision);
> + unsigned int len = wi::blocks_needed (precision);
> HOST_WIDE_INT *val = result.write_val ();
> for (unsigned int i = 0; i < len; ++i)
> val[i] = 0;
> @@ -180,7 +178,7 @@ wi::to_mpz (const wide_int_ref &x, mpz_t
> {
> int len = x.get_len ();
> const HOST_WIDE_INT *v = x.get_val ();
> - int excess = len * HOST_BITS_PER_WIDE_INT - x.get_precision ();
> + unsigned int excess = wi::excess_bits (len, x.get_precision ());
>
> if (wi::neg_p (x, sgn))
> {
> @@ -306,7 +304,8 @@ wi::force_to_size (HOST_WIDE_INT *val, c
> unsigned int xlen, unsigned int xprecision,
> unsigned int precision, signop sgn)
> {
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> + unsigned int xblocks_needed = wi::blocks_needed (xprecision);
> unsigned int len = blocks_needed < xlen ? blocks_needed : xlen;
> for (unsigned i = 0; i < len; i++)
> val[i] = xval[i];
> @@ -318,11 +317,11 @@ wi::force_to_size (HOST_WIDE_INT *val, c
> /* Expanding. */
> if (sgn == UNSIGNED)
> {
> - if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
> + if (small_xprecision && len == xblocks_needed)
> val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
> else if (val[len - 1] < 0)
> {
> - while (len < BLOCKS_NEEDED (xprecision))
> + while (len < xblocks_needed)
> val[len++] = -1;
> if (small_xprecision)
> val[len - 1] = zext_hwi (val[len - 1], small_xprecision);
> @@ -332,7 +331,7 @@ wi::force_to_size (HOST_WIDE_INT *val, c
> }
> else
> {
> - if (small_xprecision && len == BLOCKS_NEEDED (xprecision))
> + if (small_xprecision && len == xblocks_needed)
> val[len - 1] = sext_hwi (val[len - 1], small_xprecision);
> }
> }
> @@ -372,10 +371,8 @@ selt (const HOST_WIDE_INT *a, unsigned i
> static inline HOST_WIDE_INT
> top_bit_of (const HOST_WIDE_INT *a, unsigned int len, unsigned int prec)
> {
> - int excess = len * HOST_BITS_PER_WIDE_INT - prec;
> unsigned HOST_WIDE_INT val = a[len - 1];
> - if (excess > 0)
> - val <<= excess;
> + val <<= wi::excess_bits (len, prec);
> return val >> (HOST_BITS_PER_WIDE_INT - 1);
> }
>
> @@ -391,28 +388,16 @@ wi::eq_p_large (const HOST_WIDE_INT *op0
> const HOST_WIDE_INT *op1, unsigned int op1len,
> unsigned int prec)
> {
> - int l0 = op0len - 1;
> - unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
> -
> if (op0len != op1len)
> return false;
>
> - if (op0len == BLOCKS_NEEDED (prec) && small_prec)
> - {
> - /* It does not matter if we zext or sext here, we just have to
> - do both the same way. */
> - if (zext_hwi (op0 [l0], small_prec) != zext_hwi (op1 [l0], small_prec))
> - return false;
> - l0--;
> - }
> -
> - while (l0 >= 0)
> - if (op0[l0] != op1[l0])
> + for (unsigned int i = 0; i < op0len - 1; i++)
> + if (op0[i] != op1[i])
> return false;
> - else
> - l0--;
>
> - return true;
> + unsigned HOST_WIDE_INT top0 = op0[op0len - 1];
> + unsigned HOST_WIDE_INT top1 = op1[op1len - 1];
> + return ((top0 ^ top1) << wi::excess_bits (op0len, prec)) == 0;
> }
>
> /* Return true if OP0 < OP1 using signed comparisons. */
> @@ -423,7 +408,7 @@ wi::lts_p_large (const HOST_WIDE_INT *op
> {
> HOST_WIDE_INT s0, s1;
> unsigned HOST_WIDE_INT u0, u1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -461,7 +446,7 @@ wi::cmps_large (const HOST_WIDE_INT *op0
> {
> HOST_WIDE_INT s0, s1;
> unsigned HOST_WIDE_INT u0, u1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -498,7 +483,7 @@ wi::ltu_p_large (const HOST_WIDE_INT *op
> {
> unsigned HOST_WIDE_INT x0;
> unsigned HOST_WIDE_INT x1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -525,7 +510,7 @@ wi::cmpu_large (const HOST_WIDE_INT *op0
> {
> unsigned HOST_WIDE_INT x0;
> unsigned HOST_WIDE_INT x1;
> - unsigned int blocks_needed = BLOCKS_NEEDED (precision);
> + unsigned int blocks_needed = wi::blocks_needed (precision);
> unsigned int small_prec = precision & (HOST_BITS_PER_WIDE_INT - 1);
> int l = MAX (op0len - 1, op1len - 1);
>
> @@ -673,7 +658,7 @@ wide_int_storage::bswap () const
> {
> wide_int result = wide_int::create (precision);
> unsigned int i, s;
> - unsigned int len = BLOCKS_NEEDED (precision);
> + unsigned int len = wi::blocks_needed (precision);
> unsigned int xlen = get_len ();
> const HOST_WIDE_INT *xval = get_val ();
> HOST_WIDE_INT *val = result.write_val ();
> @@ -1149,7 +1134,7 @@ wi_unpack (unsigned HOST_HALF_WIDE_INT *
> unsigned int i;
> unsigned int j = 0;
> unsigned int small_prec = prec & (HOST_BITS_PER_WIDE_INT - 1);
> - unsigned int blocks_needed = BLOCKS_NEEDED (prec);
> + unsigned int blocks_needed = wi::blocks_needed (prec);
> HOST_WIDE_INT mask;
>
> if (sgn == SIGNED)
> @@ -1222,7 +1207,7 @@ wi::mul_internal (HOST_WIDE_INT *val, co
> unsigned HOST_WIDE_INT o0, o1, k, t;
> unsigned int i;
> unsigned int j;
> - unsigned int blocks_needed = BLOCKS_NEEDED (prec);
> + unsigned int blocks_needed = wi::blocks_needed (prec);
> unsigned int half_blocks_needed = blocks_needed * 2;
> /* The sizes here are scaled to support a 2x largest mode by 2x
> largest mode yielding a 4x largest mode result. This is what is
> @@ -1426,6 +1411,9 @@ wi::popcount (const wide_int_ref &x)
> unsigned int i;
> int count;
>
> + if (x.precision == 0)
> + return 0;
> +
> /* The high order block is special if it is the last block and the
> precision is not an even multiple of HOST_BITS_PER_WIDE_INT. We
> have to clear out any ones above the precision before doing
> @@ -1645,8 +1633,8 @@ wi::divmod_internal (HOST_WIDE_INT *quot
> unsigned int divisor_prec, signop sgn,
> bool *oflow)
> {
> - unsigned int dividend_blocks_needed = 2 * BLOCKS_NEEDED (dividend_prec);
> - unsigned int divisor_blocks_needed = 2 * BLOCKS_NEEDED (divisor_prec);
> + unsigned int dividend_blocks_needed = 2 * wi::blocks_needed (dividend_prec);
> + unsigned int divisor_blocks_needed = 2 * wi::blocks_needed (divisor_prec);
> unsigned HOST_HALF_WIDE_INT
> b_quotient[4 * MAX_BITSIZE_MODE_ANY_INT / HOST_BITS_PER_HALF_WIDE_INT];
> unsigned HOST_HALF_WIDE_INT
> @@ -1671,7 +1659,7 @@ wi::divmod_internal (HOST_WIDE_INT *quot
> /* The smallest signed number / -1 causes overflow. The dividend_len
> check is for speed rather than correctness. */
> if (sgn == SIGNED
> - && dividend_len == BLOCKS_NEEDED (dividend_prec)
> + && dividend_len == wi::blocks_needed (dividend_prec)
> && divisor == -1
> && wi::only_sign_bit_p (dividend))
> overflow = true;
> @@ -1838,7 +1826,7 @@ wi::lshift_large (HOST_WIDE_INT *val, co
> unsigned int small_shift = shift % HOST_BITS_PER_WIDE_INT;
>
> /* The whole-block shift fills with zeros. */
> - unsigned int len = BLOCKS_NEEDED (precision);
> + unsigned int len = wi::blocks_needed (precision);
> for (unsigned int i = 0; i < skip; ++i)
> val[i] = 0;
>
> @@ -1876,7 +1864,7 @@ rshift_large_common (HOST_WIDE_INT *val,
>
> /* Work out how many blocks are needed to store the significant bits
> (excluding the upper zeros or signs). */
> - unsigned int len = BLOCKS_NEEDED (xprecision - shift);
> + unsigned int len = wi::blocks_needed (xprecision - shift);
>
> /* It's easier to handle the simple block case specially. */
> if (small_shift == 0)
> @@ -1949,6 +1937,9 @@ wi::arshift_large (HOST_WIDE_INT *val, c
> int
> wi::clz (const wide_int_ref &x)
> {
> + if (x.precision == 0)
> + return 0;
> +
> /* Calculate how many bits there above the highest represented block. */
> int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
>
> @@ -1973,6 +1964,9 @@ wi::clz (const wide_int_ref &x)
> int
> wi::clrsb (const wide_int_ref &x)
> {
> + if (x.precision == 0)
> + return 0;
> +
> /* Calculate how many bits there above the highest represented block. */
> int count = x.precision - x.len * HOST_BITS_PER_WIDE_INT;
>
> @@ -2017,6 +2011,9 @@ wi::ctz (const wide_int_ref &x)
> int
> wi::exact_log2 (const wide_int_ref &x)
> {
> + if (x.precision == 0)
> + return -1;
> +
> /* Reject cases where there are implicit -1 blocks above HIGH. */
> if (x.len * HOST_BITS_PER_WIDE_INT < x.precision && x.sign_mask () < 0)
> return -1;
> Index: gcc/wide-int.h
> ===================================================================
> --- gcc/wide-int.h 2014-05-02 16:28:07.657847935 +0100
> +++ gcc/wide-int.h 2014-05-02 16:28:09.561842842 +0100
> @@ -430,6 +430,9 @@ #define WIDE_INT_REF_FOR(T) \
> /* Public functions for querying and operating on integers. */
> namespace wi
> {
> + unsigned int excess_bits (unsigned int, unsigned int);
> + unsigned int blocks_needed (unsigned int);
> +
> template <typename T>
> unsigned int get_precision (const T &);
>
> @@ -740,7 +743,7 @@ inline generic_wide_int <storage>::gener
> inline HOST_WIDE_INT
> generic_wide_int <storage>::to_shwi (unsigned int precision) const
> {
> - if (precision < HOST_BITS_PER_WIDE_INT)
> + if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
> return sext_hwi (this->get_val ()[0], precision);
> else
> return this->get_val ()[0];
> @@ -764,7 +767,7 @@ generic_wide_int <storage>::to_shwi () c
> inline unsigned HOST_WIDE_INT
> generic_wide_int <storage>::to_uhwi (unsigned int precision) const
> {
> - if (precision < HOST_BITS_PER_WIDE_INT)
> + if (precision > 0 && precision < HOST_BITS_PER_WIDE_INT)
> return zext_hwi (this->get_val ()[0], precision);
> else
> return this->get_val ()[0];
> @@ -797,12 +800,7 @@ generic_wide_int <storage>::sign_mask ()
> unsigned int len = this->get_len ();
> unsigned HOST_WIDE_INT high = this->get_val ()[len - 1];
> if (!is_sign_extended)
> - {
> - unsigned int precision = this->get_precision ();
> - int excess = len * HOST_BITS_PER_WIDE_INT - precision;
> - if (excess > 0)
> - high <<= excess;
> - }
> + high <<= wi::excess_bits (len, this->get_precision ());
> return (HOST_WIDE_INT) (high) < 0 ? -1 : 0;
> }
>
> @@ -1068,7 +1066,7 @@ wide_int_storage::write_val ()
> wide_int_storage::set_len (unsigned int l, bool is_sign_extended)
> {
> len = l;
> - if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > precision)
> + if (!is_sign_extended && wi::excess_bits (len, precision) > 0)
> val[len - 1] = sext_hwi (val[len - 1],
> precision % HOST_BITS_PER_WIDE_INT);
> }
> @@ -1347,7 +1345,7 @@ trailing_wide_int_storage::write_val ()
> trailing_wide_int_storage::set_len (unsigned int len, bool is_sign_extended)
> {
> *m_len = len;
> - if (!is_sign_extended && len * HOST_BITS_PER_WIDE_INT > m_precision)
> + if (!is_sign_extended && wi::excess_bits (len, m_precision) > 0)
> m_val[len - 1] = sext_hwi (m_val[len - 1],
> m_precision % HOST_BITS_PER_WIDE_INT);
> }
> @@ -1368,8 +1366,7 @@ trailing_wide_int_storage::operator = (c
> trailing_wide_ints <N>::set_precision (unsigned int precision)
> {
> m_precision = precision;
> - m_max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
> - / HOST_BITS_PER_WIDE_INT);
> + m_max_len = wi::blocks_needed (precision);
> }
>
> /* Return a reference to element INDEX. */
> @@ -1387,9 +1384,7 @@ trailing_wide_ints <N>::operator [] (uns
> inline size_t
> trailing_wide_ints <N>::extra_size (unsigned int precision)
> {
> - unsigned int max_len = ((precision + HOST_BITS_PER_WIDE_INT - 1)
> - / HOST_BITS_PER_WIDE_INT);
> - return (N * max_len - 1) * sizeof (HOST_WIDE_INT);
> + return (N * wi::blocks_needed (precision) - 1) * sizeof (HOST_WIDE_INT);
> }
>
> /* This macro is used in structures that end with a trailing_wide_ints field
> @@ -1621,6 +1616,26 @@ decompose (HOST_WIDE_INT *scratch, unsig
> signop, bool *);
> }
>
> +/* If a value of length LEN blocks has more than PRECISION bits, return
> + the number of excess bits, otherwise return 0. For the special case
> + of PRECISION being zero, the single HWI must have the value zero and
> + there are no excess bits. Handling zero precision this way means
> + that the result is always a valid shift amount. */
> +inline unsigned int
> +wi::excess_bits (unsigned int len, unsigned int precision)
> +{
> + unsigned int excess = len * HOST_BITS_PER_WIDE_INT - precision;
> + return excess < HOST_BITS_PER_WIDE_INT ? excess : 0;
> +}
> +
> +/* Return the number of blocks needed for precision PRECISION. */
> +inline unsigned int
> +wi::blocks_needed (unsigned int precision)
> +{
> + return precision == 0 ? 1 : ((precision + HOST_BITS_PER_WIDE_INT - 1)
> + / HOST_BITS_PER_WIDE_INT);
> +}
> +
> /* Return the number of bits that integer X can hold. */
> template <typename T>
> inline unsigned int
> @@ -1729,9 +1744,7 @@ wi::eq_p (const T1 &x, const T2 &y)
> return xi.val[0] == 0;
> /* Otherwise flush out any excess bits first. */
> unsigned HOST_WIDE_INT diff = xi.val[0] ^ yi.val[0];
> - int excess = HOST_BITS_PER_WIDE_INT - precision;
> - if (excess > 0)
> - diff <<= excess;
> + diff <<= wi::excess_bits (1, precision);
> return diff == 0;
> }
> return eq_p_large (xi.val, xi.len, yi.val, yi.len, precision);
> @@ -2323,7 +2336,9 @@ wi::add (const T1 &x, const T2 &y, signo
> unsigned HOST_WIDE_INT xl = xi.ulow ();
> unsigned HOST_WIDE_INT yl = yi.ulow ();
> unsigned HOST_WIDE_INT resultl = xl + yl;
> - if (sgn == SIGNED)
> + if (precision == 0)
> + *overflow = false;
> + else if (sgn == SIGNED)
> *overflow = (((resultl ^ xl) & (resultl ^ yl))
> >> (precision - 1)) & 1;
> else
> @@ -2396,7 +2411,9 @@ wi::sub (const T1 &x, const T2 &y, signo
> unsigned HOST_WIDE_INT xl = xi.ulow ();
> unsigned HOST_WIDE_INT yl = yi.ulow ();
> unsigned HOST_WIDE_INT resultl = xl - yl;
> - if (sgn == SIGNED)
> + if (precision == 0)
> + *overflow = false;
> + else if (sgn == SIGNED)
> *overflow = (((xl ^ yl) & (resultl ^ xl)) >> (precision - 1)) & 1;
> else
> *overflow = ((resultl << (HOST_BITS_PER_WIDE_INT - precision))
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-05 8:31 ` Richard Biener
@ 2014-05-05 10:54 ` Richard Sandiford
2014-05-05 12:16 ` Richard Biener
0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2014-05-05 10:54 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Kenneth Zadeck, Mike Stump
Richard Biener <richard.guenther@gmail.com> writes:
> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>> Richard's patch to remove min amd max values from zero-width bitfields,
>> but a boostrap-ubsan showed otherwise. One source is in:
>>
>> null_pointer_node = build_int_cst (build_pointer_type (void_type_node), 0);
>>
>> if no_target, since the precision of the type comes from ptr_mode,
>> which remains the default VOIDmode. Maybe that's a bug, but setting
>> it to an arbitrary nonzero value would also be dangerous.
>
> Can you explain what 'no_target' should be? ptr_mode should never be
> VOIDmode.
Sorry, meant "no_backend" rather than "no_target". See do_compile.
> So I don't think we want this patch. Instead stor-layout should
> ICE on zero-precision integer/pointer types.
What should happen for void_zero_node?
Thanks,
Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-05 10:54 ` Richard Sandiford
@ 2014-05-05 12:16 ` Richard Biener
2014-05-05 14:51 ` Richard Sandiford
0 siblings, 1 reply; 15+ messages in thread
From: Richard Biener @ 2014-05-05 12:16 UTC (permalink / raw)
To: Richard Biener, GCC Patches, Kenneth Zadeck, Mike Stump,
Richard Sandiford
On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>
>>> null_pointer_node = build_int_cst (build_pointer_type (void_type_node), 0);
>>>
>>> if no_target, since the precision of the type comes from ptr_mode,
>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>> it to an arbitrary nonzero value would also be dangerous.
>>
>> Can you explain what 'no_target' should be? ptr_mode should never be
>> VOIDmode.
>
> Sorry, meant "no_backend" rather than "no_target". See do_compile.
Ok. So we do
/* This must be run always, because it is needed to compute the FP
predefined macros, such as __LDBL_MAX__, for targets using non
default FP formats. */
init_adjust_machine_modes ();
/* Set up the back-end if requested. */
if (!no_backend)
backend_init ();
where I think that init_adjust_machine_modes should initialize the
{byte,word,ptr}_mode globals. Move from init_emit_once.
But why do we even run into uses of null_pointer_node for no_backend?
For sure for -fsyntax-only we can't really omit backend init as IMHO
no parser piece is decoupled enough to never call target hooks or so.
Thus, omit less of the initialization for no_backend.
>> So I don't think we want this patch. Instead stor-layout should
>> ICE on zero-precision integer/pointer types.
>
> What should happen for void_zero_node?
Not sure what that beast is supposed to be or why it should be
of INTEGER_CST kind (it's not even initialized in any meaningful
way).
That said, the wide-int code shouldn't be slowed down by
precision == 0 checks. We should never ever reach wide-int
with such a constant.
Richard.
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-05 12:16 ` Richard Biener
@ 2014-05-05 14:51 ` Richard Sandiford
2014-05-05 17:32 ` Richard Biener
0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2014-05-05 14:51 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Kenneth Zadeck, Mike Stump
Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>>
>>>> null_pointer_node = build_int_cst (build_pointer_type
>>>> (void_type_node), 0);
>>>>
>>>> if no_target, since the precision of the type comes from ptr_mode,
>>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>>> it to an arbitrary nonzero value would also be dangerous.
>>>
>>> Can you explain what 'no_target' should be? ptr_mode should never be
>>> VOIDmode.
>>
>> Sorry, meant "no_backend" rather than "no_target". See do_compile.
>
> Ok. So we do
>
> /* This must be run always, because it is needed to compute the FP
> predefined macros, such as __LDBL_MAX__, for targets using non
> default FP formats. */
> init_adjust_machine_modes ();
>
> /* Set up the back-end if requested. */
> if (!no_backend)
> backend_init ();
>
> where I think that init_adjust_machine_modes should initialize the
> {byte,word,ptr}_mode globals. Move from init_emit_once.
>
> But why do we even run into uses of null_pointer_node for no_backend?
> For sure for -fsyntax-only we can't really omit backend init as IMHO
> no parser piece is decoupled enough to never call target hooks or so.
>
> Thus, omit less of the initialization for no_backend.
OK, but I don't think the wide-int merge should be held up for clean-ups
like that. At the moment the tree & gimple leve do use 0 precision, and
although I think we both agree it'd be better if they didn't, let's do
one thing at a time.
>>> So I don't think we want this patch. Instead stor-layout should
>>> ICE on zero-precision integer/pointer types.
>>
>> What should happen for void_zero_node?
>
> Not sure what that beast is supposed to be or why it should be
> of INTEGER_CST kind (it's not even initialized in any meaningful
> way).
>
> That said, the wide-int code shouldn't be slowed down by
> precision == 0 checks. We should never ever reach wide-int
> with such a constant.
void_zero_node is used for ubsan too, and survives into gimple.
I did hit this in real tests, it wasn't just theoretical.
Thanks,
Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-05 14:51 ` Richard Sandiford
@ 2014-05-05 17:32 ` Richard Biener
2014-05-05 18:04 ` Marek Polacek
2014-05-05 20:58 ` Richard Sandiford
0 siblings, 2 replies; 15+ messages in thread
From: Richard Biener @ 2014-05-05 17:32 UTC (permalink / raw)
To: Richard Biener, GCC Patches, Kenneth Zadeck, Mike Stump,
Richard Sandiford
On Mon, May 5, 2014 at 4:51 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>>>
>>>>> null_pointer_node = build_int_cst (build_pointer_type
>>>>> (void_type_node), 0);
>>>>>
>>>>> if no_target, since the precision of the type comes from ptr_mode,
>>>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>>>> it to an arbitrary nonzero value would also be dangerous.
>>>>
>>>> Can you explain what 'no_target' should be? ptr_mode should never be
>>>> VOIDmode.
>>>
>>> Sorry, meant "no_backend" rather than "no_target". See do_compile.
>>
>> Ok. So we do
>>
>> /* This must be run always, because it is needed to compute the FP
>> predefined macros, such as __LDBL_MAX__, for targets using non
>> default FP formats. */
>> init_adjust_machine_modes ();
>>
>> /* Set up the back-end if requested. */
>> if (!no_backend)
>> backend_init ();
>>
>> where I think that init_adjust_machine_modes should initialize the
>> {byte,word,ptr}_mode globals. Move from init_emit_once.
>>
>> But why do we even run into uses of null_pointer_node for no_backend?
>> For sure for -fsyntax-only we can't really omit backend init as IMHO
>> no parser piece is decoupled enough to never call target hooks or so.
>>
>> Thus, omit less of the initialization for no_backend.
>
> OK, but I don't think the wide-int merge should be held up for clean-ups
> like that. At the moment the tree & gimple leve do use 0 precision, and
> although I think we both agree it'd be better if they didn't, let's do
> one thing at a time.
Fair enough.
>>>> So I don't think we want this patch. Instead stor-layout should
>>>> ICE on zero-precision integer/pointer types.
>>>
>>> What should happen for void_zero_node?
>>
>> Not sure what that beast is supposed to be or why it should be
>> of INTEGER_CST kind (it's not even initialized in any meaningful
>> way).
>>
>> That said, the wide-int code shouldn't be slowed down by
>> precision == 0 checks. We should never ever reach wide-int
>> with such a constant.
>
> void_zero_node is used for ubsan too, and survives into gimple.
> I did hit this in real tests, it wasn't just theoretical.
Ugh - for what does it use that ... :/
Please remember how to trigger those issues and I'll happily have
a look after the merge.
Thanks,
Richard.
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-05 17:32 ` Richard Biener
@ 2014-05-05 18:04 ` Marek Polacek
2014-05-05 20:58 ` Richard Sandiford
1 sibling, 0 replies; 15+ messages in thread
From: Marek Polacek @ 2014-05-05 18:04 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Kenneth Zadeck, Mike Stump, Richard Sandiford
On Mon, May 05, 2014 at 07:32:31PM +0200, Richard Biener wrote:
> > void_zero_node is used for ubsan too, and survives into gimple.
> > I did hit this in real tests, it wasn't just theoretical.
>
> Ugh - for what does it use that ... :/
It's used like this:
t = fold_build3 (COND_EXPR, void_type_node, t, tt, void_zero_node);
so if condition T isn't true, nothing should happen. I remember that
initially I used something similar to void_zero_node, but that
resulted in ICEs.
Marek
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-05 17:32 ` Richard Biener
2014-05-05 18:04 ` Marek Polacek
@ 2014-05-05 20:58 ` Richard Sandiford
2014-05-06 8:08 ` Richard Biener
1 sibling, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2014-05-05 20:58 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Kenneth Zadeck, Mike Stump
Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, May 5, 2014 at 4:51 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>>>>> <rdsandiford@googlemail.com> wrote:
>>>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>>>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>>>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>>>>
>>>>>> null_pointer_node = build_int_cst (build_pointer_type
>>>>>> (void_type_node), 0);
>>>>>>
>>>>>> if no_target, since the precision of the type comes from ptr_mode,
>>>>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>>>>> it to an arbitrary nonzero value would also be dangerous.
>>>>>
>>>>> Can you explain what 'no_target' should be? ptr_mode should never be
>>>>> VOIDmode.
>>>>
>>>> Sorry, meant "no_backend" rather than "no_target". See do_compile.
>>>
>>> Ok. So we do
>>>
>>> /* This must be run always, because it is needed to compute the FP
>>> predefined macros, such as __LDBL_MAX__, for targets using non
>>> default FP formats. */
>>> init_adjust_machine_modes ();
>>>
>>> /* Set up the back-end if requested. */
>>> if (!no_backend)
>>> backend_init ();
>>>
>>> where I think that init_adjust_machine_modes should initialize the
>>> {byte,word,ptr}_mode globals. Move from init_emit_once.
init_adjust_machine_modes is an auto-generated function so I ended up
using a new function instead. Tested on x86_64-linux-gnu. OK to install?
>>>>> So I don't think we want this patch. Instead stor-layout should
>>>>> ICE on zero-precision integer/pointer types.
>>>>
>>>> What should happen for void_zero_node?
>>>
>>> Not sure what that beast is supposed to be or why it should be
>>> of INTEGER_CST kind (it's not even initialized in any meaningful
>>> way).
>>>
>>> That said, the wide-int code shouldn't be slowed down by
>>> precision == 0 checks. We should never ever reach wide-int
>>> with such a constant.
>>
>> void_zero_node is used for ubsan too, and survives into gimple.
>> I did hit this in real tests, it wasn't just theoretical.
>
> Ugh - for what does it use that ... :/
>
> Please remember how to trigger those issues and I'll happily have
> a look after the merge.
At the time it was just a normal bootstrap-ubsan, but that was
before the zero-precision patch. Probably the best way of
checking for zero-precision tree constants is to put an assert
for nonzero precisions in:
inline unsigned int
wi::int_traits <const_tree>::get_precision (const_tree tcst)
{
return TYPE_PRECISION (TREE_TYPE (tcst));
}
and:
template <int N>
inline wi::extended_tree <N>::extended_tree (const_tree t)
: m_t (t)
{
gcc_checking_assert (TYPE_PRECISION (TREE_TYPE (t)) <= N);
}
and then bootstrap-ubsan. But like Marek says, the ubsan code uses
void_zero_node by name.
Thanks,
Richard
gcc/
* emit-rtl.c (init_derived_machine_modes): New functionm, split
out from...
(init_emit_once): ...here.
* rtl.h (init_derived_machine_modes): Declare.
* toplev.c (do_compile): Call it even if no_backend.
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c 2014-05-03 20:18:49.157107743 +0100
+++ gcc/emit-rtl.c 2014-05-05 17:44:53.579038259 +0100
@@ -5620,6 +5620,30 @@ init_emit_regs (void)
}
}
+/* Initialize global machine_mode variables. */
+
+void
+init_derived_machine_modes (void)
+{
+ byte_mode = VOIDmode;
+ word_mode = VOIDmode;
+
+ for (enum machine_mode mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
+ mode != VOIDmode;
+ mode = GET_MODE_WIDER_MODE (mode))
+ {
+ if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT
+ && byte_mode == VOIDmode)
+ byte_mode = mode;
+
+ if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD
+ && word_mode == VOIDmode)
+ word_mode = mode;
+ }
+
+ ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0);
+}
+
/* Create some permanent unique rtl objects shared between all functions. */
void
@@ -5643,36 +5667,6 @@ init_emit_once (void)
reg_attrs_htab = htab_create_ggc (37, reg_attrs_htab_hash,
reg_attrs_htab_eq, NULL);
- /* Compute the word and byte modes. */
-
- byte_mode = VOIDmode;
- word_mode = VOIDmode;
- double_mode = VOIDmode;
-
- for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
- mode != VOIDmode;
- mode = GET_MODE_WIDER_MODE (mode))
- {
- if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT
- && byte_mode == VOIDmode)
- byte_mode = mode;
-
- if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD
- && word_mode == VOIDmode)
- word_mode = mode;
- }
-
- for (mode = GET_CLASS_NARROWEST_MODE (MODE_FLOAT);
- mode != VOIDmode;
- mode = GET_MODE_WIDER_MODE (mode))
- {
- if (GET_MODE_BITSIZE (mode) == DOUBLE_TYPE_SIZE
- && double_mode == VOIDmode)
- double_mode = mode;
- }
-
- ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0);
-
#ifdef INIT_EXPANDERS
/* This is to initialize {init|mark|free}_machine_status before the first
call to push_function_context_to. This is needed by the Chill front
@@ -5695,6 +5689,8 @@ init_emit_once (void)
else
const_true_rtx = gen_rtx_CONST_INT (VOIDmode, STORE_FLAG_VALUE);
+ double_mode = mode_for_size (DOUBLE_TYPE_SIZE, MODE_FLOAT, 0);
+
REAL_VALUE_FROM_INT (dconst0, 0, 0, double_mode);
REAL_VALUE_FROM_INT (dconst1, 1, 0, double_mode);
REAL_VALUE_FROM_INT (dconst2, 2, 0, double_mode);
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h 2014-05-03 20:18:49.157107743 +0100
+++ gcc/rtl.h 2014-05-05 17:40:26.035785011 +0100
@@ -2545,6 +2545,7 @@ extern int get_max_insn_count (void);
extern int in_sequence_p (void);
extern void init_emit (void);
extern void init_emit_regs (void);
+extern void init_derived_machine_modes (void);
extern void init_emit_once (void);
extern void push_topmost_sequence (void);
extern void pop_topmost_sequence (void);
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c 2014-04-26 16:05:43.076775028 +0100
+++ gcc/toplev.c 2014-05-05 17:41:00.168079259 +0100
@@ -1891,6 +1891,7 @@ do_compile (void)
predefined macros, such as __LDBL_MAX__, for targets using non
default FP formats. */
init_adjust_machine_modes ();
+ init_derived_machine_modes ();
/* Set up the back-end if requested. */
if (!no_backend)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-05 20:58 ` Richard Sandiford
@ 2014-05-06 8:08 ` Richard Biener
2014-05-06 8:11 ` Richard Sandiford
0 siblings, 1 reply; 15+ messages in thread
From: Richard Biener @ 2014-05-06 8:08 UTC (permalink / raw)
To: Richard Biener, GCC Patches, Kenneth Zadeck, Mike Stump,
Richard Sandiford
On Mon, May 5, 2014 at 10:58 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, May 5, 2014 at 4:51 PM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>>>>>> <rdsandiford@googlemail.com> wrote:
>>>>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>>>>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>>>>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>>>>>
>>>>>>> null_pointer_node = build_int_cst (build_pointer_type
>>>>>>> (void_type_node), 0);
>>>>>>>
>>>>>>> if no_target, since the precision of the type comes from ptr_mode,
>>>>>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>>>>>> it to an arbitrary nonzero value would also be dangerous.
>>>>>>
>>>>>> Can you explain what 'no_target' should be? ptr_mode should never be
>>>>>> VOIDmode.
>>>>>
>>>>> Sorry, meant "no_backend" rather than "no_target". See do_compile.
>>>>
>>>> Ok. So we do
>>>>
>>>> /* This must be run always, because it is needed to compute the FP
>>>> predefined macros, such as __LDBL_MAX__, for targets using non
>>>> default FP formats. */
>>>> init_adjust_machine_modes ();
>>>>
>>>> /* Set up the back-end if requested. */
>>>> if (!no_backend)
>>>> backend_init ();
>>>>
>>>> where I think that init_adjust_machine_modes should initialize the
>>>> {byte,word,ptr}_mode globals. Move from init_emit_once.
>
> init_adjust_machine_modes is an auto-generated function so I ended up
> using a new function instead. Tested on x86_64-linux-gnu. OK to install?
Ok with also setting double_mode there.
>>>>>> So I don't think we want this patch. Instead stor-layout should
>>>>>> ICE on zero-precision integer/pointer types.
>>>>>
>>>>> What should happen for void_zero_node?
>>>>
>>>> Not sure what that beast is supposed to be or why it should be
>>>> of INTEGER_CST kind (it's not even initialized in any meaningful
>>>> way).
>>>>
>>>> That said, the wide-int code shouldn't be slowed down by
>>>> precision == 0 checks. We should never ever reach wide-int
>>>> with such a constant.
>>>
>>> void_zero_node is used for ubsan too, and survives into gimple.
>>> I did hit this in real tests, it wasn't just theoretical.
>>
>> Ugh - for what does it use that ... :/
>>
>> Please remember how to trigger those issues and I'll happily have
>> a look after the merge.
>
> At the time it was just a normal bootstrap-ubsan, but that was
> before the zero-precision patch. Probably the best way of
> checking for zero-precision tree constants is to put an assert
> for nonzero precisions in:
>
> inline unsigned int
> wi::int_traits <const_tree>::get_precision (const_tree tcst)
> {
> return TYPE_PRECISION (TREE_TYPE (tcst));
> }
>
> and:
>
> template <int N>
> inline wi::extended_tree <N>::extended_tree (const_tree t)
> : m_t (t)
> {
> gcc_checking_assert (TYPE_PRECISION (TREE_TYPE (t)) <= N);
> }
>
> and then bootstrap-ubsan. But like Marek says, the ubsan code uses
> void_zero_node by name.
I think for the uses it has it can just use NULL_TREE in place of
void_zero_node.
Richard.
> Thanks,
> Richard
>
>
> gcc/
> * emit-rtl.c (init_derived_machine_modes): New functionm, split
> out from...
> (init_emit_once): ...here.
> * rtl.h (init_derived_machine_modes): Declare.
> * toplev.c (do_compile): Call it even if no_backend.
>
> Index: gcc/emit-rtl.c
> ===================================================================
> --- gcc/emit-rtl.c 2014-05-03 20:18:49.157107743 +0100
> +++ gcc/emit-rtl.c 2014-05-05 17:44:53.579038259 +0100
> @@ -5620,6 +5620,30 @@ init_emit_regs (void)
> }
> }
>
> +/* Initialize global machine_mode variables. */
> +
> +void
> +init_derived_machine_modes (void)
> +{
> + byte_mode = VOIDmode;
> + word_mode = VOIDmode;
> +
> + for (enum machine_mode mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
> + mode != VOIDmode;
> + mode = GET_MODE_WIDER_MODE (mode))
> + {
> + if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT
> + && byte_mode == VOIDmode)
> + byte_mode = mode;
> +
> + if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD
> + && word_mode == VOIDmode)
> + word_mode = mode;
> + }
> +
> + ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0);
> +}
> +
> /* Create some permanent unique rtl objects shared between all functions. */
>
> void
> @@ -5643,36 +5667,6 @@ init_emit_once (void)
> reg_attrs_htab = htab_create_ggc (37, reg_attrs_htab_hash,
> reg_attrs_htab_eq, NULL);
>
> - /* Compute the word and byte modes. */
> -
> - byte_mode = VOIDmode;
> - word_mode = VOIDmode;
> - double_mode = VOIDmode;
> -
> - for (mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
> - mode != VOIDmode;
> - mode = GET_MODE_WIDER_MODE (mode))
> - {
> - if (GET_MODE_BITSIZE (mode) == BITS_PER_UNIT
> - && byte_mode == VOIDmode)
> - byte_mode = mode;
> -
> - if (GET_MODE_BITSIZE (mode) == BITS_PER_WORD
> - && word_mode == VOIDmode)
> - word_mode = mode;
> - }
> -
> - for (mode = GET_CLASS_NARROWEST_MODE (MODE_FLOAT);
> - mode != VOIDmode;
> - mode = GET_MODE_WIDER_MODE (mode))
> - {
> - if (GET_MODE_BITSIZE (mode) == DOUBLE_TYPE_SIZE
> - && double_mode == VOIDmode)
> - double_mode = mode;
> - }
> -
> - ptr_mode = mode_for_size (POINTER_SIZE, GET_MODE_CLASS (Pmode), 0);
> -
> #ifdef INIT_EXPANDERS
> /* This is to initialize {init|mark|free}_machine_status before the first
> call to push_function_context_to. This is needed by the Chill front
> @@ -5695,6 +5689,8 @@ init_emit_once (void)
> else
> const_true_rtx = gen_rtx_CONST_INT (VOIDmode, STORE_FLAG_VALUE);
>
> + double_mode = mode_for_size (DOUBLE_TYPE_SIZE, MODE_FLOAT, 0);
> +
> REAL_VALUE_FROM_INT (dconst0, 0, 0, double_mode);
> REAL_VALUE_FROM_INT (dconst1, 1, 0, double_mode);
> REAL_VALUE_FROM_INT (dconst2, 2, 0, double_mode);
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h 2014-05-03 20:18:49.157107743 +0100
> +++ gcc/rtl.h 2014-05-05 17:40:26.035785011 +0100
> @@ -2545,6 +2545,7 @@ extern int get_max_insn_count (void);
> extern int in_sequence_p (void);
> extern void init_emit (void);
> extern void init_emit_regs (void);
> +extern void init_derived_machine_modes (void);
> extern void init_emit_once (void);
> extern void push_topmost_sequence (void);
> extern void pop_topmost_sequence (void);
> Index: gcc/toplev.c
> ===================================================================
> --- gcc/toplev.c 2014-04-26 16:05:43.076775028 +0100
> +++ gcc/toplev.c 2014-05-05 17:41:00.168079259 +0100
> @@ -1891,6 +1891,7 @@ do_compile (void)
> predefined macros, such as __LDBL_MAX__, for targets using non
> default FP formats. */
> init_adjust_machine_modes ();
> + init_derived_machine_modes ();
>
> /* Set up the back-end if requested. */
> if (!no_backend)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-06 8:08 ` Richard Biener
@ 2014-05-06 8:11 ` Richard Sandiford
2014-05-06 8:20 ` Richard Biener
0 siblings, 1 reply; 15+ messages in thread
From: Richard Sandiford @ 2014-05-06 8:11 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches, Kenneth Zadeck, Mike Stump
Richard Biener <richard.guenther@gmail.com> writes:
> On Mon, May 5, 2014 at 10:58 PM, Richard Sandiford
> <rdsandiford@googlemail.com> wrote:
>> Richard Biener <richard.guenther@gmail.com> writes:
>>> On Mon, May 5, 2014 at 4:51 PM, Richard Sandiford
>>> <rdsandiford@googlemail.com> wrote:
>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
>>>>> <rdsandiford@googlemail.com> wrote:
>>>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>>>>>>> <rdsandiford@googlemail.com> wrote:
>>>>>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>>>>>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>>>>>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>>>>>>
>>>>>>>> null_pointer_node = build_int_cst (build_pointer_type
>>>>>>>> (void_type_node), 0);
>>>>>>>>
>>>>>>>> if no_target, since the precision of the type comes from ptr_mode,
>>>>>>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>>>>>>> it to an arbitrary nonzero value would also be dangerous.
>>>>>>>
>>>>>>> Can you explain what 'no_target' should be? ptr_mode should never be
>>>>>>> VOIDmode.
>>>>>>
>>>>>> Sorry, meant "no_backend" rather than "no_target". See do_compile.
>>>>>
>>>>> Ok. So we do
>>>>>
>>>>> /* This must be run always, because it is needed to compute the FP
>>>>> predefined macros, such as __LDBL_MAX__, for targets using non
>>>>> default FP formats. */
>>>>> init_adjust_machine_modes ();
>>>>>
>>>>> /* Set up the back-end if requested. */
>>>>> if (!no_backend)
>>>>> backend_init ();
>>>>>
>>>>> where I think that init_adjust_machine_modes should initialize the
>>>>> {byte,word,ptr}_mode globals. Move from init_emit_once.
>>
>> init_adjust_machine_modes is an auto-generated function so I ended up
>> using a new function instead. Tested on x86_64-linux-gnu. OK to install?
>
> Ok with also setting double_mode there.
double_mode is just a local variable though, it's only used to set up
dconstN.
Thanks,
Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [wide-int] Handle zero-precision INTEGER_CSTs again
2014-05-06 8:11 ` Richard Sandiford
@ 2014-05-06 8:20 ` Richard Biener
0 siblings, 0 replies; 15+ messages in thread
From: Richard Biener @ 2014-05-06 8:20 UTC (permalink / raw)
To: Richard Biener, GCC Patches, Kenneth Zadeck, Mike Stump,
Richard Sandiford
On Tue, May 6, 2014 at 10:11 AM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> On Mon, May 5, 2014 at 10:58 PM, Richard Sandiford
>> <rdsandiford@googlemail.com> wrote:
>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>> On Mon, May 5, 2014 at 4:51 PM, Richard Sandiford
>>>> <rdsandiford@googlemail.com> wrote:
>>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>>> On Mon, May 5, 2014 at 12:54 PM, Richard Sandiford
>>>>>> <rdsandiford@googlemail.com> wrote:
>>>>>>> Richard Biener <richard.guenther@gmail.com> writes:
>>>>>>>> On Fri, May 2, 2014 at 9:19 PM, Richard Sandiford
>>>>>>>> <rdsandiford@googlemail.com> wrote:
>>>>>>>>> I'd hoped the days of zero-precision INTEGER_CSTs were behind us after
>>>>>>>>> Richard's patch to remove min amd max values from zero-width bitfields,
>>>>>>>>> but a boostrap-ubsan showed otherwise. One source is in:
>>>>>>>>>
>>>>>>>>> null_pointer_node = build_int_cst (build_pointer_type
>>>>>>>>> (void_type_node), 0);
>>>>>>>>>
>>>>>>>>> if no_target, since the precision of the type comes from ptr_mode,
>>>>>>>>> which remains the default VOIDmode. Maybe that's a bug, but setting
>>>>>>>>> it to an arbitrary nonzero value would also be dangerous.
>>>>>>>>
>>>>>>>> Can you explain what 'no_target' should be? ptr_mode should never be
>>>>>>>> VOIDmode.
>>>>>>>
>>>>>>> Sorry, meant "no_backend" rather than "no_target". See do_compile.
>>>>>>
>>>>>> Ok. So we do
>>>>>>
>>>>>> /* This must be run always, because it is needed to compute the FP
>>>>>> predefined macros, such as __LDBL_MAX__, for targets using non
>>>>>> default FP formats. */
>>>>>> init_adjust_machine_modes ();
>>>>>>
>>>>>> /* Set up the back-end if requested. */
>>>>>> if (!no_backend)
>>>>>> backend_init ();
>>>>>>
>>>>>> where I think that init_adjust_machine_modes should initialize the
>>>>>> {byte,word,ptr}_mode globals. Move from init_emit_once.
>>>
>>> init_adjust_machine_modes is an auto-generated function so I ended up
>>> using a new function instead. Tested on x86_64-linux-gnu. OK to install?
>>
>> Ok with also setting double_mode there.
>
> double_mode is just a local variable though, it's only used to set up
> dconstN.
Ah, I see. Fine then as-is.
Thanks,
Richard.
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-05-06 8:20 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-02 19:19 [wide-int] Handle zero-precision INTEGER_CSTs again Richard Sandiford
2014-05-03 16:42 ` Kenneth Zadeck
2014-05-03 18:59 ` Richard Sandiford
2014-05-03 19:53 ` Kenneth Zadeck
2014-05-04 19:56 ` Richard Sandiford
2014-05-05 8:31 ` Richard Biener
2014-05-05 10:54 ` Richard Sandiford
2014-05-05 12:16 ` Richard Biener
2014-05-05 14:51 ` Richard Sandiford
2014-05-05 17:32 ` Richard Biener
2014-05-05 18:04 ` Marek Polacek
2014-05-05 20:58 ` Richard Sandiford
2014-05-06 8:08 ` Richard Biener
2014-05-06 8:11 ` Richard Sandiford
2014-05-06 8:20 ` Richard Biener
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).