Message ID | CAPK5YPbNvp9g-9O=DUb06S3yNEvXiBrYeTbbsW2cDJuwd2_UXQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
Ping. On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
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 <eraman@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 <eraman@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 <eraman@google.com> wrote: > Ping. > > On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
Ping. On Wed, Jul 31, 2013 at 4:34 PM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >> Ping. >> >> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >> Ping. >> >> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
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.guenther@gmail.com> wrote: > On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>> Ping. >>> >>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman <eraman@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.guenther@gmail.com> wrote: >> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>>> Ping. >>>> >>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman <eraman@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.guenther@gmail.com> wrote: >>> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>>>> Ping. >>>>> >>>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
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
On Fri, Aug 30, 2013 at 9:26 AM, Jakub Jelinek <jakub@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
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
On Fri, Aug 30, 2013 at 6:13 PM, Easwaran Raman <eraman@google.com> wrote: > On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman <eraman@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.guenther@gmail.com> wrote: >>>> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>>>>> Ping. >>>>>> >>>>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
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.guenther@gmail.com> wrote: > On Fri, Aug 30, 2013 at 6:13 PM, Easwaran Raman <eraman@google.com> wrote: >> On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener >> <richard.guenther@gmail.com> wrote: >>> On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman <eraman@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.guenther@gmail.com> wrote: >>>>> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>>>>>> Ping. >>>>>>> >>>>>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>> Ping. >>> >>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
On Thu, Sep 5, 2013 at 9:23 PM, Easwaran Raman <eraman@google.com> wrote: > On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>>> Ping. >>>> >>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
On Fri, Sep 6, 2013 at 12:05 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Sep 5, 2013 at 9:23 PM, Easwaran Raman <eraman@google.com> wrote: >> On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener >> <richard.guenther@gmail.com> wrote: >>> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>>>> Ping. >>>>> >>>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
On Fri, Sep 6, 2013 at 8:47 PM, Easwaran Raman <eraman@google.com> wrote: > On Fri, Sep 6, 2013 at 12:05 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Thu, Sep 5, 2013 at 9:23 PM, Easwaran Raman <eraman@google.com> wrote: >>> On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener >>> <richard.guenther@gmail.com> wrote: >>>> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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 <eraman@google.com> wrote: >>>>>> Ping. >>>>>> >>>>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <eraman@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 <eraman@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 <eraman@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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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
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.1600000000e2_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.3333333336e0_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.9600000000e2_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.3333333336e0_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