diff mbox series

ipa-sra: Allow IPA-SRA in presence of returns which will be removed

Message ID ri6edk05b4a.fsf@suse.cz
State New
Headers show
Series ipa-sra: Allow IPA-SRA in presence of returns which will be removed | expand

Commit Message

Martin Jambor Aug. 18, 2023, 8:49 p.m. UTC
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  <mjambor@suse.cz>

	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  <mjambor@suse.cz>

	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

Comments

Martin Jambor Sept. 1, 2023, 1:30 p.m. UTC | #1
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  <mjambor@suse.cz>
>
> 	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  <mjambor@suse.cz>
>
> 	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<greturn *>(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<greturn *>(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
diff mbox series

Patch

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<greturn *>(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<greturn *>(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" } } */