This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix PR middle-end/57370
- From: Easwaran Raman <eraman at google dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 5 Sep 2013 12:23:12 -0700
- Subject: Re: Fix PR middle-end/57370
- Authentication-results: sourceware.org; auth=none
- References: <CAPK5YPbNvp9g-9O=DUb06S3yNEvXiBrYeTbbsW2cDJuwd2_UXQ at mail dot gmail dot com> <CAPK5YPaXwtv6O41hgk4gdfdU0H2d=H3Ac5YUJsmYDQWLKc7VGg at mail dot gmail dot com> <CAPK5YPYXHtxs=+LpJNeZuKU+hmx80zjG1qQgeetxLt6qtGBSJA at mail dot gmail dot com> <CAFiYyc1_zfQzzJZQpTh+di=Q1RVrEMXnNtksTRb9UaGeDZCjvQ at mail dot gmail dot com>
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