Fortran: ordering of hidden procedure arguments [PR107441]
Commit Message
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
Comments
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?).
From b7646403557eca19612c81437f381d4b4dcd51c8 Mon Sep 17 00:00:00 2001
From: Harald Anlauf <anlauf@gmx.de>
Date: Fri, 28 Oct 2022 21:58:08 +0200
Subject: [PATCH] Fortran: ordering of hidden procedure arguments [PR107441]
gcc/fortran/ChangeLog:
PR fortran/107441
* trans-decl.cc (create_function_arglist): Adjust the ordering of
automatically generated hidden procedure arguments to match the
documented ABI for gfortran.
gcc/testsuite/ChangeLog:
PR fortran/107441
* gfortran.dg/optional_absent_6.f90: New test.
---
gcc/fortran/trans-decl.cc | 15 +++--
.../gfortran.dg/optional_absent_6.f90 | 60 +++++++++++++++++++
2 files changed, 71 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/optional_absent_6.f90
@@ -2508,7 +2508,7 @@ create_function_arglist (gfc_symbol * sym)
tree fndecl;
gfc_formal_arglist *f;
tree typelist, hidden_typelist;
- tree arglist, hidden_arglist;
+ tree arglist, hidden_arglist, optional_arglist, strlen_arglist;
tree type;
tree parm;
@@ -2518,6 +2518,7 @@ create_function_arglist (gfc_symbol * sym)
the new FUNCTION_DECL node. */
arglist = NULL_TREE;
hidden_arglist = NULL_TREE;
+ strlen_arglist = optional_arglist = NULL_TREE;
typelist = TYPE_ARG_TYPES (TREE_TYPE (fndecl));
if (sym->attr.entry_master)
@@ -2644,7 +2645,7 @@ create_function_arglist (gfc_symbol * sym)
length = build_decl (input_location,
PARM_DECL, get_identifier (name), len_type);
- hidden_arglist = chainon (hidden_arglist, length);
+ strlen_arglist = chainon (strlen_arglist, length);
DECL_CONTEXT (length) = fndecl;
DECL_ARTIFICIAL (length) = 1;
DECL_ARG_TYPE (length) = len_type;
@@ -2712,7 +2713,7 @@ create_function_arglist (gfc_symbol * sym)
PARM_DECL, get_identifier (name),
boolean_type_node);
- hidden_arglist = chainon (hidden_arglist, tmp);
+ optional_arglist = chainon (optional_arglist, tmp);
DECL_CONTEXT (tmp) = fndecl;
DECL_ARTIFICIAL (tmp) = 1;
DECL_ARG_TYPE (tmp) = boolean_type_node;
@@ -2863,10 +2864,16 @@ create_function_arglist (gfc_symbol * sym)
typelist = TREE_CHAIN (typelist);
}
+ /* Add hidden present status for optional+value arguments. */
+ arglist = chainon (arglist, optional_arglist);
+
/* Add the hidden string length parameters, unless the procedure
is bind(C). */
if (!sym->attr.is_bind_c)
- arglist = chainon (arglist, hidden_arglist);
+ arglist = chainon (arglist, strlen_arglist);
+
+ /* Add hidden extra arguments for the gfortran library. */
+ arglist = chainon (arglist, hidden_arglist);
gcc_assert (hidden_typelist == NULL_TREE
|| TREE_VALUE (hidden_typelist) == void_type_node);
new file mode 100644
@@ -0,0 +1,60 @@
+! { dg-do run }
+! PR fortran/107441
+!
+! Test VALUE + OPTIONAL for integer/real/...
+! in the presence of non-optional character dummies
+
+program bugdemo
+ implicit none
+ character :: s = 'a'
+ integer :: t
+
+ t = testoptional(s)
+ call test2 (s)
+ call test3 (s)
+ call test4 (w='123',x=42)
+
+contains
+
+ function testoptional (w, x) result(t)
+ character, intent(in) :: w
+ integer, intent(in), value, optional :: x
+ integer :: t
+ print *, 'present(x) is', present(x)
+ t = 0
+ if (present (x)) stop 1
+ end function testoptional
+
+ subroutine test2 (w, x)
+ character, intent(in) :: w
+ integer, intent(in), value, optional :: x
+ print*, 'present(x) is', present(x)
+ if (present (x)) stop 2
+ end subroutine test2
+
+ subroutine test3 (w, x)
+ character, intent(in), optional :: w
+ integer, intent(in), value, optional :: x
+ print *, 'present(w) is', present(w)
+ print *, 'present(x) is', present(x)
+ if (.not. present (w)) stop 3
+ if (present (x)) stop 4
+ end subroutine test3
+
+ subroutine test4 (r, w, x)
+ real, value, optional :: r
+ character(*), intent(in), optional :: w
+ integer, value, optional :: x
+ print *, 'present(r) is', present(r)
+ print *, 'present(w) is', present(w)
+ print *, 'present(x) is', present(x)
+ if (present (r)) stop 5
+ if (.not. present (w)) stop 6
+ if (.not. present (x)) stop 7
+ print *, 'x=', x
+ print *, 'len(w)=', len(w)
+ if (len(w) /= 3) stop 8
+ if (x /= 42) stop 9
+ end subroutine test4
+
+end program bugdemo
--
2.35.3