From patchwork Thu Oct 5 12:06:47 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1843883 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=suse.cz header.i=@suse.cz header.a=rsa-sha256 header.s=susede2_rsa header.b=GfAqbVsH; dkim=pass header.d=suse.cz header.i=@suse.cz header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=ydBp5z9c; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4S1Vz42BZ2z1yqD for ; Thu, 5 Oct 2023 23:21:20 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 709B13882662 for ; Thu, 5 Oct 2023 12:21:06 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 3EEE63857701 for ; Thu, 5 Oct 2023 12:20:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3EEE63857701 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-out1.suse.de (Postfix) with ESMTPS id 6D0AB21870 for ; Thu, 5 Oct 2023 12:20:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1696508421; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: resent-to:resent-from:resent-message-id:in-reply-to:in-reply-to: references:references; bh=LFItY6AGoMDDmUH4QCwrGz811EDp2HliApsOTd2UFDc=; b=GfAqbVsH8UKpBkPOCeFmolUKN5wFMFklZZkEbu44fQKf6OqaPyw+HBTW7YstgaqS1Tsegf 5mGrrtYP2zSxaHhnuaycDuB2a4+Mcd03FKkqgIsJ96ujU84ChwAFm90HwxIqiDW4xMZ+qw HaLHhhFTRcenNR6EAlvi+55qq3ocvTg= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1696508421; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: resent-to:resent-from:resent-message-id:in-reply-to:in-reply-to: references:references; bh=LFItY6AGoMDDmUH4QCwrGz811EDp2HliApsOTd2UFDc=; b=ydBp5z9c9WAkuelTTZbdEB/YvEVvrIWo+WvWUbr+U0mCgMFOYcczO32sS0fo+yy+9B/gw0 UPPBicPBh6a9ccAA== 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 565E3139C2 for ; Thu, 5 Oct 2023 12:20:21 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id R44SFQWqHmUmZQAAMHmgww (envelope-from ) for ; Thu, 05 Oct 2023 12:20:21 +0000 Resent-From: Martin Jambor Resent-Date: Thu, 5 Oct 2023 14:20:20 +0200 Resent-Message-ID: Resent-To: gcc-patches@gcc.gnu.org Message-ID: In-Reply-To: References: From: Martin Jambor Date: Thu, 5 Oct 2023 14:06:47 +0200 Subject: [PATCH 2/3] ipa: Prune any IPA-CP aggregate constants known by modref to be killed (111157) To: GCC Patches Cc: Jan Hubicka X-Spam-Status: No, score=-11.5 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 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org PR 111157 shows that IPA-modref and IPA-CP (when plugged into value numbering) can optimize out a store both before a call (because the call will overwrite it) and in the call (because the store is of the same value) and by eliminating both create miscompilation. This patch fixes that by pruning any constants from the list of IPA-CP aggregate value constants that it knows the contents of the memory can be "killed." Unfortunately, doing so is tricky. First, IPA-modref loads override kills and so only stores not loaded are truly not necessary. Looking stuff up there means doing what most of what modref_may_alias may do but doing exactly what it does is tricky because it takes also aliasing into account and has bail-out counters. To err on the side of caution in order to avoid this miscompilation we have to prune a constant when in doubt. However, pruning can interfere with the mechanism of how clone materialization distinguishes between the cases when a parameter was entirely removed and when it was both IPA-CPed and IPA-SRAed (in order to make up for the removal in debug info, which can bump into an assert when compiling g++.dg/torture/pr103669.C when we are not careful). Therefore this patch: 1) marks constants that IPA-modref has in its kill list with a new "killed" flag, and 2) prunes the list from entries with this flag after materialization and IPA-CP transformation is done using the template introduced in the previous patch It does not try to look up anything in the load lists, this will be done as a follow-up in order to ease review. gcc/ChangeLog: 2023-09-19 Martin Jambor PR ipa/111157 * ipa-prop.h (struct ipa_argagg_value): Newf flag killed. * ipa-modref.cc (ipcp_argagg_and_kill_overlap_p): New function. (update_signature): Mark any any IPA-CP aggregate constants at positions known to be killed as killed. Move check that there is clone_info after this pruning. * ipa-cp.cc (ipa_argagg_value_list::dump): Dump the killed flag. (ipa_argagg_value_list::push_adjusted_values): Clear the new flag. (push_agg_values_from_plats): Likewise. (ipa_push_agg_values_from_jfunc): Likewise. (estimate_local_effects): Likewise. (push_agg_values_for_index_from_edge): Likewise. * ipa-prop.cc (write_ipcp_transformation_info): Stream the killed flag. (read_ipcp_transformation_info): Likewise. (ipcp_get_aggregate_const): Update comment, assert that encountered record does not have killed flag set. (ipcp_transform_function): Prune all aggregate constants with killed set. gcc/testsuite/ChangeLog: 2023-09-18 Martin Jambor PR ipa/111157 * gcc.dg/lto/pr111157_0.c: New test. * gcc.dg/lto/pr111157_1.c: Second file of the same new test. --- gcc/ipa-cp.cc | 8 ++++ gcc/ipa-modref.cc | 58 +++++++++++++++++++++++++-- gcc/ipa-prop.cc | 17 +++++++- gcc/ipa-prop.h | 4 ++ gcc/testsuite/gcc.dg/lto/pr111157_0.c | 24 +++++++++++ gcc/testsuite/gcc.dg/lto/pr111157_1.c | 10 +++++ 6 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/lto/pr111157_0.c create mode 100644 gcc/testsuite/gcc.dg/lto/pr111157_1.c diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index 071c607fbe8..bb49a1b2959 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -1271,6 +1271,8 @@ ipa_argagg_value_list::dump (FILE *f) print_generic_expr (f, av.value); if (av.by_ref) fprintf (f, "(by_ref)"); + if (av.killed) + fprintf (f, "(killed)"); comma = true; } fprintf (f, "\n"); @@ -1437,6 +1439,8 @@ ipa_argagg_value_list::push_adjusted_values (unsigned src_index, new_av.unit_offset = av->unit_offset - unit_delta; new_av.index = dest_index; new_av.by_ref = av->by_ref; + gcc_assert (!av->killed); + new_av.killed = false; /* Quick check that the offsets we push are indeed increasing. */ gcc_assert (first @@ -1473,6 +1477,7 @@ push_agg_values_from_plats (ipcp_param_lattices *plats, int dest_index, iav.unit_offset = aglat->offset / BITS_PER_UNIT - unit_delta; iav.index = dest_index; iav.by_ref = plats->aggs_by_ref; + iav.killed = false; gcc_assert (first || iav.unit_offset > prev_unit_offset); @@ -2139,6 +2144,7 @@ ipa_push_agg_values_from_jfunc (ipa_node_params *info, cgraph_node *node, iav.unit_offset = item.offset / BITS_PER_UNIT; iav.index = dst_index; iav.by_ref = agg_jfunc->by_ref; + iav.killed = 0; gcc_assert (first || iav.unit_offset > prev_unit_offset); @@ -3970,6 +3976,7 @@ estimate_local_effects (struct cgraph_node *node) avals.m_known_aggs[j].unit_offset = unit_offset; avals.m_known_aggs[j].index = index; avals.m_known_aggs[j].by_ref = plats->aggs_by_ref; + avals.m_known_aggs[j].killed = false; perform_estimation_of_a_value (node, &avals, removable_params_cost, 0, val); @@ -5846,6 +5853,7 @@ push_agg_values_for_index_from_edge (struct cgraph_edge *cs, int index, iav.unit_offset = agg_jf.offset / BITS_PER_UNIT; iav.index = index; iav.by_ref = jfunc->agg.by_ref; + iav.killed = false; gcc_assert (first || iav.unit_offset > prev_unit_offset); diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc index c04f9f44c06..a8fcf159259 100644 --- a/gcc/ipa-modref.cc +++ b/gcc/ipa-modref.cc @@ -4065,21 +4065,71 @@ remap_kills (vec &kills, const vec &map) i++; } +/* Return true if the V can overlap with KILL. */ + +static bool +ipcp_argagg_and_kill_overlap_p (const ipa_argagg_value &v, + const modref_access_node &kill) +{ + if (kill.parm_index == v.index) + { + gcc_assert (kill.parm_offset_known); + gcc_assert (known_eq (kill.max_size, kill.size)); + poly_int64 repl_size; + bool ok = poly_int_tree_p (TYPE_SIZE (TREE_TYPE (v.value)), + &repl_size); + gcc_assert (ok); + poly_int64 repl_offset (v.unit_offset); + repl_offset <<= LOG2_BITS_PER_UNIT; + poly_int64 combined_offset + = (kill.parm_offset << LOG2_BITS_PER_UNIT) + kill.offset; + if (ranges_maybe_overlap_p (repl_offset, repl_size, + combined_offset, kill.size)) + return true; + } + return false; +} + /* If signature changed, update the summary. */ static void update_signature (struct cgraph_node *node) { - clone_info *info = clone_info::get (node); - if (!info || !info->param_adjustments) - return; - modref_summary *r = optimization_summaries ? optimization_summaries->get (node) : NULL; modref_summary_lto *r_lto = summaries_lto ? summaries_lto->get (node) : NULL; if (!r && !r_lto) return; + + ipcp_transformation *ipcp_ts = ipcp_get_transformation_summary (node); + if (ipcp_ts) + { + for (auto &v : ipcp_ts->m_agg_values) + { + if (!v.by_ref) + continue; + if (r) + for (const modref_access_node &kill : r->kills) + if (ipcp_argagg_and_kill_overlap_p (v, kill)) + { + v.killed = true; + break; + } + if (!v.killed && r_lto) + for (const modref_access_node &kill : r_lto->kills) + if (ipcp_argagg_and_kill_overlap_p (v, kill)) + { + v.killed = 1; + break; + } + } + } + + clone_info *info = clone_info::get (node); + if (!info || !info->param_adjustments) + return; + if (dump_file) { fprintf (dump_file, "Updating summary for %s from:\n", diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index 9efaa5cb848..3367a3a01c2 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -5414,6 +5414,7 @@ write_ipcp_transformation_info (output_block *ob, cgraph_node *node, bp = bitpack_create (ob->main_stream); bp_pack_value (&bp, av.by_ref, 1); + bp_pack_value (&bp, av.killed, 1); streamer_write_bitpack (&bp); } @@ -5459,6 +5460,7 @@ read_ipcp_transformation_info (lto_input_block *ib, cgraph_node *node, bitpack_d bp = streamer_read_bitpack (ib); av->by_ref = bp_unpack_value (&bp, 1); + av->killed = bp_unpack_value (&bp, 1); } } @@ -5762,7 +5764,9 @@ ipcp_modif_dom_walker::before_dom_children (basic_block bb) /* If IPA-CP discovered a constant in parameter PARM at OFFSET of a given SIZE - whether passed by reference or not is given by BY_REF - return that - constant. Otherwise return NULL_TREE. */ + constant. Otherwise return NULL_TREE. The is supposed to be used only + after clone materialization and transformation is done (because it asserts + that killed constants have been pruned). */ tree ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, @@ -5780,7 +5784,11 @@ ipcp_get_aggregate_const (struct function *func, tree parm, bool by_ref, ipa_argagg_value_list avl (ts); unsigned unit_offset = bit_offset / BITS_PER_UNIT; - tree v = avl.get_value (index, unit_offset, by_ref); + const ipa_argagg_value *av = avl.get_elt (index, unit_offset); + if (!av || av->by_ref != by_ref) + return NULL_TREE; + gcc_assert (!av->killed); + tree v = av->value; if (!v || maybe_ne (tree_to_poly_int64 (TYPE_SIZE (TREE_TYPE (v))), bit_size)) return NULL_TREE; @@ -6059,6 +6067,11 @@ ipcp_transform_function (struct cgraph_node *node) free_ipa_bb_info (bi); fbi.bb_infos.release (); + ts->remove_argaggs_if ([](const ipa_argagg_value &v) + { + return v.killed; + }); + vec_free (descriptors); if (cfg_changed) delete_unreachable_blocks_update_callgraph (node, false); diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index 815855006e8..e3a62bcf1ea 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -213,6 +213,10 @@ struct GTY(()) ipa_argagg_value unsigned index : IPA_PROP_ARG_INDEX_LIMIT_BITS; /* Whether the value was passed by reference. */ unsigned by_ref : 1; + /* Set if the value should not be used after materialization in + value_numbering. It is kept around just so that clone materialization can + distinguish a combined IPA-CP and IPA-SRA from a deleted argument. */ + unsigned killed : 1; }; /* A view into a sorted list of aggregate values in a particular context, be it diff --git a/gcc/testsuite/gcc.dg/lto/pr111157_0.c b/gcc/testsuite/gcc.dg/lto/pr111157_0.c new file mode 100644 index 00000000000..8bb4c656721 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr111157_0.c @@ -0,0 +1,24 @@ +/* { dg-lto-do run } */ +/* { dg-lto-options { { -O2 -flto=auto } } } */ +/* { dg-extra-ld-options { -flto-partition=1to1 } } */ + +extern __attribute__((noinline)) +void foo (int *p); + + +void __attribute__((noinline)) +bar (void) +{ + int istat; + + istat = 1234; + foo (&istat); + if (istat != 1234) + __builtin_abort (); +} + +int main (int argc, char **argv) +{ + bar (); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/lto/pr111157_1.c b/gcc/testsuite/gcc.dg/lto/pr111157_1.c new file mode 100644 index 00000000000..f9ba5afb42f --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr111157_1.c @@ -0,0 +1,10 @@ +volatile int v = 0; + +void __attribute__((noinline)) +foo (int *p) +{ + *p = 1234; + if (v) + *p = 0; + return; +}