On Thu, Apr 14, 2011 at 15:28, Jakub Jelinek <[email protected]> wrote:
> If bitpack_word_t has BITS_PER_BITPACK_WORD bits, then for
> nbits = BITS_PER_BITPACK_WORD this will be undefined.
> Use say
> mask = ((bitpack_word_t) 2 << (nbits - 1)) - 1;
> or something similar (assertion ensures that nbits isn't 0).
Quite right, thanks. In the meantime, I've changed my mind with this.
I think it's safer if we just assert that the value we are about to
pack fit in the number of bits the caller specified.
The only problematic user is pack_ts_type_value_fields when it tries
to pack a -1 for the type's alias set. I think we should just stream
that as an integer and not go through the bitpacking overhead.
For now, I'm applying this to the pph branch. Tested on x86_64. No
LTO failures.
Diego.
* lto-streamer-out.c (pack_ts_type_value_fields): Pack all bits
of -1 value.
* lto-streamer.h (bitpack_create): Assert that the value to
pack does not overflow NBITS.
* lto-streamer-in.c (unpack_ts_type_value_fields): Unpack
BITS_PER_BITPACK_WORD bits for TYPE_ALIAS_SET.
diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
index 97b86ce..f04e031 100644
--- a/gcc/lto-streamer-in.c
+++ b/gcc/lto-streamer-in.c
@@ -1734,7 +1734,7 @@ unpack_ts_type_value_fields (struct bitpack_d
*bp, tree expr)
TYPE_USER_ALIGN (expr) = (unsigned) bp_unpack_value (bp, 1);
TYPE_READONLY (expr) = (unsigned) bp_unpack_value (bp, 1);
TYPE_ALIGN (expr) = (unsigned) bp_unpack_value (bp, HOST_BITS_PER_INT);
- TYPE_ALIAS_SET (expr) = bp_unpack_value (bp, HOST_BITS_PER_INT);
+ TYPE_ALIAS_SET (expr) = bp_unpack_value (bp, BITS_PER_BITPACK_WORD);
}
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index 3ccad8b..89ad9c5 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -518,7 +518,8 @@ pack_ts_type_value_fields (struct bitpack_d *bp, tree expr)
bp_pack_value (bp, TYPE_USER_ALIGN (expr), 1);
bp_pack_value (bp, TYPE_READONLY (expr), 1);
bp_pack_value (bp, TYPE_ALIGN (expr), HOST_BITS_PER_INT);
- bp_pack_value (bp, TYPE_ALIAS_SET (expr) == 0 ? 0 : -1, HOST_BITS_PER_INT);
+ bp_pack_value (bp, TYPE_ALIAS_SET (expr) == 0 ? 0 : -1,
+ BITS_PER_BITPACK_WORD);
}
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index 0d49430..73afd46 100644
--- a/gcc/lto-streamer.h
+++ b/gcc/lto-streamer.h
@@ -1190,18 +1190,15 @@ bitpack_create (struct lto_output_stream *s)
static inline void
bp_pack_value (struct bitpack_d *bp, bitpack_word_t val, unsigned nbits)
{
- bitpack_word_t mask, word;
+ bitpack_word_t word = bp->word;
int pos = bp->pos;
- word = bp->word;
-
+ /* We shouldn't try to pack more bits than can fit in a bitpack word. */
gcc_assert (nbits > 0 && nbits <= BITS_PER_BITPACK_WORD);
- /* Make sure that VAL only has the lower NBITS set. Generate a
- mask with the lower NBITS set and use it to filter the upper
- bits from VAL. */
- mask = ((bitpack_word_t) 1 << nbits) - 1;
- val = val & mask;
+ /* The value to pack should not overflow NBITS. */
+ gcc_assert (nbits == BITS_PER_BITPACK_WORD
+ || val <= ((bitpack_word_t) 1 << nbits));
/* If val does not fit into the current bitpack word switch to the
next one. */
