Patch ping Re: [PATCH] ipa: Avoid another ICE when dealing with type-incompatibilities (PR 108959)

[Jakub Jelinek <jakub@redhat.com>]

Hi!

Honza, could you please have a look?

This is one of the few remaining P1s.

On Thu, Mar 23, 2023 at 11:09:19AM +0100, Martin Jambor wrote:
> Hi,
> 
> PR 108959 shows one more example where undefined code with type
> incompatible accesses to stuff passed in parameters can cause an ICE
> because we try to create a VIEW_CONVERT_EXPR of mismatching sizes:
> 
> 1. IPA-CP tries to push one type from above,
> 
> 2. IPA-SRA (correctly) decides the parameter is useless because it is
>    only used to construct an argument to another function which does not
>    use it and so the formal parameter should be removed,
> 
> 3. but the code reconciling IPA-CP and IPA-SRA transformations still
>    wants to perform the IPA-CP and overrides the built-in DCE of
>    useless statements and tries to stuff constants into them
>    instead, constants of a type with mismatching type and size.
> 
> This patch avoids the situation in IPA-SRA by purging the IPA-CP
> results from any "aggregate" constants that are passed in parameters
> which are detected to be useless.  It also removes IPA value range and
> bits information associated with removed parameters stored in the same
> structure so that the useless information is not streamed later on.
> 
> Bootstrapped and LTO-bootstrapped and tested on x86_64-linux.  OK for
> trunk?
> 
> gcc/ChangeLog:
> 
> 2023-03-22  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/108959
> 	* ipa-sra.cc (zap_useless_ipcp_results): New function.
> 	(process_isra_node_results): Call it.
> 
> gcc/testsuite/ChangeLog:
> 
> 2023-03-17  Martin Jambor  <mjambor@suse.cz>
> 
> 	PR ipa/108959
> 	* gcc.dg/ipa/pr108959.c: New test.
> ---
>  gcc/ipa-sra.cc                      | 66 +++++++++++++++++++++++++++++
>  gcc/testsuite/gcc.dg/ipa/pr108959.c | 22 ++++++++++
>  2 files changed, 88 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.dg/ipa/pr108959.c
> 
> diff --git a/gcc/ipa-sra.cc b/gcc/ipa-sra.cc
> index 3de7d426b7e..7b8260bc9e1 100644
> --- a/gcc/ipa-sra.cc
> +++ b/gcc/ipa-sra.cc
> @@ -4028,6 +4028,70 @@ mark_callers_calls_comdat_local (struct cgraph_node *node, void *)
>    return false;
>  }
>  
> +/* Remove any IPA-CP results stored in TS that are associated with removed
> +   parameters as marked in IFS. */
> +
> +static void
> +zap_useless_ipcp_results (const isra_func_summary *ifs, ipcp_transformation *ts)
> +{
> +  unsigned ts_len = vec_safe_length (ts->m_agg_values);
> +
> +  if (ts_len == 0)
> +    return;
> +
> +  bool removed_item = false;
> +  unsigned dst_index = 0;
> +
> +  for (unsigned i = 0; i < ts_len; i++)
> +    {
> +      ipa_argagg_value *v = &(*ts->m_agg_values)[i];
> +      const isra_param_desc *desc = &(*ifs->m_parameters)[v->index];
> +
> +      if (!desc->locally_unused)
> +	{
> +	  if (removed_item)
> +	    (*ts->m_agg_values)[dst_index] = *v;
> +	  dst_index++;
> +	}
> +      else
> +	removed_item = true;
> +    }
> +  if (dst_index == 0)
> +    {
> +      ggc_free (ts->m_agg_values);
> +      ts->m_agg_values = NULL;
> +    }
> +  else if (removed_item)
> +    ts->m_agg_values->truncate (dst_index);
> +
> +  bool useful_bits = false;
> +  unsigned count = vec_safe_length (ts->bits);
> +  for (unsigned i = 0; i < count; i++)
> +    if ((*ts->bits)[i])
> +    {
> +      const isra_param_desc *desc = &(*ifs->m_parameters)[i];
> +      if (desc->locally_unused)
> +	(*ts->bits)[i] = NULL;
> +      else
> +	useful_bits = true;
> +    }
> +  if (!useful_bits)
> +    ts->bits = NULL;
> +
> +  bool useful_vr = false;
> +  count = vec_safe_length (ts->m_vr);
> +  for (unsigned i = 0; i < count; i++)
> +    if ((*ts->m_vr)[i].known)
> +      {
> +	const isra_param_desc *desc = &(*ifs->m_parameters)[i];
> +	if (desc->locally_unused)
> +	  (*ts->m_vr)[i].known = false;
> +	else
> +	  useful_vr = true;
> +      }
> +  if (!useful_vr)
> +    ts->m_vr = NULL;
> +}
>  
>  /* Do final processing of results of IPA propagation regarding NODE, clone it
>     if appropriate.  */
> @@ -4080,6 +4144,8 @@ process_isra_node_results (cgraph_node *node,
>      }
>  
>    ipcp_transformation *ipcp_ts = ipcp_get_transformation_summary (node);
> +  if (ipcp_ts)
> +    zap_useless_ipcp_results (ifs, ipcp_ts);
>    vec<ipa_adjusted_param, va_gc> *new_params = NULL;
>    if (ipa_param_adjustments *old_adjustments
>  	 = cinfo ? cinfo->param_adjustments : NULL)
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr108959.c b/gcc/testsuite/gcc.dg/ipa/pr108959.c
> new file mode 100644
> index 00000000000..cd1f88658ef
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr108959.c
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +union U2 {
> +  long f0;
> +  int f1;
> +};
> +int g_16;
> +int g_70[20];
> +static int func_61(int) {
> +  for (;;)
> +    g_70[g_16] = 4;
> +}
> +static int func_43(int *p_44)
> +{
> +  func_61(*p_44);
> +}
> +int main() {
> +  union U2 l_38 = {9};
> +  int *l_49 = (int *) &l_38;
> +  func_43(l_49);
> +}
> -- 
> 2.40.0

	Jakub