This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix PR middle-end/57370


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]