Thanks Paul,

I tried the patch and it does fix CFI_is_contigous()


There is still an issue with INTENT(IN)


For example, if we declare

integer :: buf(0:1025,0:1025)

and then

call foo(buf(1:1024,1:1024)

in which foo declares buf as INTENT(IN)


You previously explained buf will be copied in order to enforce INTENT(IN),

and that is why base_addr is not &buf(0,0).

The dim[] of the CFI_cdesc_t structure describes the subarray,

CFI_is_contiguous() returns false, but (it seems) the array was copied *and packed*

(base_addr was malloc'ed with size=1024*1024*4).

Bottom line, the CFI_cdesc_t description does not match the contiguous layout of the (sub)array copy, and this is an issue.



I previously stated that in my opinion, copy-in was not desirable for performance reasons.

I also found an other potential issue with copy-in.

If in Fortran, we

call foo(buf(0,0))

then the C subroutine can only access buf(0,0), and other elements such as buf(1025,1025) cannot be accessed.

Such elements are valid in buf, but out of bounds in the copy (that contains a single element).

Strictly speaking, I cannot say whether this is a violation of the standard or not, but I can see how this will

break a lot of existing apps (once again, those apps might be incorrect in the first place, but most of us got used

to them working).

To me, this is a second reason why copy-in is not desirable (at least as a default option).



Cheers,


Gilles

On 4/9/2019 7:18 PM, Paul Richard Thomas wrote:
The most part of this patch is concerned with implementing calls from
C of of fortran bind c procedures with assumed shape or assumed rank
dummies to completely fix PR89843. The conversion of the descriptors
from CFI to gfc occur on entry to and reversed on exit from the
procedure.

This patch is safe for trunk, even at this late stage, because its
effects are barricaded behind the tests for CFI descriptors. I believe
that it appropriately rewards the bug reporters to have this feature
work as well as possible at release.

Between comments and the ChangeLogs, this patch is self explanatory.

Bootstrapped and regtested on FC29/x86_64 - OK for trunk?

Paul

2019-04-09  Paul Thomas  <pa...@gcc.gnu.org>

     PR fortran/89843
     * trans-decl.c (gfc_get_symbol_decl): Assumed shape and assumed
     rank dummies of bind C procs require deferred initialization.
     (convert_CFI_desc): New procedure to convert incoming CFI
     descriptors to gfc types and back again.
     (gfc_trans_deferred_vars): Call it.
     * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Null the CFI
     descriptor pointer. Free the descriptor in all cases.

     PR fortran/90022
     * trans-decl.c (gfc_get_symbol_decl): Make sure that the se
     expression is a pointer type before converting it to the symbol
     backend_decl type.

2019-04-09  Paul Thomas  <pa...@gcc.gnu.org>

     PR fortran/89843
     * gfortran.dg/ISO_Fortran_binding_4.f90: Modify the value of x
     in ctg. Test the conversion of the descriptor types in the main
     program.
     * gfortran.dg/ISO_Fortran_binding_10.f90: New test.
     * gfortran.dg/ISO_Fortran_binding_10.c: Called by it.

     PR fortran/90022
     * gfortran.dg/ISO_Fortran_binding_1.c: Correct the indexing for
     the computation of 'ans'. Also, change the expected results for
     CFI_is_contiguous to comply with standard.
     * gfortran.dg/ISO_Fortran_binding_1.f90: Correct the expected
     results for CFI_is_contiguous to comply with standard.
     * gfortran.dg/ISO_Fortran_binding_9.f90: New test.
     * gfortran.dg/ISO_Fortran_binding_9.c: Called by it.

2019-04-09  Paul Thomas  <pa...@gcc.gnu.org>

     PR fortran/89843
     * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Only
     return immediately if the source pointer is null. Bring
     forward the extraction of the gfc type. Extract the kind so
     that the element size can be correctly computed for sections
     and components of derived type arrays. Remove the free of the
     CFI descriptor since this is now done in trans-expr.c.
     (gfc_desc_to_cfi_desc): Only allocate the CFI descriptor if it
     is not null.
     (CFI_section): Normalise the difference between the upper and
     lower bounds by the stride to correctly calculate the extents
     of the section.

     PR fortran/90022
     * runtime/ISO_Fortran_binding.c (CFI_is_contiguous) : Return
     1 for true and 0 otherwise to comply with the standard. Correct
     the contiguity check for rank 3 and greater by using the stride
     measure of the lower dimension rather than the element length.

Reply via email to