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 <mjam...@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 <mjam...@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
