Hi!

The following testcase is miscompiled, because -fipa-pure-const discovers
that bar is const, but when sccvn during fre3 sees
  # .MEM_140 = VDEF <.MEM_96>
  *__pred$__d_43 = _50 (_49);
where _50 value numbers to &bar, it value numbers .MEM_140 to
vuse_ssa_val (gimple_vuse (stmt)).  For const/pure calls that return
a SSA_NAME (or don't have lhs) that is fine, those calls don't store
anything, but if the lhs is present and not an SSA_NAME, value numbering
the vdef to anything but itself means that e.g. walk_non_aliased_vuses
won't consider the call, but the call acts as a store to its lhs.
When it is ignored, sccvn will return whatever has been stored to the
lhs earlier.

I've bootstrapped/regtested an earlier version of this patch, which did the
if (!lhs && gimple_call_lhs (stmt))
  changed |= set_ssa_val_to (vdef, vdef);
part before else if (vnresult->result_vdef), and that regressed
+FAIL: gcc.dg/pr51879-16.c scan-tree-dump-times pre "foo \\\\(" 1
+FAIL: gcc.dg/pr51879-16.c scan-tree-dump-times pre "foo2 \\\\(" 1
so this updated patch uses result_vdef there as before and only otherwise
(which I think must be the const/pure case) decides based on whether the
lhs is non-SSA_NAME.

Ok for trunk if it passes another bootstrap/regtest?

2022-02-24  Jakub Jelinek  <ja...@redhat.com>

        PR tree-optimization/104601
        * tree-ssa-sccvn.cc (visit_reference_op_call): For calls with
        non-SSA_NAME lhs value number vdef to itself instead of e.g. the
        vuse value number.

        * g++.dg/torture/pr104601.C: New test.

--- gcc/tree-ssa-sccvn.cc.jj    2022-02-11 00:19:22.432063254 +0100
+++ gcc/tree-ssa-sccvn.cc       2022-02-24 09:43:39.715051959 +0100
@@ -5218,12 +5218,20 @@ visit_reference_op_call (tree lhs, gcall
 
   if (vnresult)
     {
-      if (vnresult->result_vdef && vdef)
-       changed |= set_ssa_val_to (vdef, vnresult->result_vdef);
-      else if (vdef)
-       /* If the call was discovered to be pure or const reflect
-          that as far as possible.  */
-       changed |= set_ssa_val_to (vdef, vuse_ssa_val (gimple_vuse (stmt)));
+      if (vdef)
+       {
+         if (vnresult->result_vdef)
+           changed |= set_ssa_val_to (vdef, vnresult->result_vdef);
+         else if (!lhs && gimple_call_lhs (stmt))
+           /* If stmt has non-SSA_NAME lhs, value number the vdef to itself,
+              as the call still acts as a lhs store.  */
+           changed |= set_ssa_val_to (vdef, vdef);
+         else
+           /* If the call was discovered to be pure or const reflect
+              that as far as possible.  */
+           changed |= set_ssa_val_to (vdef,
+                                      vuse_ssa_val (gimple_vuse (stmt)));
+       }
 
       if (!vnresult->result && lhs)
        vnresult->result = lhs;
@@ -5248,7 +5256,11 @@ visit_reference_op_call (tree lhs, gcall
              if (TREE_CODE (fn) == ADDR_EXPR
                  && TREE_CODE (TREE_OPERAND (fn, 0)) == FUNCTION_DECL
                  && (flags_from_decl_or_type (TREE_OPERAND (fn, 0))
-                     & (ECF_CONST | ECF_PURE)))
+                     & (ECF_CONST | ECF_PURE))
+                 /* If stmt has non-SSA_NAME lhs, value number the
+                    vdef to itself, as the call still acts as a lhs
+                    store.  */
+                 && (lhs || gimple_call_lhs (stmt) == NULL_TREE))
                vdef_val = vuse_ssa_val (gimple_vuse (stmt));
            }
          changed |= set_ssa_val_to (vdef, vdef_val);
--- gcc/testsuite/g++.dg/torture/pr104601.C.jj  2022-02-23 16:23:52.437366019 
+0100
+++ gcc/testsuite/g++.dg/torture/pr104601.C     2022-02-23 16:23:37.080578941 
+0100
@@ -0,0 +1,32 @@
+// PR tree-optimization/104601
+// { dg-do run }
+// { dg-options "-std=c++17" }
+
+#include <algorithm>
+#include <optional>
+
+inline std::optional<int>
+foo (std::vector<int>::iterator b, std::vector<int>::iterator c,
+     std::optional<int> h (int))
+{
+  std::optional<int> d;
+  find_if (b, c, [&](auto e) { d = h(e); return d; });
+  return d;
+}
+
+std::optional<int>
+bar (int)
+{
+  return 1;
+}
+
+int
+main ()
+{
+  std::vector<int> g(10);
+  auto b = g.begin ();
+  auto c = g.end ();
+  auto e = foo (b, c, bar);
+  if (!e)
+    __builtin_abort ();
+}

        Jakub

Reply via email to