From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2408 invoked by alias); 6 Sep 2013 18:47:23 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 2395 invoked by uid 89); 6 Sep 2013 18:47:23 -0000 Received: from mail-ie0-f182.google.com (HELO mail-ie0-f182.google.com) (209.85.223.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 06 Sep 2013 18:47:23 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=BAYES_00,KHOP_THREADED,NO_RELAYS autolearn=ham version=3.3.2 X-HELO: mail-ie0-f182.google.com Received: by mail-ie0-f182.google.com with SMTP id aq17so7646009iec.41 for ; Fri, 06 Sep 2013 11:47:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=sWTlLCCH/NnKs3nA5ROY+7/TKULHL0lOhSD75E65KYA=; b=B1MosnNMox/MUM6geBE6/fW1efxxWvPquXo2iACI2H9uyhOPWwz6kIQWNZEJ2Nq+pD YxMmCQqG0m88pOqebKkHTE6uMPS6lUyDbBinFhRoFSHUzmiqGNBUDjNHHr1K5WRO3xgk DiQEILnayd56yDHMi5tp29TeKHIhuh/GpbAQcz1rkU9I03Olc2Z8nnz2AdQvr3ziGYue 6LuR1kRQ/BfVIiGJwTVjKodKhYzSlb9yXloc3lMFX1F3rgxS4SBa1dL22V1Syt7o8Vvz OkwRYDYhS8QSLHGOkSV701a+3xiF9kTdzAhLVSPSeozRWrC1fEuweHjx/gL/QIru4y7S X+bA== X-Gm-Message-State: ALoCoQkIJGfGIofX9Io3X1zSBbNc4kM+qrjTtB2HaAyF3QdsRbTX4WLon4RISL3lwI20ACryX90FjZ/0hn4cuXlxcW9dPQ+Y/mXGO9p25oIVbk+jVDjmtdnbMdE6p7RGAcm6cdknBJ+JIV5x8wQ7ar32pc2XCkcRF2g+LMOmiRe/f/WiEmPClwTo9XjY/NU+H6ZJ05Ii/sV7E3FuMWdUTCop/PwxOoHojw== MIME-Version: 1.0 X-Received: by 10.50.77.72 with SMTP id q8mr11234701igw.14.1378493240314; Fri, 06 Sep 2013 11:47:20 -0700 (PDT) Received: by 10.64.62.98 with HTTP; Fri, 6 Sep 2013 11:47:20 -0700 (PDT) In-Reply-To: References: Date: Fri, 06 Sep 2013 18:47:00 -0000 Message-ID: Subject: Re: Fix PR middle-end/57370 From: Easwaran Raman To: Richard Biener Cc: GCC Patches Content-Type: text/plain; charset=ISO-8859-1 X-IsSubscribed: yes X-SW-Source: 2013-09/txt/msg00439.txt.bz2 On Fri, Sep 6, 2013 at 12:05 AM, Richard Biener wrote: > On Thu, Sep 5, 2013 at 9:23 PM, Easwaran Raman wrote: >> On Tue, Aug 27, 2013 at 3:35 AM, Richard Biener >> wrote: >>> On Thu, Aug 1, 2013 at 1:34 AM, Easwaran Raman 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 >>>> >>>> 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 >>>> >>>> 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 wrote: >>>>> Ping. >>>>> >>>>> On Thu, Jun 27, 2013 at 10:15 AM, Easwaran Raman 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 >>>>>> >>>>>> 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 >>>>>> >>>>>> 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