Bug 54767 - [4.7 Regression] Incorrect code generated with "-O2 -fcheck=bounds"
Summary: [4.7 Regression] Incorrect code generated with "-O2 -fcheck=bounds"
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.7.1
: P2 normal
Target Milestone: 4.7.3
Assignee: Richard Biener
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-01 15:07 UTC by Arnaud Desitter
Modified: 2013-02-05 12:54 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.4.6, 4.5.3, 4.6.3, 4.8.0
Known to fail: 4.7.1, 4.7.2
Last reconfirmed: 2012-10-01 00:00:00


Attachments
test case (852 bytes, application/octet-stream)
2012-10-01 15:08 UTC, Arnaud Desitter
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arnaud Desitter 2012-10-01 15:07:48 UTC
Using the code attached:

>gfortran --version | head -2
GNU Fortran (GCC) 4.7.1
Copyright (C) 2012 Free Software Foundation, Inc.
>gfortran -O0 -fcheck=bounds xxx_1.f xxx_2.f
>/a.out
           2           1
           3           2
 T
>gfortran -O2 -fcheck=bounds xxx_1.f xxx_2.f
>./a.out
           2           1
           3           1
 F

The results are different and incorrect when compiling with "-O2 -fcheck=bounds". They are still correct when compiling with "-O1 -fcheck=bounds".
Comment 1 Arnaud Desitter 2012-10-01 15:08:57 UTC
Created attachment 28313 [details]
test case
Comment 2 Dominique d'Humieres 2012-10-01 17:45:19 UTC
This seems to be a 4.7/4.8 regression. Compiling the test with ' -Wall -Wextra -Wuninitialized -O3 -fbounds-check' gives the expected results for 4.4.6, 4.5.3, and 4.6.3, but not for 4.7.2 or trunk.
Revision 176696 (2011-07-23) is OK, but not revision 177649 (2011-08-11).

If I initialize iaii as iaii=huge(0), I get the expected result.

Note to be pedantic, the use of INTENT in the subroutine requires an interface (but it does not change anything either to add the interface or to remove the intent).
Comment 3 Tobias Burnus 2012-10-01 18:01:26 UTC
Confirmed.

For the failure, it is sufficient to use:
  gfortran -fcheck=bounds -O1 -fstrict-overflow -ftree-pre -ftree-vrp xxx_2.f

(xxx_1.f can be compiled with any option.)
Comment 4 janus 2012-10-01 18:22:40 UTC
Here is a reduced test case, which shows the same behavior:


  SUBROUTINE XXX (IL, IU)
    implicit none
    integer, INTENT(IN) :: IL, IU
    
    integer :: NXX (3) = (/ 0, 1, 2 /)
    integer :: ivvv, ia, iyyye, ja, iaii
    logical :: qop, qvv

    IYYYE=-1
    QOP=.FALSE.

    DO IA=IL,IU
    
      JA=NXX(IA)
      QVV=JA.GT.0
      IF (.NOT. QOP .and. QVV) THEN
	IAII=IA
	ivvv=1
	QOP=.TRUE.
      ENDIF

      IF (QOP) THEN
	IF (QVV) THEN
	  ivvv=IA-IAII+1       ! mis-compiled
	  write(*,*) ia,ivvv
	ELSE IF (IA+IYYYE<1) THEN
	  QOP=.FALSE.
	  IYYYE=0
	ENDIF
      ENDIF

    ENDDO

    write(*,*) ivvv==2
    
  END subroutine

  
program p
  implicit none
  CALL XXX (1, 3)
end
Comment 5 Mikael Morin 2013-01-14 21:01:34 UTC
Workaround: -fno-tree-vrp
Comment 6 H.J. Lu 2013-01-14 21:46:17 UTC
It is caused by revision 176918:

http://gcc.gnu.org/ml/gcc-cvs/2011-07/msg01186.html
Comment 7 Jakub Jelinek 2013-01-15 10:47:36 UTC
Yeah, indeed.  The bug seems to be in the second vrp pass on xxx function.
iaii_23: [_79, _79]  EQUIVALENCES: { } (0 elements)
looks wrong (iaii_23 is only conditionally equal to _79 (in the first iteration that sets iaii, i.e. ia = 2, second iteration), it can be the value from previous cycle too (in the ia = 3, third iteration).
This then leads to
  _35 = prephitmp_50 - iaii_23;
being
_35: [0, 0]
(again, wrong) and that is why we end up with 1 instead of 2 in the second row.

There is:
Visiting PHI node: iaii_23 = PHI <iaii_1(11), prephitmp_50(10)>

    Argument #0 (11 -> 12 not executable)
    
    Argument #1 (10 -> 12 executable)
        prephitmp_50
        Value: [_79, _79]  EQUIVALENCES: { _79 } (1 elements)
Found new range for iaii_23: [_79, _79]

which is wrong, but not sure if this is the first spot in the dump.  iaii_1 here is only conditionally UNDEFINED, it might be iaii value from previous iteration too.
Comment 8 Richard Biener 2013-01-15 10:49:49 UTC
I don't see anything wrong by what VRP does:

Visiting statement:
prephitmp_2 = _79;

Found new range for prephitmp_2: [_79, _79]

Simulating statement (from ssa_edges): iaii_23 = PHI <iaii_1(11), prephitmp_2(10)>

Visiting PHI node: iaii_23 = PHI <iaii_1(11), prephitmp_2(10)>

    Argument #0 (11 -> 12 executable)
        iaii_1
        Value: [_79, _79]

    Argument #1 (10 -> 12 executable)
        prephitmp_2
        Value: [_79, _79]  EQUIVALENCES: { _79 } (1 elements)
Meeting
  [_79, _79]
and
  [_79, _79]  EQUIVALENCES: { _79 } (1 elements)
to
  [_79, _79]
Found new range for iaii_23: [_79, _79]

Simulating statement (from ssa_edges): _35 = prephitmp_2 - iaii_23;

Visiting statement:
_35 = prephitmp_2 - iaii_23;

Found new range for _35: [0, 0]

Visiting statement:
ivvv.5_36 = _35 + 1;

Found new range for ivvv.5_36: [1, 1]


Now changed behavior is from:

Visiting PHI node: iaii_1 = PHI <iaii_18(D)(4), iaii_48(17)>

    Argument #0 (4 -> 5 executable)
        iaii_18(D)
        Value: UNDEFINED

    Argument #1 (17 -> 5 executable)
        iaii_48
        Value: [_79, _79]
Meeting
  UNDEFINED
and
  [_79, _79]
to
  [_79, _79]
Found new range for iaii_1: [_79, _79]

and this is all in a cycle.


I think this may be mixing _79 from different iterations.  Reduced testcase,
maybe easier to look at:

SUBROUTINE XXX (IL, IU)
  implicit none
  integer, INTENT(IN) :: IL, IU

  integer :: NXX (3) = (/ 0, 1, 2 /)
  integer :: ivvv, ia, ja, iaii
  logical :: qop

  QOP=.FALSE.

  DO IA=IL,IU
    JA=NXX(IA)
    IF (.NOT. QOP .and. JA.GT.0) THEN
      IAII=IA
      QOP=.TRUE.
    ENDIF

    IF (QOP) THEN
      ivvv=IA-IAII+1       ! mis-compiled
    ENDIF
  ENDDO

  IF (ivvv.NE.2) THEN
    call abort
  ENDIF
END subroutine

program p
  implicit none
  CALL XXX (1, 3)
end
Comment 9 Richard Biener 2013-01-15 11:28:48 UTC
Exactly the same issue as PR53465, just instead of an equivalence we have
a symbolic range [_79, _79] which is of course also an "equivalence".

Trunk fails for the reduced testcase if you enlarge the array:

  integer :: NXX (14) = (/ 0, 1, 2, 3, 4, 5, 6, 7, 9, 10, 11, 12, 13, 14 /)

otherwise the loop is unrolled.
Comment 10 Richard Biener 2013-01-15 11:36:17 UTC
Struggling to build a C testcase with the bounds checking made explicit.
Comment 11 Richard Biener 2013-01-16 13:57:53 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
Comment 12 Richard Biener 2013-01-16 15:23:06 UTC
Fixed on trunk sofar.
Comment 13 Richard Biener 2013-02-05 12:54:17 UTC
Author: rguenth
Date: Tue Feb  5 12:54:12 2013
New Revision: 195754

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

	PR tree-optimization/54767
	* tree-vrp.c (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:
    branches/gcc-4_7-branch/gcc/testsuite/gfortran.fortran-torture/execute/pr54767.f90
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 14 Richard Biener 2013-02-05 12:54:41 UTC
Fixed.