On Thu, 24 Feb 2022, Jakub Jelinek wrote: > 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?
OK. Thanks, Richard. > 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 > > -- Richard Biener <rguent...@suse.de> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)