Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-26 Thread Paul Richard Thomas
Hi Manfred,

I took a look at it and couldn't see what the problem is. I'll have
another stab at it tomorrow morning.

Cheers

Paul

On Fri, 26 Apr 2019 at 16:39, Manfred Schwarb  wrote:
>
> Hi,
>
> I still see this issue on 32bit, is the plan to let
> the test ISO_Fortran_binding_4.f90 be broken for the 9.0 release?
>
> Cheers,
> Manfred
>
>
> Am 15.04.19 um 10:30 schrieb Dominique d'Humières:
> > Hi Paul,
> >
> > I have found another glitch with -m32 and -O1 or -Os, but not with other 
> > values:
> >
> > % gfc /opt/gcc/_clean/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_4.f90 
> > -m32 -O
> > % ./a.out
> >  FAIL
> > Note: The following floating-point exceptions are signalling: IEEE_DENORMAL
> > STOP 1
> >
> > This looks tricky: if I add a line
> >
> >   print *, x
> >
> > before
> >
> >   if (any (abs (x - [1.,20.,3.,40.,5.,60.]) > 1.e-6)) stop 2
> >
> > the test succeeds!-(
> >
> > Also you don’t want pr89844 to be solved, don’t you?
> >
> > TIA
> >
> > Dominique
> >
> >
> >> Le 11 avr. 2019 à 16:44, Paul Richard Thomas 
> >>  a écrit :
> >>
> >> Hi Dominique,
> >>
> >> Yes indeed - I used int(kind(loc(res))) to achieve the same effect.
> >>
> >> I am looking for but failing to find a similar problem for PR89846.
> >> Tomorrow I turn my attention to an incorrect cast in the compiler.
> >>
> >> Regards
> >>
> >> Paul
> >
> >
>


-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-26 Thread Manfred Schwarb
Hi,

I still see this issue on 32bit, is the plan to let
the test ISO_Fortran_binding_4.f90 be broken for the 9.0 release?

Cheers,
Manfred


Am 15.04.19 um 10:30 schrieb Dominique d'Humières:
> Hi Paul,
>
> I have found another glitch with -m32 and -O1 or -Os, but not with other 
> values:
>
> % gfc /opt/gcc/_clean/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_4.f90 
> -m32 -O
> % ./a.out
>  FAIL
> Note: The following floating-point exceptions are signalling: IEEE_DENORMAL
> STOP 1
>
> This looks tricky: if I add a line
>
>   print *, x
>
> before
>
>   if (any (abs (x - [1.,20.,3.,40.,5.,60.]) > 1.e-6)) stop 2
>
> the test succeeds!-(
>
> Also you don’t want pr89844 to be solved, don’t you?
>
> TIA
>
> Dominique
>
>
>> Le 11 avr. 2019 à 16:44, Paul Richard Thomas  
>> a écrit :
>>
>> Hi Dominique,
>>
>> Yes indeed - I used int(kind(loc(res))) to achieve the same effect.
>>
>> I am looking for but failing to find a similar problem for PR89846.
>> Tomorrow I turn my attention to an incorrect cast in the compiler.
>>
>> Regards
>>
>> Paul
>
>



Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-15 Thread Paul Richard Thomas
Dear Dominique, Gilles and Reinhold,

Thank you for your rapid feedback. We might even get a reasonably
functional ISO Fortran binding in place for 9-branch release :-)  On
your remaining nits:

(i) ISO_Fortran_binding_4.f90 -m32 -O1/Os looks awful. I will take a
look, though.

(ii) pr89844 being fixed by an earlier patch led me to give it lower
priority. I will look to see whether another testcase is required to
nail it down.

(iii) I will take a look at 90093 - it should be straight forward. I
do not regard it as being a regression, however, since the arguments
were not being correctly handled until now - ie. were not converted
from cfi to gfc descriptors.

Cheers

Paul

On Mon, 15 Apr 2019 at 10:27, Bader, Reinhold  wrote:
>
> Dear Paul,
>
> mostly looks good. Apart from a regression with optional arguments reported as
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90093
> all other  test cases I have now execute correctly.
>
> Cheers
> Reinhold
>
> > -Ursprüngliche Nachricht-
> > Von: Paul Richard Thomas 
> > Gesendet: Sonntag, 14. April 2019 20:16
> > An: Thomas Koenig 
> > Cc: Gilles Gouaillardet ; Bader, Reinhold
> > ; fort...@gcc.gnu.org; gcc-patches  > patc...@gcc.gnu.org>
> > Betreff: Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
> >
> > Hi Thomas,
> >
> > Thanks a lot. Committed as revision 270353.
> >
> > I was determined not to repeat the PDT experience, which is still nagging at
> > me. That has to be the next major gfc project, I guess.
> >
> > Regards
> >
> > Paul
> >
> > On Sun, 14 Apr 2019 at 18:08, Thomas Koenig 
> > wrote:
> > >
> > > Hi Paul,
> > >
> > >
> > > > Please find attached the updated patch, which fixes the problem with
> > > > -m32 in PR90022, eliminates the temporary creation for INTENT(IN)
> > > > dummies and fixes PR89846.
> > > >
> > > > While it looks like it should be intrusive because of its size, I
> > > > believe that the patch is still safe for trunk since it is hidden
> > > > behind tests for CFI descriptors.
> > > >
> > > > Bootstraps and regtests on FC29/x86_64 - OK for trunk?
> > >
> > > OK.
> > >
> > > I we're going into the gcc 9 release with an implementation of the C
> > > interop features, it will be better with fewer bugs :-)
> > >
> > > Thanks a lot for working on it!
> > >
> > > Regards
> > >
> > > Thomas
> >
> >
> >
> > --
> > "If you can't explain it simply, you don't understand it well enough"
> > - Albert Einstein



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein


AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-15 Thread Bader, Reinhold
Dear Paul, 

mostly looks good. Apart from a regression with optional arguments reported as
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90093 
all other  test cases I have now execute correctly.

Cheers
Reinhold

> -Ursprüngliche Nachricht-
> Von: Paul Richard Thomas 
> Gesendet: Sonntag, 14. April 2019 20:16
> An: Thomas Koenig 
> Cc: Gilles Gouaillardet ; Bader, Reinhold
> ; fort...@gcc.gnu.org; gcc-patches  patc...@gcc.gnu.org>
> Betreff: Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.
> 
> Hi Thomas,
> 
> Thanks a lot. Committed as revision 270353.
> 
> I was determined not to repeat the PDT experience, which is still nagging at
> me. That has to be the next major gfc project, I guess.
> 
> Regards
> 
> Paul
> 
> On Sun, 14 Apr 2019 at 18:08, Thomas Koenig 
> wrote:
> >
> > Hi Paul,
> >
> >
> > > Please find attached the updated patch, which fixes the problem with
> > > -m32 in PR90022, eliminates the temporary creation for INTENT(IN)
> > > dummies and fixes PR89846.
> > >
> > > While it looks like it should be intrusive because of its size, I
> > > believe that the patch is still safe for trunk since it is hidden
> > > behind tests for CFI descriptors.
> > >
> > > Bootstraps and regtests on FC29/x86_64 - OK for trunk?
> >
> > OK.
> >
> > I we're going into the gcc 9 release with an implementation of the C
> > interop features, it will be better with fewer bugs :-)
> >
> > Thanks a lot for working on it!
> >
> > Regards
> >
> > Thomas
> 
> 
> 
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein


smime.p7s
Description: S/MIME cryptographic signature


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-15 Thread Dominique d'Humières
Hi Paul,

I have found another glitch with -m32 and -O1 or -Os, but not with other values:

% gfc /opt/gcc/_clean/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_4.f90 -m32 
-O
% ./a.out
 FAIL
Note: The following floating-point exceptions are signalling: IEEE_DENORMAL
STOP 1

This looks tricky: if I add a line

  print *, x

before

  if (any (abs (x - [1.,20.,3.,40.,5.,60.]) > 1.e-6)) stop 2

the test succeeds!-(

Also you don’t want pr89844 to be solved, don’t you?

TIA

Dominique


> Le 11 avr. 2019 à 16:44, Paul Richard Thomas  
> a écrit :
> 
> Hi Dominique,
> 
> Yes indeed - I used int(kind(loc(res))) to achieve the same effect.
> 
> I am looking for but failing to find a similar problem for PR89846.
> Tomorrow I turn my attention to an incorrect cast in the compiler.
> 
> Regards
> 
> Paul



Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-14 Thread Paul Richard Thomas
Hi Thomas,

Thanks a lot. Committed as revision 270353.

I was determined not to repeat the PDT experience, which is still
nagging at me. That has to be the next major gfc project, I guess.

Regards

Paul

On Sun, 14 Apr 2019 at 18:08, Thomas Koenig  wrote:
>
> Hi Paul,
>
>
> > Please find attached the updated patch, which fixes the problem with
> > -m32 in PR90022, eliminates the temporary creation for INTENT(IN)
> > dummies and fixes PR89846.
> >
> > While it looks like it should be intrusive because of its size, I
> > believe that the patch is still safe for trunk since it is hidden
> > behind tests for CFI descriptors.
> >
> > Bootstraps and regtests on FC29/x86_64 - OK for trunk?
>
> OK.
>
> I we're going into the gcc 9 release with an implementation of the
> C interop features, it will be better with fewer bugs :-)
>
> Thanks a lot for working on it!
>
> Regards
>
> Thomas



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-14 Thread Thomas Koenig

Hi Paul,



Please find attached the updated patch, which fixes the problem with
-m32 in PR90022, eliminates the temporary creation for INTENT(IN)
dummies and fixes PR89846.

While it looks like it should be intrusive because of its size, I
believe that the patch is still safe for trunk since it is hidden
behind tests for CFI descriptors.

Bootstraps and regtests on FC29/x86_64 - OK for trunk?


OK.

I we're going into the gcc 9 release with an implementation of the
C interop features, it will be better with fewer bugs :-)

Thanks a lot for working on it!

Regards

Thomas


Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-14 Thread Paul Richard Thomas
Dear All,

Please find attached the updated patch, which fixes the problem with
-m32 in PR90022, eliminates the temporary creation for INTENT(IN)
dummies and fixes PR89846.

While it looks like it should be intrusive because of its size, I
believe that the patch is still safe for trunk since it is hidden
behind tests for CFI descriptors.

Bootstraps and regtests on FC29/x86_64 - OK for trunk?

Cheers

Paul

2019-04-14  Paul Thomas  

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/89846
* expr.c (is_CFI_desc): New function.
(is_subref_array): Tidy up by referencing the symbol directly.
* gfortran.h : Prototype for is_CFI_desc.
* trans_array.c (get_CFI_desc): New function.
(gfc_get_array_span, gfc_conv_scalarized_array_ref,
gfc_conv_array_ref): Use it.
* trans.c (get_array_span): Extract the span from descriptors
that are indirect references.

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.
* trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Eliminate
temporary creation for intent(in).

2019-04-14  Paul Thomas  

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/89846
* gfortran.dg/ISO_Fortran_binding_11.f90: New test.
* gfortran.dg/ISO_Fortran_binding_11.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-14  Paul Thomas  

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/89846
* runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Use
the stride measure for the gfc span if it is not a multiple
of the element length. Otherwise use the element length.

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.

On Fri, 12 Apr 2019 at 10:08, Paul Richard Thomas
 wrote:
>
> Gilles & Reinhold,
>
> Following the discussion, I have decided to remove the copy-in for
> intent(in). Dominique and I have located the problem with -m32 for
> PR90022 and I am working on PR89846 right now.
>
> I will be resubmitting a bit later on.
>
> Cheers
>
> Paul
>
>
> On Fri, 12 Apr 2019 at 01:25, Gilles Gouaillardet  wrote:
> >
> > Reinhold,
> >
> >
> > Thanks for the insights !
> >
> >
> > That means there is currently an other issue since copy-in is performed
> > even if the argument is declared as ASYNCHRONOUS.
> >
> >
> > I gave the copy-in mechanism some more thoughts, and as a library
> > developers, I can clearly see a need *not* to do that
> >
> > on a case-by-case basis, mainly for performance reasons, but also to be
> > friendly with legacy apps that are not strictly standard compliant.
> >
> > At this stage, I think the best way to move forward is to add an other
> > directive in the interface definition.
> >
> >
> > for example, we could declare
> >
> >
> > module foo
> >
> > interface
> >
> > subroutine bar_f08(buf) BIND(C, name="bar_c")
> >
> > implicit none
> >
> > !GCC$ ATTRIBUTES NO_COPY_IN :: buf
> >
> > TYPE(*), DIMENSION(..), INTENT(IN) :: buf
> >
> > end subroutine
> >
> > end interface
> >
> > end module
> >
> >
> > Does this make sense ?
> >
> >
> > Gilles
> >
> > On 

Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-12 Thread Paul Richard Thomas
Gilles & Reinhold,

Following the discussion, I have decided to remove the copy-in for
intent(in). Dominique and I have located the problem with -m32 for
PR90022 and I am working on PR89846 right now.

I will be resubmitting a bit later on.

Cheers

Paul


On Fri, 12 Apr 2019 at 01:25, Gilles Gouaillardet  wrote:
>
> Reinhold,
>
>
> Thanks for the insights !
>
>
> That means there is currently an other issue since copy-in is performed
> even if the argument is declared as ASYNCHRONOUS.
>
>
> I gave the copy-in mechanism some more thoughts, and as a library
> developers, I can clearly see a need *not* to do that
>
> on a case-by-case basis, mainly for performance reasons, but also to be
> friendly with legacy apps that are not strictly standard compliant.
>
> At this stage, I think the best way to move forward is to add an other
> directive in the interface definition.
>
>
> for example, we could declare
>
>
> module foo
>
> interface
>
> subroutine bar_f08(buf) BIND(C, name="bar_c")
>
> implicit none
>
> !GCC$ ATTRIBUTES NO_COPY_IN :: buf
>
> TYPE(*), DIMENSION(..), INTENT(IN) :: buf
>
> end subroutine
>
> end interface
>
> end module
>
>
> Does this make sense ?
>
>
> Gilles
>
> On 4/10/2019 4:22 PM, Bader, Reinhold wrote:
> > Hi Gilles,
> >
> >> 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).
> >
> > The above call will only be conforming if the dummy argument is declared
> > assumed or explicit size.
> > Otherwise, the compiler should reject it due to rank mismatch. For assumed
> > rank, the call would be
> > legitimate, but the rank of the dummy argument is then zero. Even if no
> > copy-in is performed,
> > accessing data beyond the address range of that scalar is not strictly
> > allowed.
> >
> > Of more interest is the situation where the dummy argument in Fortran is
> > declared, e.g.,
> >
> > TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..)
> >
> > The standard's semantics *forbids* performing copy-in/out in this case, 
> > IIRC.
> > Otherwise
> > ASYNCHRONOUS semantics would not work, and non-blocking MPI calls would fail
> > due
> > to buffers vanishing into thin air.
> >
> > Regards
> > Reinhold
> >
> >> 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  
> >>>
> >>>   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  
> >>>
> >>>   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
> >>>  

Re: AW: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-12 Thread Gilles Gouaillardet

Dear Reinhold,


Thanks again for the insight !


My preference is also to avoid copy-in whenever possible.

But since my primary concern is performance (vs enforcing correctness of 
the user apps), I am not willing to


use existing bugs to (fallaciously) conclude copy-in should be simply 
dropped (unless mandatory).



Cheers,


Gilles

On 4/12/2019 4:53 PM, Bader, Reinhold wrote:

Dear Gilles,


-Ursprüngliche Nachricht-
Von: Gilles Gouaillardet 
Gesendet: Freitag, 12. April 2019 02:25
An: Bader, Reinhold ; Paul Richard Thomas
; fort...@gcc.gnu.org; gcc-patches 
Betreff: Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop
fixes.

Reinhold,


Thanks for the insights !


That means there is currently an other issue since copy-in is performed even
if the argument is declared as ASYNCHRONOUS.


I gave the copy-in mechanism some more thoughts, and as a library
developers, I can clearly see a need *not* to do that

on a case-by-case basis, mainly for performance reasons, but also to be
friendly with legacy apps that are not strictly standard compliant.

At this stage, I think the best way to move forward is to add an other
directive in the interface definition.


for example, we could declare


module foo

interface

subroutine bar_f08(buf) BIND(C, name="bar_c")

implicit none

!GCC$ ATTRIBUTES NO_COPY_IN :: buf

maybe so, but some care must be taken to perform proper checking in case 
copy-in is necessary - these situations exist as well.
For example, if the declaration for buf is

TYPE(*), DIMENSION(..), INTENT(IN), CONTIGUOUS :: buf

and the actual argument is a discontiguous array section.

My preference is for the compiler to automatically avoid copy-in whenever 
possible.

Cheers
Reinhold



TYPE(*), DIMENSION(..), INTENT(IN) :: buf

end subroutine

end interface

end module


Does this make sense ?


Gilles

On 4/10/2019 4:22 PM, Bader, Reinhold wrote:

Hi Gilles,


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).

The above call will only be conforming if the dummy argument is declared
assumed or explicit size.
Otherwise, the compiler should reject it due to rank mismatch. For

assumed

rank, the call would be
legitimate, but the rank of the dummy argument is then zero. Even if no
copy-in is performed,
accessing data beyond the address range of that scalar is not strictly
allowed.

Of more interest is the situation where the dummy argument in Fortran is
declared, e.g.,

TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..)

The standard's semantics *forbids* performing copy-in/out in this case,

IIRC.

Otherwise
ASYNCHRONOUS semantics would not work, and non-blocking MPI calls

would fail

due
to buffers vanishing into thin air.

Regards
Reinhold


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  

   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  

   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.
   * g

AW: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-12 Thread Bader, Reinhold
Dear Gilles, 

> -Ursprüngliche Nachricht-
> Von: Gilles Gouaillardet 
> Gesendet: Freitag, 12. April 2019 02:25
> An: Bader, Reinhold ; Paul Richard Thomas
> ; fort...@gcc.gnu.org; gcc-patches  patc...@gcc.gnu.org>
> Betreff: Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop
> fixes.
> 
> Reinhold,
> 
> 
> Thanks for the insights !
> 
> 
> That means there is currently an other issue since copy-in is performed even
> if the argument is declared as ASYNCHRONOUS.
> 
> 
> I gave the copy-in mechanism some more thoughts, and as a library
> developers, I can clearly see a need *not* to do that
> 
> on a case-by-case basis, mainly for performance reasons, but also to be
> friendly with legacy apps that are not strictly standard compliant.
> 
> At this stage, I think the best way to move forward is to add an other
> directive in the interface definition.
> 
> 
> for example, we could declare
> 
> 
> module foo
> 
> interface
> 
> subroutine bar_f08(buf) BIND(C, name="bar_c")
> 
> implicit none
> 
> !GCC$ ATTRIBUTES NO_COPY_IN :: buf

maybe so, but some care must be taken to perform proper checking in case 
copy-in is necessary - these situations exist as well. 
For example, if the declaration for buf is

TYPE(*), DIMENSION(..), INTENT(IN), CONTIGUOUS :: buf

and the actual argument is a discontiguous array section.

My preference is for the compiler to automatically avoid copy-in whenever 
possible.

Cheers
Reinhold


> 
> TYPE(*), DIMENSION(..), INTENT(IN) :: buf
> 
> end subroutine
> 
> end interface
> 
> end module
> 
> 
> Does this make sense ?
> 
> 
> Gilles
> 
> On 4/10/2019 4:22 PM, Bader, Reinhold wrote:
> > Hi Gilles,
> >
> >> 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).
> >
> > The above call will only be conforming if the dummy argument is declared
> > assumed or explicit size.
> > Otherwise, the compiler should reject it due to rank mismatch. For
> assumed
> > rank, the call would be
> > legitimate, but the rank of the dummy argument is then zero. Even if no
> > copy-in is performed,
> > accessing data beyond the address range of that scalar is not strictly
> > allowed.
> >
> > Of more interest is the situation where the dummy argument in Fortran is
> > declared, e.g.,
> >
> > TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..)
> >
> > The standard's semantics *forbids* performing copy-in/out in this case,
> IIRC.
> > Otherwise
> > ASYNCHRONOUS semantics would not work, and non-blocking MPI calls
> would fail
> > due
> > to buffers vanishing into thin air.
> >
> > Regards
> > Reinhold
> >
> >> 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  
> >>>
> >>>   

Re: AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-11 Thread Gilles Gouaillardet

Reinhold,


Thanks for the insights !


That means there is currently an other issue since copy-in is performed 
even if the argument is declared as ASYNCHRONOUS.



I gave the copy-in mechanism some more thoughts, and as a library 
developers, I can clearly see a need *not* to do that


on a case-by-case basis, mainly for performance reasons, but also to be 
friendly with legacy apps that are not strictly standard compliant.


At this stage, I think the best way to move forward is to add an other 
directive in the interface definition.



for example, we could declare


module foo

interface

subroutine bar_f08(buf) BIND(C, name="bar_c")

implicit none

!GCC$ ATTRIBUTES NO_COPY_IN :: buf

TYPE(*), DIMENSION(..), INTENT(IN) :: buf

end subroutine

end interface

end module


Does this make sense ?


Gilles

On 4/10/2019 4:22 PM, Bader, Reinhold wrote:

Hi Gilles,


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).


The above call will only be conforming if the dummy argument is declared
assumed or explicit size.
Otherwise, the compiler should reject it due to rank mismatch. For assumed
rank, the call would be
legitimate, but the rank of the dummy argument is then zero. Even if no
copy-in is performed,
accessing data beyond the address range of that scalar is not strictly
allowed.

Of more interest is the situation where the dummy argument in Fortran is
declared, e.g.,

TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..)

The standard's semantics *forbids* performing copy-in/out in this case, IIRC.
Otherwise
ASYNCHRONOUS semantics would not work, and non-blocking MPI calls would fail
due
to buffers vanishing into thin air.

Regards
Reinhold


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  

  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  

  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  

  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 

Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-11 Thread Paul Richard Thomas
Hi Dominique,

Yes indeed - I used int(kind(loc(res))) to achieve the same effect.

I am looking for but failing to find a similar problem for PR89846.
Tomorrow I turn my attention to an incorrect cast in the compiler.

Regards

Paul

On Thu, 11 Apr 2019 at 14:54, Dominique d'Humières  wrote:
>
>
> With the following code
>
> module cdesc
>   interface
>   function cdesc_f08(buf, expected) result (res) BIND(C, name="cdesc_c")
>   USE, INTRINSIC :: ISO_C_BINDING
>   implicit none
>   INTEGER(C_INT) :: res
>   type(*), dimension(..), INTENT(INOUT) :: buf
>   type(c_ptr),INTENT(IN) :: expected
> end function cdesc_f08
>   end interface
> end module
>
> program cdesc_test
>   USE, INTRINSIC :: ISO_C_BINDING
>   use cdesc
>   implicit none
>   integer(c_int), target :: a0, a1(10), a2(10,10), a3(10,10,10)
>   if (cdesc_f08(a0, C_LOC(a0)) .ne. 1) stop 1
>   if (cdesc_f08(a1, C_LOC(a1(1))) .ne. 1) stop 2
>   if (cdesc_f08(a2, C_LOC(a2(1,1))) .ne. 1) stop 3
>   if (cdesc_f08(a3, C_LOC(a3(1,1,1))) .ne. 1) stop 4
> end program
>
> The test ISO_Fortran_binding_9.f90 executes without failure for both -m32 and 
> -m64.
>
> Dominique
>
> > Le 10 avr. 2019 à 00:42, Paul Richard Thomas 
> >  a écrit :
> >
> > Many thanks, sir! I will look into it.
> >
> > Paul
>


-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-11 Thread Dominique d'Humières


With the following code

module cdesc
  interface
  function cdesc_f08(buf, expected) result (res) BIND(C, name="cdesc_c")
  USE, INTRINSIC :: ISO_C_BINDING
  implicit none
  INTEGER(C_INT) :: res
  type(*), dimension(..), INTENT(INOUT) :: buf
  type(c_ptr),INTENT(IN) :: expected
end function cdesc_f08
  end interface
end module

program cdesc_test
  USE, INTRINSIC :: ISO_C_BINDING
  use cdesc
  implicit none
  integer(c_int), target :: a0, a1(10), a2(10,10), a3(10,10,10)
  if (cdesc_f08(a0, C_LOC(a0)) .ne. 1) stop 1
  if (cdesc_f08(a1, C_LOC(a1(1))) .ne. 1) stop 2
  if (cdesc_f08(a2, C_LOC(a2(1,1))) .ne. 1) stop 3
  if (cdesc_f08(a3, C_LOC(a3(1,1,1))) .ne. 1) stop 4
end program

The test ISO_Fortran_binding_9.f90 executes without failure for both -m32 and 
-m64.

Dominique

> Le 10 avr. 2019 à 00:42, Paul Richard Thomas  
> a écrit :
> 
> Many thanks, sir! I will look into it.
> 
> Paul



AW: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-10 Thread Bader, Reinhold
Hi Gilles,

>
> 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).


The above call will only be conforming if the dummy argument is declared 
assumed or explicit size.
Otherwise, the compiler should reject it due to rank mismatch. For assumed 
rank, the call would be
legitimate, but the rank of the dummy argument is then zero. Even if no 
copy-in is performed,
accessing data beyond the address range of that scalar is not strictly 
allowed.

Of more interest is the situation where the dummy argument in Fortran is 
declared, e.g.,

TYPE(*), ASYNCHRONOUS, INTENT(IN) :: BUF(..)

The standard's semantics *forbids* performing copy-in/out in this case, IIRC. 
Otherwise
ASYNCHRONOUS semantics would not work, and non-blocking MPI calls would fail 
due
to buffers vanishing into thin air.

Regards
Reinhold

>
> 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  
> >
> >  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  
> >
> >  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  
> >
> >  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.


smime.p7s
Description: S/MIME cryptographic signature


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-10 Thread Gilles Gouaillardet

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 (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  

 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  

 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  

 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.


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-09 Thread Steve Kargl
On Tue, Apr 09, 2019 at 11:18:11AM +0100, Paul Richard Thomas wrote:
> 
> 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.
> 

I cannot look at this until Saturday, if no one reviews
by then I'll give it a quick glance.

-- 
Steve
20170425 https://www.youtube.com/watch?v=VWUpyCsUKR4
20161221 https://www.youtube.com/watch?v=IbCHE-hONow


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-09 Thread Paul Richard Thomas
Many thanks, sir! I will look into it.

Paul

On Tue, 9 Apr 2019 at 17:43, Dominique d'Humières  wrote:
>
> Hi Paul,
>
> With your patch the test gfortran.dg/ISO_Fortran_binding_9.f90 fails in the 
> 32 bit mode, due to the errors
>
> /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:24:6: 
> Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to 
> INTEGER(8)
> /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:25:6: 
> Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to 
> INTEGER(8)
> /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:26:6: 
> Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to 
> INTEGER(8)
> /opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:27:6: 
> Error: Type mismatch in argument 'expected' at (1); passed INTEGER(4) to 
> INTEGER(8)
>
> Note that LOC is a gnu extension.
>
> The patch also fixes pr89846 for -m64, but not for -m32
>
> % gfc pr89846.c pr89846.f90 -m32
> % a.out
>  FAIL 2:
>  FAIL 4:
> % gfc pr89846.c pr89846.f90
> % a.out
>  OK
>  OK
>
> TIA
>
> Dominique
>


-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein


Re: [Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-09 Thread Dominique d'Humières
Hi Paul,

With your patch the test gfortran.dg/ISO_Fortran_binding_9.f90 fails in the 32 
bit mode, due to the errors

/opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:24:6: Error: 
Type mismatch in argument 'expected' at (1); passed INTEGER(4) to INTEGER(8)
/opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:25:6: Error: 
Type mismatch in argument 'expected' at (1); passed INTEGER(4) to INTEGER(8)
/opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:26:6: Error: 
Type mismatch in argument 'expected' at (1); passed INTEGER(4) to INTEGER(8)
/opt/gcc/work/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_9.f90:27:6: Error: 
Type mismatch in argument 'expected' at (1); passed INTEGER(4) to INTEGER(8)

Note that LOC is a gnu extension.

The patch also fixes pr89846 for -m64, but not for -m32

% gfc pr89846.c pr89846.f90 -m32
% a.out
 FAIL 2: 
 FAIL 4: 
% gfc pr89846.c pr89846.f90 
% a.out
 OK
 OK

TIA

Dominique



[Patch, fortran] PRs 89843 and 90022 - C Fortran Interop fixes.

2019-04-09 Thread Paul Richard Thomas
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  

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  

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  

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.
Index: gcc/fortran/trans-decl.c
===
*** gcc/fortran/trans-decl.c	(revision 270149)
--- gcc/fortran/trans-decl.c	(working copy)
*** gfc_get_symbol_decl (gfc_symbol * sym)
*** 1494,1499 
--- 1494,1506 
&& sym->attr.dummy)
  gfc_defer_symbol_init (sym);
  
+   if (sym->attr.dummy
+   && sym->ns->proc_name->attr.is_bind_c
+   && sym->attr.dimension
+   && (sym->as->type == AS_ASSUMED_SHAPE
+ 	  || sym->as->type == AS_ASSUMED_RANK))
+ gfc_defer_symbol_init (sym);
+ 
/* All deferred character length procedures need to retain the backend
   decl, which is a pointer to the character length in the caller's
   namespace and to declare a local character length.  */
*** gfc_null_and_pass_deferred_len (gfc_symb
*** 4268,4273 
--- 4275,4353 
  }
  
  
+ /* Convert CFI descriptor dummies into gfc types and back again.  */
+ static void
+ convert_CFI_desc (gfc_wrapped_block * block, gfc_symbol *sym)
+ {
+   tree gfc_desc;
+   tree gfc_desc_ptr;
+   tree CFI_desc;
+   tree CFI_desc_ptr;
+   tree dummy_ptr;
+   tree tmp;
+   tree incoming;
+   tree outgoing;
+   stmtblock_t tmpblock;
+ 
+   /* dummy_ptr will be the pointer to the passed array descriptor,
+  while CFI_desc is the descriptor itself.  */
+   dummy_ptr = sym->backend_decl;
+   CFI_desc = sym->backend_decl;
+   if (POINTER_TYPE_P (TREE_TYPE (CFI_desc)))
+ CFI_desc = build_fold_indirect_ref_loc (input_location, CFI_desc);
+   if (!GFC_DESCRIPTOR_TYPE_P (TREE_TYPE (CFI_desc)))
+ {
+   if (DECL_LANG_SPECIFIC (sym->backend_decl))
+ 	CFI_desc = GFC_DECL_SAVED_DESCRIPTOR (sym->backend_decl);
+   else
+ 	CFI_desc = NULL;
+   dummy_ptr = CFI_desc;
+ }
+ 
+   if (CFI_desc)
+ {
+   if (POINTER_TYPE_P (TREE_TYPE (CFI_desc)))
+ 	CFI_desc = build_fold_indirect_ref_loc (input_location, CFI_desc);
+ 
+   /* The compiler will have given CFI_desc the