Re: Fix PR middle-end/57370

2013-09-09 Thread Richard Biener
On Fri, Sep 6, 2013 at 8:47 PM, Easwaran Raman era...@google.com wrote:
 On Fri, Sep 6, 2013 at 12:05 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Sep 5, 2013 at 9:23 PM, Easwaran Raman era...@google.com wrote:
 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.

 I looked into the problem related to the debug stmts in this failing
 test case in detail. Initially, the code sequence looks


 s$n_13 = MEM[(struct S *)s];
 # DEBUG s$n = s$n_13
 _2 = (double) s$n_13;
 _4 = _2 * a_3(D);
 _6 = (double) i_5(D);
 _7 = _4 * _6;
 _9 = (double) j_8(D);
 _10 = _7 * _9;
 bar (_10);
 # DEBUG D#2 = (double) k_12(D)
 # DEBUG D#1 = _7 * D#2
 # DEBUG t$n = (int) D#1

 After reassociation

 s$n_13 = MEM[(struct S *)s];
 # DEBUG s$n = s$n_13
 _2 = (double) s$n_13;
 _6 = (double) i_5(D);
 # DEBUG D#3 = _4 * _6
 _9 = (double) j_8(D);
 _4 = _9 * _2;
 _7 = _4 * a_3(D);
 _10 = _7 * _6;
 bar (_10);
 # DEBUG D#2 = (double) k_12(D)
 # DEBUG D#1 = D#3 * D#2
 # DEBUG t$n = (int) D#1

 In the above, # DEBUG D#3 = _4 * _6  appears above the statement that
 defines _4. But even if I move the def of D#3 below the statement
 defining _4, it is not sufficient. Before reassociation, t$n refers to
 the expression (s$n_13 * a_3(D) * i_5(D) * k_12(D)), but after
 reassociation it would refer to ( j_8(D) * s$n_13 *  i_5(D) *
 k_12(D)). It seems the correct fix is to discard the debug temps whose
 RHS contains a variable that is involved in reassociation and then
 reconstruct it. Is that the expected fix here?

 The value of the DEBUG expression changes because the value
 that _4 computes changes - that is a no-no that may not happen.
 We cannot re-use _4 (without previously releasing it and allocating
 it newly) with a new value.  releasing _4 should have fixed up the
 debug stmts.

 So - can you verify whether we are indeed just replacing the
 RHS of _4 = _2 * a_3(D) to _9 * _2 without changing the SSA name of
 that expression?

 I have confirmed that the SSA name of the LHS remains the same. The
 reassociation code simply calls gimple_assign_set_rhs2 to modify RHS2
 and then calls update_stmt.

That's definitely not allowed (though ISTR I remember that reassoc does
this kind of things :/) - it will result in wrong debug info.

Richard.

 - Easwaran

 Another reason why re-using the same LHS for another value is
 wrong is that annotated information like points-to sets, alignment
 and soon value-range information will be wrong.

 Thanks,
 Richard.

 - Easwaran


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com 
 wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of 
 a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   

Re: Fix PR middle-end/57370

2013-09-06 Thread Richard Biener
On Thu, Sep 5, 2013 at 9:23 PM, Easwaran Raman era...@google.com wrote:
 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.

 I looked into the problem related to the debug stmts in this failing
 test case in detail. Initially, the code sequence looks


 s$n_13 = MEM[(struct S *)s];
 # DEBUG s$n = s$n_13
 _2 = (double) s$n_13;
 _4 = _2 * a_3(D);
 _6 = (double) i_5(D);
 _7 = _4 * _6;
 _9 = (double) j_8(D);
 _10 = _7 * _9;
 bar (_10);
 # DEBUG D#2 = (double) k_12(D)
 # DEBUG D#1 = _7 * D#2
 # DEBUG t$n = (int) D#1

 After reassociation

 s$n_13 = MEM[(struct S *)s];
 # DEBUG s$n = s$n_13
 _2 = (double) s$n_13;
 _6 = (double) i_5(D);
 # DEBUG D#3 = _4 * _6
 _9 = (double) j_8(D);
 _4 = _9 * _2;
 _7 = _4 * a_3(D);
 _10 = _7 * _6;
 bar (_10);
 # DEBUG D#2 = (double) k_12(D)
 # DEBUG D#1 = D#3 * D#2
 # DEBUG t$n = (int) D#1

 In the above, # DEBUG D#3 = _4 * _6  appears above the statement that
 defines _4. But even if I move the def of D#3 below the statement
 defining _4, it is not sufficient. Before reassociation, t$n refers to
 the expression (s$n_13 * a_3(D) * i_5(D) * k_12(D)), but after
 reassociation it would refer to ( j_8(D) * s$n_13 *  i_5(D) *
 k_12(D)). It seems the correct fix is to discard the debug temps whose
 RHS contains a variable that is involved in reassociation and then
 reconstruct it. Is that the expected fix here?

The value of the DEBUG expression changes because the value
that _4 computes changes - that is a no-no that may not happen.
We cannot re-use _4 (without previously releasing it and allocating
it newly) with a new value.  releasing _4 should have fixed up the
debug stmts.

So - can you verify whether we are indeed just replacing the
RHS of _4 = _2 * a_3(D) to _9 * _2 without changing the SSA name of
that expression?

Another reason why re-using the same LHS for another value is
wrong is that annotated information like points-to sets, alignment
and soon value-range information will be wrong.

Thanks,
Richard.

 - Easwaran


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a 
 phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = 

Re: Fix PR middle-end/57370

2013-09-06 Thread Easwaran Raman
On Fri, Sep 6, 2013 at 12:05 AM, Richard Biener
richard.guent...@gmail.com wrote:
 On Thu, Sep 5, 2013 at 9:23 PM, Easwaran Raman era...@google.com wrote:
 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.

 I looked into the problem related to the debug stmts in this failing
 test case in detail. Initially, the code sequence looks


 s$n_13 = MEM[(struct S *)s];
 # DEBUG s$n = s$n_13
 _2 = (double) s$n_13;
 _4 = _2 * a_3(D);
 _6 = (double) i_5(D);
 _7 = _4 * _6;
 _9 = (double) j_8(D);
 _10 = _7 * _9;
 bar (_10);
 # DEBUG D#2 = (double) k_12(D)
 # DEBUG D#1 = _7 * D#2
 # DEBUG t$n = (int) D#1

 After reassociation

 s$n_13 = MEM[(struct S *)s];
 # DEBUG s$n = s$n_13
 _2 = (double) s$n_13;
 _6 = (double) i_5(D);
 # DEBUG D#3 = _4 * _6
 _9 = (double) j_8(D);
 _4 = _9 * _2;
 _7 = _4 * a_3(D);
 _10 = _7 * _6;
 bar (_10);
 # DEBUG D#2 = (double) k_12(D)
 # DEBUG D#1 = D#3 * D#2
 # DEBUG t$n = (int) D#1

 In the above, # DEBUG D#3 = _4 * _6  appears above the statement that
 defines _4. But even if I move the def of D#3 below the statement
 defining _4, it is not sufficient. Before reassociation, t$n refers to
 the expression (s$n_13 * a_3(D) * i_5(D) * k_12(D)), but after
 reassociation it would refer to ( j_8(D) * s$n_13 *  i_5(D) *
 k_12(D)). It seems the correct fix is to discard the debug temps whose
 RHS contains a variable that is involved in reassociation and then
 reconstruct it. Is that the expected fix here?

 The value of the DEBUG expression changes because the value
 that _4 computes changes - that is a no-no that may not happen.
 We cannot re-use _4 (without previously releasing it and allocating
 it newly) with a new value.  releasing _4 should have fixed up the
 debug stmts.

 So - can you verify whether we are indeed just replacing the
 RHS of _4 = _2 * a_3(D) to _9 * _2 without changing the SSA name of
 that expression?

I have confirmed that the SSA name of the LHS remains the same. The
reassociation code simply calls gimple_assign_set_rhs2 to modify RHS2
and then calls update_stmt.

- Easwaran

 Another reason why re-using the same LHS for another value is
 wrong is that annotated information like points-to sets, alignment
 and soon value-range information will be wrong.

 Thanks,
 Richard.

 - Easwaran


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com 
 wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of 
 a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +

Re: Fix PR middle-end/57370

2013-09-05 Thread Easwaran Raman
On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.

I looked into the problem related to the debug stmts in this failing
test case in detail. Initially, the code sequence looks


s$n_13 = MEM[(struct S *)s];
# DEBUG s$n = s$n_13
_2 = (double) s$n_13;
_4 = _2 * a_3(D);
_6 = (double) i_5(D);
_7 = _4 * _6;
_9 = (double) j_8(D);
_10 = _7 * _9;
bar (_10);
# DEBUG D#2 = (double) k_12(D)
# DEBUG D#1 = _7 * D#2
# DEBUG t$n = (int) D#1

After reassociation

s$n_13 = MEM[(struct S *)s];
# DEBUG s$n = s$n_13
_2 = (double) s$n_13;
_6 = (double) i_5(D);
# DEBUG D#3 = _4 * _6
_9 = (double) j_8(D);
_4 = _9 * _2;
_7 = _4 * a_3(D);
_10 = _7 * _6;
bar (_10);
# DEBUG D#2 = (double) k_12(D)
# DEBUG D#1 = D#3 * D#2
# DEBUG t$n = (int) D#1

In the above, # DEBUG D#3 = _4 * _6  appears above the statement that
defines _4. But even if I move the def of D#3 below the statement
defining _4, it is not sufficient. Before reassociation, t$n refers to
the expression (s$n_13 * a_3(D) * i_5(D) * k_12(D)), but after
reassociation it would refer to ( j_8(D) * s$n_13 *  i_5(D) *
k_12(D)). It seems the correct fix is to discard the debug temps whose
RHS contains a variable that is involved in reassociation and then
reconstruct it. Is that the expected fix here?

- Easwaran


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a 
 phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 +t5531 + t5533 + t5535 + t5537 + 
 +t5540
 +t5565 = t112 * t1685
 +t5575 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564 - 
 +0.80e1_dp * t5565 + t5568 + 

Re: Fix PR middle-end/57370

2013-09-04 Thread Easwaran Raman
Submitted the patch (r202262) without the insert_stmt_after hunk. Also
fixed nits pointed out by Jakub.

- Easwaran

On Mon, Sep 2, 2013 at 2:31 AM, Richard Biener
richard.guent...@gmail.com wrote:
 On Fri, Aug 30, 2013 at 6:13 PM, Easwaran Raman era...@google.com wrote:
 On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman era...@google.com wrote:
 Richard,
 Do you want me to commit everything but the  insert_stmt_after hunk
 now?

 Yes.

 There are more similar failures reported in
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
 the updated patch there. Shall I send that for review? Apart from the
 debug statement issue, almost all the bugs are due to dependence
 violation because certain newly inserted statements do not have the
 right UID. Instead of trying to catch all of them, will it be better
 if I check if the stmt has a proper uid (non-zero if it is not the
 first stmt) and assign a sensible value at the point where it is used
 (find_insert_point and appears_later_in_bb) instead of where the stmt
 is created? I think that would be less brittle.

 In the end all this placement stuff should be reverted and done as
 post-processing after reassoc is finished.  You can remember the
 inserted stmts that are candidates for moving (just set a pass-local
 flag on them) and assign UIDs for the whole function after all stmts
 are inserted.

 The problem is we need sane UID values as reassoc is happening - not
 after it is finished. As it process each stmt in reassoc_bb, it might
 generate new stmts which might have to be compared in
 find_insert_point or appears_later_in_bb.

 But if you no longer find_insert_point during reassoc but instead do
 a scheduling pass after it finished you won't need sane UIDs during
 reassoc.

 Richard.

 - Easwaran

 Richard.


 - Easwaran



 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and 
 reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com 
 wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com 
 wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID 
 of a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 

Re: Fix PR middle-end/57370

2013-09-02 Thread Richard Biener
On Fri, Aug 30, 2013 at 6:13 PM, Easwaran Raman era...@google.com wrote:
 On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman era...@google.com wrote:
 Richard,
 Do you want me to commit everything but the  insert_stmt_after hunk
 now?

 Yes.

 There are more similar failures reported in
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
 the updated patch there. Shall I send that for review? Apart from the
 debug statement issue, almost all the bugs are due to dependence
 violation because certain newly inserted statements do not have the
 right UID. Instead of trying to catch all of them, will it be better
 if I check if the stmt has a proper uid (non-zero if it is not the
 first stmt) and assign a sensible value at the point where it is used
 (find_insert_point and appears_later_in_bb) instead of where the stmt
 is created? I think that would be less brittle.

 In the end all this placement stuff should be reverted and done as
 post-processing after reassoc is finished.  You can remember the
 inserted stmts that are candidates for moving (just set a pass-local
 flag on them) and assign UIDs for the whole function after all stmts
 are inserted.

 The problem is we need sane UID values as reassoc is happening - not
 after it is finished. As it process each stmt in reassoc_bb, it might
 generate new stmts which might have to be compared in
 find_insert_point or appears_later_in_bb.

But if you no longer find_insert_point during reassoc but instead do
a scheduling pass after it finished you won't need sane UIDs during
reassoc.

Richard.

 - Easwaran

 Richard.


 - Easwaran



 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com 
 wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of 
 a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 

Re: Fix PR middle-end/57370

2013-08-30 Thread Richard Biener
On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman era...@google.com wrote:
 Richard,
 Do you want me to commit everything but the  insert_stmt_after hunk
 now?

Yes.

 There are more similar failures reported in
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
 the updated patch there. Shall I send that for review? Apart from the
 debug statement issue, almost all the bugs are due to dependence
 violation because certain newly inserted statements do not have the
 right UID. Instead of trying to catch all of them, will it be better
 if I check if the stmt has a proper uid (non-zero if it is not the
 first stmt) and assign a sensible value at the point where it is used
 (find_insert_point and appears_later_in_bb) instead of where the stmt
 is created? I think that would be less brittle.

In the end all this placement stuff should be reverted and done as
post-processing after reassoc is finished.  You can remember the
inserted stmts that are candidates for moving (just set a pass-local
flag on them) and assign UIDs for the whole function after all stmts
are inserted.

Richard.


 - Easwaran



 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a 
 phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 +t5531 + t5533 + t5535 + t5537 + 
 +t5540
 +t5565 = t112 * t1685
 +t5575 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564 - 
 +0.80e1_dp * t5565 + t5568 + t5572 + 
 +t5574
 +t5611 = t5579 - t5585 + 

Re: Fix PR middle-end/57370

2013-08-30 Thread Easwaran Raman
On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener
richard.guent...@gmail.com wrote:
 On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman era...@google.com wrote:
 Richard,
 Do you want me to commit everything but the  insert_stmt_after hunk
 now?

 Yes.

 There are more similar failures reported in
 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
 the updated patch there. Shall I send that for review? Apart from the
 debug statement issue, almost all the bugs are due to dependence
 violation because certain newly inserted statements do not have the
 right UID. Instead of trying to catch all of them, will it be better
 if I check if the stmt has a proper uid (non-zero if it is not the
 first stmt) and assign a sensible value at the point where it is used
 (find_insert_point and appears_later_in_bb) instead of where the stmt
 is created? I think that would be less brittle.

 In the end all this placement stuff should be reverted and done as
 post-processing after reassoc is finished.  You can remember the
 inserted stmts that are candidates for moving (just set a pass-local
 flag on them) and assign UIDs for the whole function after all stmts
 are inserted.

The problem is we need sane UID values as reassoc is happening - not
after it is finished. As it process each stmt in reassoc_bb, it might
generate new stmts which might have to be compared in
find_insert_point or appears_later_in_bb.

- Easwaran

 Richard.


 - Easwaran



 On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
 richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com 
 wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of 
 a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 + 

Re: Fix PR middle-end/57370

2013-08-30 Thread Jakub Jelinek
On Fri, Aug 30, 2013 at 09:13:34AM -0700, Easwaran Raman wrote:
  There are more similar failures reported in
  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
  the updated patch there. Shall I send that for review? Apart from the
  debug statement issue, almost all the bugs are due to dependence
  violation because certain newly inserted statements do not have the
  right UID. Instead of trying to catch all of them, will it be better
  if I check if the stmt has a proper uid (non-zero if it is not the
  first stmt) and assign a sensible value at the point where it is used
  (find_insert_point and appears_later_in_bb) instead of where the stmt
  is created? I think that would be less brittle.
 
  In the end all this placement stuff should be reverted and done as
  post-processing after reassoc is finished.  You can remember the
  inserted stmts that are candidates for moving (just set a pass-local
  flag on them) and assign UIDs for the whole function after all stmts
  are inserted.
 
 The problem is we need sane UID values as reassoc is happening - not
 after it is finished. As it process each stmt in reassoc_bb, it might
 generate new stmts which might have to be compared in
 find_insert_point or appears_later_in_bb.

A new stmt will be created with UID 0 and in case there is stmt movement,
you could zero the UID during movement.  Then, you can just special case
UID zero when you are looking at UIDs.  As on trunk/4.8 gsi_for_stmt is
O(1), you can easily walk a couple of previous and/or later stmts and look
for the first non-zero UID around it, and treat it as if the UID was
previous non-zero UID + 0.5 or next non-zero UID - 0.5.  And, if you see
too many consecutive statements with UID 0 (more than some constant, 32?),
just reassign UIDs to the whole bb.  Or you could initially assign UIDs
with some gaps, then keep filling those gaps and once you fill them,
renumber again with new gaps.

Jakub


Re: Fix PR middle-end/57370

2013-08-30 Thread Jakub Jelinek
On Fri, Aug 30, 2013 at 09:49:59AM -0700, Easwaran Raman wrote:
 Yes, this is pretty much what I was proposing. The current
 implementation doesn't rely on UIDs being unique - they only have to
 be monotonically non-decreasing. So, when a UID of 0 is encountered,
 go up till a non-zero UID is found and then go down and assign this
 non-zero UID. This effectively implies that each UID-0 stmt is visited
 at most twice. I don't think we need to check if I see more than
 certain number of UID-0 stmts and redo the entire BB.

Sure, renumbering the entire BB would be only for compile time reasons;
if you end up with a huge bb where 90% of stmts will have the same UID,
it is as if you weren't using UIDs at all and always walked the entire BB
to find out what stmt precedes what.  You could spend a lot of time in
appears_later_in_bb.

Looking at the code, couple of nits:
  return ((bb_a == bb_b  gimple_uid (a)   gimple_uid (b))
extra space before .

  gsi_next (gsi);
  if (gsi_end_p (gsi))
return stmt1;
  for (; !gsi_end_p (gsi); gsi_next (gsi))
{
...
}
  return stmt1;

Why not just for (gsi_next (gsi); !gsi_end_p (gsi); gsi_next (gsi)) ?
The extra if (gsi_end_p (gsi)) return stmt1; doesn't make any sense
when the loop does exactly that too.

And for the debug stmts, are you sure you are resetting only those debug
stmts which you have to reset?  I mean, if there are say debug stmts using
the SSA_NAME in other bb, it doesn't need to be reset (unless you reuse
the same SSA_NAME for something else), and the compiler even has code
to reconstruct SSA_NAMEs that have been removed, but were still referenced
in debug stmts, using expressions in debug stmts and debug temporaries.

Jakub


Re: Fix PR middle-end/57370

2013-08-30 Thread Easwaran Raman
On Fri, Aug 30, 2013 at 9:26 AM, Jakub Jelinek ja...@redhat.com wrote:
 On Fri, Aug 30, 2013 at 09:13:34AM -0700, Easwaran Raman wrote:
  There are more similar failures reported in
  http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
  the updated patch there. Shall I send that for review? Apart from the
  debug statement issue, almost all the bugs are due to dependence
  violation because certain newly inserted statements do not have the
  right UID. Instead of trying to catch all of them, will it be better
  if I check if the stmt has a proper uid (non-zero if it is not the
  first stmt) and assign a sensible value at the point where it is used
  (find_insert_point and appears_later_in_bb) instead of where the stmt
  is created? I think that would be less brittle.
 
  In the end all this placement stuff should be reverted and done as
  post-processing after reassoc is finished.  You can remember the
  inserted stmts that are candidates for moving (just set a pass-local
  flag on them) and assign UIDs for the whole function after all stmts
  are inserted.

 The problem is we need sane UID values as reassoc is happening - not
 after it is finished. As it process each stmt in reassoc_bb, it might
 generate new stmts which might have to be compared in
 find_insert_point or appears_later_in_bb.

 A new stmt will be created with UID 0 and in case there is stmt movement,
 you could zero the UID during movement.  Then, you can just special case
 UID zero when you are looking at UIDs.  As on trunk/4.8 gsi_for_stmt is
 O(1), you can easily walk a couple of previous and/or later stmts and look
 for the first non-zero UID around it, and treat it as if the UID was
 previous non-zero UID + 0.5 or next non-zero UID - 0.5.  And, if you see
 too many consecutive statements with UID 0 (more than some constant, 32?),
 just reassign UIDs to the whole bb.  Or you could initially assign UIDs
 with some gaps, then keep filling those gaps and once you fill them,
 renumber again with new gaps.
 Jakub

Yes, this is pretty much what I was proposing. The current
implementation doesn't rely on UIDs being unique - they only have to
be monotonically non-decreasing. So, when a UID of 0 is encountered,
go up till a non-zero UID is found and then go down and assign this
non-zero UID. This effectively implies that each UID-0 stmt is visited
at most twice. I don't think we need to check if I see more than
certain number of UID-0 stmts and redo the entire BB.

- Easwaran


Re: Fix PR middle-end/57370

2013-08-29 Thread Easwaran Raman
Richard,
Do you want me to commit everything but the  insert_stmt_after hunk
now? There are more similar failures reported in
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached
the updated patch there. Shall I send that for review? Apart from the
debug statement issue, almost all the bugs are due to dependence
violation because certain newly inserted statements do not have the
right UID. Instead of trying to catch all of them, will it be better
if I check if the stmt has a proper uid (non-zero if it is not the
first stmt) and assign a sensible value at the point where it is used
(find_insert_point and appears_later_in_bb) instead of where the stmt
is created? I think that would be less brittle.

- Easwaran



On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener
richard.guent...@gmail.com wrote:
 On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

 Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
 at all.  The other hunks are ok, but we need to work harder to preserve
 debug stmts - simply removing all is not going to fly.

 Richard.


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a 
 phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 +t5531 + t5533 + t5535 + t5537 + 
 +t5540
 +t5565 = t112 * t1685
 +t5575 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564 - 
 +0.80e1_dp * t5565 + t5568 + t5572 + 
 +t5574
 +t5611 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t5613 = t5469 + t5541 + t5575 + t5611
 +t6223 = t6189 - 
 +0.36e0_dp  * t83 * t84 * t5613 + 
 +t6222
 +t6227 = - t8 * (t5305 + t6223)
 +e_ndrho_ndrho_rho(ii) = e_ndrho_ndrho_rho(ii) + 
 + 

Re: Fix PR middle-end/57370

2013-08-27 Thread Richard Biener
On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.

Missing ChangeLog entry for the insert_stmt_after hunk which I do not like
at all.  The other hunks are ok, but we need to work harder to preserve
debug stmts - simply removing all is not going to fly.

Richard.


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a 
 phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 +t5531 + t5533 + t5535 + t5537 + 
 +t5540
 +t5565 = t112 * t1685
 +t5575 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564 - 
 +0.80e1_dp * t5565 + t5568 + t5572 + 
 +t5574
 +t5611 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t5613 = t5469 + t5541 + t5575 + t5611
 +t6223 = t6189 - 
 +0.36e0_dp  * t83 * t84 * t5613 + 
 +t6222
 +t6227 = - t8 * (t5305 + t6223)
 +e_ndrho_ndrho_rho(ii) = e_ndrho_ndrho_rho(ii) + 
 + t6227 * sx
 +t6352 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 + 
 +0.40e1_dp * t102  * t327 * t2057 * t557 - 
 +t5456 + t5459 - t5462 + t5466 - 
 +t5468
 +t6363 = t5480 - t5489 + 
 +0.96e2_dp  * t1054 * t640 * t3679
 +t6367 = t5472 - t5474 - t5478 - t5523 + 
 +t5525 + t5531 + t5533 + t5535 + 
 +t5537 - 0.20e1_dp * t102 * t105 * t6363 + 
 +t5540
 +t6370 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564  - 
 +0.40e1_dp * t5565 + 
 +t5568 + t5572 + t5574
 +t6373 = t5579 - t5585 + t5590 - t5595 + 
 

Re: Fix PR middle-end/57370

2013-08-07 Thread Easwaran Raman
Ping.

On Wed, Jul 31, 2013 at 4:34 PM, Easwaran Raman era...@google.com wrote:
 I have a new patch that supersedes this. The new patch also fixes PR
 tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
 no test regression on x86_64/linux. Ok for trunk?

 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
 of debug statements that cause inconsistent IR.


 testsuite/ChangeLog:
 2013-07-31  Easwaran Raman  era...@google.com

 PR middle-end/57370
 PR tree-optimization/57393
 PR tree-optimization/58011
 * gfortran.dg/reassoc_12.f90: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.
 * gcc.dg/tree-ssa/reassoc-31.c: New testcase.


 On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a 
 phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 +t5531 + t5533 + t5535 + t5537 + 
 +t5540
 +t5565 = t112 * t1685
 +t5575 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564 - 
 +0.80e1_dp * t5565 + t5568 + t5572 + 
 +t5574
 +t5611 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t5613 = t5469 + t5541 + t5575 + t5611
 +t6223 = t6189 - 
 +0.36e0_dp  * t83 * t84 * t5613 + 
 +t6222
 +t6227 = - t8 * (t5305 + t6223)
 +e_ndrho_ndrho_rho(ii) = e_ndrho_ndrho_rho(ii) + 
 + t6227 * sx
 +t6352 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 + 
 +0.40e1_dp * t102  * t327 * t2057 * t557 - 
 +t5456 + t5459 - t5462 + t5466 - 
 +t5468
 +t6363 = t5480 - t5489 + 
 +0.96e2_dp  * t1054 * t640 * t3679
 +t6367 = t5472 - t5474 - t5478 - t5523 + 
 +t5525 + t5531 + t5533 + t5535 + 
 +t5537 - 0.20e1_dp * t102 * t105 * t6363 + 
 +t5540
 +t6370 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564  - 
 +0.40e1_dp * t5565 + 
 +t5568 + t5572 + t5574
 +t6373 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t6375 = t6352 + t6367 + t6370 + t6373
 +t6380 = - 0.36e0_dp * t83 * t84 * t6375 + t5701
 

Re: Fix PR middle-end/57370

2013-07-31 Thread Easwaran Raman
I have a new patch that supersedes this. The new patch also fixes PR
tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and
no test regression on x86_64/linux. Ok for trunk?

2013-07-31  Easwaran Raman  era...@google.com

PR middle-end/57370
* tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and reset
of debug statements that cause inconsistent IR.


testsuite/ChangeLog:
2013-07-31  Easwaran Raman  era...@google.com

PR middle-end/57370
PR tree-optimization/57393
PR tree-optimization/58011
* gfortran.dg/reassoc_12.f90: New testcase.
* gcc.dg/tree-ssa/reassoc-31.c: New testcase.
* gcc.dg/tree-ssa/reassoc-31.c: New testcase.


On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman era...@google.com wrote:
 Ping.

 On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 +t5531 + t5533 + t5535 + t5537 + 
 +t5540
 +t5565 = t112 * t1685
 +t5575 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564 - 
 +0.80e1_dp * t5565 + t5568 + t5572 + 
 +t5574
 +t5611 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t5613 = t5469 + t5541 + t5575 + t5611
 +t6223 = t6189 - 
 +0.36e0_dp  * t83 * t84 * t5613 + 
 +t6222
 +t6227 = - t8 * (t5305 + t6223)
 +e_ndrho_ndrho_rho(ii) = e_ndrho_ndrho_rho(ii) + 
 + t6227 * sx
 +t6352 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 + 
 +0.40e1_dp * t102  * t327 * t2057 * t557 - 
 +t5456 + t5459 - t5462 + t5466 - 
 +t5468
 +t6363 = t5480 - t5489 + 
 +0.96e2_dp  * t1054 * t640 * t3679
 +t6367 = t5472 - t5474 - t5478 - t5523 + 
 +t5525 + t5531 + t5533 + t5535 + 
 +t5537 - 0.20e1_dp * t102 * t105 * t6363 + 
 +t5540
 +t6370 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564  - 
 +0.40e1_dp * t5565 + 
 +t5568 + t5572 + t5574
 +t6373 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t6375 = t6352 + t6367 + t6370 + t6373
 +t6380 = - 0.36e0_dp * t83 * t84 * t6375 + t5701
 +t6669 = -t4704 - t8 * (t6344 + t6380 + t6665)
 +e_ndrho_ndrho_ndrho(ii) 

Re: Fix PR middle-end/57370

2013-07-12 Thread Easwaran Raman
Ping.

On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman era...@google.com wrote:
 A newly generated statement in build_and_add_sum  function of
 tree-ssa-reassoc.c has to be assigned the UID of its adjacent
 statement. In one instance, it was assigned the wrong uid (of an
 earlier phi statement) which messed up the IR and caused the test
 program to hang. Bootstraps and no test regressions on x86_64/linux.
 Ok for trunk?

 Thanks,
 Easwaran


 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a phi
 node for a non-phi gimple statement.

 testsuite/ChangeLog:
 2013-06-27  Easwaran Raman  era...@google.com

 PR middle-end/57370
 * gfortran.dg/reassoc_12.f90: New testcase.


 Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
 ===
 --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
 @@ -0,0 +1,74 @@
 +! { dg-do compile }
 +! { dg-options -O2 -ffast-math }
 +! PR middle-end/57370
 +
 + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
 +   grad_deriv,npoints, sx)
 +IMPLICIT REAL*8 (t)
 +INTEGER, PARAMETER :: dp=8
 +REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
 +   e_ndrho_ndrho_rho
 +  DO ii=1,npoints
 +  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
 +t1425 = t233 * t557
 +t1429 = beta * t225
 +t1622 = t327 * t1621
 +t1626 = t327 * t1625
 +t1632 = t327 * t1631
 +t1685 = t105 * t1684
 +t2057 = t1636 + t8 * (t2635 + t3288)
 +  END IF
 +  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
 +t5469 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 - t5456 + t5459  - 
 +t5462 + t5466 - t5468
 +t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
 +t5489 = 0.16e2_dp * t1429 * t1658
 +t5531 = 0.160e2_dp * t112 * t1626
 +t5533 = 0.160e2_dp * t112 * t1632
 +t5537 = 0.160e2_dp * t112 * t1622
 +t5541 = t5472 - t5478 - t5523 + t5525 + 
 +t5531 + t5533 + t5535 + t5537 + 
 +t5540
 +t5565 = t112 * t1685
 +t5575 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564 - 
 +0.80e1_dp * t5565 + t5568 + t5572 + 
 +t5574
 +t5611 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t5613 = t5469 + t5541 + t5575 + t5611
 +t6223 = t6189 - 
 +0.36e0_dp  * t83 * t84 * t5613 + 
 +t6222
 +t6227 = - t8 * (t5305 + t6223)
 +e_ndrho_ndrho_rho(ii) = e_ndrho_ndrho_rho(ii) + 
 + t6227 * sx
 +t6352 = t5440 - t5443 - t5446 - t5449 - 
 +t5451 - t5454 + 
 +0.40e1_dp * t102  * t327 * t2057 * t557 - 
 +t5456 + t5459 - t5462 + t5466 - 
 +t5468
 +t6363 = t5480 - t5489 + 
 +0.96e2_dp  * t1054 * t640 * t3679
 +t6367 = t5472 - t5474 - t5478 - t5523 + 
 +t5525 + t5531 + t5533 + t5535 + 
 +t5537 - 0.20e1_dp * t102 * t105 * t6363 + 
 +t5540
 +t6370 = t5545 - t5548 + t5551 + t5553 - 
 +t5558 + t5560 - t5562 + t5564  - 
 +0.40e1_dp * t5565 + 
 +t5568 + t5572 + t5574
 +t6373 = t5579 - t5585 + t5590 - t5595 + 
 +t5597 - t5602 + t5604 + t5607 + 
 +t5610
 +t6375 = t6352 + t6367 + t6370 + t6373
 +t6380 = - 0.36e0_dp * t83 * t84 * t6375 + t5701
 +t6669 = -t4704 - t8 * (t6344 + t6380 + t6665)
 +e_ndrho_ndrho_ndrho(ii) = e_ndrho_ndrho_ndrho(ii) + 
 + t6669 * sx
 +  END IF
 +  END DO
 +  END SUBROUTINE xb88_lr_adiabatic_lda_calc
 +
 Index: gcc/tree-ssa-reassoc.c
 ===
 --- gcc/tree-ssa-reassoc.c(revision 200429)
 +++ gcc/tree-ssa-reassoc.c(working copy)
 @@ -1207,7 +1207,7 @@ build_and_add_sum (tree type, tree op1, tree op2,
if (gimple_code (op1def) == GIMPLE_PHI)
  {
gsi = gsi_after_labels (gimple_bb (op1def));
 -   gimple_set_uid (sum, gimple_uid (op1def));
 +  gimple_set_uid (sum, gimple_uid (gsi_stmt (gsi)));
gsi_insert_before (gsi, sum, GSI_NEW_STMT);
  }
else


Fix PR middle-end/57370

2013-06-27 Thread Easwaran Raman
A newly generated statement in build_and_add_sum  function of
tree-ssa-reassoc.c has to be assigned the UID of its adjacent
statement. In one instance, it was assigned the wrong uid (of an
earlier phi statement) which messed up the IR and caused the test
program to hang. Bootstraps and no test regressions on x86_64/linux.
Ok for trunk?

Thanks,
Easwaran


2013-06-27  Easwaran Raman  era...@google.com

PR middle-end/57370
* tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID of a phi
node for a non-phi gimple statement.

testsuite/ChangeLog:
2013-06-27  Easwaran Raman  era...@google.com

PR middle-end/57370
* gfortran.dg/reassoc_12.f90: New testcase.


Index: gcc/testsuite/gfortran.dg/reassoc_12.f90
===
--- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0)
@@ -0,0 +1,74 @@
+! { dg-do compile }
+! { dg-options -O2 -ffast-math }
+! PR middle-end/57370
+
+ SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, 
+   grad_deriv,npoints, sx)
+IMPLICIT REAL*8 (t)
+INTEGER, PARAMETER :: dp=8
+REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, 
+   e_ndrho_ndrho_rho
+  DO ii=1,npoints
+  IF( grad_deriv = 2 .OR. grad_deriv == -2 ) THEN
+t1425 = t233 * t557
+t1429 = beta * t225
+t1622 = t327 * t1621
+t1626 = t327 * t1625
+t1632 = t327 * t1631
+t1685 = t105 * t1684
+t2057 = t1636 + t8 * (t2635 + t3288)
+  END IF
+  IF( grad_deriv = 3 .OR. grad_deriv == -3 ) THEN
+t5469 = t5440 - t5443 - t5446 - t5449 - 
+t5451 - t5454 - t5456 + t5459  - 
+t5462 + t5466 - t5468
+t5478 = 0.240e2_dp * t1616 * t973 * t645 * t1425
+t5489 = 0.16e2_dp * t1429 * t1658
+t5531 = 0.160e2_dp * t112 * t1626
+t5533 = 0.160e2_dp * t112 * t1632
+t5537 = 0.160e2_dp * t112 * t1622
+t5541 = t5472 - t5478 - t5523 + t5525 + 
+t5531 + t5533 + t5535 + t5537 + 
+t5540
+t5565 = t112 * t1685
+t5575 = t5545 - t5548 + t5551 + t5553 - 
+t5558 + t5560 - t5562 + t5564 - 
+0.80e1_dp * t5565 + t5568 + t5572 + 
+t5574
+t5611 = t5579 - t5585 + t5590 - t5595 + 
+t5597 - t5602 + t5604 + t5607 + 
+t5610
+t5613 = t5469 + t5541 + t5575 + t5611
+t6223 = t6189 - 
+0.36e0_dp  * t83 * t84 * t5613 + 
+t6222
+t6227 = - t8 * (t5305 + t6223)
+e_ndrho_ndrho_rho(ii) = e_ndrho_ndrho_rho(ii) + 
+ t6227 * sx
+t6352 = t5440 - t5443 - t5446 - t5449 - 
+t5451 - t5454 + 
+0.40e1_dp * t102  * t327 * t2057 * t557 - 
+t5456 + t5459 - t5462 + t5466 - 
+t5468
+t6363 = t5480 - t5489 + 
+0.96e2_dp  * t1054 * t640 * t3679
+t6367 = t5472 - t5474 - t5478 - t5523 + 
+t5525 + t5531 + t5533 + t5535 + 
+t5537 - 0.20e1_dp * t102 * t105 * t6363 + 
+t5540
+t6370 = t5545 - t5548 + t5551 + t5553 - 
+t5558 + t5560 - t5562 + t5564  - 
+0.40e1_dp * t5565 + 
+t5568 + t5572 + t5574
+t6373 = t5579 - t5585 + t5590 - t5595 + 
+t5597 - t5602 + t5604 + t5607 + 
+t5610
+t6375 = t6352 + t6367 + t6370 + t6373
+t6380 = - 0.36e0_dp * t83 * t84 * t6375 + t5701
+t6669 = -t4704 - t8 * (t6344 + t6380 + t6665)
+e_ndrho_ndrho_ndrho(ii) = e_ndrho_ndrho_ndrho(ii) + 
+ t6669 * sx
+  END IF
+  END DO
+  END SUBROUTINE xb88_lr_adiabatic_lda_calc
+
Index: gcc/tree-ssa-reassoc.c
===
--- gcc/tree-ssa-reassoc.c(revision 200429)
+++ gcc/tree-ssa-reassoc.c(working copy)
@@ -1207,7 +1207,7 @@ build_and_add_sum (tree type, tree op1, tree op2,
   if (gimple_code (op1def) == GIMPLE_PHI)
 {
   gsi = gsi_after_labels (gimple_bb (op1def));
-   gimple_set_uid (sum, gimple_uid (op1def));
+  gimple_set_uid (sum, gimple_uid (gsi_stmt (gsi)));
   gsi_insert_before (gsi, sum, GSI_NEW_STMT);
 }
   else