[og12] OpenACC: Don't gang-privatize artificial variables: restrict to blocks (was: [PATCH] [og12] OpenACC: Don't gang-privatize artificial variables)
Hi!
On 2022-10-18T15:59:24+0100, Julian Brown wrote:
> On Tue, 18 Oct 2022 16:46:07 +0200 Thomas Schwinge
> wrote:
>> On 2022-10-14T13:38:56+, Julian Brown wrote:
>> ..., but to my surprised, that did fire in one occasion:
>>
>> > --- a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
>> > +++ b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
>> > @@ -94,9 +94,7 @@ contains
>> > !$acc parallel copy(array)
>> > !$acc loop gang private(array) ! { dg-line l_loop[incr c_loop] }
>> > ! { dg-note {variable 'i' in 'private' clause isn't candidate for
>> > adjusting OpenACC privatization level: not addressable} "" { target *-*-*
>> > } l_loop$c_loop }
>> > -! { dg-note {variable 'array\.[0-9]+' in 'private' clause is
>> > candidate for adjusting OpenACC privatization level} "" { target *-*-* }
>> > l_loop$c_loop }
>> > -! { dg-note {variable 'array\.[0-9]+' ought to be adjusted for
>> > OpenACC privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop }
>> > -! { dg-note {variable 'array\.[0-9]+' adjusted for OpenACC
>> > privatization level: 'gang'} "" { target { ! { openacc_host_selected || {
>> > openacc_nvidia_accel_selected && __OPTIMIZE__ } } } } l_loop$c_loop }
>> > +! { dg-note {variable 'array\.[0-9]+' in 'private' clause isn't
>> > candidate for adjusting OpenACC privatization level: artificial} "" {
>> > target *-*-* } l_loop$c_loop }
>> > ! { dg-message {sorry, unimplemented: target cannot support alloca}
>> > PR65181 { target openacc_nvidia_accel_selected } l_loop$c_loop }
>> > do i = 1, 10
>> >array(i) = 9*i
>>
>> ... here. Note "variable 'array\.[0-9]+' in 'private' clause";
>> everywhere else we have "declared in block".
>>
>> As part of your verification, have you already looked into whether the
>> new behavior is correct here, or does this one need to continue to be
>> "adjusted for OpenACC privatization level: 'gang'"? If the latter,
>> should we check 'if (res && block && DECL_ARTIFICIAL (decl))' instead
>> of 'if (res && DECL_ARTIFICIAL (decl))', or is there some wrong
>> setting of 'DECL_ARTIFICIAL' -- or are we maybe looking at an
>> inappropriate 'decl'? (Thinking of commit
>> r12-7580-g7a5e036b61aa088e6b8564bc9383d37dfbb4801e "[OpenACC
>> privatization] Analyze 'lookup_decl'-translated DECL [PR90115,
>> PR102330, PR104774]", for example.)
>
> I haven't looked in detail, but it seems to me that the "artificial"
> flag isn't appropriate for that decl, which is (derived from?) a
> user-visible symbol. So, I'm not sure what's going on there (and yes
> the commit you mention looks like it could be relevant, I think?).
> There are probably subtleties I'm not aware of...
Until we've got that worked out, let's simply restrict the
'DECL_ARTIFICIAL' handling to 'block's only; pushed to devel/omp/gcc-12
commit 9a50d282f03f7f1e1ad00de917143a2a8e0c0ee0
"[og12] OpenACC: Don't gang-privatize artificial variables: restrict to blocks",
see attached.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
Re: [og12] OpenACC: Don't gang-privatize artificial variables: restrict to blocks (was: [PATCH] [og12] OpenACC: Don't gang-privatize artificial variables)
Hi!
On 2022-10-28T10:11:04+0200, I wrote:
> On 2022-10-18T15:59:24+0100, Julian Brown wrote:
>> On Tue, 18 Oct 2022 16:46:07 +0200 Thomas Schwinge
>> wrote:
>>> On 2022-10-14T13:38:56+, Julian Brown wrote:
>>> ..., but to my surprised, that did fire in one occasion:
>>>
>>> > --- a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
>>> > +++ b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
>>> > @@ -94,9 +94,7 @@ contains
>>> > !$acc parallel copy(array)
>>> > !$acc loop gang private(array) ! { dg-line l_loop[incr c_loop] }
>>> > ! { dg-note {variable 'i' in 'private' clause isn't candidate for
>>> > adjusting OpenACC privatization level: not addressable} "" { target *-*-*
>>> > } l_loop$c_loop }
>>> > -! { dg-note {variable 'array\.[0-9]+' in 'private' clause is
>>> > candidate for adjusting OpenACC privatization level} "" { target *-*-* }
>>> > l_loop$c_loop }
>>> > -! { dg-note {variable 'array\.[0-9]+' ought to be adjusted for
>>> > OpenACC privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop }
>>> > -! { dg-note {variable 'array\.[0-9]+' adjusted for OpenACC
>>> > privatization level: 'gang'} "" { target { ! { openacc_host_selected || {
>>> > openacc_nvidia_accel_selected && __OPTIMIZE__ } } } } l_loop$c_loop }
>>> > +! { dg-note {variable 'array\.[0-9]+' in 'private' clause isn't
>>> > candidate for adjusting OpenACC privatization level: artificial} "" {
>>> > target *-*-* } l_loop$c_loop }
>>> > ! { dg-message {sorry, unimplemented: target cannot support alloca}
>>> > PR65181 { target openacc_nvidia_accel_selected } l_loop$c_loop }
>>> > do i = 1, 10
>>> >array(i) = 9*i
>>>
>>> ... here. Note "variable 'array\.[0-9]+' in 'private' clause";
>>> everywhere else we have "declared in block".
>>>
>>> As part of your verification, have you already looked into whether the
>>> new behavior is correct here, or does this one need to continue to be
>>> "adjusted for OpenACC privatization level: 'gang'"? If the latter,
>>> should we check 'if (res && block && DECL_ARTIFICIAL (decl))' instead
>>> of 'if (res && DECL_ARTIFICIAL (decl))', or is there some wrong
>>> setting of 'DECL_ARTIFICIAL' -- or are we maybe looking at an
>>> inappropriate 'decl'? (Thinking of commit
>>> r12-7580-g7a5e036b61aa088e6b8564bc9383d37dfbb4801e "[OpenACC
>>> privatization] Analyze 'lookup_decl'-translated DECL [PR90115,
>>> PR102330, PR104774]", for example.)
>>
>> I haven't looked in detail, but it seems to me that the "artificial"
>> flag isn't appropriate for that decl, which is (derived from?) a
>> user-visible symbol. So, I'm not sure what's going on there (and yes
>> the commit you mention looks like it could be relevant, I think?).
>> There are probably subtleties I'm not aware of...
>
> Until we've got that worked out, let's simply restrict the
> 'DECL_ARTIFICIAL' handling to 'block's only; pushed to devel/omp/gcc-12
> commit 9a50d282f03f7f1e1ad00de917143a2a8e0c0ee0
> "[og12] OpenACC: Don't gang-privatize artificial variables: restrict to
> blocks"
..., see attached now really.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From 9a50d282f03f7f1e1ad00de917143a2a8e0c0ee0 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Tue, 18 Oct 2022 16:59:54 +0200
Subject: [PATCH] [og12] OpenACC: Don't gang-privatize artificial variables:
restrict to blocks
Follow-up to og12 commit d4504346d2a1d6ffecb8b2d8e3e04ab8ea259785
"[og12] OpenACC: Don't gang-privatize artificial variables", to restore
the previous behavior, until we understand what it means for a
'DECL_ARTIFICIAL' to appear in a 'private' clause.
gcc/
* omp-low.cc (oacc_privatization_candidate_p) :
Restrict to 'block's.
libgomp/
* testsuite/libgomp.oacc-fortran/privatized-ref-2.f90: Adjust.
---
gcc/omp-low.cc | 2 +-
libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
index 002f91d930a..66aa11cd32d 100644
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -11409,7 +11409,7 @@ oacc_privatization_candidate_p (const location_t loc, const tree c,
At present, no compiler-generated artificial variables require such
sharing semantics, so this is safe. */
- if (res && DECL_ARTIFICIAL (decl))
+ if (res && block && DECL_ARTIFICIAL (decl))
{
res = false;
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90 b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
index 936285e9f69..498ef70b63a 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
@@ -94,7 +94,9 @
OpenACC: Don't gang-privatize artificial variables [PR90115] (was: [PATCH] [og12] OpenACC: Don't gang-privatize artificial variables)
Hi!
On 2022-10-18T16:46:07+0200, Thomas Schwinge wrote:
> On 2022-10-14T13:38:56+, Julian Brown wrote:
>> This patch prevents compiler-generated artificial variables from being
>> treated as privatization candidates for OpenACC.
>>
>> The rationale is that e.g. "gang-private" variables actually must be
>> shared by each worker and vector spawned within a particular gang, but
>> that sharing is not necessary for any compiler-generated variable (at
>> least at present, but no such need is anticipated either). Variables on
>> the stack (and machine registers) are already private per-"thread"
>> (gang, worker and/or vector), and that's fine for artificial variables.
>
> OK, that seems fine rationale for this change in behavior.
> No contradicting test case jumped onto me, either.
>> Several tests need their scan output patterns adjusted to compensate.
>
> ACK -- surprisingly few. (Some minor fine-tuning necessary for GCC
> master branch, as had to be expected; I'm working on that.)
With those changes...
>> --- a/gcc/omp-low.cc
>> +++ b/gcc/omp-low.cc
>> @@ -11400,6 +11400,28 @@ oacc_privatization_candidate_p (const location_t
>> loc, const tree c,
>> }
>> }
>>
>> + /* If an artificial variable has been added to a bind, e.g.
>> + a compiler-generated temporary structure used by the Fortran
>> front-end, do
>> + not consider it as a privatization candidate. Note that variables on
>> + the stack are private per-thread by default: making them "gang-private"
>> + for OpenACC actually means to share a single instance of a variable
>> + amongst all workers and threads spawned within each gang.
>> + At present, no compiler-generated artificial variables require such
>> + sharing semantics, so this is safe. */
>> +
>> + if (res && DECL_ARTIFICIAL (decl))
>> +{
>> + res = false;
>> +
>> + if (dump_enabled_p ())
>> +{
>> + oacc_privatization_begin_diagnose_var (l_dump_flags, loc, c, decl);
>> + dump_printf (l_dump_flags,
>> + "isn%'t candidate for adjusting OpenACC privatization "
>> + "level: %s\n", "artificial");
>> +}
>> +}
>
> In the source code comment, you say "added to a bind", and that's indeed
> what I was expecting, too, and thus put in:
>
>if (res && DECL_ARTIFICIAL (decl))
> {
> + gcc_checking_assert (block);
> +
>res = false;
>
> ..., but to my surprised, that did fire in one occasion:
>
>> --- a/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
>> +++ b/libgomp/testsuite/libgomp.oacc-fortran/privatized-ref-2.f90
>> @@ -94,9 +94,7 @@ contains
>> !$acc parallel copy(array)
>> !$acc loop gang private(array) ! { dg-line l_loop[incr c_loop] }
>> ! { dg-note {variable 'i' in 'private' clause isn't candidate for
>> adjusting OpenACC privatization level: not addressable} "" { target *-*-* }
>> l_loop$c_loop }
>> -! { dg-note {variable 'array\.[0-9]+' in 'private' clause is candidate
>> for adjusting OpenACC privatization level} "" { target *-*-* } l_loop$c_loop
>> }
>> -! { dg-note {variable 'array\.[0-9]+' ought to be adjusted for OpenACC
>> privatization level: 'gang'} "" { target *-*-* } l_loop$c_loop }
>> -! { dg-note {variable 'array\.[0-9]+' adjusted for OpenACC
>> privatization level: 'gang'} "" { target { ! { openacc_host_selected || {
>> openacc_nvidia_accel_selected && __OPTIMIZE__ } } } } l_loop$c_loop }
>> +! { dg-note {variable 'array\.[0-9]+' in 'private' clause isn't
>> candidate for adjusting OpenACC privatization level: artificial} "" { target
>> *-*-* } l_loop$c_loop }
>> ! { dg-message {sorry, unimplemented: target cannot support alloca}
>> PR65181 { target openacc_nvidia_accel_selected } l_loop$c_loop }
>> do i = 1, 10
>>array(i) = 9*i
>
> ... here. Note "variable 'array\.[0-9]+' in 'private' clause";
> everywhere else we have "declared in block".
>
> As part of your verification, have you already looked into whether the
> new behavior is correct here, or does this one need to continue to be
> "adjusted for OpenACC privatization level: 'gang'"? If the latter,
> should we check 'if (res && block && DECL_ARTIFICIAL (decl))' instead of
> 'if (res && DECL_ARTIFICIAL (decl))'
..., and that change merged in, I've then pushed to master branch
commit 11e811d8e2f63667f60f73731bb934273f5882b8
"OpenACC: Don't gang-privatize artificial variables [PR90115]", see
attached.
Cherry-picked pushed to releases/gcc-12 branch in
commit 9b116c51a451995f1bae8fdac0748fcf3f06aafe
"OpenACC: Don't gang-privatize artificial variables [PR90115]", see
attached.
I've then also done a merge from releases/gcc-12 branch into
devel/omp/gcc-12 branch, taking care of merge conflicts due to
"fine-tuning necessary for GCC master branch", which shouldn't propagate
into devel/omp/gcc-12 branch. Pushed to devel/omp/gcc-12 branch
commit 33eae55cd9effd9e0bb0c3659cc5dfc100b6fd4e
"Merge commit '9b116
Re: [PATCH v2 1/3] libcpp: reject codepoints above 0x10FFFF
On Thu, 2022-10-27 at 19:16 -0400, Ben Boeckel wrote: > Unicode does not support such values because they are unrepresentable > in > UTF-16. Wikipedia pointed me to RFC 3629, which was when UTF-8 introduced this restriction, whereas libcpp was implementing the higher upper limit from the earlier, superceded RFC 2279. The patch looks good to me, assuming it bootstraps and passes usual regression testing, but... > > Signed-off-by: Ben Boeckel > --- > libcpp/ChangeLog | 6 ++ > libcpp/charset.cc | 4 ++-- > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog > index 18d5bcceaf0..4d707277531 100644 > --- a/libcpp/ChangeLog > +++ b/libcpp/ChangeLog > @@ -1,3 +1,9 @@ > +2022-10-27 Ben Boeckel > + > + * include/charset.cc: Reject encodings of codepoints above > 0x10. > + UTF-16 does not support such codepoints and therefore all > Unicode > + rejects such values. > + > 2022-10-19 Lewis Hyatt ...AIUI we now put ChangeLog entries in the blurb part of the patch, so that server-side git scripts add them to the actual ChangeLog file. Does the patch pass: ./contrib/gcc-changelog/git_check_commit.py ? Thanks Dave > > * include/cpplib.h (struct cpp_string): Use new > "string_length" GTY. > diff --git a/libcpp/charset.cc b/libcpp/charset.cc > index 12a398e7527..e9da6674b5f 100644 > --- a/libcpp/charset.cc > +++ b/libcpp/charset.cc > @@ -216,7 +216,7 @@ one_utf8_to_cppchar (const uchar **inbufp, size_t > *inbytesleftp, > if (c <= 0x3FF && nbytes > 5) return EILSEQ; > > /* Make sure the character is valid. */ > - if (c > 0x7FFF || (c >= 0xD800 && c <= 0xDFFF)) return EILSEQ; > + if (c > 0x10 || (c >= 0xD800 && c <= 0xDFFF)) return EILSEQ; > > *cp = c; > *inbufp = inbuf; > @@ -320,7 +320,7 @@ one_utf32_to_utf8 (iconv_t bigend, const uchar > **inbufp, size_t *inbytesleftp, > s += inbuf[bigend ? 2 : 1] << 8; > s += inbuf[bigend ? 3 : 0]; > > - if (s >= 0x7FFF || (s >= 0xD800 && s <= 0xDFFF)) > + if (s > 0x10 || (s >= 0xD800 && s <= 0xDFFF)) > return EILSEQ; > > rval = one_cppchar_to_utf8 (s, outbufp, outbytesleftp);
Re: [PATCH v2 2/3] libcpp: add a function to determine UTF-8 validity of a C string
On Thu, 2022-10-27 at 19:16 -0400, Ben Boeckel wrote: > This simplifies the interface for other UTF-8 validity detections > when a > simple "yes" or "no" answer is sufficient. > > Signed-off-by: Ben Boeckel > --- > libcpp/ChangeLog | 6 ++ > libcpp/charset.cc | 18 ++ > libcpp/internal.h | 2 ++ > 3 files changed, 26 insertions(+) > > diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog > index 4d707277531..4e2c7900ae2 100644 > --- a/libcpp/ChangeLog > +++ b/libcpp/ChangeLog > @@ -1,3 +1,9 @@ > +2022-10-27 Ben Boeckel > + > + * include/charset.cc: Add `_cpp_valid_utf8_str` which > determines > + whether a C string is valid UTF-8 or not. > + * include/internal.h: Add prototype for > `_cpp_valid_utf8_str`. > + > 2022-10-27 Ben Boeckel > > * include/charset.cc: Reject encodings of codepoints above > 0x10. The patch looks good to me, with the same potential caveat that you might need to move the ChangeLog entry from the patch "body" to the leading blurb, to satisfy: ./contrib/gcc-changelog/git_check_commit.py Thanks Dave
adding attributes
Assuming there's no technical reason not to, can someone say what would be involved in adding relevant attributes (at least function ones) like those for C? I'm thinking particularly of target_clones, but others probably make sense. I don't know my way around, but I had a quick look, and it at least wouldn't be as straightforward as I hoped.
Re: [PATCH v2 2/3] libcpp: add a function to determine UTF-8 validity of a C string
On Fri, Oct 28, 2022 at 08:59:16 -0400, David Malcolm wrote: > On Thu, 2022-10-27 at 19:16 -0400, Ben Boeckel wrote: > > This simplifies the interface for other UTF-8 validity detections > > when a > > simple "yes" or "no" answer is sufficient. > > > > Signed-off-by: Ben Boeckel > > --- > > libcpp/ChangeLog | 6 ++ > > libcpp/charset.cc | 18 ++ > > libcpp/internal.h | 2 ++ > > 3 files changed, 26 insertions(+) > > > > diff --git a/libcpp/ChangeLog b/libcpp/ChangeLog > > index 4d707277531..4e2c7900ae2 100644 > > --- a/libcpp/ChangeLog > > +++ b/libcpp/ChangeLog > > @@ -1,3 +1,9 @@ > > +2022-10-27 Ben Boeckel > > + > > + * include/charset.cc: Add `_cpp_valid_utf8_str` which > > determines > > + whether a C string is valid UTF-8 or not. > > + * include/internal.h: Add prototype for > > `_cpp_valid_utf8_str`. > > + > > 2022-10-27 Ben Boeckel > > > > * include/charset.cc: Reject encodings of codepoints above > > 0x10. > > The patch looks good to me, with the same potential caveat that you > might need to move the ChangeLog entry from the patch "body" to the > leading blurb, to satisfy: > ./contrib/gcc-changelog/git_check_commit.py Ah, I had missed that. Now fixed locally for patches 1 and 2; will be in v3 pending some time for further reviews. THanks, --Ben
Re: [PATCH v2 3/3] p1689r5: initial support
On Thu, Oct 27, 2022 at 19:16:44 -0400, Ben Boeckel wrote:
> diff --git a/gcc/testsuite/g++.dg/modules/modules.exp
> b/gcc/testsuite/g++.dg/modules/modules.exp
> index afb323d0efd..7fe8825144f 100644
> --- a/gcc/testsuite/g++.dg/modules/modules.exp
> +++ b/gcc/testsuite/g++.dg/modules/modules.exp
> @@ -28,6 +28,7 @@
> # { dg-module-do [link|run] [xfail] [options] } # link [and run]
>
> load_lib g++-dg.exp
> +load_lib modules.exp
>
> # If a testcase doesn't have special options, use these.
> global DEFAULT_CXXFLAGS
> @@ -237,6 +238,13 @@ proc cleanup_module_files { files } {
> }
> }
>
> +# delete the specified set of dep files
> +proc cleanup_dep_files { files } {
> +foreach file $files {
> + file_on_host delete $file
> +}
> +}
> +
> global testdir
> set testdir $srcdir/$subdir
> proc srcdir {} {
> @@ -310,6 +318,7 @@ foreach src [lsort [find $srcdir/$subdir {*_a.[CHX}]] {
> set std_list [module-init $src]
> foreach std $std_list {
> set mod_files {}
> + set dep_files {}
> global module_do
> set module_do {"compile" "P"}
> set asm_list {}
> @@ -346,6 +355,8 @@ foreach src [lsort [find $srcdir/$subdir {*_a.[CHX}]] {
> set mod_files [find $DEFAULT_REPO *.gcm]
> }
> cleanup_module_files $mod_files
> +
> + cleanup_dep_files $dep_files
> }
> }
> }
These `cleanup_dep_files` hunks are leftovers from my attempts at
getting the P1689 and flags tests working; they'll be gone in v3.
--Ben
[PATCH] 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
From b7646403557eca19612c81437f381d4b4dcd51c8 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
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
diff --git a/gcc/fortran/trans-decl.cc b/gcc/fortran/trans-decl.cc
index 63515b9072a..18842fe2c4b 100644
--- a/gcc/fortran/trans-decl.cc
+++ b/gcc/fortran/trans-decl.cc
@@ -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);
diff --git a/gcc/testsuite/gfortran.dg/optional_absent_6.f90 b/gcc/testsuite/gfortran.dg/optional_absent_6.f90
new file mode 100644
index 000..b8abb06980a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/optional_absent_6.f90
@@ -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, op
