Message ID | ri67cv7vkxc.fsf@suse.cz |
---|---|
State | New |
Headers | show |
Series | ipa: Avoid another ICE when dealing with type-incompatibilities (PR 108959) | expand |
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
> 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. OK, thanks! Honza
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); +}