Submitted the patch (r202262) without the insert_stmt_after hunk. Also fixed nits pointed out by Jakub.
- Easwaran On Mon, Sep 2, 2013 at 2:31 AM, Richard Biener <richard.guent...@gmail.com> wrote: > On Fri, Aug 30, 2013 at 6:13 PM, Easwaran Raman <era...@google.com> wrote: >> On Fri, Aug 30, 2013 at 1:25 AM, Richard Biener >> <richard.guent...@gmail.com> wrote: >>> On Fri, Aug 30, 2013 at 2:30 AM, Easwaran Raman <era...@google.com> wrote: >>>> Richard, >>>> Do you want me to commit everything but the insert_stmt_after hunk >>>> now? >>> >>> Yes. >>> >>>> There are more similar failures reported in >>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57393 and I have attached >>>> the updated patch there. Shall I send that for review? Apart from the >>>> debug statement issue, almost all the bugs are due to dependence >>>> violation because certain newly inserted statements do not have the >>>> right UID. Instead of trying to catch all of them, will it be better >>>> if I check if the stmt has a proper uid (non-zero if it is not the >>>> first stmt) and assign a sensible value at the point where it is used >>>> (find_insert_point and appears_later_in_bb) instead of where the stmt >>>> is created? I think that would be less brittle. >>> >>> In the end all this placement stuff should be reverted and done as >>> post-processing after reassoc is finished. You can remember the >>> inserted stmts that are candidates for moving (just set a pass-local >>> flag on them) and assign UIDs for the whole function after all stmts >>> are inserted. >> >> The problem is we need sane UID values as reassoc is happening - not >> after it is finished. As it process each stmt in reassoc_bb, it might >> generate new stmts which might have to be compared in >> find_insert_point or appears_later_in_bb. > > But if you no longer find_insert_point during reassoc but instead do > a "scheduling" pass after it finished you won't need sane UIDs during > reassoc. > > Richard. > >> - Easwaran >> >>> Richard. >>> >>> >>>> - Easwaran >>>> >>>> >>>> >>>> On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener >>>> <richard.guent...@gmail.com> wrote: >>>>> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman <era...@google.com> wrote: >>>>>> I have a new patch that supersedes this. The new patch also fixes PR >>>>>> tree-optimization/57393 and PR tree-optimization/58011. Bootstraps and >>>>>> no test regression on x86_64/linux. Ok for trunk? >>>>>> >>>>>> 2013-07-31 Easwaran Raman <era...@google.com> >>>>>> >>>>>> PR middle-end/57370 >>>>>> * tree-ssa-reassoc.c (build_and_add_sum): Fix UID assignment and >>>>>> reset >>>>>> of debug statements that cause inconsistent IR. >>>>> >>>>> Missing ChangeLog entry for the insert_stmt_after hunk which I do not like >>>>> at all. The other hunks are ok, but we need to work harder to preserve >>>>> debug stmts - simply removing all is not going to fly. >>>>> >>>>> Richard. >>>>> >>>>>> >>>>>> testsuite/ChangeLog: >>>>>> 2013-07-31 Easwaran Raman <era...@google.com> >>>>>> >>>>>> PR middle-end/57370 >>>>>> PR tree-optimization/57393 >>>>>> PR tree-optimization/58011 >>>>>> * gfortran.dg/reassoc_12.f90: New testcase. >>>>>> * gcc.dg/tree-ssa/reassoc-31.c: New testcase. >>>>>> * gcc.dg/tree-ssa/reassoc-31.c: New testcase. >>>>>> >>>>>> >>>>>> On Fri, Jul 12, 2013 at 7:57 AM, Easwaran Raman <era...@google.com> >>>>>> wrote: >>>>>>> Ping. >>>>>>> >>>>>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman <era...@google.com> >>>>>>> wrote: >>>>>>>> A newly generated statement in build_and_add_sum function of >>>>>>>> tree-ssa-reassoc.c has to be assigned the UID of its adjacent >>>>>>>> statement. In one instance, it was assigned the wrong uid (of an >>>>>>>> earlier phi statement) which messed up the IR and caused the test >>>>>>>> program to hang. Bootstraps and no test regressions on x86_64/linux. >>>>>>>> Ok for trunk? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Easwaran >>>>>>>> >>>>>>>> >>>>>>>> 2013-06-27 Easwaran Raman <era...@google.com> >>>>>>>> >>>>>>>> PR middle-end/57370 >>>>>>>> * tree-ssa-reassoc.c (build_and_add_sum): Do not use the UID >>>>>>>> of a phi >>>>>>>> node for a non-phi gimple statement. >>>>>>>> >>>>>>>> testsuite/ChangeLog: >>>>>>>> 2013-06-27 Easwaran Raman <era...@google.com> >>>>>>>> >>>>>>>> PR middle-end/57370 >>>>>>>> * gfortran.dg/reassoc_12.f90: New testcase. >>>>>>>> >>>>>>>> >>>>>>>> Index: gcc/testsuite/gfortran.dg/reassoc_12.f90 >>>>>>>> =================================================================== >>>>>>>> --- gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0) >>>>>>>> +++ gcc/testsuite/gfortran.dg/reassoc_12.f90 (revision 0) >>>>>>>> @@ -0,0 +1,74 @@ >>>>>>>> +! { dg-do compile } >>>>>>>> +! { dg-options "-O2 -ffast-math" } >>>>>>>> +! PR middle-end/57370 >>>>>>>> + >>>>>>>> + SUBROUTINE xb88_lr_adiabatic_lda_calc(e_ndrho_ndrho_ndrho, & >>>>>>>> + grad_deriv,npoints, sx) >>>>>>>> + IMPLICIT REAL*8 (t) >>>>>>>> + INTEGER, PARAMETER :: dp=8 >>>>>>>> + REAL(kind=dp), DIMENSION(1:npoints) :: e_ndrho_ndrho_ndrho, & >>>>>>>> + e_ndrho_ndrho_rho >>>>>>>> + DO ii=1,npoints >>>>>>>> + IF( grad_deriv >= 2 .OR. grad_deriv == -2 ) THEN >>>>>>>> + t1425 = t233 * t557 >>>>>>>> + t1429 = beta * t225 >>>>>>>> + t1622 = t327 * t1621 >>>>>>>> + t1626 = t327 * t1625 >>>>>>>> + t1632 = t327 * t1631 >>>>>>>> + t1685 = t105 * t1684 >>>>>>>> + t2057 = t1636 + t8 * (t2635 + t3288) >>>>>>>> + END IF >>>>>>>> + IF( grad_deriv >= 3 .OR. grad_deriv == -3 ) THEN >>>>>>>> + t5469 = t5440 - t5443 - t5446 - 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
