[gcc r16-9357] ipa-cp: Only create all-context nodes for local cgraph nodes (PR125207)
Martin Jambor via Gcc-cvs <[email protected]>
| Newsgroups | gmane.comp.gcc.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://gcc.gnu.org/g:8748d41721d839844988b583d09720582ba0cb33 commit r16-9357-g8748d41721d839844988b583d09720582ba0cb33 Author: Martin Jambor <[email protected]> Date: Thu Jul 9 17:17:50 2026 +0200 ipa-cp: Only create all-context nodes for local cgraph nodes (PR125207) IPA-CP contains special logic to create specially marked clones with constants that come from all callees (that are constant in all contexts). In the past that was the only case in which the heuristics tried to assess the effect of multiple constants at once (that is no longer true since GCC 16) and they are also useful when we realize we can refine them still further in the case of recursive calls (such as in testsuite/gcc.dg/ipa/pr93707.c) turn to carry more constants after the initial "all-context" clone is created. Unfortunately, the code creating the clones simply redirects all incoming call graph edges which may include those which were indirect in the phase of the algorithm which gathered IPA-CP lattices and the arguments in these calls might contain different values than all initially known ones, leading to miscompilations like the one in PR125207. Since GCC 16 already feeds the heuristics with all known constants that are passed along with the constant the evaluation was initiated for, the all-context nodes are really only necessary for the recursive refinement cases described above. It seems appropriate to only create them when they are what they are called, when there can be no indirect or unknown calls to them, i.e. when they are local. This has the nice effect of simplifying decide_whether_version_node somewhat. There is some testsuite fallout. First, IPA-CP will now no longer clone a non-static function to only remove a (non-constant) parameter. This likely only makes sense with tiny values of param ipa-cp-eval-threshold or very small functions which are likely going to be inlined anyway. But gcc.dg/vla-1.c expected that to happen so I made the function to be cloned static. The testcase libgomp.c/ipcp-cb-spec1.c stopped working for the same reason (the outlined task body has its address taken) but looking into the dumps the test does not seem to test what it is supposed to, the test function was completely eliminated before IPA. I have therefore changed it to really propagate constants. gcc/ChangeLog: 2026-06-02 Martin Jambor <[email protected]> PR ipa/125207 * ipa-cp.cc (decide_about_value): Add detailed dumping about skipping a value when there are no edges for it. Make dumping about skipping because of growth limit violation consistent with other cases. (decide_whether_version_node): Only create all-context nodes for local nodes, remove the logic doing it in other cases, allow cloning for single constant lattices for non-local nodes. Adjust dumping. gcc/testsuite/ChangeLog: 2026-06-02 Martin Jambor <[email protected]> PR ipa/125207 * gcc.dg/ipa/pr125207.c: New test. * gcc.dg/vla-1.c (f1): Make function f1 static. libgomp/ChangeLog: 2026-06-03 Martin Jambor <[email protected]> PR ipa/125207 * testsuite/libgomp.c/ipcp-cb-spec1.c (test): Rework. (cherry picked from commit 8d7dc3ed288b9fec82721b432d5b683ffade0b1b) Diff: --- gcc/ipa-cp.cc | 102 +++++++++++----------------- gcc/testsuite/gcc.dg/ipa/pr125207.c | 43 ++++++++++++ gcc/testsuite/gcc.dg/vla-1.c | 2 +- libgomp/testsuite/libgomp.c/ipcp-cb-spec1.c | 14 ++-- 4 files changed, 93 insertions(+), 68 deletions(-) diff --git a/gcc/ipa-cp.cc b/gcc/ipa-cp.cc index b040a30e3737..7a50a7ee811a 100644 --- a/gcc/ipa-cp.cc +++ b/gcc/ipa-cp.cc @@ -5963,7 +5963,7 @@ decide_about_value (struct cgraph_node *node, int index, HOST_WIDE_INT offset, > get_max_overall_size (node, cur_sweep)) { if (dump_file && (dump_flags & TDF_DETAILS)) - fprintf (dump_file, " Ignoring candidate value because " + fprintf (dump_file, " - ignoring candidate value because " "maximum unit size would be reached with %li.\n", val->local_size_cost + overall_size); return false; @@ -5971,7 +5971,19 @@ decide_about_value (struct cgraph_node *node, int index, HOST_WIDE_INT offset, else if (!get_info_about_necessary_edges (val, node, &freq_sum, &caller_count, &rec_count_sum, &count_sum, &called_without_ipa_profile)) - return false; + { + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, " - skipping candidate value "); + print_ipcp_constant_value (dump_file, val->value); + fprintf (dump_file, " for "); + ipa_dump_param (dump_file, ipa_node_params_sum->get (node), index); + if (offset != -1) + fprintf (dump_file, ", offset: " HOST_WIDE_INT_PRINT_DEC, offset); + fprintf (dump_file, ": no relevant callers\n"); + } + return false; + } if (!dbg_cnt (ipa_cp_values)) return false; @@ -6173,22 +6185,33 @@ decide_whether_version_node (struct cgraph_node *node, int cur_sweep) if (info->node_dead || count == 0) return false; + bool clone_for_all_contexts = node->local; if (dump_file && (dump_flags & TDF_DETAILS)) - fprintf (dump_file, "\nEvaluating opportunities for %s.\n", - node->dump_name ()); + { + fprintf (dump_file, "\nEvaluating opportunities for %s.", + node->dump_name ()); + if (clone_for_all_contexts) + fprintf (dump_file, " Will try to create a special all-context " + "clone.\n"); + fprintf (dump_file, "\n"); + } auto_vec <cloning_opportunity_ranking, 32> opp_ranking; for (int i = 0; i < count;i++) { if (!ipa_is_param_used (info, i)) - continue; + { + if (dump_file && (dump_flags & TDF_DETAILS)) + fprintf (dump_file, " - ignoring unused parameter %i.\n", i); + continue; + } class ipcp_param_lattices *plats = ipa_get_parm_lattices (info, i); ipcp_lattice<tree> *lat = &plats->itself; ipcp_lattice<ipa_polymorphic_call_context> *ctxlat = &plats->ctxlat; if (!lat->bottom - && !lat->is_single_const ()) + && (!clone_for_all_contexts || !lat->is_single_const ())) { ipcp_value<tree> *val; for (val = lat->values; val; val = val->next) @@ -6230,9 +6253,10 @@ decide_whether_version_node (struct cgraph_node *node, int cur_sweep) ipcp_value<tree> *val; for (aglat = plats->aggs; aglat; aglat = aglat->next) if (!aglat->bottom && aglat->values - /* If the following is false, the one value has been considered + /* If the following is false, the one value will be considered for cloning for all contexts. */ - && (plats->aggs_contain_variable + && (!clone_for_all_contexts + || plats->aggs_contain_variable || !aglat->is_single_const ())) for (val = aglat->values; val; val = val->next) { @@ -6246,7 +6270,7 @@ decide_whether_version_node (struct cgraph_node *node, int cur_sweep) } if (!ctxlat->bottom - && !ctxlat->is_single_const ()) + && (!clone_for_all_contexts || !ctxlat->is_single_const ())) { ipcp_value<ipa_polymorphic_call_context> *val; for (val = ctxlat->values; val; val = val->next) @@ -6289,19 +6313,21 @@ decide_whether_version_node (struct cgraph_node *node, int cur_sweep) } } + if (!clone_for_all_contexts) + return ret; + struct caller_statistics stats; init_caller_stats (&stats); node->call_for_symbol_thunks_and_aliases (gather_caller_stats, &stats, false); if (!stats.n_calls) { - if (dump_file) + if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, " Not cloning for all contexts because " "there are no callers of the original node (any more).\n"); return ret; } - bool do_clone_for_all_contexts = false; ipa_auto_call_arg_values avals; int removable_params_cost; bool ctx_independent_const @@ -6310,60 +6336,10 @@ decide_whether_version_node (struct cgraph_node *node, int cur_sweep) if (ctx_independent_const || devirt_bonus > 0 || (removable_params_cost && clone_for_param_removal_p (node))) { - ipa_call_estimates estimates; - - estimate_ipcp_clone_size_and_time (node, &avals, &estimates); - sreal time = estimates.nonspecialized_time - estimates.time; - time += devirt_bonus; - time += hint_time_bonus (node, estimates); - time += removable_params_cost; - int size = estimates.size - stats.n_calls * removable_params_cost; - - if (dump_file && (dump_flags & TDF_DETAILS)) - fprintf (dump_file, " - context independent values, size: %i, " - "time_benefit: %f\n", size, (time).to_double ()); - - if (size <= 0 || node->local) - { - if (!dbg_cnt (ipa_cp_values)) - return ret; - - do_clone_for_all_contexts = true; - if (dump_file) - fprintf (dump_file, " Decided to specialize for all " - "known contexts, code not going to grow.\n"); - } - else if (good_cloning_opportunity_p (node, time, stats.freq_sum, - stats.count_sum, size, - stats.called_without_ipa_profile, - cur_sweep)) - { - if (size + overall_size <= get_max_overall_size (node, cur_sweep)) - { - if (!dbg_cnt (ipa_cp_values)) - return ret; - - do_clone_for_all_contexts = true; - overall_size += size; - if (dump_file) - fprintf (dump_file, " Decided to specialize for all " - "known contexts, growth (to %li) deemed " - "beneficial.\n", overall_size); - } - else if (dump_file && (dump_flags & TDF_DETAILS)) - fprintf (dump_file, " Not cloning for all contexts because " - "maximum unit size would be reached with %li.\n", - size + overall_size); - } - else if (dump_file && (dump_flags & TDF_DETAILS)) - fprintf (dump_file, " Not cloning for all contexts because " - "!good_cloning_opportunity_p.\n"); - } + if (!dbg_cnt (ipa_cp_values)) + return ret; - if (do_clone_for_all_contexts) - { auto_vec<cgraph_edge *> callers = node->collect_callers (); - for (int i = callers.length () - 1; i >= 0; i--) { cgraph_edge *cs = callers[i]; diff --git a/gcc/testsuite/gcc.dg/ipa/pr125207.c b/gcc/testsuite/gcc.dg/ipa/pr125207.c new file mode 100644 index 000000000000..9c48add4810e --- /dev/null +++ b/gcc/testsuite/gcc.dg/ipa/pr125207.c @@ -0,0 +1,43 @@ +/* { dg-do run } */ +/* { dg-options "-O3 --param ipa-cp-eval-threshold=1" } */ + +typedef int(*fp_t_5)(int, int, int); +int g2, g30, g21, g17, g16, g14, g9, g3, g12, g31; +_Bool g22, g26; +static +int f11(int, fp_t_5 a1, int, int, int) +{ + do { + g12 = a1(1 ^ g14, 0, g9); + if (g22) + if (g2 != g17) + if (g26) + return g30; + } while(true); +} +static __attribute__((noinline)) +int f2(int a0, int, int a2) +{ + static int t = 0; + t++; + if (t > 2) + __builtin_abort(); + g26 = true; + g22 = true; + if (g3 != a0) { + g31 = 2; + a0 = g31; + } + if (a0 == 2) + if (a2 != 80) + return 0; + g17 = 6; + g21 += f11(0, f2, 0, 0, 0); + return 0; +} +void f1() { f2(0, 0, 0); } +int main() +{ + f2(0, 0, 0); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/vla-1.c b/gcc/testsuite/gcc.dg/vla-1.c index 2ab2b7a48acf..6132c83f06d2 100644 --- a/gcc/testsuite/gcc.dg/vla-1.c +++ b/gcc/testsuite/gcc.dg/vla-1.c @@ -1,7 +1,7 @@ /* { dg-do compile } */ /* { dg-options "-g -O3 -fdump-tree-optimized -fvar-tracking-assignments -fno-selective-scheduling -fno-selective-scheduling2 -fno-ipa-vrp" } */ -int __attribute__((noinline)) +static int __attribute__((noinline)) f1 (int i) { char a[i + 1]; diff --git a/libgomp/testsuite/libgomp.c/ipcp-cb-spec1.c b/libgomp/testsuite/libgomp.c/ipcp-cb-spec1.c index ff82f4c2f29d..9439fb1fb873 100644 --- a/libgomp/testsuite/libgomp.c/ipcp-cb-spec1.c +++ b/libgomp/testsuite/libgomp.c/ipcp-cb-spec1.c @@ -4,15 +4,21 @@ /* { dg-options "-O3 -fopenmp -std=gnu99 -fdump-ipa-cp-details" } */ /* { dg-require-effective-target fopenmp } */ -void test(int c) { +static inline void __attribute__((always_inline)) test(int c) { for (int i = 0; i < c; i++) if (!__builtin_constant_p(c)) __builtin_abort(); } -int main() { + +int foo(int c) { #pragma omp task - test(7); + test(c); + return 0; +} + +int main() { + foo(7); return 0; } -/* { dg-final { scan-ipa-dump "Creating a specialized node of main._omp_fn" "cp" } } */ +/* { dg-final { scan-ipa-dump "Creating a specialized node of foo._omp_fn" "cp" } } */