From patchwork Tue May 24 21:18:08 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Richard Thomas X-Patchwork-Id: 97228 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2647AB6F82 for ; Wed, 25 May 2011 07:18:35 +1000 (EST) Received: (qmail 14545 invoked by alias); 24 May 2011 21:18:33 -0000 Received: (qmail 14514 invoked by uid 22791); 24 May 2011 21:18:30 -0000 X-SWARE-Spam-Status: No, hits=-2.2 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, RFC_ABUSE_POST X-Spam-Check-By: sourceware.org Received: from mail-bw0-f47.google.com (HELO mail-bw0-f47.google.com) (209.85.214.47) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 24 May 2011 21:18:11 +0000 Received: by bwz5 with SMTP id 5so7455679bwz.20 for ; Tue, 24 May 2011 14:18:09 -0700 (PDT) MIME-Version: 1.0 Received: by 10.204.127.1 with SMTP id e1mr2914010bks.77.1306271888624; Tue, 24 May 2011 14:18:08 -0700 (PDT) Received: by 10.204.39.65 with HTTP; Tue, 24 May 2011 14:18:08 -0700 (PDT) In-Reply-To: References: <4DD92C77.5090107@netcologne.de> Date: Tue, 24 May 2011 23:18:08 +0200 Message-ID: Subject: Re: [patch, fortran] [4.6/4.7 Regression] Fix PR 48955 From: Paul Richard Thomas To: Thomas Koenig Cc: "fortran@gcc.gnu.org" , gcc-patches Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Dear All, > > I have posted a simpler alternative on the PR that uses your > suggestion that forward and backward dependences need to to be > recorded to get this right. > > I believe that it's OK but have only now had the opportunity to put it > on to regtest. > Following some comments from Thomas, the attached is the version that I wound up with. Tell us which one you prefer and Thomas and I will do the honours. Bootstrapped and regtested on FC9/x86_64 - OK for trunk and 4.6? Paul 2011-05-24 Paul Thomas Thomas Koenig PR fortran/48955 * trans-expr.c (gfc_trans_assignment_1): GFC_REVERSE_NOT_SET changed to GFC_ENABLE_REVERSE. * trans-array.c (gfc_init_loopinfo): GFC_CANNOT_REVERSE changed to GFC_INHIBIT_REVERSE. * gfortran.h : Enum gfc_reverse is now GFC_ENABLE_REVERSE, GFC_FORWARD_SET, GFC_REVERSE_SET and GFC_INHIBIT_REVERSE. * dependency.c (gfc_dep_resolver): Change names for elements of gfc_reverse as necessary. Change the logic so that forward dependences are remembered as well as backward ones. When both have appeared, force a temporary. 2011-05-24 Thomas Koenig PR fortran/48955 * gfortran.dg/dependency_40.f90 : New test. Index: gcc/fortran/trans-expr.c =================================================================== *** gcc/fortran/trans-expr.c (revision 173213) --- gcc/fortran/trans-expr.c (working copy) *************** gfc_trans_assignment_1 (gfc_expr * expr1 *** 6069,6076 **** /* Calculate the bounds of the scalarization. */ gfc_conv_ss_startstride (&loop); /* Enable loop reversal. */ ! for (n = 0; n < loop.dimen; n++) ! loop.reverse[n] = GFC_REVERSE_NOT_SET; /* Resolve any data dependencies in the statement. */ gfc_conv_resolve_dependencies (&loop, lss, rss); /* Setup the scalarizing loops. */ --- 6069,6076 ---- /* Calculate the bounds of the scalarization. */ gfc_conv_ss_startstride (&loop); /* Enable loop reversal. */ ! for (n = 0; n < GFC_MAX_DIMENSIONS; n++) ! loop.reverse[n] = GFC_ENABLE_REVERSE; /* Resolve any data dependencies in the statement. */ gfc_conv_resolve_dependencies (&loop, lss, rss); /* Setup the scalarizing loops. */ Index: gcc/fortran/trans-array.c =================================================================== *** gcc/fortran/trans-array.c (revision 173212) --- gcc/fortran/trans-array.c (working copy) *************** gfc_init_loopinfo (gfc_loopinfo * loop) *** 2244,2250 **** for (n = 0; n < GFC_MAX_DIMENSIONS; n++) { loop->order[n] = n; ! loop->reverse[n] = GFC_CANNOT_REVERSE; } loop->ss = gfc_ss_terminator; --- 2244,2250 ---- for (n = 0; n < GFC_MAX_DIMENSIONS; n++) { loop->order[n] = n; ! loop->reverse[n] = GFC_INHIBIT_REVERSE; } loop->ss = gfc_ss_terminator; Index: gcc/fortran/gfortran.h =================================================================== *** gcc/fortran/gfortran.h (revision 173212) --- gcc/fortran/gfortran.h (working copy) *************** gfc_fcoarray; *** 578,587 **** typedef enum { ! GFC_REVERSE_NOT_SET, GFC_REVERSE_SET, ! GFC_CAN_REVERSE, ! GFC_CANNOT_REVERSE } gfc_reverse; --- 578,587 ---- typedef enum { ! GFC_ENABLE_REVERSE, ! GFC_FORWARD_SET, GFC_REVERSE_SET, ! GFC_INHIBIT_REVERSE } gfc_reverse; Index: gcc/fortran/dependency.c =================================================================== *** gcc/fortran/dependency.c (revision 173212) --- gcc/fortran/dependency.c (working copy) *************** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 1807,1813 **** /* Now deal with the loop reversal logic: This only works on ranges and is activated by setting ! reverse[n] == GFC_CAN_REVERSE The ability to reverse or not is set by previous conditions in this dimension. If reversal is not activated, the value GFC_DEP_BACKWARD is reset to GFC_DEP_OVERLAP. */ --- 1807,1813 ---- /* Now deal with the loop reversal logic: This only works on ranges and is activated by setting ! reverse[n] == GFC_ENABLE_REVERSE The ability to reverse or not is set by previous conditions in this dimension. If reversal is not activated, the value GFC_DEP_BACKWARD is reset to GFC_DEP_OVERLAP. */ *************** gfc_dep_resolver (gfc_ref *lref, gfc_ref *** 1815,1839 **** && lref->u.ar.dimen_type[n] == DIMEN_RANGE) { /* Set reverse if backward dependence and not inhibited. */ ! if (reverse && reverse[n] != GFC_CANNOT_REVERSE) reverse[n] = (this_dep == GFC_DEP_BACKWARD) ? GFC_REVERSE_SET : reverse[n]; ! /* Inhibit loop reversal if dependence not compatible. */ ! if (reverse && reverse[n] != GFC_REVERSE_NOT_SET ! && this_dep != GFC_DEP_EQUAL ! && this_dep != GFC_DEP_BACKWARD ! && this_dep != GFC_DEP_NODEP) { ! reverse[n] = GFC_CANNOT_REVERSE; ! if (this_dep != GFC_DEP_FORWARD) ! this_dep = GFC_DEP_OVERLAP; } /* If no intention of reversing or reversing is explicitly inhibited, convert backward dependence to overlap. */ ! if (this_dep == GFC_DEP_BACKWARD ! && (reverse == NULL || reverse[n] == GFC_CANNOT_REVERSE)) this_dep = GFC_DEP_OVERLAP; } --- 1815,1848 ---- && lref->u.ar.dimen_type[n] == DIMEN_RANGE) { /* Set reverse if backward dependence and not inhibited. */ ! if (reverse && reverse[n] == GFC_ENABLE_REVERSE) reverse[n] = (this_dep == GFC_DEP_BACKWARD) ? GFC_REVERSE_SET : reverse[n]; ! /* Set forward if forward dependence and not inhibited. */ ! if (reverse && reverse[n] == GFC_ENABLE_REVERSE) ! reverse[n] = (this_dep == GFC_DEP_FORWARD) ? ! GFC_FORWARD_SET : reverse[n]; ! ! /* Flag up overlap if dependence not compatible with ! the overall state of the expression. */ ! if (reverse && reverse[n] == GFC_REVERSE_SET ! && this_dep == GFC_DEP_FORWARD) { ! reverse[n] = GFC_INHIBIT_REVERSE; ! this_dep = GFC_DEP_OVERLAP; ! } ! else if (reverse && reverse[n] == GFC_FORWARD_SET ! && this_dep == GFC_DEP_BACKWARD) ! { ! reverse[n] = GFC_INHIBIT_REVERSE; ! this_dep = GFC_DEP_OVERLAP; } /* If no intention of reversing or reversing is explicitly inhibited, convert backward dependence to overlap. */ ! if ((reverse == NULL && this_dep == GFC_DEP_BACKWARD) ! || (reverse != NULL && reverse[n] == GFC_INHIBIT_REVERSE)) this_dep = GFC_DEP_OVERLAP; } Index: gcc/testsuite/gfortran.dg/dependency_40.f90 =================================================================== *** gcc/testsuite/gfortran.dg/dependency_40.f90 (revision 0) --- gcc/testsuite/gfortran.dg/dependency_40.f90 (revision 0) *************** *** 0 **** --- 1,29 ---- + ! { dg-do run } + ! PR 48955 - missing array temporary when there was both a forward + ! and a backward dependency. + ! Test case slightly modified from the original one by Kacper Kowalik. + program ala + implicit none + + integer, parameter :: n = 6 + real, dimension(n), parameter :: result = [1.,10.,30.,90.,270., 243.]; + real, dimension(n) :: v0, v1 + character(len=80) :: line1, line2 + + v0 = [1.0, 3.0, 9.0, 27.0, 81.0, 243.0] + v1 = v0 + + v1(2:n-1) = v1(1:n-2) + v1(3:n) + if (any(v1 /= result)) call abort + v1 = v0 + v1(2:n-1) = v0(1:n-2) + v0(3:n) + if (any(v1 /= result)) call abort + + v1 = v0 + v1(2:n-1) = v1(3:n) + v1(1:n-2) + if (any(v1 /= result)) call abort + v1 = v0 + v1(2:n-1) = v0(3:n) + v0(1:n-2) + if (any(v1 /= result)) call abort + + end program ala