Hi Richard,
On 30/01/17 21:08, Richard Biener wrote:
On Mon, Jan 30, 2017 at 12:23 AM, kugan
<kugan.vivekanandara...@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
>From e2cd620b8876b67066fc2781f68adaa087094669 Mon Sep 17 00:00:00 2001
From: Kugan Vivekanandarajah <kugan.vivekanandara...@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
