Created attachment 47987 [details] assumed_rank_19_aux.c – to be used with … As found by José and reported at https://gcc.gnu.org/ml/fortran/2020-03/msg00016.html Testing shows that assumed-rank arrays are mishandled in several ways * When required by the standard, the lower bound is not always shifted to "1" – this causes misreported bounds. * Likewise, for BIND(C), the bounds are not always the expected 0. * When allocatable/pointer, the argument cannot be assumed-rank; hence, "-1" is a perfectly valid upper and lower bound and size()/shape() shall report >= 0 and not a negative value! Likewise for dim[rank-1].extent with bind(C): the value shall not be negative. See attached test case. All code which works uses STOP and assert(). All code which doesn't uses (__builtin_)print(f) diagnostic. Caveat: I hope I haven't made any mistake when writing the test code. * * * Compile with: gfortran assumed_rank_19.f90 assumed_rank_19_aux.c When using DejaGNU/before committing: use the commented ISO_Fortran_binding.h #include as otherwise the testsuite may fail to find that file.
Created attachment 47988 [details] … with assumed_rank_19.f90
(In reply to Tobias Burnus from comment #0) > Testing shows that assumed-rank arrays are mishandled in several ways Additionally, with assumed-size arrays passed to assumed-rank dummies: both size(x) and size(x,dim=3) use "0" instead of "-1" for the last dimension (and hence: -20 in the first case). – That was the originally found issue by José.
*** Bug 94020 has been marked as a duplicate of this bug. ***
José's PR (which came earlier then mine, hmm) has also an extensive test case in attachment 47960 [details] for assumed-size arrays.
Confirmed.
Some bound issues were fixed with PR99043 – but my bet is that the BIND(C) issues still exist. (→ testcase (C + Fortran) attached to this PR). * * * Additionally: PR 94020 (duplicate of this PR) with attachment 47960 [details] seems to be mostly fixed, however for the following case, the result is wrong. if 'lb' (or second argument to 'foo') is >= 0, size2 == 0 – otherwise, size2 > 0'. Expected: same result as for PRODUCT(SHAPE(ARRAY,KIND)) (→ 16.9.179 SIZE (ARRAY [, DIM, KIND])) That is: lb = 5 → shape = [5 -1] → -5 lb = -12 → shape = [5 - 1] → -5 implicit none integer :: A(5,5) call foo(a,-12) call foo(a,0) call foo(a,5) contains subroutine foo(x,lb) integer :: lb integer :: x(5,lb:*) print *, size2(x) if (size2(x) /= -5) stop 1 end integer function size2(y) integer :: y(..) size2 = size(y) end end
I think the remaining bind(c) problems are the same bug that is causing failures in tests interoperability/cf-out-descriptor-6.f90 interoperability/fc-descriptor-6.f90 interoperability/fc-out-descriptor-6.f90 interoperability/ff-descriptor-6.f90 intrinsics/size.f90 from the new TS29113 testsuite posted at https://gcc.gnu.org/pipermail/gcc-patches/2021-July/574115.html TS29113 says: (1) for an assumed-rank object that is associated with an assumed-size array, the result has the value −1 if DIM is present and equal to the rank of ARRAY Also in the description of CFI_cdesc_t in section 8.3.3, it says In a C descriptor of an assumed-size array, the extent member of the last element of the dim member has the value −1. Neither of these things appears to be happening. This might also be related to PR 94289?
In gfc_desc_to_cfi_desc (in libgfortran/runtime/ISO_Fortran_binding.c): /* Assumed size arrays have gfc ubound == 0 and CFI extent = -1. */ if (n == GFC_DESCRIPTOR_RANK (s) - 1 && GFC_DESCRIPTOR_LBOUND(s, n) == 1 && GFC_DESCRIPTOR_UBOUND(s, n) == 0) d->dim[n].extent = -1; else d->dim[n].extent = (CFI_index_t)GFC_DESCRIPTOR_UBOUND(s, n) - (CFI_index_t)GFC_DESCRIPTOR_LBOUND(s, n) + 1; The comment and test are only correct if the lower bound of the array dimension either defaults to 1 or is explicitly specified as 1. It does appear that the ubound == 0 part is true, but this means e.g. an array dimension specified as -3:* is indistinguishable from -3:0. I think this needs to be corrected at the point where the GFC descriptor is created; perhaps set ubound = lbound - 1? Or also set lbound = 1 as the code snippet above checks? Assumed-size arrays can't be pointers or allocatable so their bounds don't need to be preserved across calls, but maybe GFC descriptors are used for other purposes where it matters?
I think my patch for moving the CFI<->GFC conversion to FE-generated code partially helps, https://gcc.gnu.org/pipermail/gcc-patches/2021-September/578904.html However, I still see the following issues: integer :: y(-1:3,4,*) call test(y, num=0) -> subroutine test (x, num) integer :: x(..), num print *, shape(x) print *, size (x); if (size (x) /= -20) print *, 'ERROR: 1' 5 4 -1 0 That's unrelated to BIND(C) as there is no BIND(C). The problem is in gcc/fortran/trans-intrinsic.c, where gfc_conv_intrinsic_shape has code for AS_ASSUMED_RANK but gfc_conv_intrinsic_size does not. → Same result with BIND(C). Likewise for: print *, size (x, dim=3); if (size (x, dim=3) /= -1) print *, 'ERROR: 4' which also shows 0 instead of -1. – Same, except that the by-dimension results are not summed up. * * * 5 4 -1 ERROR 65 Expected is: 5, 4, 1 – the problem is that the last dimension is -1:-1. Looks as if gfc_conv_intrinsic_shape misses a check whether the argument is !attr.allocatable && !attr.pointer with symbol_attribute attr = gfc_expr_attr (expr); before assuming that the argument can be assumed size. → Same issue with BIND(C). * * * The following does not occur with BIND(C) - with my patch applied - but without BIND(C): ERROR: 25 5 4 -1 Another SHAPE issue. Issue: Last value should be '1' not '-1' as the argument is: allocate (B(-1:3,4,-1:-1)) The dump shows: test (&b, &C.4394); thus, the actual argument has the declared bounds – while in the callee is should have '1' as lower bound. I am not sure whether that's a caller or a callee problem. If handled in the caller, the following needs to work: subroutine bar(x) integer, allocatable :: x(..) print*, lbound(x) ! -> has allocated bounds call test(x) i.e. assumed-rank allocatable -> assumed-rank nonallocatable. In general, we have code which generates a temporary descriptor, e.g. for 'call test(b(:,:,:))'. (With C binding, we generate code which sets lbound to 0 in the caller.) → gfc_conv_procedure_call for the call handling / gfc_conv_expr_descriptor for the conversion. I wonder whether we need to set se->direct_byref = 0 in this case. Additionally failing: 5 4 -1 ERROR: 31 -1 ERROR 34 Same cause – just exposed via UBOUND() and UBOUND(…,dim=3). And also only without BIND(C).
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>: https://gcc.gnu.org/g:00f6de9c69119594f7dad3bd525937c94c8200d0 commit r12-3897-g00f6de9c69119594f7dad3bd525937c94c8200d0 Author: Tobias Burnus <tobias@codesourcery.com> Date: Mon Sep 27 14:04:54 2021 +0200 Fortran: Fix assumed-size to assumed-rank passing [PR94070] This code inlines the size0 and size1 libgfortran calls, the former is still used by libgfortan itself (and by old code). Besides permitting more optimizations, it also permits to handle assumed-rank dummies better: If the dummy argument is a nonpointer/nonallocatable, an assumed-size actual arg is repesented by having ubound == -1 for the last dimension. However, for allocatable/pointers, this value can also exist. Hence, the dummy arg attr has to be honored. For that reason, when calling an assumed-rank procedure with nonpointer, nonallocatable dummy arguments, the bounds have to be updated to avoid the case ubound == -1 for the last dimension. PR fortran/94070 gcc/fortran/ChangeLog: * trans-array.c (gfc_tree_array_size): New function to find size inline (whole array or one dimension). (array_parameter_size): Use it, take stmt_block as arg. (gfc_conv_array_parameter): Update call. * trans-array.h (gfc_tree_array_size): Add prototype. * trans-decl.c (gfor_fndecl_size0, gfor_fndecl_size1): Remove these global vars. (gfc_build_intrinsic_function_decls): Remove their initialization. * trans-expr.c (gfc_conv_procedure_call): Update bounds of pointer/allocatable actual args to nonallocatable/nonpointer dummies to be one based. * trans-intrinsic.c (gfc_conv_intrinsic_shape): Fix case for assumed rank with allocatable/pointer dummy. (gfc_conv_intrinsic_size): Update to use inline function. * trans.h (gfor_fndecl_size0, gfor_fndecl_size1): Remove var decl. libgfortran/ChangeLog: * intrinsics/size.c (size0, size1): Comment that now not used by newer compiler code. libgomp/ChangeLog: * testsuite/libgomp.oacc-fortran/privatized-ref-2.f90: Update expected dg-note output. gcc/testsuite/ChangeLog: * gfortran.dg/c-interop/cf-out-descriptor-6.f90: Remove xfail. * gfortran.dg/c-interop/size.f90: Remove xfail. * gfortran.dg/intrinsic_size_3.f90: Update scan-tree-dump-times. * gfortran.dg/transpose_optimization_2.f90: Likewise. * gfortran.dg/size_optional_dim_1.f90: Add scan-tree-dump-not. * gfortran.dg/assumed_rank_22.f90: New test. * gfortran.dg/assumed_rank_22_aux.c: New test.
There are still some bugs present with class arrays. E.g., this test case ICEs: module m type :: t integer :: id real :: xyz(3) end type contains subroutine testit2p(a) class(t), pointer :: a(..) print *, shape(a) end subroutine end module SHAPE is implemented as a library call and it's trying to copy the pointer array to a temporary instead of just dereferencing it. I'm looking at replacing that logic to handle it inline same as LBOUND/UBOUND.
The master branch has been updated by Sandra Loosemore <sandra@gcc.gnu.org>: https://gcc.gnu.org/g:1af78e731feb9327a17c99ebaa19a4cca1125caf commit r12-4591-g1af78e731feb9327a17c99ebaa19a4cca1125caf Author: Sandra Loosemore <sandra@codesourcery.com> Date: Tue Oct 19 21:11:15 2021 -0700 Fortran: Fixes and additional tests for shape/ubound/size [PR94070] This patch reimplements the SHAPE intrinsic to be inlined similarly to LBOUND and UBOUND, instead of as a library call, to avoid an unnecessary array copy. Various bugs are also fixed. gcc/fortran/ PR fortran/94070 * expr.c (gfc_simplify_expr): Handle GFC_ISYM_SHAPE along with GFC_ISYM_LBOUND and GFC_ISYM_UBOUND. * trans-array.c (gfc_conv_ss_startstride): Likewise. (set_loop_bounds): Likewise. * trans-intrinsic.c (gfc_trans_intrinsic_bound): Extend to handle SHAPE. Correct logic for zero-size special cases and detecting assumed-rank arrays associated with an assumed-size argument. (gfc_conv_intrinsic_shape): Deleted. (gfc_conv_intrinsic_function): Handle GFC_ISYM_SHAPE like GFC_ISYM_LBOUND and GFC_ISYM_UBOUND. (gfc_add_intrinsic_ss_code): Likewise. (gfc_walk_intrinsic_bound): Likewise. gcc/testsuite/ PR fortran/94070 * gfortran.dg/c-interop/shape-bindc.f90: New test. * gfortran.dg/c-interop/shape-poly.f90: New test. * gfortran.dg/c-interop/size-bindc.f90: New test. * gfortran.dg/c-interop/size-poly.f90: New test. * gfortran.dg/c-interop/ubound-bindc.f90: New test. * gfortran.dg/c-interop/ubound-poly.f90: New test.
I believe this issue is fixed now. The original test case at the top of the issue has been committed (now named assumed_rank_22), and my last commit added a set of tests for the thing that was still triggering the ICE plus some more for bind(c).