[Bug tree-optimization/113831] New: [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398-g6b9fc1782effc67dd9f6def16207653d79647553

[Bug tree-optimization/113831] New: [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398-g6b9fc1782effc67dd9f6def16207653d79647553

[rguenth at gcc dot gnu.org]

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

            Bug ID: 113831
           Summary: [11/12/13/14 Regression] Wrong VN with structurally
                    identical ref since
                    r9-398-g6b9fc1782effc67dd9f6def16207653d79647553
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: rguenth at gcc dot gnu.org
  Target Milestone: ---

The following is miscompiled by FRE with -O2

int a[3];
int __attribute__((noipa))
foo(int i, int x)
{
  int tem = 0;
  a[2] = x;
  if (i < 1)
    ++i;
  else
    {
      ++i;
      tem = a[i];
    }
  tem += a[i];
  return tem;
}

int main() { if (foo (0, 7) != 0) __builtin_abort(); }

[Bug tree-optimization/113831] [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398-g6b9fc1782effc67dd9f6def16207653d79647553

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |ASSIGNED
           Keywords|                            |wrong-code
   Target Milestone|---                         |11.5
   Last reconfirmed|                            |2024-02-08

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Mine.

[Bug tree-optimization/113831] [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398-g6b9fc1782effc67dd9f6def16207653d79647553

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think the issue is that we're using range info for get_ref_base_and_extent
but we fail to do so when valueizing refs.

[Bug tree-optimization/113831] [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13/14 Regression]    |[11/12/13/14 Regression]
                   |Wrong VN with structurally  |Wrong VN with structurally
                   |identical ref since         |identical ref since r9-398
                   |r9-398-g6b9fc1782effc67dd9f |
                   |6def16207653d79647553       |
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Shortening the summary.
Started with r9-398-g6b9fc1782effc67dd9f6def16207653d79647553.
Originally from PR113774.

[Bug tree-optimization/113831] [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=108355

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
The related bug might be also fixed then.

[Bug tree-optimization/113831] [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398

[rguenth at gcc dot gnu.org]

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

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
So we have equal vn_reference but with different ao_ref.  Note the recorded
vn_reference has value-numbers in operands (not sanitized via AVAIL to a
specific location) but the ao_ref is eventually initialized from
get_ref_base_and_extent on the original ref which can use context sensitive
info.  That doesn't actually compute a constant array index from a variable
one but instead it constrains the extend of the access which eventually
gets to max_size == size.

To apply the same logic consistently to the VN representation (which is
eventually valueized) we can only look at ranges on names either from the
original ref (during copy_reference_ops_from_ref) or when valueizing with
AVAIL in mind.  For consistency operating from copy_reference_ops_from_ref
would be preferred.

It's going to be quite sophisticated to reverse-engineer all constant
array indexes from the overall [offset, offset + size] computed by
get_ref_base_and_extent (we definitely want to do that only once per
copy_reference_ops_from_ref).  For PRE we do need all the components,
so we have to somehow post-process the vn_reference ops.

The other possibility for a fix would be to try to fend off ranges being
used by get_ref_base_and_extent (but only for the calls on the refs
we're going to insert into the expression hash table).  get_range_query
cannot be tricked so it would be an extra arg to get_ref_base_and_extent
and possibly ao_ref_init.  That sounds a bit ugly.

I will try to implement the post-processing.

[Bug tree-optimization/113831] [11/12/13/14 Regression] Wrong VN with structurally identical ref since r9-398

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from GCC 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:938a419182f8c43bd1212ffb98f8aa6077cf8326

commit r14-8929-g938a419182f8c43bd1212ffb98f8aa6077cf8326
Author: Richard Biener <rguenther@suse.de>
Date:   Fri Feb 9 10:16:38 2024 +0100

    tree-optimization/113831 - wrong VN with structurally identical ref

    When we use get_ref_base_and_extent during VN and that ends up using
    global ranges to restrict the range of a ref we have to take care
    of not using the same expression in the hashtable as for a ref that
    could not use that global range.  The following attempts to ensure
    this by applying similar logic as get_ref_base_and_extent to
    copy_reference_ops_from_ref so they behave consistent.

            PR tree-optimization/113831
            PR tree-optimization/108355
            * tree-ssa-sccvn.cc (copy_reference_ops_from_ref): When
            we see variable array indices and get_ref_base_and_extent
            can resolve those to constants fix up the ops to constants
            as well.
            (ao_ref_init_from_vn_reference): Use 'off' member for
            ARRAY_REF and ARRAY_RANGE_REF instead of recomputing it.
            (valueize_refs_1): Also fixup 'off' of ARRAY_RANGE_REF.

            * gcc.dg/torture/pr113831.c: New testcase.
            * gcc.dg/tree-ssa/ssa-fre-104.c: Likewise.

[Bug tree-optimization/113831] [11/12/13 Regression] Wrong VN with structurally identical ref since r9-398

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |14.0
            Summary|[11/12/13/14 Regression]    |[11/12/13 Regression] Wrong
                   |Wrong VN with structurally  |VN with structurally
                   |identical ref since r9-398  |identical ref since r9-398

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.

[Bug tree-optimization/113831] [11/12/13 Regression] Wrong VN with structurally identical ref since r9-398

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from GCC 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:743577e36de66a082d329f71877789599f3ee3b5

commit r14-8954-g743577e36de66a082d329f71877789599f3ee3b5
Author: Richard Biener <rguenther@suse.de>
Date:   Tue Feb 13 12:55:19 2024 +0100

    Fix comment typo in ao_ref_init_from_vn_reference

            PR tree-optimization/113831
            * tree-ssa-sccvn.cc (ao_ref_init_from_vn_reference): Fix
            typo in comment.

[Bug tree-optimization/113831] [11/12/13 Regression] Wrong VN with structurally identical ref since r9-398

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from GCC 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:5c01ede02a1f9ba1a58ab8d96a73e46e0484d820

commit r14-9216-g5c01ede02a1f9ba1a58ab8d96a73e46e0484d820
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Feb 28 13:45:57 2024 +0100

    tree-optimization/113831 - revert original fix

    This reverts the original fix for PR113831 which is better fixed by
    the PR114121 fix.  I've XFAILed instead of removing the PR108355
    testcase again.

            PR tree-optimization/113831
            PR tree-optimization/108355
            * tree-ssa-sccvn.cc (copy_reference_ops_from_ref): Revert
            PR113831 fix.

            * gcc.dg/tree-ssa/ssa-fre-104.c: XFAIL.

[Bug tree-optimization/113831] [11/12/13 Regression] Wrong VN with structurally identical ref since r9-398

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:14a16787d99831a28b0c9690e80c420d765ba26f

commit r13-8702-g14a16787d99831a28b0c9690e80c420d765ba26f
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Feb 28 12:37:07 2024 +0100

    tree-optimization/114121 - wrong VN with context sensitive range info

    When VN ends up exploiting range-info specifying the ao_ref offset
    and max_size we have to make sure to reflect this in the hashtable
    entry for the recorded expression.  The PR113831 fix handled the
    case where we can encode this in the operands themselves but this
    bug shows the issue is more widespread.

    So instead of altering the operands the following instead records
    this extra info that's possibly used, only throwing it away when
    the value-numbering didn't come up with a non-VARYING value which
    is an important detail to preserve CSE as opposed to constant
    folding which is where all cases currently known popped up.

    With this the original PR113831 fix can be reverted.

            PR tree-optimization/113831
            PR tree-optimization/114121
            * tree-ssa-sccvn.h (vn_reference_s::offset,
            vn_reference_s::max_size): New fields.
            (vn_reference_insert_pieces): Adjust prototype.
            * tree-ssa-pre.cc (phi_translate_1): Preserve offset/max_size.
            * tree-ssa-sccvn.cc (vn_reference_eq): Compare offset and
            size, allow using "don't know" state.
            (vn_walk_cb_data::finish): Pass along offset/max_size.
            (vn_reference_lookup_or_insert_for_pieces): Take offset and
            max_size as argument and use it.
            (vn_reference_lookup_3): Properly adjust offset and max_size
            according to the adjusted ao_ref.
            (vn_reference_lookup_pieces): Initialize offset and max_size.
            (vn_reference_lookup): Likewise.
            (vn_reference_lookup_call): Likewise.
            (vn_reference_insert): Likewise.
            (visit_reference_op_call): Likewise.
            (vn_reference_insert_pieces): Take offset and max_size
            as argument and use it.

            * gcc.dg/torture/pr113831.c: New testcase.

    (cherry picked from commit c841144a94363ff26e40ab3f26b14702c32987a8)