Re: [Patch] Fortran: Fix assumed-size to assumed-rank passing [PR94070]
Hi Thomas, hi Harald, hi all,
now committed r12-3897-g00f6de9c69119594f7dad3bd525937c94c8200d0
with the following changes:
* Removed now unused gfor_fndecl_size0/gfor_fndecl_size1 (trans{-decl.c,.h})
* Add a scan-dump-not check for those.
See below for some comments.
On 24.09.21 22:38, Thomas Koenig wrote:
OK for mainline?
As promised on IRC, here's the review.
Thanks for the quick review :-)
Maybe you can add a test case which shows that the call to the size
intrinsic really does not happen.
OK with that.
I think you mean to the (_gfortran_)size0/size1 function libgfortran.
Unsurprisingly, the intrinsic itself _is_ still used and the simplify.c +
trans-instrinsic.c's functions are called. However, the libgfortran
functions size0/size1 shouldn't be callable – given that I deleted
the function decl in the front end. I have nonetheless added some
-fdump-tree checks that size0 and size1 do not appear.
Hmm, looking at my patch again – I think I did intent to remove
the decl – but did not actually do it.
In this patch, I have now actually done what I intended and wrote
above: I removed the gfor_fndecl_size0/gfor_fndecl_size1 also from
trans.h (declared) and trans-decl.c (global var, init with fn decl).
size_optional_dim_1.f90 was the only testcase that used size1 before
the patch (it also used size0). Thus, I added the dump check to it
and to the new assumed_rank_22.f90, which has 7 size0 calls with an
unpatched compiler.
Thus: Thanks for asking for the dump check as it showed that I did
forget to remove something ... :-)
Conclusion: Reviews are very helpful :-)
* * *
As the following email by Harald did not show up at the gcc-patches mailing
list:
you can find it at
https://gcc.gnu.org/pipermail/fortran/2021-September/056578.html
In my email, it shows up with "To: fortran@" and "CC: gcc-patches@", thus,
I have no idea why it did not arrive at the mailing-list archive :-(
On 24.09.21 23:12, Harald Anlauf via Fortran wrote:
I played around with your patch and was unable to break it.
Good. That means we can now hand it over to Gerald ;-)
Are you tracking the xfailed parts?
For my newly added xfail, it is fixed by the posted CFI<->GFC
descriptor patch, which I am currently updating (for several reasons).
Otherwise, I lost a bit track of the various TS29113, C-interop,
class(*), type(*), dimension(..) etc. PRs. I think they do cover
most of it. – Besides that CFI/GFC descriptor patch, some additional
patches are still in Sandra's and my pipeline.
And once the CFI/GFC descriptor patch is in, I think it makes sense
to check a bunch of PRs to see whether they are now fixed or something
still needs to be done. Likewise for José's patches. I think they
will be partially superseded by the patches committed, submitted or
soon to be submitted – but I am sure not all issues are fixed.
While playing I stumbled over the fact that when allocating an array
with a dimension that has extent 0, e.g. 4:-5, the lbound gets reset
to 1 and ubound set to 0.
I am not sure, whether I fully understand what you wrote. For:
integer, allocatable :: a(:)
allocate(a(4:-5))
print *, size(a), size(a, dim=1), shape(a) ! should print the '0 0 0'
print *, lbound(a, dim=1) ! should print '1'
print *, ubound(a, dim=1) ! should print '0'
where the last line is due to F2018, 16.9.196, which has:
'Otherwise, if DIM is present, the result has a value equal to the
number of elements in dimension DIM of ARRAY.'
And lbound(a,dim=1) == 1 due to the "otherwise" case of F2018:16.9.109 LBOUND:
"Case (i): If DIM is present, ARRAY is a whole array, and either
ARRAY is an assumed-size array of rank DIM or dimension DIM of
ARRAY has nonzero extent, the result has a value equal to the
lower bound for subscript DIM of ARRAY. Otherwise, if DIM is
present, the result value is 1."
And when doing
call f(a)
call g(a)
with 'subroutine f(x); integer :: x(:)'
and 'subroutine g(y); integer :: y(..)'
Here, ubound == 0 due to the reason above and lbound is set to
the declared lower bound, which is for 'x' the default ("1") but
could also be 5 with "x(5:)" and for 'y' it cannot be specified.
For 'x', see last sentence of F2018:8.5.8.3. For 'y', I did not
find the exact location but it follows alongsize.
With BIND(C) applied to f and g, ubound remains the same but
lbound is now 0 instead of 1.
Has the standard has changed in this respect?
I doubt it, but only looked at F2018 and not at older standards.
I am probably not the best person to review the trans-* parts, but
I did not spot anything I could point at, and the dump-tree looked
reasonable. Therefore OK from my side.
Thanks for the work!
Thanks also for your review.
Thanks,
Tobias
PS: I saw that we recently had a couple of double reviews. I think it is
useful if multiple persons look at patches, but hope that we do not
start requiring two reviews for each patch ;-)
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 20
[committed] libgomp.oacc-fortran/privatized-ref-2.f90: Fix dg-note (was: [Patch] Fortran: Fix assumed-size to assumed-rank passing [PR94070])
On 27.09.21 14:07, Tobias Burnus wrote:
now committed r12-3897-g00f6de9c69119594f7dad3bd525937c94c8200d0
I accidentally changed dg-note to dg-message when updating the expected
output, as the dump has changed. (Copying seemingly the sorry line
instead of the dg-note lines as template.)
Changed back to dg-note & committed as
r12-3898-gda1f6391b7c255e4e2eea983832120eff4f7d3df.
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
commit da1f6391b7c255e4e2eea983832120eff4f7d3df
Author: Tobias Burnus
Date: Mon Sep 27 14:33:39 2021 +0200
libgomp.oacc-fortran/privatized-ref-2.f90: Fix dg-note
In my last commit, r12-3897-g00f6de9c69119594f7dad3bd525937c94c8200d0,
which inlined array-size code, I had to update the expected output. However,
in doing so, I accidentally (copy'n'paste) changed dg-note into dg-message.
libgomp/
* testsuite/libgomp.oacc-fortran/privatized-ref-2.f90: Change
dg-message back to dg-note.
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
index 2ff60226109..588f528b2d5 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
@@ -75,9 +75,9 @@ contains
! { dg-note {variable 'A\.[0-9]+' declared in block isn't candidate for adjusting OpenACC privatization level: static} "" { target *-*-* } l_compute$c_compute }
array = [(-2*i, i = 1, size(array))]
!$acc loop gang private(array) ! { dg-line l_loop[incr c_loop] }
-! { dg-message {variable 'i' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_loop$c_loop }
-! { dg-message {variable 'array\.[0-9]+' in 'private' clause is candidate for adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_loop }
-! { dg-message {variable 'array\.[0-9]+' ought to be adjusted for OpenACC privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop }
+! { dg-note {variable 'i' in 'private' clause isn't candidate for adjusting OpenACC privatization level: not addressable} "" { target *-*-* } l_loop$c_loop }
+! { dg-note {variable 'array\.[0-9]+' in 'private' clause is candidate for adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_loop }
+! { dg-note {variable 'array\.[0-9]+' ought to be adjusted for OpenACC privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop }
! { dg-message {sorry, unimplemented: target cannot support alloca} PR65181 { target openacc_nvidia_accel_selected } l_loop$c_loop }
Aw: Re: [PATCH] PR fortran/102458 - ICE tree check: expected array_type, have pointer_type in gfc_conv_array_initializer, at fortran/trans-array.c:6136
Hi Thomas, > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > > It was actually 10.1.12 :-) dang, you're right. Steve had it right, and I failed miserably on copy&paste. I should fix the comment. > OK for trunk. > > Thanks for the patch! Thanks for the review, which came after Jerry's! I should have waited for yours, too. Harald > Best regards > > Thomas
[PATCH, RFH] PR 102458 - simplification of SIZE intrinsic applied to automatic array
Dear Fortranner,
while looking at an issue that bugged me when analyzing PR 102458,
I got sort of stuck at the following case:
subroutine s4
integer, parameter :: n = 4
integer:: x(transfer(n, n)) = 1
integer:: y(2*int(n) - n) = 2
integer, parameter :: k = size (x)
integer, parameter :: m = size (y)
print *, k, m
if (k /= m .or. m /= n) stop 1
end
This should be entirely legal code, and it is accepted by Intel.
We lack to simplify the SIZE:
pr102458-part2.f90:5:34:
5 | integer, parameter :: k = size (x)
| 1
Error: Array 'x' at (1) is a variable, which does not reduce to a constant
expression
pr102458-part2.f90:6:34:
6 | integer, parameter :: m = size (y)
| 1
Error: Array 'y' at (1) is a variable, which does not reduce to a constant
expression
My debugging sessions suggest that we fail to resolve the intrinsics -
in the present testcase it is TRANSFER and INT - at the right time,
so that simplification fails when processing SIZE(x), SIZE(y).
What is the best place to do this?
The following patch fixes this issue, but I am unsure whether this
is the right approach:
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index b46cbfa90ab..5d37cfef183 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -7527,6 +7527,9 @@ simplify_size (gfc_expr *array, gfc_expr *dim, int k)
return simplified;
}
+ if (array->ref && array->ref->u.ar.as)
+gfc_resolve_array_spec (array->ref->u.ar.as, 0);
+
if (dim == NULL)
{
if (!gfc_array_size (array, &size))
It is also the only change so far that I can come up with which regtests
fine. (Enforcing resolution too early can lead to funny problems.)
Comments? Further insight on where to look instead?
Thanks,
Harald
Re: [Patch] Fortran: Fix assumed-size to assumed-rank passing [PR94070]
Hi Harald, hi all, On 27.09.21 21:34, Harald Anlauf via Gcc-patches wrote: [...] here is what I played with: program p implicit none integer, pointer :: x(:,:) allocate (x(-3:3,4:0)) print *, "lbound =", lbound (x) call sub (x) contains subroutine sub (y) integer, pointer :: y(..) print *, "lbound =", lbound (y) print *, "ubound =", ubound (y) end subroutine sub end (Slightly shortened) This prints: lbound = -3 1 lbound = -3 1 ubound = 3 0 For some reason Intel prints different lbound lbound = -3 1 lbound = -3 4 ubound = 3 3 First, that should be rather unrelated to my patch as here the dummy argument is a pointer (could be also allocatable), where the argument is passed through "as is". For the latter reason, the expectation is that, both in the caller and callee, the result is the same - which ifort's result isn't. Otherwise, the quote from F2018 of my previous email applies: F2018:16.9.109 LBOUND has for "case(i)", i.e. with a 'dim' argument the following. The case without 'dim' just iterates through case (i) for each dim. Thus: "If DIM is present, ARRAY is a whole array, and either ARRAY is an assumed-size array of rank DIM or dimension DIM of ARRAY has nonzero extent, the result has a value equal to the lower bound for subscript DIM of ARRAY. Otherwise, if DIM is present, the result value is 1." Here, we assume dim=2 is present [either directly or via case(ii)], ARRAY is a whole array but it neither is of assumed size nor has nonzero extent. Hence, the "otherwise" applies and the result is 1 - as gfortran has and ifort has in the caller. The ubound then follows – there is a long list of conditions which are all not fulfilled (you could check to confirm this) and remaining is then only: "Otherwise, if DIM is present, the result has a value equal to the number of elements in dimension DIM of ARRAY." And following your quote, there are zero elements in dim=2 of your array. → ubound(…, dim=2) == 0. So for the first dimension everything is fine, but for the second dim, which has extent zero, my question is: what should the lbound be? 1 or 4? 1 With BIND(C) applied to f and g, ubound remains the same but lbound is now 0 instead of 1. I haven't check the BIND(C) complications. There aren't real complications, except that with the C descriptor, there is no lbound/ubound anymore but the descr->dim[i].lower_bound + ...[i].extent are directly referenced. For GCC, the difference is that GCC uses lbound + ubound and CFI uses lbound + extent. (The other difference is the use of stride in number of elements vs. sm/stride multipler in number of bytes.) The CFI array descriptor is IMHO more sensible than the GFC descriptor, but for legacy reasons, we still carry it along. For "common" Fortran code, I looked at 9.7.1.2(1): ... It is the word "determine" in first sentence that made me stumble. I think the Fortran standard does often not really tell what the bounds are but just what lbound/ubound/size/shape produce. That's perfectly fine – and permits different implementations in the background. (For C interop, they had to specify, for obvious reasons, what's in the descriptor itself.) Cheers, Tobias - Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
