[2/4] ipa-sra: Introduce a mini-DCE to tree-inline.c (PR 93385)

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 similar def-use walk when
materializing clones, marking which statements should not be copied
and which SSA_NAMEs will be removed by call redirections and now need
to be replaced with anything valid.  Default-definition SSA_NAMEs of
parameters which are removed and all SSA_NAMEs derived from them (in a
phi node or a simple assignment statement) are then remapped to
error_mark_node - a sure way to spot it if any is left in place.

There is one exception to the above rule, if such SSA_NAMEs appear as
an argument of a call, they need to be removed by call redirection and
not as part of clone materialization.  So to have something valid
there until that time, this patch pulls out dummy declarations out of
thin air.  If you do not like that, see patch number 4 in the series,
which changes this, but probably in a controversial way.

This patch only resets debug statements using the removed SSA_NAMEs.
The first follow-up patch adjusts debug statements in the current
function to still try to make the removed values available in debugger
in the current function and the subsequent one also in other functions
where they are passed.

gcc/ChangeLog:

2020-05-14  Martin Jambor  <mjambor@suse.cz>

	PR ipa/93385
	* ipa-param-manipulation.h (class ipa_param_body_adjustments): New
	members m_dead_stmts, m_dead_ssas, mark_dead_statements and
	get_removed_call_arg_placeholder.
	* ipa-param-manipulation.c (phi_arg_will_live_p): New function.
	(ipa_param_body_adjustments::mark_dead_statements): New method.
	(ipa_param_body_adjustments::common_initialization): Call it.
	(ipa_param_body_adjustments::ipa_param_body_adjustments): Initialize
	new mwmbers.
	(ipa_param_body_adjustments::get_removed_call_arg_placeholder): New.
	(ipa_param_body_adjustments::modify_call_stmt): Replace dead SSAs
	with dummy decls.
	* tree-inline.c (remap_gimple_stmt): Do not copy dead statements,
	reset dead debug statements.
	(copy_phis_for_bb): Do not copy dead PHI nodes.

gcc/testsuite/ChangeLog:

2020-05-14  Martin Jambor  <mjambor@suse.cz>

	PR ipa/93385
	* gcc.dg/ipa/pr93385.c: New test.
	* gcc.dg/ipa/ipa-sra-23.c: Likewise.
---
 gcc/ipa-param-manipulation.c          | 142 ++++++++++++++++++++++++--
 gcc/ipa-param-manipulation.h          |   8 ++
 gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c |  24 +++++
 gcc/testsuite/gcc.dg/ipa/pr93385.c    |  27 +++++
 gcc/tree-inline.c                     |  18 +++-
 5 files changed, 205 insertions(+), 14 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c
 create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93385.c

On Thu, 28 May 2020, Martin Jambor wrote:

> 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 similar def-use walk when
> materializing clones, marking which statements should not be copied
> and which SSA_NAMEs will be removed by call redirections and now need
> to be replaced with anything valid.  Default-definition SSA_NAMEs of
> parameters which are removed and all SSA_NAMEs derived from them (in a
> phi node or a simple assignment statement) are then remapped to
> error_mark_node - a sure way to spot it if any is left in place.
> 
> There is one exception to the above rule, if such SSA_NAMEs appear as
> an argument of a call, they need to be removed by call redirection and
> not as part of clone materialization.  So to have something valid
> there until that time, this patch pulls out dummy declarations out of
> thin air.  If you do not like that, see patch number 4 in the series,
> which changes this, but probably in a controversial way.
> 
> This patch only resets debug statements using the removed SSA_NAMEs.
> The first follow-up patch adjusts debug statements in the current
> function to still try to make the removed values available in debugger
> in the current function and the subsequent one also in other functions
> where they are passed.
> 
> gcc/ChangeLog:
> 
> 2020-05-14  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/93385
> 	* ipa-param-manipulation.h (class ipa_param_body_adjustments): New
> 	members m_dead_stmts, m_dead_ssas, mark_dead_statements and
> 	get_removed_call_arg_placeholder.
> 	* ipa-param-manipulation.c (phi_arg_will_live_p): New function.
> 	(ipa_param_body_adjustments::mark_dead_statements): New method.
> 	(ipa_param_body_adjustments::common_initialization): Call it.
> 	(ipa_param_body_adjustments::ipa_param_body_adjustments): Initialize
> 	new mwmbers.
> 	(ipa_param_body_adjustments::get_removed_call_arg_placeholder): New.
> 	(ipa_param_body_adjustments::modify_call_stmt): Replace dead SSAs
> 	with dummy decls.
> 	* tree-inline.c (remap_gimple_stmt): Do not copy dead statements,
> 	reset dead debug statements.
> 	(copy_phis_for_bb): Do not copy dead PHI nodes.
> 
> gcc/testsuite/ChangeLog:
> 
> 2020-05-14  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/93385
> 	* gcc.dg/ipa/pr93385.c: New test.
> 	* gcc.dg/ipa/ipa-sra-23.c: Likewise.
> ---
>  gcc/ipa-param-manipulation.c          | 142 ++++++++++++++++++++++++--
>  gcc/ipa-param-manipulation.h          |   8 ++
>  gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c |  24 +++++
>  gcc/testsuite/gcc.dg/ipa/pr93385.c    |  27 +++++
>  gcc/tree-inline.c                     |  18 +++-
>  5 files changed, 205 insertions(+), 14 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/ipa-sra-23.c
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr93385.c
> 
> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
> index 978916057f0..1f47f3a4268 100644
> --- a/gcc/ipa-param-manipulation.c
> +++ b/gcc/ipa-param-manipulation.c
> @@ -953,6 +953,99 @@ 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.  */
> +
> +void
> +ipa_param_body_adjustments::mark_dead_statements (tree dead_param)
> +{
> +  if (!is_gimple_reg (dead_param))

Hmm, so non-registers are not a problem?  I guess IPA SRA simply
ensures there are no actual uses (but call arguments) in that case?
Please clearly document this function matches IPA SRA capabilities.

> +    return;
> +  tree parm_ddef = ssa_default_def (m_id->src_cfun, dead_param);
> +  if (!parm_ddef || has_zero_uses (parm_ddef))
> +    return;
> +
> +  auto_vec<tree, 4> 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 <gphi *> (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.contains (res))

You can combine this with the add below which returns false if the
item was not already present.

> +		    {
> +		      stack.safe_push (res);
> +		      m_dead_ssas.add (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.contains (lhs))
> +		    {
> +		      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
> @@ -1095,6 +1188,11 @@ ipa_param_body_adjustments::common_initialization (tree old_fndecl,
>  		/* 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 (!isra_dummy_decls[i])
> +		  mark_dead_statements (m_oparms[i]);
>  	      }
>  	  }
>  	else
> @@ -1151,9 +1249,10 @@ ipa_param_body_adjustments
>  ::ipa_param_body_adjustments (vec<ipa_adjusted_param, va_gc> *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)
>  {
>    common_initialization (fndecl, NULL, NULL);
>  }
> @@ -1167,9 +1266,9 @@ 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_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)
>  {
>    common_initialization (fndecl, NULL, NULL);
> @@ -1190,9 +1289,10 @@ ipa_param_body_adjustments
>  			      copy_body_data *id, tree *vars,
>  			      vec<ipa_replace_map *, va_gc> *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)
>  {
>    common_initialization (old_fndecl, vars, tree_map);
>  }
> @@ -1519,6 +1619,19 @@ remap_split_decl_to_dummy (tree *tp, int *walk_subtrees, void *data)
>    return NULL_TREE;
>  }
>  
> +/* Given ARG which is a SSA_NAME call argument which we are however removing
> +   from the current function and which will be thus removed from the call
> +   statement by ipa_param_adjustments::modify_call, return something that can
> +   be used as a placeholder and which the operand scanner will accept until
> +   then.  */
> +
> +tree
> +ipa_param_body_adjustments::get_removed_call_arg_placeholder (tree arg)
> +{
> +  tree t = create_tmp_var (TREE_TYPE (arg));

If it is temporarily then use _raw.  It looks like you can get called
multiple times for the same arg and each time you get a new temporary
decl?  Ugh.

> +  insert_decl_map (m_id, t, t);

I wonder why you can't use/keep the SSA default def of the removed
parameter instead?  Or a literal zero?  That is, below you don't
seem to be anything special during inlining itself so I presume
those are all removed by call redirection?  Why are the actual
parameters then still needed?  (and error_mark_node doesn't work?)

> +  return t;
> +}
>  
>  /* 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
> @@ -1658,7 +1771,10 @@ ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p)
>  	  else
>  	    {
>  	      tree t = gimple_call_arg (stmt, i);
> -	      modify_expression (&t, true);
> +	      if (TREE_CODE (t) == SSA_NAME && m_dead_ssas.contains(t))

space after function call (likewise elsewhere).

> +		t = get_removed_call_arg_placeholder (t);
> +	      else
> +		modify_expression (&t, true);
>  	      vargs.safe_push (t);
>  	    }
>  	}
> @@ -1680,7 +1796,11 @@ ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p)
>    for (unsigned i = 0; i < nargs; i++)
>      {
>        tree *t = gimple_call_arg_ptr (stmt, i);
> -      modified |= modify_expression (t, true);
> +
> +      if (TREE_CODE (*t) == SSA_NAME && m_dead_ssas.contains(*t))
> +	*t = get_removed_call_arg_placeholder (*t);
> +      else
> +	modified |= modify_expression (t, true);
>      }
>  
>    if (gimple_call_lhs (stmt))
> diff --git a/gcc/ipa-param-manipulation.h b/gcc/ipa-param-manipulation.h
> index 0b038ea57f1..59060ae5dcc 100644
> --- a/gcc/ipa-param-manipulation.h
> +++ b/gcc/ipa-param-manipulation.h
> @@ -370,6 +370,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<gimple *> m_dead_stmts;
> +  hash_set<tree> m_dead_ssas;
> +
>  private:
>    void common_initialization (tree old_fndecl, tree *vars,
>  			      vec<ipa_replace_map *, va_gc> *tree_map);
> @@ -383,6 +389,8 @@ private:
>    bool modify_call_stmt (gcall **stmt_p);
>    bool modify_cfun_body ();
>    void reset_debug_stmts ();
> +  void mark_dead_statements (tree dead_param);
> +  tree get_removed_call_arg_placeholder (tree arg);
>  
>    /* Declaration of the function that is being transformed.  */
>  
> 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/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 3160ca3f88a..60087dd5e7b 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -1524,6 +1524,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
> @@ -1788,10 +1793,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);
> @@ -2671,7 +2681,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)
>

Hi,

On Tue, Jun 02 2020, Richard Biener wrote:
> On Thu, 28 May 2020, Martin Jambor wrote:
>
>> 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 similar def-use walk when
>> materializing clones, marking which statements should not be copied
>> and which SSA_NAMEs will be removed by call redirections and now need
>> to be replaced with anything valid.  Default-definition SSA_NAMEs of
>> parameters which are removed and all SSA_NAMEs derived from them (in a
>> phi node or a simple assignment statement) are then remapped to
>> error_mark_node - a sure way to spot it if any is left in place.
>> 
>> There is one exception to the above rule, if such SSA_NAMEs appear as
>> an argument of a call, they need to be removed by call redirection and
>> not as part of clone materialization.  So to have something valid
>> there until that time, this patch pulls out dummy declarations out of
>> thin air.  If you do not like that, see patch number 4 in the series,
>> which changes this, but probably in a controversial way.
>> 
>> This patch only resets debug statements using the removed SSA_NAMEs.
>> The first follow-up patch adjusts debug statements in the current
>> function to still try to make the removed values available in debugger
>> in the current function and the subsequent one also in other functions
>> where they are passed.
>> 
>> gcc/ChangeLog:
>> 
>> 2020-05-14  Martin Jambor  <mjambor@suse.cz>
>> 
>> 	PR ipa/93385
>> 	* ipa-param-manipulation.h (class ipa_param_body_adjustments): New
>> 	members m_dead_stmts, m_dead_ssas, mark_dead_statements and
>> 	get_removed_call_arg_placeholder.
>> 	* ipa-param-manipulation.c (phi_arg_will_live_p): New function.
>> 	(ipa_param_body_adjustments::mark_dead_statements): New method.
>> 	(ipa_param_body_adjustments::common_initialization): Call it.
>> 	(ipa_param_body_adjustments::ipa_param_body_adjustments): Initialize
>> 	new mwmbers.
>> 	(ipa_param_body_adjustments::get_removed_call_arg_placeholder): New.
>> 	(ipa_param_body_adjustments::modify_call_stmt): Replace dead SSAs
>> 	with dummy decls.
>> 	* tree-inline.c (remap_gimple_stmt): Do not copy dead statements,
>> 	reset dead debug statements.
>> 	(copy_phis_for_bb): Do not copy dead PHI nodes.
>> 
>> gcc/testsuite/ChangeLog:
>> 

[...]

>> 
>> diff --git a/gcc/ipa-param-manipulation.c b/gcc/ipa-param-manipulation.c
>> index 978916057f0..1f47f3a4268 100644
>> --- a/gcc/ipa-param-manipulation.c
>> +++ b/gcc/ipa-param-manipulation.c
>> @@ -953,6 +953,99 @@ 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.  */
>> +
>> +void
>> +ipa_param_body_adjustments::mark_dead_statements (tree dead_param)
>> +{
>> +  if (!is_gimple_reg (dead_param))
>
> Hmm, so non-registers are not a problem?  I guess IPA SRA simply
> ensures there are no actual uses (but call arguments) in that case?
> Please clearly document this function matches IPA SRA capabilities.

OK, added.

>
>> +    return;
>> +  tree parm_ddef = ssa_default_def (m_id->src_cfun, dead_param);
>> +  if (!parm_ddef || has_zero_uses (parm_ddef))
>> +    return;
>> +
>> +  auto_vec<tree, 4> 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 <gphi *> (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.contains (res))
>
> You can combine this with the add below which returns false if the
> item was not already present.

I see, done.

>
>> +		    {
>> +		      stack.safe_push (res);
>> +		      m_dead_ssas.add (res);
>> +		    }
>> +		}
>> +	    }

[...]

>> @@ -1519,6 +1619,19 @@ remap_split_decl_to_dummy (tree *tp, int *walk_subtrees, void *data)
>>    return NULL_TREE;
>>  }
>>  
>> +/* Given ARG which is a SSA_NAME call argument which we are however removing
>> +   from the current function and which will be thus removed from the call
>> +   statement by ipa_param_adjustments::modify_call, return something that can
>> +   be used as a placeholder and which the operand scanner will accept until
>> +   then.  */
>> +
>> +tree
>> +ipa_param_body_adjustments::get_removed_call_arg_placeholder (tree arg)
>> +{
>> +  tree t = create_tmp_var (TREE_TYPE (arg));
>
> If it is temporarily then use _raw.  It looks like you can get called
> multiple times for the same arg and each time you get a new temporary
> decl?  Ugh.
>
>> +  insert_decl_map (m_id, t, t);
>
> I wonder why you can't use/keep the SSA default def of the removed
> parameter instead?  Or a literal zero?  That is, below you don't
> seem to be anything special during inlining itself so I presume
> those are all removed by call redirection?  Why are the actual
> parameters then still needed?  (and error_mark_node doesn't work?)

Please see the last patch in the series, which removes the ugly decl and
puts here - for a defined time period - either error_mark_node (with
-g0) or DEBUG_EXPR_DECL (when generating debug info).  But I expect that
patch to be at least slightly controversial because it basically extends
gimple during IPA passes.

I tried but cannot cannot use literal zero here because that then leaks
into debug information and debugger thinks the parameter really was
zero.

So if the last patch in the series is not acceptable, I can see two
options:

1) We can decide we will simply never attempt to generate debug info for
   such removed parameters and then we can either have one such dummy
   decl per function (type mismatch hopefully would not matter) or
   remove the parameters outright and mark them as already removed in
   some bitmap on associated cgraph_edges.

2) Or we decide we do want to attempt generating debug info about the
   value of the removed parameter but not extend IL during IPA passes
   and then my plan was to create a unique dummy decl here and create a
   mapping between this decl and the DEBUG_EXPR_DECL containing debug
   info - and keep it up to date for the rest of IPA machinery.  But the
   code keeping things up to date when the created clone is itself
   cloned or inlined quickly got so ugly I decided to try and propose
   the IL extension where all of this happens automatically (as the
   testcase in the patch shows).

>
>> +  return t;
>> +}
>>  
>>  /* 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
>> @@ -1658,7 +1771,10 @@ ipa_param_body_adjustments::modify_call_stmt (gcall **stmt_p)
>>  	  else
>>  	    {
>>  	      tree t = gimple_call_arg (stmt, i);
>> -	      modify_expression (&t, true);
>> +	      if (TREE_CODE (t) == SSA_NAME && m_dead_ssas.contains(t))
>
> space after function call (likewise elsewhere).

Oops, fixed.

Thanks,

Martin