https://gcc.gnu.org/g:1162861439fd3c4b30fc3ccd49462e47e876f04a
commit r14-9840-g1162861439fd3c4b30fc3ccd49462e47e876f04a Author: Martin Jambor <mjam...@suse.cz> Date: Mon Apr 8 18:53:23 2024 +0200 ipa: Compare jump functions in ICF (PR 113907) In PR 113907 comment #58, Honza found a case where ICF thinks bodies of functions are equivalent but becaise of difference in aliases in a memory access, different aggregate jump functions are associated with supposedly equivalent call statements. This patch adds a way to compare jump functions and plugs it into ICF to avoid the issue. gcc/ChangeLog: 2024-03-20 Martin Jambor <mjam...@suse.cz> PR ipa/113907 * ipa-prop.h (class ipa_vr): Declare new overload of a member function equal_p. (ipa_jump_functions_equivalent_p): Declare. * ipa-prop.cc (ipa_vr::equal_p): New function. (ipa_agg_pass_through_jf_equivalent_p): Likewise. (ipa_agg_jump_functions_equivalent_p): Likewise. (ipa_jump_functions_equivalent_p): Likewise. * ipa-cp.h (values_equal_for_ipcp_p): Declare. * ipa-cp.cc (values_equal_for_ipcp_p): Make function public. * ipa-icf-gimple.cc: Include alloc-pool.h, symbol-summary.h, sreal.h, ipa-cp.h and ipa-prop.h. (func_checker::compare_gimple_call): Comapre jump functions. gcc/testsuite/ChangeLog: 2024-03-20 Martin Jambor <mjam...@suse.cz> PR ipa/113907 * gcc.dg/lto/pr113907_0.c: New. * gcc.dg/lto/pr113907_1.c: Likewise. * gcc.dg/lto/pr113907_2.c: Likewise. Diff: --- gcc/ipa-cp.cc | 2 +- gcc/ipa-cp.h | 2 + gcc/ipa-icf-gimple.cc | 30 ++++++ gcc/ipa-prop.cc | 167 ++++++++++++++++++++++++++++++++++ gcc/ipa-prop.h | 3 + gcc/testsuite/gcc.dg/lto/pr113907_0.c | 18 ++++ gcc/testsuite/gcc.dg/lto/pr113907_1.c | 35 +++++++ gcc/testsuite/gcc.dg/lto/pr113907_2.c | 11 +++ 8 files changed, 267 insertions(+), 1 deletion(-) diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index 2a1da631e9c..b7add455bd5 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -201,7 +201,7 @@ ipcp_lattice<valtype>::is_single_const () /* Return true iff X and Y should be considered equal values by IPA-CP. */ -static bool +bool values_equal_for_ipcp_p (tree x, tree y) { gcc_checking_assert (x != NULL_TREE && y != NULL_TREE); diff --git a/gcc/ipa-cp.h b/gcc/ipa-cp.h index 0b3cfe4b526..7ff74fb5c98 100644 --- a/gcc/ipa-cp.h +++ b/gcc/ipa-cp.h @@ -289,4 +289,6 @@ public: bool virt_call = false; }; +bool values_equal_for_ipcp_p (tree x, tree y); + #endif /* IPA_CP_H */ diff --git a/gcc/ipa-icf-gimple.cc b/gcc/ipa-icf-gimple.cc index 8c2df7a354e..17f62bec068 100644 --- a/gcc/ipa-icf-gimple.cc +++ b/gcc/ipa-icf-gimple.cc @@ -41,7 +41,12 @@ along with GCC; see the file COPYING3. If not see #include "gimple-walk.h" #include "tree-ssa-alias-compare.h" +#include "alloc-pool.h" +#include "symbol-summary.h" #include "ipa-icf-gimple.h" +#include "sreal.h" +#include "ipa-cp.h" +#include "ipa-prop.h" namespace ipa_icf_gimple { @@ -714,6 +719,31 @@ func_checker::compare_gimple_call (gcall *s1, gcall *s2) && !compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2))) return return_false_with_msg ("GIMPLE internal call LHS type mismatch"); + if (!gimple_call_internal_p (s1)) + { + cgraph_edge *e1 = cgraph_node::get (m_source_func_decl)->get_edge (s1); + cgraph_edge *e2 = cgraph_node::get (m_target_func_decl)->get_edge (s2); + class ipa_edge_args *args1 = ipa_edge_args_sum->get (e1); + class ipa_edge_args *args2 = ipa_edge_args_sum->get (e2); + if ((args1 != nullptr) != (args2 != nullptr)) + return return_false_with_msg ("ipa_edge_args mismatch"); + if (args1) + { + int n1 = ipa_get_cs_argument_count (args1); + int n2 = ipa_get_cs_argument_count (args2); + if (n1 != n2) + return return_false_with_msg ("ipa_edge_args nargs mismatch"); + for (int i = 0; i < n1; i++) + { + struct ipa_jump_func *jf1 = ipa_get_ith_jump_func (args1, i); + struct ipa_jump_func *jf2 = ipa_get_ith_jump_func (args2, i); + if (((jf1 != nullptr) != (jf2 != nullptr)) + || (jf1 && !ipa_jump_functions_equivalent_p (jf1, jf2))) + return return_false_with_msg ("jump function mismatch"); + } + } + } + return compare_operand (t1, t2, get_operand_access_type (&map, t1)); } diff --git a/gcc/ipa-prop.cc b/gcc/ipa-prop.cc index e8e4918d5a8..374e998aa64 100644 --- a/gcc/ipa-prop.cc +++ b/gcc/ipa-prop.cc @@ -156,6 +156,20 @@ ipa_vr::equal_p (const vrange &r) const return (types_compatible_p (m_type, r.type ()) && m_storage->equal_p (r)); } +bool +ipa_vr::equal_p (const ipa_vr &o) const +{ + if (!known_p ()) + return !o.known_p (); + + if (!types_compatible_p (m_type, o.m_type)) + return false; + + Value_Range r; + o.get_vrange (r); + return m_storage->equal_p (r); +} + void ipa_vr::get_vrange (Value_Range &r) const { @@ -6057,4 +6071,157 @@ ipa_prop_cc_finalize (void) ipa_node_params_sum = NULL; } +/* Return true if the two pass_through components of two jump functions are + known to be equivalent. AGG_JF denotes whether they are part of aggregate + functions or not. The function can be used before the IPA phase of IPA-CP + or inlining because it cannot cope with refdesc changes these passes can + carry out. */ + +static bool +ipa_agg_pass_through_jf_equivalent_p (ipa_pass_through_data *ipt1, + ipa_pass_through_data *ipt2, + bool agg_jf) + +{ + gcc_assert (agg_jf || + (!ipt1->refdesc_decremented && !ipt2->refdesc_decremented)); + if (ipt1->operation != ipt2->operation + || ipt1->formal_id != ipt2->formal_id + || (!agg_jf && (ipt1->agg_preserved != ipt2->agg_preserved))) + return false; + if (((ipt1->operand != NULL_TREE) != (ipt2->operand != NULL_TREE)) + || (ipt1->operand + && !values_equal_for_ipcp_p (ipt1->operand, ipt2->operand))) + return false; + return true; +} + +/* Return true if the two aggregate jump functions are known to be equivalent. + The function can be used before the IPA phase of IPA-CP or inlining because + it cannot cope with refdesc changes these passes can carry out. */ + +static bool +ipa_agg_jump_functions_equivalent_p (ipa_agg_jf_item *ajf1, + ipa_agg_jf_item *ajf2) +{ + if (ajf1->offset != ajf2->offset + || ajf1->jftype != ajf2->jftype + || !types_compatible_p (ajf1->type, ajf2->type)) + return false; + + switch (ajf1->jftype) + { + case IPA_JF_CONST: + if (!values_equal_for_ipcp_p (ajf1->value.constant, + ajf2->value.constant)) + return false; + break; + case IPA_JF_PASS_THROUGH: + { + ipa_pass_through_data *ipt1 = &ajf1->value.pass_through; + ipa_pass_through_data *ipt2 = &ajf2->value.pass_through; + if (!ipa_agg_pass_through_jf_equivalent_p (ipt1, ipt2, true)) + return false; + } + break; + case IPA_JF_LOAD_AGG: + { + ipa_load_agg_data *ila1 = &ajf1->value.load_agg; + ipa_load_agg_data *ila2 = &ajf2->value.load_agg; + if (!ipa_agg_pass_through_jf_equivalent_p (&ila1->pass_through, + &ila2->pass_through, true)) + return false; + if (ila1->offset != ila2->offset + || ila1->by_ref != ila2->by_ref + || !types_compatible_p (ila1->type, ila2->type)) + return false; + } + break; + default: + gcc_unreachable (); + } + return true; +} + +/* Return true if the two jump functions are known to be equivalent. The + function can be used before the IPA phase of IPA-CP or inlining because it + cannot cope with refdesc changes these passes can carry out. */ + +bool +ipa_jump_functions_equivalent_p (ipa_jump_func *jf1, ipa_jump_func *jf2) +{ + if (jf1->type != jf2->type) + return false; + + switch (jf1->type) + { + case IPA_JF_UNKNOWN: + break; + case IPA_JF_CONST: + { + tree cst1 = ipa_get_jf_constant (jf1); + tree cst2 = ipa_get_jf_constant (jf2); + if (!values_equal_for_ipcp_p (cst1, cst2)) + return false; + + ipa_cst_ref_desc *rd1 = jfunc_rdesc_usable (jf1); + ipa_cst_ref_desc *rd2 = jfunc_rdesc_usable (jf2); + if (rd1 && rd2) + { + gcc_assert (rd1->refcount == 1 + && rd2->refcount == 1); + gcc_assert (!rd1->next_duplicate && !rd2->next_duplicate); + } + else if (rd1) + return false; + else if (rd2) + return false; + } + break; + case IPA_JF_PASS_THROUGH: + { + ipa_pass_through_data *ipt1 = &jf1->value.pass_through; + ipa_pass_through_data *ipt2 = &jf2->value.pass_through; + if (!ipa_agg_pass_through_jf_equivalent_p (ipt1, ipt2, false)) + return false; + } + break; + case IPA_JF_ANCESTOR: + { + ipa_ancestor_jf_data *ia1 = &jf1->value.ancestor; + ipa_ancestor_jf_data *ia2 = &jf2->value.ancestor; + + if (ia1->formal_id != ia2->formal_id + || ia1->agg_preserved != ia2->agg_preserved + || ia1->keep_null != ia2->keep_null + || ia1->offset != ia2->offset) + return false; + } + break; + default: + gcc_unreachable (); + } + + if (((jf1->m_vr != nullptr) != (jf2->m_vr != nullptr)) + || (jf1->m_vr && !jf1->m_vr->equal_p (*jf2->m_vr))) + return false; + + unsigned alen = vec_safe_length (jf1->agg.items); + if (vec_safe_length (jf2->agg.items) != alen) + return false; + + if (!alen) + return true; + + if (jf1->agg.by_ref != jf2->agg.by_ref) + return false; + + for (unsigned i = 0 ; i < alen; i++) + if (!ipa_agg_jump_functions_equivalent_p (&(*jf1->agg.items)[i], + &(*jf2->agg.items)[i])) + return false; + + return true; +} + #include "gt-ipa-prop.h" diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h index ee3c0006add..93d1b87b1f7 100644 --- a/gcc/ipa-prop.h +++ b/gcc/ipa-prop.h @@ -308,6 +308,7 @@ public: tree type () const { return m_type; } void get_vrange (Value_Range &) const; bool equal_p (const vrange &) const; + bool equal_p (const ipa_vr &) const; const vrange_storage *storage () const { return m_storage; } void streamer_read (lto_input_block *, class data_in *); void streamer_write (output_block *) const; @@ -1278,5 +1279,7 @@ ipa_range_set_and_normalize (vrange &r, tree val) bool ipa_return_value_range (Value_Range &range, tree decl); void ipa_record_return_value_range (Value_Range val); +bool ipa_jump_functions_equivalent_p (ipa_jump_func *jf1, ipa_jump_func *jf2); + #endif /* IPA_PROP_H */ diff --git a/gcc/testsuite/gcc.dg/lto/pr113907_0.c b/gcc/testsuite/gcc.dg/lto/pr113907_0.c new file mode 100644 index 00000000000..3c4dd475c01 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr113907_0.c @@ -0,0 +1,18 @@ +/* { dg-lto-do run } */ +/* { dg-lto-options {{-O3 -flto}} } */ + +struct bar {int a;}; +struct foo {int a;}; +struct barp {struct bar *f; struct bar *g;}; +extern struct foo **ptr; +int test2 (void *); +int test3 (void *); +int +testb(void) +{ + struct bar *fp; + test2 ((void *)&fp); + fp = (void *) 0; + (*ptr)++; + test3 ((void *)&fp); +} diff --git a/gcc/testsuite/gcc.dg/lto/pr113907_1.c b/gcc/testsuite/gcc.dg/lto/pr113907_1.c new file mode 100644 index 00000000000..1c48bfd83a4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr113907_1.c @@ -0,0 +1,35 @@ +__attribute__((used)) int val,val2 = 1; + +struct foo {int a;}; + +struct foo **ptr; + +__attribute__ ((noipa)) +int +test2 (void *a) +{ + ptr = (struct foo **)a; +} +int test3 (void *a); + +int +test(void) +{ + struct foo *fp; + test2 ((void *)&fp); + fp = (void *) 0; + (*ptr)++; + test3 ((void *)&fp); +} + +int testb (void); + +int +main() +{ + for (int i = 0; i < val2; i++) + if (val) + testb (); + else + test(); +} diff --git a/gcc/testsuite/gcc.dg/lto/pr113907_2.c b/gcc/testsuite/gcc.dg/lto/pr113907_2.c new file mode 100644 index 00000000000..172e86e8b0b --- /dev/null +++ b/gcc/testsuite/gcc.dg/lto/pr113907_2.c @@ -0,0 +1,11 @@ +/* { dg-options "-O3 -flto -fno-strict-aliasing" } */ + +__attribute__ ((noinline)) +int +test3 (void *a) +{ + if (!*(void **)a) + __builtin_abort (); + return 0; +} +