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, 29 Aug 2013 17:30:17 -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>
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