| Message ID | f0326ad1-bd43-0d18-7115-f22d4acbe7a7@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 4, 2016 at 10:09 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote: > Hi Richard, > > Thanks for the review. > > On 04/08/16 17:26, Richard Biener wrote: >> >> On Thu, Aug 4, 2016 at 6:12 AM, kugan <kugan.vivekanandarajah@linaro.org> >> wrote: >>> >>> Hi, >>> >>> During IPA-VRP implementation, I realized that we don't support streaming >>> wide_int in LTO. Attached patch does this. Tested with IPA-VRP. Is this >>> OK >>> for trunk if bootstrap and regression testing is fine. >> >> >> Hmm, those functions belong to data-streamer-{in,out}.c and >> data-streamer.h >> and should be named streamer_write_wide_int / streamer_read_wide_int. >> >> Note that we already have (non-exported) streamer_write_wi / >> streamer_read_wi >> which operate on widest_ints. Those also reside in >> lto-streamer-{in,out}.c and >> should be moved to data-streamer.h (and be renamed to >> streamer_write_widest_int). > > > I have now streamer_write_wide_int and streamer_write_widest_int. Similarly > for reading. There is lot of similarity. I am not very familiar with > wide_int so kept it that way. Is this OK now? I also thought about merging both cases but I don't see how it is easily possible in a way that would reduce the number of source lines (you could split out an inline worker, but that wouldn't save anything I think) Yes. Thanks, Richard. > Thanks, > Kugan > > gcc/ChangeLog: > > 2016-08-04 Kugan Vivekanandarajah <kuganv@linaro.org> > > * data-streamer-in.c (streamer_read_wide_int): New. > (streamer_read_widest_int): Renamed function. > * data-streamer-out.c (streamer_write_wide_int): New > (streamer_write_widest_int): Renamed function. > * lto-streamer-in.c (streamer_read_wi): Renamed and moved to > data-stream-in.c. > (input_cfg): Call renamed function. > * lto-streamer-out.c (streamer_write_wi): Renamed and moved to > data-stream-out.c. > (output_cfg): Call renamed function. > * data-streamer.h: Add declarations. > >> >> There is no need to add additional hooks. >> >> Can you do this please? >> >> Thanks, >> Richard. >> >>> Thanks, >>> Kugan >>> >>> gcc/ChangeLog: >>> >>> 2016-08-04 Kugan Vivekanandarajah <kuganv@linaro.org> >>> >>> * lto-streamer-in.c (lto_input_wide_int): New. >>> * lto-streamer-out.c (lto_output_wide_int): Likewise. >>> * lto-streamer.c (lto_streamer_hooks_init): Init write_wide_int >>> and >>> read_wide_int. >>> * lto-streamer.h: Declare lto_input_wide_int and >>> lto_output_wide_int. >>> * streamer-hooks.h (struct streamer_hooks): Add write_wide_int >>> and >>> read_wide_int. >>> (stream_write_wide_int): New macro. >>> (stream_read_wide_int): Likewise. >>> >
> Hi Richard, > > Thanks for the review. > > On 04/08/16 17:26, Richard Biener wrote: > >On Thu, Aug 4, 2016 at 6:12 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote: > >>Hi, > >> > >>During IPA-VRP implementation, I realized that we don't support streaming > >>wide_int in LTO. Attached patch does this. Tested with IPA-VRP. Is this OK > >>for trunk if bootstrap and regression testing is fine. > > > >Hmm, those functions belong to data-streamer-{in,out}.c and data-streamer.h > >and should be named streamer_write_wide_int / streamer_read_wide_int. > > > >Note that we already have (non-exported) streamer_write_wi / streamer_read_wi > >which operate on widest_ints. Those also reside in lto-streamer-{in,out}.c and > >should be moved to data-streamer.h (and be renamed to > >streamer_write_widest_int). > > I have now streamer_write_wide_int and streamer_write_widest_int. > Similarly for reading. There is lot of similarity. I am not very > familiar with wide_int so kept it that way. Is this OK now? > > Thanks, > Kugan > > gcc/ChangeLog: > > 2016-08-04 Kugan Vivekanandarajah <kuganv@linaro.org> > > * data-streamer-in.c (streamer_read_wide_int): New. > (streamer_read_widest_int): Renamed function. > * data-streamer-out.c (streamer_write_wide_int): New > (streamer_write_widest_int): Renamed function. I wondered, given we do C++ now, if we don't want to just have stream_in/stream_out member functions for our classes and/or use just one function name for all of them so one does not need to look up somewhat irregular function names. I find LTO streaming API very hard to memorize and use without constantly looking up existing code. Honza
On Thu, Aug 4, 2016 at 11:32 AM, Jan Hubicka <hubicka@ucw.cz> wrote: >> Hi Richard, >> >> Thanks for the review. >> >> On 04/08/16 17:26, Richard Biener wrote: >> >On Thu, Aug 4, 2016 at 6:12 AM, kugan <kugan.vivekanandarajah@linaro.org> wrote: >> >>Hi, >> >> >> >>During IPA-VRP implementation, I realized that we don't support streaming >> >>wide_int in LTO. Attached patch does this. Tested with IPA-VRP. Is this OK >> >>for trunk if bootstrap and regression testing is fine. >> > >> >Hmm, those functions belong to data-streamer-{in,out}.c and data-streamer.h >> >and should be named streamer_write_wide_int / streamer_read_wide_int. >> > >> >Note that we already have (non-exported) streamer_write_wi / streamer_read_wi >> >which operate on widest_ints. Those also reside in lto-streamer-{in,out}.c and >> >should be moved to data-streamer.h (and be renamed to >> >streamer_write_widest_int). >> >> I have now streamer_write_wide_int and streamer_write_widest_int. >> Similarly for reading. There is lot of similarity. I am not very >> familiar with wide_int so kept it that way. Is this OK now? >> >> Thanks, >> Kugan >> >> gcc/ChangeLog: >> >> 2016-08-04 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> * data-streamer-in.c (streamer_read_wide_int): New. >> (streamer_read_widest_int): Renamed function. >> * data-streamer-out.c (streamer_write_wide_int): New >> (streamer_write_widest_int): Renamed function. > > I wondered, given we do C++ now, if we don't want to just have > stream_in/stream_out member functions for our classes and/or use just one > function name for all of them so one does not need to look up somewhat > irregular function names. > > I find LTO streaming API very hard to memorize and use without constantly > looking up existing code. Not sure if that would help given the arguments are different even besides the thing you want to stream. Richard. > Honza
From fb2561bcdaf656c464b98dad28db96fcdf74af17 Mon Sep 17 00:00:00 2001 From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org> Date: Thu, 4 Aug 2016 11:54:00 +1000 Subject: [PATCH 6/8] Add wide_int streaming support --- gcc/data-streamer-in.c | 31 +++++++++++++++++++++++++++++++ gcc/data-streamer-out.c | 27 +++++++++++++++++++++++++++ gcc/data-streamer.h | 4 ++++ gcc/lto-streamer-in.c | 21 +++------------------ gcc/lto-streamer-out.c | 20 +++----------------- 5 files changed, 68 insertions(+), 35 deletions(-) diff --git a/gcc/data-streamer-in.c b/gcc/data-streamer-in.c index 2625af6..8664a86 100644 --- a/gcc/data-streamer-in.c +++ b/gcc/data-streamer-in.c @@ -184,3 +184,34 @@ streamer_read_gcov_count (struct lto_input_block *ib) gcc_assert (ret >= 0); return ret; } + +/* Read the physical representation of a wide_int val from + input block IB. */ + +wide_int +streamer_read_wide_int (struct lto_input_block *ib) +{ + HOST_WIDE_INT a[WIDE_INT_MAX_ELTS]; + int i; + int prec = streamer_read_uhwi (ib); + int len = streamer_read_uhwi (ib); + for (i = 0; i < len; i++) + a[i] = streamer_read_hwi (ib); + return wide_int_storage::from_array (a, len, prec); +} + +/* Read the physical representation of a widest_int val from + input block IB. */ + +widest_int +streamer_read_widest_int (struct lto_input_block *ib) +{ + HOST_WIDE_INT a[WIDE_INT_MAX_ELTS]; + int i; + int prec ATTRIBUTE_UNUSED = streamer_read_uhwi (ib); + int len = streamer_read_uhwi (ib); + for (i = 0; i < len; i++) + a[i] = streamer_read_hwi (ib); + return widest_int::from_array (a, len); +} + diff --git a/gcc/data-streamer-out.c b/gcc/data-streamer-out.c index e476530..3dd423b 100644 --- a/gcc/data-streamer-out.c +++ b/gcc/data-streamer-out.c @@ -375,3 +375,30 @@ streamer_write_data_stream (struct lto_output_stream *obs, const void *data, } } +/* Emit the physical representation of wide_int VAL to output block OB. */ + +void +streamer_write_wide_int (struct output_block *ob, const wide_int &val) +{ + int len = val.get_len (); + + streamer_write_uhwi (ob, val.get_precision ()); + streamer_write_uhwi (ob, len); + for (int i = 0; i < len; i++) + streamer_write_hwi (ob, val.elt (i)); +} + +/* Emit the physical representation of widest_int W to output block OB. */ + +void +streamer_write_widest_int (struct output_block *ob, + const widest_int &w) +{ + int len = w.get_len (); + + streamer_write_uhwi (ob, w.get_precision ()); + streamer_write_uhwi (ob, len); + for (int i = 0; i < len; i++) + streamer_write_hwi (ob, w.elt (i)); +} + diff --git a/gcc/data-streamer.h b/gcc/data-streamer.h index 0048f66..ff479a6 100644 --- a/gcc/data-streamer.h +++ b/gcc/data-streamer.h @@ -69,6 +69,8 @@ void streamer_write_hwi_stream (struct lto_output_stream *, HOST_WIDE_INT); void streamer_write_gcov_count_stream (struct lto_output_stream *, gcov_type); void streamer_write_data_stream (struct lto_output_stream *, const void *, size_t); +void streamer_write_wide_int (struct output_block *, const wide_int &); +void streamer_write_widest_int (struct output_block *, const widest_int &); /* In data-streamer-in.c */ const char *streamer_read_string (struct data_in *, struct lto_input_block *); @@ -81,6 +83,8 @@ const char *bp_unpack_string (struct data_in *, struct bitpack_d *); unsigned HOST_WIDE_INT streamer_read_uhwi (struct lto_input_block *); HOST_WIDE_INT streamer_read_hwi (struct lto_input_block *); gcov_type streamer_read_gcov_count (struct lto_input_block *); +wide_int streamer_read_wide_int (struct lto_input_block *); +widest_int streamer_read_widest_int (struct lto_input_block *); /* Returns a new bit-packing context for bit-packing into S. */ static inline struct bitpack_d diff --git a/gcc/lto-streamer-in.c b/gcc/lto-streamer-in.c index 1d56d21..5075b56 100644 --- a/gcc/lto-streamer-in.c +++ b/gcc/lto-streamer-in.c @@ -710,21 +710,6 @@ make_new_block (struct function *fn, unsigned int index) } -/* Read a wide-int. */ - -static widest_int -streamer_read_wi (struct lto_input_block *ib) -{ - HOST_WIDE_INT a[WIDE_INT_MAX_ELTS]; - int i; - int prec ATTRIBUTE_UNUSED = streamer_read_uhwi (ib); - int len = streamer_read_uhwi (ib); - for (i = 0; i < len; i++) - a[i] = streamer_read_hwi (ib); - return widest_int::from_array (a, len); -} - - /* Read the CFG for function FN from input block IB. */ static void @@ -834,13 +819,13 @@ input_cfg (struct lto_input_block *ib, struct data_in *data_in, loop->estimate_state = streamer_read_enum (ib, loop_estimation, EST_LAST); loop->any_upper_bound = streamer_read_hwi (ib); if (loop->any_upper_bound) - loop->nb_iterations_upper_bound = streamer_read_wi (ib); + loop->nb_iterations_upper_bound = streamer_read_widest_int (ib); loop->any_likely_upper_bound = streamer_read_hwi (ib); if (loop->any_likely_upper_bound) - loop->nb_iterations_likely_upper_bound = streamer_read_wi (ib); + loop->nb_iterations_likely_upper_bound = streamer_read_widest_int (ib); loop->any_estimate = streamer_read_hwi (ib); if (loop->any_estimate) - loop->nb_iterations_estimate = streamer_read_wi (ib); + loop->nb_iterations_estimate = streamer_read_widest_int (ib); /* Read OMP SIMD related info. */ loop->safelen = streamer_read_hwi (ib); diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c index aa6b589..bc45721 100644 --- a/gcc/lto-streamer-out.c +++ b/gcc/lto-streamer-out.c @@ -1828,20 +1828,6 @@ output_ssa_names (struct output_block *ob, struct function *fn) } -/* Output a wide-int. */ - -static void -streamer_write_wi (struct output_block *ob, - const widest_int &w) -{ - int len = w.get_len (); - - streamer_write_uhwi (ob, w.get_precision ()); - streamer_write_uhwi (ob, len); - for (int i = 0; i < len; i++) - streamer_write_hwi (ob, w.elt (i)); -} - /* Output the cfg. */ @@ -1915,13 +1901,13 @@ output_cfg (struct output_block *ob, struct function *fn) loop_estimation, EST_LAST, loop->estimate_state); streamer_write_hwi (ob, loop->any_upper_bound); if (loop->any_upper_bound) - streamer_write_wi (ob, loop->nb_iterations_upper_bound); + streamer_write_widest_int (ob, loop->nb_iterations_upper_bound); streamer_write_hwi (ob, loop->any_likely_upper_bound); if (loop->any_likely_upper_bound) - streamer_write_wi (ob, loop->nb_iterations_likely_upper_bound); + streamer_write_widest_int (ob, loop->nb_iterations_likely_upper_bound); streamer_write_hwi (ob, loop->any_estimate); if (loop->any_estimate) - streamer_write_wi (ob, loop->nb_iterations_estimate); + streamer_write_widest_int (ob, loop->nb_iterations_estimate); /* Write OMP SIMD related info. */ streamer_write_hwi (ob, loop->safelen); -- 1.9.1