[Bug tree-optimization/96522] New: Incorrect with with -O -fno-tree-pta

[Bug tree-optimization/96522] New: Incorrect with with -O -fno-tree-pta

[trt at alumni dot duke.edu]

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

            Bug ID: 96522
           Summary: Incorrect with with -O -fno-tree-pta
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: trt at alumni dot duke.edu
  Target Milestone: ---

Created attachment 49020
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49020&action=edit
demo of flaw in no-tree-pta

The attached program t.c, when compiled with

# gcc (GCC) 10.2.0
gcc t.c -O -fno-tree-pta

has this output:

I think p is non-NULL
I think p is non-NULL

But it is obvious that p is NULL

[Bug tree-optimization/96522] [9/10/11 Regression] Incorrect with with -O -fno-tree-pta

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |9.4
             Status|UNCONFIRMED                 |NEW
                 CC|                            |jakub at gcc dot gnu.org
     Ever confirmed|0                           |1
            Summary|Incorrect with with -O      |[9/10/11 Regression]
                   |-fno-tree-pta               |Incorrect with with -O
                   |                            |-fno-tree-pta
   Last reconfirmed|                            |2020-08-07

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r263662 aka r9-2475-g4864297f7858617a5fe406d3651a46446b41db7a

[Bug tree-optimization/96522] [9/10/11 Regression] Incorrect with with -O -fno-tree-pta

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Slightly adjusted testcase that aborts if miscompiled:
/* { dg-do run } */
/* { dg-options "-O -fno-tree-pta" } */

__attribute__((noipa)) void
bar (void)
{
  volatile int v = 1;
  if (v)
    __builtin_abort ();
}

__attribute__((noipa)) void
baz (void)
{
}

__attribute__((noipa)) void
foo (int n, double *p, double *x)
{
  if (n < 10 && p != 0)
    for (int i = 0; i < 10; i++)
      if (x[0] < p[i])
        x[i] = 0;
  if (p != 0)
    bar ();
  else
    baz ();
}

int
main ()
{
  double arr[10];
  foo (1000, 0, arr);
  return 0;
}

Note, with -O1 neither evrp nor vrp{1,2} are done, but this happens during
dom3.

What I see happening is that we have
  _7 = p_16(D) + _6;
in the IL and as it is used in a block guarded with p != 0, we determine (but
don't store for the p_16(D) SSA_NAME that p_16(D) at that point must be
non-NULL and so (IMO correctly) set _7 to be non-NULL in the vr info as well as
SSA_NAME_PTR_INFO.  The problem is that during ivopts rewrite_use_address calls
copy_ref_info when copying ref info from *_7 to a TARGET_MEM_REF
MEM[base: p_16(D), index: ivtmp.17_24, offset: 0B] and that in turn will do:
1018      /* We can transfer points-to information from an old pointer
1019         or decl base to the new one.  */
1020      if (new_ptr_base
1021          && TREE_CODE (new_ptr_base) == SSA_NAME
1022          && !SSA_NAME_PTR_INFO (new_ptr_base))
I bet the intent is that if new_ptr_base has been a newly created SSA_NAME,
then it is ok to copy the info (maybe), and for other cases without the
-fno-tree-pta the existing SSA_NAMEs will likely have SSA_NAME_PTR_INFO
non-NULL.
Anyway, even if it would be ok to copy the points-to info that way, I think it
isn't ok to copy whether the pointer can be NULL or not.
So, either the condition should include && flag_tree_pta, or at least do
reset_flow_sensitive_info (new_ptr_info).  Or maybe callers of copy_ref_info
should pass in a flag whether the new_ptr_info is a freshly created SSA_NAME
(in that case like e.g. in slsr it is probably ok to do what it does ATM), or
not (and then either not do it at all, or do the reset_flow_sensitive_info on
it afterwards, thus forget alignment and non-NULL info that could be only in a
particular region and not valid elsewhere.

[Bug tree-optimization/96522] [9/10/11 Regression] Incorrect with with -O -fno-tree-pta

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug tree-optimization/96522] [9/10/11 Regression] Incorrect with with -O -fno-tree-pta

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
Let me take this.

[Bug tree-optimization/96522] [9/10/11 Regression] Incorrect with with -O -fno-tree-pta

[rguenth at gcc dot gnu.org]

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

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 49137
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49137&action=edit
patch

The same holds true for copying alignment.  I don't think SLSR is "safe" here
since it also picks a new base from possibly outside of a controlling stmt.
But obviously transfering alignment is important - but we could implement
this by adjusting the access type instead.  I'm quite sure non-nullness
isn't so important.  Now, misalignment info isn't easy to transfer in the
generic routine.

I'm going to test the attached.  [for backporting the MR_DEPENDENCE hunk
can be stripped]

[Bug tree-optimization/96522] [9/10/11 Regression] Incorrect with with -O -fno-tree-pta

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 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:eb68d9d828f94d28afa5900fbf3072bbcd64ba8a

commit r11-2906-geb68d9d828f94d28afa5900fbf3072bbcd64ba8a
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Aug 27 11:48:15 2020 +0200

    tree-optimization/96522 - transfer of flow-sensitive info in copy_ref_info

    This removes the bogus tranfer of flow-sensitive info in copy_ref_info
    plus fixes one oversight in FRE when flow-sensitive non-NULLness was added
to
    points-to info.

    2020-08-27  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/96522
            * tree-ssa-address.c (copy_ref_info): Reset flow-sensitive
            info of the copied points-to.  Transfer bigger alignment
            via the access type.
            * tree-ssa-sccvn.c (eliminate_dom_walker::eliminate_stmt):
            Reset all flow-sensitive info.

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

[Bug tree-optimization/96522] [9/10 Regression] Incorrect with with -O -fno-tree-pta

[rguenth at gcc dot gnu.org]

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

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Hmm, picking the ref onto gcc-10 causes

FAIL: gcc.dg/vect/pr81410.c execution test

due to an unaligned access.

[Bug tree-optimization/96522] [9/10 Regression] Incorrect with with -O -fno-tree-pta

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #6)
> Hmm, picking the ref onto gcc-10 causes
> 
> FAIL: gcc.dg/vect/pr81410.c execution test
> 
> due to an unaligned access.

Looks like a latent issue.  The following on an unpatched tree is enough
to provoke the FAIL (but ptr-info should not affect correctness).

diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 9ace345fc5e..ea32b70a4f3 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -9565,10 +9567,12 @@ vectorizable_load (stmt_vec_info stmt_info,
gimple_stmt_iterator *gsi,
                      }
                    else
                      misalign = DR_MISALIGNMENT (first_dr_info);
+#if 0
                    if (dataref_offset == NULL_TREE
                        && TREE_CODE (dataref_ptr) == SSA_NAME)
                      set_ptr_info_alignment (get_ptr_info (dataref_ptr),
                                              align, misalign);
+#endif

                    if (final_mask)
                      {

[Bug tree-optimization/96522] [9/10 Regression] Incorrect with with -O -fno-tree-pta

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:1dbb919d0868319a5503b91049283a189ac1b4ac

commit r10-8760-g1dbb919d0868319a5503b91049283a189ac1b4ac
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Aug 27 11:48:15 2020 +0200

    tree-optimization/96522 - transfer of flow-sensitive info in copy_ref_info

    This removes the bogus tranfer of flow-sensitive info in copy_ref_info
    plus fixes one oversight in FRE when flow-sensitive non-NULLness was added
to
    points-to info.

    2020-08-27  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/96522
            * tree-ssa-address.c (copy_ref_info): Reset flow-sensitive
            info of the copied points-to.  Transfer bigger alignment
            via the access type.
            * tree-ssa-sccvn.c (eliminate_dom_walker::eliminate_stmt):
            Reset all flow-sensitive info.

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

    (cherry picked from commit eb68d9d828f94d28afa5900fbf3072bbcd64ba8a)

[Bug tree-optimization/96522] [9 Regression] Incorrect with with -O -fno-tree-pta

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96522
Bug 96522 depends on bug 97043, which changed state.

Bug 97043 Summary: latent wrong-code with SLP vectorization
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97043

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

[Bug tree-optimization/96522] [9 Regression] Incorrect with with -O -fno-tree-pta

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.4                         |9.5

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.

[Bug tree-optimization/96522] [9 Regression] Incorrect with with -O -fno-tree-pta

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:308de43fe48321588dca9830a617b92441779429

commit r9-9958-g308de43fe48321588dca9830a617b92441779429
Author: Richard Biener <rguenther@suse.de>
Date:   Thu Aug 27 11:48:15 2020 +0200

    tree-optimization/96522 - transfer of flow-sensitive info in copy_ref_info

    This removes the bogus tranfer of flow-sensitive info in copy_ref_info
    plus fixes one oversight in FRE when flow-sensitive non-NULLness was added
to
    points-to info.

    2020-08-27  Richard Biener  <rguenther@suse.de>

            PR tree-optimization/96522
            * tree-ssa-address.c (copy_ref_info): Reset flow-sensitive
            info of the copied points-to.  Transfer bigger alignment
            via the access type.
            * tree-ssa-sccvn.c (eliminate_dom_walker::eliminate_stmt):
            Reset all flow-sensitive info.

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

    (cherry picked from commit eb68d9d828f94d28afa5900fbf3072bbcd64ba8a)

[Bug tree-optimization/96522] [9 Regression] Incorrect with with -O -fno-tree-pta

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED
      Known to fail|                            |9.4.0
      Known to work|                            |9.4.1

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.