From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id 52AB8385B19A for ; Mon, 28 Nov 2022 15:46:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 52AB8385B19A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.cz Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 05E7E1FD6C; Mon, 28 Nov 2022 15:46:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1669650377; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=l01GVxTlOXXY8coiwYutg1TzBnbP9Q07Rhlx6HrK5DU=; b=QxEOQFhLWYJKyPDUEz9Q5vJtUyPhd+D6hzeILzKUtnj8f6JxnKhzF2TWpztwxnQNzl+uIF +cmfqnb8xYGTmmdILbVhJaoOUkWkqLx2ERie2ZuvGHUKbnBTm7xRJ0JTqqd9d8zFlQdtzO vGwg5rOsVqNBgey1J5gqerLTkWJu8jM= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1669650377; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=l01GVxTlOXXY8coiwYutg1TzBnbP9Q07Rhlx6HrK5DU=; b=eb7iov3uDAJsUTQ/0oBi3626MUtDbSnUGHPCx/za5dTjqrsHgiCPFKwMY9babMWe8OqWoh /V8iNl1UFMYNfrBA== Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id ED58213273; Mon, 28 Nov 2022 15:46:16 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id ExPtOcjXhGPZTwAAMHmgww (envelope-from ); Mon, 28 Nov 2022 15:46:16 +0000 From: Martin Jambor To: Jan Hubicka Cc: GCC Patches Subject: Re: [PATCH 03/12] ipa-cp: Write transformation summaries of all functions In-Reply-To: References: User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/28.1 (x86_64-suse-linux-gnu) Date: Mon, 28 Nov 2022 16:46:16 +0100 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 >> >> * 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 * 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 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 (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 (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 (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 (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 (snode)) -- 2.38.1