| Message ID | 7869ae12-9a1a-9d52-d194-60e8cd1cea45@linaro.org |
|---|---|
| State | New |
| Headers | show |
> > 2017-02-02 Kugan Vivekanandarajah <kuganv@linaro.org> > > * ipa-cp.c (ipcp_store_bits_results): Construct bits vector. > (ipcp_store_vr_results): Constrict m_vr vector. > * ipa-prop.c (ipa_node_params_t::remove): Set transaction summary to > null. > (ipa_node_params_t::duplicate): Construct bits and m_vr vector. > (read_ipcp_transformation_info): Likewise. > > > Thanks, > Kugan > > >>I tried similar think without variable structure like: > >>diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > >>index 93a2390c..b0cc832 100644 > >>--- a/gcc/ipa-prop.h > >>+++ b/gcc/ipa-prop.h > >>@@ -525,7 +525,7 @@ struct GTY(()) ipcp_transformation_summary > >> /* Known bits information. */ > >> vec<ipa_bits, va_gc> *bits; > >> /* Value range information. */ > >>- vec<ipa_vr, va_gc> *m_vr; > >>+ vec<ipa_vr *, va_gc> *m_vr; > >> }; > >> > >>This also has the same issue so I don't think it has anything to do with > >>variable structure. > > > >You have to debug that detail yourself but I wonder why the transformation > >summary has a different representation than the jump function (and I think > >the jump function size is the issue). > > > >The JF has > > > > /* Information about zero/non-zero bits. */ > > struct ipa_bits bits; > > > > /* Information about value range, containing valid data only when vr_known is > > true. */ > > value_range m_vr; > > bool vr_known; > > > >with ipa_bits having two widest_ints and value_range having two trees > >and an unused bitmap and ipa_vr having two wide_ints (widest_ints are > >smaller!). > > > >Richard. > > > >> > >>Thanks, > >>Kugan > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > index aa3c997..5103555 100644 > --- a/gcc/ipa-cp.c > +++ b/gcc/ipa-cp.c > @@ -4865,6 +4865,8 @@ ipcp_store_bits_results (void) > > ipcp_grow_transformations_if_necessary (); > ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > + if (!ts->bits) > + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); > vec_safe_reserve_exact (ts->bits, count); > > for (unsigned i = 0; i < count; i++) > @@ -4940,6 +4942,8 @@ ipcp_store_vr_results (void) > > ipcp_grow_transformations_if_necessary (); > ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > + if (!ts->m_vr) > + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); > vec_safe_reserve_exact (ts->m_vr, count); > > for (unsigned i = 0; i < count; i++) > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c > index 834c27d..b992a7f 100644 > --- a/gcc/ipa-prop.c > +++ b/gcc/ipa-prop.c > @@ -3759,13 +3759,20 @@ ipa_node_params_t::insert (cgraph_node *, ipa_node_params *info) > to. */ > > void > -ipa_node_params_t::remove (cgraph_node *, ipa_node_params *info) > +ipa_node_params_t::remove (cgraph_node *node, ipa_node_params *info) > { > free (info->lattices); > /* Lattice values and their sources are deallocated with their alocation > pool. */ > info->known_csts.release (); > info->known_contexts.release (); > + ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > + if (ts != NULL) > + { > + ts->agg_values = NULL; > + ts->bits = NULL; > + ts->m_vr = NULL; > + } I would go for explicit ggc_free for them: garbage collrector is hardly ever run during WPA stage and thus we are not going to get the memory back otherwise. Patch is OK with that change. Honza > } > > /* Hook that is called by summary when a node is duplicated. */ > @@ -3811,8 +3818,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, > ipcp_grow_transformations_if_necessary (); > src_trans = ipcp_get_transformation_summary (src); > const vec<ipa_vr, va_gc> *src_vr = src_trans->m_vr; > - vec<ipa_vr, va_gc> *&dst_vr > - = ipcp_get_transformation_summary (dst)->m_vr; > + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); > + if (!dts->m_vr) > + dts->m_vr = (new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec ()); > + vec<ipa_vr, va_gc> *&dst_vr = dts->m_vr; > if (vec_safe_length (src_trans->m_vr) > 0) > { > vec_safe_reserve_exact (dst_vr, src_vr->length ()); > @@ -3826,8 +3835,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, > ipcp_grow_transformations_if_necessary (); > src_trans = ipcp_get_transformation_summary (src); > const vec<ipa_bits, va_gc> *src_bits = src_trans->bits; > - vec<ipa_bits, va_gc> *&dst_bits > - = ipcp_get_transformation_summary (dst)->bits; > + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); > + if (!dts->bits) > + dts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); > + vec<ipa_bits, va_gc> *&dst_bits = dts->bits; > vec_safe_reserve_exact (dst_bits, src_bits->length ()); > for (unsigned i = 0; i < src_bits->length (); ++i) > dst_bits->quick_push ((*src_bits)[i]); > @@ -5283,6 +5294,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, > ipcp_grow_transformations_if_necessary (); > > ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > + if (!ts->m_vr) > + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); > vec_safe_grow_cleared (ts->m_vr, count); > for (i = 0; i < count; i++) > { > @@ -5306,6 +5319,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, > ipcp_grow_transformations_if_necessary (); > > ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > + if (!ts->bits) > + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); > vec_safe_grow_cleared (ts->bits, count); > > for (i = 0; i < count; i++) > diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > index 93a2390c..6573a78 100644 > --- a/gcc/ipa-prop.h > +++ b/gcc/ipa-prop.h > @@ -528,6 +528,9 @@ struct GTY(()) ipcp_transformation_summary > vec<ipa_vr, va_gc> *m_vr; > }; > > +typedef vec<ipa_vr, va_gc> ipa_vr_vec; > +typedef vec<ipa_bits, va_gc> ipa_bits_vec; > + > void ipa_set_node_agg_value_chain (struct cgraph_node *node, > struct ipa_agg_replacement_value *aggvals); > void ipcp_grow_transformations_if_necessary (void);
On Thu, Feb 2, 2017 at 1:48 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >> >> 2017-02-02 Kugan Vivekanandarajah <kuganv@linaro.org> >> >> * ipa-cp.c (ipcp_store_bits_results): Construct bits vector. >> (ipcp_store_vr_results): Constrict m_vr vector. >> * ipa-prop.c (ipa_node_params_t::remove): Set transaction summary to >> null. >> (ipa_node_params_t::duplicate): Construct bits and m_vr vector. >> (read_ipcp_transformation_info): Likewise. >> >> >> Thanks, >> Kugan >> >> >>I tried similar think without variable structure like: >> >>diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h >> >>index 93a2390c..b0cc832 100644 >> >>--- a/gcc/ipa-prop.h >> >>+++ b/gcc/ipa-prop.h >> >>@@ -525,7 +525,7 @@ struct GTY(()) ipcp_transformation_summary >> >> /* Known bits information. */ >> >> vec<ipa_bits, va_gc> *bits; >> >> /* Value range information. */ >> >>- vec<ipa_vr, va_gc> *m_vr; >> >>+ vec<ipa_vr *, va_gc> *m_vr; >> >> }; >> >> >> >>This also has the same issue so I don't think it has anything to do with >> >>variable structure. >> > >> >You have to debug that detail yourself but I wonder why the transformation >> >summary has a different representation than the jump function (and I think >> >the jump function size is the issue). >> > >> >The JF has >> > >> > /* Information about zero/non-zero bits. */ >> > struct ipa_bits bits; >> > >> > /* Information about value range, containing valid data only when vr_known is >> > true. */ >> > value_range m_vr; >> > bool vr_known; >> > >> >with ipa_bits having two widest_ints and value_range having two trees >> >and an unused bitmap and ipa_vr having two wide_ints (widest_ints are >> >smaller!). >> > >> >Richard. >> > >> >> >> >>Thanks, >> >>Kugan > >> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c >> index aa3c997..5103555 100644 >> --- a/gcc/ipa-cp.c >> +++ b/gcc/ipa-cp.c >> @@ -4865,6 +4865,8 @@ ipcp_store_bits_results (void) >> >> ipcp_grow_transformations_if_necessary (); >> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >> + if (!ts->bits) >> + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); Why do you need those placement news? Richard. >> vec_safe_reserve_exact (ts->bits, count); >> >> for (unsigned i = 0; i < count; i++) >> @@ -4940,6 +4942,8 @@ ipcp_store_vr_results (void) >> >> ipcp_grow_transformations_if_necessary (); >> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >> + if (!ts->m_vr) >> + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); >> vec_safe_reserve_exact (ts->m_vr, count); >> >> for (unsigned i = 0; i < count; i++) >> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c >> index 834c27d..b992a7f 100644 >> --- a/gcc/ipa-prop.c >> +++ b/gcc/ipa-prop.c >> @@ -3759,13 +3759,20 @@ ipa_node_params_t::insert (cgraph_node *, ipa_node_params *info) >> to. */ >> >> void >> -ipa_node_params_t::remove (cgraph_node *, ipa_node_params *info) >> +ipa_node_params_t::remove (cgraph_node *node, ipa_node_params *info) >> { >> free (info->lattices); >> /* Lattice values and their sources are deallocated with their alocation >> pool. */ >> info->known_csts.release (); >> info->known_contexts.release (); >> + ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >> + if (ts != NULL) >> + { >> + ts->agg_values = NULL; >> + ts->bits = NULL; >> + ts->m_vr = NULL; >> + } > > I would go for explicit ggc_free for them: garbage collrector is hardly ever run during > WPA stage and thus we are not going to get the memory back otherwise. > > Patch is OK with that change. > > Honza >> } >> >> /* Hook that is called by summary when a node is duplicated. */ >> @@ -3811,8 +3818,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, >> ipcp_grow_transformations_if_necessary (); >> src_trans = ipcp_get_transformation_summary (src); >> const vec<ipa_vr, va_gc> *src_vr = src_trans->m_vr; >> - vec<ipa_vr, va_gc> *&dst_vr >> - = ipcp_get_transformation_summary (dst)->m_vr; >> + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); >> + if (!dts->m_vr) >> + dts->m_vr = (new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec ()); >> + vec<ipa_vr, va_gc> *&dst_vr = dts->m_vr; >> if (vec_safe_length (src_trans->m_vr) > 0) >> { >> vec_safe_reserve_exact (dst_vr, src_vr->length ()); >> @@ -3826,8 +3835,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, >> ipcp_grow_transformations_if_necessary (); >> src_trans = ipcp_get_transformation_summary (src); >> const vec<ipa_bits, va_gc> *src_bits = src_trans->bits; >> - vec<ipa_bits, va_gc> *&dst_bits >> - = ipcp_get_transformation_summary (dst)->bits; >> + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); >> + if (!dts->bits) >> + dts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); >> + vec<ipa_bits, va_gc> *&dst_bits = dts->bits; >> vec_safe_reserve_exact (dst_bits, src_bits->length ()); >> for (unsigned i = 0; i < src_bits->length (); ++i) >> dst_bits->quick_push ((*src_bits)[i]); >> @@ -5283,6 +5294,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, >> ipcp_grow_transformations_if_necessary (); >> >> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >> + if (!ts->m_vr) >> + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); >> vec_safe_grow_cleared (ts->m_vr, count); >> for (i = 0; i < count; i++) >> { >> @@ -5306,6 +5319,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, >> ipcp_grow_transformations_if_necessary (); >> >> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >> + if (!ts->bits) >> + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); >> vec_safe_grow_cleared (ts->bits, count); >> >> for (i = 0; i < count; i++) >> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h >> index 93a2390c..6573a78 100644 >> --- a/gcc/ipa-prop.h >> +++ b/gcc/ipa-prop.h >> @@ -528,6 +528,9 @@ struct GTY(()) ipcp_transformation_summary >> vec<ipa_vr, va_gc> *m_vr; >> }; >> >> +typedef vec<ipa_vr, va_gc> ipa_vr_vec; >> +typedef vec<ipa_bits, va_gc> ipa_bits_vec; >> + >> void ipa_set_node_agg_value_chain (struct cgraph_node *node, >> struct ipa_agg_replacement_value *aggvals); >> void ipcp_grow_transformations_if_necessary (void); >
On 02/02/2017 02:36 AM, kugan wrote:
> This is due to an existing issue. That is, in ipa_node_params_t::remove, m_vr and bits vectors are not set to null such that the gc can claim it.
I've just sent patch that should remove such need as ~ipa_node_params_t should be called
just once:
https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00148.html
Thanks,
Martin
On Thu, Feb 2, 2017 at 1:52 PM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Feb 2, 2017 at 1:48 PM, Jan Hubicka <hubicka@ucw.cz> wrote: >>> >>> 2017-02-02 Kugan Vivekanandarajah <kuganv@linaro.org> >>> >>> * ipa-cp.c (ipcp_store_bits_results): Construct bits vector. >>> (ipcp_store_vr_results): Constrict m_vr vector. >>> * ipa-prop.c (ipa_node_params_t::remove): Set transaction summary to >>> null. >>> (ipa_node_params_t::duplicate): Construct bits and m_vr vector. >>> (read_ipcp_transformation_info): Likewise. >>> >>> >>> Thanks, >>> Kugan >>> >>> >>I tried similar think without variable structure like: >>> >>diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h >>> >>index 93a2390c..b0cc832 100644 >>> >>--- a/gcc/ipa-prop.h >>> >>+++ b/gcc/ipa-prop.h >>> >>@@ -525,7 +525,7 @@ struct GTY(()) ipcp_transformation_summary >>> >> /* Known bits information. */ >>> >> vec<ipa_bits, va_gc> *bits; >>> >> /* Value range information. */ >>> >>- vec<ipa_vr, va_gc> *m_vr; >>> >>+ vec<ipa_vr *, va_gc> *m_vr; >>> >> }; >>> >> >>> >>This also has the same issue so I don't think it has anything to do with >>> >>variable structure. >>> > >>> >You have to debug that detail yourself but I wonder why the transformation >>> >summary has a different representation than the jump function (and I think >>> >the jump function size is the issue). >>> > >>> >The JF has >>> > >>> > /* Information about zero/non-zero bits. */ >>> > struct ipa_bits bits; >>> > >>> > /* Information about value range, containing valid data only when vr_known is >>> > true. */ >>> > value_range m_vr; >>> > bool vr_known; >>> > >>> >with ipa_bits having two widest_ints and value_range having two trees >>> >and an unused bitmap and ipa_vr having two wide_ints (widest_ints are >>> >smaller!). >>> > >>> >Richard. >>> > >>> >> >>> >>Thanks, >>> >>Kugan >> >>> diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c >>> index aa3c997..5103555 100644 >>> --- a/gcc/ipa-cp.c >>> +++ b/gcc/ipa-cp.c >>> @@ -4865,6 +4865,8 @@ ipcp_store_bits_results (void) >>> >>> ipcp_grow_transformations_if_necessary (); >>> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >>> + if (!ts->bits) >>> + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); > > > Why do you need those placement news? Ah, I see we handle finalization but not construction in ggc_[cleared_]alloc... > Richard. > >>> vec_safe_reserve_exact (ts->bits, count); >>> >>> for (unsigned i = 0; i < count; i++) >>> @@ -4940,6 +4942,8 @@ ipcp_store_vr_results (void) >>> >>> ipcp_grow_transformations_if_necessary (); >>> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >>> + if (!ts->m_vr) >>> + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); >>> vec_safe_reserve_exact (ts->m_vr, count); >>> >>> for (unsigned i = 0; i < count; i++) >>> diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c >>> index 834c27d..b992a7f 100644 >>> --- a/gcc/ipa-prop.c >>> +++ b/gcc/ipa-prop.c >>> @@ -3759,13 +3759,20 @@ ipa_node_params_t::insert (cgraph_node *, ipa_node_params *info) >>> to. */ >>> >>> void >>> -ipa_node_params_t::remove (cgraph_node *, ipa_node_params *info) >>> +ipa_node_params_t::remove (cgraph_node *node, ipa_node_params *info) >>> { >>> free (info->lattices); >>> /* Lattice values and their sources are deallocated with their alocation >>> pool. */ >>> info->known_csts.release (); >>> info->known_contexts.release (); >>> + ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >>> + if (ts != NULL) >>> + { >>> + ts->agg_values = NULL; >>> + ts->bits = NULL; >>> + ts->m_vr = NULL; >>> + } >> >> I would go for explicit ggc_free for them: garbage collrector is hardly ever run during >> WPA stage and thus we are not going to get the memory back otherwise. >> >> Patch is OK with that change. >> >> Honza >>> } >>> >>> /* Hook that is called by summary when a node is duplicated. */ >>> @@ -3811,8 +3818,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, >>> ipcp_grow_transformations_if_necessary (); >>> src_trans = ipcp_get_transformation_summary (src); >>> const vec<ipa_vr, va_gc> *src_vr = src_trans->m_vr; >>> - vec<ipa_vr, va_gc> *&dst_vr >>> - = ipcp_get_transformation_summary (dst)->m_vr; >>> + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); >>> + if (!dts->m_vr) >>> + dts->m_vr = (new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec ()); >>> + vec<ipa_vr, va_gc> *&dst_vr = dts->m_vr; >>> if (vec_safe_length (src_trans->m_vr) > 0) >>> { >>> vec_safe_reserve_exact (dst_vr, src_vr->length ()); >>> @@ -3826,8 +3835,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, >>> ipcp_grow_transformations_if_necessary (); >>> src_trans = ipcp_get_transformation_summary (src); >>> const vec<ipa_bits, va_gc> *src_bits = src_trans->bits; >>> - vec<ipa_bits, va_gc> *&dst_bits >>> - = ipcp_get_transformation_summary (dst)->bits; >>> + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); >>> + if (!dts->bits) >>> + dts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); >>> + vec<ipa_bits, va_gc> *&dst_bits = dts->bits; >>> vec_safe_reserve_exact (dst_bits, src_bits->length ()); >>> for (unsigned i = 0; i < src_bits->length (); ++i) >>> dst_bits->quick_push ((*src_bits)[i]); >>> @@ -5283,6 +5294,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, >>> ipcp_grow_transformations_if_necessary (); >>> >>> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >>> + if (!ts->m_vr) >>> + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); >>> vec_safe_grow_cleared (ts->m_vr, count); >>> for (i = 0; i < count; i++) >>> { >>> @@ -5306,6 +5319,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, >>> ipcp_grow_transformations_if_necessary (); >>> >>> ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); >>> + if (!ts->bits) >>> + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); >>> vec_safe_grow_cleared (ts->bits, count); >>> >>> for (i = 0; i < count; i++) >>> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h >>> index 93a2390c..6573a78 100644 >>> --- a/gcc/ipa-prop.h >>> +++ b/gcc/ipa-prop.h >>> @@ -528,6 +528,9 @@ struct GTY(()) ipcp_transformation_summary >>> vec<ipa_vr, va_gc> *m_vr; >>> }; >>> >>> +typedef vec<ipa_vr, va_gc> ipa_vr_vec; >>> +typedef vec<ipa_bits, va_gc> ipa_bits_vec; >>> + >>> void ipa_set_node_agg_value_chain (struct cgraph_node *node, >>> struct ipa_agg_replacement_value *aggvals); >>> void ipcp_grow_transformations_if_necessary (void); >>
Hi, I am sorry, I am apparently not really able to follow all email this week and am mostly skimming through this thread too, but... On Thu, Feb 02, 2017 at 01:48:26PM +0100, Jan Hubicka wrote: > > > > 2017-02-02 Kugan Vivekanandarajah <kuganv@linaro.org> > > > > * ipa-cp.c (ipcp_store_bits_results): Construct bits vector. > > (ipcp_store_vr_results): Constrict m_vr vector. > > * ipa-prop.c (ipa_node_params_t::remove): Set transaction summary to > > null. > > (ipa_node_params_t::duplicate): Construct bits and m_vr vector. > > (read_ipcp_transformation_info): Likewise. > > > > > > Thanks, > > Kugan > > > > >>I tried similar think without variable structure like: > > >>diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h > > >>index 93a2390c..b0cc832 100644 > > >>--- a/gcc/ipa-prop.h > > >>+++ b/gcc/ipa-prop.h > > >>@@ -525,7 +525,7 @@ struct GTY(()) ipcp_transformation_summary > > >> /* Known bits information. */ > > >> vec<ipa_bits, va_gc> *bits; > > >> /* Value range information. */ > > >>- vec<ipa_vr, va_gc> *m_vr; > > >>+ vec<ipa_vr *, va_gc> *m_vr; > > >> }; > > >> > > >>This also has the same issue so I don't think it has anything to do with > > >>variable structure. > > > > > >You have to debug that detail yourself but I wonder why the transformation > > >summary has a different representation than the jump function (and I think > > >the jump function size is the issue). > > > > > >The JF has > > > > > > /* Information about zero/non-zero bits. */ > > > struct ipa_bits bits; > > > > > > /* Information about value range, containing valid data only when vr_known is > > > true. */ > > > value_range m_vr; > > > bool vr_known; > > > > > >with ipa_bits having two widest_ints and value_range having two trees > > >and an unused bitmap and ipa_vr having two wide_ints (widest_ints are > > >smaller!). > > > > > >Richard. > > > > > >> > > >>Thanks, > > >>Kugan > > > diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c > > index aa3c997..5103555 100644 > > --- a/gcc/ipa-cp.c > > +++ b/gcc/ipa-cp.c > > @@ -4865,6 +4865,8 @@ ipcp_store_bits_results (void) > > > > ipcp_grow_transformations_if_necessary (); > > ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > > + if (!ts->bits) > > + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); > > vec_safe_reserve_exact (ts->bits, count); > > > > for (unsigned i = 0; i < count; i++) > > @@ -4940,6 +4942,8 @@ ipcp_store_vr_results (void) > > > > ipcp_grow_transformations_if_necessary (); > > ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > > + if (!ts->m_vr) > > + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); > > vec_safe_reserve_exact (ts->m_vr, count); > > > > for (unsigned i = 0; i < count; i++) > > diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c > > index 834c27d..b992a7f 100644 > > --- a/gcc/ipa-prop.c > > +++ b/gcc/ipa-prop.c > > @@ -3759,13 +3759,20 @@ ipa_node_params_t::insert (cgraph_node *, ipa_node_params *info) > > to. */ > > > > void > > -ipa_node_params_t::remove (cgraph_node *, ipa_node_params *info) > > +ipa_node_params_t::remove (cgraph_node *node, ipa_node_params *info) > > { > > free (info->lattices); > > /* Lattice values and their sources are deallocated with their alocation > > pool. */ > > info->known_csts.release (); > > info->known_contexts.release (); > > + ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); > > + if (ts != NULL) why des this need to be conditional? ipcp_get_transformation_summary also lives in garbage collector so it should be able to hold any necessary references properly. > > + { > > + ts->agg_values = NULL; > > + ts->bits = NULL; > > + ts->m_vr = NULL; > > + } > > I would go for explicit ggc_free for them: garbage collrector is hardly ever run during > WPA stage and thus we are not going to get the memory back otherwise. ggc_freeing might make a difference but I fail to see how the above can, unless ipa_node_params_t still holds a reference to the info, which it is about to drop. Moreover... > > Patch is OK with that change. I believe this patch conflicts with Martin's fix for 79337 which might deal with parts of what Kugan wants to achieve better so it may be better to re-base the patch? Thanks, Martin
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c index aa3c997..5103555 100644 --- a/gcc/ipa-cp.c +++ b/gcc/ipa-cp.c @@ -4865,6 +4865,8 @@ ipcp_store_bits_results (void) ipcp_grow_transformations_if_necessary (); ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); + if (!ts->bits) + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); vec_safe_reserve_exact (ts->bits, count); for (unsigned i = 0; i < count; i++) @@ -4940,6 +4942,8 @@ ipcp_store_vr_results (void) ipcp_grow_transformations_if_necessary (); ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); + if (!ts->m_vr) + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); vec_safe_reserve_exact (ts->m_vr, count); for (unsigned i = 0; i < count; i++) diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c index 834c27d..b992a7f 100644 --- a/gcc/ipa-prop.c +++ b/gcc/ipa-prop.c @@ -3759,13 +3759,20 @@ ipa_node_params_t::insert (cgraph_node *, ipa_node_params *info) to. */ void -ipa_node_params_t::remove (cgraph_node *, ipa_node_params *info) +ipa_node_params_t::remove (cgraph_node *node, ipa_node_params *info) { free (info->lattices); /* Lattice values and their sources are deallocated with their alocation pool. */ info->known_csts.release (); info->known_contexts.release (); + ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); + if (ts != NULL) + { + ts->agg_values = NULL; + ts->bits = NULL; + ts->m_vr = NULL; + } } /* Hook that is called by summary when a node is duplicated. */ @@ -3811,8 +3818,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, ipcp_grow_transformations_if_necessary (); src_trans = ipcp_get_transformation_summary (src); const vec<ipa_vr, va_gc> *src_vr = src_trans->m_vr; - vec<ipa_vr, va_gc> *&dst_vr - = ipcp_get_transformation_summary (dst)->m_vr; + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); + if (!dts->m_vr) + dts->m_vr = (new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec ()); + vec<ipa_vr, va_gc> *&dst_vr = dts->m_vr; if (vec_safe_length (src_trans->m_vr) > 0) { vec_safe_reserve_exact (dst_vr, src_vr->length ()); @@ -3826,8 +3835,10 @@ ipa_node_params_t::duplicate(cgraph_node *src, cgraph_node *dst, ipcp_grow_transformations_if_necessary (); src_trans = ipcp_get_transformation_summary (src); const vec<ipa_bits, va_gc> *src_bits = src_trans->bits; - vec<ipa_bits, va_gc> *&dst_bits - = ipcp_get_transformation_summary (dst)->bits; + ipcp_transformation_summary *dts = ipcp_get_transformation_summary (dst); + if (!dts->bits) + dts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); + vec<ipa_bits, va_gc> *&dst_bits = dts->bits; vec_safe_reserve_exact (dst_bits, src_bits->length ()); for (unsigned i = 0; i < src_bits->length (); ++i) dst_bits->quick_push ((*src_bits)[i]); @@ -5283,6 +5294,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, ipcp_grow_transformations_if_necessary (); ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); + if (!ts->m_vr) + ts->m_vr = new (ggc_cleared_alloc<ipa_vr_vec> ()) ipa_vr_vec (); vec_safe_grow_cleared (ts->m_vr, count); for (i = 0; i < count; i++) { @@ -5306,6 +5319,8 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, ipcp_grow_transformations_if_necessary (); ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node); + if (!ts->bits) + ts->bits = (new (ggc_cleared_alloc<ipa_bits_vec> ()) ipa_bits_vec ()); vec_safe_grow_cleared (ts->bits, count); for (i = 0; i < count; i++) diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 93a2390c..6573a78 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -528,6 +528,9 @@ struct GTY(()) ipcp_transformation_summary vec<ipa_vr, va_gc> *m_vr; }; +typedef vec<ipa_vr, va_gc> ipa_vr_vec; +typedef vec<ipa_bits, va_gc> ipa_bits_vec; + void ipa_set_node_agg_value_chain (struct cgraph_node *node, struct ipa_agg_replacement_value *aggvals); void ipcp_grow_transformations_if_necessary (void);