ipa-cp: Do not be too optimistic about self-recursive edges (PR 107661)
authorMartin Jambor <mjambor@suse.cz>
Tue, 22 Nov 2022 17:22:03 +0000 (18:22 +0100)
committerMartin Jambor <mjambor@suse.cz>
Tue, 22 Nov 2022 17:28:44 +0000 (18:28 +0100)
PR 107661 shows that function push_agg_values_for_index_from_edge
should not attempt to optimize self-recursive call graph edges when
called from cgraph_edge_brings_all_agg_vals_for_node.  Unlike when
being called from find_aggregate_values_for_callers_subset, we cannot
expect that any cloning for constants would lead to the edge leading
from a new clone to the same new clone, in this case it would only be
redirected to a new callee.

Fixed by adding a parameter to push_agg_values_from_edge whether being
optimistic about self-recursive edges is possible.

gcc/ChangeLog:

2022-11-22  Martin Jambor  <mjambor@suse.cz>

PR ipa/107661
* ipa-cp.cc (push_agg_values_from_edge): New parameter
optimize_self_recursion, use it to decide whether to pass interim to
the helper function.
(find_aggregate_values_for_callers_subset): Pass true in the new
parameter of push_agg_values_from_edge.
(cgraph_edge_brings_all_agg_vals_for_node): Pass false in the new
parameter of push_agg_values_from_edge.

gcc/testsuite/ChangeLog:

2022-11-22  Martin Jambor  <mjambor@suse.cz>

PR ipa/107661
* g++.dg/ipa/pr107661.C: New test.

gcc/ipa-cp.cc
gcc/testsuite/g++.dg/ipa/pr107661.C [new file with mode: 0644]

index d2bcd5e..f0feb4b 100644 (file)
@@ -5752,14 +5752,16 @@ push_agg_values_for_index_from_edge (struct cgraph_edge *cs, int index,
    description of ultimate callee of CS or the one it was cloned from (the
    summary where lattices are).  If INTERIM is non-NULL, it contains the
    current interim state of collected aggregate values which can be used to
-   compute values passed over self-recursive edges and to skip values which
-   clearly will not be part of intersection with INTERIM.  */
+   compute values passed over self-recursive edges (if OPTIMIZE_SELF_RECURSION
+   is true) and to skip values which clearly will not be part of intersection
+   with INTERIM.  */
 
 static void
 push_agg_values_from_edge (struct cgraph_edge *cs,
                           ipa_node_params *dest_info,
                           vec<ipa_argagg_value> *res,
-                          const ipa_argagg_value_list *interim)
+                          const ipa_argagg_value_list *interim,
+                          bool optimize_self_recursion)
 {
   ipa_edge_args *args = ipa_edge_args_sum->get (cs);
   if (!args)
@@ -5785,7 +5787,9 @@ push_agg_values_from_edge (struct cgraph_edge *cs,
       ipcp_param_lattices *plats = ipa_get_parm_lattices (dest_info, index);
       if (plats->aggs_bottom)
        continue;
-      push_agg_values_for_index_from_edge (cs, index, res, interim);
+      push_agg_values_for_index_from_edge (cs, index, res,
+                                          optimize_self_recursion ? interim
+                                          : NULL);
     }
 }
 
@@ -5804,7 +5808,7 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
   /* gather_edges_for_value puts a non-recursive call into the first element of
      callers if it can.  */
   auto_vec<ipa_argagg_value, 32> interim;
-  push_agg_values_from_edge (callers[0], dest_info, &interim, NULL);
+  push_agg_values_from_edge (callers[0], dest_info, &interim, NULL, true);
 
   unsigned valid_entries = interim.length ();
   if (!valid_entries)
@@ -5815,7 +5819,7 @@ find_aggregate_values_for_callers_subset (struct cgraph_node *node,
     {
       auto_vec<ipa_argagg_value, 32> last;
       ipa_argagg_value_list avs (&interim);
-      push_agg_values_from_edge (callers[i], dest_info, &last, &avs);
+      push_agg_values_from_edge (callers[i], dest_info, &last, &avs, true);
 
       valid_entries = intersect_argaggs_with (interim, last);
       if (!valid_entries)
@@ -5882,7 +5886,7 @@ cgraph_edge_brings_all_agg_vals_for_node (struct cgraph_edge *cs,
   ipa_node_params *dest_info = ipa_node_params_sum->get (node);
   gcc_checking_assert (dest_info->ipcp_orig_node);
   dest_info = ipa_node_params_sum->get (dest_info->ipcp_orig_node);
-  push_agg_values_from_edge (cs, dest_info, &edge_values, &existing);
+  push_agg_values_from_edge (cs, dest_info, &edge_values, &existing, false);
   const ipa_argagg_value_list avl (&edge_values);
   return avl.superset_of_p (existing);
 }
diff --git a/gcc/testsuite/g++.dg/ipa/pr107661.C b/gcc/testsuite/g++.dg/ipa/pr107661.C
new file mode 100644 (file)
index 0000000..cc6f853
--- /dev/null
@@ -0,0 +1,45 @@
+/* { dg-do run  { target c++11 } } */
+/* { dg-options "-O1 -fipa-cp -fipa-cp-clone" } */
+
+struct R {} RGood;
+struct L {} LBad;
+
+volatile int vi;
+static void __attribute__((noipa)) L_run(void) { vi = 0; __builtin_abort (); }
+static void callback_fn_L(void) { vi = 1; L_run(); }
+static void callback_fn_R(void) { vi = 2; }
+
+struct function_ref {
+  void (*callback)(void) = nullptr;
+
+  function_ref(L * pl) { callback = callback_fn_L; }
+  function_ref(R * pr) { callback = callback_fn_R; }
+};
+
+// allow one level of recursion to call callback twice
+static int is_recur(void) {
+    static int n = 0;
+    switch (n++) {
+      case 0: return 1;
+      default: return 0;
+    }
+}
+
+static void do3(volatile int * punused, function_ref Expired) {
+  Expired.callback();
+
+  if (is_recur())
+      do3(punused, Expired);
+}
+
+static void do1(function_ref Expired) {
+  volatile int unused = 42;
+
+  do3(&unused, Expired);
+}
+
+int main(int, const char **) { do1(&RGood); return 0; }
+
+void seemingly_unused_foo(void) { do1(&LBad); }
+
+void (*fnptr)(void) = seemingly_unused_foo;