Hi,
On Wed, Nov 16 2022, Jan Hubicka wrote:
>> IPA-CP transformation summary streaming code currently won't stream
>> out transformations necessary for clones which are only necessary for
>> materialization of other clones (such as an IPA-CP clone which is then
>> cloned again by IPA-SRA). However, a follow-up patch for bettor
>> reconciling IPA-SRA and IPA-CP modifications requires to have that
>> information at its disposal and so this one reworks the streaming to
>> write out all non-empty transformation summaries.
>>
>> This should actually mean less streaming in typical case because
>> previously we streamed three zeros for all nodes in a partition with
>> no useful information associated with them. Currently we don't stream
>> anything for those.
>>
>> When reworking the streaming, I also simplified it a little a
>> converted it writing to nicer C++ vector iterations.
>>
>> Bootstrapped and tested on x86_64-linux. OK for master?
>>
>> Thanks,
>>
>> Martin
>>
>>
>> gcc/ChangeLog:
>>
>> 2022-11-11 Martin Jambor <mjam...@suse.cz>
>>
>> * ipa-prop.cc (useful_ipcp_transformation_info_p): New function.
>> (write_ipcp_transformation_info): Added a parameter, simplified
>> given that is known not to be NULL.
>> (ipcp_write_transformation_summaries): Write out all useful
>> transformation summaries.
>> (read_ipcp_transformation_info): Simplify given that some info
>> will be read.
>> (read_replacements_section): Remove assert.
>> ---
>> gcc/ipa-prop.cc | 151 +++++++++++++++++++++++-------------------------
>> 1 file changed, 71 insertions(+), 80 deletions(-)
>>
>> diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
>> index e6cf25591b3..cfd12a97b36 100644
>> --- a/gcc/ipa-prop.cc
>> +++ b/gcc/ipa-prop.cc
>> @@ -5279,80 +5279,72 @@ ipa_prop_read_jump_functions (void)
>> }
>> }
>>
>> -void
>> -write_ipcp_transformation_info (output_block *ob, cgraph_node *node)
>> +/* Return true if the IPA-CP transformation summary TS is non-NULL and
>> contains
>> + useful info. */
>> +static bool
>> +useful_ipcp_transformation_info_p (ipcp_transformation *ts)
>> {
>> - int node_ref;
>> - unsigned int count = 0;
>> - lto_symtab_encoder_t encoder;
>> + if (!ts)
>> + return false;
>> + if (!vec_safe_is_empty (ts->m_agg_values)
>> + || !vec_safe_is_empty (ts->bits)
>> + || !vec_safe_is_empty (ts->m_vr))
>> + return true;
>> + return false;
>> +}
>
> This way we stream transformation info for everything in the boundary.
> Even for functions that belongs to other partitions and they are called
> from current partition. Perhaps we want to check that the function has
> body streamed?
>
Please see the updated patch below, which has survived bootstrap,
LTO-bootstrap and testing.
> Also would be possible to make a testcase? I think it should be possible
> to use -flto-partition=max and builtin_constant_p checks verifying that
> intended transformation happens.
>
make -k check-gcc RUNTESTFLAGS="execute.exp=20001017-2.c"
is a testcase which ICEs if IPA-CP info is not streamed and the
subsequent patch is applied.
Thanks,
Martin
IPA-CP transformation summary streaming code currently won't stream
out transformations necessary for clones which are only necessary for
materialization of other clones (such as an IPA-CP clone which is then
cloned again by IPA-SRA). However, a follow-up patch for bettor
reconciling IPA-SRA and IPA-CP modifications requires to have that
information at its disposal and so this one reworks the streaming to
write out all non-empty transformation summaries.
In order not to stream transformation summaies into partitions where
the node itself nor any of its clones are materialized, I had to make
sure that clones also get encode_body flag in the encoder (so that it
could be tested) and therefore in turn lto_output understands it needs
to skip clones.
This should actually mean less streaming in typical case because
previously we streamed three zeros for all nodes in a partition with
no useful information associated with them. Currently we don't stream
anything for those.
When reworking the streaming, I also simplified it a little a
converted it writing to nicer C++ vector iterations.
gcc/ChangeLog:
2022-11-25 Martin Jambor <mjam...@suse.cz>
* ipa-prop.cc (useful_ipcp_transformation_info_p): New function.
(write_ipcp_transformation_info): Added a parameter, simplified
given that is known not to be NULL.
(ipcp_write_transformation_summaries): Write out all useful
transformation summaries.
(read_ipcp_transformation_info): Simplify given that some info
will be read.
(read_replacements_section): Remove assert.
* lto-cgraph.cc (add_node_to): Also set encode_body for clones.
* lto-streamer-out.cc (lto_output): Do not output virtual clones.
---
gcc/ipa-prop.cc | 153 +++++++++++++++++++---------------------
gcc/lto-cgraph.cc | 2 +-
gcc/lto-streamer-out.cc | 3 +-
3 files changed, 76 insertions(+), 82 deletions(-)
diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc
index e6cf25591b3..fcadf64ead7 100644
--- a/gcc/ipa-prop.cc
+++ b/gcc/ipa-prop.cc
@@ -5279,80 +5279,72 @@ ipa_prop_read_jump_functions (void)
}
}
-void
-write_ipcp_transformation_info (output_block *ob, cgraph_node *node)
+/* Return true if the IPA-CP transformation summary TS is non-NULL and contains
+ useful info. */
+static bool
+useful_ipcp_transformation_info_p (ipcp_transformation *ts)
{
- int node_ref;
- unsigned int count = 0;
- lto_symtab_encoder_t encoder;
+ if (!ts)
+ return false;
+ if (!vec_safe_is_empty (ts->m_agg_values)
+ || !vec_safe_is_empty (ts->bits)
+ || !vec_safe_is_empty (ts->m_vr))
+ return true;
+ return false;
+}
- encoder = ob->decl_state->symtab_node_encoder;
- node_ref = lto_symtab_encoder_encode (encoder, node);
+/* Write into OB IPA-CP transfromation summary TS describing NODE. */
+
+void
+write_ipcp_transformation_info (output_block *ob, cgraph_node *node,
+ ipcp_transformation *ts)
+{
+ lto_symtab_encoder_t encoder = ob->decl_state->symtab_node_encoder;
+ int node_ref = lto_symtab_encoder_encode (encoder, node);
streamer_write_uhwi (ob, node_ref);
- ipcp_transformation *ts = ipcp_get_transformation_summary (node);
- if (ts && !vec_safe_is_empty (ts->m_agg_values))
+ streamer_write_uhwi (ob, vec_safe_length (ts->m_agg_values));
+ for (const ipa_argagg_value &av : ts->m_agg_values)
{
- streamer_write_uhwi (ob, ts->m_agg_values->length ());
- for (const ipa_argagg_value &av : ts->m_agg_values)
+ struct bitpack_d bp;
+
+ stream_write_tree (ob, av.value, true);
+ streamer_write_uhwi (ob, av.unit_offset);
+ streamer_write_uhwi (ob, av.index);
+
+ bp = bitpack_create (ob->main_stream);
+ bp_pack_value (&bp, av.by_ref, 1);
+ streamer_write_bitpack (&bp);
+ }
+
+ streamer_write_uhwi (ob, vec_safe_length (ts->m_vr));
+ for (const ipa_vr &parm_vr : ts->m_vr)
+ {
+ struct bitpack_d bp;
+ bp = bitpack_create (ob->main_stream);
+ bp_pack_value (&bp, parm_vr.known, 1);
+ streamer_write_bitpack (&bp);
+ if (parm_vr.known)
{
- struct bitpack_d bp;
-
- stream_write_tree (ob, av.value, true);
- streamer_write_uhwi (ob, av.unit_offset);
- streamer_write_uhwi (ob, av.index);
-
- bp = bitpack_create (ob->main_stream);
- bp_pack_value (&bp, av.by_ref, 1);
- streamer_write_bitpack (&bp);
+ 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);
}
}
- else
- streamer_write_uhwi (ob, 0);
- if (ts && vec_safe_length (ts->m_vr) > 0)
+ streamer_write_uhwi (ob, vec_safe_length (ts->bits));
+ for (const ipa_bits *bits_jfunc : ts->bits)
{
- count = ts->m_vr->length ();
- streamer_write_uhwi (ob, count);
- for (unsigned i = 0; i < count; ++i)
+ struct bitpack_d bp = bitpack_create (ob->main_stream);
+ bp_pack_value (&bp, !!bits_jfunc, 1);
+ streamer_write_bitpack (&bp);
+ if (bits_jfunc)
{
- struct bitpack_d bp;
- 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,
- VR_LAST, parm_vr->type);
- streamer_write_wide_int (ob, parm_vr->min);
- streamer_write_wide_int (ob, parm_vr->max);
- }
+ streamer_write_widest_int (ob, bits_jfunc->value);
+ streamer_write_widest_int (ob, bits_jfunc->mask);
}
}
- else
- streamer_write_uhwi (ob, 0);
-
- if (ts && vec_safe_length (ts->bits) > 0)
- {
- count = ts->bits->length ();
- streamer_write_uhwi (ob, count);
-
- for (unsigned i = 0; i < count; ++i)
- {
- const ipa_bits *bits_jfunc = (*ts->bits)[i];
- struct bitpack_d bp = bitpack_create (ob->main_stream);
- bp_pack_value (&bp, !!bits_jfunc, 1);
- streamer_write_bitpack (&bp);
- if (bits_jfunc)
- {
- streamer_write_widest_int (ob, bits_jfunc->value);
- streamer_write_widest_int (ob, bits_jfunc->mask);
- }
- }
- }
- else
- streamer_write_uhwi (ob, 0);
}
/* Stream in the aggregate value replacement chain for NODE from IB. */
@@ -5362,12 +5354,12 @@ read_ipcp_transformation_info (lto_input_block *ib,
cgraph_node *node,
data_in *data_in)
{
unsigned int count, i;
+ ipcp_transformation_initialize ();
+ ipcp_transformation *ts = ipcp_transformation_sum->get_create (node);
count = streamer_read_uhwi (ib);
if (count > 0)
{
- ipcp_transformation_initialize ();
- ipcp_transformation *ts = ipcp_transformation_sum->get_create (node);
vec_safe_grow_cleared (ts->m_agg_values, count, true);
for (i = 0; i <count; i++)
{
@@ -5385,8 +5377,6 @@ read_ipcp_transformation_info (lto_input_block *ib,
cgraph_node *node,
count = streamer_read_uhwi (ib);
if (count > 0)
{
- ipcp_transformation_initialize ();
- ipcp_transformation *ts = ipcp_transformation_sum->get_create (node);
vec_safe_grow_cleared (ts->m_vr, count, true);
for (i = 0; i < count; i++)
{
@@ -5407,10 +5397,7 @@ read_ipcp_transformation_info (lto_input_block *ib,
cgraph_node *node,
count = streamer_read_uhwi (ib);
if (count > 0)
{
- ipcp_transformation_initialize ();
- ipcp_transformation *ts = ipcp_transformation_sum->get_create (node);
vec_safe_grow_cleared (ts->bits, count, true);
-
for (i = 0; i < count; i++)
{
struct bitpack_d bp = streamer_read_bitpack (ib);
@@ -5432,31 +5419,38 @@ read_ipcp_transformation_info (lto_input_block *ib,
cgraph_node *node,
void
ipcp_write_transformation_summaries (void)
{
- struct cgraph_node *node;
struct output_block *ob;
unsigned int count = 0;
- lto_symtab_encoder_iterator lsei;
lto_symtab_encoder_t encoder;
ob = create_output_block (LTO_section_ipcp_transform);
encoder = ob->decl_state->symtab_node_encoder;
ob->symbol = NULL;
- for (lsei = lsei_start_function_in_partition (encoder); !lsei_end_p (lsei);
- lsei_next_function_in_partition (&lsei))
+
+ for (int i = 0; i < lto_symtab_encoder_size (encoder); i++)
{
- node = lsei_cgraph_node (lsei);
- if (node->has_gimple_body_p ())
+ symtab_node *snode = lto_symtab_encoder_deref (encoder, i);
+ cgraph_node *cnode = dyn_cast <cgraph_node *> (snode);
+ if (!cnode)
+ continue;
+ ipcp_transformation *ts = ipcp_get_transformation_summary (cnode);
+ if (useful_ipcp_transformation_info_p (ts)
+ && lto_symtab_encoder_encode_body_p (encoder, cnode))
count++;
}
streamer_write_uhwi (ob, count);
- for (lsei = lsei_start_function_in_partition (encoder); !lsei_end_p (lsei);
- lsei_next_function_in_partition (&lsei))
+ for (int i = 0; i < lto_symtab_encoder_size (encoder); i++)
{
- node = lsei_cgraph_node (lsei);
- if (node->has_gimple_body_p ())
- write_ipcp_transformation_info (ob, node);
+ symtab_node *snode = lto_symtab_encoder_deref (encoder, i);
+ cgraph_node *cnode = dyn_cast <cgraph_node *> (snode);
+ if (!cnode)
+ continue;
+ ipcp_transformation *ts = ipcp_get_transformation_summary (cnode);
+ if (useful_ipcp_transformation_info_p (ts)
+ && lto_symtab_encoder_encode_body_p (encoder, cnode))
+ write_ipcp_transformation_info (ob, cnode, ts);
}
streamer_write_char_stream (ob->main_stream, 0);
produce_asm (ob, NULL);
@@ -5497,7 +5491,6 @@ read_replacements_section (struct lto_file_decl_data
*file_data,
encoder = file_data->symtab_node_encoder;
node = dyn_cast<cgraph_node *> (lto_symtab_encoder_deref (encoder,
index));
- gcc_assert (node->definition);
read_ipcp_transformation_info (&ib_main, node, data_in);
}
lto_free_section_data (file_data, LTO_section_jump_functions, NULL, data,
diff --git a/gcc/lto-cgraph.cc b/gcc/lto-cgraph.cc
index 350195d86db..11079b0f0f0 100644
--- a/gcc/lto-cgraph.cc
+++ b/gcc/lto-cgraph.cc
@@ -797,7 +797,7 @@ add_node_to (lto_symtab_encoder_t encoder, struct
cgraph_node *node,
{
if (node->clone_of)
add_node_to (encoder, node->clone_of, include_body);
- else if (include_body)
+ if (include_body)
lto_set_symtab_encoder_encode_body (encoder, node);
lto_symtab_encoder_encode (encoder, node);
}
diff --git a/gcc/lto-streamer-out.cc b/gcc/lto-streamer-out.cc
index 1e389049062..08f75b09204 100644
--- a/gcc/lto-streamer-out.cc
+++ b/gcc/lto-streamer-out.cc
@@ -2752,7 +2752,8 @@ lto_output (void)
continue;
if (cgraph_node *node = dyn_cast <cgraph_node *> (snode))
{
- if (lto_symtab_encoder_encode_body_p (encoder, node))
+ if (lto_symtab_encoder_encode_body_p (encoder, node)
+ && !node->clone_of)
symbols_to_copy.safe_push (node);
}
else if (varpool_node *node = dyn_cast <varpool_node *> (snode))
--
2.38.1
