From patchwork Tue Mar 30 13:02:51 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1460031 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Received: from 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 RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4F8qPN3dM5z9sVq for ; Wed, 31 Mar 2021 00:03:03 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 967903857C61; Tue, 30 Mar 2021 13:03:00 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id DEBBB3858002 for ; Tue, 30 Mar 2021 13:02:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org DEBBB3858002 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mjambor@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id A00A8B2F9; Tue, 30 Mar 2021 13:02:51 +0000 (UTC) From: Martin Jambor To: GCC Patches Subject: [GCC 12 PATCH 1/2] ipa-sra: Introduce a mini-DCE to tree-inline.c (PR 93385) User-Agent: Notmuch/0.31.4 (https://notmuchmail.org) Emacs/27.1 (x86_64-suse-linux-gnu) Date: Tue, 30 Mar 2021 15:02:51 +0200 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-3039.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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: , Cc: Jan Hubicka Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, PR 93385 reveals that if the user explicitely disables DCE, IPA-SRA can leave behind statements which are useless because their results are eventually not used but can have problematic side effects, especially since their inputs are now bogus that useless parameters were removed. This patch fixes the problem by doing a def-use walk when materializing clones, marking which statements should not be copied and which SSA_NAMEs do not need to be computed because eventually they would be DCEd. When an argument of a call within such a function is removed, however, that change needs to be communicated to call redirection code. This is call specific information and therefore cannot be reasonably encoded in clone node summary and has to be put in call summaries. Combining these with stuff in performed_splits in clone_info would be very cumbersome and therefore this patch removes performed_splits and moves all information it into call summaries too. This has also the advantage that the code is hopefully a bit easier to understand and we do not need any special dummy variables. The new edge summaries are private to ipa-param-manipulation.c and hopefully will never be needed elsewhere. It simply contains 1) a mapping from the original argument indices to the actual indices in the call statement as it is now, 2) information needed to identify arguments representing pass-through IPA-SRA splits with which have been added to the call arguments in place of an original argument/reference and 3) a delta to the index where va_args may start. Bootstrapped and tested on x86_64-linux, i686-linux and aarch64-linux. Also LTO-bootstrapped and LTO-profiledbootstrapped on x86_64-linux. I would like to commit the patch early in stage1 for GCC 12 and will be grateful for a review and any feedback. Thanks, Martin gcc/ChangeLog: 2021-03-24 Martin Jambor PR ipa/93385 * symtab-clones.h (clone_info): Removed member param_adjustments. * ipa-param-manipulation.h: Adjust initial comment to reflect how we deal with pass-through splits now. (ipa_param_performed_split): Removed. (ipa_param_adjustments::modify_call): Adjusted parameters. (class ipa_param_body_adjustments): New members m_dead_stmts, m_dead_ssas, mark_dead_statements, modify_call_argument and m_new_call_arg_modification_info. Adjusted parameters of register_replacement, modify_gimple_stmt and modify_call_stmt. (ipa_verify_edge_has_no_modifications): Declare. * ipa-param-manipulation.c (struct pass_through_split_map): New type. (ipa_edge_modification_info): Likewise. (ipa_edge_modification_sum): Likewise. (ipa_edge_modifications): New edge summary. (ipa_verify_edge_has_no_modifications): New function. (transitive_split_p): Removed. (transitive_split_map): Likewise. (init_transitive_splits): Likewise. (ipa_param_adjustments::modify_call): Adjusted to use the new edge summary instead of performed_splits. (ipa_param_body_adjustments::register_replacement): Drop dummy parameter, set base_index of the created ipa_param_body_replacement. (phi_arg_will_live_p): New function. (ipa_param_body_adjustments::mark_dead_statements): New method. (ipa_param_body_adjustments::common_initialization): Call it. Do not create IPA_SRA dummy decls. (ipa_param_body_adjustments::ipa_param_body_adjustments): Initialize new mwmbers. (simple_tree_swap_info): Removed. (remap_split_decl_to_dummy): Likewise. (record_argument_state_1): New function. (record_argument_state): Likewise. (ipa_param_body_adjustments::modify_call_stmt): New parameter orig_stmt. Do not work with dummy decls, save necessary info about changes to ipa_edge_modifications. (ipa_param_body_adjustments::modify_gimple_stmt): New parameter orig_stmt, pass it to modify_call_stmt. (ipa_param_body_adjustments::modify_cfun_body): Adjust call to modify_gimple_stmt. * tree-inline.c (remap_gimple_stmt): Do not copy dead statements, reset dead debug statements, pass original statement to modify_gimple_stmt. (copy_phis_for_bb): Do not copy dead PHI nodes. (expand_call_inline): Do not remap performed_splits. (update_clone_info): Likewise. gcc/testsuite/ChangeLog: 2021-03-22 Martin Jambor PR ipa/93385 * gcc.dg/ipa/pr93385.c: New test. * gcc.dg/ipa/ipa-sra-23.c: Likewise. * gcc.dg/ipa/ipa-sra-24.c: Likewise. * g++.dg/ipa/ipa-sra-4.C: Likewise. --- gcc/cgraph.c | 22 +- gcc/cgraphclones.c | 3 - gcc/ipa-param-manipulation.c | 916 ++++++++++++++++---------- gcc/ipa-param-manipulation.h | 92 ++- gcc/symtab-clones.h | 15 +- gcc/testsuite/g++.dg/ipa/ipa-sra-4.C | 37 ++ gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c | 24 + gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c | 20 + gcc/testsuite/gcc.dg/ipa/pr93385.c | 27 + gcc/tree-inline.c | 121 +--- 10 files changed, 721 insertions(+), 556 deletions(-) create mode 100644 gcc/testsuite/g++.dg/ipa/ipa-sra-4.C create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93385.c diff --git a/gcc/cgraph.c b/gcc/cgraph.c index 80140757d16..ec8edac71f5 100644 --- a/gcc/cgraph.c +++ b/gcc/cgraph.c @@ -1506,8 +1506,6 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) } clone_info *callee_info = clone_info::get (e->callee); - clone_info *caller_info = clone_info::get (e->caller); - if (symtab->dump_file) { fprintf (symtab->dump_file, "updating call of %s -> %s: ", @@ -1515,18 +1513,6 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) print_gimple_stmt (symtab->dump_file, e->call_stmt, 0, dump_flags); if (callee_info && callee_info->param_adjustments) callee_info->param_adjustments->dump (symtab->dump_file); - unsigned performed_len - = caller_info ? vec_safe_length (caller_info->performed_splits) : 0; - if (performed_len > 0) - fprintf (symtab->dump_file, "Performed splits records:\n"); - for (unsigned i = 0; i < performed_len; i++) - { - ipa_param_performed_split *sm - = &(*caller_info->performed_splits)[i]; - print_node_brief (symtab->dump_file, " dummy_decl: ", sm->dummy_decl, - TDF_UID); - fprintf (symtab->dump_file, ", unit_offset: %u\n", sm->unit_offset); - } } if (ipa_param_adjustments *padjs @@ -1541,10 +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->call_stmt, - caller_info - ? caller_info->performed_splits : NULL, - e->callee->decl, false); + new_stmt = padjs->modify_call (e, false); cgraph_node *origin = e->callee; while (origin->clone_of) origin = origin->clone_of; @@ -1564,6 +1547,9 @@ cgraph_edge::redirect_call_stmt_to_callee (cgraph_edge *e) } else { + if (flag_checking + && !fndecl_built_in_p (e->callee->decl, BUILT_IN_UNREACHABLE)) + ipa_verify_edge_has_no_modifications (e); new_stmt = e->call_stmt; gimple_call_set_fndecl (new_stmt, e->callee->decl); update_stmt_fn (DECL_STRUCT_FUNCTION (e->caller->decl), new_stmt); diff --git a/gcc/cgraphclones.c b/gcc/cgraphclones.c index 95103a423f7..fede83c8b57 100644 --- a/gcc/cgraphclones.c +++ b/gcc/cgraphclones.c @@ -414,9 +414,6 @@ cgraph_node::create_clone (tree new_decl, profile_count prof_count, else if (info && info->param_adjustments) clone_info::get_create (new_node)->param_adjustments = info->param_adjustments; - if (info && info->performed_splits) - clone_info::get_create (new_node)->performed_splits - = vec_safe_copy (info->performed_splits); new_node->split_part = split_part; FOR_EACH_VEC_ELT (redirect_callers, i, e) diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c index 132bb24f76a..3e07fd72fe2 100644 --- a/gcc/ipa-param-manipulation.c +++ b/gcc/ipa-param-manipulation.c @@ -62,6 +62,80 @@ static const char *ipa_param_op_names[IPA_PARAM_PREFIX_COUNT] "IPA_PARAM_OP_NEW", "IPA_PARAM_OP_SPLIT"}; +/* Structure to hold declarations representing pass-through IPA-SRA splits. In + essence, it tells new index for a combination of original index and + offset. */ + +struct pass_through_split_map +{ + /* Original argument index. */ + unsigned base_index; + /* Offset of the split part in the original argument. */ + unsigned unit_offset; + /* Index of the split part in the call statement - where clone + materialization put it. */ + int new_index; +}; + +/* Information about some call statements that needs to be conveyed from clone + materialization to edge redirection. */ + +class ipa_edge_modification_info +{ + public: + ipa_edge_modification_info () + {} + + /* Mapping of original argument indices to where those arguments sit in the + call statement now or to a negative index if they were removed. */ + auto_vec index_map; + /* Information about ISRA replacements put into the call statement at the + clone materialization stages. */ + auto_vec pass_through_map; + /* Necessary adjustment to ipa_param_adjustments::m_always_copy_start when + redirecting the call. */ + int always_copy_delta = 0; +}; + +/* Class for storing and retrieving summaries about cal statement + modifications. */ + +class ipa_edge_modification_sum + : public call_summary +{ + public: + ipa_edge_modification_sum (symbol_table *table) + : call_summary (table) + { + } + + /* Hook that is called by summary when an edge is duplicated. */ + + virtual void duplicate (cgraph_edge *, + cgraph_edge *, + ipa_edge_modification_info *old_info, + ipa_edge_modification_info *new_info) + { + new_info->index_map.safe_splice (old_info->index_map); + new_info->pass_through_map.safe_splice (old_info->pass_through_map); + new_info->always_copy_delta = old_info->always_copy_delta; + } +}; + +/* Call summary to store information about edges which have had their arguments + partially modified already. */ + +static ipa_edge_modification_sum *ipa_edge_modifications; + +/* Fail compilation if CS has any summary associated with it in + ipa_edge_modifications. */ + +DEBUG_FUNCTION void +ipa_verify_edge_has_no_modifications (cgraph_edge *cs) +{ + gcc_assert (!ipa_edge_modifications || !ipa_edge_modifications->get (cs)); +} + /* Fill an empty vector ARGS with PARM_DECLs representing formal parameters of FNDECL. The function should not be called during LTO WPA phase except for thunks (or functions with bodies streamed in). */ @@ -459,147 +533,46 @@ isra_get_ref_base_and_offset (tree expr, tree *base_p, unsigned *unit_offset_p) return true; } -/* Return true if EXPR describes a transitive split (i.e. one that happened for - both the caller and the callee) as recorded in PERFORMED_SPLITS. In that - case, store index of the respective record in PERFORMED_SPLITS into - *SM_IDX_P and the unit offset from all handled components in EXPR into - *UNIT_OFFSET_P. */ - -static bool -transitive_split_p (vec *performed_splits, - tree expr, unsigned *sm_idx_p, unsigned *unit_offset_p) -{ - tree base; - if (!isra_get_ref_base_and_offset (expr, &base, unit_offset_p)) - return false; - - if (TREE_CODE (base) == SSA_NAME) - { - base = SSA_NAME_VAR (base); - if (!base) - return false; - } - - unsigned len = vec_safe_length (performed_splits); - for (unsigned i = 0 ; i < len; i++) - { - ipa_param_performed_split *sm = &(*performed_splits)[i]; - if (sm->dummy_decl == base) - { - *sm_idx_p = i; - return true; - } - } - return false; -} - -/* Structure to hold declarations representing transitive IPA-SRA splits. In - essence, if we need to pass UNIT_OFFSET of a parameter which originally has - number BASE_INDEX, we should pass down REPL. */ - -struct transitive_split_map -{ - tree repl; - unsigned base_index; - unsigned unit_offset; -}; - -/* If call STMT contains any parameters representing transitive splits as - described by PERFORMED_SPLITS, return the number of extra parameters that - were addded during clone materialization and fill in INDEX_MAP with adjusted - indices of corresponding original parameters and TRANS_MAP with description - of all transitive replacement descriptions. Otherwise return zero. */ - -static unsigned -init_transitive_splits (vec *performed_splits, - gcall *stmt, vec *index_map, - auto_vec *trans_map) -{ - unsigned phony_arguments = 0; - unsigned stmt_idx = 0, base_index = 0; - unsigned nargs = gimple_call_num_args (stmt); - while (stmt_idx < nargs) - { - unsigned unit_offset_delta; - tree base_arg = gimple_call_arg (stmt, stmt_idx); - - if (phony_arguments > 0) - index_map->safe_push (stmt_idx); - - unsigned sm_idx; - stmt_idx++; - if (transitive_split_p (performed_splits, base_arg, &sm_idx, - &unit_offset_delta)) - { - if (phony_arguments == 0) - /* We have optimistically avoided constructing index_map do far but - now it is clear it will be necessary, so let's create the easy - bit we skipped until now. */ - for (unsigned k = 0; k < stmt_idx; k++) - index_map->safe_push (k); - - tree dummy = (*performed_splits)[sm_idx].dummy_decl; - for (unsigned j = sm_idx; j < performed_splits->length (); j++) - { - ipa_param_performed_split *caller_split - = &(*performed_splits)[j]; - if (caller_split->dummy_decl != dummy) - break; - - tree arg = gimple_call_arg (stmt, stmt_idx); - struct transitive_split_map tsm; - tsm.repl = arg; - tsm.base_index = base_index; - if (caller_split->unit_offset >= unit_offset_delta) - { - tsm.unit_offset - = (caller_split->unit_offset - unit_offset_delta); - trans_map->safe_push (tsm); - } - - phony_arguments++; - stmt_idx++; - } - } - base_index++; - } - return phony_arguments; -} - -/* Modify actual arguments of a function call in statement STMT, assuming it - calls CALLEE_DECL. CALLER_ADJ must be the description of parameter - adjustments of the caller or NULL if there are none. Return the new - statement that replaced the old one. When invoked, cfun and - current_function_decl have to be set to the caller. */ +/* 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. */ gcall * -ipa_param_adjustments::modify_call (gcall *stmt, - vec *performed_splits, - tree callee_decl, bool update_references) +ipa_param_adjustments::modify_call (cgraph_edge *cs, + bool update_references) { + gcall *stmt = cs->call_stmt; + tree callee_decl = cs->callee->decl; + + ipa_edge_modification_info *mod_info + = ipa_edge_modifications ? ipa_edge_modifications->get (cs) : NULL; + if (mod_info && symtab->dump_file) + { + fprintf (symtab->dump_file, "Information about pre-exiting " + "modifications.\n Index map:"); + unsigned idx_len = mod_info->index_map.length (); + for (unsigned i = 0; i < idx_len; i++) + fprintf (symtab->dump_file, " %i", mod_info->index_map[i]); + fprintf (symtab->dump_file, "\n Pass-through split map: "); + unsigned ptm_len = mod_info->pass_through_map.length (); + for (unsigned i = 0; i < ptm_len; i++) + fprintf (symtab->dump_file, + " (base_index: %u, offset: %u, new_index: %i)", + mod_info->pass_through_map[i].base_index, + mod_info->pass_through_map[i].unit_offset, + mod_info->pass_through_map[i].new_index); + fprintf (symtab->dump_file, "\n Always-copy delta: %i\n", + mod_info->always_copy_delta); + } + unsigned len = vec_safe_length (m_adj_params); auto_vec vargs (len); - tree old_decl = gimple_call_fndecl (stmt); unsigned old_nargs = gimple_call_num_args (stmt); + unsigned orig_nargs = mod_info ? mod_info->index_map.length () : old_nargs; auto_vec kept (old_nargs); kept.quick_grow_cleared (old_nargs); - auto_vec index_map; - auto_vec trans_map; - bool transitive_remapping = false; - - if (performed_splits) - { - unsigned removed = init_transitive_splits (performed_splits, - stmt, &index_map, &trans_map); - if (removed > 0) - { - transitive_remapping = true; - old_nargs -= removed; - } - } - cgraph_node *current_node = cgraph_node::get (current_function_decl); if (update_references) current_node->remove_stmt_references (stmt); @@ -612,13 +585,16 @@ ipa_param_adjustments::modify_call (gcall *stmt, ipa_adjusted_param *apm = &(*m_adj_params)[i]; if (apm->op == IPA_PARAM_OP_COPY) { - unsigned index = apm->base_index; - if (index >= old_nargs) + int index = apm->base_index; + if ((unsigned) index >= orig_nargs) /* Can happen if the original call has argument mismatch, ignore. */ continue; - if (transitive_remapping) - index = index_map[apm->base_index]; + if (mod_info) + { + index = mod_info->index_map[apm->base_index]; + gcc_assert (index >= 0); + } tree arg = gimple_call_arg (stmt, index); @@ -636,14 +612,17 @@ ipa_param_adjustments::modify_call (gcall *stmt, materialization. */ gcc_assert (apm->op == IPA_PARAM_OP_SPLIT); - /* We have to handle transitive changes differently using the maps we - have created before. So look into them first. */ + /* We have to handle pass-through changes differently using the map + clone materialziation might have left behind. */ tree repl = NULL_TREE; - for (unsigned j = 0; j < trans_map.length (); j++) - if (trans_map[j].base_index == apm->base_index - && trans_map[j].unit_offset == apm->unit_offset) + unsigned ptm_len = mod_info ? mod_info->pass_through_map.length () : 0; + for (unsigned j = 0; j < ptm_len; j++) + if (mod_info->pass_through_map[j].base_index == apm->base_index + && mod_info->pass_through_map[j].unit_offset == apm->unit_offset) { - repl = trans_map[j].repl; + int repl_idx = mod_info->pass_through_map[j].new_index; + gcc_assert (repl_idx >= 0); + repl = gimple_call_arg (stmt, repl_idx); break; } if (repl) @@ -652,12 +631,15 @@ ipa_param_adjustments::modify_call (gcall *stmt, continue; } - unsigned index = apm->base_index; - if (index >= old_nargs) + int index = apm->base_index; + if ((unsigned) index >= orig_nargs) /* Can happen if the original call has argument mismatch, ignore. */ continue; - if (transitive_remapping) - index = index_map[apm->base_index]; + if (mod_info) + { + index = mod_info->index_map[apm->base_index]; + gcc_assert (index >= 0); + } tree base = gimple_call_arg (stmt, index); /* We create a new parameter out of the value of the old one, we can @@ -773,8 +755,16 @@ ipa_param_adjustments::modify_call (gcall *stmt, } if (m_always_copy_start >= 0) - for (unsigned i = m_always_copy_start; i < old_nargs; i++) - vargs.safe_push (gimple_call_arg (stmt, i)); + { + int always_copy_start = m_always_copy_start; + if (mod_info) + { + always_copy_start += mod_info->always_copy_delta; + gcc_assert (always_copy_start >= 0); + } + for (unsigned i = always_copy_start; i < old_nargs; i++) + vargs.safe_push (gimple_call_arg (stmt, i)); + } /* For optimized away parameters, add on the caller side before the call @@ -782,6 +772,7 @@ ipa_param_adjustments::modify_call (gcall *stmt, stmts and associate D#X with parm in decl_debug_args_lookup vector to say for debug info that if parameter parm had been passed, it would have value parm_Y(D). */ + tree old_decl = gimple_call_fndecl (stmt); if (MAY_HAVE_DEBUG_BIND_STMTS && old_decl && callee_decl) { vec **debug_args = NULL; @@ -799,13 +790,17 @@ ipa_param_adjustments::modify_call (gcall *stmt, { if (!is_gimple_reg (old_parm) || kept[i]) continue; - tree origin = DECL_ORIGIN (old_parm); tree arg; - if (transitive_remapping) - arg = gimple_call_arg (stmt, index_map[i]); + if (mod_info) + { + if (mod_info->index_map[i] < 0) + continue; + arg = gimple_call_arg (stmt, mod_info->index_map[i]); + } else arg = gimple_call_arg (stmt, i); + tree origin = DECL_ORIGIN (old_parm); if (!useless_type_conversion_p (TREE_TYPE (origin), TREE_TYPE (arg))) { if (!fold_convertible_p (TREE_TYPE (origin), arg)) @@ -905,6 +900,9 @@ ipa_param_adjustments::modify_call (gcall *stmt, gsi_prev (&gsi); } while (gsi_stmt (gsi) != gsi_stmt (prev_gsi)); + + if (mod_info) + ipa_edge_modifications->remove (cs); return new_stmt; } @@ -927,13 +925,11 @@ ipa_param_adjustments::debug () dump (stderr); } -/* Register that REPLACEMENT should replace parameter described in APM and - optionally as DUMMY to mark transitive splits across calls. */ +/* Register that REPLACEMENT should replace parameter described in APM. */ void ipa_param_body_adjustments::register_replacement (ipa_adjusted_param *apm, - tree replacement, - tree dummy) + tree replacement) { gcc_checking_assert (apm->op == IPA_PARAM_OP_SPLIT || apm->op == IPA_PARAM_OP_NEW); @@ -941,7 +937,6 @@ ipa_param_body_adjustments::register_replacement (ipa_adjusted_param *apm, ipa_param_body_replacement psr; psr.base = m_oparms[apm->prev_clone_index]; psr.repl = replacement; - psr.dummy = dummy; psr.unit_offset = apm->unit_offset; m_replacements.safe_push (psr); } @@ -970,6 +965,97 @@ ipa_param_body_adjustments::carry_over_param (tree t) return new_parm; } +/* Return true if BLOCKS_TO_COPY is NULL or if PHI has an argument ARG in + position that corresponds to an edge that is coming from a block that has + the corresponding bit set in BLOCKS_TO_COPY. */ + +static bool +phi_arg_will_live_p (gphi *phi, bitmap blocks_to_copy, tree arg) +{ + bool arg_will_survive = false; + if (!blocks_to_copy) + arg_will_survive = true; + else + for (unsigned i = 0; i < gimple_phi_num_args (phi); i++) + if (gimple_phi_arg_def (phi, i) == arg + && bitmap_bit_p (blocks_to_copy, + gimple_phi_arg_edge (phi, i)->src->index)) + { + arg_will_survive = true; + break; + } + return arg_will_survive; +} + +/* Populate m_dead_stmts given that DEAD_PARAM is going to be removed without + any replacement or splitting. REPL is the replacement VAR_SECL to base any + remaining uses of a removed parameter on. */ + +void +ipa_param_body_adjustments::mark_dead_statements (tree dead_param) +{ + /* Current IPA analyses which remove unused parameters never remove a + non-gimple register ones which have any use except as parameters in other + calls, so we can safely leve them as they are. */ + if (!is_gimple_reg (dead_param)) + return; + tree parm_ddef = ssa_default_def (m_id->src_cfun, dead_param); + if (!parm_ddef || has_zero_uses (parm_ddef)) + return; + + auto_vec stack; + m_dead_ssas.add (parm_ddef); + stack.safe_push (parm_ddef); + while (!stack.is_empty ()) + { + tree t = stack.pop (); + + imm_use_iterator imm_iter; + gimple *stmt; + + insert_decl_map (m_id, t, error_mark_node); + FOR_EACH_IMM_USE_STMT (stmt, imm_iter, t) + { + if (is_gimple_call (stmt) + || (m_id->blocks_to_copy + && !bitmap_bit_p (m_id->blocks_to_copy, + gimple_bb (stmt)->index))) + continue; + + if (is_gimple_debug (stmt)) + { + m_dead_stmts.add (stmt); + gcc_assert (gimple_debug_bind_p (stmt)); + } + else if (gimple_code (stmt) == GIMPLE_PHI) + { + gphi *phi = as_a (stmt); + if (phi_arg_will_live_p (phi, m_id->blocks_to_copy, t)) + { + m_dead_stmts.add (phi); + tree res = gimple_phi_result (phi); + if (!m_dead_ssas.add (res)) + stack.safe_push (res); + } + } + else if (is_gimple_assign (stmt)) + { + m_dead_stmts.add (stmt); + if (!gimple_clobber_p (stmt)) + { + tree lhs = gimple_assign_lhs (stmt); + gcc_assert (TREE_CODE (lhs) == SSA_NAME); + if (!m_dead_ssas.add (lhs)) + stack.safe_push (lhs); + } + } + else + /* IPA-SRA does not analyze other types of statements. */ + gcc_unreachable (); + } + } +} + /* Common initialization performed by all ipa_param_body_adjustments constructors. OLD_FNDECL is the declaration we take original arguments from, (it may be the same as M_FNDECL). VARS, if non-NULL, is a pointer to @@ -1003,9 +1089,9 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl, auto_vec kept; kept.reserve_exact (m_oparms.length ()); kept.quick_grow_cleared (m_oparms.length ()); - auto_vec isra_dummy_decls; - isra_dummy_decls.reserve_exact (m_oparms.length ()); - isra_dummy_decls.quick_grow_cleared (m_oparms.length ()); + auto_vec split; + split.reserve_exact (m_oparms.length ()); + split.quick_grow_cleared (m_oparms.length ()); unsigned adj_len = vec_safe_length (m_adj_params); m_method2func = ((TREE_CODE (TREE_TYPE (m_fndecl)) == METHOD_TYPE) @@ -1051,35 +1137,8 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl, if (apm->op == IPA_PARAM_OP_SPLIT) { m_split_modifications_p = true; - - if (m_id) - { - tree dummy_decl; - if (!isra_dummy_decls[prev_index]) - { - dummy_decl = copy_decl_to_var (m_oparms[prev_index], - m_id); - /* Any attempt to remap this dummy in this particular - instance of clone materialization should yield - itself. */ - insert_decl_map (m_id, dummy_decl, dummy_decl); - - DECL_CHAIN (dummy_decl) = *vars; - *vars = dummy_decl; - isra_dummy_decls[prev_index] = dummy_decl; - } - else - dummy_decl = isra_dummy_decls[prev_index]; - - register_replacement (apm, new_parm, dummy_decl); - ipa_param_performed_split ps; - ps.dummy_decl = dummy_decl; - ps.unit_offset = apm->unit_offset; - vec_safe_push (clone_info::get_create - (m_id->dst_node)->performed_splits, ps); - } - else - register_replacement (apm, new_parm); + split[prev_index] = true; + register_replacement (apm, new_parm); } } else @@ -1106,13 +1165,16 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl, { if (!m_id->decl_map->get (m_oparms[i])) { - /* TODO: Perhaps at least aggregate-type params could re-use - their isra_dummy_decl here? */ tree var = copy_decl_to_var (m_oparms[i], m_id); insert_decl_map (m_id, m_oparms[i], var); /* Declare this new variable. */ DECL_CHAIN (var) = *vars; *vars = var; + + /* If this is not a split but a real removal, init hash sets + that will guide what not to copy to the new body. */ + if (!split[i]) + mark_dead_statements (m_oparms[i]); } } else @@ -1169,9 +1231,10 @@ ipa_param_body_adjustments ::ipa_param_body_adjustments (vec *adj_params, tree fndecl) : m_adj_params (adj_params), m_adjustments (NULL), m_reset_debug_decls (), - m_split_modifications_p (false), m_fndecl (fndecl), m_id (NULL), - m_oparms (), m_new_decls (), m_new_types (), m_replacements (), - m_removed_decls (), m_removed_map (), m_method2func (false) + m_split_modifications_p (false), m_dead_stmts (), m_dead_ssas (), + m_fndecl (fndecl), m_id (NULL), m_oparms (), m_new_decls (), + m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (), + m_method2func (false), m_new_call_arg_modification_info (false) { common_initialization (fndecl, NULL, NULL); } @@ -1185,10 +1248,10 @@ ipa_param_body_adjustments ::ipa_param_body_adjustments (ipa_param_adjustments *adjustments, tree fndecl) : m_adj_params (adjustments->m_adj_params), m_adjustments (adjustments), - m_reset_debug_decls (), m_split_modifications_p (false), m_fndecl (fndecl), - m_id (NULL), m_oparms (), m_new_decls (), m_new_types (), - m_replacements (), m_removed_decls (), m_removed_map (), - m_method2func (false) + m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (), + m_dead_ssas (), m_fndecl (fndecl), m_id (NULL), m_oparms (), m_new_decls (), + m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (), + m_method2func (false), m_new_call_arg_modification_info (false) { common_initialization (fndecl, NULL, NULL); } @@ -1208,9 +1271,10 @@ ipa_param_body_adjustments copy_body_data *id, tree *vars, vec *tree_map) : m_adj_params (adjustments->m_adj_params), m_adjustments (adjustments), - m_reset_debug_decls (), m_split_modifications_p (false), m_fndecl (fndecl), - m_id (id), m_oparms (), m_new_decls (), m_new_types (), m_replacements (), - m_removed_decls (), m_removed_map (), m_method2func (false) + m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (), + m_dead_ssas (),m_fndecl (fndecl), m_id (id), m_oparms (), m_new_decls (), + m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (), + m_method2func (false), m_new_call_arg_modification_info (false) { common_initialization (old_fndecl, vars, tree_map); } @@ -1498,218 +1562,338 @@ ipa_param_body_adjustments::modify_assignment (gimple *stmt, return any; } -/* Data passed to remap_split_decl_to_dummy through walk_tree. */ +/* Record information about what modifications to call arguments have already + been done by clone materialization into a summary describing CS. The + information is stored in NEW_INDEX_MAP, NEW_PT_MAP and NEW_ALWAYS_COPY_DELTA + and correspond to equivalent fields in ipa_edge_modification_info. Return + the edge summary. */ + +static ipa_edge_modification_info * +record_argument_state_1 (cgraph_edge *cs, const vec &new_index_map, + const vec &new_pt_map, + int new_always_copy_delta) -struct simple_tree_swap_info { - /* Change FROM to TO. */ - tree from, to; - /* And set DONE to true when doing so. */ - bool done; -}; + ipa_edge_modification_info *sum = ipa_edge_modifications->get_create (cs); -/* Simple remapper to remap a split parameter to the same expression based on a - special dummy decl so that edge redirections can detect transitive splitting - and finish them. */ - -static tree -remap_split_decl_to_dummy (tree *tp, int *walk_subtrees, void *data) -{ - tree t = *tp; - - if (DECL_P (t) || TREE_CODE (t) == SSA_NAME) + unsigned len = sum->pass_through_map.length (); + for (unsigned i = 0; i < len; i++) { - struct simple_tree_swap_info *swapinfo - = (struct simple_tree_swap_info *) data; - if (t == swapinfo->from - || (TREE_CODE (t) == SSA_NAME - && SSA_NAME_VAR (t) == swapinfo->from)) - { - *tp = swapinfo->to; - swapinfo->done = true; - } - *walk_subtrees = 0; + unsigned oldnew = sum->pass_through_map[i].new_index; + sum->pass_through_map[i].new_index = new_index_map[oldnew]; + } + + len = sum->index_map.length (); + if (len > 0) + { + unsigned nptlen = new_pt_map.length (); + for (unsigned j = 0; j < nptlen; j++) + { + int inverse = -1; + for (unsigned i = 0; i < len ; i++) + if ((unsigned) sum->index_map[i] == new_pt_map[j].base_index) + { + inverse = i; + break; + } + gcc_assert (inverse >= 0); + pass_through_split_map ptm_item; + + ptm_item.base_index = inverse; + ptm_item.unit_offset = new_pt_map[j].unit_offset; + ptm_item.new_index = new_pt_map[j].new_index; + sum->pass_through_map.safe_push (ptm_item); + } + + for (unsigned i = 0; i < len; i++) + { + int idx = sum->index_map[i]; + if (idx < 0) + continue; + sum->index_map[i] = new_index_map[idx]; + } } - else if (TYPE_P (t)) - *walk_subtrees = 0; else - *walk_subtrees = 1; - return NULL_TREE; + { + sum->pass_through_map.safe_splice (new_pt_map); + sum->index_map.safe_splice (new_index_map); + } + sum->always_copy_delta += new_always_copy_delta; + return sum; } +/* Record information about what modifications to call arguments have already + been done by clone materialization into a summary of an edge describing the + call in this clone and all its clones. NEW_INDEX_MAP, NEW_PT_MAP and + NEW_ALWAYS_COPY_DELTA have the same meaning as record_argument_state_1. + + In order to associate the info with the right edge summaries, we need + address of the ORIG_STMT in the function from which we are cloning (because + the edges have not yet been re-assigned to the new statement that has just + been created) and ID, the structure governing function body copying. */ + +static void +record_argument_state (copy_body_data *id, gimple *orig_stmt, + const vec &new_index_map, + const vec &new_pt_map, + int new_always_copy_delta) +{ + if (!ipa_edge_modifications) + ipa_edge_modifications = new ipa_edge_modification_sum (symtab); + + struct cgraph_node *this_node = id->dst_node; + ipa_edge_modification_info *first_sum = NULL; + cgraph_edge *cs = this_node->get_edge (orig_stmt); + if (cs) + first_sum = record_argument_state_1 (cs, new_index_map, new_pt_map, + new_always_copy_delta); + else + gcc_assert (this_node->clones); + + if (!this_node->clones) + return; + for (cgraph_node *subclone = this_node->clones; subclone != this_node;) + { + cs = subclone->get_edge (orig_stmt); + if (cs) + { + if (!first_sum) + first_sum = record_argument_state_1 (cs, new_index_map, new_pt_map, + new_always_copy_delta); + else + { + ipa_edge_modification_info *s2 + = ipa_edge_modifications->get_create (cs); + s2->index_map.truncate (0); + s2->index_map.safe_splice (first_sum->index_map); + s2->pass_through_map.truncate (0); + s2->pass_through_map.safe_splice (first_sum->pass_through_map); + s2->always_copy_delta = first_sum->always_copy_delta; + } + } + else + gcc_assert (subclone->clones); + + if (subclone->clones) + subclone = subclone->clones; + else if (subclone->next_sibling_clone) + subclone = subclone->next_sibling_clone; + else + { + while (subclone != this_node && !subclone->next_sibling_clone) + subclone = subclone->clone_of; + if (subclone != this_node) + subclone = subclone->next_sibling_clone; + } + } +} /* If the call statement pointed at by STMT_P contains any expressions that need to replaced with a different one as noted by ADJUSTMENTS, do so. f the statement needs to be rebuilt, do so. Return true if any modifications have - been performed. + been performed. ORIG_STMT, if not NULL, is the original statement in the + function that is being cloned from, which at this point can be used to look + up call_graph edges. If the method is invoked as a part of IPA clone materialization and if any - parameter split is transitive, i.e. it applies to the functin that is being - modified and also to the callee of the statement, replace the parameter - passed to old callee with an equivalent expression based on a dummy decl - followed by PARM_DECLs representing the actual replacements. The actual - replacements will be then converted into SSA_NAMEs and then - ipa_param_adjustments::modify_call will find the appropriate ones and leave - only those in the call. */ + parameter split is pass-through, i.e. it applies to the functin that is + being modified and also to the callee of the statement, replace the + parameter passed to old callee with all of the replacement a callee might + possibly want and record the performed argument modifications in + ipa_edge_modifications. Likewise if any argument has already been left out + because it is not necessary. */ bool -ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p) +ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p, + gimple *orig_stmt) { - gcall *stmt = *stmt_p; auto_vec pass_through_args; auto_vec pass_through_pbr_indices; + auto_vec pass_through_offsets; + gcall *stmt = *stmt_p; + unsigned nargs = gimple_call_num_args (stmt); + bool recreate = false; - if (m_split_modifications_p && m_id) + for (unsigned i = 0; i < gimple_call_num_args (stmt); i++) { - for (unsigned i = 0; i < gimple_call_num_args (stmt); i++) + tree t = gimple_call_arg (stmt, i); + gcc_assert (TREE_CODE (t) != BIT_FIELD_REF + && TREE_CODE (t) != IMAGPART_EXPR + && TREE_CODE (t) != REALPART_EXPR); + + if (TREE_CODE (t) == SSA_NAME + && m_dead_ssas.contains (t)) + recreate = true; + + if (!m_split_modifications_p) + continue; + + tree base; + unsigned agg_arg_offset; + if (!isra_get_ref_base_and_offset (t, &base, &agg_arg_offset)) + continue; + + bool by_ref = false; + if (TREE_CODE (base) == SSA_NAME) { - tree t = gimple_call_arg (stmt, i); - gcc_assert (TREE_CODE (t) != BIT_FIELD_REF - && TREE_CODE (t) != IMAGPART_EXPR - && TREE_CODE (t) != REALPART_EXPR); - - tree base; - unsigned unit_offset; - if (!isra_get_ref_base_and_offset (t, &base, &unit_offset)) + if (!SSA_NAME_IS_DEFAULT_DEF (base)) continue; + base = SSA_NAME_VAR (base); + gcc_checking_assert (base); + by_ref = true; + } + if (TREE_CODE (base) != PARM_DECL) + continue; - bool by_ref = false; - if (TREE_CODE (base) == SSA_NAME) + bool base_among_replacements = false; + unsigned j, repl_list_len = m_replacements.length (); + for (j = 0; j < repl_list_len; j++) + { + ipa_param_body_replacement *pbr = &m_replacements[j]; + if (pbr->base == base) { - if (!SSA_NAME_IS_DEFAULT_DEF (base)) - continue; - base = SSA_NAME_VAR (base); - gcc_checking_assert (base); - by_ref = true; + base_among_replacements = true; + break; } - if (TREE_CODE (base) != PARM_DECL) - continue; + } + if (!base_among_replacements) + continue; - bool base_among_replacements = false; - unsigned j, repl_list_len = m_replacements.length (); - for (j = 0; j < repl_list_len; j++) + /* We still have to distinguish between an end-use that we have to + transform now and a pass-through, which happens in the following + two cases. */ + + /* TODO: After we adjust ptr_parm_has_nonarg_uses to also consider + &MEM_REF[ssa_name + offset], we will also have to detect that case + here. */ + + if (TREE_CODE (t) == SSA_NAME + && SSA_NAME_IS_DEFAULT_DEF (t) + && SSA_NAME_VAR (t) + && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL) + { + /* This must be a by_reference pass-through. */ + recreate = true; + gcc_assert (POINTER_TYPE_P (TREE_TYPE (t))); + pass_through_args.safe_push (i); + pass_through_pbr_indices.safe_push (j); + pass_through_offsets.safe_push (agg_arg_offset); + } + else if (!by_ref && AGGREGATE_TYPE_P (TREE_TYPE (t))) + { + /* Currently IPA-SRA guarantees the aggregate access type + exactly matches in this case. So if it does not match, it is + a pass-through argument that will be sorted out at edge + redirection time. */ + ipa_param_body_replacement *pbr + = lookup_replacement_1 (base, agg_arg_offset); + + if (!pbr + || (TYPE_MAIN_VARIANT (TREE_TYPE (t)) + != TYPE_MAIN_VARIANT (TREE_TYPE (pbr->repl)))) { - ipa_param_body_replacement *pbr = &m_replacements[j]; - if (pbr->base == base) - { - base_among_replacements = true; - break; - } - } - if (!base_among_replacements) - continue; - - /* We still have to distinguish between an end-use that we have to - transform now and a pass-through, which happens in the following - two cases. */ - - /* TODO: After we adjust ptr_parm_has_nonarg_uses to also consider - &MEM_REF[ssa_name + offset], we will also have to detect that case - here. */ - - if (TREE_CODE (t) == SSA_NAME - && SSA_NAME_IS_DEFAULT_DEF (t) - && SSA_NAME_VAR (t) - && TREE_CODE (SSA_NAME_VAR (t)) == PARM_DECL) - { - /* This must be a by_reference pass-through. */ - gcc_assert (POINTER_TYPE_P (TREE_TYPE (t))); + recreate = true; pass_through_args.safe_push (i); pass_through_pbr_indices.safe_push (j); - } - else if (!by_ref && AGGREGATE_TYPE_P (TREE_TYPE (t))) - { - /* Currently IPA-SRA guarantees the aggregate access type - exactly matches in this case. So if it does not match, it is - a pass-through argument that will be sorted out at edge - redirection time. */ - ipa_param_body_replacement *pbr - = lookup_replacement_1 (base, unit_offset); - - if (!pbr - || (TYPE_MAIN_VARIANT (TREE_TYPE (t)) - != TYPE_MAIN_VARIANT (TREE_TYPE (pbr->repl)))) - { - pass_through_args.safe_push (i); - pass_through_pbr_indices.safe_push (j); - } + pass_through_offsets.safe_push (agg_arg_offset); } } } - unsigned nargs = gimple_call_num_args (stmt); - if (!pass_through_args.is_empty ()) + if (!recreate) { - auto_vec vargs; - unsigned pt_idx = 0; + /* No need to rebuild the statement, let's just modify arguments + and the LHS if/as appropriate. */ + bool modified = false; for (unsigned i = 0; i < nargs; i++) { - if (pt_idx < pass_through_args.length () - && i == pass_through_args[pt_idx]) - { - unsigned j = pass_through_pbr_indices[pt_idx]; - pt_idx++; - tree base = m_replacements[j].base; + tree *t = gimple_call_arg_ptr (stmt, i); + modified |= modify_expression (t, true); + } + if (gimple_call_lhs (stmt)) + { + tree *t = gimple_call_lhs_ptr (stmt); + modified |= modify_expression (t, false); + } + return modified; + } - /* Map base will get mapped to the special transitive-isra marker - dummy decl. */ - struct simple_tree_swap_info swapinfo; - swapinfo.from = base; - swapinfo.to = m_replacements[j].dummy; - swapinfo.done = false; - tree arg = gimple_call_arg (stmt, i); - walk_tree (&arg, remap_split_decl_to_dummy, &swapinfo, NULL); - gcc_assert (swapinfo.done); - vargs.safe_push (arg); - /* Now let's push all replacements pertaining to this parameter - so that all gimple register ones get correct SSA_NAMES. Edge - redirection will weed out the dummy argument as well as all - unused replacements later. */ - unsigned int repl_list_len = m_replacements.length (); - for (; j < repl_list_len; j++) - { - if (m_replacements[j].base != base) - break; - vargs.safe_push (m_replacements[j].repl); - } + auto_vec index_map; + auto_vec pass_through_map; + auto_vec vargs; + int always_copy_delta = 0; + unsigned pt_idx = 0; + int new_arg_idx = 0; + for (unsigned i = 0; i < nargs; i++) + { + if (pt_idx < pass_through_args.length () + && i == pass_through_args[pt_idx]) + { + unsigned j = pass_through_pbr_indices[pt_idx]; + unsigned agg_arg_offset = pass_through_offsets[pt_idx]; + pt_idx++; + always_copy_delta--; + tree base = m_replacements[j].base; + + /* In order to be put into SSA form, we have to push all replacements + pertaining to this parameter as parameters to the call statement. + Edge redirection will need to use edge summary to weed out the + unnecessary ones. */ + unsigned repl_list_len = m_replacements.length (); + for (; j < repl_list_len; j++) + { + if (m_replacements[j].base != base) + break; + if (m_replacements[j].unit_offset < agg_arg_offset) + continue; + pass_through_split_map pt_map; + pt_map.base_index = i; + pt_map.unit_offset + = m_replacements[j].unit_offset - agg_arg_offset; + pt_map.new_index = new_arg_idx; + pass_through_map.safe_push (pt_map); + vargs.safe_push (m_replacements[j].repl); + new_arg_idx++; + always_copy_delta++; + } + index_map.safe_push (-1); + } + else + { + tree t = gimple_call_arg (stmt, i); + if (TREE_CODE (t) == SSA_NAME + && m_dead_ssas.contains (t)) + { + always_copy_delta--; + index_map.safe_push (-1); } else { - tree t = gimple_call_arg (stmt, i); modify_expression (&t, true); vargs.safe_push (t); + index_map.safe_push (new_arg_idx); + new_arg_idx++; } } - gcall *new_stmt = gimple_build_call_vec (gimple_call_fn (stmt), vargs); - if (gimple_has_location (stmt)) - gimple_set_location (new_stmt, gimple_location (stmt)); - gimple_call_set_chain (new_stmt, gimple_call_chain (stmt)); - gimple_call_copy_flags (new_stmt, stmt); - if (tree lhs = gimple_call_lhs (stmt)) - { - modify_expression (&lhs, false); - gimple_call_set_lhs (new_stmt, lhs); - } - *stmt_p = new_stmt; - return true; } - /* Otherwise, no need to rebuild the statement, let's just modify arguments - and the LHS if/as appropriate. */ - bool modified = false; - for (unsigned i = 0; i < nargs; i++) + gcall *new_stmt = gimple_build_call_vec (gimple_call_fn (stmt), vargs); + if (gimple_has_location (stmt)) + gimple_set_location (new_stmt, gimple_location (stmt)); + gimple_call_set_chain (new_stmt, gimple_call_chain (stmt)); + gimple_call_copy_flags (new_stmt, stmt); + if (tree lhs = gimple_call_lhs (stmt)) { - tree *t = gimple_call_arg_ptr (stmt, i); - modified |= modify_expression (t, true); + modify_expression (&lhs, false); + gimple_call_set_lhs (new_stmt, lhs); } + *stmt_p = new_stmt; - if (gimple_call_lhs (stmt)) - { - tree *t = gimple_call_lhs_ptr (stmt); - modified |= modify_expression (t, false); - } - - return modified; + m_new_call_arg_modification_info = true; + if (orig_stmt) + record_argument_state (m_id, orig_stmt, index_map, pass_through_map, + always_copy_delta); + return true; } /* If the statement STMT contains any expressions that need to replaced with a @@ -1720,7 +1904,8 @@ ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p) bool ipa_param_body_adjustments::modify_gimple_stmt (gimple **stmt, - gimple_seq *extra_stmts) + gimple_seq *extra_stmts, + gimple *orig_stmt) { bool modified = false; tree *t; @@ -1740,7 +1925,7 @@ ipa_param_body_adjustments::modify_gimple_stmt (gimple **stmt, break; case GIMPLE_CALL: - modified |= modify_call_stmt ((gcall **) stmt); + modified |= modify_call_stmt ((gcall **) stmt, orig_stmt); break; case GIMPLE_ASM: @@ -1797,7 +1982,7 @@ ipa_param_body_adjustments::modify_cfun_body () gimple *stmt = gsi_stmt (gsi); gimple *stmt_copy = stmt; gimple_seq extra_stmts = NULL; - bool modified = modify_gimple_stmt (&stmt, &extra_stmts); + bool modified = modify_gimple_stmt (&stmt, &extra_stmts, NULL); if (stmt != stmt_copy) { gcc_checking_assert (modified); @@ -1945,4 +2130,3 @@ ipa_param_body_adjustments::perform_cfun_body_modifications () return cfg_changed; } - diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h index c80e1bc5d6b..f59d17717ee 100644 --- a/gcc/ipa-param-manipulation.h +++ b/gcc/ipa-param-manipulation.h @@ -54,7 +54,7 @@ or only a vector of ipa_adjusted_params. When these classes are used in the context of call graph clone materialization and subsequent call statement redirection - which is the point at which we modify arguments in call statements - they need to cooperate with each other in -order to handle what we refer to as transitive (IPA-SRA) splits. These are +order to handle what we refer to as pass-through (IPA-SRA) splits. These are situations when a formal parameter of one function is split into several smaller ones and some of them are then passed on in a call to another function because the formal parameter of this callee has also been split. @@ -83,7 +83,7 @@ baz () Both bar and foo would have their parameter split. Foo would receive one replacement representing s.b. Function bar would see its parameter split into one replacement representing z.s.a and another representing z.s.b which would -be passed on to foo. It would be a so called transitive split IPA-SRA +be passed on to foo. It would be a so called pass-through split IPA-SRA replacement, one which is passed in a call as an actual argument to another IPA-SRA replacement in another function. @@ -95,30 +95,25 @@ all of the above. Call redirection has to be able to find the right decl or SSA_NAME that corresponds to the transitive split in the caller. The SSA names are assigned -right after clone materialization/ modification and cannot be "added" -afterwards. Moreover, if the caller has been inlined the SSA_NAMEs in question -no longer belong to PARM_DECLs but to VAR_DECLs, indistinguishable from any -others. +right after clone materialization/ modification and cannot be "added" to call +arguments at any later point. Moreover, if the caller has been inlined the +SSA_NAMEs in question no longer belong to PARM_DECLs but to VAR_DECLs, +indistinguishable from any others. Therefore, when clone materialization finds a call statement which it knows is -a part of a transitive split, it will modify it into: +a part of a transitive split, it will simply add as arguments all new "split" +replacements (that have grater or equal offset than the original call +argument): - foo (DUMMY_Z_VAR.s, repl_for_a, repl_for_b, ); + foo (repl_for_a, repl_for_b, ); -It will also store {DUMMY_S_VAR, 32} and {DUMMY_S_VAR, 64} representing offsets -of z.s.a and z.s.b (assuming a 32-bit int) into foo's cgraph node -clone->performed_splits vector (which is storing structures of type -ipa_param_performed_split also defined in this header file). - -Call redirection will identify that expression DUMMY_Z_VAR.s is based on a -variable stored in performed_splits vector and learn that the following -arguments, already in SSA form, represent offsets 32 and 64 in a split original -parameter. It subtracts offset of DUMMY_Z_VAR.s from 32 and 64 and arrives at -offsets 0 and 32 within callee's original parameter. At this point it also -knows from the call graph that only the bit with offset 32 is needed and so -changes the call statement into final: - -bar (repl_for_b, ); */ +It will also store into ipa_edge_modification_info (which is internal to +ipa-param-modification.c) information about which replacement is which and +where original arguments are. Call redirection will then invoke +ipa_param_adjustments::modify_call which will access this information and +eliminate all replacements which the callee does not expect (repl_for_a in our +example above). In between these two steps, however, a call statement might +have extraneous arguments. */ #ifndef IPA_PARAM_MANIPULATION_H #define IPA_PARAM_MANIPULATION_H @@ -207,21 +202,6 @@ struct GTY(()) ipa_adjusted_param void ipa_dump_adjusted_parameters (FILE *f, vec *adj_params); -/* Structure to remember the split performed on a node so that edge redirection - (i.e. splitting arguments of call statements) know how split formal - parameters of the caller are represented. */ - -struct GTY(()) ipa_param_performed_split -{ - /* The dummy VAR_DECL that was created instead of the split parameter that - sits in the call in the meantime between clone materialization and call - redirection. All entries in a vector of performed splits that correspond - to the same dumy decl must be grouped together. */ - tree dummy_decl; - /* Offset into the original parameter. */ - unsigned unit_offset; -}; - /* Class used to record planned modifications to parameters of a function and also to perform necessary modifications at the caller side at the gimple level. Used to describe all cgraph node clones that have their parameters @@ -244,9 +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 (gcall *stmt, - vec *performed_splits, - tree callee_decl, bool update_references); + 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. */ @@ -293,15 +271,9 @@ struct ipa_param_body_replacement tree base; /* The new decl it should be replaced with. */ tree repl; - /* When modifying clones during IPA clone materialization, this is a dummy - decl used to mark calls in which we need to apply transitive splitting, - these dummy delcls are inserted as arguments to such calls and then - followed by all the replacements with offset info stored in - ipa_param_performed_split. - - Users of ipa_param_body_adjustments that modify standalone functions - outside of IPA clone materialization can use this field for their internal - purposes. */ + /* Users of ipa_param_body_adjustments that modify standalone functions + outside of IPA clone materialization can use the following field for their + internal purposes. */ tree dummy; /* The offset within BASE that REPL represents. */ unsigned unit_offset; @@ -342,8 +314,7 @@ public: /* Change the PARM_DECLs. */ void modify_formal_parameters (); /* Register a replacement decl for the transformation done in APM. */ - void register_replacement (ipa_adjusted_param *apm, tree replacement, - tree dummy = NULL_TREE); + void register_replacement (ipa_adjusted_param *apm, tree replacement); /* Lookup a replacement for a given offset within a given parameter. */ tree lookup_replacement (tree base, unsigned unit_offset); /* Lookup a replacement for an expression, if there is one. */ @@ -353,7 +324,8 @@ public: parameter. */ tree get_replacement_ssa_base (tree old_decl); /* Modify a statement. */ - bool modify_gimple_stmt (gimple **stmt, gimple_seq *extra_stmts); + bool modify_gimple_stmt (gimple **stmt, gimple_seq *extra_stmts, + gimple *orig_stmt); /* Return the new chain of parameters. */ tree get_new_param_chain (); @@ -370,6 +342,12 @@ public: /* Set to true if there are any IPA_PARAM_OP_SPLIT adjustments among stored adjustments. */ bool m_split_modifications_p; + + /* Sets of statements and SSA_NAMEs that only manipulate data from parameters + removed because they are not necessary. */ + hash_set m_dead_stmts; + hash_set m_dead_ssas; + private: void common_initialization (tree old_fndecl, tree *vars, vec *tree_map); @@ -380,9 +358,10 @@ private: tree replace_removed_params_ssa_names (tree old_name, gimple *stmt); bool modify_expression (tree *expr_p, bool convert); bool modify_assignment (gimple *stmt, gimple_seq *extra_stmts); - bool modify_call_stmt (gcall **stmt_p); + bool modify_call_stmt (gcall **stmt_p, gimple *orig_stmt); bool modify_cfun_body (); void reset_debug_stmts (); + void mark_dead_statements (tree dead_param); /* Declaration of the function that is being transformed. */ @@ -427,9 +406,16 @@ private: its this pointer and must be converted to a normal function. */ bool m_method2func; + + /* Set to true if any new information has been stored to + ipa_edge_modifications as part of this body transformation. */ + + bool m_new_call_arg_modification_info; }; void push_function_arg_decls (vec *args, tree fndecl); void push_function_arg_types (vec *types, tree fntype); +void ipa_verify_edge_has_no_modifications (cgraph_edge *cs); + #endif /* IPA_PARAM_MANIPULATION_H */ diff --git a/gcc/symtab-clones.h b/gcc/symtab-clones.h index 5695a434f6a..a6ad4a6e27f 100644 --- a/gcc/symtab-clones.h +++ b/gcc/symtab-clones.h @@ -26,8 +26,7 @@ struct GTY(()) clone_info /* Constructor. */ clone_info () : tree_map (NULL), - param_adjustments (NULL), - performed_splits (NULL) + param_adjustments (NULL) { } /* Constants discovered by IPA-CP, i.e. which parameter should be replaced @@ -35,18 +34,6 @@ struct GTY(()) clone_info vec *tree_map; /* Parameter modification that IPA-SRA decided to perform. */ ipa_param_adjustments *param_adjustments; - /* Lists of dummy-decl and offset pairs representing split formal parameters - in the caller. Offsets of all new replacements are enumerated, those - coming from the same original parameter have the same dummy decl stored - along with them. - - Dummy decls sit in call statement arguments followed by new parameter - decls (or their SSA names) in between (caller) clone materialization and - call redirection. Redirection then recognizes the dummy variable and - together with the stored offsets can reconstruct what exactly the new - parameter decls represent and can leave in place only those that the - callee expects. */ - vec *performed_splits; /* Return clone_info, if available. */ static clone_info *get (cgraph_node *node); diff --git a/gcc/testsuite/g++.dg/ipa/ipa-sra-4.C b/gcc/testsuite/g++.dg/ipa/ipa-sra-4.C new file mode 100644 index 00000000000..56d59f9fd9a --- /dev/null +++ b/gcc/testsuite/g++.dg/ipa/ipa-sra-4.C @@ -0,0 +1,37 @@ +/* { dg-do compile { target c++11 } } */ +/* { dg-options "-O2 -fipa-sra" } */ + +void __throw_bad_alloc() __attribute__((__noreturn__)); +void __throw_bad_array_new_length(); +template class allocator {}; +template struct allocator_traits; +int *allocate___trans_tmp_2; +template struct allocator_traits> { + using allocator_type = allocator<_Tp>; + using pointer = _Tp *; + using size_type = long; + static pointer allocate(allocator_type &, size_type __n) { + long __trans_tmp_3 = __n; + if (__builtin_expect(__trans_tmp_3, false)) + if (__trans_tmp_3) + __throw_bad_array_new_length(); + operator new(sizeof(int)); + return allocate___trans_tmp_2; + } +}; +class throw_allocator_base { + allocator _M_allocator; +public: + int *allocate(long __n) { + if (__n) + __throw_bad_alloc(); + int *a = allocator_traits>::allocate(_M_allocator, __n); + return a; + } +}; +template void check_allocate_max_size() { + Alloc a; + long __trans_tmp_1 = 0; + a.allocate(__trans_tmp_1 + 1); +} +int main() { check_allocate_max_size(); } diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c new file mode 100644 index 00000000000..f438b509614 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c @@ -0,0 +1,24 @@ +/* { dg-do compile } */ +/* { dg-options "-O2" } */ + +extern int g; + +static int __attribute__((noinline)) +bar (int i, int j) +{ + return 2*g + i; +} + +static int __attribute__((noinline)) +foo (int i, int j) +{ + if (i > 5) + j = 22; + return bar (i, j) + 1; +} + +int +entry (int l, int k) +{ + return foo (l, k); +} diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c new file mode 100644 index 00000000000..7b5bf0825fc --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-24.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -Wmaybe-uninitialized -Werror" } */ + +int *ttmp_1; +_Bool pt_ins_tipdo, pq_ins_apd, pq_ins_tt2; +int gtrphdt; + +void pl_ins(int, _Bool, _Bool); +inline void pt_ins(int *, _Bool apdo) { + int list = *ttmp_1; + pl_ins(list, apdo, pt_ins_tipdo); +} +void pq_ins(int *t) { + if (pq_ins_tt2) + pt_ins(t, pq_ins_apd); +} +int gtr_post_hd() { + pq_ins(>rphdt); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/ipa/pr93385.c b/gcc/testsuite/gcc.dg/ipa/pr93385.c new file mode 100644 index 00000000000..6d1d0d7cd27 --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr93385.c @@ -0,0 +1,27 @@ +/* { dg-do run } */ +/* { dg-options "-O2 -fno-dce -fno-ipa-cp -fno-tree-dce" } */ + +char a, b; + +#ifdef __SIZEOF_INT128__ +#define T unsigned __int128 +#else +#define T unsigned +#endif + +static inline int +c (T d) +{ + char e = 0; + d %= (unsigned) d; + e -= 0; + __builtin_strncpy (&a, &e, 1); + return e + b; +} + +int +main (void) +{ + c (~0); + return 0; +} diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 1dcb31c0267..165c4ad7c72 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -1528,6 +1528,11 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id) : !opt_for_fn (id->dst_fn, flag_var_tracking_assignments))) return NULL; + if (!is_gimple_debug (stmt) + && id->param_body_adjs + && id->param_body_adjs->m_dead_stmts.contains (stmt)) + return NULL; + /* Begin by recognizing trees that we'll completely rewrite for the inlining context. Our output for these trees is completely different from our input (e.g. RETURN_EXPR is deleted and morphs @@ -1792,10 +1797,15 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id) if (gimple_debug_bind_p (stmt)) { + tree value; + if (id->param_body_adjs + && id->param_body_adjs->m_dead_stmts.contains (stmt)) + value = NULL_TREE; + else + value = gimple_debug_bind_get_value (stmt); gdebug *copy = gimple_build_debug_bind (gimple_debug_bind_get_var (stmt), - gimple_debug_bind_get_value (stmt), - stmt); + value, stmt); if (id->reset_location) gimple_set_location (copy, input_location); id->debug_stmts.safe_push (copy); @@ -1924,7 +1934,7 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id) if (id->param_body_adjs) { gimple_seq extra_stmts = NULL; - id->param_body_adjs->modify_gimple_stmt (©, &extra_stmts); + id->param_body_adjs->modify_gimple_stmt (©, &extra_stmts, stmt); if (!gimple_seq_empty_p (extra_stmts)) { memset (&wi, 0, sizeof (wi)); @@ -2674,7 +2684,9 @@ copy_phis_for_bb (basic_block bb, copy_body_data *id) phi = si.phi (); res = PHI_RESULT (phi); new_res = res; - if (!virtual_operand_p (res)) + if (!virtual_operand_p (res) + && (!id->param_body_adjs + || !id->param_body_adjs->m_dead_stmts.contains (phi))) { walk_tree (&new_res, copy_tree_body_r, id, NULL); if (EDGE_COUNT (new_bb->preds) == 0) @@ -4740,7 +4752,6 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id, use_operand_p use; gimple *simtenter_stmt = NULL; vec *simtvars_save; - clone_info *info; /* The gimplifier uses input_location in too many places, such as internal_get_tmp_var (). */ @@ -5065,40 +5076,6 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id, /* Add local vars in this inlined callee to caller. */ add_local_variables (id->src_cfun, cfun, id); - info = clone_info::get (id->src_node); - if (info && info->performed_splits) - { - clone_info *dst_info = clone_info::get_create (id->dst_node); - /* Any calls from the inlined function will be turned into calls from the - function we inline into. We must preserve notes about how to split - parameters such calls should be redirected/updated. */ - unsigned len = vec_safe_length (info->performed_splits); - for (unsigned i = 0; i < len; i++) - { - ipa_param_performed_split ps - = (*info->performed_splits)[i]; - ps.dummy_decl = remap_decl (ps.dummy_decl, id); - vec_safe_push (dst_info->performed_splits, ps); - } - - if (flag_checking) - { - len = vec_safe_length (dst_info->performed_splits); - for (unsigned i = 0; i < len; i++) - { - ipa_param_performed_split *ps1 - = &(*dst_info->performed_splits)[i]; - for (unsigned j = i + 1; j < len; j++) - { - ipa_param_performed_split *ps2 - = &(*dst_info->performed_splits)[j]; - gcc_assert (ps1->dummy_decl != ps2->dummy_decl - || ps1->unit_offset != ps2->unit_offset); - } - } - } - } - if (dump_enabled_p ()) { char buf[128]; @@ -6117,23 +6094,10 @@ tree_versionable_function_p (tree fndecl) static void update_clone_info (copy_body_data * id) { - clone_info *dst_info = clone_info::get (id->dst_node); - vec *cur_performed_splits - = dst_info ? dst_info->performed_splits : NULL; - if (cur_performed_splits) - { - unsigned len = cur_performed_splits->length (); - for (unsigned i = 0; i < len; i++) - { - ipa_param_performed_split *ps = &(*cur_performed_splits)[i]; - ps->dummy_decl = remap_decl (ps->dummy_decl, id); - } - } - - struct cgraph_node *node; - if (!id->dst_node->clones) + struct cgraph_node *this_node = id->dst_node; + if (!this_node->clones) return; - for (node = id->dst_node->clones; node != id->dst_node;) + for (cgraph_node *node = this_node->clones; node != this_node;) { /* First update replace maps to match the new body. */ clone_info *info = clone_info::get (node); @@ -6147,53 +6111,6 @@ update_clone_info (copy_body_data * id) walk_tree (&replace_info->new_tree, copy_tree_body_r, id, NULL); } } - if (info && info->performed_splits) - { - unsigned len = vec_safe_length (info->performed_splits); - for (unsigned i = 0; i < len; i++) - { - ipa_param_performed_split *ps - = &(*info->performed_splits)[i]; - ps->dummy_decl = remap_decl (ps->dummy_decl, id); - } - } - if (unsigned len = vec_safe_length (cur_performed_splits)) - { - /* We do not want to add current performed splits when we are saving - a copy of function body for later during inlining, that would just - duplicate all entries. So let's have a look whether anything - referring to the first dummy_decl is present. */ - if (!info) - info = clone_info::get_create (node); - unsigned dst_len = vec_safe_length (info->performed_splits); - ipa_param_performed_split *first = &(*cur_performed_splits)[0]; - for (unsigned i = 0; i < dst_len; i++) - if ((*info->performed_splits)[i].dummy_decl - == first->dummy_decl) - { - len = 0; - break; - } - - for (unsigned i = 0; i < len; i++) - vec_safe_push (info->performed_splits, - (*cur_performed_splits)[i]); - if (flag_checking) - { - for (unsigned i = 0; i < dst_len; i++) - { - ipa_param_performed_split *ps1 - = &(*info->performed_splits)[i]; - for (unsigned j = i + 1; j < dst_len; j++) - { - ipa_param_performed_split *ps2 - = &(*info->performed_splits)[j]; - gcc_assert (ps1->dummy_decl != ps2->dummy_decl - || ps1->unit_offset != ps2->unit_offset); - } - } - } - } if (node->clones) node = node->clones; From patchwork Tue Mar 30 13:03:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1460032 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4F8qPv0SfWz9sVt for ; Wed, 31 Mar 2021 00:03:30 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id A17EF38515F3; Tue, 30 Mar 2021 13:03:28 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id C0D843858002 for ; Tue, 30 Mar 2021 13:03:23 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org C0D843858002 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mjambor@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id D6F10B2FF; Tue, 30 Mar 2021 13:03:22 +0000 (UTC) From: Martin Jambor To: GCC Patches Subject: [GCC 12 PATCH 2/2] ipa-sra: Improve debug info for removed parameters (PR 93385) User-Agent: Notmuch/0.31.4 (https://notmuchmail.org) Emacs/27.1 (x86_64-suse-linux-gnu) Date: Tue, 30 Mar 2021 15:03:22 +0200 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-3039.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, 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: , Cc: Jan Hubicka Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, Whereas the previous patch fixed issues with code left behind after IPA-SRA removed a parameter but only reset all affected debug bind statements, this one updates them with expressions which can allow the debugger to print the removed value - see the added test-case. Even though I originally did not want to create DEBUG_EXPR_DECLs for intermediate values, I ended up doing so, because otherwise the code started creating statements like # DEBUG __aD.198693 => &MEM[(const struct _Alloc_nodeD.171110 *)D#195]._M_tD.184726->_M_implD.171154 which not only is a bit scary but also gimple-fold ICEs on it. Therefore I decided they are probably quite necessary and have them. The patch simply notes each removed SSA name present in a debug statement and then works from it backwards, looking if it can reconstruct the expression it represents (which can fail if a non-degenerate PHI node is in the way). If it can, it populates two hash maps with those expressions so that 1) removed assignments are replaced with a debug bind defining a new intermediate debug_decl_expr and 2) existing debug binds that refer to SSA names that are bing removed now refer to corresponding debug_decl_exprs. If a removed parameter is passed to another function, the debugging information still cannot describe its value there - see the xfailed test in the testcase. I sort of know what needs to be done but the handling of debug information for removed parameters is LTO unfriendly in general and so needs a bit more work. Bootstrapped and tested on x86_64-linux, i686-linux and aarch64-linux. Also LTO-bootstrapped and LTO-profiledbootstrapped on x86_64-linux. I would like to commit the patch early in stage1 for GCC 12 and will be grateful for a review and any feedback. Thanks, Martin gcc/ChangeLog: 2021-03-29 Martin Jambor PR ipa/93385 * ipa-param-manipulation.h (class ipa_param_body_adjustments): New members remap_with_debug_expressions, m_dead_ssa_debug_equiv, m_dead_stmt_debug_equiv and prepare_debug_expressions. Added parameter to mark_dead_statements. * ipa-param-manipulation.c: Include tree-phinodes.h and cfgexpand.h. (ipa_param_body_adjustments::mark_dead_statements): New parameter debugstack, push into it all SSA names used in debug statements, produce m_dead_ssa_debug_equiv mapping for the removed param. (replace_with_mapped_expr): New function. (ipa_param_body_adjustments::remap_with_debug_expressions): Likewise. (ipa_param_body_adjustments::prepare_debug_expressions): Likewise. (ipa_param_body_adjustments::common_initialization): Gather and procecc SSA which will be removed but are in debug statements. Simplify. (ipa_param_body_adjustments::ipa_param_body_adjustments): Initialize new members. * tree-inline.c (remap_gimple_stmt): Create a debug bind when possible when avoiding a copy of an unnecessary statement. Remap removed SSA names in existing debug statements. (tree_function_versioning): Do not create DEBUG_EXPR_DECL for removed parameters if we have already done so. gcc/testsuite/ChangeLog: 2021-03-29 Martin Jambor PR ipa/93385 * gcc.dg/guality/ipa-sra-1.c: New test. --- gcc/ipa-param-manipulation.c | 281 ++++++++++++++++++----- gcc/ipa-param-manipulation.h | 12 +- gcc/testsuite/gcc.dg/guality/ipa-sra-1.c | 45 ++++ gcc/tree-inline.c | 45 ++-- 4 files changed, 306 insertions(+), 77 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/guality/ipa-sra-1.c diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c index 3e07fd72fe2..a202501fc95 100644 --- a/gcc/ipa-param-manipulation.c +++ b/gcc/ipa-param-manipulation.c @@ -43,6 +43,8 @@ along with GCC; see the file COPYING3. If not see #include "alloc-pool.h" #include "symbol-summary.h" #include "symtab-clones.h" +#include "tree-phinodes.h" +#include "cfgexpand.h" /* Actual prefixes of different newly synthetized parameters. Keep in sync @@ -989,10 +991,12 @@ phi_arg_will_live_p (gphi *phi, bitmap blocks_to_copy, tree arg) /* Populate m_dead_stmts given that DEAD_PARAM is going to be removed without any replacement or splitting. REPL is the replacement VAR_SECL to base any - remaining uses of a removed parameter on. */ + remaining uses of a removed parameter on. Push all removed SSA names that + are used within debug statements to DEBUGSTACK. */ void -ipa_param_body_adjustments::mark_dead_statements (tree dead_param) +ipa_param_body_adjustments::mark_dead_statements (tree dead_param, + vec *debugstack) { /* Current IPA analyses which remove unused parameters never remove a non-gimple register ones which have any use except as parameters in other @@ -1004,6 +1008,7 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param) return; auto_vec stack; + hash_set used_in_debug; m_dead_ssas.add (parm_ddef); stack.safe_push (parm_ddef); while (!stack.is_empty ()) @@ -1026,6 +1031,11 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param) { m_dead_stmts.add (stmt); gcc_assert (gimple_debug_bind_p (stmt)); + if (!used_in_debug.contains (t)) + { + used_in_debug.add (t); + debugstack->safe_push (t); + } } else if (gimple_code (stmt) == GIMPLE_PHI) { @@ -1054,6 +1064,149 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param) gcc_unreachable (); } } + + if (!MAY_HAVE_DEBUG_STMTS) + { + gcc_assert (debugstack->is_empty ()); + return; + } + + tree dp_ddecl = make_node (DEBUG_EXPR_DECL); + DECL_ARTIFICIAL (dp_ddecl) = 1; + TREE_TYPE (dp_ddecl) = TREE_TYPE (dead_param); + SET_DECL_MODE (dp_ddecl, DECL_MODE (dead_param)); + m_dead_ssa_debug_equiv.put (parm_ddef, dp_ddecl); +} + +/* Callback to walk_tree. If REMAP is an SSA_NAME that is present in hash_map + passed in DATA, replace it with unshared version of what it was mapped + to. */ + +static tree +replace_with_mapped_expr (tree *remap, int *walk_subtrees, void *data) +{ + if (TYPE_P (*remap)) + { + *walk_subtrees = 0; + return 0; + } + if (TREE_CODE (*remap) != SSA_NAME) + return 0; + + *walk_subtrees = 0; + + hash_map *equivs = (hash_map *) data; + if (tree *p = equivs->get (*remap)) + *remap = unshare_expr (*p); + return 0; +} + +/* Replace all occurances of SSAs in m_dead_ssa_debug_equiv in t with what they + are mapped to. */ + +void +ipa_param_body_adjustments::remap_with_debug_expressions (tree *t) +{ + /* If *t is an SSA_NAME which should have its debug statements reset, it is + mapped to NULL in the hash_map. We need to handle that case separately or + otherwise the walker would segfault. No expression that is more + complicated than that can have its operands mapped to NULL. */ + if (TREE_CODE (*t) == SSA_NAME) + { + if (tree *p = m_dead_ssa_debug_equiv.get (*t)) + *t = *p; + } + else + walk_tree (t, replace_with_mapped_expr, &m_dead_ssa_debug_equiv, NULL); +} + +/* For an SSA_NAME DEAD_SSA which is about to be DCEd because it is based on a + useless parameter, prepare an expression that should represent it in + debug_binds in the cloned function and add a mapping from DEAD_SSA to + m_dead_ssa_debug_equiv. That mapping is to NULL when the associated + debug_statement has to be reset instead. In such case return false, + ottherwise return true. If DEAD_SSA comes from a basic block which is not + about to be copied, ignore it and return true. */ + +bool +ipa_param_body_adjustments::prepare_debug_expressions (tree dead_ssa) +{ + gcc_checking_assert (m_dead_ssas.contains (dead_ssa)); + if (tree *d = m_dead_ssa_debug_equiv.get (dead_ssa)) + return (*d != NULL_TREE); + + gcc_assert (!SSA_NAME_IS_DEFAULT_DEF (dead_ssa)); + gimple *def = SSA_NAME_DEF_STMT (dead_ssa); + if (m_id->blocks_to_copy + && !bitmap_bit_p (m_id->blocks_to_copy, gimple_bb (def)->index)) + return true; + + if (gimple_code (def) == GIMPLE_PHI) + { + /* In theory, we could ignore all SSAs coming from BBs not in + m_id->blocks_to_copy but at the time of the writing this code that + should never really be the case because only fnsplit uses that bitmap, + so don't bother. */ + tree value = degenerate_phi_result (as_a (def)); + if (!value + || (m_dead_ssas.contains (value) + && !prepare_debug_expressions (value))) + { + m_dead_ssa_debug_equiv.put (dead_ssa, NULL_TREE); + return false; + } + + gcc_assert (TREE_CODE (value) == SSA_NAME); + tree *d = m_dead_ssa_debug_equiv.get (value); + m_dead_ssa_debug_equiv.put (dead_ssa, *d); + return true; + } + + bool lost = false; + use_operand_p use_p; + ssa_op_iter oi; + FOR_EACH_PHI_OR_STMT_USE (use_p, def, oi, SSA_OP_USE) + { + tree use = USE_FROM_PTR (use_p); + if (m_dead_ssas.contains (use) + && !prepare_debug_expressions (use)) + { + lost = true; + break; + } + } + + if (lost) + { + m_dead_ssa_debug_equiv.put (dead_ssa, NULL_TREE); + return false; + } + + if (is_gimple_assign (def)) + { + gcc_assert (!gimple_clobber_p (def)); + if (gimple_assign_copy_p (def) + && TREE_CODE (gimple_assign_rhs1 (def)) == SSA_NAME) + { + tree *d = m_dead_ssa_debug_equiv.get (gimple_assign_rhs1 (def)); + m_dead_ssa_debug_equiv.put (dead_ssa, *d); + return (*d != NULL_TREE); + } + + tree val = gimple_assign_rhs_to_tree (def); + SET_EXPR_LOCATION (val, UNKNOWN_LOCATION); + remap_with_debug_expressions (&val); + + tree vexpr = make_node (DEBUG_EXPR_DECL); + DECL_ARTIFICIAL (vexpr) = 1; + TREE_TYPE (vexpr) = TREE_TYPE (val); + SET_DECL_MODE (vexpr, TYPE_MODE (TREE_TYPE (val))); + m_dead_stmt_debug_equiv.put (def, val); + m_dead_ssa_debug_equiv.put (dead_ssa, vexpr); + return true; + } + else + gcc_unreachable (); } /* Common initialization performed by all ipa_param_body_adjustments @@ -1145,65 +1298,21 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl, gcc_unreachable (); } - - /* As part of body modifications, we will also have to replace remaining uses - of remaining uses of removed PARM_DECLs (which do not however use the - initial value) with their VAR_DECL copies. - - We do this differently with and without m_id. With m_id, we rely on its - mapping and create a replacement straight away. Without it, we have our - own mechanism for which we have to populate m_removed_decls vector. Just - don't mix them, that is why you should not call - replace_removed_params_ssa_names or perform_cfun_body_modifications when - you construct with ID not equal to NULL. */ - - unsigned op_len = m_oparms.length (); - for (unsigned i = 0; i < op_len; i++) - if (!kept[i]) - { - if (m_id) - { - if (!m_id->decl_map->get (m_oparms[i])) - { - tree var = copy_decl_to_var (m_oparms[i], m_id); - insert_decl_map (m_id, m_oparms[i], var); - /* Declare this new variable. */ - DECL_CHAIN (var) = *vars; - *vars = var; - - /* If this is not a split but a real removal, init hash sets - that will guide what not to copy to the new body. */ - if (!split[i]) - mark_dead_statements (m_oparms[i]); - } - } - else - { - m_removed_decls.safe_push (m_oparms[i]); - m_removed_map.put (m_oparms[i], m_removed_decls.length () - 1); - } - } - - if (!MAY_HAVE_DEBUG_STMTS) - return; - - /* Finally, when generating debug info, we fill vector m_reset_debug_decls - with removed parameters declarations. We do this in order to re-map their - debug bind statements and create debug decls for them. */ - if (tree_map) { - /* Do not output debuginfo for parameter declarations as if they vanished - when they were in fact replaced by a constant. */ + /* Do not treat parameters which were replaced with a constant as + completely vanished. */ auto_vec index_mapping; bool need_remap = false; - clone_info *info = clone_info::get (m_id->src_node); - if (m_id && info && info->param_adjustments) + if (m_id) { - ipa_param_adjustments *prev_adjustments = info->param_adjustments; - prev_adjustments->get_updated_indices (&index_mapping); - need_remap = true; + clone_info *cinfo = clone_info::get (m_id->src_node); + if (cinfo && cinfo->param_adjustments) + { + cinfo->param_adjustments->get_updated_indices (&index_mapping); + need_remap = true; + } } for (unsigned i = 0; i < tree_map->length (); i++) @@ -1216,9 +1325,52 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl, } } + /* As part of body modifications, we will also have to replace remaining uses + of remaining uses of removed PARM_DECLs (which do not however use the + initial value) with their VAR_DECL copies. + + We do this differently with and without m_id. With m_id, we rely on its + mapping and create a replacement straight away. Without it, we have our + own mechanism for which we have to populate m_removed_decls vector. Just + don't mix them, that is why you should not call + replace_removed_params_ssa_names or perform_cfun_body_modifications when + you construct with ID not equal to NULL. */ + + auto_vec ssas_to_process_debug; + unsigned op_len = m_oparms.length (); for (unsigned i = 0; i < op_len; i++) - if (!kept[i] && is_gimple_reg (m_oparms[i])) - m_reset_debug_decls.safe_push (m_oparms[i]); + if (!kept[i]) + { + if (m_id) + { + gcc_assert (!m_id->decl_map->get (m_oparms[i])); + tree var = copy_decl_to_var (m_oparms[i], m_id); + insert_decl_map (m_id, m_oparms[i], var); + /* Declare this new variable. */ + DECL_CHAIN (var) = *vars; + *vars = var; + + /* If this is not a split but a real removal, init hash sets + that will guide what not to copy to the new body. */ + if (!split[i]) + mark_dead_statements (m_oparms[i], &ssas_to_process_debug); + if (MAY_HAVE_DEBUG_STMTS + && is_gimple_reg (m_oparms[i])) + m_reset_debug_decls.safe_push (m_oparms[i]); + } + else + { + m_removed_decls.safe_push (m_oparms[i]); + m_removed_map.put (m_oparms[i], m_removed_decls.length () - 1); + if (MAY_HAVE_DEBUG_STMTS + && !kept[i] + && is_gimple_reg (m_oparms[i])) + m_reset_debug_decls.safe_push (m_oparms[i]); + } + } + + while (!ssas_to_process_debug.is_empty ()) + prepare_debug_expressions (ssas_to_process_debug.pop ()); } /* Constructor of ipa_param_body_adjustments from a simple list of @@ -1232,9 +1384,10 @@ ipa_param_body_adjustments tree fndecl) : m_adj_params (adj_params), m_adjustments (NULL), m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (), m_dead_ssas (), - m_fndecl (fndecl), m_id (NULL), m_oparms (), m_new_decls (), - m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (), - m_method2func (false), m_new_call_arg_modification_info (false) + m_dead_ssa_debug_equiv (), m_dead_stmt_debug_equiv (), m_fndecl (fndecl), + m_id (NULL), m_oparms (), m_new_decls (), m_new_types (), m_replacements (), + m_removed_decls (), m_removed_map (), m_method2func (false), + m_new_call_arg_modification_info (false) { common_initialization (fndecl, NULL, NULL); } @@ -1249,7 +1402,8 @@ ipa_param_body_adjustments tree fndecl) : m_adj_params (adjustments->m_adj_params), m_adjustments (adjustments), m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (), - m_dead_ssas (), m_fndecl (fndecl), m_id (NULL), m_oparms (), m_new_decls (), + m_dead_ssas (), m_dead_ssa_debug_equiv (), m_dead_stmt_debug_equiv (), + m_fndecl (fndecl), m_id (NULL), m_oparms (), m_new_decls (), m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (), m_method2func (false), m_new_call_arg_modification_info (false) { @@ -1272,8 +1426,9 @@ ipa_param_body_adjustments vec *tree_map) : m_adj_params (adjustments->m_adj_params), m_adjustments (adjustments), m_reset_debug_decls (), m_split_modifications_p (false), m_dead_stmts (), - m_dead_ssas (),m_fndecl (fndecl), m_id (id), m_oparms (), m_new_decls (), - m_new_types (), m_replacements (), m_removed_decls (), m_removed_map (), + m_dead_ssas (), m_dead_ssa_debug_equiv (), m_dead_stmt_debug_equiv (), + m_fndecl (fndecl), m_id (id), m_oparms (), m_new_decls (), m_new_types (), + m_replacements (), m_removed_decls (), m_removed_map (), m_method2func (false), m_new_call_arg_modification_info (false) { common_initialization (old_fndecl, vars, tree_map); diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h index f59d17717ee..fa05859f1f3 100644 --- a/gcc/ipa-param-manipulation.h +++ b/gcc/ipa-param-manipulation.h @@ -328,6 +328,9 @@ public: gimple *orig_stmt); /* Return the new chain of parameters. */ tree get_new_param_chain (); + /* Replace all occurances of SSAs in m_dead_ssa_debug_equiv in t with what + they are mapped to. */ + void remap_with_debug_expressions (tree *t); /* Pointers to data structures defining how the function should be modified. */ @@ -348,6 +351,12 @@ public: hash_set m_dead_stmts; hash_set m_dead_ssas; + /* Mapping from DCEd SSAs to what their potential debug_binds should be. */ + hash_map m_dead_ssa_debug_equiv; + /* Mapping from DCEd statements to debug expressions that will be placed on + the RHS of debug statement that will replace this one. */ + hash_map m_dead_stmt_debug_equiv; + private: void common_initialization (tree old_fndecl, tree *vars, vec *tree_map); @@ -361,7 +370,8 @@ private: bool modify_call_stmt (gcall **stmt_p, gimple *orig_stmt); bool modify_cfun_body (); void reset_debug_stmts (); - void mark_dead_statements (tree dead_param); + void mark_dead_statements (tree dead_param, vec *debugstack); + bool prepare_debug_expressions (tree dead_ssa); /* Declaration of the function that is being transformed. */ diff --git a/gcc/testsuite/gcc.dg/guality/ipa-sra-1.c b/gcc/testsuite/gcc.dg/guality/ipa-sra-1.c new file mode 100644 index 00000000000..5434b3d7665 --- /dev/null +++ b/gcc/testsuite/gcc.dg/guality/ipa-sra-1.c @@ -0,0 +1,45 @@ +/* { dg-do run } */ +/* { dg-options "-g -fno-ipa-icf" } */ + + +void __attribute__((noipa)) +use (int x) +{ + asm volatile ("" : : "r" (x) : "memory"); +} + +static int __attribute__((noinline)) +bar (int i, int k) +{ + asm ("" : "+r" (i)); + use (i); /* { dg-final { gdb-test . "k" "3" { xfail *-*-* } } } */ + return 6; +} + +volatile int v; + +static int __attribute__((noinline)) +foo (int i, int k) +{ + int r; + v = 9; + k = (k + 14)/k; + r = bar (i, k); /* { dg-final { gdb-test . "k" "3" } } */ + return r; +} + +volatile int v; + +int __attribute__((noipa)) +get_val1 (void) {return 20;} +int __attribute__((noipa)) +get_val2 (void) {return 7;} + +int +main (void) +{ + int k = get_val2 (); + int r = foo (get_val1 (), k); + v = r + k; /* k has to live accross the call or all is probably lost */ + return 0; +} diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c index 165c4ad7c72..0a480e9b0e6 100644 --- a/gcc/tree-inline.c +++ b/gcc/tree-inline.c @@ -1531,7 +1531,21 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id) if (!is_gimple_debug (stmt) && id->param_body_adjs && id->param_body_adjs->m_dead_stmts.contains (stmt)) - return NULL; + { + tree *dval = id->param_body_adjs->m_dead_stmt_debug_equiv.get (stmt); + if (!dval) + return NULL; + + gcc_assert (is_gimple_assign (stmt)); + tree lhs = gimple_assign_lhs (stmt); + tree *dvar = id->param_body_adjs->m_dead_ssa_debug_equiv.get (lhs); + gdebug *bind = gimple_build_debug_bind (*dvar, *dval, stmt); + if (id->reset_location) + gimple_set_location (bind, input_location); + id->debug_stmts.safe_push (bind); + gimple_seq_add_stmt (&stmts, bind); + return stmts; + } /* Begin by recognizing trees that we'll completely rewrite for the inlining context. Our output for these trees is completely @@ -1797,15 +1811,13 @@ remap_gimple_stmt (gimple *stmt, copy_body_data *id) if (gimple_debug_bind_p (stmt)) { - tree value; + tree var = gimple_debug_bind_get_var (stmt); + tree value = gimple_debug_bind_get_value (stmt); if (id->param_body_adjs && id->param_body_adjs->m_dead_stmts.contains (stmt)) - value = NULL_TREE; - else - value = gimple_debug_bind_get_value (stmt); - gdebug *copy - = gimple_build_debug_bind (gimple_debug_bind_get_var (stmt), - value, stmt); + id->param_body_adjs->remap_with_debug_expressions (&value); + + gdebug *copy = gimple_build_debug_bind (var, value, stmt); if (id->reset_location) gimple_set_location (copy, input_location); id->debug_stmts.safe_push (copy); @@ -6431,7 +6443,6 @@ tree_function_versioning (tree old_decl, tree new_decl, in the debug info that var (whole DECL_ORIGIN is the parm PARM_DECL) is optimized away, but could be looked up at the call site as value of D#X there. */ - tree vexpr; gimple_stmt_iterator cgsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))); gimple *def_temp; @@ -6439,17 +6450,25 @@ tree_function_versioning (tree old_decl, tree new_decl, i = vec_safe_length (*debug_args); do { + tree vexpr = NULL_TREE; i -= 2; while (var != NULL_TREE && DECL_ABSTRACT_ORIGIN (var) != (**debug_args)[i]) var = TREE_CHAIN (var); if (var == NULL_TREE) break; - vexpr = make_node (DEBUG_EXPR_DECL); tree parm = (**debug_args)[i]; - DECL_ARTIFICIAL (vexpr) = 1; - TREE_TYPE (vexpr) = TREE_TYPE (parm); - SET_DECL_MODE (vexpr, DECL_MODE (parm)); + if (tree parm_ddef = ssa_default_def (id.src_cfun, parm)) + if (tree *d + = param_body_adjs->m_dead_ssa_debug_equiv.get (parm_ddef)) + vexpr = *d; + if (!vexpr) + { + vexpr = make_node (DEBUG_EXPR_DECL); + DECL_ARTIFICIAL (vexpr) = 1; + TREE_TYPE (vexpr) = TREE_TYPE (parm); + SET_DECL_MODE (vexpr, DECL_MODE (parm)); + } def_temp = gimple_build_debug_bind (var, vexpr, NULL); gsi_insert_before (&cgsi, def_temp, GSI_NEW_STMT); def_temp = gimple_build_debug_source_bind (vexpr, parm, NULL);