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)

Reply via email to