[Bug c++/110204] New: Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[Bug c++/110204] New: Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[jun at junz dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

            Bug ID: 110204
           Summary: Suspicous warning when compiling ranges-v3 using GCC
                    trunk (iteration 9223372036854775807 invokes undefined
                    behavior)
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jun at junz dot org
  Target Milestone: ---

Given code:

#include <vector>
#include <map>
#include <range/v3/all.hpp>

std::map<int, int> m;

std::vector<int> foo() {
  return ranges::to<std::vector>(m | ranges::views::keys);
}

GCC trunk reports:

/opt/compiler-explorer/gcc-trunk-20230610/include/c++/14.0.0/bits/stl_iterator_base_funcs.h:175:9:
warning: iteration 9223372036854775807 invokes undefined behavior
[-Waggressive-loop-optimizations]
  175 |         while (__n--)
      |         ^~~~~
/opt/compiler-explorer/gcc-trunk-20230610/include/c++/14.0.0/bits/stl_iterator_base_funcs.h:175:9:
note: within this loop

while Clang and GCC 13 both say OK. I think this is a regression.
goldbolt link: https://godbolt.org/z/W33aMqr5c

[Bug tree-optimization/110204] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Created attachment 55298
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55298&action=edit
Testcase preprocessed source

Please next time attach the preprocessed source as requested at
https://gcc.gnu.org/bugs/ .

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
            Summary|Suspicous warning when      |[14 Regression] Suspicous
                   |compiling ranges-v3 using   |warning when compiling
                   |GCC trunk (iteration        |ranges-v3 using GCC trunk
                   |9223372036854775807 invokes |(iteration
                   |undefined behavior)         |9223372036854775807 invokes
                   |                            |undefined behavior)
           Keywords|                            |needs-bisection

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The preprocessed source that is produced by GCC 13 is warning free.

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
PRE leaves around:
  <bb 11> [local count: 118111600]:
...
  pretmp_163 = 0;
....
  <bb 30> [local count: 19488414]:
  if (pretmp_163 != 0)
    goto <bb 31>; [59.00%]
  else
    goto <bb 35>; [41.00%]

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Replaced _40 - _41 with pretmp_163 in all uses of _42 = _40 - _41;
Replaced _42 /[ex] 4 with pretmp_162 in all uses of _43 = _42 /[ex] 4;
Replaced (long unsigned int) _43 with pretmp_161 in all uses of _44 = (long
unsigned int) _43;
Removing unexecutable edge from if (_42 != 0)
Removing dead stmt _44 = (long unsigned int) _43;
Removing dead stmt _43 = _42 /[ex] 4;
Removing dead stmt _42 = _40 - _41;
Removing dead stmt _41 = MEM[(const struct vector
*)_3(D)].D.214899._M_impl.D.214244._M_start;
Removing dead stmt _40 = MEM[(const struct vector
*)_3(D)].D.214899._M_impl.D.214244._M_finish;


What is interesting is before PRE we had:
```
  <bb 28> [local count: 19488414]:
  if (_42 != 0)
    goto <bb 29>; [59.00%]
  else
    goto <bb 33>; [41.00%]

  <bb 29> [local count: 11498164]:
  __n_154 = _43 + -1;
  if (_42 != 0)
    goto <bb 44>; [89.00%]
  else
    goto <bb 33>; [11.00%]
```

After we got:
```
  <bb 30> [local count: 19488414]:
  if (pretmp_163 != 0)
    goto <bb 31>; [59.00%]
  else
    goto <bb 58>; [41.00%]

  <bb 58> [local count: 7990250]:
  goto <bb 35>; [100.00%]

  <bb 31> [local count: 11498164]:
  __n_154 = pretmp_162 + -1;
```

That is only the second condition based on _42 was removed and not the first
...

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
Yeah, the issue is that PRE figures out a new value here but we've already done
value-numbering so we can't alter the "old" solution here.  So what we do
is add a '0' with value '_42' (instead of value '0').  This "second order"
value numbering leaves more opportunities on the plate.  It was IMHO
cleaner to insert a

  pretmp_163 = 0;

than substituting '0' everywhere but then leaving around the third order
optimizations in case we had _42 + 1 that would then simplify to sth = 1 ...

Previously we'd have inserted a degenerate PHI, now we get these kind of
copies.  "Now" is for a long time so this isn't new for PRE at least.

We could "hack" this by doing

diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
index 11061a374a2..effb4f4de73 100644
--- a/gcc/tree-ssa-sccvn.cc
+++ b/gcc/tree-ssa-sccvn.cc
@@ -6615,6 +6615,13 @@ eliminate_dom_walker::eliminate_push_avail (basic_block,
tree op)
       if (avail[SSA_NAME_VERSION (valnum)])
        pushop = avail[SSA_NAME_VERSION (valnum)];
       avail_stack.safe_push (pushop);
+      if (gassign *ass = dyn_cast <gassign *> (SSA_NAME_DEF_STMT (op)))
+       if (gimple_assign_rhs_class (ass) == GIMPLE_SINGLE_RHS)
+         {
+           tree rhs1 = gimple_assign_rhs1 (ass);
+           if (CONSTANT_CLASS_P (rhs1) || TREE_CODE (rhs1) == SSA_NAME)
+             op = rhs1;
+         }
       avail[SSA_NAME_VERSION (valnum)] = op;
     }
 }

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #5)
> Yeah, the issue is that PRE figures out a new value here but we've already
> done value-numbering so we can't alter the "old" solution here.  So what we
> do
> is add a '0' with value '_42' (instead of value '0').  This "second order"
> value numbering leaves more opportunities on the plate.  It was IMHO
> cleaner to insert a
> 
>   pretmp_163 = 0;
> 
> than substituting '0' everywhere but then leaving around the third order
> optimizations in case we had _42 + 1 that would then simplify to sth = 1 ...
> 
> Previously we'd have inserted a degenerate PHI, now we get these kind of
> copies.  "Now" is for a long time so this isn't new for PRE at least.
> 
> We could "hack" this by doing
> 
> diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
> index 11061a374a2..effb4f4de73 100644
> --- a/gcc/tree-ssa-sccvn.cc
> +++ b/gcc/tree-ssa-sccvn.cc
> @@ -6615,6 +6615,13 @@ eliminate_dom_walker::eliminate_push_avail
> (basic_block, tree op)
>        if (avail[SSA_NAME_VERSION (valnum)])
>         pushop = avail[SSA_NAME_VERSION (valnum)];
>        avail_stack.safe_push (pushop);
> +      if (gassign *ass = dyn_cast <gassign *> (SSA_NAME_DEF_STMT (op)))
> +       if (gimple_assign_rhs_class (ass) == GIMPLE_SINGLE_RHS)
> +         {
> +           tree rhs1 = gimple_assign_rhs1 (ass);
> +           if (CONSTANT_CLASS_P (rhs1) || TREE_CODE (rhs1) == SSA_NAME)
> +             op = rhs1;
> +         }
>        avail[SSA_NAME_VERSION (valnum)] = op;
>      }
>  }

Ah, that breaks the avail stack handling.  We can do the trick in
eliminate_avail though where it's more expensive or make the avail
stack more complicated.

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:d0de3bf9175877d6c51c94fe04662c6e031876e1

commit r14-2674-gd0de3bf9175877d6c51c94fe04662c6e031876e1
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Jul 17 12:15:29 2023 +0200

    tree-optimization/110204 - second level redundancy and simplification

    When PRE discovers a full redundancy during insertion it cannot unite
    the two value sets.  Instead it inserts a copy old-val = new-val where
    new-val can also be a constant.  The following looks through such
    copies during elimination, providing one extra level of constant and
    copy propagation.  For the PR this helps avoiding a bogus diagnostic
    that's emitted on unreachable code during loop optimization.

            PR tree-optimization/110204
            * tree-ssa-sccvn.cc (eliminate_dom_walker::eliminate_avail):
            Look through copies generated by PRE.

[Bug tree-optimization/110204] [14 Regression] Suspicous warning when compiling ranges-v3 using GCC trunk (iteration 9223372036854775807 invokes undefined behavior)

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110204

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed by better optimizing this.