On 8/4/11, Gabriel Charette <gch...@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 <gch...@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 > -- Lawrence Crowl