[pph] Stream and merge line table. (issue4836050)

IMPORTANT this patch goes on top of trunk patch issue4835047, which has yet to be approved (I included the change as part of this patch for pph for now).

We now stream out the line_table from the pph'ed header and merge it back in when compiling reading a pph.

All of the source_location's read from the pph are then applied an offset for them to match their expected source_location in the compiled file (except for builtins, we should not be streaming builtins, but as the FIXME comment in pph_write_location mentions, it appears we are; I will look at that in another patch).

This fixes the asm diff c1eabi1.cc and modifies the one in p4eabi1.cc (in the good compile the unique ID's go from $12, before this patch incorrectly from $185, and now incorrectly from $611). I will look into fixing this last asm diff, but I don't think it should hold this check-in.

Also, if we can omit the pph_trace or change the way the trace works, we could avoid calling pph_out/in_location only for it to call lto_input/output_location which calls back pph_read/write_location....

Added lto streamer hooks to do this, but Cary (ccoutant) was saying he'd want it directly in lto.

Do we want to apply the changes to libcpp to trunk now or wait??

Tested with bootstrap and pph regression testing.

Gab

2011-08-04  Gabriel Charette  <gchare@google.com>

	* decl.c (finish_function): Remove line 0 hack.

	* pph-streamer-in.c (pph_loc_offset): New global variable.
	(pph_read_location): New.
	(pph_in_linenum_type): New.
	(pph_in_source_location): New.
	(pph_in_line_map): New.
	(pph_in_and_merge_line_table): New.
	(pph_read_file_contents): Call pph_in_and_merge_line_table.
	* pph-streamer-out.c (pph_write_location): New.
	(pph_out_linenum_type): New.
	(pph_out_source_location): New.
	(pph_out_line_map): New.
	(pph_out_line_table): New.
	(pph_write_file_contents): Call pph_out_line_table.
	* pph-streamer.c (pph_hooks_init): Init input_location
	and output_location hooks.
	* pph-streamer.h (PPH_NUM_IGNORED_LINE_TABLE_ENTRIES): Define.

	* lto-streamer-in.c (lto_input_location): Handle input_location
	streamer hook.
	* lto-streamer-out.c (lto_output_location): Handle output_location
	streamer hook.
	* lto-streamer.h (streamer_hooks): Add input_location and
	output_location hooks.

	* include/line-map.h (LC_REASON_BIT): Define as CHAR_BIT.
	(COLUMN_BITS_BIT): Define as 8.
	(struct line_map): Use LC_REASON_BIT and COLUMN_BITS_BIT to represent
	field specific bit requirements.
	* line-map.c (linemap_ensure_extra_space_available): New.
	(linemap_add): Call linemap_ensure_extra_space_available.
	(linemap_line_start): Fixed missing space.


--
This patch is available for review at http://codereview.appspot.com/4836050

On 8/4/11, Gabriel Charette <gchare@google.com> wrote:
> IMPORTANT this patch goes on top of trunk patch issue4835047, which has yet
> to be approved (I included the change as part of this patch for pph for
> now).
>
> We now stream out the line_table from the pph'ed header and merge it back in
> when compiling reading a pph.
>
> All of the source_location's read from the pph are then applied an offset
> for them to match their expected source_location in the compiled file
> (except for builtins, we should not be streaming builtins, but as the FIXME
> comment in pph_write_location mentions, it appears we are; I will look at
> that in another patch).
>
> This fixes the asm diff c1eabi1.cc and modifies the one in p4eabi1.cc (in
> the good compile the unique ID's go from $12, before this patch incorrectly
> from $185, and now incorrectly from $611). I will look into fixing this last
> asm diff, but I don't think it should hold this check-in.

LGTM.

>
> Also, if we can omit the pph_trace or change the way the trace works, we
> could avoid calling pph_out/in_location only for it to call
> lto_input/output_location which calls back pph_read/write_location....
>
> Added lto streamer hooks to do this, but Cary (ccoutant) was saying he'd
> want it directly in lto.
>
> Do we want to apply the changes to libcpp to trunk now or wait??

How urgent is the merge?  It wouldn't hurt to run a few more test
cases.  I have some hidden in my client....

>
> Tested with bootstrap and pph regression testing.
>
> Gab
>
> 2011-08-04  Gabriel Charette  <gchare@google.com>
>
> 	* decl.c (finish_function): Remove line 0 hack.
>
> 	* pph-streamer-in.c (pph_loc_offset): New global variable.
> 	(pph_read_location): New.
> 	(pph_in_linenum_type): New.
> 	(pph_in_source_location): New.
> 	(pph_in_line_map): New.
> 	(pph_in_and_merge_line_table): New.
> 	(pph_read_file_contents): Call pph_in_and_merge_line_table.
> 	* pph-streamer-out.c (pph_write_location): New.
> 	(pph_out_linenum_type): New.
> 	(pph_out_source_location): New.
> 	(pph_out_line_map): New.
> 	(pph_out_line_table): New.
> 	(pph_write_file_contents): Call pph_out_line_table.
> 	* pph-streamer.c (pph_hooks_init): Init input_location
> 	and output_location hooks.
> 	* pph-streamer.h (PPH_NUM_IGNORED_LINE_TABLE_ENTRIES): Define.
>
> 	* lto-streamer-in.c (lto_input_location): Handle input_location
> 	streamer hook.
> 	* lto-streamer-out.c (lto_output_location): Handle output_location
> 	streamer hook.
> 	* lto-streamer.h (streamer_hooks): Add input_location and
> 	output_location hooks.
>
> 	* include/line-map.h (LC_REASON_BIT): Define as CHAR_BIT.
> 	(COLUMN_BITS_BIT): Define as 8.
> 	(struct line_map): Use LC_REASON_BIT and COLUMN_BITS_BIT to represent
> 	field specific bit requirements.
> 	* line-map.c (linemap_ensure_extra_space_available): New.
> 	(linemap_add): Call linemap_ensure_extra_space_available.
> 	(linemap_line_start): Fixed missing space.
>
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index 3b312d3..8097cd3 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -13232,22 +13232,13 @@ finish_function (int flags)
>      {
>        if (DECL_MAIN_P (current_function_decl))
>  	{
> -	  tree stmt;
> -
>  	  /* Make it so that `main' always returns 0 by default (or
>  	     1 for VMS).  */
>  #if VMS_TARGET
> -	  stmt = finish_return_stmt (integer_one_node);
> +	  finish_return_stmt (integer_one_node);
>  #else
> -	  stmt = finish_return_stmt (integer_zero_node);
> +	  finish_return_stmt (integer_zero_node);
>  #endif
> -	  /* Hack.  We don't want the middle-end to warn that this
> -	     return is unreachable, so put the statement on the
> -	     special line 0.  */
> -	  {
> -	    location_t linezero = linemap_line_start (line_table, 0, 1);
> -	    SET_EXPR_LOCATION (stmt, linezero);
> -	  }
>  	}
>
>        if (use_eh_spec_block (current_function_decl))
> diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
> index e57e1e7..6e2e79b 100644
> --- a/gcc/cp/pph-streamer-in.c
> +++ b/gcc/cp/pph-streamer-in.c
> @@ -68,6 +68,10 @@ static VEC(char_p,heap) *string_tables = NULL;
>        pph_register_shared_data (STREAM, ALT_DATA, IX);			\
>      } while (0)
>
> +/* Set in pph_in_and_merge_line_table. Represents the source_location
> offset
> +   which every streamed in token must add to it's serialized
> source_location. */
> +int pph_loc_offset;
> +
>  /* Callback for unpacking value fields in ASTs.  BP is the bitpack
>     we are unpacking from.  EXPR is the tree to unpack.  */
>
> @@ -238,6 +242,32 @@ pph_register_shared_data (pph_stream *stream, void
> *data, unsigned ix)
>  }
>
>
> +/* Callback for streamer_hooks.input_location.  An offset is applied to
> +   the location_t read in according to the properties of the merged
> +   line_table.  IB and DATA_IN are as in lto_input_location.  This function
> +   should only be called after pph_in_and_merge_line_table was called as
> +   we expect pph_loc_offset to be set.  */
> +
> +location_t
> +pph_read_location (struct lto_input_block *ib,
> +                   struct data_in *data_in ATTRIBUTE_UNUSED)
> +{
> +  struct bitpack_d bp;
> +  bool is_builtin;
> +  unsigned HOST_WIDE_INT n;
> +  location_t old_loc;
> +
> +  bp = lto_input_bitpack (ib);
> +  is_builtin = bp_unpack_value (&bp, 1);
> +
> +  n = lto_input_uleb128 (ib);
> +  old_loc = (location_t) n;
> +  gcc_assert (old_loc == n);
> +
> +  return is_builtin ? old_loc : old_loc + pph_loc_offset;
> +}
> +
> +
>  /* Given a type index TYPE_IDX and TYPE_KIND specifying the kind of type,
>     return a type from integer_types or global_trees.  */
>
> @@ -1412,6 +1442,106 @@ pph_in_includes (pph_stream *stream)
>  }
>
>
> +/* Read a linenum_type from STREAM.  */
> +
> +static inline linenum_type
> +pph_in_linenum_type (pph_stream *stream)
> +{
> +  return (linenum_type) pph_in_uint (stream);
> +}
> +
> +
> +/* Read a source_location from STREAM.  */
> +
> +static inline source_location
> +pph_in_source_location (pph_stream *stream)
> +{
> +  return (source_location) pph_in_uint (stream);
> +}
> +
> +
> +/* Read a line_map from STREAM into LM.  */
> +
> +static void
> +pph_in_line_map (pph_stream *stream, struct line_map *lm)
> +{
> +  struct bitpack_d bp;
> +
> +  lm->to_file = pph_in_string (stream);
> +  lm->to_line = pph_in_linenum_type (stream);
> +  lm->start_location = pph_in_source_location (stream);
> +  lm->included_from = (int) pph_in_uint (stream);
> +
> +  bp = pph_in_bitpack (stream);
> +  lm->reason = (enum lc_reason) bp_unpack_value (&bp, LC_REASON_BIT);
> +  gcc_assert (lm->reason == LC_ENTER
> +              || lm->reason == LC_LEAVE
> +              || lm->reason == LC_RENAME
> +              || lm->reason == LC_RENAME_VERBATIM);
> +  lm->sysp = (unsigned char) bp_unpack_value (&bp, CHAR_BIT);
> +  lm->column_bits = bp_unpack_value (&bp, COLUMN_BITS_BIT);
> +}
> +
> +
> +/* Read the line_table from STREAM and merge it in LINETAB.  */
> +
> +static void
> +pph_in_and_merge_line_table (pph_stream *stream, struct line_maps *linetab)
> +{
> +  unsigned int ix, pph_used, old_depth;
> +  int entries_offset = linetab->used - PPH_NUM_IGNORED_LINE_TABLE_ENTRIES;
> +
> +  pph_used = pph_in_uint (stream);
> +
> +  for (ix = 0; ix < pph_used; ix++, linetab->used++)
> +    {
> +      struct line_map *lm;
> +
> +      linemap_ensure_extra_space_available (linetab);
> +
> +      lm = &linetab->maps[linetab->used];
> +
> +      pph_in_line_map (stream, lm);
> +
> +      if (ix == 0)
> +        {
> +          pph_loc_offset = (linetab->highest_location + 1) -
> lm->start_location;
> +
> +          /* When parsing the pph the header itself wasn't included by
> anything,
> +             now it's included by the C file we are currently compiling.
> */
> +          gcc_assert(lm->included_from == -1);
> +          lm->included_from = linetab->used - 1;
> +        }
> +
> +      /* For the other entries in the pph's line_table which were
> included_from
> +         another entry, reflect their included_from to the new position of
> the
> +         entry which they were included from.  */
> +      else if (lm->included_from != -1)
> +        lm->included_from += entries_offset;
> +
> +      gcc_assert (lm->included_from < (int) linetab->used);
> +
> +      lm->start_location += pph_loc_offset;
> +    }
> +
> +  linetab->highest_location = pph_loc_offset + pph_in_uint (stream);
> +  linetab->highest_line = pph_loc_offset + pph_in_uint (stream);
> +
> +  /* The MAX_COLUMN_HINT can be directly overwritten.  */
> +  linetab->max_column_hint = pph_in_uint (stream);
> +
> +  /* The line_table doesn't store the last LC_LEAVE in any given
> compilation;
> +     thus we need to replay the LC_LEAVE for the header now.  For that same
> +     reason, the line_table should currently be in a state representing a
> depth
> +     one include deeper then the depth at which this pph was included.  The
> +     LC_LEAVE replay will then bring the depth back to what it was before
> +     calling this function.  */
> +  old_depth = linetab->depth++;
> +  linemap_add (linetab, LC_LEAVE, 0, NULL, 0);
> +  gcc_assert (linetab->depth == old_depth);
> +}
> +
> +
>  /* Read contents of PPH file in STREAM.  */
>
>  static void
> @@ -1441,6 +1571,9 @@ pph_read_file_contents (pph_stream *stream)
>    /* Re-instantiate all the pre-processor symbols defined by STREAM.  */
>    cpp_lt_replay (parse_in, &idents_used);
>
> +  /* Read in the pph's line table and merge it in the current line table.
> */
> +  pph_in_and_merge_line_table (stream, line_table);
> +
>    /* Read the bindings from STREAM and merge them with the current
> bindings.  */
>    pph_in_scope_chain (stream);
>
> diff --git a/gcc/cp/pph-streamer-out.c b/gcc/cp/pph-streamer-out.c
> index 5090a40..b54adc4 100644
> --- a/gcc/cp/pph-streamer-out.c
> +++ b/gcc/cp/pph-streamer-out.c
> @@ -122,6 +122,34 @@ pph_end_section (void)
>  }
>
>
> +/* Callback for streamer_hooks.output_location.  Output the LOC directly,
> +   an offset will be applied on input after rebuilding the line_table.
> +   OB and LOC are as in lto_output_location.  */
> +
> +void
> +pph_write_location (struct output_block *ob, location_t loc)
> +{
> +  /* FIXME pph: we are streaming builtin locations, which implies that we
> are
> +     streaming some builtins, we probably want to figure out what those are
> and
> +     simply add them to the cache in the preload.  */
> +  struct bitpack_d bp;
> +  location_t highest_builtin_loc = line_table->maps[2].start_location - 1;
> +
> +  bp = bitpack_create (ob->main_stream);
> +  if (loc < highest_builtin_loc)
> +    bp_pack_value (&bp, true, 1);
> +  else
> +    {
> +      gcc_assert (loc >=
> +
> line_table->maps[PPH_NUM_IGNORED_LINE_TABLE_ENTRIES].start_location);
> +      bp_pack_value (&bp, false, 1);
> +    }
> +
> +  lto_output_bitpack (&bp);
> +  lto_output_sleb128_stream (ob->main_stream, loc);
> +}
> +
> +
>  /* Write the header for the PPH file represented by STREAM.  */
>
>  static void
> @@ -1218,6 +1246,87 @@ pph_out_includes (pph_stream *stream)
>  }
>
>
> +/* Emit linenum_type LN to STREAM.  */
> +
> +static inline void
> +pph_out_linenum_type (pph_stream *stream, linenum_type ln)
> +{
> +  pph_out_uint (stream, ln);
> +}
> +
> +
> +/* Emit source_location SL to STREAM.  */
> +
> +static inline void
> +pph_out_source_location (pph_stream *stream, source_location sl)
> +{
> +  pph_out_uint (stream, sl);
> +}
> +
> +
> +/* Emit all information contained in LM to STREAM.  */
> +
> +static void
> +pph_out_line_map (pph_stream *stream, struct line_map *lm)
> +{
> +  struct bitpack_d bp;
> +
> +  pph_out_string (stream, lm->to_file);
> +  pph_out_linenum_type (stream, lm->to_line);
> +  pph_out_source_location (stream, lm->start_location);
> +  pph_out_uint (stream, (unsigned int) lm->included_from);
> +
> +  bp = bitpack_create (stream->encoder.w.ob->main_stream);
> +  bp_pack_value (&bp, lm->reason, CHAR_BIT);
> +  bp_pack_value (&bp, lm->sysp, CHAR_BIT);
> +  bp_pack_value (&bp, lm->column_bits, COLUMN_BITS_BIT);
> +  pph_out_bitpack (stream, &bp);
> +}
> +
> +
> +/* Emit the required line_map entry and some properties in LINETAB to
> STREAM,
> +   ignoring builtin and command-line entries.  */
> +
> +static void
> +pph_out_line_table (pph_stream *stream, struct line_maps *linetab)
> +{
> +  unsigned int ix;
> +
> +  /* Any #include should have been fully parsed and exited at this point.
> */
> +  gcc_assert (linetab->depth == 0);
> +
> +  /* Only output used as the number of line_map entries we actually output.
>  */
> +  pph_out_uint (stream, linetab->used -
> PPH_NUM_IGNORED_LINE_TABLE_ENTRIES);
> +
> +  /* Output all the non-ignored line_map entries.
> +     Special casing the first one.  */
> +  for (ix = PPH_NUM_IGNORED_LINE_TABLE_ENTRIES; ix < linetab->used; ix++)
> +    {
> +      struct line_map *lm = &linetab->maps[ix];
> +
> +      if (ix == PPH_NUM_IGNORED_LINE_TABLE_ENTRIES)
> +        {
> +          /* The first non-ignored entry should be an LC_RENAME back in the
> +            header after inserting the builtin and command-line entries.
> When
> +            reading the pph we want this to be a simple LC_ENTER as the
> builtin
> +            and command_line entries will already exist and we are now
> entering
> +	    a #include.  */
> +          gcc_assert (lm->reason == LC_RENAME);
> +          lm->reason = LC_ENTER;
> +        }
> +
> +      pph_out_line_map (stream, lm);
> +
> +      if (ix == PPH_NUM_IGNORED_LINE_TABLE_ENTRIES)
> +        lm->reason = LC_RENAME;
> +    }
> +
> +  pph_out_source_location (stream, linetab->highest_location);
> +  pph_out_source_location (stream, linetab->highest_line);
> +
> +  pph_out_uint (stream, linetab->max_column_hint);
> +}
> +
>  /* Write PPH output symbols and IDENTS_USED to STREAM as an object.  */
>
>  static void
> @@ -1231,6 +1340,9 @@ pph_write_file_contents (pph_stream *stream,
> cpp_idents_used *idents_used)
>       namespace.  */
>    pph_out_identifiers (stream, idents_used);
>
> +  /* Emit the line table entries.  */
> +  pph_out_line_table (stream, line_table);
> +
>    /* Emit the bindings for the global namespace.  */
>    pph_out_scope_chain (stream, scope_chain);
>    if (flag_pph_dump_tree)
> diff --git a/gcc/cp/pph-streamer.c b/gcc/cp/pph-streamer.c
> index a7a59a9..7805dd8 100644
> --- a/gcc/cp/pph-streamer.c
> +++ b/gcc/cp/pph-streamer.c
> @@ -112,6 +112,8 @@ pph_hooks_init (void)
>    streamer_hooks.unpack_value_fields = pph_unpack_value_fields;
>    streamer_hooks.alloc_tree = pph_alloc_tree;
>    streamer_hooks.output_tree_header = pph_out_tree_header;
> +  streamer_hooks.input_location = pph_read_location;
> +  streamer_hooks.output_location = pph_write_location;
>    streamer_hooks.has_unique_integer_csts_p = true;
>  }
>
> diff --git a/gcc/cp/pph-streamer.h b/gcc/cp/pph-streamer.h
> index 0f29c31..ebdcc59 100644
> --- a/gcc/cp/pph-streamer.h
> +++ b/gcc/cp/pph-streamer.h
> @@ -53,6 +53,14 @@ enum pph_symtab_marker {
>     exactly 8 bytes in the file.  */
>  static const char pph_id_str[] = "PPH0x42";
>
> +/* When streaming out the line_table we will ignore the first 3 entries.
> +   The first one is the entrance in the header, the second one is for
> +   builtins, the third one is the command line, the fourth one is finally
> +   the LC_RENAME back to the header file, we want to stream out starting at
> +   that one, changing it's reason to LC_ENTER (as we ignored the original
> +   entrance), and then streaming every other entry as is from that point
> on.  */
> +#define PPH_NUM_IGNORED_LINE_TABLE_ENTRIES 3
> +
>  /* Structure of the header of a PPH file.  */
>  typedef struct pph_file_header {
>    /* Identification string.  */
> @@ -195,6 +203,7 @@ void pph_add_decl_to_symtab (tree);
>  void pph_add_include (pph_stream *);
>  void pph_writer_init (void);
>  void pph_writer_finish (void);
> +void pph_write_location (struct output_block *, location_t);
>
>  /* In name-lookup.c.  */
>  struct binding_table_s;
> @@ -207,6 +216,7 @@ void pph_read_tree (struct lto_input_block *, struct
> data_in *, tree);
>  void pph_unpack_value_fields (struct bitpack_d *, tree);
>  tree pph_alloc_tree (enum tree_code, struct lto_input_block *,
>  	             struct data_in *);
> +location_t pph_read_location (struct lto_input_block *, struct data_in *);
>  void pph_read_file (const char *);
>
>  /* In pt.c.  */
> @@ -328,7 +338,10 @@ pph_out_tree_VEC (pph_stream *stream, VEC(tree,gc) *v)
>  }
>  #endif
>
> -/* Write location LOC of length to STREAM.  */
> +/* Write location LOC of length to STREAM.
> +   FIXME pph: If pph_trace didn't depend on STREAM, we could avoid having
> to
> +   call this function, only for it to call lto_output_location, which calls
> the
> +   streamer hook back to pph_write_location.  */
>  static inline void
>  pph_out_location (pph_stream *stream, location_t loc)
>  {
> @@ -399,7 +412,11 @@ pph_in_string (pph_stream *stream)
>    return s;
>  }
>
> -/* Read and return a location_t from STREAM.  */
> +/* Read and return a location_t from STREAM.
> +   FIXME pph: If pph_trace didn't depend on STREAM, we could avoid having
> to
> +   call this function, only for it to call lto_input_location, which calls
> the
> +   streamer hook back to pph_read_location.  */
> +
>  static inline location_t
>  pph_in_location (pph_stream *stream)
>  {
> diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c
> index 1b74cb1..aec32bc 100644
> --- a/gcc/lto-streamer-in.c
> +++ b/gcc/lto-streamer-in.c
> @@ -331,15 +331,23 @@ lto_input_location_bitpack (struct data_in *data_in,
> struct bitpack_d *bp)
>  }
>
>
> -/* Read a location from input block IB.  */
> +/* Read a location from input block IB.
> +   If the input_location streamer hook exists, call it.
> +   Otherwise, proceed with reading the location from the
> +   expanded location bitpack.  */
>
>  location_t
>  lto_input_location (struct lto_input_block *ib, struct data_in *data_in)
>  {
> -  struct bitpack_d bp;
> +  if(streamer_hooks.input_location)
> +    return streamer_hooks.input_location (ib, data_in);
> +  else
> +    {
> +      struct bitpack_d bp;
>
> -  bp = lto_input_bitpack (ib);
> -  return lto_input_location_bitpack (data_in, &bp);
> +      bp = lto_input_bitpack (ib);
> +      return lto_input_location_bitpack (data_in, &bp);
> +    }
>  }
>
>
> diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
> index 3e6f303..e584864 100644
> --- a/gcc/lto-streamer-out.c
> +++ b/gcc/lto-streamer-out.c
> @@ -641,15 +641,21 @@ lto_output_location_bitpack (struct bitpack_d *bp,
>
>
>  /* Emit location LOC to output block OB.
> -   When bitpack is handy, it is more space effecient to call
> +   If the output_location streamer hook exists, call it.
> +   Otherwise, when bitpack is handy, it is more space efficient to call
>     lto_output_location_bitpack with existing bitpack.  */
>
>  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);
> +  if (streamer_hooks.output_location)
> +    streamer_hooks.output_location (ob, loc);
> +  else
> +    {
> +      struct bitpack_d bp = bitpack_create (ob->main_stream);
> +      lto_output_location_bitpack (&bp, ob, loc);
> +      lto_output_bitpack (&bp);
> +    }
>  }
>
>
> diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
> index 7e65989..ac2878b 100644
> --- a/gcc/lto-streamer.h
> +++ b/gcc/lto-streamer.h
> @@ -832,6 +832,16 @@ struct streamer_hooks {
>       the number of arguments to the CALL_EXPR).  */
>    void (*output_tree_header) (struct output_block *, tree);
>
> +  /* [OPT] Called by lto_input_location to retrieve the source location of
> the
> +     tree currently being read. If this hook returns NULL,
> lto_input_location
> +     defaults to calling lto_input_location_bitpack.  */
> +  location_t (*input_location) (struct lto_input_block *, struct data_in
> *);
> +
> +  /* [OPT] Called by lto_output_location to write the source_location of
> the
> +     tree currently being written. If this hook returns NULL,
> +     lto_output_location defaults to calling lto_output_location_bitpack.
> */
> +  void (*output_location) (struct output_block *, location_t);
> +
>    /* [OPT] Non-zero if the streamer has special constants that cannot be
>       shared and are used in pointer-equality tests (e.g., void_zero_node,
>       truthvalue_false_node, etc).  These constants will be present in
> diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
> index 3234423..657e47f 100644
> --- a/libcpp/include/line-map.h
> +++ b/libcpp/include/line-map.h
> @@ -44,6 +44,9 @@ typedef unsigned int source_location;
>  /* Memory allocation function typedef.  Works like xrealloc.  */
>  typedef void *(*line_map_realloc) (void *, size_t);
>
> +#define LC_REASON_BIT CHAR_BIT
> +#define COLUMN_BITS_BIT 8
> +
>  /* Physical source file TO_FILE at line TO_LINE at column 0 is represented
>     by the logical START_LOCATION.  TO_LINE+L at column C is represented by
>     START_LOCATION+(L*(1<<column_bits))+C, as long as C<(1<<column_bits),
> @@ -61,11 +64,11 @@ struct GTY(()) line_map {
>    linenum_type to_line;
>    source_location start_location;
>    int included_from;
> -  ENUM_BITFIELD (lc_reason) reason : CHAR_BIT;
> +  ENUM_BITFIELD (lc_reason) reason : LC_REASON_BIT;
>    /* The sysp field isn't really needed now that it's in cpp_buffer.  */
>    unsigned char sysp;
>    /* Number of the low-order source_location bits used for a column number.
>  */
> -  unsigned int column_bits : 8;
> +  unsigned int column_bits : COLUMN_BITS_BIT;
>  };
>
>  /* A set of chronological line_map structures.  */
> @@ -190,4 +193,24 @@ extern const struct line_map *linemap_lookup
>  extern source_location
>  linemap_position_for_column (struct line_maps *set, unsigned int
> to_column);
>
> +/* Makes sure SET has at least one more unused line_map allocated.
> +   If it doesn't, this function allocates more room in SET.  */
> +
> +static inline void
> +linemap_ensure_extra_space_available (struct line_maps *set)
> +{
> +  if (set->used == set->allocated)
> +    {
> +      line_map_realloc reallocator;
> +      reallocator  = set->reallocator ? set->reallocator : xrealloc;
> +
> +      set->allocated = 2 * set->allocated + 256;
> +      set->maps = (struct line_map *) (*reallocator) (set->maps,
> +                               set->allocated * sizeof (struct line_map));
> +
> +      memset (&set->maps[set->used], 0,
> +              (set->allocated - set->used) * sizeof (struct line_map));
> +    }
> +}
> +
>  #endif /* !LIBCPP_LINE_MAP_H  */
> diff --git a/libcpp/line-map.c b/libcpp/line-map.c
> index 86e2484..01ed7b5 100644
> --- a/libcpp/line-map.c
> +++ b/libcpp/line-map.c
> @@ -94,18 +94,7 @@ linemap_add (struct line_maps *set, enum lc_reason
> reason,
>    if (set->used && start_location < set->maps[set->used -
> 1].start_location)
>      abort ();
>
> -  if (set->used == set->allocated)
> -    {
> -      line_map_realloc reallocator
> -	= set->reallocator ? set->reallocator : xrealloc;
> -      set->allocated = 2 * set->allocated + 256;
> -      set->maps
> -	= (struct line_map *) (*reallocator) (set->maps,
> -					      set->allocated
> -					      * sizeof (struct line_map));
> -      memset (&set->maps[set->used], 0, ((set->allocated - set->used)
> -					 * sizeof (struct line_map)));
> -    }
> +  linemap_ensure_extra_space_available (set);
>
>    map = &set->maps[set->used];
>
> @@ -212,7 +201,7 @@ linemap_line_start (struct line_maps *set, linenum_type
> to_line,
>  	  /* If the column number is ridiculous or we've allocated a huge
>  	     number of source_locations, give up on column numbers. */
>  	  max_column_hint = 0;
> -	  if (highest >0xF0000000)
> +	  if (highest > 0xF0000000)
>  	    return 0;
>  	  column_bits = 0;
>  	}
>
> --
> This patch is available for review at http://codereview.appspot.com/4836050
>

[+ccoutant]

>> Added lto streamer hooks to do this, but Cary (ccoutant) was saying he'd
>> want it directly in lto.
>>
>> Do we want to apply the changes to libcpp to trunk now or wait??
>
> How urgent is the merge?  It wouldn't hurt to run a few more test
> cases.  I have some hidden in my client....


Neither the merge to lto (Cary said he'd like that), nor the libcpp
merge to trunk are required (libcpp changes are trivial, just pulling
out some inline code into a function for re-use and defined some
constants to avoid "magic numbers" in the code, but there is not much
point for them without the pph code (i.e. they'll only be used once
each, where they were originally pulled out from)).

Gab
We can definitely keep it in pph for a little while until it's stable.

Hello Gabriel,

gchare@google.com (Gabriel Charette) a écrit:

[...]

> Do we want to apply the changes to libcpp to trunk now or wait??

[...]


> 2011-08-04  Gabriel Charette  <gchare@google.com>

[...]

> 	* include/line-map.h (LC_REASON_BIT): Define as CHAR_BIT.
> 	(COLUMN_BITS_BIT): Define as 8.
> 	(struct line_map): Use LC_REASON_BIT and COLUMN_BITS_BIT to represent
> 	field specific bit requirements.
> 	* line-map.c (linemap_ensure_extra_space_available): New.
> 	(linemap_add): Call linemap_ensure_extra_space_available.
> 	(linemap_line_start): Fixed missing space.

[...]

> diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h
> index 3234423..657e47f 100644
> --- a/libcpp/include/line-map.h
> +++ b/libcpp/include/line-map.h
> @@ -44,6 +44,9 @@ typedef unsigned int source_location;
>  /* Memory allocation function typedef.  Works like xrealloc.  */
>  typedef void *(*line_map_realloc) (void *, size_t);
>  
> +#define LC_REASON_BIT CHAR_BIT

This is a nit, but I would keep this hunk ... 

>  /* Physical source file TO_FILE at line TO_LINE at column 0 is represented
>     by the logical START_LOCATION.  TO_LINE+L at column C is represented by
>     START_LOCATION+(L*(1<<column_bits))+C, as long as C<(1<<column_bits),
> @@ -61,11 +64,11 @@ struct GTY(()) line_map {
>    linenum_type to_line;
>    source_location start_location;
>    int included_from;
> -  ENUM_BITFIELD (lc_reason) reason : CHAR_BIT;
> +  ENUM_BITFIELD (lc_reason) reason : LC_REASON_BIT;

... and this one into your private branch for now and apply it to trunk
(provided the maintainers agree) with the rest of this patch.
Otherwise, in isolation, this new define could look like a gratuitous
change.

[...]

>  /* A set of chronological line_map structures.  */
> @@ -190,4 +193,24 @@ extern const struct line_map *linemap_lookup
>  extern source_location
>  linemap_position_for_column (struct line_maps *set, unsigned int to_column);
>  
> +/* Makes sure SET has at least one more unused line_map allocated.
> +   If it doesn't, this function allocates more room in SET.  */
> +
> +static inline void
> +linemap_ensure_extra_space_available (struct line_maps *set)
> +{
> +  if (set->used == set->allocated)
> +    {
> +      line_map_realloc reallocator;
> +      reallocator  = set->reallocator ? set->reallocator : xrealloc;
> +
> +      set->allocated = 2 * set->allocated + 256;
> +      set->maps = (struct line_map *) (*reallocator) (set->maps,
> +                               set->allocated * sizeof (struct line_map));
> +
> +      memset (&set->maps[set->used], 0,
> +              (set->allocated - set->used) * sizeof (struct line_map));
> +    }
> +}
> +
>  #endif /* !LIBCPP_LINE_MAP_H  */
> diff --git a/libcpp/line-map.c b/libcpp/line-map.c
> index 86e2484..01ed7b5 100644
> --- a/libcpp/line-map.c
> +++ b/libcpp/line-map.c
> @@ -94,18 +94,7 @@ linemap_add (struct line_maps *set, enum lc_reason reason,
>    if (set->used && start_location < set->maps[set->used - 1].start_location)
>      abort ();
>  
> -  if (set->used == set->allocated)
> -    {
> -      line_map_realloc reallocator
> -	= set->reallocator ? set->reallocator : xrealloc;
> -      set->allocated = 2 * set->allocated + 256;
> -      set->maps
> -	= (struct line_map *) (*reallocator) (set->maps,
> -					      set->allocated
> -					      * sizeof (struct line_map));
> -      memset (&set->maps[set->used], 0, ((set->allocated - set->used)
> -					 * sizeof (struct line_map)));
> -    }
> +  linemap_ensure_extra_space_available (set);

Just FYI, in my macro location tracking patch set (that is also that is
being reviewed "at the moment") I have added a similar function named
new_linemap, that allocates a new line map and adds it to the set of
line maps.  The relevant patch is at [1].

I am CC-ing Tom, as I cannot approve or reject this patch.

Thanks.

[1]: http://tinyurl.com/3jkaeh2