[Bug tree-optimization/53465] New: [4.7 regression] wrong code with -O1 -ftree-vrp

[Bug tree-optimization/53465] New: [4.7 regression] wrong code with -O1 -ftree-vrp

[luto at mit dot edu]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

             Bug #: 53465
           Summary: [4.7 regression] wrong code with -O1 -ftree-vrp
    Classification: Unclassified
           Product: gcc
           Version: 4.7.1
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: luto@mit.edu


This code aborts with -O1 -ftree-vrp.  It shouldn't.  gcc 4.6 is okay;
4_7-branch from today and trunk are both bad.

extern void abort();

static const char input[] = {1,2};

void test(char const *data, int len)
{
  int i;
  int found_x = 0;
  char prev_x;
  for(i = 0; i < len; i++) {
    char x = data[i];

    if (x == 0)
      break;

    if (found_x && x <= prev_x)
      abort();

    prev_x = x;
    found_x = 1;
  }
}

int main()
{
  test(input, 2);
  return 1;
}

Changing char x to volatile char x or initializing prev_x before the loop will
prevent the crash.

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-05-24
                 CC|                            |rguenth at gcc dot gnu.org
   Target Milestone|---                         |4.7.1
     Ever Confirmed|0                           |1

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> 2012-05-24 02:47:33 UTC ---
It is caused by revision 176918:

http://gcc.gnu.org/ml/gcc-cvs/2011-07/msg01186.html

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

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

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

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 07:21:28 UTC ---
We have
  x_9 = *D.1724_8;
...
  x_16 = ASSERT_EXPR <x_9, x_9 != 0>;
  D.1726_10 = found_x_2 != 0;
  D.1727_11 = x_16 <= prev_x_3;
...
  # found_x_2 = PHI <0(2), 1(6)>
  # prev_x_3 = PHI <prev_x_4(D)(2), x_16(6)>
...
  loop to top

prev_x_3: ~[0, 0]  EQUIVALENCES: { x_9 } (1 elements)
prev_x_4(D): UNDEFINED
x_9: VARYING
x_16: ~[0, 0]  EQUIVALENCES: { x_9 } (1 elements)

I think the problem are the equivalences, or their handling.  When we merge on
PHI UNDEFINED with ~[0, 0] EQUIVALENCES: { x_9 } into the latter, we push the
equivalence into the next loop iteration where x_9 already can have a different
value.  I guess either we could drop all equivalences when merging UNDEFINED
with some range with EQUIVALENCES, or at least those equivalences which don't
dominate the PHI.  Richard, what do you think?

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> 2012-05-24 07:29:52 UTC ---
On Thu, 24 May 2012, jakub at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465
> 
> Jakub Jelinek <jakub at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |jakub at gcc dot gnu.org
> 
> --- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 07:21:28 UTC ---
> We have
>   x_9 = *D.1724_8;
> ...
>   x_16 = ASSERT_EXPR <x_9, x_9 != 0>;
>   D.1726_10 = found_x_2 != 0;
>   D.1727_11 = x_16 <= prev_x_3;
> ...
>   # found_x_2 = PHI <0(2), 1(6)>
>   # prev_x_3 = PHI <prev_x_4(D)(2), x_16(6)>
> ...
>   loop to top
> 
> prev_x_3: ~[0, 0]  EQUIVALENCES: { x_9 } (1 elements)
> prev_x_4(D): UNDEFINED
> x_9: VARYING
> x_16: ~[0, 0]  EQUIVALENCES: { x_9 } (1 elements)
> 
> I think the problem are the equivalences, or their handling.  When we merge on
> PHI UNDEFINED with ~[0, 0] EQUIVALENCES: { x_9 } into the latter, we push the
> equivalence into the next loop iteration where x_9 already can have a different
> value.  I guess either we could drop all equivalences when merging UNDEFINED
> with some range with EQUIVALENCES, or at least those equivalences which don't
> dominate the PHI.  Richard, what do you think?

Yes, I think we need to clear all equivalences in vrp_meet

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 07:42:26 UTC ---
Dropping all might pessimize some code (of course, this is VR_UNDEFINED, so the
question is if we need to care that much about broken code), like:
int
foo (int x, int y)
{
  int z;
  if (x <= 3)
    return 7;
  if (y)
    z = x;
  return z == x;
}

where the equivalence could be very well kept.  Or
int
foo (int y)
{
  int z;
  int x = bar ();
  if (x <= 3)
    return 7;
  if (y)
    z = x;
  return z == x;
}

In both cases the x_3 in the equivalence bitmap dominates the PHI stmt (in one
case it is the default parm def, in the second case not).  vrp_meet would
probably need to be called with gimple stmt in that case on which the meet
happens.

Anyway, if you think it is overkill, we can just drop the equivalences
altogether.

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[rguenther at suse dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

--- Comment #5 from rguenther at suse dot de <rguenther at suse dot de> 2012-05-24 07:57:58 UTC ---
On Thu, 24 May 2012, jakub at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465
> 
> --- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 07:42:26 UTC ---
> Dropping all might pessimize some code (of course, this is VR_UNDEFINED, so the
> question is if we need to care that much about broken code), like:
> int
> foo (int x, int y)
> {
>   int z;
>   if (x <= 3)
>     return 7;
>   if (y)
>     z = x;
>   return z == x;
> }
> 
> where the equivalence could be very well kept.  Or
> int
> foo (int y)
> {
>   int z;
>   int x = bar ();
>   if (x <= 3)
>     return 7;
>   if (y)
>     z = x;
>   return z == x;
> }
> 
> In both cases the x_3 in the equivalence bitmap dominates the PHI stmt (in one
> case it is the default parm def, in the second case not).  vrp_meet would
> probably need to be called with gimple stmt in that case on which the meet
> happens.
> 
> Anyway, if you think it is overkill, we can just drop the equivalences
> altogether.

I indeed think this is overkill, definitely for the 4.7 branch.

Richard.

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |jakub at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 08:42:00 UTC ---
Created attachment 27485
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27485
gcc48-pr53465.patch

Untested fix.

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 11:51:15 UTC ---
Author: jakub
Date: Thu May 24 11:51:09 2012
New Revision: 187827

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187827
Log:
    PR tree-optimization/53465
    * tree-vrp.c (extract_range_from_cond_expr): First copy_value_range
    vr0 into *vr, then vrp_meet that.
    (vrp_meet): If one vr type is VR_UNDEFINED, ensure the result doesn't
    have any equivalences.
    (vrp_visit_phi_node): Call copy_value_range instead of vrp_meet the
    first time.

    * gcc.c-torture/execute/pr53465.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr53465.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vrp.c

[Bug tree-optimization/53465] [4.7/4.8 Regression] wrong code with -O1 -ftree-vrp

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
            Summary|[4.7 regression] wrong code |[4.7/4.8 Regression] wrong
                   |with -O1 -ftree-vrp         |code with -O1 -ftree-vrp

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 11:53:33 UTC ---
Author: jakub
Date: Thu May 24 11:53:29 2012
New Revision: 187828

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187828
Log:
    PR tree-optimization/53465
    * tree-vrp.c (extract_range_from_cond_expr): First copy_value_range
    vr0 into *vr, then vrp_meet that.
    (vrp_meet): If one vr type is VR_UNDEFINED, ensure the result doesn't
    have any equivalences.
    (vrp_visit_phi_node): Call copy_value_range instead of vrp_meet the
    first time.

    * gcc.c-torture/execute/pr53465.c: New test.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.c-torture/execute/pr53465.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-vrp.c

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 11:54:30 UTC ---
Fixed.

[Bug tree-optimization/53465] [4.7 regression] wrong code with -O1 -ftree-vrp

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 11:53:33 UTC ---
Author: jakub
Date: Thu May 24 11:53:29 2012
New Revision: 187828

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187828
Log:
    PR tree-optimization/53465
    * tree-vrp.c (extract_range_from_cond_expr): First copy_value_range
    vr0 into *vr, then vrp_meet that.
    (vrp_meet): If one vr type is VR_UNDEFINED, ensure the result doesn't
    have any equivalences.
    (vrp_visit_phi_node): Call copy_value_range instead of vrp_meet the
    first time.

    * gcc.c-torture/execute/pr53465.c: New test.

Added:
    branches/gcc-4_7-branch/gcc/testsuite/gcc.c-torture/execute/pr53465.c
Modified:
    branches/gcc-4_7-branch/gcc/ChangeLog
    branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_7-branch/gcc/tree-vrp.c

[Bug tree-optimization/53465] [4.7/4.8 Regression] wrong code with -O1 -ftree-vrp

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
            Summary|[4.7 regression] wrong code |[4.7/4.8 Regression] wrong
                   |with -O1 -ftree-vrp         |code with -O1 -ftree-vrp

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-05-24 11:54:30 UTC ---
Fixed.

[Bug tree-optimization/53465] [4.7/4.8 Regression] wrong code with -O1 -ftree-vrp

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53465

--- Comment #10 from Richard Biener <rguenth at gcc dot gnu.org> 2013-01-16 13:57:54 UTC ---
Author: rguenth
Date: Wed Jan 16 13:57:48 2013
New Revision: 195238

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195238
Log:
2013-01-16  Richard Biener  <rguenther@suse.de>

    PR tree-optimization/54767
    PR tree-optimization/53465
    * tree-vrp.c (vrp_meet_1): Revert original fix for PR53465.
    (vrp_visit_phi_node): For PHI arguments coming via backedges
    drop all symbolical range information.
    (execute_vrp): Compute backedges.

    * gfortran.fortran-torture/execute/pr54767.f90: New testcase.

Added:
    trunk/gcc/testsuite/gfortran.fortran-torture/execute/pr54767.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vrp.c