* [RFC] Bug lto/78140
@ 2017-01-30 0:04 kugan
2017-01-30 10:13 ` Richard Biener
0 siblings, 1 reply; 9+ messages in thread
From: kugan @ 2017-01-30 0:04 UTC (permalink / raw)
To: gcc-patches, Richard Biener, Jan Hubicka, Martin Jambor
[-- Attachment #1: Type: text/plain, Size: 3277 bytes --]
Hi All,
As suggested by Richard in the PR, I tried to implement variable size
structures for VR as shown in attached patch. That is, I changed
ipa-prop.h to:
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 93a2390c..acab2aa 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -157,13 +157,15 @@ struct GTY(()) ipa_bits
};
/* Info about value ranges. */
-struct GTY(()) ipa_vr
+struct GTY ((variable_size)) ipa_vr
{
/* The data fields below are valid only if known is true. */
bool known;
enum value_range_type type;
- wide_int min;
- wide_int max;
+ /* Minimum and maximum. */
+ TRAILING_WIDE_INT_ACCESSOR (min, ints, 0)
+ TRAILING_WIDE_INT_ACCESSOR (max, ints, 1)
+ trailing_wide_ints <2> ints;
};
/* A jump function for a callsite represents the values passed as actual
@@ -525,7 +527,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;
};
void ipa_set_node_agg_value_chain (struct cgraph_node *node,
However, I am running into error when I do LTO bootstrap that memory
seems to have deallocated by the garbage collector. Since we have the
reference to the memory allocated by ggc_internal_alloc in the vector
(m_vr), I thought it will not be deallocated. But during the bootstrap,
when in ipa_node_params_t::duplicate, it seems to have been deallocated
as shown in the back trace. I dont understand internals of gc in gcc so
any help is appreciated.
lto1: internal compiler error: Segmentation fault
0xdedc4b crash_signal
../../gcc/gcc/toplev.c:333
0xb46680 ipa_node_params_t::duplicate(cgraph_node*, cgraph_node*,
ipa_node_params*, ipa_node_params*)
../../gcc/gcc/ipa-prop.c:3819
0xb306a3
function_summary<ipa_node_params*>::symtab_duplication(cgraph_node*,
cgraph_node*, void*)
../../gcc/gcc/symbol-summary.h:187
0x85aba7 symbol_table::call_cgraph_duplication_hooks(cgraph_node*,
cgraph_node*)
../../gcc/gcc/cgraph.c:488
0x8765bf cgraph_node::create_clone(tree_node*, long, int, bool,
vec<cgraph_edge*, va_heap, vl_ptr>, bool, cgraph_node*, bitmap_head*,
char const*)
../../gcc/gcc/cgraphclones.c:522
0x166fb3b clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
../../gcc/gcc/ipa-inline-transform.c:227
0x166fbd7 clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
../../gcc/gcc/ipa-inline-transform.c:242
0x1670893 inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap,
vl_ptr>*, int*, bool, bool*)
../../gcc/gcc/ipa-inline-transform.c:449
0x1665bd3 inline_small_functions
../../gcc/gcc/ipa-inline.c:2024
0x1667157 ipa_inline
../../gcc/gcc/ipa-inline.c:2434
0x1667fa7 execute
../../gcc/gcc/ipa-inline.c:2845
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.
Thanks,
Kugan
[-- Attachment #2: p2.txt --]
[-- Type: text/plain, Size: 8386 bytes --]
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index aa3c997..bc6670f 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4945,21 +4945,29 @@ ipcp_store_vr_results (void)
for (unsigned i = 0; i < count; i++)
{
ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
- ipa_vr vr;
+ ipa_vr *vr;
if (!plats->m_value_range.bottom_p ()
&& !plats->m_value_range.top_p ())
{
- vr.known = true;
- vr.type = plats->m_value_range.m_vr.type;
- vr.min = plats->m_value_range.m_vr.min;
- vr.max = plats->m_value_range.m_vr.max;
+ wide_int min = plats->m_value_range.m_vr.min;
+ wide_int max = plats->m_value_range.m_vr.max;
+ unsigned int precision = min.get_precision ();
+ size_t size = (sizeof (ipa_vr)
+ + trailing_wide_ints <2>::extra_size (precision));
+ vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ vr->ints.set_precision (precision);
+ vr->known = true;
+ vr->type = plats->m_value_range.m_vr.type;
+ vr->set_min (min);
+ vr->set_max (max);
}
else
{
- vr.known = false;
- vr.type = VR_VARYING;
- vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
+ size_t size = (sizeof (ipa_vr));
+ vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ vr->known = false;
+ vr->type = VR_VARYING;
}
ts->m_vr->quick_push (vr);
}
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 834c27d..98e8a1b 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3810,14 +3810,37 @@ 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
+ 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;
if (vec_safe_length (src_trans->m_vr) > 0)
{
vec_safe_reserve_exact (dst_vr, src_vr->length ());
for (unsigned i = 0; i < src_vr->length (); ++i)
- dst_vr->quick_push ((*src_vr)[i]);
+ {
+ ipa_vr *dst1, *src1 = (*src_vr)[i];
+ if (src1->known)
+ {
+ size_t size;
+ unsigned int precision = src1->get_min ().get_precision ();
+ size = (sizeof (ipa_vr)
+ + trailing_wide_ints <2>::extra_size (precision));
+ dst1 = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ dst1->ints.set_precision (precision);
+ dst1->known = true;
+ dst1->type = src1->type;
+ dst1->set_min (src1->get_min ());
+ dst1->set_max (src1->get_max ());
+ }
+ else
+ {
+ size_t size = (sizeof (ipa_vr));
+ dst1 = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ dst1->known = false;
+ dst1->type = src1->type;
+ }
+ dst_vr->quick_push (dst1);
+ }
}
}
@@ -5213,7 +5236,7 @@ write_ipcp_transformation_info (output_block *ob, cgraph_node *node)
for (unsigned i = 0; i < count; ++i)
{
struct bitpack_d bp;
- ipa_vr *parm_vr = &(*ts->m_vr)[i];
+ ipa_vr *parm_vr = (*ts->m_vr)[i];
bp = bitpack_create (ob->main_stream);
bp_pack_value (&bp, parm_vr->known, 1);
streamer_write_bitpack (&bp);
@@ -5221,8 +5244,8 @@ write_ipcp_transformation_info (output_block *ob, cgraph_node *node)
{
streamer_write_enum (ob->main_stream, value_rang_type,
VR_LAST, parm_vr->type);
- streamer_write_wide_int (ob, parm_vr->min);
- streamer_write_wide_int (ob, parm_vr->max);
+ streamer_write_wide_int (ob, parm_vr->get_min ());
+ streamer_write_wide_int (ob, parm_vr->get_max ());
}
}
}
@@ -5283,21 +5306,38 @@ 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);
- vec_safe_grow_cleared (ts->m_vr, count);
+ vec_safe_reserve_exact (ts->m_vr, count);
for (i = 0; i < count; i++)
{
ipa_vr *parm_vr;
- parm_vr = &(*ts->m_vr)[i];
struct bitpack_d bp;
bp = streamer_read_bitpack (ib);
- parm_vr->known = bp_unpack_value (&bp, 1);
- if (parm_vr->known)
+ bool known = bp_unpack_value (&bp, 1);
+ if (known)
+ {
+ enum value_range_type type = streamer_read_enum (ib,
+ value_range_type,
+ VR_LAST);
+ wide_int min = streamer_read_wide_int (ib);
+ wide_int max = streamer_read_wide_int (ib);
+ unsigned int precision = min.get_precision ();
+ size_t size = (sizeof (ipa_vr)
+ + trailing_wide_ints <2>::extra_size (precision));
+ parm_vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ (parm_vr)->ints.set_precision (precision);
+ (parm_vr)->known = known;
+ (parm_vr)->type = type;
+ (parm_vr)->set_min (min);
+ (parm_vr)->set_max (max);
+ }
+ else
{
- parm_vr->type = streamer_read_enum (ib, value_range_type,
- VR_LAST);
- parm_vr->min = streamer_read_wide_int (ib);
- parm_vr->max = streamer_read_wide_int (ib);
+ size_t size = (sizeof (ipa_vr));
+ parm_vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ (parm_vr)->known = known;
+ (parm_vr)->type = VR_VARYING;
}
+ ts->m_vr->quick_push (parm_vr);
}
}
count = streamer_read_uhwi (ib);
@@ -5673,7 +5713,7 @@ ipcp_update_vr (struct cgraph_node *node)
ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
if (!ts || vec_safe_length (ts->m_vr) == 0)
return;
- const vec<ipa_vr, va_gc> &vr = *ts->m_vr;
+ const vec<ipa_vr *, va_gc> &vr = *ts->m_vr;
unsigned count = vr.length ();
for (unsigned i = 0; i < count; ++i, parm = next_parm)
@@ -5688,8 +5728,8 @@ ipcp_update_vr (struct cgraph_node *node)
if (!ddef || !is_gimple_reg (parm))
continue;
- if (vr[i].known
- && (vr[i].type == VR_RANGE || vr[i].type == VR_ANTI_RANGE))
+ if (vr[i]->known
+ && (vr[i]->type == VR_RANGE || vr[i]->type == VR_ANTI_RANGE))
{
tree type = TREE_TYPE (ddef);
unsigned prec = TYPE_PRECISION (type);
@@ -5699,22 +5739,22 @@ ipcp_update_vr (struct cgraph_node *node)
{
fprintf (dump_file, "Setting value range of param %u ", i);
fprintf (dump_file, "%s[",
- (vr[i].type == VR_ANTI_RANGE) ? "~" : "");
- print_decs (vr[i].min, dump_file);
+ (vr[i]->type == VR_ANTI_RANGE) ? "~" : "");
+ print_decs (vr[i]->get_min (), dump_file);
fprintf (dump_file, ", ");
- print_decs (vr[i].max, dump_file);
+ print_decs (vr[i]->get_max (), dump_file);
fprintf (dump_file, "]\n");
}
- set_range_info (ddef, vr[i].type,
- wide_int_storage::from (vr[i].min, prec,
+ set_range_info (ddef, vr[i]->type,
+ wide_int_storage::from (vr[i]->get_min (), prec,
TYPE_SIGN (type)),
- wide_int_storage::from (vr[i].max, prec,
+ wide_int_storage::from (vr[i]->get_max (), prec,
TYPE_SIGN (type)));
}
else if (POINTER_TYPE_P (TREE_TYPE (ddef))
- && vr[i].type == VR_ANTI_RANGE
- && wi::eq_p (vr[i].min, 0)
- && wi::eq_p (vr[i].max, 0))
+ && vr[i]->type == VR_ANTI_RANGE
+ && wi::eq_p (vr[i]->get_min (), 0)
+ && wi::eq_p (vr[i]->get_max (), 0))
{
if (dump_file)
fprintf (dump_file, "Setting nonnull for %u\n", i);
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 93a2390c..acab2aa 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -157,13 +157,15 @@ struct GTY(()) ipa_bits
};
/* Info about value ranges. */
-struct GTY(()) ipa_vr
+struct GTY ((variable_size)) ipa_vr
{
/* The data fields below are valid only if known is true. */
bool known;
enum value_range_type type;
- wide_int min;
- wide_int max;
+ /* Minimum and maximum. */
+ TRAILING_WIDE_INT_ACCESSOR (min, ints, 0)
+ TRAILING_WIDE_INT_ACCESSOR (max, ints, 1)
+ trailing_wide_ints <2> ints;
};
/* A jump function for a callsite represents the values passed as actual
@@ -525,7 +527,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;
};
void ipa_set_node_agg_value_chain (struct cgraph_node *node,
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-01-30 0:04 [RFC] Bug lto/78140 kugan
@ 2017-01-30 10:13 ` Richard Biener
2017-02-02 1:37 ` kugan
2017-02-02 2:59 ` kugan
0 siblings, 2 replies; 9+ messages in thread
From: Richard Biener @ 2017-01-30 10:13 UTC (permalink / raw)
To: kugan; +Cc: gcc-patches, Jan Hubicka, Martin Jambor
On Mon, Jan 30, 2017 at 12:23 AM, kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> Hi All,
>
> As suggested by Richard in the PR, I tried to implement variable size
> structures for VR as shown in attached patch. That is, I changed ipa-prop.h
> to:
>
> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
> index 93a2390c..acab2aa 100644
> --- a/gcc/ipa-prop.h
> +++ b/gcc/ipa-prop.h
> @@ -157,13 +157,15 @@ struct GTY(()) ipa_bits
> };
>
> /* Info about value ranges. */
> -struct GTY(()) ipa_vr
> +struct GTY ((variable_size)) ipa_vr
> {
> /* The data fields below are valid only if known is true. */
> bool known;
> enum value_range_type type;
> - wide_int min;
> - wide_int max;
> + /* Minimum and maximum. */
> + TRAILING_WIDE_INT_ACCESSOR (min, ints, 0)
> + TRAILING_WIDE_INT_ACCESSOR (max, ints, 1)
> + trailing_wide_ints <2> ints;
> };
>
> /* A jump function for a callsite represents the values passed as actual
> @@ -525,7 +527,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;
> };
>
> void ipa_set_node_agg_value_chain (struct cgraph_node *node,
>
> However, I am running into error when I do LTO bootstrap that memory seems
> to have deallocated by the garbage collector. Since we have the reference to
> the memory allocated by ggc_internal_alloc in the vector (m_vr), I thought
> it will not be deallocated. But during the bootstrap, when in
> ipa_node_params_t::duplicate, it seems to have been deallocated as shown in
> the back trace. I dont understand internals of gc in gcc so any help is
> appreciated.
>
>
> lto1: internal compiler error: Segmentation fault
> 0xdedc4b crash_signal
> ../../gcc/gcc/toplev.c:333
> 0xb46680 ipa_node_params_t::duplicate(cgraph_node*, cgraph_node*,
> ipa_node_params*, ipa_node_params*)
> ../../gcc/gcc/ipa-prop.c:3819
> 0xb306a3
> function_summary<ipa_node_params*>::symtab_duplication(cgraph_node*,
> cgraph_node*, void*)
> ../../gcc/gcc/symbol-summary.h:187
> 0x85aba7 symbol_table::call_cgraph_duplication_hooks(cgraph_node*,
> cgraph_node*)
> ../../gcc/gcc/cgraph.c:488
> 0x8765bf cgraph_node::create_clone(tree_node*, long, int, bool,
> vec<cgraph_edge*, va_heap, vl_ptr>, bool, cgraph_node*, bitmap_head*, char
> const*)
> ../../gcc/gcc/cgraphclones.c:522
> 0x166fb3b clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
> ../../gcc/gcc/ipa-inline-transform.c:227
> 0x166fbd7 clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
> ../../gcc/gcc/ipa-inline-transform.c:242
> 0x1670893 inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap,
> vl_ptr>*, int*, bool, bool*)
> ../../gcc/gcc/ipa-inline-transform.c:449
> 0x1665bd3 inline_small_functions
> ../../gcc/gcc/ipa-inline.c:2024
> 0x1667157 ipa_inline
> ../../gcc/gcc/ipa-inline.c:2434
> 0x1667fa7 execute
> ../../gcc/gcc/ipa-inline.c:2845
>
>
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-01-30 10:13 ` Richard Biener
@ 2017-02-02 1:37 ` kugan
2017-02-02 12:48 ` Jan Hubicka
2017-02-02 12:55 ` Martin Liška
2017-02-02 2:59 ` kugan
1 sibling, 2 replies; 9+ messages in thread
From: kugan @ 2017-02-02 1:37 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, Jan Hubicka, Martin Jambor
[-- Attachment #1: Type: text/plain, Size: 3469 bytes --]
Hi Richard,
On 30/01/17 21:08, Richard Biener wrote:
> On Mon, Jan 30, 2017 at 12:23 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> lto1: internal compiler error: Segmentation fault
>> 0xdedc4b crash_signal
>> ../../gcc/gcc/toplev.c:333
>> 0xb46680 ipa_node_params_t::duplicate(cgraph_node*, cgraph_node*,
>> ipa_node_params*, ipa_node_params*)
>> ../../gcc/gcc/ipa-prop.c:3819
>> 0xb306a3
>> function_summary<ipa_node_params*>::symtab_duplication(cgraph_node*,
>> cgraph_node*, void*)
>> ../../gcc/gcc/symbol-summary.h:187
>> 0x85aba7 symbol_table::call_cgraph_duplication_hooks(cgraph_node*,
>> cgraph_node*)
>> ../../gcc/gcc/cgraph.c:488
>> 0x8765bf cgraph_node::create_clone(tree_node*, long, int, bool,
>> vec<cgraph_edge*, va_heap, vl_ptr>, bool, cgraph_node*, bitmap_head*, char
>> const*)
>> ../../gcc/gcc/cgraphclones.c:522
>> 0x166fb3b clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
>> ../../gcc/gcc/ipa-inline-transform.c:227
>> 0x166fbd7 clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
>> ../../gcc/gcc/ipa-inline-transform.c:242
>> 0x1670893 inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap,
>> vl_ptr>*, int*, bool, bool*)
>> ../../gcc/gcc/ipa-inline-transform.c:449
>> 0x1665bd3 inline_small_functions
>> ../../gcc/gcc/ipa-inline.c:2024
>> 0x1667157 ipa_inline
>> ../../gcc/gcc/ipa-inline.c:2434
>> 0x1667fa7 execute
>> ../../gcc/gcc/ipa-inline.c:2845
>>
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 also noticed that we don't create m_vr and bits vectors. Attached
patch does this. This was bootstrapped and regression tested with the
above patch. I am now testing the attached patch alone. Is this OK if
no regressions?
gcc/ChangeLog:
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
[-- Attachment #2: p1.txt --]
[-- Type: text/plain, Size: 4352 bytes --]
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);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-01-30 10:13 ` Richard Biener
2017-02-02 1:37 ` kugan
@ 2017-02-02 2:59 ` kugan
1 sibling, 0 replies; 9+ messages in thread
From: kugan @ 2017-02-02 2:59 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, Jan Hubicka, Martin Jambor
[-- Attachment #1: Type: text/plain, Size: 4956 bytes --]
Hi Richard,
On 30/01/17 21:08, Richard Biener wrote:
> On Mon, Jan 30, 2017 at 12:23 AM, kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Hi All,
>>
>> As suggested by Richard in the PR, I tried to implement variable size
>> structures for VR as shown in attached patch. That is, I changed ipa-prop.h
>> to:
>>
>> diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
>> index 93a2390c..acab2aa 100644
>> --- a/gcc/ipa-prop.h
>> +++ b/gcc/ipa-prop.h
>> @@ -157,13 +157,15 @@ struct GTY(()) ipa_bits
>> };
>>
>> /* Info about value ranges. */
>> -struct GTY(()) ipa_vr
>> +struct GTY ((variable_size)) ipa_vr
>> {
>> /* The data fields below are valid only if known is true. */
>> bool known;
>> enum value_range_type type;
>> - wide_int min;
>> - wide_int max;
>> + /* Minimum and maximum. */
>> + TRAILING_WIDE_INT_ACCESSOR (min, ints, 0)
>> + TRAILING_WIDE_INT_ACCESSOR (max, ints, 1)
>> + trailing_wide_ints <2> ints;
>> };
>>
>> /* A jump function for a callsite represents the values passed as actual
>> @@ -525,7 +527,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;
>> };
>>
>> void ipa_set_node_agg_value_chain (struct cgraph_node *node,
>>
>> However, I am running into error when I do LTO bootstrap that memory seems
>> to have deallocated by the garbage collector. Since we have the reference to
>> the memory allocated by ggc_internal_alloc in the vector (m_vr), I thought
>> it will not be deallocated. But during the bootstrap, when in
>> ipa_node_params_t::duplicate, it seems to have been deallocated as shown in
>> the back trace. I dont understand internals of gc in gcc so any help is
>> appreciated.
>>
>>
>> lto1: internal compiler error: Segmentation fault
>> 0xdedc4b crash_signal
>> ../../gcc/gcc/toplev.c:333
>> 0xb46680 ipa_node_params_t::duplicate(cgraph_node*, cgraph_node*,
>> ipa_node_params*, ipa_node_params*)
>> ../../gcc/gcc/ipa-prop.c:3819
>> 0xb306a3
>> function_summary<ipa_node_params*>::symtab_duplication(cgraph_node*,
>> cgraph_node*, void*)
>> ../../gcc/gcc/symbol-summary.h:187
>> 0x85aba7 symbol_table::call_cgraph_duplication_hooks(cgraph_node*,
>> cgraph_node*)
>> ../../gcc/gcc/cgraph.c:488
>> 0x8765bf cgraph_node::create_clone(tree_node*, long, int, bool,
>> vec<cgraph_edge*, va_heap, vl_ptr>, bool, cgraph_node*, bitmap_head*, char
>> const*)
>> ../../gcc/gcc/cgraphclones.c:522
>> 0x166fb3b clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
>> ../../gcc/gcc/ipa-inline-transform.c:227
>> 0x166fbd7 clone_inlined_nodes(cgraph_edge*, bool, bool, int*, int)
>> ../../gcc/gcc/ipa-inline-transform.c:242
>> 0x1670893 inline_call(cgraph_edge*, bool, vec<cgraph_edge*, va_heap,
>> vl_ptr>*, int*, bool, bool*)
>> ../../gcc/gcc/ipa-inline-transform.c:449
>> 0x1665bd3 inline_small_functions
>> ../../gcc/gcc/ipa-inline.c:2024
>> 0x1667157 ipa_inline
>> ../../gcc/gcc/ipa-inline.c:2434
>> 0x1667fa7 execute
>> ../../gcc/gcc/ipa-inline.c:2845
>>
>>
>> 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!).
>
We now have ipa_vr (with wide_int) for ipcp_transaction_summary.
value_range is used in jump functions as it uses tree-vrp for most of
the handling. Attached patch uses variable_structure for ipa_vr. A
version of this patch passed regression and lto bootstrapped (before I
separated the part that prevented testing and posted that separately in
https://gcc.gnu.org/ml/gcc-patches/2017-02/msg00103.html). Does this
approach looks OK? I will do full testing and post again based on the
feedback.
I am also going to do the same for ipa-bits based on the feedback. Do
you also want to do variable structure with widest_ints?
Thanks,
Kugan
[-- Attachment #2: 0002-p2.patch --]
[-- Type: text/x-diff, Size: 8293 bytes --]
From e2cd620b8876b67066fc2781f68adaa087094669 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandarajah@linaro.org>
Date: Thu, 2 Feb 2017 13:37:27 +1100
Subject: [PATCH 2/2] p2
---
gcc/ipa-cp.c | 24 +++++++++++++-------
gcc/ipa-prop.c | 69 ++++++++++++++++++++++++++++++++++++----------------------
gcc/ipa-prop.h | 12 +++++-----
3 files changed, 66 insertions(+), 39 deletions(-)
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 5103555..79c9894 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -4949,21 +4949,29 @@ ipcp_store_vr_results (void)
for (unsigned i = 0; i < count; i++)
{
ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i);
- ipa_vr vr;
+ ipa_vr *vr;
if (!plats->m_value_range.bottom_p ()
&& !plats->m_value_range.top_p ())
{
- vr.known = true;
- vr.type = plats->m_value_range.m_vr.type;
- vr.min = plats->m_value_range.m_vr.min;
- vr.max = plats->m_value_range.m_vr.max;
+ wide_int min = plats->m_value_range.m_vr.min;
+ wide_int max = plats->m_value_range.m_vr.max;
+ unsigned int precision = min.get_precision ();
+ size_t size = (sizeof (ipa_vr)
+ + trailing_wide_ints <2>::extra_size (precision));
+ vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ vr->ints.set_precision (precision);
+ vr->known = true;
+ vr->type = plats->m_value_range.m_vr.type;
+ vr->set_min (min);
+ vr->set_max (max);
}
else
{
- vr.known = false;
- vr.type = VR_VARYING;
- vr.min = vr.max = wi::zero (INT_TYPE_SIZE);
+ size_t size = (sizeof (ipa_vr));
+ vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ vr->known = false;
+ vr->type = VR_VARYING;
}
ts->m_vr->quick_push (vr);
}
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index b992a7f..81f83b8 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3817,11 +3817,11 @@ 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;
+ const vec<ipa_vr *, va_gc> *src_vr = src_trans->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;
+ 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 ());
@@ -5224,16 +5224,16 @@ write_ipcp_transformation_info (output_block *ob, cgraph_node *node)
for (unsigned i = 0; i < count; ++i)
{
struct bitpack_d bp;
- ipa_vr *parm_vr = &(*ts->m_vr)[i];
+ ipa_vr *parm_vr = (*ts->m_vr)[i];
bp = bitpack_create (ob->main_stream);
bp_pack_value (&bp, parm_vr->known, 1);
streamer_write_bitpack (&bp);
if (parm_vr->known)
{
- streamer_write_enum (ob->main_stream, value_rang_type,
+ streamer_write_enum (ob->main_stream, value_range_type,
VR_LAST, parm_vr->type);
- streamer_write_wide_int (ob, parm_vr->min);
- streamer_write_wide_int (ob, parm_vr->max);
+ streamer_write_wide_int (ob, parm_vr->get_min ());
+ streamer_write_wide_int (ob, parm_vr->get_max ());
}
}
}
@@ -5296,21 +5296,38 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node,
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);
+ vec_safe_reserve_exact (ts->m_vr, count);
for (i = 0; i < count; i++)
{
ipa_vr *parm_vr;
- parm_vr = &(*ts->m_vr)[i];
struct bitpack_d bp;
bp = streamer_read_bitpack (ib);
- parm_vr->known = bp_unpack_value (&bp, 1);
- if (parm_vr->known)
+ bool known = bp_unpack_value (&bp, 1);
+ if (known)
+ {
+ enum value_range_type type = streamer_read_enum (ib,
+ value_range_type,
+ VR_LAST);
+ wide_int min = streamer_read_wide_int (ib);
+ wide_int max = streamer_read_wide_int (ib);
+ unsigned int precision = min.get_precision ();
+ size_t size = (sizeof (ipa_vr)
+ + trailing_wide_ints <2>::extra_size (precision));
+ parm_vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ parm_vr->ints.set_precision (precision);
+ parm_vr->known = known;
+ parm_vr->type = type;
+ parm_vr->set_min (min);
+ parm_vr->set_max (max);
+ }
+ else
{
- parm_vr->type = streamer_read_enum (ib, value_range_type,
- VR_LAST);
- parm_vr->min = streamer_read_wide_int (ib);
- parm_vr->max = streamer_read_wide_int (ib);
+ size_t size = (sizeof (ipa_vr));
+ parm_vr = static_cast<ipa_vr *> (ggc_internal_alloc (size));
+ parm_vr->known = known;
+ parm_vr->type = VR_VARYING;
}
+ ts->m_vr->quick_push (parm_vr);
}
}
count = streamer_read_uhwi (ib);
@@ -5688,7 +5705,7 @@ ipcp_update_vr (struct cgraph_node *node)
ipcp_transformation_summary *ts = ipcp_get_transformation_summary (node);
if (!ts || vec_safe_length (ts->m_vr) == 0)
return;
- const vec<ipa_vr, va_gc> &vr = *ts->m_vr;
+ const vec<ipa_vr *, va_gc> &vr = *ts->m_vr;
unsigned count = vr.length ();
for (unsigned i = 0; i < count; ++i, parm = next_parm)
@@ -5703,8 +5720,8 @@ ipcp_update_vr (struct cgraph_node *node)
if (!ddef || !is_gimple_reg (parm))
continue;
- if (vr[i].known
- && (vr[i].type == VR_RANGE || vr[i].type == VR_ANTI_RANGE))
+ if (vr[i]->known
+ && (vr[i]->type == VR_RANGE || vr[i]->type == VR_ANTI_RANGE))
{
tree type = TREE_TYPE (ddef);
unsigned prec = TYPE_PRECISION (type);
@@ -5714,22 +5731,22 @@ ipcp_update_vr (struct cgraph_node *node)
{
fprintf (dump_file, "Setting value range of param %u ", i);
fprintf (dump_file, "%s[",
- (vr[i].type == VR_ANTI_RANGE) ? "~" : "");
- print_decs (vr[i].min, dump_file);
+ (vr[i]->type == VR_ANTI_RANGE) ? "~" : "");
+ print_decs (vr[i]->get_min (), dump_file);
fprintf (dump_file, ", ");
- print_decs (vr[i].max, dump_file);
+ print_decs (vr[i]->get_max (), dump_file);
fprintf (dump_file, "]\n");
}
- set_range_info (ddef, vr[i].type,
- wide_int_storage::from (vr[i].min, prec,
+ set_range_info (ddef, vr[i]->type,
+ wide_int_storage::from (vr[i]->get_min (), prec,
TYPE_SIGN (type)),
- wide_int_storage::from (vr[i].max, prec,
+ wide_int_storage::from (vr[i]->get_max (), prec,
TYPE_SIGN (type)));
}
else if (POINTER_TYPE_P (TREE_TYPE (ddef))
- && vr[i].type == VR_ANTI_RANGE
- && wi::eq_p (vr[i].min, 0)
- && wi::eq_p (vr[i].max, 0))
+ && vr[i]->type == VR_ANTI_RANGE
+ && wi::eq_p (vr[i]->get_min (), 0)
+ && wi::eq_p (vr[i]->get_max (), 0))
{
if (dump_file)
fprintf (dump_file, "Setting nonnull for %u\n", i);
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index 6573a78..ffbf007 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -157,13 +157,15 @@ struct GTY(()) ipa_bits
};
/* Info about value ranges. */
-struct GTY(()) ipa_vr
+struct GTY ((variable_size)) ipa_vr
{
/* The data fields below are valid only if known is true. */
bool known;
enum value_range_type type;
- wide_int min;
- wide_int max;
+ /* Minimum and maximum. */
+ TRAILING_WIDE_INT_ACCESSOR (min, ints, 0)
+ TRAILING_WIDE_INT_ACCESSOR (max, ints, 1)
+ trailing_wide_ints <2> ints;
};
/* A jump function for a callsite represents the values passed as actual
@@ -525,10 +527,10 @@ 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;
};
-typedef vec<ipa_vr, va_gc> ipa_vr_vec;
+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,
--
2.7.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-02-02 1:37 ` kugan
@ 2017-02-02 12:48 ` Jan Hubicka
2017-02-02 12:52 ` Richard Biener
2017-02-02 16:02 ` Martin Jambor
2017-02-02 12:55 ` Martin Liška
1 sibling, 2 replies; 9+ messages in thread
From: Jan Hubicka @ 2017-02-02 12:48 UTC (permalink / raw)
To: kugan; +Cc: Richard Biener, gcc-patches, Jan Hubicka, Martin Jambor
>
> 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);
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-02-02 12:48 ` Jan Hubicka
@ 2017-02-02 12:52 ` Richard Biener
2017-02-02 12:57 ` Richard Biener
2017-02-02 16:02 ` Martin Jambor
1 sibling, 1 reply; 9+ messages in thread
From: Richard Biener @ 2017-02-02 12:52 UTC (permalink / raw)
To: Jan Hubicka; +Cc: kugan, gcc-patches, Martin Jambor
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);
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-02-02 1:37 ` kugan
2017-02-02 12:48 ` Jan Hubicka
@ 2017-02-02 12:55 ` Martin Liška
1 sibling, 0 replies; 9+ messages in thread
From: Martin Liška @ 2017-02-02 12:55 UTC (permalink / raw)
To: kugan, Richard Biener; +Cc: gcc-patches, Jan Hubicka, Martin Jambor
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-02-02 12:52 ` Richard Biener
@ 2017-02-02 12:57 ` Richard Biener
0 siblings, 0 replies; 9+ messages in thread
From: Richard Biener @ 2017-02-02 12:57 UTC (permalink / raw)
To: Jan Hubicka; +Cc: kugan, gcc-patches, Martin Jambor
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);
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RFC] Bug lto/78140
2017-02-02 12:48 ` Jan Hubicka
2017-02-02 12:52 ` Richard Biener
@ 2017-02-02 16:02 ` Martin Jambor
1 sibling, 0 replies; 9+ messages in thread
From: Martin Jambor @ 2017-02-02 16:02 UTC (permalink / raw)
To: Jan Hubicka; +Cc: kugan, Richard Biener, gcc-patches
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-02-02 16:02 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-30 0:04 [RFC] Bug lto/78140 kugan
2017-01-30 10:13 ` Richard Biener
2017-02-02 1:37 ` kugan
2017-02-02 12:48 ` Jan Hubicka
2017-02-02 12:52 ` Richard Biener
2017-02-02 12:57 ` Richard Biener
2017-02-02 16:02 ` Martin Jambor
2017-02-02 12:55 ` Martin Liška
2017-02-02 2:59 ` kugan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).