From patchwork Thu Oct 5 11:59: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: 1843869 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=a14xOJ8y; dkim=pass header.d=suse.cz header.i=@suse.cz header.a=ed25519-sha256 header.s=susede2_ed25519 header.b=9210oWby; 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 4S1VVZ1y4gz1yqD for ; Thu, 5 Oct 2023 23:00:06 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A17DD3861827 for ; Thu, 5 Oct 2023 12:00:03 +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 610E7385DC0B for ; Thu, 5 Oct 2023 11:59:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 610E7385DC0B 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 5250721864 for ; Thu, 5 Oct 2023 11:59:48 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1696507188; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=Q2rwUbPslC/YL67tuWoqCqUdCKhNtSYsKYyjt08t7QE=; b=a14xOJ8y+kcjXJ88wBYho63o6EUvhBvfZzwQSbDW/CaYA2hLxlTG7XTWsLe10MwJCb2ntA rfI9J1VV2yEGp9ohHAZD5M8eXAyLcQjxY7OmLP+PXKzLLVQEnPLtvJ4df+COtJHxcxMfbZ M4uYf8DuebLOEH2rDs3MsNU38V58NfE= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1696507188; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=Q2rwUbPslC/YL67tuWoqCqUdCKhNtSYsKYyjt08t7QE=; b=9210oWbyDbphv27r6MKSnxdrDKdMedvtF1y6MJH1EMp5wZuMz23lQ/psPFxOIFjMlxYhhK yX5QmpCEXvSYFaBg== 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 47370139C2 for ; Thu, 5 Oct 2023 11:59:48 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id 7cNYETSlHmUSWQAAMHmgww (envelope-from ) for ; Thu, 05 Oct 2023 11:59:48 +0000 From: Martin Jambor To: GCC Patches Cc: Subject: [PATCH] Revert "ipa: Self-DCE of uses of removed call LHSs (PR 108007)" User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/29.1 (x86_64-suse-linux-gnu) Date: Thu, 05 Oct 2023 13:59:47 +0200 Message-ID: MIME-Version: 1.0 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 Hello, I am going to commit the following patch to fix PR 111688 (bootstrap on ppc64le broken) and will re-fix 108007 (issues with IPA-SRA when user explicitely turns off DCE) when I figure out what's going wrong. Sorry for the breakage, Martin [PATCH] Revert "ipa: Self-DCE of uses of removed call LHSs (PR 108007)" This reverts commit 1be18ea110a2d69570dbc494588a7c73173883be. As reported in PR bootstrap/111688, it broke ppc64le bootstrap because of a debug-compare failure. --- gcc/cgraph.cc | 10 +--- gcc/cgraph.h | 9 +-- gcc/ipa-param-manipulation.cc | 88 ++++++++--------------------- gcc/ipa-param-manipulation.h | 3 +- gcc/testsuite/gcc.dg/ipa/pr108007.c | 32 ----------- gcc/tree-inline.cc | 28 ++++----- 6 files changed, 38 insertions(+), 132 deletions(-) delete mode 100644 gcc/testsuite/gcc.dg/ipa/pr108007.c diff --git a/gcc/cgraph.cc b/gcc/cgraph.cc index b82367ac342..e41e5ad3ae7 100644 --- a/gcc/cgraph.cc +++ b/gcc/cgraph.cc @@ -1403,17 +1403,11 @@ cgraph_edge::redirect_callee (cgraph_node *n) speculative indirect call, remove "speculative" of the indirect call and also redirect stmt to it's final direct target. - When called from within tree-inline, KILLED_SSAs has to contain the pointer - to killed_new_ssa_names within the copy_body_data structure and SSAs - discovered to be useless (if LHS is removed) will be added to it, otherwise - it needs to be NULL. - It is up to caller to iteratively transform each "speculative" direct call as appropriate. */ gimple * -cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e, - hash_set *killed_ssas) +cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) { tree decl = gimple_call_fndecl (e->call_stmt); gcall *new_stmt; @@ -1533,7 +1527,7 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e, remove_stmt_from_eh_lp (e->call_stmt); tree old_fntype = gimple_call_fntype (e->call_stmt); - new_stmt = padjs->modify_call (e, false, killed_ssas); + new_stmt = padjs->modify_call (e, false); cgraph_node *origin = e->callee; while (origin->clone_of) origin = origin->clone_of; diff --git a/gcc/cgraph.h b/gcc/cgraph.h index d7162efeeb4..cedaaac3a45 100644 --- a/gcc/cgraph.h +++ b/gcc/cgraph.h @@ -1833,16 +1833,9 @@ public: speculative indirect call, remove "speculative" of the indirect call and also redirect stmt to it's final direct target. - When called from within tree-inline, KILLED_SSAs has to contain the - pointer to killed_new_ssa_names within the copy_body_data structure and - SSAs discovered to be useless (if LHS is removed) will be added to it, - otherwise it needs to be NULL. - It is up to caller to iteratively transform each "speculative" direct call as appropriate. */ - static gimple *redirect_call_stmt_to_callee (cgraph_edge *e, - hash_set - *killed_ssas = nullptr); + static gimple *redirect_call_stmt_to_callee (cgraph_edge *e); /* Create clone of edge in the node N represented by CALL_EXPR the callgraph. */ diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc index 014939bf754..ae52f17b2c9 100644 --- a/gcc/ipa-param-manipulation.cc +++ b/gcc/ipa-param-manipulation.cc @@ -593,66 +593,14 @@ isra_get_ref_base_and_offset (tree expr, tree *base_p, unsigned *unit_offset_p) return true; } -/* Remove all statements that use NAME and transitively those that use the - result of such statements. KILLED_SSAS contains the SSA_NAMEs that are - already being or have been processed and new ones need to be added to it. - The funtction only has to process situations handled by - ssa_name_only_returned_p in ipa-sra.cc with the exception that it can assume - it must never reach a use in a return statement. */ - -static void -purge_transitive_uses (tree name, hash_set *killed_ssas) -{ - imm_use_iterator imm_iter; - gimple *stmt; - auto_vec worklist; - - worklist.safe_push (name); - while (!worklist.is_empty ()) - { - tree cur_name = worklist.pop (); - FOR_EACH_IMM_USE_STMT (stmt, imm_iter, cur_name) - { - if (gimple_debug_bind_p (stmt)) - { - /* When runing within tree-inline, we will never end up here but - adding the SSAs to killed_ssas will do the trick in this case - and the respective debug statements will get reset. */ - gimple_debug_bind_reset_value (stmt); - update_stmt (stmt); - continue; - } - - tree lhs = NULL_TREE; - if (is_gimple_assign (stmt)) - lhs = gimple_assign_lhs (stmt); - else if (gimple_code (stmt) == GIMPLE_PHI) - lhs = gimple_phi_result (stmt); - gcc_assert (lhs - && (TREE_CODE (lhs) == SSA_NAME) - && !gimple_vdef (stmt)); - if (!killed_ssas->add (lhs)) - { - worklist.safe_push (lhs); - gimple_stmt_iterator gsi = gsi_for_stmt (stmt); - gsi_remove (&gsi, true); - } - } - } -} - /* Modify actual arguments of a function call in statement currently belonging to CS, and make it call CS->callee->decl. Return the new statement that replaced the old one. When invoked, cfun and current_function_decl have to - be set to the caller. When called from within tree-inline, KILLED_SSAs has - to contain the pointer to killed_new_ssa_names within the copy_body_data - structure and SSAs discovered to be useless (if LHS is removed) will be - added to it, otherwise it needs to be NULL. */ + be set to the caller. */ gcall * ipa_param_adjustments::modify_call (cgraph_edge *cs, - bool update_references, - hash_set *killed_ssas) + bool update_references) { gcall *stmt = cs->call_stmt; tree callee_decl = cs->callee->decl; @@ -962,20 +910,32 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, gcall *new_stmt = gimple_build_call_vec (callee_decl, vargs); - hash_set *ssas_to_remove = NULL; + tree ssa_to_remove = NULL; if (tree lhs = gimple_call_lhs (stmt)) { if (!m_skip_return) gimple_call_set_lhs (new_stmt, lhs); else if (TREE_CODE (lhs) == SSA_NAME) { - if (!killed_ssas) + /* LHS should now by a default-def SSA. Unfortunately default-def + SSA_NAMEs need a backing variable (or at least some code examining + SSAs assumes it is non-NULL). So we either have to re-use the + decl we have at hand or introdice a new one. */ + tree repl = create_tmp_var (TREE_TYPE (lhs), "removed_return"); + repl = get_or_create_ssa_default_def (cfun, repl); + SSA_NAME_IS_DEFAULT_DEF (repl) = true; + imm_use_iterator ui; + use_operand_p use_p; + gimple *using_stmt; + FOR_EACH_IMM_USE_STMT (using_stmt, ui, lhs) { - ssas_to_remove = new hash_set (8); - killed_ssas = ssas_to_remove; + FOR_EACH_IMM_USE_ON_STMT (use_p, ui) + { + SET_USE (use_p, repl); + } + update_stmt (using_stmt); } - killed_ssas->add (lhs); - purge_transitive_uses (lhs, killed_ssas); + ssa_to_remove = lhs; } } @@ -994,12 +954,8 @@ ipa_param_adjustments::modify_call (cgraph_edge *cs, fprintf (dump_file, "\n"); } gsi_replace (&gsi, new_stmt, true); - if (ssas_to_remove) - { - for (tree sn : *ssas_to_remove) - release_ssa_name (sn); - delete ssas_to_remove; - } + if (ssa_to_remove) + release_ssa_name (ssa_to_remove); if (update_references) do { diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h index b7e56fe6379..d6a26e9ad36 100644 --- a/gcc/ipa-param-manipulation.h +++ b/gcc/ipa-param-manipulation.h @@ -224,8 +224,7 @@ public: /* Modify a call statement arguments (and possibly remove the return value) as described in the data fields of this class. */ - gcall *modify_call (cgraph_edge *cs, bool update_references, - hash_set *killed_ssas); + gcall *modify_call (cgraph_edge *cs, bool update_references); /* Return if the first parameter is left intact. */ bool first_param_intact_p (); /* Build a function type corresponding to the modified call. */ diff --git a/gcc/testsuite/gcc.dg/ipa/pr108007.c b/gcc/testsuite/gcc.dg/ipa/pr108007.c deleted file mode 100644 index 77fc95975cf..00000000000 --- a/gcc/testsuite/gcc.dg/ipa/pr108007.c +++ /dev/null @@ -1,32 +0,0 @@ -/* { dg-do run } */ -/* { dg-options "-Os -fno-dce -fno-tree-dce -g" } */ - -/* This tests that when IPA-SRA removes a LHS of a call statement which, in the - original source, is fed into a useless operation which however can trap when - given nonsensical input, that we remove it even when the user has turned off - normal DCE. */ - -int a, b, d, e, f = 10000000, h; -short c, g; -static int *i() { - g = f; - L: - h = e = ~g; - g = ~f % g & e; - if (!g) - goto L; - c++; - while (g < 1) - ; - return &a; -} -static void k() { - int *l, m = 2; - l = i(); - for (; d < 1; d++) - m |= *l >= b; -} -int main() { - k(); - return 0; -} diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc index 8daef6955fd..d63060c9429 100644 --- a/gcc/tree-inline.cc +++ b/gcc/tree-inline.cc @@ -2988,19 +2988,20 @@ redirect_all_calls (copy_body_data * id, basic_block bb) struct cgraph_edge *edge = id->dst_node->get_edge (stmt); if (edge) { - if (!id->killed_new_ssa_names) - id->killed_new_ssa_names = new hash_set (16); gimple *new_stmt - = cgraph_edge::redirect_call_stmt_to_callee (edge, - id->killed_new_ssa_names); + = cgraph_edge::redirect_call_stmt_to_callee (edge); + /* If IPA-SRA transformation, run as part of edge redirection, + removed the LHS because it is unused, save it to + killed_new_ssa_names so that we can prune it from debug + statements. */ if (old_lhs && TREE_CODE (old_lhs) == SSA_NAME && !gimple_call_lhs (new_stmt)) - /* In case of IPA-SRA removing the LHS, the name should have - been already added to the hash. But in case of redirecting - to builtin_unreachable it was not and the name still should - be pruned from debug statements. */ - id->killed_new_ssa_names->add (old_lhs); + { + if (!id->killed_new_ssa_names) + id->killed_new_ssa_names = new hash_set (16); + id->killed_new_ssa_names->add (old_lhs); + } if (stmt == last && id->call_stmt && maybe_clean_eh_stmt (stmt)) gimple_purge_dead_eh_edges (bb); @@ -3327,13 +3328,8 @@ copy_body (copy_body_data *id, body = copy_cfg_body (id, entry_block_map, exit_block_map, new_entry); copy_debug_stmts (id); - if (id->killed_new_ssa_names) - { - for (tree sn : *id->killed_new_ssa_names) - release_ssa_name (sn); - delete id->killed_new_ssa_names; - id->killed_new_ssa_names = NULL; - } + delete id->killed_new_ssa_names; + id->killed_new_ssa_names = NULL; return body; }