Faster streaming of enums

Faster streaming of enums

[Jan Hubicka]

Hi,
after fixing 1 byte i/o function call and most of hash table overhead,
functions to handle ulebs and slebs shows top in profile.  We use them in
many cases where we know value range of the operand will fit in 1 byte. In
particular to handle enums.
This is also dangerous since we generally assume enums to be in their value
range.

This patch adds i/o bits for enums and integers in range that should inline
well and add some sanity checking.

I converted only tree streamer tags, but if accepted, I will convert more.

Bootstrapped/regtested x86_64-linux, OK?

	* lto-streamer-out.c (output_record_start): Use lto_output_enum
	(lto_output_tree): Use output_record_start.
	* lto-streamer-in.c (input_record_start): Use lto_input_enum
	(lto_get_pickled_tree): Use input_record_start.
	* lto-section-in.c (lto_section_overrun): Turn into fatal error.
	(lto_value_range_error): New function.
	* lto-streamer.h (lto_value_range_error): Declare.
	(lto_output_int_in_range, lto_input_int_in_range): New functions.
	(lto_output_enum, lto_input_enum): New macros.
Index: lto-streamer-out.c
===================================================================
*** lto-streamer-out.c	(revision 174175)
--- lto-streamer-out.c	(working copy)
*************** output_sleb128 (struct output_block *ob,
*** 270,281 ****
  
  /* Output the start of a record with TAG to output block OB.  */
  
! static void
  output_record_start (struct output_block *ob, enum LTO_tags tag)
  {
!   /* Make sure TAG fits inside an unsigned int.  */
!   gcc_assert (tag == (enum LTO_tags) (unsigned) tag);
!   output_uleb128 (ob, tag);
  }
  
  
--- 270,279 ----
  
  /* Output the start of a record with TAG to output block OB.  */
  
! static inline void
  output_record_start (struct output_block *ob, enum LTO_tags tag)
  {
!   lto_output_enum (ob->main_stream, LTO_tags, LTO_NUM_TAGS, tag);
  }
  
  
*************** lto_output_tree (struct output_block *ob
*** 1401,1407 ****
  	 will instantiate two different nodes for the same object.  */
        output_record_start (ob, LTO_tree_pickle_reference);
        output_uleb128 (ob, ix);
!       output_uleb128 (ob, lto_tree_code_to_tag (TREE_CODE (expr)));
      }
    else if (lto_stream_as_builtin_p (expr))
      {
--- 1399,1405 ----
  	 will instantiate two different nodes for the same object.  */
        output_record_start (ob, LTO_tree_pickle_reference);
        output_uleb128 (ob, ix);
!       output_record_start (ob, lto_tree_code_to_tag (TREE_CODE (expr)));
      }
    else if (lto_stream_as_builtin_p (expr))
      {
Index: lto-streamer-in.c
===================================================================
*** lto-streamer-in.c	(revision 174175)
--- lto-streamer-in.c	(working copy)
*************** lto_input_string (struct data_in *data_i
*** 231,241 ****
  
  /* Return the next tag in the input block IB.  */
  
! static enum LTO_tags
  input_record_start (struct lto_input_block *ib)
  {
!   enum LTO_tags tag = (enum LTO_tags) lto_input_uleb128 (ib);
!   return tag;
  }
  
  
--- 231,240 ----
  
  /* Return the next tag in the input block IB.  */
  
! static inline enum LTO_tags
  input_record_start (struct lto_input_block *ib)
  {
!   return lto_input_enum (ib, LTO_tags, LTO_NUM_TAGS);
  }
  
  
*************** lto_get_pickled_tree (struct lto_input_b
*** 2558,2564 ****
    enum LTO_tags expected_tag;
  
    ix = lto_input_uleb128 (ib);
!   expected_tag = (enum LTO_tags) lto_input_uleb128 (ib);
  
    result = lto_streamer_cache_get (data_in->reader_cache, ix);
    gcc_assert (result
--- 2557,2563 ----
    enum LTO_tags expected_tag;
  
    ix = lto_input_uleb128 (ib);
!   expected_tag = input_record_start (ib);
  
    result = lto_streamer_cache_get (data_in->reader_cache, ix);
    gcc_assert (result
Index: lto-section-in.c
===================================================================
*** lto-section-in.c	(revision 174175)
--- lto-section-in.c	(working copy)
*************** lto_get_function_in_decl_state (struct l
*** 483,488 ****
  void
  lto_section_overrun (struct lto_input_block *ib)
  {
!   internal_error ("bytecode stream: trying to read %d bytes "
! 	          "after the end of the input buffer", ib->p - ib->len);
  }
--- 483,498 ----
  void
  lto_section_overrun (struct lto_input_block *ib)
  {
!   fatal_error ("bytecode stream: trying to read %d bytes "
! 	       "after the end of the input buffer", ib->p - ib->len);
! }
! 
! /* Report out of range value.  */
! 
! void
! lto_value_range_error (const char *purpose, HOST_WIDE_INT val,
! 		       HOST_WIDE_INT min, HOST_WIDE_INT max)
! {
!   fatal_error ("%s out of range: Range is %i to %i, value is %i",
! 	       purpose, (int)min, (int)max, (int)val);
  }
Index: lto-streamer.h
===================================================================
*** lto-streamer.h	(revision 174175)
--- lto-streamer.h	(working copy)
*************** extern int lto_eq_in_decl_state (const v
*** 771,776 ****
--- 771,779 ----
  extern struct lto_in_decl_state *lto_get_function_in_decl_state (
  				      struct lto_file_decl_data *, tree);
  extern void lto_section_overrun (struct lto_input_block *) ATTRIBUTE_NORETURN;
+ extern void lto_value_range_error (const char *,
+ 				   HOST_WIDE_INT, HOST_WIDE_INT,
+ 				   HOST_WIDE_INT) ATTRIBUTE_NORETURN;
  
  /* In lto-section-out.c  */
  extern hashval_t lto_hash_decl_slot_node (const void *);
*************** lto_input_1_unsigned (struct lto_input_b
*** 1199,1202 ****
--- 1202,1267 ----
    return (ib->data[ib->p++]);
  }
  
+ /* Output VAL into OBS and verify it is in range MIN...MAX that is supposed
+    to be compile time constant.
+    Be host independent, limit range to 31bits.  */
+ 
+ static inline void
+ lto_output_int_in_range (struct lto_output_stream *obs,
+ 			 HOST_WIDE_INT min,
+ 			 HOST_WIDE_INT max,
+ 			 HOST_WIDE_INT val)
+ {
+   HOST_WIDE_INT range = max - min;
+ 
+   gcc_checking_assert (val >= min && val <= max && range > 0
+ 		       && range < 0x7fffffff);
+ 
+   val -= min;
+   lto_output_1_stream (obs, val & 255);
+   if (range >= 0xff)
+     lto_output_1_stream (obs, (val << 8) & 255);
+   if (range >= 0xffff)
+     lto_output_1_stream (obs, (val << 16) & 255);
+   if (range >= 0xffffff)
+     lto_output_1_stream (obs, (val << 24) & 255);
+ }
+ 
+ /* Input VAL into OBS and verify it is in range MIN...MAX that is supposed
+    to be compile time constant.  PURPOSE is used for error reporting.  */
+ 
+ static inline HOST_WIDE_INT
+ lto_input_int_in_range (struct lto_input_block *ib,
+ 			const char *purpose,
+ 			HOST_WIDE_INT min,
+ 			HOST_WIDE_INT max)
+ {
+   HOST_WIDE_INT range = max - min;
+   HOST_WIDE_INT val = lto_input_1_unsigned (ib);
+ 
+   gcc_checking_assert (range > 0);
+ 
+   if (range >= 0xff)
+     val |= ((HOST_WIDE_INT)lto_input_1_unsigned (ib)) << 8;
+   if (range >= 0xffff)
+     val |= ((HOST_WIDE_INT)lto_input_1_unsigned (ib)) << 16;
+   if (range >= 0xffffff)
+     val |= ((HOST_WIDE_INT)lto_input_1_unsigned (ib)) << 24;
+   val += min;
+   if (val < min || val > max)
+     lto_value_range_error (purpose, val, min, max);
+   return val;
+ }
+ 
+ /* Output VAL of type "enum enum_name" into OBS.
+    Assume range 0...ENUM_LAST - 1.  */
+ #define lto_output_enum(obs,enum_name,enum_last,val) \
+   lto_output_int_in_range ((obs), 0, (int)(enum_last) - 1, (int)(val))
+ 
+ /* Input enum of type "enum enum_name" from IB.
+    Assume range 0...ENUM_LAST - 1.  */
+ #define lto_input_enum(ib,enum_name,enum_last) \
+   (enum enum_name)lto_input_int_in_range ((ib), #enum_name, 0, \
+ 					  (int)(enum_last) - 1)
+ 
  #endif /* GCC_LTO_STREAMER_H  */

Re: Faster streaming of enums

[Richard Guenther]

On Wed, May 25, 2011 at 11:45 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> after fixing 1 byte i/o function call and most of hash table overhead,
> functions to handle ulebs and slebs shows top in profile.  We use them in
> many cases where we know value range of the operand will fit in 1 byte. In
> particular to handle enums.
> This is also dangerous since we generally assume enums to be in their value
> range.
>
> This patch adds i/o bits for enums and integers in range that should inline
> well and add some sanity checking.
>
> I converted only tree streamer tags, but if accepted, I will convert more.
>
> Bootstrapped/regtested x86_64-linux, OK?
>
>        * lto-streamer-out.c (output_record_start): Use lto_output_enum
>        (lto_output_tree): Use output_record_start.
>        * lto-streamer-in.c (input_record_start): Use lto_input_enum
>        (lto_get_pickled_tree): Use input_record_start.
>        * lto-section-in.c (lto_section_overrun): Turn into fatal error.
>        (lto_value_range_error): New function.
>        * lto-streamer.h (lto_value_range_error): Declare.
>        (lto_output_int_in_range, lto_input_int_in_range): New functions.
>        (lto_output_enum, lto_input_enum): New macros.
> Index: lto-streamer-out.c
> ===================================================================
> *** lto-streamer-out.c  (revision 174175)
> --- lto-streamer-out.c  (working copy)
> *************** output_sleb128 (struct output_block *ob,
> *** 270,281 ****
>
>  /* Output the start of a record with TAG to output block OB.  */
>
> ! static void
>  output_record_start (struct output_block *ob, enum LTO_tags tag)
>  {
> !   /* Make sure TAG fits inside an unsigned int.  */
> !   gcc_assert (tag == (enum LTO_tags) (unsigned) tag);
> !   output_uleb128 (ob, tag);
>  }
>
>
> --- 270,279 ----
>
>  /* Output the start of a record with TAG to output block OB.  */
>
> ! static inline void
>  output_record_start (struct output_block *ob, enum LTO_tags tag)
>  {
> !   lto_output_enum (ob->main_stream, LTO_tags, LTO_NUM_TAGS, tag);
>  }
>
>
> *************** lto_output_tree (struct output_block *ob
> *** 1401,1407 ****
>         will instantiate two different nodes for the same object.  */
>        output_record_start (ob, LTO_tree_pickle_reference);
>        output_uleb128 (ob, ix);
> !       output_uleb128 (ob, lto_tree_code_to_tag (TREE_CODE (expr)));
>      }
>    else if (lto_stream_as_builtin_p (expr))
>      {
> --- 1399,1405 ----
>         will instantiate two different nodes for the same object.  */
>        output_record_start (ob, LTO_tree_pickle_reference);
>        output_uleb128 (ob, ix);
> !       output_record_start (ob, lto_tree_code_to_tag (TREE_CODE (expr)));

I'd prefer lto_output_enum here as we don't really start a new output
record but just emit something for a sanity check.

>      }
>    else if (lto_stream_as_builtin_p (expr))
>      {
> Index: lto-streamer-in.c
> ===================================================================
> *** lto-streamer-in.c   (revision 174175)
> --- lto-streamer-in.c   (working copy)
> *************** lto_input_string (struct data_in *data_i
> *** 231,241 ****
>
>  /* Return the next tag in the input block IB.  */
>
> ! static enum LTO_tags
>  input_record_start (struct lto_input_block *ib)
>  {
> !   enum LTO_tags tag = (enum LTO_tags) lto_input_uleb128 (ib);
> !   return tag;
>  }
>
>
> --- 231,240 ----
>
>  /* Return the next tag in the input block IB.  */
>
> ! static inline enum LTO_tags
>  input_record_start (struct lto_input_block *ib)
>  {
> !   return lto_input_enum (ib, LTO_tags, LTO_NUM_TAGS);
>  }
>
>
> *************** lto_get_pickled_tree (struct lto_input_b
> *** 2558,2564 ****
>    enum LTO_tags expected_tag;
>
>    ix = lto_input_uleb128 (ib);
> !   expected_tag = (enum LTO_tags) lto_input_uleb128 (ib);
>
>    result = lto_streamer_cache_get (data_in->reader_cache, ix);
>    gcc_assert (result
> --- 2557,2563 ----
>    enum LTO_tags expected_tag;
>
>    ix = lto_input_uleb128 (ib);
> !   expected_tag = input_record_start (ib);

Likewise use input_enum.

>    result = lto_streamer_cache_get (data_in->reader_cache, ix);
>    gcc_assert (result
> Index: lto-section-in.c
> ===================================================================
> *** lto-section-in.c    (revision 174175)
> --- lto-section-in.c    (working copy)
> *************** lto_get_function_in_decl_state (struct l
> *** 483,488 ****
>  void
>  lto_section_overrun (struct lto_input_block *ib)
>  {
> !   internal_error ("bytecode stream: trying to read %d bytes "
> !                 "after the end of the input buffer", ib->p - ib->len);
>  }
> --- 483,498 ----
>  void
>  lto_section_overrun (struct lto_input_block *ib)
>  {
> !   fatal_error ("bytecode stream: trying to read %d bytes "
> !              "after the end of the input buffer", ib->p - ib->len);
> ! }
> !
> ! /* Report out of range value.  */
> !
> ! void
> ! lto_value_range_error (const char *purpose, HOST_WIDE_INT val,
> !                      HOST_WIDE_INT min, HOST_WIDE_INT max)
> ! {
> !   fatal_error ("%s out of range: Range is %i to %i, value is %i",
> !              purpose, (int)min, (int)max, (int)val);
>  }
> Index: lto-streamer.h
> ===================================================================
> *** lto-streamer.h      (revision 174175)
> --- lto-streamer.h      (working copy)
> *************** extern int lto_eq_in_decl_state (const v
> *** 771,776 ****
> --- 771,779 ----
>  extern struct lto_in_decl_state *lto_get_function_in_decl_state (
>                                      struct lto_file_decl_data *, tree);
>  extern void lto_section_overrun (struct lto_input_block *) ATTRIBUTE_NORETURN;
> + extern void lto_value_range_error (const char *,
> +                                  HOST_WIDE_INT, HOST_WIDE_INT,
> +                                  HOST_WIDE_INT) ATTRIBUTE_NORETURN;
>
>  /* In lto-section-out.c  */
>  extern hashval_t lto_hash_decl_slot_node (const void *);
> *************** lto_input_1_unsigned (struct lto_input_b
> *** 1199,1202 ****
> --- 1202,1267 ----
>    return (ib->data[ib->p++]);
>  }
>
> + /* Output VAL into OBS and verify it is in range MIN...MAX that is supposed
> +    to be compile time constant.
> +    Be host independent, limit range to 31bits.  */
> +
> + static inline void
> + lto_output_int_in_range (struct lto_output_stream *obs,
> +                        HOST_WIDE_INT min,
> +                        HOST_WIDE_INT max,
> +                        HOST_WIDE_INT val)
> + {
> +   HOST_WIDE_INT range = max - min;
> +
> +   gcc_checking_assert (val >= min && val <= max && range > 0
> +                      && range < 0x7fffffff);
> +
> +   val -= min;
> +   lto_output_1_stream (obs, val & 255);
> +   if (range >= 0xff)
> +     lto_output_1_stream (obs, (val << 8) & 255);
> +   if (range >= 0xffff)
> +     lto_output_1_stream (obs, (val << 16) & 255);
> +   if (range >= 0xffffff)
> +     lto_output_1_stream (obs, (val << 24) & 255);

so you didn't want to create a bitpack_pack_int_in_range and use
bitpacks for enums?  I suppose for some of the remaining cases
packing them into existing bitpacks would be preferable?

> + }
> +
> + /* Input VAL into OBS and verify it is in range MIN...MAX that is supposed
> +    to be compile time constant.  PURPOSE is used for error reporting.  */
> +
> + static inline HOST_WIDE_INT
> + lto_input_int_in_range (struct lto_input_block *ib,
> +                       const char *purpose,
> +                       HOST_WIDE_INT min,
> +                       HOST_WIDE_INT max)
> + {
> +   HOST_WIDE_INT range = max - min;
> +   HOST_WIDE_INT val = lto_input_1_unsigned (ib);
> +
> +   gcc_checking_assert (range > 0);

The assert doesn't match the one during output.

> +
> +   if (range >= 0xff)
> +     val |= ((HOST_WIDE_INT)lto_input_1_unsigned (ib)) << 8;
> +   if (range >= 0xffff)
> +     val |= ((HOST_WIDE_INT)lto_input_1_unsigned (ib)) << 16;
> +   if (range >= 0xffffff)
> +     val |= ((HOST_WIDE_INT)lto_input_1_unsigned (ib)) << 24;
> +   val += min;
> +   if (val < min || val > max)
> +     lto_value_range_error (purpose, val, min, max);
> +   return val;
> + }
> +
> + /* Output VAL of type "enum enum_name" into OBS.
> +    Assume range 0...ENUM_LAST - 1.  */
> + #define lto_output_enum(obs,enum_name,enum_last,val) \
> +   lto_output_int_in_range ((obs), 0, (int)(enum_last) - 1, (int)(val))
> +
> + /* Input enum of type "enum enum_name" from IB.
> +    Assume range 0...ENUM_LAST - 1.  */
> + #define lto_input_enum(ib,enum_name,enum_last) \
> +   (enum enum_name)lto_input_int_in_range ((ib), #enum_name, 0, \
> +                                         (int)(enum_last) - 1)
> +
>  #endif /* GCC_LTO_STREAMER_H  */
>

Re: Faster streaming of enums

[Jan Hubicka]

> > *************** lto_output_tree (struct output_block *ob
> > *** 1401,1407 ****
> >         will instantiate two different nodes for the same object.  */
> >        output_record_start (ob, LTO_tree_pickle_reference);
> >        output_uleb128 (ob, ix);
> > !       output_uleb128 (ob, lto_tree_code_to_tag (TREE_CODE (expr)));
> >      }
> >    else if (lto_stream_as_builtin_p (expr))
> >      {
> > --- 1399,1405 ----
> >         will instantiate two different nodes for the same object.  */
> >        output_record_start (ob, LTO_tree_pickle_reference);
> >        output_uleb128 (ob, ix);
> > !       output_record_start (ob, lto_tree_code_to_tag (TREE_CODE (expr)));
> 
> I'd prefer lto_output_enum here as we don't really start a new output
> record but just emit something for a sanity check.

OK, I wondered what is cleaner, will update the patch.
> > + /* Output VAL into OBS and verify it is in range MIN...MAX that is supposed
> > +    to be compile time constant.
> > +    Be host independent, limit range to 31bits.  */
> > +
> > + static inline void
> > + lto_output_int_in_range (struct lto_output_stream *obs,
> > +                        HOST_WIDE_INT min,
> > +                        HOST_WIDE_INT max,
> > +                        HOST_WIDE_INT val)
> > + {
> > +   HOST_WIDE_INT range = max - min;
> > +
> > +   gcc_checking_assert (val >= min && val <= max && range > 0
> > +                      && range < 0x7fffffff);
> > +
> > +   val -= min;
> > +   lto_output_1_stream (obs, val & 255);
> > +   if (range >= 0xff)
> > +     lto_output_1_stream (obs, (val << 8) & 255);
> > +   if (range >= 0xffff)
> > +     lto_output_1_stream (obs, (val << 16) & 255);
> > +   if (range >= 0xffffff)
> > +     lto_output_1_stream (obs, (val << 24) & 255);
> 
> so you didn't want to create a bitpack_pack_int_in_range and use
> bitpacks for enums?  I suppose for some of the remaining cases
> packing them into existing bitpacks would be preferable?

Well, in my TODO list is to have both.  Where we don't bitpatck enums with
other values (that is the most common case of enums) this way we produce less
overhead and have extra sanity check that the bits unused by enum are really 0.

I guess final API should have both lto_output_enum and lto_bitpack_output_enum.
I don't really care if the first have the implementation above or just creates its
own bitpack to handle the value.
> > + {
> > +   HOST_WIDE_INT range = max - min;
> > +   HOST_WIDE_INT val = lto_input_1_unsigned (ib);
> > +
> > +   gcc_checking_assert (range > 0);
> 
> The assert doesn't match the one during output.

Hmm, OK, will match.

Honza

Re: Faster streaming of enums

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2955 bytes --]

On Wed, 25 May 2011, Jan Hubicka wrote:

> > > *************** lto_output_tree (struct output_block *ob
> > > *** 1401,1407 ****
> > >         will instantiate two different nodes for the same object.  */
> > >        output_record_start (ob, LTO_tree_pickle_reference);
> > >        output_uleb128 (ob, ix);
> > > !       output_uleb128 (ob, lto_tree_code_to_tag (TREE_CODE (expr)));
> > >      }
> > >    else if (lto_stream_as_builtin_p (expr))
> > >      {
> > > --- 1399,1405 ----
> > >         will instantiate two different nodes for the same object.  */
> > >        output_record_start (ob, LTO_tree_pickle_reference);
> > >        output_uleb128 (ob, ix);
> > > !       output_record_start (ob, lto_tree_code_to_tag (TREE_CODE (expr)));
> > 
> > I'd prefer lto_output_enum here as we don't really start a new output
> > record but just emit something for a sanity check.
> 
> OK, I wondered what is cleaner, will update the patch.
> > > + /* Output VAL into OBS and verify it is in range MIN...MAX that is supposed
> > > +    to be compile time constant.
> > > +    Be host independent, limit range to 31bits.  */
> > > +
> > > + static inline void
> > > + lto_output_int_in_range (struct lto_output_stream *obs,
> > > +                        HOST_WIDE_INT min,
> > > +                        HOST_WIDE_INT max,
> > > +                        HOST_WIDE_INT val)
> > > + {
> > > +   HOST_WIDE_INT range = max - min;
> > > +
> > > +   gcc_checking_assert (val >= min && val <= max && range > 0
> > > +                      && range < 0x7fffffff);
> > > +
> > > +   val -= min;
> > > +   lto_output_1_stream (obs, val & 255);
> > > +   if (range >= 0xff)
> > > +     lto_output_1_stream (obs, (val << 8) & 255);
> > > +   if (range >= 0xffff)
> > > +     lto_output_1_stream (obs, (val << 16) & 255);
> > > +   if (range >= 0xffffff)
> > > +     lto_output_1_stream (obs, (val << 24) & 255);
> > 
> > so you didn't want to create a bitpack_pack_int_in_range and use
> > bitpacks for enums?  I suppose for some of the remaining cases
> > packing them into existing bitpacks would be preferable?
> 
> Well, in my TODO list is to have both.  Where we don't bitpatck enums with
> other values (that is the most common case of enums) this way we produce less
> overhead and have extra sanity check that the bits unused by enum are really 0.
> 
> I guess final API should have both lto_output_enum and lto_bitpack_output_enum.
> I don't really care if the first have the implementation above or just creates its
> own bitpack to handle the value.

Ok.

> > > + {
> > > +   HOST_WIDE_INT range = max - min;
> > > +   HOST_WIDE_INT val = lto_input_1_unsigned (ib);
> > > +
> > > +   gcc_checking_assert (range > 0);
> > 
> > The assert doesn't match the one during output.
> 
> Hmm, OK, will match.

Patch is ok with the changes.

Thanks,
Richard.

Re: Faster streaming of enums

[Jan Hubicka]

Hi,
this is updated patch.  For whatever reason we now end up with longer .o file
for tramp3d than with my prevoius attempt (by 9KB). We need at average 15 bytes
for location, well, the encoding of small ints might be better with uleb style,
perhaps with smaller chunk (like 4 bits per chunk as opposed to 8).
But that is easy followup.

Bootstrapping/regtesting x86_64-linux, OK if it passes?

	* lto-streamer-out.c (lto_string_index): break out from...; offset by 1
	so 0 means NULL string.
	(lto_output_string_with_length): ... here.
	(lto_output_string, output_string_cst, output_identifier): Update handling
	of NULL strings.
	(lto_output_location_bitpack): New function.
	(lto_output_location): Use it.
	(lto_output_tree_ref): Use output_record_start.
	* lto-streamer-in.c (string_for_index): Break out from ...; offset values by 1.
	(input_string_internal): ... here; 
	(input_string_cst, input_identifier, lto_input_string): Update handling of NULL strings.
	(lto_input_location_bitpack): New function
	(lto_input_location): Use it.
	* lto-streamer.h (bp_pack_small_unsigned, bp_pack_small_int, bp_unpack_small_unsigned,
	bp_unpack_small_int): New functions.
Index: lto-streamer-out.c
===================================================================
*** lto-streamer-out.c	(revision 174268)
--- lto-streamer-out.c	(working copy)
*************** destroy_output_block (struct output_bloc
*** 143,158 ****
    free (ob);
  }
  
! 
! /* Output STRING of LEN characters to the string
!    table in OB. The string might or might not include a trailing '\0'.
     Then put the index onto the INDEX_STREAM.  */
  
! void
! lto_output_string_with_length (struct output_block *ob,
! 			       struct lto_output_stream *index_stream,
! 			       const char *s,
! 			       unsigned int len)
  {
    struct string_slot **slot;
    struct string_slot s_slot;
--- 143,156 ----
    free (ob);
  }
  
! /* Return index used to reference STRING of LEN characters in the string table
!    in OB.  The string might or might not include a trailing '\0'.
     Then put the index onto the INDEX_STREAM.  */
  
! static unsigned
! lto_string_index (struct output_block *ob,
! 		  const char *s,
! 		  unsigned int len)
  {
    struct string_slot **slot;
    struct string_slot s_slot;
*************** lto_output_string_with_length (struct ou
*** 164,172 ****
    s_slot.len = len;
    s_slot.slot_num = 0;
  
-   /* Indicate that this is not a NULL string.  */
-   lto_output_uleb128_stream (index_stream, 0);
- 
    slot = (struct string_slot **) htab_find_slot (ob->string_hash_table,
  						 &s_slot, INSERT);
    if (*slot == NULL)
--- 162,167 ----
*************** lto_output_string_with_length (struct ou
*** 180,197 ****
        new_slot->len = len;
        new_slot->slot_num = start;
        *slot = new_slot;
-       lto_output_uleb128_stream (index_stream, start);
        lto_output_uleb128_stream (string_stream, len);
        lto_output_data_stream (string_stream, string, len);
      }
    else
      {
        struct string_slot *old_slot = *slot;
-       lto_output_uleb128_stream (index_stream, old_slot->slot_num);
        free (string);
      }
  }
  
  /* Output the '\0' terminated STRING to the string
     table in OB.  Then put the index onto the INDEX_STREAM.  */
  
--- 175,207 ----
        new_slot->len = len;
        new_slot->slot_num = start;
        *slot = new_slot;
        lto_output_uleb128_stream (string_stream, len);
        lto_output_data_stream (string_stream, string, len);
+       return start + 1;
      }
    else
      {
        struct string_slot *old_slot = *slot;
        free (string);
+       return old_slot->slot_num + 1;
      }
  }
  
+ 
+ /* Output STRING of LEN characters to the string
+    table in OB. The string might or might not include a trailing '\0'.
+    Then put the index onto the INDEX_STREAM.  */
+ 
+ void
+ lto_output_string_with_length (struct output_block *ob,
+ 			       struct lto_output_stream *index_stream,
+ 			       const char *s,
+ 			       unsigned int len)
+ {
+   lto_output_uleb128_stream (index_stream,
+ 			     lto_string_index (ob, s, len));
+ }
+ 
  /* Output the '\0' terminated STRING to the string
     table in OB.  Then put the index onto the INDEX_STREAM.  */
  
*************** lto_output_string (struct output_block *
*** 204,210 ****
      lto_output_string_with_length (ob, index_stream, string,
  				   strlen (string) + 1);
    else
!     lto_output_uleb128_stream (index_stream, 1);
  }
  
  
--- 214,220 ----
      lto_output_string_with_length (ob, index_stream, string,
  				   strlen (string) + 1);
    else
!     lto_output_1_stream (index_stream, 0);
  }
  
  
*************** output_string_cst (struct output_block *
*** 221,227 ****
  				   TREE_STRING_POINTER (string),
  				   TREE_STRING_LENGTH (string ));
    else
!     lto_output_uleb128_stream (index_stream, 1);
  }
  
  
--- 231,237 ----
  				   TREE_STRING_POINTER (string),
  				   TREE_STRING_LENGTH (string ));
    else
!     lto_output_1_stream (index_stream, 0);
  }
  
  
*************** output_identifier (struct output_block *
*** 238,246 ****
  				   IDENTIFIER_POINTER (id),
  				   IDENTIFIER_LENGTH (id));
    else
!     lto_output_uleb128_stream (index_stream, 1);
  }
  
  /* Write a zero to the output stream.  */
  
  static void
--- 248,257 ----
  				   IDENTIFIER_POINTER (id),
  				   IDENTIFIER_LENGTH (id));
    else
!     lto_output_1_stream (index_stream, 0);
  }
  
+ 
  /* Write a zero to the output stream.  */
  
  static void
*************** pack_value_fields (struct bitpack_d *bp,
*** 587,618 ****
  }
  
  
! /* Emit location LOC to output block OB.  */
  
! static void
! lto_output_location (struct output_block *ob, location_t loc)
  {
    expanded_location xloc;
  
    if (loc == UNKNOWN_LOCATION)
!     {
!       lto_output_string (ob, ob->main_stream, NULL);
!       return;
!     }
  
    xloc = expand_location (loc);
  
!   lto_output_string (ob, ob->main_stream, xloc.file);
!   output_sleb128 (ob, xloc.line);
!   output_sleb128 (ob, xloc.column);
!   output_sleb128 (ob, xloc.sysp);
! 
    ob->current_file = xloc.file;
    ob->current_line = xloc.line;
    ob->current_col = xloc.column;
  }
  
  
  /* Return true if tree node T is written to various tables.  For these
     nodes, we sometimes want to write their phyiscal representation
     (via lto_output_tree), and sometimes we need to emit an index
--- 598,652 ----
  }
  
  
! /* Output info about new location into bitpack BP.
!    After outputting bitpack, lto_output_location_data has
!    to be done to output actual data.  */
  
! static inline void
! lto_output_location_bitpack (struct bitpack_d *bp,
! 			     struct output_block *ob,
! 			     location_t loc)
  {
    expanded_location xloc;
  
+   bp_pack_value (bp, loc == UNKNOWN_LOCATION, 1);
    if (loc == UNKNOWN_LOCATION)
!     return;
  
    xloc = expand_location (loc);
  
!   bp_pack_value (bp, ob->current_file != xloc.file, 1);
!   if (ob->current_file != xloc.file)
!     bp_pack_small_unsigned (bp, lto_string_index (ob,
! 					          xloc.file,
! 						  strlen (xloc.file)))++;
    ob->current_file = xloc.file;
+ 
+   bp_pack_value (bp, ob->current_line != xloc.line, 1);
+   if (ob->current_line != xloc.line)
+     bp_pack_small_unsigned (bp, xloc.line)++;
    ob->current_line = xloc.line;
+ 
+   bp_pack_value (bp, ob->current_col != xloc.column, 1);
+   if (ob->current_col != xloc.column)
+     bp_pack_small_unsigned (bp, xloc.column)++;
    ob->current_col = xloc.column;
  }
  
  
+ /* Emit location LOC to output block OB.
+    When bitpack is handy, it is more space effecient to call
+    lto_output_location_bitpack with existing bitpack.  */
+ 
+ static void
+ lto_output_location (struct output_block *ob, location_t loc)
+ {
+   struct bitpack_d bp = bitpack_create (ob->main_stream);
+   lto_output_location_bitpack (&bp, ob, loc);
+   lto_output_bitpack (&bp);
+ }
+ 
+ 
  /* Return true if tree node T is written to various tables.  For these
     nodes, we sometimes want to write their phyiscal representation
     (via lto_output_tree), and sometimes we need to emit an index
*************** lto_output_tree_ref (struct output_block
*** 642,648 ****
  
    if (expr == NULL_TREE)
      {
!       output_zero (ob);
        return;
      }
  
--- 678,684 ----
  
    if (expr == NULL_TREE)
      {
!       output_record_start (ob, LTO_null);
        return;
      }
  
Index: lto-streamer-in.c
===================================================================
*** lto-streamer-in.c	(revision 174268)
--- lto-streamer-in.c	(working copy)
*************** eq_string_slot_node (const void *p1, con
*** 132,150 ****
     IB.  Write the length to RLEN.  */
  
  static const char *
! input_string_internal (struct data_in *data_in, struct lto_input_block *ib,
! 		       unsigned int *rlen)
  {
    struct lto_input_block str_tab;
    unsigned int len;
-   unsigned int loc;
    const char *result;
  
!   /* Read the location of the string from IB.  */
!   loc = lto_input_uleb128 (ib);
  
    /* Get the string stored at location LOC in DATA_IN->STRINGS.  */
!   LTO_INIT_INPUT_BLOCK (str_tab, data_in->strings, loc, data_in->strings_len);
    len = lto_input_uleb128 (&str_tab);
    *rlen = len;
  
--- 132,153 ----
     IB.  Write the length to RLEN.  */
  
  static const char *
! string_for_index (struct data_in *data_in,
! 		  unsigned int loc,
! 		  unsigned int *rlen)
  {
    struct lto_input_block str_tab;
    unsigned int len;
    const char *result;
  
!   if (!loc)
!     {
!       *rlen = 0;
!       return NULL;
!     }
  
    /* Get the string stored at location LOC in DATA_IN->STRINGS.  */
!   LTO_INIT_INPUT_BLOCK (str_tab, data_in->strings, loc - 1, data_in->strings_len);
    len = lto_input_uleb128 (&str_tab);
    *rlen = len;
  
*************** input_string_internal (struct data_in *d
*** 157,162 ****
--- 160,176 ----
  }
  
  
+ /* Read a string from the string table in DATA_IN using input block
+    IB.  Write the length to RLEN.  */
+ 
+ static const char *
+ input_string_internal (struct data_in *data_in, struct lto_input_block *ib,
+ 		       unsigned int *rlen)
+ {
+   return string_for_index (data_in, lto_input_uleb128 (ib), rlen);
+ }
+ 
+ 
  /* Read a STRING_CST from the string table in DATA_IN using input
     block IB.  */
  
*************** input_string_cst (struct data_in *data_i
*** 165,177 ****
  {
    unsigned int len;
    const char * ptr;
-   unsigned int is_null;
- 
-   is_null = lto_input_uleb128 (ib);
-   if (is_null)
-     return NULL;
  
    ptr = input_string_internal (data_in, ib, &len);
    return build_string (len, ptr);
  }
  
--- 179,188 ----
  {
    unsigned int len;
    const char * ptr;
  
    ptr = input_string_internal (data_in, ib, &len);
+   if (!ptr)
+     return NULL;
    return build_string (len, ptr);
  }
  
*************** input_identifier (struct data_in *data_i
*** 184,196 ****
  {
    unsigned int len;
    const char *ptr;
-   unsigned int is_null;
- 
-   is_null = lto_input_uleb128 (ib);
-   if (is_null)
-     return NULL;
  
    ptr = input_string_internal (data_in, ib, &len);
    return get_identifier_with_length (ptr, len);
  }
  
--- 195,204 ----
  {
    unsigned int len;
    const char *ptr;
  
    ptr = input_string_internal (data_in, ib, &len);
+   if (!ptr)
+     return NULL;
    return get_identifier_with_length (ptr, len);
  }
  
*************** lto_input_string (struct data_in *data_i
*** 215,227 ****
  {
    unsigned int len;
    const char *ptr;
-   unsigned int is_null;
- 
-   is_null = lto_input_uleb128 (ib);
-   if (is_null)
-     return NULL;
  
    ptr = input_string_internal (data_in, ib, &len);
    if (ptr[len - 1] != '\0')
      internal_error ("bytecode stream: found non-null terminated string");
  
--- 223,232 ----
  {
    unsigned int len;
    const char *ptr;
  
    ptr = input_string_internal (data_in, ib, &len);
+   if (!ptr)
+     return NULL;
    if (ptr[len - 1] != '\0')
      internal_error ("bytecode stream: found non-null terminated string");
  
*************** clear_line_info (struct data_in *data_in
*** 284,320 ****
  }
  
  
! /* Read a location from input block IB.  */
  
  static location_t
! lto_input_location (struct lto_input_block *ib, struct data_in *data_in)
  {
!   expanded_location xloc;
  
!   xloc.file = lto_input_string (data_in, ib);
!   if (xloc.file == NULL)
      return UNKNOWN_LOCATION;
  
!   xloc.file = canon_file_name (xloc.file);
!   xloc.line = lto_input_sleb128 (ib);
!   xloc.column = lto_input_sleb128 (ib);
!   xloc.sysp = lto_input_sleb128 (ib);
  
!   if (data_in->current_file != xloc.file)
      {
!       if (data_in->current_file)
  	linemap_add (line_table, LC_LEAVE, false, NULL, 0);
  
!       linemap_add (line_table, LC_ENTER, xloc.sysp, xloc.file, xloc.line);
      }
!   else if (data_in->current_line != xloc.line)
!     linemap_line_start (line_table, xloc.line, xloc.column);
  
-   data_in->current_file = xloc.file;
-   data_in->current_line = xloc.line;
-   data_in->current_col = xloc.column;
  
!   return linemap_position_for_column (line_table, xloc.column);
  }
  
  
--- 289,345 ----
  }
  
  
! /* Read a location bitpack from input block IB.  */
  
  static location_t
! lto_input_location_bitpack (struct data_in *data_in, struct bitpack_d *bp)
  {
!   bool file_change, line_change, column_change;
!   unsigned len;
!   bool prev_file = data_in->current_file != NULL;
  
!   if (bp_unpack_value (bp, 1))
      return UNKNOWN_LOCATION;
  
!   file_change = bp_unpack_value (bp, 1);
!   if (file_change)
!     data_in->current_file = canon_file_name
! 			      (string_for_index (data_in,
! 						 bp_unpack_small_unsigned (bp),
! 					         &len));
! 
!   line_change = bp_unpack_value (bp, 1);
!   if (line_change)
!     data_in->current_line = bp_unpack_small_unsigned (bp);
! 
!   column_change = bp_unpack_value (bp, 1);
!   if (column_change)
!     data_in->current_col = bp_unpack_small_unsigned (bp);
  
!   if (file_change)
      {
!       if (prev_file)
  	linemap_add (line_table, LC_LEAVE, false, NULL, 0);
  
!       linemap_add (line_table, LC_ENTER, false, data_in->current_file,
! 		   data_in->current_line);
      }
!   else if (line_change)
!     linemap_line_start (line_table, data_in->current_line, data_in->current_col);
! 
!   return linemap_position_for_column (line_table, data_in->current_col);
! }
  
  
! /* Read a location from input block IB.  */
! 
! static location_t
! lto_input_location (struct lto_input_block *ib, struct data_in *data_in)
! {
!   struct bitpack_d bp;
! 
!   bp = lto_input_bitpack (ib);
!   return lto_input_location_bitpack (data_in, &bp);
  }
  
  
Index: lto-streamer.h
===================================================================
*** lto-streamer.h	(revision 174268)
--- lto-streamer.h	(working copy)
*************** bp_unpack_value (struct bitpack_d *bp, u
*** 1175,1180 ****
--- 1175,1224 ----
  }
  
  
+ /* Pack VAL into BP, be more effetive when VAL is small.
+    Implementation is sily, we may want to do better later.  */
+ 
+ static inline void
+ bp_pack_small_unsigned (struct bitpack_d *bp, unsigned HOST_WIDE_INT val)
+ {
+   int nbits = floor_log2 (val) + 1;
+   gcc_checking_assert (nbits < 32);
+   bp_pack_value (bp, nbits, 5);
+   bp_pack_value (bp, val, nbits);
+ }
+ 
+ 
+ /* Pack VAL into BP, be more effetive when VAL is small.
+    Implementation is sily, we may want to do better later.  */
+ 
+ static inline void
+ bp_pack_small_int (struct bitpack_d *bp, HOST_WIDE_INT val)
+ {
+   bp_pack_value (bp, (val < 0), 1);
+   bp_pack_small_unsigned (bp, (val < 0) ? -val : val);
+ }
+ 
+ 
+ /* Pack VAL into BP, be more effetive when VAL is small.  */
+ 
+ static inline unsigned HOST_WIDE_INT
+ bp_unpack_small_unsigned (struct bitpack_d *bp)
+ {
+   return bp_unpack_value (bp, bp_unpack_value (bp, 5));
+ }
+ 
+ 
+ /* Pack VAL into BP, be more effetive when VAL is small.  */
+ 
+ static inline HOST_WIDE_INT
+ bp_unpack_small_int (struct bitpack_d *bp)
+ {
+   bool negative = bp_unpack_value (bp, 1);
+   HOST_WIDE_INT val = bp_unpack_value (bp, bp_unpack_value (bp, 5));
+   return negative ? -val:val;
+ }
+ 
+ 
  /* Write a character to the output block.  */
  
  static inline void

Re: Faster streaming of enums

[Tom Tromey]

>>>>> "Jan" == Jan Hubicka <hubicka@ucw.cz> writes:

Some typos...

Jan> + /* Pack VAL into BP, be more effetive when VAL is small.
Jan> +    Implementation is sily, we may want to do better later.  */

"effective" and "silly".

Jan> + /* Pack VAL into BP, be more effetive when VAL is small.
Jan> +    Implementation is sily, we may want to do better later.  */

Ditto.

Jan> + /* Pack VAL into BP, be more effetive when VAL is small.  */

"effective"

Jan> + /* Pack VAL into BP, be more effetive when VAL is small.  */

Ditto.

Tom