On Thu, Apr 14, 2011 at 15:28, Jakub Jelinek <ja...@redhat.com> 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.  */

Reply via email to