public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Patch, fortran] PR91926 - assumed rank optional
@ 2019-10-05 18:31 Paul Richard Thomas
  2019-10-09 10:18 ` Christophe Lyon
  2019-10-21 17:59 ` Paul Richard Thomas
  0 siblings, 2 replies; 7+ messages in thread
From: Paul Richard Thomas @ 2019-10-05 18:31 UTC (permalink / raw)
  To: fortran, gcc-patches; +Cc: Gilles Gouaillardet

I must apologise not posting this before committing. I left for a
vacation this morning and I thought that this problem and the one
posted by Gilles were best fixed before departing. The patch only
touches the new ISO_Fortran binding feature and so I thought that I
would be safe to do this.

It was fully regtested and only applies to trunk.

Paul

Author: pault
Date: Sat Oct  5 08:17:55 2019
New Revision: 276624

URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev
Log:
2019-10-05  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/91926
        * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
        assignment of the attribute field to account correctly for an
        assumed shape dummy. Assign separately to the gfc and cfi
        descriptors since the atribute can be different. Add btanch to
        correctly handle missing optional dummies.

2019-10-05  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/91926
        * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
        * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
        * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.

2019-10-05  Paul Thomas  <pault@gcc.gnu.org>

        PR fortran/91926
        * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not
        modify the bounds and offset for CFI_other.

Added:
    trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
    trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
    trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
Modified:
    trunk/gcc/fortran/ChangeLog
    trunk/gcc/fortran/trans-expr.c
    trunk/gcc/testsuite/ChangeLog
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/runtime/ISO_Fortran_binding.c

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch, fortran] PR91926 - assumed rank optional
  2019-10-05 18:31 [Patch, fortran] PR91926 - assumed rank optional Paul Richard Thomas
@ 2019-10-09 10:18 ` Christophe Lyon
  2019-10-09 11:35   ` Paul Richard Thomas
  2019-10-21 17:59 ` Paul Richard Thomas
  1 sibling, 1 reply; 7+ messages in thread
From: Christophe Lyon @ 2019-10-09 10:18 UTC (permalink / raw)
  To: Paul Richard Thomas; +Cc: fortran, gcc-patches, Gilles Gouaillardet

Hi,


On Sat, 5 Oct 2019 at 20:31, Paul Richard Thomas <
paul.richard.thomas@gmail.com> wrote:

> I must apologise not posting this before committing. I left for a
> vacation this morning and I thought that this problem and the one
> posted by Gilles were best fixed before departing. The patch only
> touches the new ISO_Fortran binding feature and so I thought that I
> would be safe to do this.
>
> It was fully regtested and only applies to trunk.
>
> Paul
>
> Author: pault
> Date: Sat Oct  5 08:17:55 2019
> New Revision: 276624
>
> URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev
> Log:
> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>
>         PR fortran/91926
>         * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
>         assignment of the attribute field to account correctly for an
>         assumed shape dummy. Assign separately to the gfc and cfi
>         descriptors since the atribute can be different. Add btanch to
>         correctly handle missing optional dummies.
>
> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>
>         PR fortran/91926
>         * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
>         * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
>         * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.
>
> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>
>         PR fortran/91926
>         * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not
>         modify the bounds and offset for CFI_other.
>
> Added:
>     trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
>     trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
>     trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
> Modified:
>     trunk/gcc/fortran/ChangeLog
>     trunk/gcc/fortran/trans-expr.c
>     trunk/gcc/testsuite/ChangeLog
>     trunk/libgfortran/ChangeLog
>     trunk/libgfortran/runtime/ISO_Fortran_binding.c
>


Since this was committed (r276624), I have noticed regressions on
arm-linux-gnueabihf:
FAIL: gfortran.dg/ISO_Fortran_binding_11.f90   -O3 -g  execution test
I've seen other reports on gcc-testresults too.

Christophe

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch, fortran] PR91926 - assumed rank optional
  2019-10-09 10:18 ` Christophe Lyon
@ 2019-10-09 11:35   ` Paul Richard Thomas
  2019-10-17 13:56     ` Tobias Burnus
  0 siblings, 1 reply; 7+ messages in thread
From: Paul Richard Thomas @ 2019-10-09 11:35 UTC (permalink / raw)
  To: Christophe Lyon; +Cc: fortran, gcc-patches, Gilles Gouaillardet

Hi Christophe,

Thanks for flagging this up - I am back at base on Saturday and will
take it up then.

Regards

Paul

On Wed, 9 Oct 2019 at 11:13, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>
> Hi,
>
>
> On Sat, 5 Oct 2019 at 20:31, Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
>>
>> I must apologise not posting this before committing. I left for a
>> vacation this morning and I thought that this problem and the one
>> posted by Gilles were best fixed before departing. The patch only
>> touches the new ISO_Fortran binding feature and so I thought that I
>> would be safe to do this.
>>
>> It was fully regtested and only applies to trunk.
>>
>> Paul
>>
>> Author: pault
>> Date: Sat Oct  5 08:17:55 2019
>> New Revision: 276624
>>
>> URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev
>> Log:
>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>>
>>         PR fortran/91926
>>         * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
>>         assignment of the attribute field to account correctly for an
>>         assumed shape dummy. Assign separately to the gfc and cfi
>>         descriptors since the atribute can be different. Add btanch to
>>         correctly handle missing optional dummies.
>>
>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>>
>>         PR fortran/91926
>>         * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
>>         * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
>>         * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.
>>
>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>>
>>         PR fortran/91926
>>         * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not
>>         modify the bounds and offset for CFI_other.
>>
>> Added:
>>     trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
>>     trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
>>     trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
>> Modified:
>>     trunk/gcc/fortran/ChangeLog
>>     trunk/gcc/fortran/trans-expr.c
>>     trunk/gcc/testsuite/ChangeLog
>>     trunk/libgfortran/ChangeLog
>>     trunk/libgfortran/runtime/ISO_Fortran_binding.c
>
>
>
> Since this was committed (r276624), I have noticed regressions on arm-linux-gnueabihf:
> FAIL: gfortran.dg/ISO_Fortran_binding_11.f90   -O3 -g  execution test
> I've seen other reports on gcc-testresults too.
>
> Christophe
>


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch, fortran] PR91926 - assumed rank optional
  2019-10-09 11:35   ` Paul Richard Thomas
@ 2019-10-17 13:56     ` Tobias Burnus
  2019-10-19 18:10       ` Paul Richard Thomas
  0 siblings, 1 reply; 7+ messages in thread
From: Tobias Burnus @ 2019-10-17 13:56 UTC (permalink / raw)
  To: Paul Richard Thomas, Christophe Lyon
  Cc: fortran, gcc-patches, Gilles Gouaillardet

See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92027
for a tracking bug – I just added also some analysis.

Tobias

PS: A better patch submission, with the actual patch attached, would 
have been nice. Please re-post the committed patch – and the new patch, 
which fixes the fall out. – Thanks!

On 10/9/19 12:26 PM, Paul Richard Thomas wrote:
> Hi Christophe,
>
> Thanks for flagging this up - I am back at base on Saturday and will
> take it up then.
>
> Regards
>
> Paul
>
> On Wed, 9 Oct 2019 at 11:13, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>> Hi,
>>
>>
>> On Sat, 5 Oct 2019 at 20:31, Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
>>> I must apologise not posting this before committing. I left for a
>>> vacation this morning and I thought that this problem and the one
>>> posted by Gilles were best fixed before departing. The patch only
>>> touches the new ISO_Fortran binding feature and so I thought that I
>>> would be safe to do this.
>>>
>>> It was fully regtested and only applies to trunk.
>>>
>>> Paul
>>>
>>> Author: pault
>>> Date: Sat Oct  5 08:17:55 2019
>>> New Revision: 276624
>>>
>>> URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev
>>> Log:
>>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>>>
>>>          PR fortran/91926
>>>          * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
>>>          assignment of the attribute field to account correctly for an
>>>          assumed shape dummy. Assign separately to the gfc and cfi
>>>          descriptors since the atribute can be different. Add btanch to
>>>          correctly handle missing optional dummies.
>>>
>>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>>>
>>>          PR fortran/91926
>>>          * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
>>>          * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
>>>          * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.
>>>
>>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
>>>
>>>          PR fortran/91926
>>>          * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not
>>>          modify the bounds and offset for CFI_other.
>>>
>>> Added:
>>>      trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
>>>      trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
>>>      trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
>>> Modified:
>>>      trunk/gcc/fortran/ChangeLog
>>>      trunk/gcc/fortran/trans-expr.c
>>>      trunk/gcc/testsuite/ChangeLog
>>>      trunk/libgfortran/ChangeLog
>>>      trunk/libgfortran/runtime/ISO_Fortran_binding.c
>>
>>
>> Since this was committed (r276624), I have noticed regressions on arm-linux-gnueabihf:
>> FAIL: gfortran.dg/ISO_Fortran_binding_11.f90   -O3 -g  execution test
>> I've seen other reports on gcc-testresults too.
>>
>> Christophe
>>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch, fortran] PR91926 - assumed rank optional
  2019-10-17 13:56     ` Tobias Burnus
@ 2019-10-19 18:10       ` Paul Richard Thomas
  0 siblings, 0 replies; 7+ messages in thread
From: Paul Richard Thomas @ 2019-10-19 18:10 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: Christophe Lyon, fortran, gcc-patches, Gilles Gouaillardet

[-- Attachment #1: Type: text/plain, Size: 4139 bytes --]

Tobias,

You are quite right to take me to task. As I wrote in the original
message to the list, I was trying to respond rapidly before stepping
out of the door on vacation. The original patch is attached. The fix
to this problem is to revert that part in
libgfortran/runtime/ISO_Fortran_binding.c. As you implicitly surmised,
I was assuming that 'd' would be initialised in the caller. I cannot
see why this should be the case but sometimes the optimizer seems to
cut away a bit too much code :-(

I have done the reversion in r277204 after regtesting, of course.

I am retesting an update to 9-branch, as requested. I will submit to
the list tomorrow.

Cheers

Paul

2019-10-19  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/91926
    * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Revert
    the change made on 2019-10-05.



On Thu, 17 Oct 2019 at 14:39, Tobias Burnus <tobias@codesourcery.com> wrote:
>
> See also: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92027
> for a tracking bug – I just added also some analysis.
>
> Tobias
>
> PS: A better patch submission, with the actual patch attached, would
> have been nice. Please re-post the committed patch – and the new patch,
> which fixes the fall out. – Thanks!
>
> On 10/9/19 12:26 PM, Paul Richard Thomas wrote:
> > Hi Christophe,
> >
> > Thanks for flagging this up - I am back at base on Saturday and will
> > take it up then.
> >
> > Regards
> >
> > Paul
> >
> > On Wed, 9 Oct 2019 at 11:13, Christophe Lyon <christophe.lyon@linaro.org> wrote:
> >> Hi,
> >>
> >>
> >> On Sat, 5 Oct 2019 at 20:31, Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
> >>> I must apologise not posting this before committing. I left for a
> >>> vacation this morning and I thought that this problem and the one
> >>> posted by Gilles were best fixed before departing. The patch only
> >>> touches the new ISO_Fortran binding feature and so I thought that I
> >>> would be safe to do this.
> >>>
> >>> It was fully regtested and only applies to trunk.
> >>>
> >>> Paul
> >>>
> >>> Author: pault
> >>> Date: Sat Oct  5 08:17:55 2019
> >>> New Revision: 276624
> >>>
> >>> URL: https://gcc.gnu.org/viewcvs?rev=276624&root=gcc&view=rev
> >>> Log:
> >>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
> >>>
> >>>          PR fortran/91926
> >>>          * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
> >>>          assignment of the attribute field to account correctly for an
> >>>          assumed shape dummy. Assign separately to the gfc and cfi
> >>>          descriptors since the atribute can be different. Add btanch to
> >>>          correctly handle missing optional dummies.
> >>>
> >>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
> >>>
> >>>          PR fortran/91926
> >>>          * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
> >>>          * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
> >>>          * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.
> >>>
> >>> 2019-10-05  Paul Thomas  <pault@gcc.gnu.org>
> >>>
> >>>          PR fortran/91926
> >>>          * runtime/ISO_Fortran_binding.c (cfi_desc_to_gfc_desc): Do not
> >>>          modify the bounds and offset for CFI_other.
> >>>
> >>> Added:
> >>>      trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
> >>>      trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
> >>>      trunk/gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
> >>> Modified:
> >>>      trunk/gcc/fortran/ChangeLog
> >>>      trunk/gcc/fortran/trans-expr.c
> >>>      trunk/gcc/testsuite/ChangeLog
> >>>      trunk/libgfortran/ChangeLog
> >>>      trunk/libgfortran/runtime/ISO_Fortran_binding.c
> >>
> >>
> >> Since this was committed (r276624), I have noticed regressions on arm-linux-gnueabihf:
> >> FAIL: gfortran.dg/ISO_Fortran_binding_11.f90   -O3 -g  execution test
> >> I've seen other reports on gcc-testresults too.
> >>
> >> Christophe
> >>
> >



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

[-- Attachment #2: submit.diff --]
[-- Type: text/x-patch, Size: 7755 bytes --]

Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c	(revision 276620)
--- gcc/fortran/trans-expr.c	(working copy)
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5202,5208 ****
--- 5202,5210 ----
    tree gfc_desc_ptr;
    tree type;
    tree cond;
+   tree desc_attr;
    int attribute;
+   int cfi_attribute;
    symbol_attribute attr = gfc_expr_attr (e);
    stmtblock_t block;
  
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5211,5222 ****
    attribute = 2;
    if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
      {
!       if (fsym->attr.pointer)
  	attribute = 0;
!       else if (fsym->attr.allocatable)
  	attribute = 1;
      }
  
    if (e->rank != 0)
      {
        parmse->force_no_tmp = 1;
--- 5213,5232 ----
    attribute = 2;
    if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
      {
!       if (attr.pointer)
  	attribute = 0;
!       else if (attr.allocatable)
  	attribute = 1;
      }
  
+   /* If the formal argument is assumed shape and neither a pointer nor
+      allocatable, it is unconditionally CFI_attribute_other.  */
+   if (fsym->as->type == AS_ASSUMED_SHAPE
+       && !fsym->attr.pointer && !fsym->attr.allocatable)
+    cfi_attribute = 2;
+   else
+    cfi_attribute = attribute;
+ 
    if (e->rank != 0)
      {
        parmse->force_no_tmp = 1;
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5283,5293 ****
  						    parmse->expr, attr);
      }
  
!   /* Set the CFI attribute field.  */
!   tmp = gfc_conv_descriptor_attribute (parmse->expr);
    tmp = fold_build2_loc (input_location, MODIFY_EXPR,
! 			 void_type_node, tmp,
! 			 build_int_cst (TREE_TYPE (tmp), attribute));
    gfc_add_expr_to_block (&parmse->pre, tmp);
  
    /* Now pass the gfc_descriptor by reference.  */
--- 5293,5304 ----
  						    parmse->expr, attr);
      }
  
!   /* Set the CFI attribute field through a temporary value for the
!      gfc attribute.  */
!   desc_attr = gfc_conv_descriptor_attribute (parmse->expr);
    tmp = fold_build2_loc (input_location, MODIFY_EXPR,
! 			 void_type_node, desc_attr,
! 			 build_int_cst (TREE_TYPE (desc_attr), cfi_attribute));
    gfc_add_expr_to_block (&parmse->pre, tmp);
  
    /* Now pass the gfc_descriptor by reference.  */
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5305,5310 ****
--- 5316,5327 ----
  			     gfor_fndecl_gfc_to_cfi, 2, tmp, gfc_desc_ptr);
    gfc_add_expr_to_block (&parmse->pre, tmp);
  
+   /* Now set the gfc descriptor attribute.  */
+   tmp = fold_build2_loc (input_location, MODIFY_EXPR,
+ 			 void_type_node, desc_attr,
+ 			 build_int_cst (TREE_TYPE (desc_attr), attribute));
+   gfc_add_expr_to_block (&parmse->pre, tmp);
+ 
    /* The CFI descriptor is passed to the bind_C procedure.  */
    parmse->expr = cfi_desc_ptr;
  
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5325,5330 ****
--- 5342,5366 ----
    tmp = build_call_expr_loc (input_location,
  			     gfor_fndecl_cfi_to_gfc, 2, gfc_desc_ptr, tmp);
    gfc_prepend_expr_to_block (&parmse->post, tmp);
+ 
+   /* Deal with an optional dummy being passed to an optional formal arg
+      by finishing the pre and post blocks and making their execution
+      conditional on the dummy being present.  */
+   if (fsym->attr.optional && e->expr_type == EXPR_VARIABLE
+       && e->symtree->n.sym->attr.optional)
+     {
+       cond = gfc_conv_expr_present (e->symtree->n.sym);
+       tmp = fold_build2 (MODIFY_EXPR, void_type_node,
+ 			 cfi_desc_ptr,
+ 			 build_int_cst (pvoid_type_node, 0));
+       tmp = build3_v (COND_EXPR, cond,
+ 		      gfc_finish_block (&parmse->pre), tmp);
+       gfc_add_expr_to_block (&parmse->pre, tmp);
+       tmp = build3_v (COND_EXPR, cond,
+ 		      gfc_finish_block (&parmse->post),
+ 		      build_empty_stmt (input_location));
+       gfc_add_expr_to_block (&parmse->post, tmp);
+     }
  }
  
  
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c	(nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c	(working copy)
***************
*** 0 ****
--- 1,12 ----
+ /* Test the fix for PR91926.  */
+ 
+ /* Contributed by José Rui Faustino de Sousa  <jrfsousa@hotmail.com> */
+ 
+ #include <stdlib.h>
+ 
+ int ifb_echo(void*);
+ 
+ int ifb_echo(void *this)
+ {
+   return this == NULL ? 1 : 2;
+ }
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90	(nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90	(working copy)
***************
*** 0 ****
--- 1,39 ----
+ ! { dg-do run { target c99_runtime } }
+ ! { dg-additional-sources ISO_Fortran_binding_13.c }
+ !
+ ! Test the fix for PR91926. The additional source is the main program.
+ !
+ ! Contributed by José Rui Faustino de Sousa  <jrfsousa@hotmail.com>
+ !
+ program ifb_p
+ 
+   implicit none
+ 
+   integer :: i = 42
+ 
+   interface
+     integer function ifb_echo_aux(this) bind(c, name="ifb_echo")
+       implicit none
+       type(*), dimension(..), & ! removing assumed rank solves segmentation fault
+         optional, intent(in) :: this
+     end function ifb_echo_aux
+   end interface
+ 
+   if (ifb_echo_aux() .ne. 1) STOP 1  ! worked
+   if (ifb_echo() .ne. 1) stop 2      ! segmentation fault
+   if (ifb_echo_aux(i) .ne. 2) stop 3 ! worked
+   if (ifb_echo(i) .ne. 2) stop 4     ! worked
+ 
+   stop
+ 
+ contains
+ 
+   integer function ifb_echo(this)
+     type(*), dimension(..), &
+       optional, intent(in) :: this
+ 
+     ifb_echo = ifb_echo_aux(this)
+     return
+   end function ifb_echo
+ 
+ end program ifb_p
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90	(nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90	(working copy)
***************
*** 0 ****
--- 1,41 ----
+ ! { dg-do run }
+ !
+ ! Correct an error in the eveluation of the CFI descriptor attribute for
+ ! the case where the bind_C formal argument is not an assumed shape array
+ ! and not allocatable or pointer.
+ !
+ ! Contributed by Gilles Gouaillardet  <gilles@rist.or.jp>
+ !
+ MODULE FOO
+ INTERFACE
+ SUBROUTINE dummy(buf) BIND(C, name="sync")
+ type(*), dimension(..) :: buf
+ END SUBROUTINE
+ END INTERFACE
+ END MODULE
+ 
+ PROGRAM main
+     USE FOO
+     IMPLICIT NONE
+     integer(8) :: before, after
+ 
+     INTEGER, parameter :: n = 1
+ 
+     INTEGER, ALLOCATABLE :: buf(:)
+     INTEGER :: buf2(n)
+     INTEGER :: i
+ 
+     ALLOCATE(buf(n))
+     before = LOC(buf(1))
+     CALL dummy (buf)
+     after = LOC(buf(1))
+ 
+     if (before .NE. after) stop 1
+ 
+     before = LOC(buf2(1))
+     CALL dummy (buf)
+     after = LOC(buf2(1))
+ 
+     if (before .NE. after) stop 2
+ 
+ END PROGRAM
Index: libgfortran/runtime/ISO_Fortran_binding.c
===================================================================
*** libgfortran/runtime/ISO_Fortran_binding.c	(revision 276620)
--- libgfortran/runtime/ISO_Fortran_binding.c	(working copy)
*************** cfi_desc_to_gfc_desc (gfc_array_void *d,
*** 63,69 ****
    d->dtype.version = s->version;
    GFC_DESCRIPTOR_RANK (d) = (signed char)s->rank;
  
!   d->dtype.attribute = (signed short)s->attribute;
  
    if (s->rank)
      {
--- 63,70 ----
    d->dtype.version = s->version;
    GFC_DESCRIPTOR_RANK (d) = (signed char)s->rank;
  
!   if (d->dtype.attribute == CFI_attribute_other)
!     return;
  
    if (s->rank)
      {

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Patch, fortran] PR91926 - assumed rank optional
  2019-10-05 18:31 [Patch, fortran] PR91926 - assumed rank optional Paul Richard Thomas
  2019-10-09 10:18 ` Christophe Lyon
@ 2019-10-21 17:59 ` Paul Richard Thomas
  2019-10-25  7:29   ` Tobias Burnus
  1 sibling, 1 reply; 7+ messages in thread
From: Paul Richard Thomas @ 2019-10-21 17:59 UTC (permalink / raw)
  To: fortran, gcc-patches; +Cc: Gilles Gouaillardet

[-- Attachment #1: Type: text/plain, Size: 953 bytes --]

Please find attached a patch to keep 9-branch up to speed with trunk
as far as the ISO_Fortran_binding feature is concerned.

It bootstraps and regtests on 9-branch and incorporates the correction
for PR92027, which caused problems for trunk on certain platforms.

OK to commit?

Paul

2019-10-21  Paul Thomas  <pault@gcc.gnu.org>

    Backport from trunk
    PR fortran/91926
    * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
    assignment of the attribute field to account correctly for an
    assumed shape dummy. Assign separately to the gfc and cfi
    descriptors since the atribute can be different. Add branch to
    correctly handle missing optional dummies.

2019-10-21  Paul Thomas  <pault@gcc.gnu.org>

    Backport from trunk
    PR fortran/91926
    * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
    * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
    * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.

[-- Attachment #2: submit9.diff --]
[-- Type: text/x-patch, Size: 7026 bytes --]

Index: gcc/fortran/trans-expr.c
===================================================================
*** gcc/fortran/trans-expr.c	(revision 276015)
--- gcc/fortran/trans-expr.c	(working copy)
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 4989,4995 ****
--- 5006,5014 ----
    tree gfc_desc_ptr;
    tree type;
    tree cond;
+   tree desc_attr;
    int attribute;
+   int cfi_attribute;
    symbol_attribute attr = gfc_expr_attr (e);
    stmtblock_t block;

*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 4998,5009 ****
    attribute = 2;
    if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
      {
!       if (fsym->attr.pointer)
  	attribute = 0;
!       else if (fsym->attr.allocatable)
  	attribute = 1;
      }

    if (e->rank != 0)
      {
        parmse->force_no_tmp = 1;
--- 5017,5036 ----
    attribute = 2;
    if (!e->rank || gfc_get_full_arrayspec_from_expr (e))
      {
!       if (attr.pointer)
  	attribute = 0;
!       else if (attr.allocatable)
  	attribute = 1;
      }

+   /* If the formal argument is assumed shape and neither a pointer nor
+      allocatable, it is unconditionally CFI_attribute_other.  */
+   if (fsym->as->type == AS_ASSUMED_SHAPE
+       && !fsym->attr.pointer && !fsym->attr.allocatable)
+    cfi_attribute = 2;
+   else
+    cfi_attribute = attribute;
+
    if (e->rank != 0)
      {
        parmse->force_no_tmp = 1;
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5070,5080 ****
  						    parmse->expr, attr);
      }

!   /* Set the CFI attribute field.  */
!   tmp = gfc_conv_descriptor_attribute (parmse->expr);
    tmp = fold_build2_loc (input_location, MODIFY_EXPR,
! 			 void_type_node, tmp,
! 			 build_int_cst (TREE_TYPE (tmp), attribute));
    gfc_add_expr_to_block (&parmse->pre, tmp);

    /* Now pass the gfc_descriptor by reference.  */
--- 5097,5108 ----
  						    parmse->expr, attr);
      }

!   /* Set the CFI attribute field through a temporary value for the
!      gfc attribute.  */
!   desc_attr = gfc_conv_descriptor_attribute (parmse->expr);
    tmp = fold_build2_loc (input_location, MODIFY_EXPR,
! 			 void_type_node, desc_attr,
! 			 build_int_cst (TREE_TYPE (desc_attr), cfi_attribute));
    gfc_add_expr_to_block (&parmse->pre, tmp);

    /* Now pass the gfc_descriptor by reference.  */
*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5092,5097 ****
--- 5120,5131 ----
  			     gfor_fndecl_gfc_to_cfi, 2, tmp, gfc_desc_ptr);
    gfc_add_expr_to_block (&parmse->pre, tmp);

+   /* Now set the gfc descriptor attribute.  */
+   tmp = fold_build2_loc (input_location, MODIFY_EXPR,
+ 			 void_type_node, desc_attr,
+ 			 build_int_cst (TREE_TYPE (desc_attr), attribute));
+   gfc_add_expr_to_block (&parmse->pre, tmp);
+
    /* The CFI descriptor is passed to the bind_C procedure.  */
    parmse->expr = cfi_desc_ptr;

*************** gfc_conv_gfc_desc_to_cfi_desc (gfc_se *p
*** 5112,5117 ****
--- 5146,5170 ----
    tmp = build_call_expr_loc (input_location,
  			     gfor_fndecl_cfi_to_gfc, 2, gfc_desc_ptr, tmp);
    gfc_prepend_expr_to_block (&parmse->post, tmp);
+
+   /* Deal with an optional dummy being passed to an optional formal arg
+      by finishing the pre and post blocks and making their execution
+      conditional on the dummy being present.  */
+   if (fsym->attr.optional && e->expr_type == EXPR_VARIABLE
+       && e->symtree->n.sym->attr.optional)
+     {
+       cond = gfc_conv_expr_present (e->symtree->n.sym);
+       tmp = fold_build2 (MODIFY_EXPR, void_type_node,
+ 			 cfi_desc_ptr,
+ 			 build_int_cst (pvoid_type_node, 0));
+       tmp = build3_v (COND_EXPR, cond,
+ 		      gfc_finish_block (&parmse->pre), tmp);
+       gfc_add_expr_to_block (&parmse->pre, tmp);
+       tmp = build3_v (COND_EXPR, cond,
+ 		      gfc_finish_block (&parmse->post),
+ 		      build_empty_stmt (input_location));
+       gfc_add_expr_to_block (&parmse->post, tmp);
+     }
  }


Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c	(nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.c	(working copy)
***************
*** 0 ****
--- 1,12 ----
+ /* Test the fix for PR91926.  */
+
+ /* Contributed by José Rui Faustino de Sousa  <jrfsousa@hotmail.com> */
+
+ #include <stdlib.h>
+
+ int ifb_echo(void*);
+
+ int ifb_echo(void *this)
+ {
+   return this == NULL ? 1 : 2;
+ }
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90	(nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_13.f90	(working copy)
***************
*** 0 ****
--- 1,39 ----
+ ! { dg-do run { target c99_runtime } }
+ ! { dg-additional-sources ISO_Fortran_binding_13.c }
+ !
+ ! Test the fix for PR91926. The additional source is the main program.
+ !
+ ! Contributed by José Rui Faustino de Sousa  <jrfsousa@hotmail.com>
+ !
+ program ifb_p
+
+   implicit none
+
+   integer :: i = 42
+
+   interface
+     integer function ifb_echo_aux(this) bind(c, name="ifb_echo")
+       implicit none
+       type(*), dimension(..), & ! removing assumed rank solves segmentation fault
+         optional, intent(in) :: this
+     end function ifb_echo_aux
+   end interface
+
+   if (ifb_echo_aux() .ne. 1) STOP 1  ! worked
+   if (ifb_echo() .ne. 1) stop 2      ! segmentation fault
+   if (ifb_echo_aux(i) .ne. 2) stop 3 ! worked
+   if (ifb_echo(i) .ne. 2) stop 4     ! worked
+
+   stop
+
+ contains
+
+   integer function ifb_echo(this)
+     type(*), dimension(..), &
+       optional, intent(in) :: this
+
+     ifb_echo = ifb_echo_aux(this)
+     return
+   end function ifb_echo
+
+ end program ifb_p
Index: gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90
===================================================================
*** gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90	(nonexistent)
--- gcc/testsuite/gfortran.dg/ISO_Fortran_binding_14.f90	(working copy)
***************
*** 0 ****
--- 1,41 ----
+ ! { dg-do run }
+ !
+ ! Correct an error in the eveluation of the CFI descriptor attribute for
+ ! the case where the bind_C formal argument is not an assumed shape array
+ ! and not allocatable or pointer.
+ !
+ ! Contributed by Gilles Gouaillardet  <gilles@rist.or.jp>
+ !
+ MODULE FOO
+ INTERFACE
+ SUBROUTINE dummy(buf) BIND(C, name="sync")
+ type(*), dimension(..) :: buf
+ END SUBROUTINE
+ END INTERFACE
+ END MODULE
+
+ PROGRAM main
+     USE FOO
+     IMPLICIT NONE
+     integer(8) :: before, after
+
+     INTEGER, parameter :: n = 1
+
+     INTEGER, ALLOCATABLE :: buf(:)
+     INTEGER :: buf2(n)
+     INTEGER :: i
+
+     ALLOCATE(buf(n))
+     before = LOC(buf(1))
+     CALL dummy (buf)
+     after = LOC(buf(1))
+
+     if (before .NE. after) stop 1
+
+     before = LOC(buf2(1))
+     CALL dummy (buf)
+     after = LOC(buf2(1))
+
+     if (before .NE. after) stop 2
+
+ END PROGRAM

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Patch, fortran] PR91926 - assumed rank optional
  2019-10-21 17:59 ` Paul Richard Thomas
@ 2019-10-25  7:29   ` Tobias Burnus
  0 siblings, 0 replies; 7+ messages in thread
From: Tobias Burnus @ 2019-10-25  7:29 UTC (permalink / raw)
  To: Paul Richard Thomas, fortran, gcc-patches; +Cc: Gilles Gouaillardet

On 10/21/19 7:28 PM, Paul Richard Thomas wrote:
> Please find attached a patch to keep 9-branch up to speed with trunk
> as far as the ISO_Fortran_binding feature is concerned.
>
> It bootstraps and regtests on 9-branch and incorporates the correction
> for PR92027, which caused problems for trunk on certain platforms.
>
> OK to commit?


OK. Thanks for the patch.

Tobias


> 2019-10-21  Paul Thomas<pault@gcc.gnu.org>
>
>      Backport from trunk
>      PR fortran/91926
>      * trans-expr.c (gfc_conv_gfc_desc_to_cfi_desc): Correct the
>      assignment of the attribute field to account correctly for an
>      assumed shape dummy. Assign separately to the gfc and cfi
>      descriptors since the atribute can be different. Add branch to
>      correctly handle missing optional dummies.
>
> 2019-10-21  Paul Thomas<pault@gcc.gnu.org>
>
>      Backport from trunk
>      PR fortran/91926
>      * gfortran.dg/ISO_Fortran_binding_13.f90 : New test.
>      * gfortran.dg/ISO_Fortran_binding_13.c : Additional source.
>      * gfortran.dg/ISO_Fortran_binding_14.f90 : New test.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-10-25  7:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-05 18:31 [Patch, fortran] PR91926 - assumed rank optional Paul Richard Thomas
2019-10-09 10:18 ` Christophe Lyon
2019-10-09 11:35   ` Paul Richard Thomas
2019-10-17 13:56     ` Tobias Burnus
2019-10-19 18:10       ` Paul Richard Thomas
2019-10-21 17:59 ` Paul Richard Thomas
2019-10-25  7:29   ` Tobias Burnus

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