On Thu, May 26, 2011 at 11:34 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
> Hi,
> we spend a lot of effort (and disk space) into streaming file/line/column and
> sys_p fields of locations. Since often the trees come from same statement, it
> is common for those to not change. We even already have current location info
> in output_block and data_in, but don't use it.
>
> This patch update streaming to stream first bitpack of what changed and then
> streaming only the changed locations. Because I did not find any use of
> insys_p field in the backend, I removed streaming it (though it would be easy
> doable to stream it along the file info).
>
> Because we need only 4 bits, it would make a lot of sense to merge location
> bitpack with the other bitpack and for this reason I split the i/o intput part
> doing bitpack and part streaming the actual data. At the moment we always do
> that at once, since doing anything saner needs a bit of reorganization of how
> we stream trees.
>
> Regtested x86_64-linux, full bootstrap®test running, OK?
This looks all very hackish with no immediate benefit mostly because
of the use of lto_output_string. I think what you should do instead
is split up lto_output_string_with_length into the piece that streams
the string itself to the string-stream and returns an index into it
and the piece streaming the index to the specified stream. Then you
can simply bitpack that index and the two int line / column fields.
Richard.
> Honza
>
> * lto-streamer-out.c (clear_line_info): Set location_output_done.
> (lto_output_location_bitpack): New function.
> (lto_output_location_data): New function.
> (lto_output_location): Reorg.
> * lto-streamer-in.c (clear_line_info): Set location_input_done.
> (lto_input_location_bitpack): New function.
> (lto_input_location_data): New function.
> (lto_input_location): Reorg.
> * lto-streamer.h (struct output_block): Add output_file, output_line,
> output_col and location_output_done flags.
> (struct data_in): Add input_file, input_line, input_col and
> location_input_done flags.
> Index: lto-streamer-out.c
> ===================================================================
> --- lto-streamer-out.c (revision 174264)
> +++ lto-streamer-out.c (working copy)
> @@ -95,6 +95,7 @@ clear_line_info (struct output_block *ob
> ob->current_file = NULL;
> ob->current_line = 0;
> ob->current_col = 0;
> + ob->location_output_done = 1;
> }
>
>
> @@ -587,29 +588,72 @@ pack_value_fields (struct bitpack_d *bp,
> }
>
>
> -/* Emit location LOC to output block OB. */
> +/* 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)
> +{
> + gcc_checking_assert (ob->location_output_done);
> + bp_pack_value (bp, loc == UNKNOWN_LOCATION, 1);
> + if (loc == UNKNOWN_LOCATION)
> + ob->output_file = ob->output_line = ob->output_col = 0;
> + else
> + {
> + expanded_location xloc;
> +
> + xloc = expand_location (loc);
> +
> + ob->output_file = xloc.file != ob->current_file;
> + ob->output_line = xloc.line != ob->current_line;
> + ob->output_col = xloc.column != ob->current_col;
> + }
> + bp_pack_value (bp, ob->output_file, 1);
> + bp_pack_value (bp, ob->output_line, 1);
> + bp_pack_value (bp, ob->output_col, 1);
> + ob->location_output_done = false;
> +}
> +
> +
> +/* Output location info we prepared to output in
> + lto_output_location_bitpack */
>
> static void
> -lto_output_location (struct output_block *ob, location_t loc)
> +lto_output_location_data (struct output_block *ob)
> {
> - expanded_location xloc;
> + gcc_checking_assert (!ob->location_output_done);
>
> - if (loc == UNKNOWN_LOCATION)
> + if (ob->output_file)
> + lto_output_string (ob, ob->main_stream, ob->current_file);
> + if (ob->output_line)
> {
> - lto_output_string (ob, ob->main_stream, NULL);
> - return;
> + gcc_checking_assert (ob->current_line >= 0);
> + output_uleb128 (ob, ob->current_line);
> }
> + if (ob->output_col)
> + {
> + gcc_checking_assert (ob->current_col >= 0);
> + output_uleb128 (ob, ob->current_col);
> + }
> + ob->location_output_done = true;
> +}
>
> - 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;
> +/* Emit location LOC to output block OB.
> + When bitpack is handy, it is more space effecient to call
> + lto_output_location_bitpack and lto_output_location_data directly
> + adding info into the 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);
> + lto_output_location_data (ob);
> }
>
>
> @@ -642,7 +686,7 @@ lto_output_tree_ref (struct output_block
>
> if (expr == NULL_TREE)
> {
> - output_zero (ob);
> + output_record_start (ob, LTO_null);
?
> return;
> }
>
> Index: lto-streamer-in.c
> ===================================================================
> --- lto-streamer-in.c (revision 174264)
> +++ lto-streamer-in.c (working copy)
> @@ -281,40 +281,69 @@ clear_line_info (struct data_in *data_in
> data_in->current_file = NULL;
> data_in->current_line = 0;
> data_in->current_col = 0;
> + data_in->location_input_done = true;
> }
>
>
> -/* Read a location from input block IB. */
> +/* Read a location bitpack from input block IB. */
> +
> +static inline void
> +lto_input_location_bitpack (struct data_in *data_in, struct bitpack_d *bp)
> +{
> + gcc_checking_assert (data_in->location_input_done);
> + data_in->location_input_done = false;
> + data_in->input_unknown_location = bp_unpack_value (bp, 1);
> + data_in->input_file_p = bp_unpack_value (bp, 1);
> + data_in->input_line_p = bp_unpack_value (bp, 1);
> + data_in->input_col_p = bp_unpack_value (bp, 1);
> +}
> +
> +
> +/* Read locaiton data we determined to change and update info. */
>
> static location_t
> -lto_input_location (struct lto_input_block *ib, struct data_in *data_in)
> +lto_input_location_data (struct lto_input_block *ib, struct data_in *data_in)
> {
> - expanded_location xloc;
> + bool prev_file = data_in->current_file != NULL;
>
> - xloc.file = lto_input_string (data_in, ib);
> - if (xloc.file == NULL)
> + gcc_checking_assert (!data_in->location_input_done);
> + data_in->location_input_done = true;
> + if (data_in->input_unknown_location)
> return UNKNOWN_LOCATION;
> + if (data_in->input_file_p)
> + {
> + data_in->current_file = lto_input_string (data_in, ib);
> + data_in->current_file = canon_file_name (data_in->current_file);
> + }
> + if (data_in->input_line_p)
> + data_in->current_line = lto_input_uleb128 (ib);
> + if (data_in->input_col_p)
> + data_in->current_col = lto_input_uleb128 (ib);
>
> - 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->input_file_p)
> {
> - if (data_in->current_file)
> + if (prev_file)
> linemap_add (line_table, LC_LEAVE, false, NULL, 0);
>
> - linemap_add (line_table, LC_ENTER, xloc.sysp, xloc.file, xloc.line);
> + linemap_add (line_table, LC_ENTER, false, data_in->current_file,
> data_in->current_line);
> }
> - else if (data_in->current_line != xloc.line)
> - linemap_line_start (line_table, xloc.line, xloc.column);
> + else if (data_in->input_line_p)
> + linemap_line_start (line_table, data_in->current_line,
> data_in->current_col);
>
> - 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, 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;
>
> - return linemap_position_for_column (line_table, xloc.column);
> + bp = lto_input_bitpack (ib);
> + lto_input_location_bitpack (data_in, &bp);
> + return lto_input_location_data (ib, data_in);
> }
>
>
> Index: lto-streamer.h
> ===================================================================
> --- lto-streamer.h (revision 174264)
> +++ lto-streamer.h (working copy)
> @@ -694,6 +694,10 @@ struct output_block
> const char *current_file;
> int current_line;
> int current_col;
> + unsigned output_file : 1;
> + unsigned output_line : 1;
> + unsigned output_col : 1;
> + unsigned location_output_done : 1;
>
> /* True if writing globals and types. */
> bool global;
> @@ -728,6 +732,11 @@ struct data_in
> const char *current_file;
> int current_line;
> int current_col;
> + unsigned input_unknown_location : 1;
> + unsigned input_file_p : 1;
> + unsigned input_line_p : 1;
> + unsigned input_col_p : 1;
> + unsigned location_input_done : 1;
>
> /* Maps each reference number to the resolution done by the linker. */
> VEC(ld_plugin_symbol_resolution_t,heap) *globals_resolution;
>
