From: Paul Richard Thomas <paul.richard.thomas@gmail.com>
To: Thomas Koenig <tkoenig@netcologne.de>
Cc: "fortran@gcc.gnu.org" <fortran@gcc.gnu.org>,
gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [patch, fortran] [4.6/4.7 Regression] Fix PR 48955
Date: Tue, 24 May 2011 22:06:00 -0000 [thread overview]
Message-ID: <BANLkTi=oKkyxhz=hM+zH-LhhhYOOTnPaOg@mail.gmail.com> (raw)
In-Reply-To: <BANLkTintJzxP6L8DAMJyKcKNMw4HErOvDQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]
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 <pault@gcc.gnu.org>
Thomas Koenig <tkoenig@gcc.gnu.org>
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 <tkoenig@gcc.gnu.org>
PR fortran/48955
* gfortran.dg/dependency_40.f90 : New test.
[-- Attachment #2: submit.diff --]
[-- Type: text/x-patch, Size: 6677 bytes --]
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
next prev parent reply other threads:[~2011-05-24 21:18 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-05-22 18:59 Thomas Koenig
2011-05-22 20:56 ` Paul Richard Thomas
2011-05-24 22:06 ` Paul Richard Thomas [this message]
2011-05-26 9:22 ` Thomas Koenig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='BANLkTi=oKkyxhz=hM+zH-LhhhYOOTnPaOg@mail.gmail.com' \
--to=paul.richard.thomas@gmail.com \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=tkoenig@netcologne.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).