From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [IPv6:2001:67c:2178:6::1c]) by sourceware.org (Postfix) with ESMTPS id EC4D73857B9B for ; Mon, 18 Sep 2023 19:36:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EC4D73857B9B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=fail 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 1690E220CC; Mon, 18 Sep 2023 19:36:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_rsa; t=1695065763; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=9gU0GMMwsc0K4KrxkHgiGM+3v9HegsYRZeFB7oA5fXM=; b=N7RgadMnyJ3G6hMQpIzreXiZoiYUHtKPN1QbCrCDsKrCwowl2L2ncYzxjTPRbaNjS5i4lc JrdR6iaFsPKOj9h3V8p5SgUY+0vd7REwSnYUXvmAHECK3cGLrxW+csmxeoIHM9uzML8SWU N+pjrzHGYkzdxTQf5v2jAIFLuGUylEU= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.cz; s=susede2_ed25519; t=1695065763; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=9gU0GMMwsc0K4KrxkHgiGM+3v9HegsYRZeFB7oA5fXM=; b=kKdxKpSSiDSX0jOFxU8GH2tKkeOl4WdX6Z15F161ALs0h83Xl/1H4EQFP7fg72JPFDKI/3 IWtLzkTPjyv2cMCg== 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 01D931358A; Mon, 18 Sep 2023 19:36:03 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id AZtsAKOmCGVeTgAAMHmgww (envelope-from ); Mon, 18 Sep 2023 19:36:03 +0000 From: Martin Jambor To: GCC Patches Cc: Jan Hubicka Subject: Re: [PATCH] ipa-sra: Allow IPA-SRA in presence of returns which will be removed User-Agent: Notmuch/0.37 (https://notmuchmail.org) Emacs/29.1 (x86_64-suse-linux-gnu) Date: Mon, 18 Sep 2023 21:36:02 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_SOFTFAIL,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello, and ping. Thanks, Martin On Fri, Sep 01 2023, Martin Jambor wrote: > Hello > > and ping. > > Thanks, > > Martin > > > On Fri, Aug 18 2023, Martin Jambor wrote: >> Hi, >> >> testing on 32bit arm revealed that even the simplest case of PR 110378 >> was still not resolved there because destructors were returning this >> pointer. Needless to say, the return value of those destructors often >> is just not used, which IPA-SRA can already detect in time. Since >> such enhancement seems generally useful, here it is. >> >> The patch simply adds two flag to respective summaries to mark down >> situations when it encounters either a simple direct use of a default >> definition SSA_NAME of a parameter, which means that the parameter may >> still be split when return value is removed, and when any derived use >> of it is returned, allowing for complete removal in that case, instead >> of discarding it as a candidate for removal or splitting like we do >> now. The IPA phase then simply checks that we indeed plan to remove >> the return value before allowing any transformation to be considered >> in such cases. >> >> Bootstrapped, LTO-bootstrapped and tested on x86_64-linux. OK for >> master? >> >> Thanks, >> >> Martin >> >> >> gcc/ChangeLog: >> >> 2023-08-18 Martin Jambor >> >> PR ipa/110378 >> * ipa-param-manipulation.cc >> (ipa_param_body_adjustments::mark_dead_statements): Verify that any >> return uses of PARAM will be removed. >> (ipa_param_body_adjustments::mark_clobbers_dead): Likewise. >> * ipa-sra.cc (isra_param_desc): New fields >> remove_only_when_retval_removed and split_only_when_retval_removed. >> (struct gensum_param_desc): Likewise. Fix comment long line. >> (ipa_sra_function_summaries::duplicate): Copy the new flags. >> (dump_gensum_param_descriptor): Dump the new flags. >> (dump_isra_param_descriptor): Likewise. >> (isra_track_scalar_value_uses): New parameter desc. Set its flag >> remove_only_when_retval_removed when encountering a simple return. >> (isra_track_scalar_param_local_uses): Replace parameter call_uses_p >> with desc. Pass it to isra_track_scalar_value_uses and set its >> call_uses. >> (ptr_parm_has_nonarg_uses): Accept parameter descriptor as a >> parameter. If there is a direct return use, mark any.. >> (create_parameter_descriptors): Pass the whole parameter descriptor to >> isra_track_scalar_param_local_uses and ptr_parm_has_nonarg_uses. >> (process_scan_results): Copy the new flags. >> (isra_write_node_summary): Stream the new flags. >> (isra_read_node_info): Likewise. >> (adjust_parameter_descriptions): Check that transformations >> requring return removal only happen when return value is removed. >> Restructure main loop. Adjust dump message. >> >> gcc/testsuite/ChangeLog: >> >> 2023-08-18 Martin Jambor >> >> PR ipa/110378 >> * gcc.dg/ipa/ipa-sra-32.c: New test. >> * gcc.dg/ipa/pr110378-4.c: Likewise. >> * gcc.dg/ipa/ipa-sra-4.c: Use a return value. >> --- >> gcc/ipa-param-manipulation.cc | 7 +- >> gcc/ipa-sra.cc | 247 +++++++++++++++++--------- >> gcc/testsuite/gcc.dg/ipa/ipa-sra-32.c | 30 ++++ >> gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c | 4 +- >> gcc/testsuite/gcc.dg/ipa/pr110378-4.c | 50 ++++++ >> 5 files changed, 251 insertions(+), 87 deletions(-) >> create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-sra-32.c >> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr110378-4.c >> >> diff --git a/gcc/ipa-param-manipulation.cc b/gcc/ipa-param-manipulation.cc >> index 4a185ddbdf4..ae52f17b2c9 100644 >> --- a/gcc/ipa-param-manipulation.cc >> +++ b/gcc/ipa-param-manipulation.cc >> @@ -1163,6 +1163,8 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param, >> stack.safe_push (lhs); >> } >> } >> + else if (gimple_code (stmt) == GIMPLE_RETURN) >> + gcc_assert (m_adjustments && m_adjustments->m_skip_return); >> else >> /* IPA-SRA does not analyze other types of statements. */ >> gcc_unreachable (); >> @@ -1182,7 +1184,8 @@ ipa_param_body_adjustments::mark_dead_statements (tree dead_param, >> } >> >> /* Put all clobbers of of dereference of default definition of PARAM into >> - m_dead_stmts. */ >> + m_dead_stmts. If there are returns among uses of the default definition of >> + PARAM, verify they will be stripped off the return value. */ >> >> void >> ipa_param_body_adjustments::mark_clobbers_dead (tree param) >> @@ -1200,6 +1203,8 @@ ipa_param_body_adjustments::mark_clobbers_dead (tree param) >> gimple *stmt = USE_STMT (use_p); >> if (gimple_clobber_p (stmt)) >> m_dead_stmts.add (stmt); >> + else if (gimple_code (stmt) == GIMPLE_RETURN) >> + gcc_assert (m_adjustments && m_adjustments->m_skip_return); >> } >> } >> >> diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc >> index edba364f56e..817f29ea62f 100644 >> --- a/gcc/ipa-sra.cc >> +++ b/gcc/ipa-sra.cc >> @@ -185,6 +185,13 @@ struct GTY(()) isra_param_desc >> unsigned split_candidate : 1; >> /* Is this a parameter passing stuff by reference? */ >> unsigned by_ref : 1; >> + /* If set, this parameter can only be a candidate for removal if the function >> + is going to loose its return value. */ >> + unsigned remove_only_when_retval_removed : 1; >> + /* If set, this parameter can only be a candidate for splitting if the >> + function is going to loose its return value. Can only be meaningfully set >> + for by_ref parameters. */ >> + unsigned split_only_when_retval_removed : 1; >> /* Parameter hint set during IPA analysis when there is a caller which does >> not construct the argument just to pass it to calls. Only meaningful for >> by_ref parameters. */ >> @@ -206,7 +213,8 @@ struct gensum_param_desc >> /* Number of accesses in the access tree rooted in field accesses. */ >> unsigned access_count; >> >> - /* If the below is non-zero, this is the number of uses as actual arguments. */ >> + /* If the below is non-zero, this is the number of uses as actual >> + arguments. */ >> int call_uses; >> /* Number of times this parameter has been directly passed to. */ >> unsigned ptr_pt_count; >> @@ -230,6 +238,13 @@ struct gensum_param_desc >> without performing further checks (for example because it is a >> REFERENCE_TYPE)? */ >> bool safe_ref; >> + /* If set, this parameter can only be a candidate for removal if the function >> + is going to loose its return value. */ >> + bool remove_only_when_retval_removed; >> + /* If set, this parameter can only be a candidate for splitting if the >> + function is going to loose its return value. Can only be meaningfully set >> + for by_ref parameters. */ >> + bool split_only_when_retval_removed; >> /* Only meaningful for by_ref parameters. If set, this parameter can only be >> a split candidate if all callers pass pointers that are known to point to >> a chunk of memory large enough to contain all accesses. */ >> @@ -445,6 +460,8 @@ ipa_sra_function_summaries::duplicate (cgraph_node *, cgraph_node *, >> d->locally_unused = s->locally_unused; >> d->split_candidate = s->split_candidate; >> d->by_ref = s->by_ref; >> + d->remove_only_when_retval_removed = s->remove_only_when_retval_removed; >> + d->split_only_when_retval_removed = s->split_only_when_retval_removed; >> d->not_specially_constructed = s->not_specially_constructed; >> d->conditionally_dereferenceable = s->conditionally_dereferenceable; >> d->safe_size_set = s->safe_size_set; >> @@ -732,17 +749,21 @@ static void >> dump_gensum_param_descriptor (FILE *f, gensum_param_desc *desc) >> { >> if (desc->locally_unused) >> - fprintf (f, " unused with %i call_uses\n", desc->call_uses); >> + fprintf (f, " unused with %i call_uses%s\n", desc->call_uses, >> + desc->remove_only_when_retval_removed ? >> + " remove_only_when_retval_removed" : ""); >> if (!desc->split_candidate) >> { >> fprintf (f, " not a candidate\n"); >> return; >> } >> if (desc->by_ref) >> - fprintf (f, " %s%s by_ref with %u pass throughs\n", >> + fprintf (f, " %s%s%s by_ref with %u pass throughs\n", >> desc->safe_ref ? "safe" : "unsafe", >> desc->conditionally_dereferenceable >> - ? " conditionally_dereferenceable" : " ok", >> + ? " conditionally_dereferenceable" : "", >> + desc->split_only_when_retval_removed >> + ? " split_only_when_retval_removed" : "", >> desc->ptr_pt_count); >> >> for (gensum_param_access *acc = desc->accesses; acc; acc = acc->next_sibling) >> @@ -790,6 +811,10 @@ dump_isra_param_descriptor (FILE *f, isra_param_desc *desc, bool hints) >> fprintf (f, " param_size_limit: %u, size_reached: %u%s", >> desc->param_size_limit, desc->size_reached, >> desc->by_ref ? ", by_ref" : ""); >> + if (desc->remove_only_when_retval_removed) >> + fprintf (f, ", remove_only_when_retval_removed"); >> + if (desc->split_only_when_retval_removed) >> + fprintf (f, ", split_only_when_retval_removed"); >> if (desc->by_ref && desc->conditionally_dereferenceable) >> fprintf (f, ", conditionally_dereferenceable"); >> if (hints) >> @@ -881,16 +906,18 @@ get_single_param_flow_source (const isra_param_flow *param_flow) >> >> /* Inspect all uses of NAME and simple arithmetic calculations involving NAME >> in FUN represented with NODE and return a negative number if any of them is >> - used for something else than either an actual call argument, simple >> - arithmetic operation or debug statement. If there are no such uses, return >> - the number of actual arguments that this parameter eventually feeds to (or >> - zero if there is none). For any such parameter, mark PARM_NUM as one of its >> - sources. ANALYZED is a bitmap that tracks which SSA names we have already >> - started investigating. */ >> + used for something else than either an actual call argument, simple return, >> + simple arithmetic operation or debug statement. If there are no such uses, >> + return the number of actual arguments that this parameter eventually feeds >> + to (or zero if there is none). If there are any simple return uses, set >> + DESC->remove_only_when_retval_removed. For any such parameter, mark >> + PARM_NUM as one of its sources. ANALYZED is a bitmap that tracks which SSA >> + names we have already started investigating. */ >> >> static int >> isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, >> - int parm_num, bitmap analyzed) >> + int parm_num, bitmap analyzed, >> + gensum_param_desc *desc) >> { >> int res = 0; >> imm_use_iterator imm_iter; >> @@ -964,7 +991,7 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, >> if (bitmap_set_bit (analyzed, SSA_NAME_VERSION (lhs))) >> { >> int tmp = isra_track_scalar_value_uses (fun, node, lhs, parm_num, >> - analyzed); >> + analyzed, desc); >> if (tmp < 0) >> { >> res = tmp; >> @@ -973,6 +1000,16 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, >> res += tmp; >> } >> } >> + else if (greturn *gr = dyn_cast(stmt)) >> + { >> + tree rv = gimple_return_retval (gr); >> + if (rv != name) >> + { >> + res = -1; >> + break; >> + } >> + desc->remove_only_when_retval_removed = true; >> + } >> else >> { >> res = -1; >> @@ -985,11 +1022,12 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, >> /* Inspect all uses of PARM, which must be a gimple register, in FUN (which is >> also described by NODE) and simple arithmetic calculations involving PARM >> and return false if any of them is used for something else than either an >> - actual call argument, simple arithmetic operation or debug statement. If >> - there are no such uses, return true and store the number of actual arguments >> - that this parameter eventually feeds to (or zero if there is none) to >> - *CALL_USES_P. For any such parameter, mark PARM_NUM as one of its >> - sources. >> + actual call argument, simple return, simple arithmetic operation or debug >> + statement. If there are no such uses, return true and store the number of >> + actual arguments that this parameter eventually feeds to (or zero if there >> + is none) to DESC->call_uses and set DESC->remove_only_when_retval_removed if >> + there are any uses in return statemens. For any such parameter, mark >> + PARM_NUM as one of its sources. >> >> This function is similar to ptr_parm_has_nonarg_uses but its results are >> meant for unused parameter removal, as opposed to splitting of parameters >> @@ -997,14 +1035,14 @@ isra_track_scalar_value_uses (function *fun, cgraph_node *node, tree name, >> >> static bool >> isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm, >> - int parm_num, int *call_uses_p) >> + int parm_num, gensum_param_desc *desc) >> { >> gcc_checking_assert (is_gimple_reg (parm)); >> >> tree name = ssa_default_def (fun, parm); >> if (!name || has_zero_uses (name)) >> { >> - *call_uses_p = 0; >> + desc->call_uses = 0; >> return false; >> } >> >> @@ -1014,11 +1052,11 @@ isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm, >> >> bitmap analyzed = BITMAP_ALLOC (NULL); >> int call_uses = isra_track_scalar_value_uses (fun, node, name, parm_num, >> - analyzed); >> + analyzed, desc); >> BITMAP_FREE (analyzed); >> if (call_uses < 0) >> return true; >> - *call_uses_p = call_uses; >> + desc->call_uses = call_uses; >> return false; >> } >> >> @@ -1026,9 +1064,11 @@ isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm, >> examine whether there are any nonarg uses that are not actual arguments or >> otherwise infeasible uses. If so, return true, otherwise return false. >> Create pass-through IPA flow records for any direct uses as argument calls >> - and if returning false, store their number into *PT_COUNT_P. NODE and FUN >> - must represent the function that is currently analyzed, PARM_NUM must be the >> - index of the analyzed parameter. >> + and if returning false, store their number into DESC->ptr_pt_count. If >> + removal of return value would still allow splitting, return true but set >> + DESC->split_only_when_retval_removed. NODE and FUN must represent the >> + function that is currently analyzed, PARM_NUM must be the index of the >> + analyzed parameter. >> >> This function is similar to isra_track_scalar_param_local_uses but its >> results are meant for splitting of parameters passed by reference or turning >> @@ -1037,7 +1077,7 @@ isra_track_scalar_param_local_uses (function *fun, cgraph_node *node, tree parm, >> >> static bool >> ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, >> - int parm_num, unsigned *pt_count_p) >> + int parm_num, gensum_param_desc *desc) >> { >> imm_use_iterator ui; >> gimple *stmt; >> @@ -1121,6 +1161,19 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, >> } >> } >> } >> + else if (greturn *gr = dyn_cast(stmt)) >> + { >> + tree rv = gimple_return_retval (gr); >> + if (rv == name) >> + { >> + uses_ok++; >> + /* Analysis for feasibility of removal must have already reached >> + the conclusion that the flag must be set if it completed. */ >> + gcc_assert (!desc->locally_unused >> + || desc->remove_only_when_retval_removed); >> + desc->split_only_when_retval_removed = true; >> + } >> + } >> >> /* If the number of valid uses does not match the number of >> uses in this stmt there is an unhandled use. */ >> @@ -1136,7 +1189,7 @@ ptr_parm_has_nonarg_uses (cgraph_node *node, function *fun, tree parm, >> } >> } >> >> - *pt_count_p = pt_count; >> + desc->ptr_pt_count = pt_count; >> return ret; >> } >> >> @@ -1166,7 +1219,6 @@ create_parameter_descriptors (cgraph_node *node, >> if (dump_file && (dump_flags & TDF_DETAILS)) >> print_generic_expr (dump_file, parm, TDF_UID); >> >> - int scalar_call_uses; >> tree type = TREE_TYPE (parm); >> if (TREE_THIS_VOLATILE (parm)) >> { >> @@ -1194,15 +1246,15 @@ create_parameter_descriptors (cgraph_node *node, >> } >> >> if (is_gimple_reg (parm) >> - && !isra_track_scalar_param_local_uses (fun, node, parm, num, >> - &scalar_call_uses)) >> + && !isra_track_scalar_param_local_uses (fun, node, parm, num, desc)) >> { >> - if (dump_file && (dump_flags & TDF_DETAILS)) >> - fprintf (dump_file, " is a scalar with only %i call uses\n", >> - scalar_call_uses); >> - >> desc->locally_unused = true; >> - desc->call_uses = scalar_call_uses; >> + >> + if (dump_file && (dump_flags & TDF_DETAILS)) >> + fprintf (dump_file, " is a scalar with only %i call uses%s\n", >> + desc->call_uses, >> + desc->remove_only_when_retval_removed >> + ? " and return uses": ""); >> } >> >> if (POINTER_TYPE_P (type)) >> @@ -1253,8 +1305,7 @@ create_parameter_descriptors (cgraph_node *node, >> "a va list\n"); >> continue; >> } >> - if (ptr_parm_has_nonarg_uses (node, fun, parm, num, >> - &desc->ptr_pt_count)) >> + if (ptr_parm_has_nonarg_uses (node, fun, parm, num, desc)) >> { >> if (dump_file && (dump_flags & TDF_DETAILS)) >> fprintf (dump_file, " not a candidate, reference has " >> @@ -2628,6 +2679,8 @@ process_scan_results (cgraph_node *node, struct function *fun, >> d->locally_unused = s->locally_unused; >> d->split_candidate = s->split_candidate; >> d->by_ref = s->by_ref; >> + d->remove_only_when_retval_removed = s->remove_only_when_retval_removed; >> + d->split_only_when_retval_removed = s->split_only_when_retval_removed; >> d->conditionally_dereferenceable = s->conditionally_dereferenceable; >> >> for (gensum_param_access *acc = s->accesses; >> @@ -2789,6 +2842,8 @@ isra_write_node_summary (output_block *ob, cgraph_node *node) >> bp_pack_value (&bp, desc->split_candidate, 1); >> bp_pack_value (&bp, desc->by_ref, 1); >> gcc_assert (!desc->not_specially_constructed); >> + bp_pack_value (&bp, desc->remove_only_when_retval_removed, 1); >> + bp_pack_value (&bp, desc->split_only_when_retval_removed, 1); >> bp_pack_value (&bp, desc->conditionally_dereferenceable, 1); >> gcc_assert (!desc->safe_size_set); >> streamer_write_bitpack (&bp); >> @@ -2913,6 +2968,8 @@ isra_read_node_info (struct lto_input_block *ib, cgraph_node *node, >> desc->split_candidate = bp_unpack_value (&bp, 1); >> desc->by_ref = bp_unpack_value (&bp, 1); >> desc->not_specially_constructed = 0; >> + desc->remove_only_when_retval_removed = bp_unpack_value (&bp, 1); >> + desc->split_only_when_retval_removed = bp_unpack_value (&bp, 1); >> desc->conditionally_dereferenceable = bp_unpack_value (&bp, 1); >> desc->safe_size_set = 0; >> } >> @@ -4256,8 +4313,32 @@ adjust_parameter_descriptions (cgraph_node *node, isra_func_summary *ifs) >> { >> desc->locally_unused = false; >> desc->split_candidate = false; >> + continue; >> + } >> + >> + if (desc->split_only_when_retval_removed >> + && !ifs->m_return_ignored) >> + { >> + if (dump_file && (dump_flags & TDF_DETAILS) >> + && (desc->locally_unused || desc->split_candidate)) >> + dump_bad_cond_indices.safe_push (i); >> + >> + gcc_checking_assert (!desc->locally_unused >> + || desc->remove_only_when_retval_removed); >> + desc->locally_unused = false; >> + desc->split_candidate = false; >> + continue; >> + } >> + if (desc->remove_only_when_retval_removed >> + && !ifs->m_return_ignored) >> + { >> + if (dump_file && (dump_flags & TDF_DETAILS) >> + && (desc->locally_unused || desc->split_candidate)) >> + dump_bad_cond_indices.safe_push (i); >> + >> + desc->locally_unused = false; >> } >> - else if (check_surviving >> + if (check_surviving >> && (i >= surviving_params.length () >> || !surviving_params[i])) >> { >> @@ -4269,67 +4350,65 @@ adjust_parameter_descriptions (cgraph_node *node, isra_func_summary *ifs) >> if (dump_file && (dump_flags & TDF_DETAILS)) >> dump_dead_indices.safe_push (i); >> } >> - else >> + >> + if (desc->split_candidate && desc->conditionally_dereferenceable) >> { >> - if (desc->split_candidate && desc->conditionally_dereferenceable) >> - { >> - gcc_assert (desc->safe_size_set); >> - for (param_access *pa : *desc->accesses) >> - if ((pa->unit_offset + pa->unit_size) > desc->safe_size) >> - { >> - if (dump_file && (dump_flags & TDF_DETAILS)) >> - dump_bad_cond_indices.safe_push (i); >> - desc->split_candidate = false; >> - break; >> - } >> - } >> + gcc_assert (desc->safe_size_set); >> + for (param_access *pa : *desc->accesses) >> + if ((pa->unit_offset + pa->unit_size) > desc->safe_size) >> + { >> + if (dump_file && (dump_flags & TDF_DETAILS)) >> + dump_bad_cond_indices.safe_push (i); >> + desc->split_candidate = false; >> + break; >> + } >> + } >> >> - if (desc->split_candidate) >> + if (desc->split_candidate) >> + { >> + if (desc->by_ref && !desc->not_specially_constructed) >> { >> - if (desc->by_ref && !desc->not_specially_constructed) >> - { >> - int extra_factor >> - = opt_for_fn (node->decl, >> - param_ipa_sra_ptrwrap_growth_factor); >> - desc->param_size_limit = extra_factor * desc->param_size_limit; >> - } >> - if (size_would_violate_limit_p (desc, desc->size_reached)) >> - desc->split_candidate = false; >> + int extra_factor >> + = opt_for_fn (node->decl, >> + param_ipa_sra_ptrwrap_growth_factor); >> + desc->param_size_limit = extra_factor * desc->param_size_limit; >> } >> + if (size_would_violate_limit_p (desc, desc->size_reached)) >> + desc->split_candidate = false; >> + } >> >> - /* Avoid ICEs on size-mismatched VIEW_CONVERT_EXPRs when callers and >> - callees don't agree on types in aggregates and we try to do both >> - IPA-CP and IPA-SRA. */ >> - if (ipcp_ts && desc->split_candidate) >> + /* Avoid ICEs on size-mismatched VIEW_CONVERT_EXPRs when callers and >> + callees don't agree on types in aggregates and we try to do both >> + IPA-CP and IPA-SRA. */ >> + if (ipcp_ts && desc->split_candidate) >> + { >> + ipa_argagg_value_list avl (ipcp_ts); >> + for (const param_access *pa : desc->accesses) >> { >> - ipa_argagg_value_list avl (ipcp_ts); >> - for (const param_access *pa : desc->accesses) >> + if (!pa->certain) >> + continue; >> + tree value = avl.get_value (i, pa->unit_offset); >> + if (value >> + && ((tree_to_uhwi (TYPE_SIZE (TREE_TYPE (value))) >> + / BITS_PER_UNIT) >> + != pa->unit_size)) >> { >> - if (!pa->certain) >> - continue; >> - tree value = avl.get_value (i, pa->unit_offset); >> - if (value >> - && ((tree_to_uhwi (TYPE_SIZE (TREE_TYPE (value))) >> - / BITS_PER_UNIT) >> - != pa->unit_size)) >> - { >> - desc->split_candidate = false; >> - if (dump_file && (dump_flags & TDF_DETAILS)) >> - dump_dead_indices.safe_push (i); >> - break; >> - } >> + desc->split_candidate = false; >> + if (dump_file && (dump_flags & TDF_DETAILS)) >> + dump_dead_indices.safe_push (i); >> + break; >> } >> } >> - >> - if (desc->locally_unused || desc->split_candidate) >> - ret = false; >> } >> + >> + if (desc->locally_unused || desc->split_candidate) >> + ret = false; >> } >> >> dump_list_of_param_indices (node, "are dead on arrival or have a type " >> "mismatch with IPA-CP", dump_dead_indices); >> - dump_list_of_param_indices (node, "are not safe to dereference in all " >> - "callers", dump_bad_cond_indices); >> + dump_list_of_param_indices (node, "fail additional requirements ", >> + dump_bad_cond_indices); >> >> return ret; >> } >> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-32.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-32.c >> new file mode 100644 >> index 00000000000..f84442816b6 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-32.c >> @@ -0,0 +1,30 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -fdump-ipa-sra" } */ >> + >> +/* Test that parameters can be removed even when they are returned but the >> + return is unused. */ >> + >> +extern int use(int use); >> + >> + >> +static int __attribute__((noinline)) >> +foo(int a, int b, int c) >> +{ >> + use (c); >> + return a + b + c; >> +} >> + >> +static int __attribute__((noinline)) >> +bar (int a, int b, int c, int d) >> +{ >> + return foo (a, b, c + d); >> +} >> + >> +int >> +baz (int a, int b, int c, int d) >> +{ >> + bar (a, b, c, d); >> + return a + d; >> +} >> + >> +/* { dg-final { scan-ipa-dump-times "Will remove parameter" 4 "sra" } } */ >> diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c b/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c >> index c86ae8320a7..5b42fbd8329 100644 >> --- a/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c >> +++ b/gcc/testsuite/gcc.dg/ipa/ipa-sra-4.c >> @@ -54,10 +54,10 @@ void caller (void) >> int b = 10; >> int c; >> >> - ox (&a); >> + c = ox (&a); >> ox_ctrl_1 (&a); >> ox_ctrl_2 (&a); >> - *holder = ox_improved (1, &b); >> + *holder = ox_improved (1, &b) + c; >> return; >> } >> >> diff --git a/gcc/testsuite/gcc.dg/ipa/pr110378-4.c b/gcc/testsuite/gcc.dg/ipa/pr110378-4.c >> new file mode 100644 >> index 00000000000..32432a8dbe7 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.dg/ipa/pr110378-4.c >> @@ -0,0 +1,50 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-O2 -fdump-ipa-sra -fdump-tree-optimized-slim" } */ >> + >> +/* This emulates what C++ trstcase pr110378-1.C looks like on 32-bit arm (or >> + any architecture where the destructor returns this pointer. It verifies >> + that when it later becomes known that the return value will be removed, we >> + can split a parameter even in this case. */ >> + >> +struct S >> +{ >> + short move_offset_of_a; >> + int *a; >> +}; >> + >> +extern int *allocate_stuff (unsigned); >> +extern void deallocate_stuff (void *); >> + >> +static void >> +something_like_a_constructor (struct S *p, int len) >> +{ >> + p->a = allocate_stuff (len * sizeof (int)); >> + *p->a = 4; >> +} >> + >> +static int >> +operation (struct S *p) >> +{ >> + return *p->a + 1; >> +} >> + >> +static struct S * __attribute__((noinline)) >> +something_like_an_arm32_destructor (struct S *p) >> +{ >> + deallocate_stuff (p->a); >> + return p; >> +} >> + >> +volatile int v2 = 20; >> + >> +int test (void) >> +{ >> + struct S shouldnotexist; >> + something_like_a_constructor (&shouldnotexist, v2); >> + v2 = operation (&shouldnotexist); >> + something_like_an_arm32_destructor (&shouldnotexist); >> + return 0; >> +} >> + >> +/* { dg-final { scan-ipa-dump "Will split parameter 0" "sra" } } */ >> +/* { dg-final { scan-tree-dump-not "shouldnotexist" "optimized" } } */ >> -- >> 2.41.0