Fortran: ordering of hidden procedure arguments [PR107441]

  Dear all,

the passing of procedure arguments in Fortran sometimes requires
ancillary parameters that are "hidden".  Examples are string length
and the presence status of scalar variables with optional+value
attribute.

The gfortran ABI is actually documented:

https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html

The reporter found that there was a discrepancy between the
caller and the callee.  This is corrected by the attached patch.

Regtested on x86_64-pc-linux-gnu.  OK for mainline?

Thanks,
Harald
  

Le 28/10/2022 à 22:12, Harald Anlauf via Fortran a écrit :
> Dear all,
> 
> the passing of procedure arguments in Fortran sometimes requires
> ancillary parameters that are "hidden".  Examples are string length
> and the presence status of scalar variables with optional+value
> attribute.
> 
> The gfortran ABI is actually documented:
> 
> https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html
> 
> The reporter found that there was a discrepancy between the
> caller and the callee.  This is corrected by the attached patch.
> 
Hello,

I think some discrepancy remains, as gfc_conv_procedure_call accumulates 
coarray stuff into the stringargs, while your change accumulates the 
associated parameter decls separately into hidden_arglist.  It's not 
completely clear to me whether it is really problematic (string length 
and coarray metadata are both integers anyway), but I suspect it is.

Another probable issue is your change to create_function_arglist changes 
arglist/hidden_arglist without also changing typelist/hidden_typelist 
accordingly.  I think a change to gfc_get_function_type is also 
necessary: as the function decl is changed, the decl type need to be 
changed as well.

I will see whether I can manage to exhibit testcases for these issues.
  

Le 30/10/2022 à 20:23, Mikael Morin a écrit :
> 
> I think some discrepancy remains, as gfc_conv_procedure_call accumulates 
> coarray stuff into the stringargs, while your change accumulates the 
> associated parameter decls separately into hidden_arglist.  It's not 
> completely clear to me whether it is really problematic (string length 
> and coarray metadata are both integers anyway), but I suspect it is.
> > I will see whether I can manage to exhibit testcases for these issues.
> 
Here is a -fcoarray=lib example.  It can be placed in gfortran/coarray.

! { dg-do run }
!
! PR fortran/107441
! Check that with -fcoarray=lib, coarray metadata arguments are passed
! in the right order to procedures.
program p
   integer :: ci[*]
   ci = 17
   call s(1, ci, "abcd")
contains
   subroutine s(ra, ca, c)
     integer :: ra, ca[*]
     character(*) :: c
     ca[1] = 13
     if (ra /= 1) stop 1
     if (this_image() == 1) then
       if (ca /= 13) stop 2
     else
       if (ca /= 17) stop 3
     end if
     if (len(c) /= 4) stop 4
     if (c /= "abcd") stop 5
   end subroutine s
end program p
  

Le 30/10/2022 à 20:23, Mikael Morin a écrit :
> Another probable issue is your change to create_function_arglist changes 
> arglist/hidden_arglist without also changing typelist/hidden_typelist 
> accordingly.  I think a change to gfc_get_function_type is also 
> necessary: as the function decl is changed, the decl type need to be 
> changed as well.
> 
> I will see whether I can manage to exhibit testcases for these issues.
> 
Here is a test for the type vs decl mismatch.

! { dg-do run }
!
! PR fortran/107441
! Check that procedure types and procedure decls match when the procedure
! has both chaacter-typed and optional value args.

program p
   interface
     subroutine i(c, o)
       character(*) :: c
       integer, optional, value :: o
     end subroutine i
   end interface
   procedure(i), pointer :: pp
   call pp("abcd")
contains
   subroutine s(c, o)
     character(*) :: c
     integer, optional, value :: o
     if (present(o)) stop 1
     if (len(c) /= 4) stop 2
     if (c /= "abcd") stop 3
   end subroutine s
end program p
  

Le 30/10/2022 à 22:25, Mikael Morin a écrit :
> Le 30/10/2022 à 20:23, Mikael Morin a écrit :
>> Another probable issue is your change to create_function_arglist 
>> changes arglist/hidden_arglist without also changing 
>> typelist/hidden_typelist accordingly.  I think a change to 
>> gfc_get_function_type is also necessary: as the function decl is 
>> changed, the decl type need to be changed as well.
>>
>> I will see whether I can manage to exhibit testcases for these issues.
>>
> Here is a test for the type vs decl mismatch.
> 
> ! { dg-do run }
> !
> ! PR fortran/107441
> ! Check that procedure types and procedure decls match when the procedure
> ! has both chaacter-typed and optional value args.
> 
> program p
>    interface
>      subroutine i(c, o)
>        character(*) :: c
>        integer, optional, value :: o
>      end subroutine i
>    end interface
>    procedure(i), pointer :: pp
A pointer initialization is missing here:
     pp => s
>    call pp("abcd")
> contains
>    subroutine s(c, o)
>      character(*) :: c
>      integer, optional, value :: o
>      if (present(o)) stop 1
>      if (len(c) /= 4) stop 2
>      if (c /= "abcd") stop 3
>    end subroutine s
> end program p
> 

With the additional initialization, the test passes, so it's not very 
useful.  The type mismatch is visible in the dump though, so maybe a 
dump match can be used.
  

Hi Mikael,

thanks a lot, your testcases broke my initial (and incorrect) patch
in multiple ways.  I understand now that the right solution is much
simpler and smaller.

I've added your testcases, see attached, with a simple scan of the
dump for the generated order of hidden arguments in the function decl
for the last testcase.

Regtested again on x86_64-pc-linux-gnu.  OK now?

Thanks,
Harald


Am 31.10.22 um 10:57 schrieb Mikael Morin:
> Le 30/10/2022 à 22:25, Mikael Morin a écrit :
>> Le 30/10/2022 à 20:23, Mikael Morin a écrit :
>>> Another probable issue is your change to create_function_arglist
>>> changes arglist/hidden_arglist without also changing
>>> typelist/hidden_typelist accordingly.  I think a change to
>>> gfc_get_function_type is also necessary: as the function decl is
>>> changed, the decl type need to be changed as well.
>>>
>>> I will see whether I can manage to exhibit testcases for these issues.
>>>
>> Here is a test for the type vs decl mismatch.
>>
>> ! { dg-do run }
>> !
>> ! PR fortran/107441
>> ! Check that procedure types and procedure decls match when the procedure
>> ! has both chaacter-typed and optional value args.
>>
>> program p
>>    interface
>>      subroutine i(c, o)
>>        character(*) :: c
>>        integer, optional, value :: o
>>      end subroutine i
>>    end interface
>>    procedure(i), pointer :: pp
> A pointer initialization is missing here:
>      pp => s
>>    call pp("abcd")
>> contains
>>    subroutine s(c, o)
>>      character(*) :: c
>>      integer, optional, value :: o
>>      if (present(o)) stop 1
>>      if (len(c) /= 4) stop 2
>>      if (c /= "abcd") stop 3
>>    end subroutine s
>> end program p
>>
>
> With the additional initialization, the test passes, so it's not very
> useful.  The type mismatch is visible in the dump though, so maybe a
> dump match can be used.
>
  

Le 31/10/2022 à 21:29, Harald Anlauf via Fortran a écrit :
> Hi Mikael,
> 
> thanks a lot, your testcases broke my initial (and incorrect) patch
> in multiple ways.  I understand now that the right solution is much
> simpler and smaller.
> 
> I've added your testcases, see attached, with a simple scan of the
> dump for the generated order of hidden arguments in the function decl
> for the last testcase.
> 
> Regtested again on x86_64-pc-linux-gnu.  OK now?
> 
Unfortunately no, the coarray case works, but the other problem remains.
The type problem is not visible in the definition of S, it is in the 
declaration of S's prototype in P.

S is defined as:

void s (character(kind=1)[1:_c] & restrict c, integer(kind=4) o, 
logical(kind=1) _o, integer(kind=8) _c)
{
...
}

but P has:

void p ()
{
   static void s (character(kind=1)[1:] & restrict, integer(kind=4), 
integer(kind=8), logical(kind=1));
   void (*<T63a>) (character(kind=1)[1:] & restrict, integer(kind=4), 
integer(kind=8), logical(kind=1)) pp;

   pp = s;
...
}
  

Am 02.11.22 um 18:20 schrieb Mikael Morin:
> Unfortunately no, the coarray case works, but the other problem remains.
> The type problem is not visible in the definition of S, it is in the
> declaration of S's prototype in P.
>
> S is defined as:
>
> void s (character(kind=1)[1:_c] & restrict c, integer(kind=4) o,
> logical(kind=1) _o, integer(kind=8) _c)
> {
> ...
> }
>
> but P has:
>
> void p ()
> {
>    static void s (character(kind=1)[1:] & restrict, integer(kind=4),
> integer(kind=8), logical(kind=1));
>    void (*<T63a>) (character(kind=1)[1:] & restrict, integer(kind=4),
> integer(kind=8), logical(kind=1)) pp;
>
>    pp = s;
> ...
> }

Right, now I see it too.  Simplified case:

program p
   call s ("abcd")
contains
   subroutine s(c, o)
     character(*) :: c
     integer, optional, value :: o
   end subroutine s
end

I do see what needs to be done in gfc_get_function_type, which seems
in fact very simple.  But I get really lost in create_function_arglist
when trying to get the typelist right.

One thing is I really don't understand how the (hidden_)typelist is
managed here.  How does that macro TREE_CHAIN work?  Can we somehow
chain two typelists the same way we chain arguments?

(Failing that, I tried to split the loop over the dummy arguments in
create_function_arglist into two passes, one for the optional+value
variant, and one for the rest.  It turned out to be a bad idea...)

Harald
  

Le 02/11/2022 à 22:19, Harald Anlauf via Fortran a écrit :
> Am 02.11.22 um 18:20 schrieb Mikael Morin:
>> Unfortunately no, the coarray case works, but the other problem remains.
>> The type problem is not visible in the definition of S, it is in the 
>> declaration of S's prototype in P.
>>
>> S is defined as:
>>
>> void s (character(kind=1)[1:_c] & restrict c, integer(kind=4) o, 
>> logical(kind=1) _o, integer(kind=8) _c)
>> {
>> ...
>> }
>>
>> but P has:
>>
>> void p ()
>> {
>>    static void s (character(kind=1)[1:] & restrict, integer(kind=4), 
>> integer(kind=8), logical(kind=1));
>>    void (*<T63a>) (character(kind=1)[1:] & restrict, integer(kind=4), 
>> integer(kind=8), logical(kind=1)) pp;
>>
>>    pp = s;
>> ...
>> }
> 
> Right, now I see it too.  Simplified case:
> 
> program p
>    call s ("abcd")
> contains
>    subroutine s(c, o)
>      character(*) :: c
>      integer, optional, value :: o
>    end subroutine s
> end
> 
> I do see what needs to be done in gfc_get_function_type, which seems
> in fact very simple.  But I get really lost in create_function_arglist
> when trying to get the typelist right.
> 
> One thing is I really don't understand how the (hidden_)typelist is
> managed here.  How does that macro TREE_CHAIN work?  Can we somehow
> chain two typelists the same way we chain arguments?
> 
TREE_CHAIN is just a linked list "next" pointer like there is in the 
fortran frontend a "next" pointer in gfc_ref or gfc_actual_arglist 
structures.
Yes, we can chain typelists; the implementation of chainon in tree.cc is 
just TREE_CHAIN appending under the hood.

> (Failing that, I tried to split the loop over the dummy arguments in
> create_function_arglist into two passes, one for the optional+value
> variant, and one for the rest.  It turned out to be a bad idea...)
> 
Not necessarily a bad idea, but one has to be careful to keep linked 
lists synchronized with argument walking.

The most simple, I think, is to move the hidden_typelist advancement for 
optional, value presence arguments from the main loop to a preliminary loop.

I hope it helps.
  

Am 03.11.22 um 11:06 schrieb Mikael Morin:
> Le 02/11/2022 à 22:19, Harald Anlauf via Fortran a écrit :
>> Am 02.11.22 um 18:20 schrieb Mikael Morin:
>>> Unfortunately no, the coarray case works, but the other problem remains.
>>> The type problem is not visible in the definition of S, it is in the
>>> declaration of S's prototype in P.
>>>
>>> S is defined as:
>>>
>>> void s (character(kind=1)[1:_c] & restrict c, integer(kind=4) o,
>>> logical(kind=1) _o, integer(kind=8) _c)
>>> {
>>> ...
>>> }
>>>
>>> but P has:
>>>
>>> void p ()
>>> {
>>>    static void s (character(kind=1)[1:] & restrict, integer(kind=4),
>>> integer(kind=8), logical(kind=1));
>>>    void (*<T63a>) (character(kind=1)[1:] & restrict, integer(kind=4),
>>> integer(kind=8), logical(kind=1)) pp;
>>>
>>>    pp = s;
>>> ...
>>> }
>>
>> Right, now I see it too.  Simplified case:
>>
>> program p
>>    call s ("abcd")
>> contains
>>    subroutine s(c, o)
>>      character(*) :: c
>>      integer, optional, value :: o
>>    end subroutine s
>> end
>>
>> I do see what needs to be done in gfc_get_function_type, which seems
>> in fact very simple.  But I get really lost in create_function_arglist
>> when trying to get the typelist right.
>>
>> One thing is I really don't understand how the (hidden_)typelist is
>> managed here.  How does that macro TREE_CHAIN work?  Can we somehow
>> chain two typelists the same way we chain arguments?
>>
> TREE_CHAIN is just a linked list "next" pointer like there is in the
> fortran frontend a "next" pointer in gfc_ref or gfc_actual_arglist
> structures.
> Yes, we can chain typelists; the implementation of chainon in tree.cc is
> just TREE_CHAIN appending under the hood.
>
>> (Failing that, I tried to split the loop over the dummy arguments in
>> create_function_arglist into two passes, one for the optional+value
>> variant, and one for the rest.  It turned out to be a bad idea...)
>>
> Not necessarily a bad idea, but one has to be careful to keep linked
> lists synchronized with argument walking.
>
> The most simple, I think, is to move the hidden_typelist advancement for
> optional, value presence arguments from the main loop to a preliminary
> loop.
>
> I hope it helps.
>

I've spent some time not only staring at create_function_arglist,
but trying several variations handling the declared hidden parms,
and applying the necessary adjustments to gfc_get_function_type.
(Managing linked trees is not the issue, just understanding them.)
I've been unable to get the declarations in sync, and would need
help how to debug the mess I've created.  Dropping my patch for
the time being.
  

Le 03/11/2022 à 23:03, Harald Anlauf a écrit :
> Am 03.11.22 um 11:06 schrieb Mikael Morin:
>> Le 02/11/2022 à 22:19, Harald Anlauf via Fortran a écrit :
>>> I do see what needs to be done in gfc_get_function_type, which seems
>>> in fact very simple.  But I get really lost in create_function_arglist
>>> when trying to get the typelist right.
>>>
>>> One thing is I really don't understand how the (hidden_)typelist is
>>> managed here.  How does that macro TREE_CHAIN work?  Can we somehow
>>> chain two typelists the same way we chain arguments?
>>>
>> TREE_CHAIN is just a linked list "next" pointer like there is in the
>> fortran frontend a "next" pointer in gfc_ref or gfc_actual_arglist
>> structures.
>> Yes, we can chain typelists; the implementation of chainon in tree.cc is
>> just TREE_CHAIN appending under the hood.
>>
>>> (Failing that, I tried to split the loop over the dummy arguments in
>>> create_function_arglist into two passes, one for the optional+value
>>> variant, and one for the rest.  It turned out to be a bad idea...)
>>>
>> Not necessarily a bad idea, but one has to be careful to keep linked
>> lists synchronized with argument walking.
>>
>> The most simple, I think, is to move the hidden_typelist advancement for
>> optional, value presence arguments from the main loop to a preliminary
>> loop.
>>
>> I hope it helps.
>>
> 
> I've spent some time not only staring at create_function_arglist,
> but trying several variations handling the declared hidden parms,
> and applying the necessary adjustments to gfc_get_function_type.
> (Managing linked trees is not the issue, just understanding them.)
> I've been unable to get the declarations in sync, and would need
> help how to debug the mess I've created.  Dropping my patch for
> the time being.
> 
If you want, we can meet on IRC somewhen (tonight?).