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 1D668385701F for ; Mon, 21 Jun 2021 20:14:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 1D668385701F 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 relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out2.suse.de (Postfix) with ESMTP id CF6DB1FD3F for ; Mon, 21 Jun 2021 20:14:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1624306452; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=M5B3icNeivMLFmtUoc50aQpqO86KlrxYtWnPZysTBYc=; b=NJmwwjvClCTIblUUfuebhUmd2dHHOsPZTcqr+Roxm3aeEkDnDvpI8l6BP3zoAl+fqk+kfU y0Mh0KxxVNL0YwaAoc4uSBt6sqCedBtcFGTWpeZxMDU6Sn9MFgJz5kiRl2uwr2z/+Lxa0w DfSBIZ09tkjQ5Mpv5ygCFOBVbhfL7LY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1624306452; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=M5B3icNeivMLFmtUoc50aQpqO86KlrxYtWnPZysTBYc=; b=Z4yy2+jAg5J9WVQsNGdIm+3QulZka6xZlwVd9bQrBc9k16Xc9WyjjRyttn6NIJKMHJYjR8 DGkaHTxYI2/jXVDg== Received: from suse.cz (unknown [10.100.200.98]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay2.suse.de (Postfix) with ESMTPS id B22AEA3B92; Mon, 21 Jun 2021 20:14:12 +0000 (UTC) From: Martin Jambor To: GCC Patches Cc: Jan Hubicka , Martin Liska Subject: [PATCH] ipa-sra: Fix thinko when overriding safe_to_import_accesses (PR 101066) User-Agent: Notmuch/0.32 (https://notmuchmail.org) Emacs/27.2 (x86_64-suse-linux-gnu) Date: Mon, 21 Jun 2021 22:14:12 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-10.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.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 21 Jun 2021 20:14:15 -0000 Hi, The "new" IPA-SRA has a more difficult job than the previous not-truly-IPA version when identifying situations in which a parameter passed by reference can be passed into a third function and only thee converted to one passed by value (and possibly "split" at the same time). In order to allow this, two conditions must be fulfilled. First the call to the third function must happen before any modifications of memory, because it could change the value passed by reference. Second, in order to make sure we do not introduce new (invalid) dereferences, the call must postdominate the entry BB. The second condition is actually not necessary if the caller function is also certain to dereference the pointer but the first one must still hold. Unfortunately, the code making this overriding decision also happen to trigger when the first condition is not fulfilled. This is fixed in the following patch. Bootstrapped, LTO-bootstrapped and tested on x86_64-linux, OK for trunk and the gcc-11 branch? On gcc-10, I might just remove the override altogether, the case might not be important enough to change LTO format. Thanks, Martin gcc/ChangeLog: 2021-06-16 Martin Jambor PR ipa/101066 * ipa-sra.c (class isra_call_summary): New member m_before_any_store, initialize it in the constructor. (isra_call_summary::dump): Dump the new field. (ipa_sra_call_summaries::duplicate): Copy it. (process_scan_results): Set it. (isra_write_edge_summary): Stream it. (isra_read_edge_summary): Likewise. (param_splitting_across_edge): Only override safe_to_import_accesses if m_before_any_store is set. gcc/testsuite/ChangeLog: 2021-06-16 Martin Jambor PR ipa/101066 * gcc.dg/ipa/pr101066.c: New test. --- gcc/ipa-sra.c | 15 +++++++++++++-- gcc/testsuite/gcc.dg/ipa/pr101066.c | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/ipa/pr101066.c diff --git a/gcc/ipa-sra.c b/gcc/ipa-sra.c index 3272daf56e4..965e246d788 100644 --- a/gcc/ipa-sra.c +++ b/gcc/ipa-sra.c @@ -343,7 +343,7 @@ class isra_call_summary public: isra_call_summary () : m_arg_flow (), m_return_ignored (false), m_return_returned (false), - m_bit_aligned_arg (false) + m_bit_aligned_arg (false), m_before_any_store (false) {} void init_inputs (unsigned arg_count); @@ -362,6 +362,10 @@ public: /* Set when any of the call arguments are not byte-aligned. */ unsigned m_bit_aligned_arg : 1; + + /* Set to true if the call happend before any (other) store to memory in the + caller. */ + unsigned m_before_any_store : 1; }; /* Class to manage function summaries. */ @@ -491,6 +495,8 @@ isra_call_summary::dump (FILE *f) fprintf (f, " return value ignored\n"); if (m_return_returned) fprintf (f, " return value used only to compute caller return value\n"); + if (m_before_any_store) + fprintf (f, " happens before any store to memory\n"); for (unsigned i = 0; i < m_arg_flow.length (); i++) { fprintf (f, " Parameter %u:\n", i); @@ -535,6 +541,7 @@ ipa_sra_call_summaries::duplicate (cgraph_edge *, cgraph_edge *, new_sum->m_return_ignored = old_sum->m_return_ignored; new_sum->m_return_returned = old_sum->m_return_returned; new_sum->m_bit_aligned_arg = old_sum->m_bit_aligned_arg; + new_sum->m_before_any_store = old_sum->m_before_any_store; } @@ -2374,6 +2381,7 @@ process_scan_results (cgraph_node *node, struct function *fun, unsigned count = gimple_call_num_args (call_stmt); isra_call_summary *csum = call_sums->get_create (cs); csum->init_inputs (count); + csum->m_before_any_store = uses_memory_as_obtained; for (unsigned argidx = 0; argidx < count; argidx++) { if (!csum->m_arg_flow[argidx].pointer_pass_through) @@ -2546,6 +2554,7 @@ isra_write_edge_summary (output_block *ob, cgraph_edge *e) bp_pack_value (&bp, csum->m_return_ignored, 1); bp_pack_value (&bp, csum->m_return_returned, 1); bp_pack_value (&bp, csum->m_bit_aligned_arg, 1); + bp_pack_value (&bp, csum->m_before_any_store, 1); streamer_write_bitpack (&bp); } @@ -2664,6 +2673,7 @@ isra_read_edge_summary (struct lto_input_block *ib, cgraph_edge *cs) csum->m_return_ignored = bp_unpack_value (&bp, 1); csum->m_return_returned = bp_unpack_value (&bp, 1); csum->m_bit_aligned_arg = bp_unpack_value (&bp, 1); + csum->m_before_any_store = bp_unpack_value (&bp, 1); } /* Read intraprocedural analysis information about NODE and all of its outgoing @@ -3420,7 +3430,8 @@ param_splitting_across_edge (cgraph_edge *cs) } else if (!ipf->safe_to_import_accesses) { - if (!all_callee_accesses_present_p (param_desc, arg_desc)) + if (!csum->m_before_any_store + || !all_callee_accesses_present_p (param_desc, arg_desc)) { if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, " %u->%u: cannot import accesses.\n", diff --git a/gcc/testsuite/gcc.dg/ipa/pr101066.c b/gcc/testsuite/gcc.dg/ipa/pr101066.c new file mode 100644 index 00000000000..1ceb6e43136 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr101066.c @@ -0,0 +1,20 @@ +/* { dg-do run } */ +/* { dg-options "-Os -fno-ipa-cp -fno-inline" } */ + +int a = 1, c, d, e; +int *b = &a; +static int g(int *h) { + c = *h; + return d; +} +static void f(int *h) { + e = *h; + *b = 0; + g(h); +} +int main() { + f(b); + if (c) + __builtin_abort(); + return 0; +} -- 2.31.1