ipa: Fix wrong code with failed propagation to builtin_constant_p [PR93940]

this patch fixes wrong code on a testcase where inline predicts
builtin_constant_p to be true but we fail to optimize its parameter to constant
becuase FRE is not run and the value is passed by an aggregate.

This patch makes the inline predicates to disable aggregate tracking
when FRE is not going to be run and similarly value range when VRP is not
going to be run.

This is just partial fix.  Even with it we can arrange FRE/VRP to fail and
produce wrong code, unforutnately.

I think for GCC11 I will need to implement transformation in ipa-inline
but this is bit hard to do: predicates only tracks that value will be constant
and do not track what constant to be.

Optimizing builtin_constant_p in a conditional is not going to do good job
when the value is used later in a place that expects it to be constant.
This is pre-existing problem that is not limited to inline tracking. For example,
FRE may do the transofrm at one place but not in another due to alias oracle
walking limits.

So I am not sure what full fix would be :(

gcc/ChangeLog:

2020-04-04  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/93940
	* ipa-fnsummary.c (vrp_will_run_p): New function.
	(fre_will_run_p): New function.
	(evaluate_properties_for_edge): Use it.
	* ipa-inline.c (can_inline_edge_by_limits_p): Do not inline
	!optimize_debug to optimize_debug.

gcc/testsuite/ChangeLog:

2020-04-04  Jan Hubicka  <hubicka@ucw.cz>

	* g++.dg/tree-ssa/pr93940.C: New test.

diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index b411bc4d6603806dab16ee1466f6d170cdab7fcb..d96c8e9b03c2d85b1d1b560a0e5bbfd9d66a3583 100644 (file)
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -503,6 +503,32 @@ evaluate_conditions_for_known_args (struct cgraph_node *node,
     *ret_nonspec_clause = nonspec_clause;
 }
 
+/* Return true if VRP will be exectued on the function.
+   We do not want to anticipate optimizations that will not happen.
+
+   FIXME: This can be confused with -fdisable and debug counters and thus
+   it should not be used for correctness (only to make heuristics work).
+   This means that inliner should do its own optimizations of expressions
+   that it predicts to be constant so wrong code can not be triggered by
+   builtin_constant_p.  */
+
+static bool
+vrp_will_run_p (struct cgraph_node *node)
+{
+  return (opt_for_fn (node->decl, optimize)
+         && !opt_for_fn (node->decl, optimize_debug)
+         && opt_for_fn (node->decl, flag_tree_vrp));
+}
+
+/* Similarly about FRE.  */
+
+static bool
+fre_will_run_p (struct cgraph_node *node)
+{
+  return (opt_for_fn (node->decl, optimize)
+         && !opt_for_fn (node->decl, optimize_debug)
+         && opt_for_fn (node->decl, flag_tree_fre));
+}
 
 /* Work out what conditions might be true at invocation of E.
    Compute costs for inlined edge if INLINE_P is true.
@@ -594,6 +620,7 @@ evaluate_properties_for_edge (struct cgraph_edge *e, bool inline_p,
 
                /* If we failed to get simple constant, try value range.  */
                if ((!cst || TREE_CODE (cst) != INTEGER_CST)
+                   && vrp_will_run_p (caller)
                    && ipa_is_param_used_by_ipa_predicates (callee_pi, i))
                  {
                    value_range vr 
@@ -609,14 +636,17 @@ evaluate_properties_for_edge (struct cgraph_edge *e, bool inline_p,
                  }
 
                /* Determine known aggregate values.  */
-               ipa_agg_value_set agg
-                   = ipa_agg_value_set_from_jfunc (caller_parms_info,
-                                                   caller, &jf->agg);
-               if (agg.items.length ())
+               if (vrp_will_run_p (caller))
                  {
-                   if (!known_aggs_ptr->length ())
-                     vec_safe_grow_cleared (known_aggs_ptr, count);
-                   (*known_aggs_ptr)[i] = agg;
+                   ipa_agg_value_set agg
+                       = ipa_agg_value_set_from_jfunc (caller_parms_info,
+                                                       caller, &jf->agg);
+                   if (agg.items.length ())
+                     {
+                       if (!known_aggs_ptr->length ())
+                         vec_safe_grow_cleared (known_aggs_ptr, count);
+                       (*known_aggs_ptr)[i] = agg;
+                     }
                  }
              }
 

diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 302ce16a6466acc6f6c706474f73e3f975ffc541..f71443feff7ede97477d19bb79d7b6c98a4734c4 100644 (file)
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -485,6 +485,7 @@ can_inline_edge_by_limits_p (struct cgraph_edge *e, bool report,
      else if (check_match (flag_wrapv)
              || check_match (flag_trapv)
              || check_match (flag_pcc_struct_return)
+             || check_maybe_down (optimize_debug)
              /* When caller or callee does FP math, be sure FP codegen flags
                 compatible.  */
              || ((caller_info->fp_expressions && callee_info->fp_expressions)

diff --git a/gcc/testsuite/g++.dg/tree-ssa/pr93940.C b/gcc/testsuite/g++.dg/tree-ssa/pr93940.C
new file mode 100644 (file)
index 0000000..b656aad
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/pr93940.C
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-Og --coverage -pthread -fdump-tree-optimized -std=c++17" } */
+using uint16_t = unsigned short;
+
+struct a {
+    uint16_t b = 0;
+};
+struct c {
+    short d;
+};
+class e {
+public:
+    void f();
+    void init_session(c);
+};
+
+auto htons = [](uint16_t s) {
+    if (__builtin_constant_p(s)) {
+        return uint16_t(uint16_t(s >> 8) | uint16_t(s << 8));
+    }
+    return uint16_t(uint16_t(s >> 8) | uint16_t(s << 8));
+};
+
+struct g {
+    e h;
+    void i(a k) {
+        h.f();
+        auto j = c();
+        j.d = htons(k.b);
+        h.init_session(j);
+    }
+};
+
+void test() {
+    g().i({});
+}
+
+/* { dg-final { scan-tree-dump-not "builtin_unreachable" "optimized"} } */