* [patch, fortran] Improve dependency checking
@ 2019-07-25 13:08 Thomas Koenig
2019-07-25 14:31 ` Steve Kargl
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Koenig @ 2019-07-25 13:08 UTC (permalink / raw)
To: fortran, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 991 bytes --]
Hello world,
the attached pach does some more work in gfc_check_dependency for
the case where an identity between arguments would also lead
to problems.
It does not lead to removal of the warning with -Warray-temporaries,
because that is still generated by the call to library function.
Instead, I checked for the names of the variables which used to
be introduced by the matmul inlining code.
Regression-tested. OK for trunk?
Regards
Thomas
2019-07-25 Thomas Koenig <tkoenig@gcc.gnu.org>
PR fortran/65819
* dependency.h (gfc_dep_resovler): Add optional argument identical.
* dependency.c (gfc_check_dependency): Do not alway return 1 if
the symbol is the same. Pass on identical to gfc_dep_resolver.
(gfc_check_element_vs_element): Whitespace fix.
(gfc_dep_resolver): Adjust comment for function. If identical is
true, return 1 if any overlap has been found.
2019-07-25 Thomas Koenig <tkoenig@gcc.gnu.org>
PR fortran/65819
* gfortran.dg/dependency_54.f90: New test.
[-- Attachment #2: p1.diff --]
[-- Type: text/x-patch, Size: 3792 bytes --]
Index: dependency.c
===================================================================
--- dependency.c (Revision 273733)
+++ dependency.c (Arbeitskopie)
@@ -1351,13 +1351,10 @@ gfc_check_dependency (gfc_expr *expr1, gfc_expr *e
return 0;
}
- if (identical)
- return 1;
-
/* Identical and disjoint ranges return 0,
overlapping ranges return 1. */
if (expr1->ref && expr2->ref)
- return gfc_dep_resolver (expr1->ref, expr2->ref, NULL);
+ return gfc_dep_resolver (expr1->ref, expr2->ref, NULL, identical);
return 1;
@@ -1884,6 +1881,7 @@ gfc_check_element_vs_element (gfc_ref *lref, gfc_r
if (i > -2)
return GFC_DEP_NODEP;
+
return GFC_DEP_EQUAL;
}
@@ -2084,13 +2082,15 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref
/* Finds if two array references are overlapping or not.
Return value
- 2 : array references are overlapping but reversal of one or
+ 2 : array references are overlapping but reversal of one or
more dimensions will clear the dependency.
- 1 : array references are overlapping.
- 0 : array references are identical or not overlapping. */
+ 1 : array references are overlapping, or identical is true and
+ there is some kind of overlap.
+ 0 : array references are identical or not overlapping. */
int
-gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse)
+gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse,
+ bool identical)
{
int n;
int m;
@@ -2124,11 +2124,15 @@ int
case REF_ARRAY:
+ /* For now, treat all coarrays as dangerous. */
+ if (lref->u.ar.codimen || rref->u.ar.codimen)
+ return 1;
+
if (ref_same_as_full_array (lref, rref))
- return 0;
+ return identical;
if (ref_same_as_full_array (rref, lref))
- return 0;
+ return identical;
if (lref->u.ar.dimen != rref->u.ar.dimen)
{
@@ -2180,6 +2184,8 @@ int
gcc_assert (rref->u.ar.dimen_type[n] == DIMEN_ELEMENT
&& lref->u.ar.dimen_type[n] == DIMEN_ELEMENT);
this_dep = gfc_check_element_vs_element (rref, lref, n);
+ if (identical && this_dep == GFC_DEP_EQUAL)
+ this_dep = GFC_DEP_OVERLAP;
}
/* If any dimension doesn't overlap, we have no dependency. */
@@ -2240,6 +2246,9 @@ int
know the worst one.*/
update_fin_dep:
+ if (identical && this_dep == GFC_DEP_EQUAL)
+ this_dep = GFC_DEP_OVERLAP;
+
if (this_dep > fin_dep)
fin_dep = this_dep;
}
@@ -2253,7 +2262,7 @@ int
/* Exactly matching and forward overlapping ranges don't cause a
dependency. */
- if (fin_dep < GFC_DEP_BACKWARD)
+ if (fin_dep < GFC_DEP_BACKWARD && !identical)
return 0;
/* Keep checking. We only have a dependency if
@@ -2267,11 +2276,14 @@ int
rref = rref->next;
}
+ /* Assume the worst if we nest to different depths. */
+ if (lref || rref)
+ return 1;
+
/* If we haven't seen any array refs then something went wrong. */
gcc_assert (fin_dep != GFC_DEP_ERROR);
- /* Assume the worst if we nest to different depths. */
- if (lref || rref)
+ if (identical && fin_dep != GFC_DEP_NODEP)
return 1;
return fin_dep == GFC_DEP_OVERLAP;
Index: dependency.h
===================================================================
--- dependency.h (Revision 273733)
+++ dependency.h (Arbeitskopie)
@@ -37,7 +37,7 @@ int gfc_check_fncall_dependency (gfc_expr *, sym_i
int gfc_check_dependency (gfc_expr *, gfc_expr *, bool);
int gfc_expr_is_one (gfc_expr *, int);
-int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *);
+int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool identical = false);
int gfc_are_equivalenced_arrays (gfc_expr *, gfc_expr *);
gfc_expr * gfc_discard_nops (gfc_expr *);
[-- Attachment #3: dependency_54.f90 --]
[-- Type: text/x-fortran, Size: 864 bytes --]
! { dg-do run }
! { dg-additional-options "-fdump-tree-original -ffrontend-optimize" }
! PR 65819 - this used to cause a temporary in matmul inlining.
! Check that these are absent by looking for the names of the
! temporary variables.
program main
implicit none
real, dimension(3,3,3) :: f
real, dimension(3,3) :: res
real, dimension(2,3,3) :: backup
integer :: three
integer :: i
data f(1,:,:) /9*-42./
data f(2:3,:,:) /2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61/
data res /652, 772, 984, 2010, 2406, 3082, 3402, 4086, 5242/
three = 3
backup = f(2:3,:,:)
f(1, 1:three, :) = matmul(f(2,1:3,2:3), f(3,2:3,:))
if (any (res /= f(1,:,:))) stop 1
if (any (f(2:3,:,:) /= backup)) stop 2
end program main
! { dg-final { scan-tree-dump-not "mma" "original" } }
! { dg-final { scan-tree-dump-not "mmb" "original" } }
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, fortran] Improve dependency checking
2019-07-25 13:08 [patch, fortran] Improve dependency checking Thomas Koenig
@ 2019-07-25 14:31 ` Steve Kargl
2019-07-25 14:52 ` Thomas Koenig
0 siblings, 1 reply; 5+ messages in thread
From: Steve Kargl @ 2019-07-25 14:31 UTC (permalink / raw)
To: Thomas Koenig; +Cc: fortran, gcc-patches
On Thu, Jul 25, 2019 at 02:57:35PM +0200, Thomas Koenig wrote:
> Index: dependency.h
> ===================================================================
> --- dependency.h (Revision 273733)
> +++ dependency.h (Arbeitskopie)
> @@ -37,7 +37,7 @@ int gfc_check_fncall_dependency (gfc_expr *, sym_i
> int gfc_check_dependency (gfc_expr *, gfc_expr *, bool);
> int gfc_expr_is_one (gfc_expr *, int);
>
> -int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *);
> +int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool identical = false);
This is changing the prototype. I would expect to see
int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool);
--
Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, fortran] Improve dependency checking
2019-07-25 14:31 ` Steve Kargl
@ 2019-07-25 14:52 ` Thomas Koenig
2019-07-25 16:26 ` Steve Kargl
0 siblings, 1 reply; 5+ messages in thread
From: Thomas Koenig @ 2019-07-25 14:52 UTC (permalink / raw)
To: sgk; +Cc: fortran, gcc-patches
Hi Steve,
>> -int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *);
>> +int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool identical = false);
> This is changing the prototype. I would expect to see
>
>
> int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool);
Usig C++'s optional arguments is actually quite useful, it's used
already used in a few places in the front end.
The idea is that you don't need to touch the other callers, just the
ones where the new argument matters.
However, in this particular case, it would also be possible to ajust
all other callers (exactly one), if you prefer.
Regards
Thomas
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, fortran] Improve dependency checking
2019-07-25 14:52 ` Thomas Koenig
@ 2019-07-25 16:26 ` Steve Kargl
2019-07-25 16:40 ` Thomas Koenig
0 siblings, 1 reply; 5+ messages in thread
From: Steve Kargl @ 2019-07-25 16:26 UTC (permalink / raw)
To: Thomas Koenig; +Cc: fortran, gcc-patches
On Thu, Jul 25, 2019 at 04:42:44PM +0200, Thomas Koenig wrote:
> Hi Steve,
>
> >> -int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *);
> >> +int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool identical = false);
> > This is changing the prototype. I would expect to see
> >
> >
> > int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool);
>
> Usig C++'s optional arguments is actually quite useful, it's used
> already used in a few places in the front end.
>
> The idea is that you don't need to touch the other callers, just the
> ones where the new argument matters.
>
> However, in this particular case, it would also be possible to ajust
> all other callers (exactly one), if you prefer.
>
Ah, I don't speak C++, and didn't know one could corrupt a
C prototype in this manner. A quick glance of gfortran.h
indeed shows a few more occurences of "bool xxx = false".
I suppose the patch is then OK.
PS: watch for long lines.
--
Steve
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch, fortran] Improve dependency checking
2019-07-25 16:26 ` Steve Kargl
@ 2019-07-25 16:40 ` Thomas Koenig
0 siblings, 0 replies; 5+ messages in thread
From: Thomas Koenig @ 2019-07-25 16:40 UTC (permalink / raw)
To: sgk; +Cc: fortran, gcc-patches
Hi Steve,
> Ah, I don't speak C++, and didn't know one could corrupt a
> C prototype in this manner. A quick glance of gfortran.h
> indeed shows a few more occurences of "bool xxx = false".
> I suppose the patch is then OK.
I don't use many C++ features, but I use this one because I feel
it should really be in C :-)
> PS: watch for long lines.
Corrected and committed as r273807.
Thanks for the review!
Regards
Thomas
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-25 16:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-25 13:08 [patch, fortran] Improve dependency checking Thomas Koenig
2019-07-25 14:31 ` Steve Kargl
2019-07-25 14:52 ` Thomas Koenig
2019-07-25 16:26 ` Steve Kargl
2019-07-25 16:40 ` Thomas Koenig
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).