[PATCH] fortran: fix handling of options -ffpe-trap and -ffpe-summary [PR110957]
Dear all, the attached simple patch fixes a mixup of error messages for -ffpe-trap and -ffpe-summary. While at it, I though it might be useful to accept 'none' as allowable argument to -ffpe-trap, so that traps previously set on the command line may be cleared. This change is also documented. Regtested on x86_64-pc-linux-gnu. OK for mainline? *** The reporter also suggested to detect and handle -fno-trapping-math when any trap is enabled. I am not so sure that this can be required. In gfortran, specifying -ffpe-trap sets the FPU mask in the main and has no further effect. Or am I missing something? Any further opinions or insights? Thanks, Harald From 75dc455f21cea07e64b422c9994ab8879df388de Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 6 Oct 2023 22:21:56 +0200 Subject: [PATCH] fortran: fix handling of options -ffpe-trap and -ffpe-summary [PR110957] gcc/fortran/ChangeLog: PR fortran/110957 * invoke.texi: Update documentation to reflect '-ffpe-trap=none'. * options.cc (gfc_handle_fpe_option): Fix mixup up of error messages for options -ffpe-trap and -ffpe-summary. Accept '-ffpe-trap=none' to clear FPU traps previously set on command line. --- gcc/fortran/invoke.texi | 6 -- gcc/fortran/options.cc | 9 ++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi index 38150b1e29e..10387e39501 100644 --- a/gcc/fortran/invoke.texi +++ b/gcc/fortran/invoke.texi @@ -1294,7 +1294,8 @@ Specify a list of floating point exception traps to enable. On most systems, if a floating point exception occurs and the trap for that exception is enabled, a SIGFPE signal will be sent and the program being aborted, producing a core file useful for debugging. @var{list} -is a (possibly empty) comma-separated list of the following +is a (possibly empty) comma-separated list of either @samp{none} (to +clear the set of exceptions to be trapped), or of the following exceptions: @samp{invalid} (invalid floating point operation, such as @code{SQRT(-1.0)}), @samp{zero} (division by zero), @samp{overflow} (overflow in a floating point operation), @samp{underflow} (underflow @@ -1314,7 +1315,8 @@ If the option is used more than once in the command line, the lists will be joined: '@code{ffpe-trap=}@var{list1} @code{ffpe-trap=}@var{list2}' is equivalent to @code{ffpe-trap=}@var{list1},@var{list2}. -Note that once enabled an exception cannot be disabled (no negative form). +Note that once enabled an exception cannot be disabled (no negative form), +except by clearing all traps by specifying @samp{none}. Many, if not most, floating point operations incur loss of precision due to rounding, and hence the @code{ffpe-trap=inexact} is likely to diff --git a/gcc/fortran/options.cc b/gcc/fortran/options.cc index 27311961325..2ad22478042 100644 --- a/gcc/fortran/options.cc +++ b/gcc/fortran/options.cc @@ -555,9 +555,12 @@ gfc_handle_fpe_option (const char *arg, bool trap) pos++; result = 0; - if (!trap && strncmp ("none", arg, pos) == 0) + if (strncmp ("none", arg, pos) == 0) { - gfc_option.fpe_summary = 0; + if (trap) + gfc_option.fpe = 0; + else + gfc_option.fpe_summary = 0; arg += pos; pos = 0; continue; @@ -586,7 +589,7 @@ gfc_handle_fpe_option (const char *arg, bool trap) break; } } - if (!result && !trap) + if (!result && trap) gfc_fatal_error ("Argument to %<-ffpe-trap%> is not valid: %s", arg); else if (!result) gfc_fatal_error ("Argument to %<-ffpe-summary%> is not valid: %s", arg); -- 2.35.3
Re: [Patch, fortran] PR67740 - Wrong association status of allocatable character pointer in derived types
Hi Paul, On 10/11/23 10:48, Paul Richard Thomas wrote: Hi All, The title line of the PR should have been changed a long time since. As noted in comment 5, the original problem was fixed in 10.5. This patch fixes the problem described in comments 4 and 6, where the hidden string length component was not being set in pointer assignment of character arrays. The fix regtests. OK for trunk and 13-branch? this is OK for both. I'd suggest to wait a couple of days or a week before backporting. Thanks for the patch! Harald Thanks are due to Harald for bringing this to my attention. Paul Fortran: Set hidden string length for pointer components [PR67440] 2023-10-11 Paul Thomas gcc/fortran PR fortran/pr67740 * trans-expr.cc (gfc_trans_pointer_assignment): Set the hidden string length component for pointer assignment to character pointer components. gcc/testsuite/ PR fortran/87477 * gfortran.dg/pr67740.f90: New test
Re: [Patch, fortran] PR67740 - Wrong association status of allocatable character pointer in derived types
Hi Paul, the patch is fine, but I forgot the mention that the testcase needs fixing: Instead of ! {dg-do compile } you'll likely want ! { dg-do run } (Note the space before the dg-command.) Cheers, Harald On 10/11/23 21:06, Harald Anlauf wrote: Hi Paul, On 10/11/23 10:48, Paul Richard Thomas wrote: Hi All, The title line of the PR should have been changed a long time since. As noted in comment 5, the original problem was fixed in 10.5. This patch fixes the problem described in comments 4 and 6, where the hidden string length component was not being set in pointer assignment of character arrays. The fix regtests. OK for trunk and 13-branch? this is OK for both. I'd suggest to wait a couple of days or a week before backporting. Thanks for the patch! Harald Thanks are due to Harald for bringing this to my attention. Paul Fortran: Set hidden string length for pointer components [PR67440] 2023-10-11 Paul Thomas gcc/fortran PR fortran/pr67740 * trans-expr.cc (gfc_trans_pointer_assignment): Set the hidden string length component for pointer assignment to character pointer components. gcc/testsuite/ PR fortran/87477 * gfortran.dg/pr67740.f90: New test
[PATCH] Fortran: name conflict between internal procedure and derived type [PR104351]
Dear All, the attached trivial patch fixes (= catches) a forgotten corner-case in the detection of a name conflict between an internal procedure and a local declaration for the case that the latter is a derived type. Another torture test by Gerhard... ;-) Used to ICE previously. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 75dc455f21cea07e64b422c9994ab8879df388de Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 6 Oct 2023 22:21:56 +0200 Subject: [PATCH] fortran: fix handling of options -ffpe-trap and -ffpe-summary [PR110957] gcc/fortran/ChangeLog: PR fortran/110957 * invoke.texi: Update documentation to reflect '-ffpe-trap=none'. * options.cc (gfc_handle_fpe_option): Fix mixup up of error messages for options -ffpe-trap and -ffpe-summary. Accept '-ffpe-trap=none' to clear FPU traps previously set on command line. --- gcc/fortran/invoke.texi | 6 -- gcc/fortran/options.cc | 9 ++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi index 38150b1e29e..10387e39501 100644 --- a/gcc/fortran/invoke.texi +++ b/gcc/fortran/invoke.texi @@ -1294,7 +1294,8 @@ Specify a list of floating point exception traps to enable. On most systems, if a floating point exception occurs and the trap for that exception is enabled, a SIGFPE signal will be sent and the program being aborted, producing a core file useful for debugging. @var{list} -is a (possibly empty) comma-separated list of the following +is a (possibly empty) comma-separated list of either @samp{none} (to +clear the set of exceptions to be trapped), or of the following exceptions: @samp{invalid} (invalid floating point operation, such as @code{SQRT(-1.0)}), @samp{zero} (division by zero), @samp{overflow} (overflow in a floating point operation), @samp{underflow} (underflow @@ -1314,7 +1315,8 @@ If the option is used more than once in the command line, the lists will be joined: '@code{ffpe-trap=}@var{list1} @code{ffpe-trap=}@var{list2}' is equivalent to @code{ffpe-trap=}@var{list1},@var{list2}. -Note that once enabled an exception cannot be disabled (no negative form). +Note that once enabled an exception cannot be disabled (no negative form), +except by clearing all traps by specifying @samp{none}. Many, if not most, floating point operations incur loss of precision due to rounding, and hence the @code{ffpe-trap=inexact} is likely to diff --git a/gcc/fortran/options.cc b/gcc/fortran/options.cc index 27311961325..2ad22478042 100644 --- a/gcc/fortran/options.cc +++ b/gcc/fortran/options.cc @@ -555,9 +555,12 @@ gfc_handle_fpe_option (const char *arg, bool trap) pos++; result = 0; - if (!trap && strncmp ("none", arg, pos) == 0) + if (strncmp ("none", arg, pos) == 0) { - gfc_option.fpe_summary = 0; + if (trap) + gfc_option.fpe = 0; + else + gfc_option.fpe_summary = 0; arg += pos; pos = 0; continue; @@ -586,7 +589,7 @@ gfc_handle_fpe_option (const char *arg, bool trap) break; } } - if (!result && !trap) + if (!result && trap) gfc_fatal_error ("Argument to %<-ffpe-trap%> is not valid: %s", arg); else if (!result) gfc_fatal_error ("Argument to %<-ffpe-summary%> is not valid: %s", arg); -- 2.35.3
Re: [PATCH] Fortran: name conflict between internal procedure and derived type [PR104351]
Dear All, sorry for attaching the wrong patch - this time it is the correct one! Harald On 10/11/23 21:39, Harald Anlauf wrote: Dear All, the attached trivial patch fixes (= catches) a forgotten corner-case in the detection of a name conflict between an internal procedure and a local declaration for the case that the latter is a derived type. Another torture test by Gerhard... ;-) Used to ICE previously. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 84de03c97f899df91f2b7e7af4a5bbc09412a3fe Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 11 Oct 2023 21:29:35 +0200 Subject: [PATCH] Fortran: name conflict between internal procedure and derived type [PR104351] gcc/fortran/ChangeLog: PR fortran/104351 * decl.cc (get_proc_name): Extend name conflict detection between internal procedure and previous declaration also to derived type. gcc/testsuite/ChangeLog: PR fortran/104351 * gfortran.dg/derived_function_interface_1.f90: Adjust pattern. * gfortran.dg/pr104351.f90: New test. --- gcc/fortran/decl.cc| 4 +++- .../gfortran.dg/derived_function_interface_1.f90 | 2 +- gcc/testsuite/gfortran.dg/pr104351.f90 | 14 ++ 3 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr104351.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index 4a3c5b86de0..bdd3be32a46 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -1404,7 +1404,9 @@ get_proc_name (const char *name, gfc_symbol **result, bool module_fcn_entry) /* Trap declarations of attributes in encompassing scope. The signature for this is that ts.kind is nonzero for no-CLASS entity. For a CLASS entity, ts.kind is zero. */ - if ((sym->ts.kind != 0 || sym->ts.type == BT_CLASS) + if ((sym->ts.kind != 0 + || sym->ts.type == BT_CLASS + || sym->ts.type == BT_DERIVED) && !sym->attr.implicit_type && sym->attr.proc == 0 && gfc_current_ns->parent != NULL diff --git a/gcc/testsuite/gfortran.dg/derived_function_interface_1.f90 b/gcc/testsuite/gfortran.dg/derived_function_interface_1.f90 index 24a00950912..5438ad49c6a 100644 --- a/gcc/testsuite/gfortran.dg/derived_function_interface_1.f90 +++ b/gcc/testsuite/gfortran.dg/derived_function_interface_1.f90 @@ -38,7 +38,7 @@ end function ext_fun contains - type(foo) function fun() ! { dg-error "already has an explicit interface" } + type(foo) function fun() ! { dg-error "has an explicit interface" } end function fun ! { dg-error "Expecting END PROGRAM" } end diff --git a/gcc/testsuite/gfortran.dg/pr104351.f90 b/gcc/testsuite/gfortran.dg/pr104351.f90 new file mode 100644 index 000..86b47e03340 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr104351.f90 @@ -0,0 +1,14 @@ +! { dg-do compile } +! PR fortran/104351 +! Contributed by G.Steinmetz + +program p + implicit none + type t + end type + type(t) :: f +contains + real function f() result(z) ! { dg-error "has an explicit interface" } +z = 0.0 ! { dg-error "assignment" } + end function f ! { dg-error "Expecting END PROGRAM" } +end -- 2.35.3
Re: [patch] fortran/intrinsic.texi: Add 'passed by value' to signal handler
Hi Tobias, Am 16.10.23 um 19:11 schrieb Tobias Burnus: Yesterday, someone was confused because the signal handler did not work. It turned out that the created Fortran procedure used as handler used pass by reference - and 'signal' passed the it by value. This patch adds the 'passed by value' to the wording: "@var{HANDLER} to be executed with a single integer argument passed by value" OK for mainline? I think the patch qualifies as obvious. While at it, you might consider removing the comment a few lines below the place you are changing, @c TODO: What should the interface of the handler be? Does it take arguments? and enhance the given example by e.g.: subroutine handler_print (signal_number) integer, value :: signal_number print *, "In handler_print: received signal number", signal_number end subroutine handler_print Thanks, Harald Tobias - 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
[PATCH] Fortran: out of bounds access with nested implied-do IO [PR111837]
Dear All, the attached patch fixes a dependency check in frontend optimzation for nested implied-do IO. The problem appeared for >= 3 loops only as the check considered dependencies to be only of band form instead of triangular form. Regtested on x86_64-pc-linux-gnu. OK for mainline? As this fixes a regression since 8-release, I plan to backport to all active branches. Thanks, Harald From 43ec8b856a67a1b70744e5c0d50ea7fa2dd9a8ee Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 16 Oct 2023 21:02:20 +0200 Subject: [PATCH] Fortran: out of bounds access with nested implied-do IO [PR111837] gcc/fortran/ChangeLog: PR fortran/111837 * frontend-passes.cc (traverse_io_block): Dependency check of loop nest shall be triangular, not banded. gcc/testsuite/ChangeLog: PR fortran/111837 * gfortran.dg/implied_do_io_8.f90: New test. --- gcc/fortran/frontend-passes.cc| 2 +- gcc/testsuite/gfortran.dg/implied_do_io_8.f90 | 18 ++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/implied_do_io_8.f90 diff --git a/gcc/fortran/frontend-passes.cc b/gcc/fortran/frontend-passes.cc index 136a292807d..536884b13f0 100644 --- a/gcc/fortran/frontend-passes.cc +++ b/gcc/fortran/frontend-passes.cc @@ -1326,7 +1326,7 @@ traverse_io_block (gfc_code *code, bool *has_reached, gfc_code *prev) if (iters[i]) { gfc_expr *var = iters[i]->var; - for (int j = i - 1; j < i; j++) + for (int j = 0; j < i; j++) { if (iters[j] && (var_in_expr (var, iters[j]->start) diff --git a/gcc/testsuite/gfortran.dg/implied_do_io_8.f90 b/gcc/testsuite/gfortran.dg/implied_do_io_8.f90 new file mode 100644 index 000..c66a0f6fde6 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/implied_do_io_8.f90 @@ -0,0 +1,18 @@ +! { dg-do run } +! { dg-additional-options "-fcheck=bounds" } +! PR fortran/111837 - out of bounds access with front-end optimization + +program implied_do_bug + implicit none + integer :: i,j,k + real:: arr(1,1,1) + integer :: ni(1) + ni(1) = 1 + arr = 1 + write(*,*) (((arr(i,j,k), i=1,ni(k)), k=1,1), j=1,1) + write(*,*) (((arr(i,j,k), i=1,ni(k)), j=1,1), k=1,1) + write(*,*) (((arr(k,i,j), i=1,ni(k)), k=1,1), j=1,1) + write(*,*) (((arr(k,i,j), i=1,ni(k)), j=1,1), k=1,1) + write(*,*) (((arr(j,k,i), i=1,ni(k)), k=1,1), j=1,1) + write(*,*) (((arr(j,k,i), i=1,ni(k)), j=1,1), k=1,1) +end -- 2.35.3
Re: [patch] fortran/intrinsic.texi: Improve SIGNAL intrinsic entry
Tobias, your latest patch - which you already pushed - removes the intrinsic declaration of signal. This can lead to a user's confusion and undesired results when the code is compiled e.g. with -std=f2018, because call signal (10, 1) ! 10 = SIGUSR1 and 1 = SIG_IGN (on some systems) could be mapped to the wrong external instead of the libgfortran function _gfortran_signal_sub_int etc., or you could get other compile-time errors with the example code. I strongly recommend to restore the intrinsic declaration. Cheers, Harald Am 17.10.23 um 09:47 schrieb Tobias Burnus: Hi Harald, On 16.10.23 20:31, Harald Anlauf wrote: Hi Tobias, Am 16.10.23 um 19:11 schrieb Tobias Burnus: OK for mainline? I think the patch qualifies as obvious. While at it, you might consider removing the comment a few lines below the place you are changing, @c TODO: What should the interface of the handler be? Does it take arguments? and enhance the given example by e.g.: Updated version attached – I will commit it later today, unless anyone has follow-up suggestions before. Thanks for the suggestions, Tobias - 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: [patch] fortran/intrinsic.texi: Improve SIGNAL intrinsic entry
Hi Tobias, On 10/17/23 19:36, Tobias Burnus wrote: Hi Harald, On 17.10.23 19:02, Harald Anlauf wrote: your latest patch - which you already pushed - removes the intrinsic declaration of signal. Only to 'signal' or also to 'sleep'? I have now added both in the attach patch. you are right: both should be declared as intrinsic. Thanks, Harald (Not yet committed.) Tobias - 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
[PATCH] Fortran/OpenMP: event handle in task detach cannot be a coarray [PR104131]
Dear all, the attached simple patch adds a forgotten check that an event handle cannot be a coarray. This case appears to have been overlooked in the original fix for this PR. I intend to commit as obvious within 24h unless there are comments. Thanks, Harald From 2b5ed32cacfe84dc4df74b4dccf16ac830d9eb98 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 24 Oct 2023 21:18:02 +0200 Subject: [PATCH] Fortran/OpenMP: event handle in task detach cannot be a coarray [PR104131] gcc/fortran/ChangeLog: PR fortran/104131 * openmp.cc (resolve_omp_clauses): Add check that event handle is not a coarray. gcc/testsuite/ChangeLog: PR fortran/104131 * gfortran.dg/gomp/pr104131-2.f90: New test. --- gcc/fortran/openmp.cc | 3 +++ gcc/testsuite/gfortran.dg/gomp/pr104131-2.f90 | 12 2 files changed, 15 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr104131-2.f90 diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc index 1cc65d7fa49..08081dacde4 100644 --- a/gcc/fortran/openmp.cc +++ b/gcc/fortran/openmp.cc @@ -8967,6 +8967,9 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, else if (omp_clauses->detach->symtree->n.sym->attr.dimension > 0) gfc_error ("The event handle at %L must not be an array element", &omp_clauses->detach->where); + else if (omp_clauses->detach->symtree->n.sym->attr.codimension) + gfc_error ("The event handle at %L must not be a coarray", + &omp_clauses->detach->where); else if (omp_clauses->detach->symtree->n.sym->ts.type == BT_DERIVED || omp_clauses->detach->symtree->n.sym->ts.type == BT_CLASS) gfc_error ("The event handle at %L must not be part of " diff --git a/gcc/testsuite/gfortran.dg/gomp/pr104131-2.f90 b/gcc/testsuite/gfortran.dg/gomp/pr104131-2.f90 new file mode 100644 index 000..3978a6ac31a --- /dev/null +++ b/gcc/testsuite/gfortran.dg/gomp/pr104131-2.f90 @@ -0,0 +1,12 @@ +! { dg-do compile } +! { dg-options "-fopenmp -fcoarray=single" } +! PR fortran/104131 - event handle cannot be a coarray + +program p + use iso_c_binding, only: c_intptr_t + implicit none + integer, parameter :: omp_event_handle_kind = c_intptr_t + integer (kind=omp_event_handle_kind) :: x[*] +!$omp task detach (x) ! { dg-error "The event handle at \\\(1\\\) must not be a coarray" } +!$omp end task +end -- 2.35.3
Re: [PATCH] Fortran/OpenMP: event handle in task detach cannot be a coarray [PR104131]
Dear all, Tobias argued in the PR that the testcase should actually be valid. Therefore withdrawing the patch. Sorry for expecting this to be a low-hanging fruit... Harald On 10/24/23 22:23, rep.dot@gmail.com wrote: On 24 October 2023 21:25:01 CEST, Harald Anlauf wrote: Dear all, the attached simple patch adds a forgotten check that an event handle cannot be a coarray. This case appears to have been overlooked in the original fix for this PR. I intend to commit as obvious within 24h unless there are comments. diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc index 1cc65d7fa49..08081dacde4 100644 --- a/gcc/fortran/openmp.cc +++ b/gcc/fortran/openmp.cc @@ -8967,6 +8967,9 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses *omp_clauses, else if (omp_clauses->detach->symtree->n.sym->attr.dimension > 0) gfc_error ("The event handle at %L must not be an array element", &omp_clauses->detach->where); + else if (omp_clauses->detach->symtree->n.sym->attr.codimension) + gfc_error ("The event handle at %L must not be a coarray", ISTM that we usually do not mention "element" when talking about undue (co)array access. Maybe we want to streamline this specific error message? LGTM otherwise. Thanks for your dedication! + &omp_clauses->detach->where); else if (omp_clauses->detach->symtree->n.sym->ts.type == BT_DERIVED || omp_clauses->detach->symtree->n.sym->ts.type == BT_CLASS) gfc_error ("The event handle at %L must not be part of "
Re: [Patch, fortran] PR104625 ICE in fixup_array_ref, at fortran/resolve.cc:9275 since r10-2912-g70570ec192745095
Hi Paul, this looks all good to me. It is great that you added the handling of nested parentheses to the resolution, as that appears to be needed also in other situations. Thanks for the patch! Harald On 10/26/23 19:14, Paul Richard Thomas wrote: Hi All, The attached patch fixes the original problem, in which parentheses around the selector in select type constructs caused ICES. Stacked parentheses caused problems in trans-stmt.cc. Rather than tracking this down, the redundant parentheses were removed on resolution of the selector expression. Fixing the primary problem revealed "Unclassifiable statement" errors when using array references of the associate variable and this was fixed as well. Finally, the error triggered by using associate variables associated with non-variable selectors was corrected to ensure that only vector indexed selectors were flagged up as such. The secondary error in associate_55.f90 was corrected for this, since the selector might or might not be vector indexed. Regtests fine - OK for trunk? Paul Fortran: Fix some problems with SELECT TYPE selectors [PR104625]. 2023-10-26 Paul Thomas gcc/fortran PR fortran/104625 * expr.cc (gfc_check_vardef_context): Check that the target does have a vector index before emitting the specific error. * match.cc (copy_ts_from_selector_to_associate): Ensure that class valued operator expressions set the selector rank and use the rank to provide the associate variable with an appropriate array spec. * resolve.cc (resolve_operator): Reduce stacked parentheses to a single pair. (fixup_array_ref): Extract selector symbol from parentheses. gcc/testsuite/ PR fortran/104625 * gfortran.dg/pr104625.f90: New test. * gfortran.dg/associate_55.f90: Change error check text.
[PATCH] Fortran: diagnostics of MODULE PROCEDURE declaration conflicts [PR104649]
Dear all, the attached patch improves the diagnostics of MODULE PROCEDURE declaration conflicts, when one of the declarations is an alternate return. We used to ICE before. Steve identified the cause of the issue and provided a partial fix. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 9d591a73f070e6090b7c59dca928b84b1c261d92 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 26 Oct 2023 22:32:35 +0200 Subject: [PATCH] Fortran: diagnostics of MODULE PROCEDURE declaration conflicts [PR104649] gcc/fortran/ChangeLog: PR fortran/104649 * decl.cc (gfc_match_formal_arglist): Handle conflicting declarations of a MODULE PROCEDURE when one of the declarations is an alternate return. gcc/testsuite/ChangeLog: PR fortran/104649 * gfortran.dg/pr104649.f90: New test. Co-authored-by: Steven G. Kargl --- gcc/fortran/decl.cc| 21 +--- gcc/testsuite/gfortran.dg/pr104649.f90 | 44 ++ 2 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/pr104649.f90 diff --git a/gcc/fortran/decl.cc b/gcc/fortran/decl.cc index bdd3be32a46..4893c582065 100644 --- a/gcc/fortran/decl.cc +++ b/gcc/fortran/decl.cc @@ -6796,12 +6796,25 @@ ok: || (p->next == NULL && q->next != NULL)) arg_count_mismatch = true; else if ((p->sym == NULL && q->sym == NULL) - || strcmp (p->sym->name, q->sym->name) == 0) + || (p->sym && q->sym + && strcmp (p->sym->name, q->sym->name) == 0)) continue; else - gfc_error_now ("Mismatch in MODULE PROCEDURE formal " - "argument names (%s/%s) at %C", - p->sym->name, q->sym->name); + { + if (q->sym == NULL) + gfc_error_now ("MODULE PROCEDURE formal argument %qs " + "conflicts with alternate return at %C", + p->sym->name); + else if (p->sym == NULL) + gfc_error_now ("MODULE PROCEDURE formal argument is " + "alternate return and conflicts with " + "%qs in the separate declaration at %C", + q->sym->name); + else + gfc_error_now ("Mismatch in MODULE PROCEDURE formal " + "argument names (%s/%s) at %C", + p->sym->name, q->sym->name); + } } if (arg_count_mismatch) diff --git a/gcc/testsuite/gfortran.dg/pr104649.f90 b/gcc/testsuite/gfortran.dg/pr104649.f90 new file mode 100644 index 000..f301ffcde1f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr104649.f90 @@ -0,0 +1,44 @@ +! { dg-do compile } +! { dg-options "-w" } +! PR fortran/104649 +! Contributed by G.Steinmetz + +module m + interface +module subroutine s(x) + real :: x +end + end interface +end +submodule(m) m2 +contains + module subroutine s(*) ! { dg-error "conflicts with alternate return" } + end +end + +module n + interface + module subroutine s(*) + end + end interface +end +submodule(n) n2 +contains + module subroutine s(x) ! { dg-error "formal argument is alternate return" } +real :: x + end +end + +module p + interface + module subroutine s(x) + real :: x + end + end interface +end +submodule(p) p2 +contains + module subroutine s(y) ! { dg-error "Mismatch in MODULE PROCEDURE formal argument names" } +real :: y + end +end -- 2.35.3
Re: [Patch, fortran] PR104555 - ICE in gfc_compare_derived_types, at fortran/interface.cc:628 since r10-2912-g70570ec192745095
Hi Paul, code->expr1->symtree->n.sym->ts = code->expr2->ts; + /* Sometimes the selector expression is given the typespec of the +'_data' field, which is logical enough but inappropraite here. */ s/inappropraite/inappropriate/ + if (code->expr2->ts.type == BT_DERIVED otherwise it LGTM. Thanks for the patch! Harald On 10/29/23 13:29, Paul Richard Thomas wrote: Bizarrely, since the fix for pr101625, the testcase compiles and runs correctly with s/select type (y => x)/select type (y => (x))/ ! The fix is straightforward and appears to be one of those wrinkles arising from the use of associate variables as a selector. The fault is reasonable since the expression is a reference to the _data field, which is of derived type. However, being a select type selector, the selector must be a class with that declared type. Regtests fine. OK for mainline? Paul Fortran: Fix a problem with SELECT TYPE selectors [PR104555]. 2023-10-29 Paul Thomas gcc/fortran PR fortran/104555 * resolve.cc (resolve_select_type): If the selector expression has no class component references and the expression is a derived type, copy the typespec of the symbol to that of the expression. gcc/testsuite/ PR fortran/104555 * gfortran.dg/pr104555.f90: New test.
Re: [Patch, fortran] PR98498 - Interp request: defined operators and unlimited polymorphic
Hi Paul, Am 01.11.23 um 19:02 schrieb Paul Richard Thomas: The interpretation request came in a long time ago but I only just got around to implementing it. The updated text from the standard is in the comment. Now I am writing this, I think that I should perhaps use switch(op)/case rather than using if/else if and depending on the order of the gfc_intrinsic_op enum being maintained. Thoughts? the logic is likely harder to parse with if/else than with switch(op)/case. However, I do not think that the order of the enum will ever be changed, as the module format relies on that very order. The testcase runs fine with both mainline and nagfor. I think that compile-only with counts of star-eq and star_not should suffice. I found other cases that are rejected even with your patch, but which are accepted by nagfor. Example: print *, ('a' == c) Nagfor prints F at runtime as expected, as it correctly resolves this to star_eq. Further examples can be easily constructed. Can you have a look? Thanks, Harald Regtests with no regressions. OK for mainline? Paul Fortran: Defined operators with unlimited polymorphic args [PR98498] 2023-11-01 Paul Thomas gcc/fortran PR fortran/98498 * interface.cc (upoly_ok): New function. (gfc_extend_expr): Use new function to ensure that defined operators using unlimited polymorphic formal arguments do not override their intrinsic uses. gcc/testsuite/ PR fortran/98498 * gfortran.dg/interface_50.f90: New test.
[PATCH] Fortran: passing of allocatable/pointer arguments to OPTIONAL+VALUE [PR92887]
Dear all, I've dusted off and cleaned up a previous attempt to fix the handling of allocatable or pointer actual arguments to OPTIONAL+VALUE dummies. The standard says that a non-allocated / non-associated actual argument in that case shall be treated as non-present. However, gfortran's calling conventions demand that the presence status for OPTIONAL+VALUE is passed as a hidden argument, while we need to pass something on the stack which has the right type. The solution is to conditionally create a temporary when needed. Testcase checked with NAG. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 6927612d97a8e7360e651bb081745fc7659a4c4b Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 1 Nov 2023 22:55:36 +0100 Subject: [PATCH] Fortran: passing of allocatable/pointer arguments to OPTIONAL+VALUE [PR92887] gcc/fortran/ChangeLog: PR fortran/92887 * trans-expr.cc (conv_cond_temp): Helper function for creation of a conditional temporary. (gfc_conv_procedure_call): Handle passing of allocatable or pointer actual argument to dummy with OPTIONAL + VALUE attribute. Actual arguments that are not allocated or associated are treated as not present. gcc/testsuite/ChangeLog: PR fortran/92887 * gfortran.dg/value_optional_1.f90: New test. --- gcc/fortran/trans-expr.cc | 50 ++- .../gfortran.dg/value_optional_1.f90 | 83 +++ 2 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/value_optional_1.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 1b8be081a17..1c06ecb3c28 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6030,6 +6030,28 @@ post_call: } +/* Create "conditional temporary" to handle scalar dummy variables with the + OPTIONAL+VALUE attribute that shall not be dereferenced. Use null value + as fallback. Only instances of intrinsic basic type are supported. */ + +void +conv_cond_temp (gfc_se * parmse, gfc_expr * e, tree cond) +{ + tree temp; + gcc_assert (e->ts.type != BT_DERIVED && e->ts.type != BT_CLASS); + gcc_assert (e->rank == 0); + temp = gfc_create_var (TREE_TYPE (parmse->expr), "condtemp"); + TREE_STATIC (temp) = 1; + TREE_CONSTANT (temp) = 1; + TREE_READONLY (temp) = 1; + DECL_INITIAL (temp) = build_zero_cst (TREE_TYPE (temp)); + parmse->expr = fold_build3_loc (input_location, COND_EXPR, + TREE_TYPE (parmse->expr), + cond, parmse->expr, temp); + parmse->expr = gfc_evaluate_now (parmse->expr, &parmse->pre); +} + + /* Generate code for a procedure call. Note can return se->post != NULL. If se->direct_byref is set then se->expr contains the return parameter. Return nonzero, if the call has alternate specifiers. @@ -6470,9 +6492,31 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && fsym->ts.type != BT_CLASS && fsym->ts.type != BT_DERIVED) { - if (e->expr_type != EXPR_VARIABLE - || !e->symtree->n.sym->attr.optional - || e->ref != NULL) + /* F2018:15.5.2.12 Argument presence and + restrictions on arguments not present. */ + if (e->expr_type == EXPR_VARIABLE + && (gfc_expr_attr (e).allocatable +|| gfc_expr_attr (e).pointer)) + { + gfc_se argse; + tree cond; + gfc_init_se (&argse, NULL); + argse.want_pointer = 1; + gfc_conv_expr (&argse, e); + cond = fold_convert (TREE_TYPE (argse.expr), + null_pointer_node); + cond = fold_build2_loc (input_location, NE_EXPR, + logical_type_node, + argse.expr, cond); + vec_safe_push (optionalargs, + fold_convert (boolean_type_node, + cond)); + /* Create "conditional temporary". */ + conv_cond_temp (&parmse, e, cond); + } + else if (e->expr_type != EXPR_VARIABLE + || !e->symtree->n.sym->attr.optional + || e->ref != NULL) vec_safe_push (optionalargs, boolean_true_node); else { diff --git a/gcc/testsuite/gfortran.dg/value_optional_1.f90 b/gcc/testsuite/gfortran.dg/value_optional_1.f90 new file mode 100644 index 000..2f95316de52 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/value_optional_1.f90 @@ -0,0 +1,83 @@ +! { dg-do run } +! PR fortran/92887 +! +! Test passing nullified/disassociated pointer or unalloc allocatable +! to OPTIONAL + VALUE + +program p + implicit none !(type, external) + integer, allocatable :: aa + real, pointer :: pp + character,allocatable :: ca + character,pointer :: cp + complex, allocatable :: za + complex, pointer :: zp + type t + integer, allocatable :: aa + real, pointer :: pp => NULL() + complex, allocatable :: za + end type t + type(t) :: tt + nullify (pp, cp, zp)
Re: [Patch, fortran] PR98498 - Interp request: defined operators and unlimited polymorphic
Hi Paul, Am 02.11.23 um 19:18 schrieb Paul Richard Thomas: Hi Harald, I was overthinking the problem. The rejected cases led me to a fix that can only be described as a considerable simplification compared with the first patch! this patch is *much* simpler, makes more sense, and works here. :-) The testcase now reflects the requirements of the standard and regtests without failures. OK for mainline? Yes, OK for mainline. Thanks, Harald Thanks Paul Fortran: Defined operators with unlimited polymorphic args [PR98498] 2023-11-02 Paul Thomas gcc/fortran PR fortran/98498 * interface.cc (upoly_ok): Defined operators using unlimited polymorphic formal arguments must not override the intrinsic operator use. gcc/testsuite/ PR fortran/98498 * gfortran.dg/interface_50.f90: New test. On Wed, 1 Nov 2023 at 20:12, Harald Anlauf wrote: Hi Paul, Am 01.11.23 um 19:02 schrieb Paul Richard Thomas: The interpretation request came in a long time ago but I only just got around to implementing it. The updated text from the standard is in the comment. Now I am writing this, I think that I should perhaps use switch(op)/case rather than using if/else if and depending on the order of the gfc_intrinsic_op enum being maintained. Thoughts? the logic is likely harder to parse with if/else than with switch(op)/case. However, I do not think that the order of the enum will ever be changed, as the module format relies on that very order. The testcase runs fine with both mainline and nagfor. I think that compile-only with counts of star-eq and star_not should suffice. I found other cases that are rejected even with your patch, but which are accepted by nagfor. Example: print *, ('a' == c) Nagfor prints F at runtime as expected, as it correctly resolves this to star_eq. Further examples can be easily constructed. Can you have a look? Thanks, Harald Regtests with no regressions. OK for mainline? Paul Fortran: Defined operators with unlimited polymorphic args [PR98498] 2023-11-01 Paul Thomas gcc/fortran PR fortran/98498 * interface.cc (upoly_ok): New function. (gfc_extend_expr): Use new function to ensure that defined operators using unlimited polymorphic formal arguments do not override their intrinsic uses. gcc/testsuite/ PR fortran/98498 * gfortran.dg/interface_50.f90: New test.
[PATCH] Fortran: fix issue with multiple references of a procedure pointer [PR97245]
Dear all, this is a rather weird bug with a very simple fix. If a procedure pointer is referenced in a CALL, a symbol was created shadowing the original declaration if it was host-associated. Funnily, this affected only references of the procedure pointer after the first CALL. Regtested on x86_64-pc-linux-gnu. OK for mainline? Would it be OK to backport this fix to 13-branch? Thanks, Harald From 1ca323b8d58846d0890a8595ba9fc7bc7eee8fdd Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 3 Nov 2023 19:41:54 +0100 Subject: [PATCH] Fortran: fix issue with multiple references of a procedure pointer [PR97245] gcc/fortran/ChangeLog: PR fortran/97245 * match.cc (gfc_match_call): If a procedure pointer has already been resolved, do not create a new symbol in a procedure reference of the same name shadowing the first one if it is host-associated. gcc/testsuite/ChangeLog: PR fortran/97245 * gfortran.dg/proc_ptr_53.f90: New test. --- gcc/fortran/match.cc | 1 + gcc/testsuite/gfortran.dg/proc_ptr_53.f90 | 35 +++ 2 files changed, 36 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/proc_ptr_53.f90 diff --git a/gcc/fortran/match.cc b/gcc/fortran/match.cc index f848e52be4c..9e3571d3dbe 100644 --- a/gcc/fortran/match.cc +++ b/gcc/fortran/match.cc @@ -5064,6 +5064,7 @@ gfc_match_call (void) right association is made. They are thrown out in resolution.) ... */ if (!sym->attr.generic + && !sym->attr.proc_pointer && !sym->attr.subroutine && !sym->attr.function) { diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_53.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_53.f90 new file mode 100644 index 000..29dd08d9f75 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/proc_ptr_53.f90 @@ -0,0 +1,35 @@ +! { dg-do compile } +! PR fortran/97245 - ASSOCIATED intrinsic did not recognize a +!pointer variable the second time it is used + +MODULE formulaciones + IMPLICIT NONE + + ABSTRACT INTERFACE + SUBROUTINE proc_void() + END SUBROUTINE proc_void + end INTERFACE + + PROCEDURE(proc_void), POINTER :: pADJSensib => NULL() + +CONTAINS + + subroutine calculo() +PROCEDURE(proc_void), POINTER :: otherprocptr => NULL() + +IF (associated(pADJSensib)) THEN + CALL pADJSensib () +ENDIF +IF (associated(pADJSensib)) THEN! this was erroneously rejected + CALL pADJSensib () +END IF + +IF (associated(otherprocptr)) THEN + CALL otherprocptr () +ENDIF +IF (associated(otherprocptr)) THEN + CALL otherprocptr () +END IF + end subroutine calculo + +END MODULE formulaciones -- 2.35.3
Re: [PATCH 1/2] libgfortran: Remove early return if extent is zero [PR112371]
Hi Mikael, Am 06.11.23 um 12:43 schrieb Mikael Morin: Remove the early return present in function templates for transformational functions doing a (masked) reduction of an array along a dimension. This early return, which triggered if the extent in the reduction dimension was zero, was wrong because even if the reduction operation degenerates to a constant value in that case, one has to loop anyway along the other dimensions to initialize every element of the resulting array with that constant value. The offending piece of code was present in several places, and this removes them all. Namely, the impacted m4 files are ifunction.m4 for regular functions and types, ifunction-s.m4 for character minloc and maxloc, and ifunction-s2.m4 for character minval and maxval. I wonder if the correct fix would be to replace (instead of deleting) diff --git a/libgfortran/m4/ifunction.m4 b/libgfortran/m4/ifunction.m4 index c64217ec5db..480649cf691 100644 --- a/libgfortran/m4/ifunction.m4 +++ b/libgfortran/m4/ifunction.m4 @@ -232,8 +232,6 @@ m'name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, } len = GFC_DESCRIPTOR_EXTENT(array,dim); - if (len <= 0) -return; mbase = mask->base_addr; by the following: if (len < 0) len = 0; See ifunction.m4, lines 56ff, which check if the result of len = GFC_DESCRIPTOR_EXTENT(array,dim); is negative. I haven't tried to create a testcase, though. Similarly for the other templates. Thanks, Harald
Re: [PATCH 2/2] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371]
Hi Mikael, Am 06.11.23 um 12:43 schrieb Mikael Morin: Remove the forced overwrite of the first dimension of the result array descriptor to set it to zero extent, in the function templates for transformational functions doing an array reduction along a dimension. This overwrite, which happened before early returning in case the result array was empty, was wrong because an array may have a non-zero extent in the first dimension and still be empty if it has a zero extent in a higher dimension. Overwriting the dimension was resulting in wrong array result upper bound for the first dimension in that case. The offending piece of code was present in several places, and this removes them all. More precisely, there is only one case to fix for logical reduction functions, and there are three cases for other reduction functions, corresponding to non-masked reduction, reduction with array mask, and reduction with scalar mask. The impacted m4 files are ifunction_logical.m4 for logical reduction functions, ifunction.m4 for regular functions and types, ifunction-s.m4 for character minloc and maxloc, ifunction-s2.m4 for character minval and maxval, and ifindloc1.m4 for findloc. while your fix seems mechanical and correct, I wonder if you looked at the following pre-existing irregularity which can be seen in this snippet: diff --git a/libgfortran/m4/ifunction.m4 b/libgfortran/m4/ifunction.m4 index 480649cf691..abc15b430ab 100644 --- a/libgfortran/m4/ifunction.m4 +++ b/libgfortran/m4/ifunction.m4 @@ -96,12 +96,7 @@ name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); if (alloc_size == 0) - { - /* Make sure we have a zero-sized array. */ - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); - return; - - } + return; } else { This is all enclosed in a block which has if (retarray->base_addr == NULL) but allocates and sets retarray->base_addr, while @@ -290,11 +285,7 @@ m'name`'rtype_qual`_'atype_code` ('rtype` * const restrict retarray, retarray->dtype.rank = rank; if (alloc_size == 0) - { - /* Make sure we have a zero-sized array. */ - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); - return; - } + return; else retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); and @@ -454,11 +445,7 @@ void alloc_size = GFC_DESCRIPTOR_STRIDE(retarray,rank-1) * extent[rank-1]; if (alloc_size == 0) - { - /* Make sure we have a zero-sized array. */ - GFC_DIMENSION_SET(retarray->dim[0], 0, -1, 1); - return; - } + return; else retarray->base_addr = xmallocarray (alloc_size, sizeof (rtype_name)); } do not set retarray->base_addr to non-NULL for alloc_size == 0. Do you know if the first snippet can be safely rewritten to avoid the (hopefully pointless) xmallocarray for alloc_size == 0? Thanks, Harald
Re: [PATCH 2/2] libgfortran: Remove empty array descriptor first dimension overwrite [PR112371]
Hi Mikael, Am 06.11.23 um 20:19 schrieb Mikael Morin: This change to the testcase: diff --git a/gcc/testsuite/gfortran.dg/bound_11.f90 b/gcc/testsuite/gfortran.dg/bound_11.f90 index 170eba4ddfd..2e96f843476 100644 --- a/gcc/testsuite/gfortran.dg/bound_11.f90 +++ b/gcc/testsuite/gfortran.dg/bound_11.f90 @@ -88,6 +88,7 @@ contains m4 = .false. i = 1 r = sum(a, dim=i) + if (.not. allocated(r)) stop 210 if (any(shape(r) /= (/ 3, 0, 7 /))) stop 211 if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 212 i = 2 @@ -104,6 +105,7 @@ contains if (any(ubound(r) /= (/ 9, 3, 0 /))) stop 218 i = 1 r = sum(a, dim=i, mask=m1) + if (.not. allocated(r)) stop 220 if (any(shape(r) /= (/ 3, 0, 7 /))) stop 221 if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 222 i = 2 @@ -120,6 +122,7 @@ contains if (any(ubound(r) /= (/ 9, 3, 0 /))) stop 228 i = 1 r = sum(a, dim=i, mask=m4) + if (.not. allocated(r)) stop 230 if (any(shape(r) /= (/ 3, 0, 7 /))) stop 231 if (any(ubound(r) /= (/ 3, 0, 7 /))) stop 232 i = 2 gives me a FAIL with STOP 220 (or STOP 230 if the STOP 220 line is commented); the first one with STOP 210 passes. So it is the first snippet with the xmallocarray (which supports zero values see memory.c) call that is the correct one. Good catch, I will open a separate PR. ah, now I see that the case of allocation of zero elements always allocates one byte, which is needed for r.data to be non-null. Go ahead! Harald Mikael
[PATCH] PR fortran/90903 [part2] Add runtime checking for the MVBITS intrinsic
Dear all, finally here comes the second part of runtime checks for the bit manipulation intrinsics, this time MVBITS. This turned out to be more elaborate than the treatment of simple function calls. I chose the path to inline expand MVBITS, which enables additional optimization opportunities in some cases, such as constant arguments. For the case of scalar arguments, this was mostly straightforward. However, for the proper handling of MVBITS as an elemental procedure all honors should go to Paul, as he not only lend me a hand and kindly guided me through the swampland of the scalarizer, but he also managed to placate the gimple part of gcc. Regtested on x86_64-pc-linux-gnu. OK for master? Thanks, Harald PR fortran/90903 [part2] - Add runtime checking for the MVBITS intrinsic Implement inline expansion of the intrinsic elemental subroutine MVBITS with optional runtime checks for valid argument range. gcc/fortran/ChangeLog: * iresolve.c (gfc_resolve_mvbits): Remove unneeded conversion of FROMPOS, LEN and TOPOS arguments to fit a C int. * trans-intrinsic.c (gfc_conv_intrinsic_mvbits): Add inline expansion of MVBITS intrinsic elemental subroutine and add code for runtime argument checking. (gfc_conv_intrinsic_subroutine): Recognise MVBITS intrinsic, but defer handling to gfc_trans_call. * trans-stmt.c (replace_ss): (gfc_trans_call): Adjust to handle inline expansion, scalarization of intrinsic subroutine MVBITS in gfc_conv_intrinsic_mvbits. * trans.h (gfc_conv_intrinsic_mvbits): Add prototype for gfc_conv_intrinsic_mvbits. gcc/testsuite/ChangeLog: * gfortran.dg/check_bits_2.f90: New test. Co-authored-by: Paul Thomas diff --git a/gcc/fortran/iresolve.c b/gcc/fortran/iresolve.c index 73769615c20..c2a4865f28f 100644 --- a/gcc/fortran/iresolve.c +++ b/gcc/fortran/iresolve.c @@ -3311,21 +3311,7 @@ gfc_resolve_mvbits (gfc_code *c) { static const sym_intent INTENTS[] = {INTENT_IN, INTENT_IN, INTENT_IN, INTENT_INOUT, INTENT_IN}; - const char *name; - gfc_typespec ts; - gfc_clear_ts (&ts); - - /* FROMPOS, LEN and TOPOS are restricted to small values. As such, - they will be converted so that they fit into a C int. */ - ts.type = BT_INTEGER; - ts.kind = gfc_c_int_kind; - if (c->ext.actual->next->expr->ts.kind != gfc_c_int_kind) -gfc_convert_type (c->ext.actual->next->expr, &ts, 2); - if (c->ext.actual->next->next->expr->ts.kind != gfc_c_int_kind) -gfc_convert_type (c->ext.actual->next->next->expr, &ts, 2); - if (c->ext.actual->next->next->next->next->expr->ts.kind != gfc_c_int_kind) -gfc_convert_type (c->ext.actual->next->next->next->next->expr, &ts, 2); /* TO and FROM are guaranteed to have the same kind parameter. */ name = gfc_get_string (PREFIX ("mvbits_i%d"), diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 32fe9886c57..3b3bd8629cd 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -11790,6 +11790,169 @@ conv_intrinsic_event_query (gfc_code *code) return gfc_finish_block (&se.pre); } + +/* This is a peculiar case because of the need to do dependency checking. + It is called via trans-stmt.c(gfc_trans_call), where it is picked out as + a special case and this function called instead of + gfc_conv_procedure_call. */ +void +gfc_conv_intrinsic_mvbits (gfc_se *se, gfc_actual_arglist *actual_args, + gfc_loopinfo *loop) +{ + gfc_actual_arglist *actual; + gfc_se argse[5]; + gfc_expr *arg[5]; + gfc_ss *lss; + int n; + + tree from, frompos, len, to, topos; + tree lenmask, oldbits, newbits, bitsize; + tree type, utype, above, mask1, mask2; + + if (loop) +lss = loop->ss; + else +lss = gfc_ss_terminator; + + actual = actual_args; + for (n = 0; n < 5; n++, actual = actual->next) +{ + arg[n] = actual->expr; + gfc_init_se (&argse[n], NULL); + + if (lss != gfc_ss_terminator) + { + gfc_copy_loopinfo_to_se (&argse[n], loop); + /* Find the ss for the expression if it is there. */ + argse[n].ss = lss; + gfc_mark_ss_chain_used (lss, 1); + } + + gfc_conv_expr (&argse[n], arg[n]); + + if (loop) + lss = argse[n].ss; +} + + from= argse[0].expr; + frompos = argse[1].expr; + len = argse[2].expr; + to = argse[3].expr; + topos = argse[4].expr; + + /* The type of the result (TO). */ + type= TREE_TYPE (to); + bitsize = build_int_cst (integer_type_node, TYPE_PRECISION (type)); + + /* Optionally generate code for runtime argument check. */ + if (gfc_option.rtcheck & GFC_RTCHECK_BITS) +{ + tree nbits, below, ccond; + tree fp = fold_convert (long_integer_type_node, frompos); + tree ln = fold_convert (long_integer_type_node, len); + tree tp = fold_convert (long_integer_type_node, topos); + below = fold_build2_loc (input_location, LT_EXPR, + logical_type_node, frompos, +
[PATCH] PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90
Dear all, my fix for PR fortran/96711 adds a testcase that failed on powerpc64-*-* as well as sparc*-sun-solaris2.11. This is a consequence of the, say, mess, on x86, where we have real kinds 4,8,10,16, with kind=10 being long double and kind=16 being represented as float128, while on sparc real(16) is mapped to long double. I'm not going to comment on power, see PR for details. A minimal solution to the issue would extend an intermediate conversion that is done for e.g. x86 to float128 to a conversion to long double on platforms where the TYPE_PRECISION of long double equals 128. This solves the issue on sparc*-sun-solaris2.11, as confirmed by Rainer. The attached patch does just this, and disables the testcase for target powerpc*-*-*. I expect a more sophisticated solution being needed for that platform. That said, the patch regtests cleanly on x86_64-pc-linux-gnu, and as reported in the PR, sparc-sun-solaris2.11 and i386-pc-solaris2.11. OK for master? Or does anyone with better knowledge of the affected platforms want to take over? As this is a technical regression, it should and could be fixed before the gcc-11 release. My intention to either close the PR after committing, or changing it into some appropriate state (SUSPENDED?) so that it can be handled later, or to rather open a new PR that is finally a target issue. Thanks, Harald PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90 The fix for PR fortran/96711 introduced an intermediate conversion to the float128 type that is used e.g. on x86 but not on some other targets. Use a conversion to long double on targets where this type has a type precision of 128 bit. gcc/fortran/ChangeLog: * trans-intrinsic.c (build_round_expr): Use conversion to long double when this type has a type precision of 128 bit. gcc/testsuite/ChangeLog: * gfortran.dg/pr96711.f90: Skip testcase for target powerpc*-*-*. diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 32fe9886c57..552cdf5f19c 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -395,6 +395,13 @@ build_round_expr (tree arg, tree restype) fn = builtin_decl_for_precision (BUILT_IN_LROUND, argprec); else if (resprec <= LONG_LONG_TYPE_SIZE) fn = builtin_decl_for_precision (BUILT_IN_LLROUND, argprec); + else if (resprec == TYPE_PRECISION (long_double_type_node) + && resprec >= argprec) +{ + int kind = TYPE_PRECISION (long_double_type_node) / 8; + arg = fold_convert (long_double_type_node, arg); + fn = gfc_builtin_decl_for_float_kind (BUILT_IN_ROUND, kind); +} else if (resprec >= argprec && resprec == 128) { /* Search for a real kind suitable as temporary for conversion. */ diff --git a/gcc/testsuite/gfortran.dg/pr96711.f90 b/gcc/testsuite/gfortran.dg/pr96711.f90 index 3761a8ea416..13b6e829ed6 100644 --- a/gcc/testsuite/gfortran.dg/pr96711.f90 +++ b/gcc/testsuite/gfortran.dg/pr96711.f90 @@ -3,6 +3,8 @@ ! { dg-require-effective-target fortran_real_16 } ! { dg-additional-options "-fdump-tree-original" } ! { dg-final { scan-tree-dump-times "_gfortran_stop_numeric" 2 "original" } } +! { dg-skip-if "" { "powerpc*-*-*" } } +! This test expects a target that supports IEEE128. ! ! PR fortran/96711 - ICE on NINT() Function
Aw: Re: [PATCH] PR/fortran 96983 - ICE compiling gfortran.dg/pr96711.f90
Dear Tobias, I can see that you want a proper fix. However, after having looked at all these comments about the powerpc situation, I do not really think I'd want to ever touch that stuff. It's clearly beyond my capabilities and ressources. I do feel responsible for the regression introduced by my naive patch for PR96711, which is a breakage for sparc and powerpc. The patch was meant as a minimal solution, or rather a bandaid to fix sparc, and skip the test for powerpc, and leave the powerpc situation to someone with more knowledge of that target. If this patch is rejected, but the regression is to be fixed before gcc-11 release, I can therefore only offer to revert the patch for PR96711, to reopen it and unassign. Thanks, Harald
[PATCH] PR fortran/97036 - [F2018] Allow ELEMENTAL RECURSIVE procedure prefix
As stated in the PR, the Fortran 2018 standard removed the restriction prohibiting ELEMENTAL RECURSIVE procedures. Adjust the relevant check. Regtested on x86_64-pc-linux-gnu. OK for master? Thanks, Harald PR fortran/97036 - [F2018] Allow ELEMENTAL RECURSIVE procedure prefix gcc/fortran/ChangeLog: * symbol.c (gfc_check_conflict): Allow ELEMENTAL RECURSIVE procedure prefix for -std=f2018. gcc/testsuite/ChangeLog: * gfortran.dg/pr97036.f90: New test. diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c index abd3b5ccfd0..df1e8965daa 100644 --- a/gcc/fortran/symbol.c +++ b/gcc/fortran/symbol.c @@ -569,7 +569,7 @@ gfc_check_conflict (symbol_attribute *attr, const char *name, locus *where) conf_std (allocatable, dummy, GFC_STD_F2003); conf_std (allocatable, function, GFC_STD_F2003); conf_std (allocatable, result, GFC_STD_F2003); - conf (elemental, recursive); + conf_std (elemental, recursive, GFC_STD_F2018); conf (in_common, dummy); conf (in_common, allocatable); diff --git a/gcc/testsuite/gfortran.dg/pr97036.f90 b/gcc/testsuite/gfortran.dg/pr97036.f90 new file mode 100644 index 000..cfe51debce1 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr97036.f90 @@ -0,0 +1,27 @@ +! { dg-do compile } +! { dg-options "-std=f2018" } +! PR fortran/97036 - [F2018] Allow ELEMENTAL RECURSIVE procedure prefix + +module m97036 + implicit none +contains + impure elemental recursive subroutine foo (n) +integer, intent(in) :: n +integer :: k(n), f(n), i +k = [ (i-1, i=1,n) ] +f = fac (k) +print *, f + end subroutine foo + elemental recursive subroutine bla () + end subroutine bla + elemental recursive function fac (k) result (f) +integer, intent(in) :: k +integer :: f +f = 1 +if (k > 1) f = k*fac (k-1) + end function fac +end module + use m97036 + implicit none + call foo ([4,5]) +end
*PING* [PATCH] PR fortran/90903 [part2] Add runtime checking for the MVBITS intrinsic
*ping* > Gesendet: Sonntag, 13. September 2020 um 23:24 Uhr > Von: "Harald Anlauf" > An: "fortran" , "gcc-patches" > Cc: "Paul Richard Thomas" > Betreff: [PATCH] PR fortran/90903 [part2] Add runtime checking for the MVBITS > intrinsic > > Dear all, > > finally here comes the second part of runtime checks for the bit > manipulation intrinsics, this time MVBITS. This turned out to be > more elaborate than the treatment of simple function calls. > > I chose the path to inline expand MVBITS, which enables additional > optimization opportunities in some cases, such as constant arguments. > For the case of scalar arguments, this was mostly straightforward. > However, for the proper handling of MVBITS as an elemental procedure > all honors should go to Paul, as he not only lend me a hand and kindly > guided me through the swampland of the scalarizer, but he also managed > to placate the gimple part of gcc. > > Regtested on x86_64-pc-linux-gnu. > > OK for master? > > Thanks, > Harald > > > PR fortran/90903 [part2] - Add runtime checking for the MVBITS intrinsic > > Implement inline expansion of the intrinsic elemental subroutine MVBITS > with optional runtime checks for valid argument range. > > gcc/fortran/ChangeLog: > > * iresolve.c (gfc_resolve_mvbits): Remove unneeded conversion of > FROMPOS, LEN and TOPOS arguments to fit a C int. > * trans-intrinsic.c (gfc_conv_intrinsic_mvbits): Add inline > expansion of MVBITS intrinsic elemental subroutine and add code > for runtime argument checking. > (gfc_conv_intrinsic_subroutine): Recognise MVBITS intrinsic, but > defer handling to gfc_trans_call. > * trans-stmt.c (replace_ss): > (gfc_trans_call): Adjust to handle inline expansion, scalarization > of intrinsic subroutine MVBITS in gfc_conv_intrinsic_mvbits. > * trans.h (gfc_conv_intrinsic_mvbits): Add prototype for > gfc_conv_intrinsic_mvbits. > > gcc/testsuite/ChangeLog: > > * gfortran.dg/check_bits_2.f90: New test. > > Co-authored-by: Paul Thomas > >
[PATCH] PR fortran/97272 - Wrong answer from MAXLOC with character arg
The generation of the library call for the MINLOC/MAXLOC intrinsic mishandled the optional KIND argument and resulted in a bad argument list passed to the library function. The fix is obvious. Regtested on x86_64-pc-linux-gnu. OK for master? As it technically wrong code, OK for backports? Thanks, Harald PR fortran/97272 - Wrong answer from MAXLOC with character arg The optional KIND argument to the MINLOC/MAXLOC intrinsic must not be passed to the library function, as the kind conversion of the result is treated explicitly elsewhere. gcc/fortran/ChangeLog: PR fortran/97272 * trans-intrinsic.c (gfc_conv_intrinsic_minmaxloc): Ignore KIND argument here, as it is treated elsewhere. gcc/testsuite/ChangeLog: PR fortran/97272 * gfortran.dg/pr97272.f90: New test. diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 3b3bd8629cd..9e9898c2bbf 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -5211,7 +5211,9 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op) while (a->next) { b = a->next; - if (b->expr == NULL || strcmp (b->name, "dim") == 0) + if (b->expr == NULL + || strcmp (b->name, "dim") == 0 + || strcmp (b->name, "kind") == 0) { a->next = b->next; b->next = NULL; diff --git a/gcc/testsuite/gfortran.dg/pr97272.f90 b/gcc/testsuite/gfortran.dg/pr97272.f90 new file mode 100644 index 000..e81903860ea --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr97272.f90 @@ -0,0 +1,19 @@ +! { dg-do run } +! PR fortran/97272 - Wrong answer from MAXLOC with character arg + +program test + implicit none + integer :: i, j, k, l = 10 + character, allocatable :: a(:) + allocate (a(l)) + a(:) = 'a' + l = l - 1 + a(l) = 'b' + i = maxloc (a, dim=1) + j = maxloc (a, dim=1, kind=2) + k = maxloc (a, dim=1, kind=8, back=.true.) +! print *, 'i = ', i, 'a(i) = ', a(i) +! print *, 'j = ', j, 'a(j) = ', a(j) +! print *, 'k = ', k, 'a(k) = ', a(k) + if (i /= l .or. j /= l .or. k /= l) stop 1 +end
Re: [PATCH] PR fortran/97272 - Wrong answer from MAXLOC with character arg
Slightly rewritten version of the patch, with the removal of the KIND argument from the argument list factored out: > The generation of the library call for the MINLOC/MAXLOC intrinsic > mishandled the optional KIND argument and resulted in a bad > argument list passed to the library function. The fix is obvious. > > Regtested on x86_64-pc-linux-gnu. > > OK for master? As it technically wrong code, OK for backports? Thanks, Harald PR fortran/97272 - Wrong answer from MAXLOC with character arg The optional KIND argument to the MINLOC/MAXLOC intrinsic must not be passed to the library function, as the kind conversion of the result is treated explicitly elsewhere. gcc/fortran/ChangeLog: PR fortran/97272 * trans-intrinsic.c (strip_kind_from_actual): Helper function for removal of KIND argument. (gfc_conv_intrinsic_minmaxloc): Ignore KIND argument here, as it is treated elsewhere. gcc/testsuite/ChangeLog: PR fortran/97272 * gfortran.dg/pr97272.f90: New test. diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index 3b3bd8629cd..8729bc12152 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -5073,6 +5073,24 @@ gfc_conv_intrinsic_dot_product (gfc_se * se, gfc_expr * expr) } +/* Remove unneeded kind= argument from actual argument list when the + result conversion is dealt with in a different place. */ + +static void +strip_kind_from_actual (gfc_actual_arglist * actual) +{ + for (gfc_actual_arglist *a = actual; a; a = a->next) +{ + gfc_actual_arglist *b = a->next; + if (b && b->name && strcmp (b->name, "kind") == 0) + { + a->next = b->next; + b->next = NULL; + gfc_free_actual_arglist (b); + } +} +} + /* Emit code for minloc or maxloc intrinsic. There are many different cases we need to handle. For performance reasons we sometimes create two loops instead of one, where the second one is much simpler. @@ -5208,6 +5226,7 @@ gfc_conv_intrinsic_minmaxloc (gfc_se * se, gfc_expr * expr, enum tree_code op) { gfc_actual_arglist *a, *b; a = actual; + strip_kind_from_actual (a); while (a->next) { b = a->next; diff --git a/gcc/testsuite/gfortran.dg/pr97272.f90 b/gcc/testsuite/gfortran.dg/pr97272.f90 new file mode 100644 index 000..e81903860ea --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr97272.f90 @@ -0,0 +1,19 @@ +! { dg-do run } +! PR fortran/97272 - Wrong answer from MAXLOC with character arg + +program test + implicit none + integer :: i, j, k, l = 10 + character, allocatable :: a(:) + allocate (a(l)) + a(:) = 'a' + l = l - 1 + a(l) = 'b' + i = maxloc (a, dim=1) + j = maxloc (a, dim=1, kind=2) + k = maxloc (a, dim=1, kind=8, back=.true.) +! print *, 'i = ', i, 'a(i) = ', a(i) +! print *, 'j = ', j, 'a(j) = ', a(j) +! print *, 'k = ', k, 'a(k) = ', a(k) + if (i /= l .or. j /= l .or. k /= l) stop 1 +end
Re: [PATCH] PR fortran/97272 - Wrong answer from MAXLOC with character arg
Hi FX, > While this is fresh in your memory, could I suggest you have a look at this > FINDLOC issue, which seems possibly related: > https://gcc.gnu.org/pipermail/fortran/2020-September/055016.html > and further messages from Thomas Koenig? I briefly checked this, but the issue with FINDLOC seems to have a different origin than MINLOC/MAXLOC. Let's see what Thomas makes of it. There are also PRs with ICEs when the KIND argument is present, e.g. PR87711 with len_trim. The non-uniformness of the internal treatment of intrinsics w.r.t. to this optional argument within gfortran is a bit frustrating. Cheers, Harald
[PATCH] PR fortran/95979 - [10/11 Regression] ICE in get_kind, at fortran/simplify.c:129
Dear all, the present PR turned out to be fixable rather easily, once Paul had the idea to add another attempt of simplification of elemental intrinsics for array-valued arguments. There was some fallout which required only small adjustments, see commit message below. Regtested cleanly on x86_64-pc-linux-gnu. OK for master / 10-branch? Thanks, Harald PR fortran/95979 - ICE in get_kind, at fortran/simplify.c:129 Simplification of the elemental intrinsic INDEX with constant array-valued arguments failed with an ICE or did not reduce to a constant array, depending also on the presence of the optional KIND argument. Add a further attempt of simplification in the case of elemental intrinsics, and make sure the KIND argument is not removed prematurely during simplification of INDEX. gcc/fortran/ChangeLog: PR fortran/95979 * expr.c (gfc_check_init_expr): Fix check of return code from gfc_intrinsic_func_interface. * intrinsic.c (gfc_intrinsic_func_interface): Add further attempt of simplification of elemental intrinsics with array arguments. * iresolve.c (gfc_resolve_index_func): Keep optional KIND argument for simplification of elemental use of INDEX. gcc/testsuite/ChangeLog: PR fortran/95979 * gfortran.dg/index_4.f90: New test. diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c index b87ae3d72a1..32d905ad179 100644 --- a/gcc/fortran/expr.c +++ b/gcc/fortran/expr.c @@ -2904,7 +2904,7 @@ gfc_check_init_expr (gfc_expr *e) && (e->value.function.isym->conversion == 1); if (!conversion && (!gfc_is_intrinsic (sym, 0, e->where) - || (m = gfc_intrinsic_func_interface (e, 0)) != MATCH_YES)) + || (m = gfc_intrinsic_func_interface (e, 0)) == MATCH_NO)) { gfc_error ("Function %qs in initialization expression at %L " "must be an intrinsic function", diff --git a/gcc/fortran/intrinsic.c b/gcc/fortran/intrinsic.c index ef33587a774..f4dfcf77e0b 100644 --- a/gcc/fortran/intrinsic.c +++ b/gcc/fortran/intrinsic.c @@ -5038,6 +5038,11 @@ got_specific: if (!sym->module) gfc_intrinsic_symbol (sym); + /* Have another stab at simplification since elemental intrinsics with array + actual arguments would be missed by the calls above to do_simplify. */ + if (isym->elemental) +gfc_simplify_expr (expr, 1); + return MATCH_YES; } diff --git a/gcc/fortran/iresolve.c b/gcc/fortran/iresolve.c index c2a4865f28f..994a9af4eb8 100644 --- a/gcc/fortran/iresolve.c +++ b/gcc/fortran/iresolve.c @@ -1296,11 +1296,7 @@ gfc_resolve_index_func (gfc_expr *f, gfc_actual_arglist *a) f->ts.type = BT_INTEGER; if (kind) -{ - f->ts.kind = mpz_get_si ((kind)->value.integer); - a_back->next = NULL; - gfc_free_actual_arglist (a_kind); -} +f->ts.kind = mpz_get_si ((kind)->value.integer); else f->ts.kind = gfc_default_integer_kind; diff --git a/gcc/testsuite/gfortran.dg/index_4.f90 b/gcc/testsuite/gfortran.dg/index_4.f90 new file mode 100644 index 000..09093784c8c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/index_4.f90 @@ -0,0 +1,19 @@ +! { dg-do run } +! { dg-options "-fdump-tree-original" } +! { dg-final { scan-tree-dump-times "string_index" 0 "original" } } +! PR fortran/95979 + +program p + implicit none + integer, parameter :: i0= index( 'abcd', 'b' , .true. , kind=4) + integer, parameter :: i1(*) = index(['abcd'], 'b' , .true. , kind=4) + integer, parameter :: i2(*) = index( 'abcd' ,['b'], .true. ) + integer, parameter :: i3(*) = index( 'abcd' , 'b' ,[.true.]) + integer, parameter :: i4(*) = index(['abcd'],['b'],[.true.], kind=8) + if (size (i1) /= 1) stop 1 + if (size (i2) /= 1) stop 2 + if (size (i3) /= 1) stop 3 + if (size (i4) /= 1) stop 4 + if (i0 /= 2)stop 5 + if (i1(1) /= 2 .or. i2(1) /= 2 .or. i3(1) /= 2 .or. i4(1) /= 2) stop 6 +end
[PATCH] PR libfortran/97063 - Wrong result for vector (step size is negative) * matrix
PR libfortran/97063 - Wrong result for vector (step size is negative) * matrix Dear all, when matrix-multiplying rank-1 times rank-2 arrays, a wrong result was produced when a negative stride was used for the rank-1 array. In that case special code for rank-2 times rank-2 was erroneously executed. We should never have gotten there, so move the check for rank-1 of the first argument before that case. The patch looks horrendously large because it consists essentially of regenerated code (nearly 99%). Regtests cleanly on x86_64-pc-linux-gnu. OK for master? And backport to all open branches where it applies? Thanks, Harald The MATMUL intrinsic provided a wrong result for rank-1 times rank-2 array when a negative stride was used for addressing the elements of the rank-1 array, because a check on strides was erroneously placed before the check on the rank. Interchange order of checks. libgfortran/ChangeLog: * m4/matmul_internal.m4: Move check for rank-1 times rank-2 before checks on strides for rank-2 times rank-2. * generated/matmul_c10.c: Regenerated. * generated/matmul_c16.c: Likewise. * generated/matmul_c4.c: Likewise. * generated/matmul_c8.c: Likewise. * generated/matmul_i1.c: Likewise. * generated/matmul_i16.c: Likewise. * generated/matmul_i2.c: Likewise. * generated/matmul_i4.c: Likewise. * generated/matmul_i8.c: Likewise. * generated/matmul_r10.c: Likewise. * generated/matmul_r16.c: Likewise. * generated/matmul_r4.c: Likewise. * generated/matmul_r8.c: Likewise. * generated/matmulavx128_c10.c: Likewise. * generated/matmulavx128_c16.c: Likewise. * generated/matmulavx128_c4.c: Likewise. * generated/matmulavx128_c8.c: Likewise. * generated/matmulavx128_i1.c: Likewise. * generated/matmulavx128_i16.c: Likewise. * generated/matmulavx128_i2.c: Likewise. * generated/matmulavx128_i4.c: Likewise. * generated/matmulavx128_i8.c: Likewise. * generated/matmulavx128_r10.c: Likewise. * generated/matmulavx128_r16.c: Likewise. * generated/matmulavx128_r4.c: Likewise. * generated/matmulavx128_r8.c: Likewise. gcc/testsuite/ChangeLog: * gfortran.dg/matmul_20.f90: New test. diff --git a/gcc/testsuite/gfortran.dg/matmul_20.f90 b/gcc/testsuite/gfortran.dg/matmul_20.f90 new file mode 100644 index 000..7a211a4974d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/matmul_20.f90 @@ -0,0 +1,47 @@ +! { dg-do run } +! PR97063 - Wrong result for vector (step size is negative) * matrix + +program p + implicit none + integer, parameter :: m = 3, k = 2*m, l = k-1, n = 4 + integer :: i, j, m1, m2, ms + integer :: ai(k), bi(k,n), ci(n), ci_ref(n), c1, c2 + real:: ar(k), br(k,n), cr(n), cr_ref(n) + + ai(:) = [(i,i=0,k-1)] + bi(:,:) = reshape ([(((5*i+j),i=0,k-1),j=0,n-1)],[k,n]) + + ! Parameters of subscript triplet + m1 = 1; m2 = l; ms = 2 + + ! Reference values for cross-checks: integer variant + c1 = dot_product (ai(m1:m2: ms), bi(m1:m2: ms,1)) + c2 = dot_product (ai(m1:m2: ms), bi(m1:m2: ms,2)) + ci_ref = matmul (ai(m1:m2: ms), bi(m1:m2: ms,:)) + ci = matmul (ai(m2:m1:-ms), bi(m2:m1:-ms,:)) + + if (ci_ref(1) /= c1 .or. ci_ref(2) /= c2) stop 1 + if (any (ci /= ci_ref)) stop 2 + + ! Real variant + ar = real (ai) + br = real (bi) + cr_ref = matmul (ar(m1:m2: ms), br(m1:m2: ms,:)) + cr = matmul (ar(m2:m1:-ms), br(m2:m1:-ms,:)) + + if (any (cr_ref /= real (ci_ref))) stop 3 + if (any (cr /= cr_ref )) stop 4 + + ! Mixed variants + cr_ref = matmul (ar(m1:m2: ms), bi(m1:m2: ms,:)) + cr = matmul (ar(m2:m1:-ms), bi(m2:m1:-ms,:)) + + if (any (cr_ref /= real (ci_ref))) stop 5 + if (any (cr /= cr_ref )) stop 6 + + cr_ref = matmul (ai(m1:m2: ms), br(m1:m2: ms,:)) + cr = matmul (ai(m2:m1:-ms), br(m2:m1:-ms,:)) + + if (any (cr_ref /= real (ci_ref))) stop 7 + if (any (cr /= cr_ref )) stop 8 +end program diff --git a/libgfortran/generated/matmul_c10.c b/libgfortran/generated/matmul_c10.c index ce5be246ddb..5bfd61d97ce 100644 --- a/libgfortran/generated/matmul_c10.c +++ b/libgfortran/generated/matmul_c10.c @@ -590,20 +590,6 @@ matmul_c10_avx (gfc_array_c10 * const restrict retarray, } } } - else if (axstride < aystride) -{ - for (y = 0; y < ycount; y++) - for (x = 0; x < xcount; x++) - dest[x*rxstride + y*rystride] = (GFC_COMPLEX_10)0; - - for (y = 0; y < ycount; y++) - for (n = 0; n < count; n++) - for (x = 0; x < xcount; x++) - /* dest[x,y] += a[x,n] * b[n,y] */ - dest[x*rxstride + y*rystride] += - abase[x*axstride + n*aystride] * - bbase[n*bxstride + y*bystride]; -} else if (GFC_DESCRIPTOR_RANK (a) == 1) { const GFC_COMPLEX_10 *restrict bbase_y; @@ -618,6 +604,20 @@ matmul_c10_avx (gfc_array_c10 * const restrict retarray, dest[y*rxstride] = s;
[Patch, RFC] PR81651/Fortran - Enhancement request: have f951 print out fully qualified module file name
Dear all, the attached patch prints the fully qualified path if an error occurs during module read. E.g., instead of a less helpful error message, pr81651.f90:2:6: 2 | use netcdf | 1 Fatal Error: File 'netcdf.mod' opened at (1) is not a GNU Fortran module file gfortran will print pr81651.f90:2:7: 2 | use netcdf | 1 Fatal Error: File '/opt/pgi/pkg/netcdf/include/netcdf.mod' opened at (1) is not a GNU Fortran module file Regtested on x86_64-pc-linux-gnu. I couldn't think of a sensible test for the testsuite, thus no testcase provided. OK for trunk? Thanks, Harald 2019-11-11 Harald Anlauf PR fortran/81651 * module.c (gzopen_included_file, gzopen_included_file_1) (gzopen_intrinsic_module, bad_module, gfc_use_module): Use fully qualified module path for error reporting. Index: gcc/fortran/module.c === --- gcc/fortran/module.c(revision 278064) +++ gcc/fortran/module.c(working copy) @@ -187,6 +187,8 @@ /* The gzFile for the module we're reading or writing. */ static gzFile module_fp; +/* Fully qualified module path */ +static char *module_fullpath = NULL; /* The name of the module we're reading (USE'ing) or writing. */ static const char *module_name; @@ -1101,6 +1103,8 @@ if (gfc_cpp_makedep ()) gfc_cpp_add_dep (fullname, system); +free (module_fullpath); +module_fullpath = xstrdup (fullname); return f; } } @@ -1116,8 +1120,14 @@ if (IS_ABSOLUTE_PATH (name) || include_cwd) { f = gzopen (name, "r"); - if (f && gfc_cpp_makedep ()) - gfc_cpp_add_dep (name, false); + if (f) + { + if (gfc_cpp_makedep ()) + gfc_cpp_add_dep (name, false); + + free (module_fullpath); + module_fullpath = xstrdup (name); + } } if (!f) @@ -1134,8 +1144,14 @@ if (IS_ABSOLUTE_PATH (name)) { f = gzopen (name, "r"); - if (f && gfc_cpp_makedep ()) -gfc_cpp_add_dep (name, true); + if (f) + { + if (gfc_cpp_makedep ()) + gfc_cpp_add_dep (name, true); + + free (module_fullpath); + module_fullpath = xstrdup (name); + } } if (!f) @@ -1181,7 +1197,7 @@ { case IO_INPUT: gfc_fatal_error ("Reading module %qs at line %d column %d: %s", - module_name, module_line, module_column, msgid); + module_fullpath, module_line, module_column, msgid); break; case IO_OUTPUT: gfc_fatal_error ("Writing module %qs at line %d column %d: %s", @@ -7141,7 +7157,7 @@ if ((start == 1 && strcmp (atom_name, "GFORTRAN") != 0) || (start == 2 && strcmp (atom_name, " module") != 0)) gfc_fatal_error ("File %qs opened at %C is not a GNU Fortran" -" module file", filename); +" module file", module_fullpath); if (start == 3) { if (strcmp (atom_name, " version") != 0 @@ -7150,7 +7166,7 @@ || strcmp (atom_string, MOD_VERSION)) gfc_fatal_error ("Cannot read module file %qs opened at %C," " because it was created by a different" -" version of GNU Fortran", filename); +" version of GNU Fortran", module_fullpath); free (atom_string); }
Re: [Patch, RFC] PR81651/Fortran - Enhancement request: have f951 print out fully qualified module file name
On 11/11/19 23:37, Janne Blomqvist wrote: > On Mon, Nov 11, 2019 at 11:54 PM Harald Anlauf wrote: >> >> Dear all, >> >> the attached patch prints the fully qualified path if an error occurs >> during module read. E.g., instead of a less helpful error message, >> >> pr81651.f90:2:6: >> >> 2 | use netcdf >> | 1 >> Fatal Error: File 'netcdf.mod' opened at (1) is not a GNU Fortran module >> file >> >> gfortran will print >> >> pr81651.f90:2:7: >> >> 2 | use netcdf >> | 1 >> Fatal Error: File '/opt/pgi/pkg/netcdf/include/netcdf.mod' opened at (1) >> is not a GNU Fortran module file >> >> Regtested on x86_64-pc-linux-gnu. >> >> I couldn't think of a sensible test for the testsuite, thus no testcase >> provided. >> >> OK for trunk? >> >> Thanks, >> Harald >> >> 2019-11-11 Harald Anlauf >> >> PR fortran/81651 >> * module.c (gzopen_included_file, gzopen_included_file_1) >> (gzopen_intrinsic_module, bad_module, gfc_use_module): Use fully >> qualified module path for error reporting. > > Ok. > Committed as svn rev. 278105. Thanks for the review! Harald
[PATCH, v1] PR fortran/48958 - Add runtime diagnostics for SIZE intrinsic function
Dear all, here is a first version to check the status of ALLOCATABLE and POINTER arguments to the SIZE intrinsic at runtime. What it does not yet cover is situations like complex, allocatable :: z(:) print *, size (z% re) Feedback, such as comments for improvement, are welcome. As is, the patch regtests cleanly on x86_64-pc-linux-gnu. Thanks, Harald PR fortran/48958 - Add runtime diagnostics for SIZE intrinsic function Add code for runtime checking of status of ALLOCATABLE and POINTER arguments to the SIZE intrinsic when -fcheck=pointer is specified. gcc/fortran/ChangeLog: * trans-intrinsic.c (gfc_conv_intrinsic_size): Generate runtime checking code for status of argument. gcc/testsuite/ChangeLog: * gfortran.dg/pr48958.f90: New test. diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c index e0afc10d105..d17b623924c 100644 --- a/gcc/fortran/trans-intrinsic.c +++ b/gcc/fortran/trans-intrinsic.c @@ -7929,6 +7929,35 @@ gfc_conv_intrinsic_size (gfc_se * se, gfc_expr * expr) && strcmp (e->ref->u.c.component->name, "_data") == 0) sym = e->symtree->n.sym; + if ((gfc_option.rtcheck & GFC_RTCHECK_POINTER) + && e + && (e->expr_type == EXPR_VARIABLE || e->expr_type == EXPR_FUNCTION)) +{ + symbol_attribute attr; + char *msg; + + attr = gfc_expr_attr (e); + if (attr.allocatable) + msg = xasprintf ("Allocatable argument '%s' is not allocated", + e->symtree->n.sym->name); + else if (attr.pointer) + msg = xasprintf ("Pointer argument '%s' is not associated", + e->symtree->n.sym->name); + else + goto end_arg_check; + + argse.descriptor_only = 1; + gfc_conv_expr_descriptor (&argse, actual->expr); + tree temp = gfc_conv_descriptor_data_get (argse.expr); + tree cond = fold_build2_loc (input_location, EQ_EXPR, + logical_type_node, temp, + fold_convert (TREE_TYPE (temp), + null_pointer_node)); + gfc_trans_runtime_check (true, false, cond, &argse.pre, &e->where, msg); + free (msg); +} + end_arg_check: + argse.data_not_needed = 1; if (gfc_is_class_array_function (e)) { diff --git a/gcc/testsuite/gfortran.dg/pr48958.f90 b/gcc/testsuite/gfortran.dg/pr48958.f90 new file mode 100644 index 000..2b109374f40 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr48958.f90 @@ -0,0 +1,25 @@ +! { dg-do run } +! { dg-options "-fcheck=pointer -fdump-tree-original" } +! { dg-shouldfail "Fortran runtime error: Allocatable argument 'a' is not allocated" } +! { dg-output "At line 13 .*" } +! PR48958 - Add runtime diagnostics for SIZE intrinsic function + +program p + integer :: n + integer, allocatable :: a(:) + integer, pointer :: b(:) + class(*), allocatable :: c(:) + integer :: d(10) + print *, size (a) + print *, size (b) + print *, size (c) + print *, size (d) + print *, size (f(n)) +contains + function f (n) +integer, intent(in) :: n +real, allocatable :: f(:) + end function f +end + +! { dg-final { scan-tree-dump-times "_gfortran_runtime_error_at" 4 "original" } }
Re: [PATCH, v1] PR fortran/48958 - Add runtime diagnostics for SIZE intrinsic function
Hi Thomas, thanks for the comments. > It feels a bit strange to have a check for an allocatable > behind -fcheck=pointer, but I'm not sure that introducing > a special check option would really be worth it. Yes, I thought about that. There's already a discrepancy between the GFC_RTCHECK_* in libgfortran.h, which has #define GFC_RTCHECK_DO (1<<3) #define GFC_RTCHECK_POINTER (1<<4) #define GFC_RTCHECK_MEM (1<<5) without the latter being documented, and gfortran.texi, which has [...] GFC_RTCHECK_DO (16), GFC_RTCHECK_POINTER (32), [...] -fcheck=mem should be checking memory allocations for things other than allocate (according to invoke.texi), like temporaries. -fcheck=pointer says: Enable generation of run-time checks for pointers and allocatables. So that is at least consistent. (Note to self: submit patch for obvious documentation fixes). > Regarding pointers: They are usually not nullified (unless > they are global). What do people think about adding a > NULL initializer to their data components when -fcheck=pointer is in > force? I was surprised when looking at the dump-tree difference of integer, pointer :: a(:) vs. integer, pointer :: a(:) => NULL() The latter has the expected: static struct array01_integer(kind=4) a = {.data=0B}; while the former has: struct array01_integer(kind=4) a; a.span = 0; Why setting .span at all, and not setting a.data = 0B ? > Regarding size(x%re): You would have to chase the refs to find > the allocatable components. I was probably too tired and drifted off from the subject. This is a more generic issue and not related to size(). > However, I think your patch already makes the situation better for > the user, so I'd say it is good for trunk already; later improvements > are always possible. Yes. Will commit soon. Thanks again, Harald
[Patch, Fortran] PR92898 - [9/10 Regression] ICE in gfc_check_is_contiguous, at fortran/check.c:7157
The testcase in the PR exhibits a corner case in a check on invalid code that was not handled appropriately and in turn ICEs. The patch below enhances that check. Instead of adding a new testcase, I modified the related one that came with the 'introduction' of the regression when fixing PR91641. Regtested on x86_64-pc-linux-gnu. OK for trunk and 9 ? Thanks, Harald 2019-12-10 Harald Anlauf PR fortran/92898 * check.c (gfc_check_is_contiguous): Adjust check to handle NULL() argument without an actual argument. 2019-12-10 Harald Anlauf PR fortran/92898 * gfortran.dg/pr91641.f90: Extend to check fix for PR92898. Index: gcc/fortran/check.c === --- gcc/fortran/check.c (Revision 279183) +++ gcc/fortran/check.c (Arbeitskopie) @@ -7154,7 +7154,9 @@ bool gfc_check_is_contiguous (gfc_expr *array) { if (array->expr_type == EXPR_NULL - && array->symtree->n.sym->attr.pointer == 1) + && (!array->symtree || + (array->symtree->n.sym && + array->symtree->n.sym->attr.pointer == 1))) { gfc_error ("Actual argument at %L of %qs intrinsic shall be an " "associated pointer", &array->where, gfc_current_intrinsic); Index: gcc/testsuite/gfortran.dg/pr91641.f90 === --- gcc/testsuite/gfortran.dg/pr91641.f90 (Revision 279183) +++ gcc/testsuite/gfortran.dg/pr91641.f90 (Arbeitskopie) @@ -1,7 +1,9 @@ ! { dg-do compile } ! PR fortran/91641 -! Code conyributed by Gerhard Steinmetz +! PR fortran/92898 +! Code contributed by Gerhard Steinmetz program p real, pointer :: z(:) print *, is_contiguous (null(z))! { dg-error "shall be an associated" } + print *, is_contiguous (null()) ! { dg-error "shall be an associated" } end
Aw: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in gfc_check_is_contiguous, at fortran/check.c:7157
Hi Thomas, > Gesendet: Dienstag, 10. Dezember 2019 um 23:34 Uhr > Von: "Thomas Koenig" > An: "Harald Anlauf" , gfortran , > gcc-patches > Betreff: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in > gfc_check_is_contiguous, at fortran/check.c:7157 > > Hello Harald, > > > Index: gcc/fortran/check.c > > === > > --- gcc/fortran/check.c (Revision 279183) > > +++ gcc/fortran/check.c (Arbeitskopie) > > @@ -7154,7 +7154,9 @@ bool > > gfc_check_is_contiguous (gfc_expr *array) > > { > > if (array->expr_type == EXPR_NULL > > - && array->symtree->n.sym->attr.pointer == 1) > > + && (!array->symtree || > > + (array->symtree->n.sym && > > + array->symtree->n.sym->attr.pointer == 1))) > > I have to admit I do not understand the original code here, nor > do I quite understand your fix. > > Is there any circumstance where array->expr_type == EXPR_NULL, but > is_contiguous is valid? What would go wrong if the other tests > were removed? Actually I do not know what the additional check was supposed to do. Removing it does not seem to do any harm. See below. > > > Index: gcc/testsuite/gfortran.dg/pr91641.f90 > > === > > --- gcc/testsuite/gfortran.dg/pr91641.f90 (Revision 279183) > > +++ gcc/testsuite/gfortran.dg/pr91641.f90 (Arbeitskopie) > > @@ -1,7 +1,9 @@ > > ! { dg-do compile } > > ! PR fortran/91641 > > -! Code conyributed by Gerhard Steinmetz > > +! PR fortran/92898 > > +! Code contributed by Gerhard Steinmetz > > program p > > real, pointer :: z(:) > > print *, is_contiguous (null(z))! { dg-error "shall be an > > associated" } > > + print *, is_contiguous (null()) ! { dg-error "shall be an > > associated" } > > end > > Sometimes, it is necessary to change test cases, when error messages > change. If this is not the case, it is better to add new tests to > new test cases - this makes regression hunting much easier. > > Regards > > Thomas Agreed. Please find the modified patches below. OK for trunk / 9 ? Thanks, Harald Index: gcc/fortran/check.c === --- gcc/fortran/check.c (Revision 279254) +++ gcc/fortran/check.c (Arbeitskopie) @@ -7153,8 +7153,7 @@ gfc_check_ttynam_sub (gfc_expr *unit, gfc_expr *na bool gfc_check_is_contiguous (gfc_expr *array) { - if (array->expr_type == EXPR_NULL - && array->symtree->n.sym->attr.pointer == 1) + if (array->expr_type == EXPR_NULL) { gfc_error ("Actual argument at %L of %qs intrinsic shall be an " "associated pointer", &array->where, gfc_current_intrinsic); Index: gcc/testsuite/gfortran.dg/pr92898.f90 =========== --- gcc/testsuite/gfortran.dg/pr92898.f90 (nicht existent) +++ gcc/testsuite/gfortran.dg/pr92898.f90 (Arbeitskopie) @@ -0,0 +1,6 @@ +! { dg-do compile } +! PR fortran/92898 +! Code contributed by Gerhard Steinmetz +program p + print *, is_contiguous (null()) ! { dg-error "shall be an associated" } +end 2019-12-11 Harald Anlauf PR fortran/92898 * check.c (gfc_check_is_contiguous): Simplify check to handle arbitrary NULL() argument. 2019-12-11 Harald Anlauf PR fortran/92898 * gfortran.dg/pr92898.f90: New test.
Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in gfc_check_is_contiguous, at fortran/check.c:7157
Hi Tobias, > Gesendet: Donnerstag, 12. Dezember 2019 um 09:01 Uhr > Von: "Tobias Burnus" > An: "Harald Anlauf" , "Thomas Koenig" > Cc: gfortran , gcc-patches > Betreff: Re: Aw: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in > gfc_check_is_contiguous, at fortran/check.c:7157 > > Hi Harald, > > let's add a LGTM or OK to this – the patch is rather obvious and Steve > explained how the now-removed check ended up in gfortran. thanks for the clarification, and thanks for the quick review. > Thanks for the patch! Committed as svn rev.279314 (trunk) and 279315 (9-branch). Harald > Tobias > > On 12/11/19 11:24 PM, Harald Anlauf wrote: > > Hi Thomas, > > > >> Gesendet: Dienstag, 10. Dezember 2019 um 23:34 Uhr > >> Von: "Thomas Koenig" > >> An: "Harald Anlauf" , gfortran , > >> gcc-patches > >> Betreff: Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in > >> gfc_check_is_contiguous, at fortran/check.c:7157 > >> > >> Hello Harald, > >> > >>> Index: gcc/fortran/check.c > >>> === > >>> --- gcc/fortran/check.c (Revision 279183) > >>> +++ gcc/fortran/check.c (Arbeitskopie) > >>> @@ -7154,7 +7154,9 @@ bool > >>>gfc_check_is_contiguous (gfc_expr *array) > >>>{ > >>> if (array->expr_type == EXPR_NULL > >>> - && array->symtree->n.sym->attr.pointer == 1) > >>> + && (!array->symtree || > >>> + (array->symtree->n.sym && > >>> +array->symtree->n.sym->attr.pointer == 1))) > >> I have to admit I do not understand the original code here, nor > >> do I quite understand your fix. > >> > >> Is there any circumstance where array->expr_type == EXPR_NULL, but > >> is_contiguous is valid? What would go wrong if the other tests > >> were removed? > > Actually I do not know what the additional check was supposed to do. > > Removing it does not seem to do any harm. See below. > > > >>> Index: gcc/testsuite/gfortran.dg/pr91641.f90 > >>> === > >>> --- gcc/testsuite/gfortran.dg/pr91641.f90 (Revision 279183) > >>> +++ gcc/testsuite/gfortran.dg/pr91641.f90 (Arbeitskopie) > >>> @@ -1,7 +1,9 @@ > >>>! { dg-do compile } > >>>! PR fortran/91641 > >>> -! Code conyributed by Gerhard Steinmetz > >>> +! PR fortran/92898 > >>> +! Code contributed by Gerhard Steinmetz > >>>program p > >>> real, pointer :: z(:) > >>> print *, is_contiguous (null(z))! { dg-error "shall be an > >>> associated" } > >>> + print *, is_contiguous (null()) ! { dg-error "shall be an > >>> associated" } > >>>end > >> Sometimes, it is necessary to change test cases, when error messages > >> change. If this is not the case, it is better to add new tests to > >> new test cases - this makes regression hunting much easier. > >> > >> Regards > >> > >>Thomas > > Agreed. Please find the modified patches below. OK for trunk / 9 ? > > > > Thanks, > > Harald > > > > Index: gcc/fortran/check.c > > === > > --- gcc/fortran/check.c (Revision 279254) > > +++ gcc/fortran/check.c (Arbeitskopie) > > @@ -7153,8 +7153,7 @@ gfc_check_ttynam_sub (gfc_expr *unit, gfc_expr *na > > bool > > gfc_check_is_contiguous (gfc_expr *array) > > { > > - if (array->expr_type == EXPR_NULL > > - && array->symtree->n.sym->attr.pointer == 1) > > + if (array->expr_type == EXPR_NULL) > > { > > gfc_error ("Actual argument at %L of %qs intrinsic shall be an " > > "associated pointer", &array->where, > > gfc_current_intrinsic); > > > > > > > > Index: gcc/testsuite/gfortran.dg/pr92898.f90 > > === > > --- gcc/testsuite/gfortran.dg/pr92898.f90 (nicht existent) > > +++ gcc/testsuite/gfortran.dg/pr92898.f90 (Arbeitskopie) > > @@ -0,0 +1,6 @@ > > +! { dg-do compile } > > +! PR fortran/92898 > > +! Code contributed by Gerhard Steinmetz > > +program p > > + print *, is_contiguous (null()) ! { dg-error "shall be an > > associated" } > > +end > > > > > > 2019-12-11 Harald Anlauf > > > > PR fortran/92898 > > * check.c (gfc_check_is_contiguous): Simplify check to handle > > arbitrary NULL() argument. > > > > 2019-12-11 Harald Anlauf > > > > PR fortran/92898 > > * gfortran.dg/pr92898.f90: New test. > > >
[Patch, fortran] PR70853 - ICE on pointing to null, in gfc_add_block_to_block, at fortran/trans.c:1599
The patch is self-explaining and practically obvious: pointer bounds remapping to NULL is not allowed, thus we shall reject it. I hope the error message is fine. If somebody prefers a formulation as in the standard ("data target", also used by the Intel compiler), please speak now. Regtested on x86_64-pc-linux-gnu. OK for trunk? Thanks, Harald Index: gcc/fortran/trans-expr.c === --- gcc/fortran/trans-expr.c(Revision 279405) +++ gcc/fortran/trans-expr.c(Arbeitskopie) @@ -9218,6 +9218,13 @@ gfc_trans_pointer_assignment (gfc_expr * expr1, gf break; rank_remap = (remap && remap->u.ar.end[0]); + if (remap && expr2->expr_type == EXPR_NULL) + { + gfc_error ("If bounds remapping is specified at %L, " +"the pointer target shall not be NULL", &expr1->where); + return NULL_TREE; + } + gfc_init_se (&lse, NULL); if (remap) lse.descriptor_only = 1; Index: gcc/testsuite/gfortran.dg/pr70853.f90 === --- gcc/testsuite/gfortran.dg/pr70853.f90 (nicht existent) +++ gcc/testsuite/gfortran.dg/pr70853.f90 (Arbeitskopie) @@ -0,0 +1,8 @@ +! { dg-do compile } +! PR fortran/70853 +! Contributed by Gerhard Steinmetz +program p + real, pointer :: z(:) + z(1:2) => null() ! { dg-error "pointer target shall not be NULL" } + z(2:1) => null() ! { dg-error "pointer target shall not be NULL" } +end 2019-12-18 Harald Anlauf PR fortran/92898 * trans-expr.c (gfc_trans_pointer_assignment): Reject bounds remapping if pointer target is NULL(). 2019-12-18 Harald Anlauf PR fortran/70853 * gfortran.dg/pr70853.f90: New test.
Re: [Patch, fortran] PR70853 - ICE on pointing to null, in gfc_add_block_to_block, at fortran/trans.c:1599
On 12/18/19 17:17, Tobias Burnus wrote: > LGTM. Thanks for the patch! Thanks, committed as r279527. > Tobias > > PS: I assume, your patch also fixes the following test case, which also > ICEs in gfc_trans_pointer_assignment: > integer, pointer, contiguous :: x(:) > nullify(x(1:1)) > end Well, that depends on your interpretation of "fix". The ICE is now replaced by a somewhat incorrect error message: x.f90:2:8: 2 | nullify(x(1:1)) |1 Error: If bounds remapping is specified at (1), the pointer target shall not be NULL For a better error message, we'd need to know that we come here from a NULLIFY statement. Can you file a PR? Thanks, Harald > On 12/18/19 5:07 PM, Harald Anlauf wrote: >> The patch is self-explaining and practically obvious: pointer bounds >> remapping to NULL is not allowed, thus we shall reject it. I hope the >> error message is fine. If somebody prefers a formulation as in the >> standard ("data target", also used by the Intel compiler), please >> speak now. >> >> Regtested on x86_64-pc-linux-gnu. >> >> OK for trunk? >> >> Thanks, >> Harald >> >> Index: gcc/fortran/trans-expr.c >> === >> --- gcc/fortran/trans-expr.c(Revision 279405) >> +++ gcc/fortran/trans-expr.c(Arbeitskopie) >> @@ -9218,6 +9218,13 @@ gfc_trans_pointer_assignment (gfc_expr * expr1, gf >>break; >> rank_remap = (remap && remap->u.ar.end[0]); >> >> + if (remap && expr2->expr_type == EXPR_NULL) >> + { >> + gfc_error ("If bounds remapping is specified at %L, " >> +"the pointer target shall not be NULL", >> &expr1->where); >> + return NULL_TREE; >> + } >> + >> gfc_init_se (&lse, NULL); >> if (remap) >> lse.descriptor_only = 1; >> >> >> Index: gcc/testsuite/gfortran.dg/pr70853.f90 >> === >> --- gcc/testsuite/gfortran.dg/pr70853.f90 (nicht existent) >> +++ gcc/testsuite/gfortran.dg/pr70853.f90 (Arbeitskopie) >> @@ -0,0 +1,8 @@ >> +! { dg-do compile } >> +! PR fortran/70853 >> +! Contributed by Gerhard Steinmetz >> +program p >> + real, pointer :: z(:) >> + z(1:2) => null() ! { dg-error "pointer target shall not be NULL" } >> + z(2:1) => null() ! { dg-error "pointer target shall not be NULL" } >> +end >> >> >> 2019-12-18 Harald Anlauf >> >> PR fortran/92898 >> * trans-expr.c (gfc_trans_pointer_assignment): Reject bounds >> remapping if pointer target is NULL(). >> >> >> 2019-12-18 Harald Anlauf >> >> PR fortran/70853 >> * gfortran.dg/pr70853.f90: New test. >
[Patch] PR92990 - fix error message for invalid argument of NULLIFY
The fix for PR70853 changed an ICE-on-invalid for NULLIFY into a misleading error message. The patch below rectifies that. OK for trunk? Regtested on x86_64-pc-linux-gnu. Thanks, Harald Index: gcc/fortran/match.c === --- gcc/fortran/match.c (Revision 279645) +++ gcc/fortran/match.c (Arbeitskopie) @@ -4588,6 +4588,23 @@ gfc_match_nullify (void) goto cleanup; } + /* Check for valid array pointer object. Bounds remapping is not +allowed with NULLIFY. */ + if (p->ref) + { + gfc_ref *remap = p->ref; + for (; remap; remap = remap->next) + if (!remap->next && remap->type == REF_ARRAY + && remap->u.ar.type != AR_FULL) + break; + if (remap) + { + gfc_error ("NULLIFY does not allow bounds remapping for " +"pointer object at %C"); + goto cleanup; + } + } + /* build ' => NULL() '. */ e = gfc_get_null_expr (&gfc_current_locus); Index: gcc/testsuite/gfortran.dg/pr92990.f90 === --- gcc/testsuite/gfortran.dg/pr92990.f90 (nicht existent) +++ gcc/testsuite/gfortran.dg/pr92990.f90 (Arbeitskopie) @@ -0,0 +1,12 @@ +! { dg-do compile } +! PR fortran/92990 +! Verify fix of error message for NULLIFY vs. pointer assignment (PR70853) +program p + integer, pointer :: x(:) + type t + integer, pointer :: y(:) + end type t + type(t) :: z + nullify (x(1:2)) ! { dg-error "does not allow bounds remapping" } + nullify (z%y(:)) ! { dg-error "does not allow bounds remapping" } +end 2019-12-20 Harald Anlauf PR fortran/92990 * match.c (gfc_match_nullify): Check for valid pointer object. Reject bounds remapping. 2019-12-20 Harald Anlauf PR fortran/92990 * gfortran.dg/pr92990.f90: New test.
Re: [Patch] PR92990 - fix error message for invalid argument of NULLIFY
> Gesendet: Samstag, 21. Dezember 2019 um 17:16 Uhr > Von: "Thomas Koenig" > An: "Harald Anlauf" , fortran , > gcc-patches > Betreff: Re: [Patch] PR92990 - fix error message for invalid argument of > NULLIFY > > Am 20.12.19 um 22:33 schrieb Harald Anlauf: > > The fix for PR70853 changed an ICE-on-invalid for NULLIFY into a > > misleading error message. The patch below rectifies that. > > > > OK for trunk? > > OK. > > Thanks for the patch! > > Regards > > Thomas > Committed as rev.279698. Thanks for the review! Harald
[Patch, commited] PR91661 - ICE in gfc_conv_intrinsic_dot_product, at fortran/trans-intrinsic.c:4804
A "side-effect" of Tobias Burnus' fix for PR92996, which addressed simplification of some array-valued constant expression, is that it fixes the ICE in the subject. To make sure that we do not regress I committed the testcase below as rev.279699. This was OKed in the PR by Thomas König. Thanks, Harald 2019-12-21 Harald Anlauf PR fortran/91661 * gfortran.dg/pr91661.f90: New test. Index: gcc/testsuite/gfortran.dg/pr91661.f90 === --- gcc/testsuite/gfortran.dg/pr91661.f90 (nicht existent) +++ gcc/testsuite/gfortran.dg/pr91661.f90 (Arbeitskopie) @@ -0,0 +1,13 @@ +! { dg-do run } +! PR fortran/91661 +! Contributed by Gerhard Steinmetz +! Verify that fix for PR92996 also fixes this one +program p + integer, parameter :: a(2)= 2 + integer, parameter :: b(a(1)) = 3 + integer, parameter :: c = dot_product(b, b) + integer, parameter :: d(a(1)+a(2)) = 3 + integer, parameter :: e = size (d,dim=1) + if (c /= 18) stop 1 ! This used to ICE + if (e /= 4) stop 2 ! This used to ICE +end
Re: [Patch, fortran PR89645/99065 No IMPLICIT type error with: ASSOCIATE( X => function() )
Hi Paul, welcome back! On 3/3/24 17:04, Paul Richard Thomas wrote: Hi Harald, Please find an updated version of the patch that rolls in Steve's patch for PR114141, fixes unlimited polymorphic function selectors and cures the memory leaks. I apologise for not working on this sooner but, as I informed you, I have been away for an extended trip to Australia. The chunks that fix PR114141 are picked out in comment 14 to the PR and the cures to the problems that you found in the first review are found at trans-stmt.cc:2047-49. Regtests fine. OK for trunk, bearing in mind that most of the patch is ring fenced by the inferred_type flag? I would say that it is almost fine. Two things that I found: - Testcase associate_65.f90 does not compile with -std=f2023, because IMAG is a GNU extension, while AIMAG is the standard version. Could you please adjust that? - I think the handling of parentheses and functions returning pointers does not work correctly. Consider: program paul implicit none type t integer :: i end type t type(t), pointer :: p(:) allocate (p(-3:3)) associate (q => p) print *, lbound (q), ubound (q) ! Should print -3 3 (OK) end associate associate (q => set_ptr()) print *, lbound (q), ubound (q) ! Should print -3 3 (OK) end associate associate (q => (p)) print *, lbound (q), ubound (q) ! Should print 1 7 (OK) end associate associate (q => (set_ptr())) ! <- are these parentheses lost? print *, lbound (q), ubound (q) ! Should print 1 7 end associate contains function set_ptr () result (res) type(t), pointer :: res(:) res => p end function set_ptr end While the first three variants give the right bounds, the last version - after applying your patch - is mishandled and the testcase now prints: -3 3 -3 3 1 7 -3 3 Both NAG and Intel support my expectation, namely that the last line should equal the next-to-last. Can you recheck the logic for that particular corner case? With these points addressed, your patch is OK from my side. Thanks for the patch and your endurance! Harald Cheers Paul On Mon, 8 Jan 2024 at 21:53, Harald Anlauf wrote: Hi Paul, your patch looks already very impressive! Regarding the patch as is, I am still trying to grok it, even with your explanations at hand... While the testcase works as advertised, I noticed that it exhibits a runtime memleak that occurs for (likely) each case where the associate target is an allocatable, class-valued function result. I tried to produce a minimal testcase using class(*), which apparently is not handled by your patch (it ICEs for me): program p implicit none class(*), allocatable :: x(:) x = foo() call prt (x) deallocate (x) ! up to here no memleak... associate (var => foo()) call prt (var) end associate contains function foo() result(res) class(*), allocatable :: res(:) res = [42] end function foo subroutine prt (x) class(*), intent(in) :: x(:) select type (x) type is (integer) print *, x class default stop 99 end select end subroutine prt end Traceback (truncated): foo.f90:9:18: 9 | call prt (var) | 1 internal compiler error: tree check: expected record_type or union_type or qual_union_type, have function_type in gfc_class_len_get, at fortran/trans-expr.cc:271 0x19fd5d5 tree_check_failed(tree_node const*, char const*, int, char const*, ...) ../../gcc-trunk/gcc/tree.cc:8952 0xe1562d tree_check3(tree_node*, char const*, int, char const*, tree_code, tree_code, tree_code) ../../gcc-trunk/gcc/tree.h:3652 0xe3e264 gfc_class_len_get(tree_node*) ../../gcc-trunk/gcc/fortran/trans-expr.cc:271 0xecda48 trans_associate_var ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2325 0xecdd09 gfc_trans_block_construct(gfc_code*) ../../gcc-trunk/gcc/fortran/trans-stmt.cc:2383 [...] I don't see anything wrong with it: NAG groks it, like Nvidia and Flang, while Intel crashes at runtime. Can you have another brief look? Thanks, Harald On 1/6/24 18:26, Paul Richard Thomas wrote: These PRs come about because of gfortran's single pass parsing. If the function in the title is parsed after the associate construct, then its type and rank are not known. The point at which this becomes a problem is when expressions within the associate block are parsed. primary.cc (gfc_match_varspec) could already deal with intrinsic types and so component references were the trigger for the problem. The two major parts of this patch are the fixup needed in gfc_match_varspec and the resolution of expressions with references in resolve.cc (gfc_fixup_inferred_type_refs). The former relies on the two new functions in symbol.cc to search for derived types with an appropri
[PATCH] Fortran: error recovery while simplifying expressions [PR103707,PR106987]
Dear all, error recovery on arithmetic errors during simplification has bugged me for a long time, especially since the occurence of ICEs depended on whether -frange-check is specified or not, whether array ctors were involved, etc. I've now come up with the attached patch that classifies the arithmetic result codes into "hard" and "soft" errors. A "soft" error means that it is an overflow or other exception (e.g. NaN) that is ignored with -fno-range-check. After the patch, a soft error will not stop simplification (a hard one will), and error status will be passed along. I took this opportunity to change the emitted error for division by zero for real and complex division dependent on whether the numerator is regular or not. This makes e.g. (0.)/0 a NaN and now says so, in accordance with some other brands. Regtested on x86_64-pc-linux-gnu. OK for mainline? Other comments? Thanks, Harald From d9b87bea6af77fbc794e1f21cfecb0468c68cb72 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 5 Mar 2024 21:54:26 +0100 Subject: [PATCH] Fortran: error recovery while simplifying expressions [PR103707,PR106987] When an exception is encountered during simplification of arithmetic expressions, the result may depend on whether range-checking is active (-frange-check) or not. However, the code path in the front-end should stay the same for "soft" errors for which the exception is triggered by the check, while "hard" errors should always terminate the simplification, so that error recovery is independent of the flag. Separation of arithmetic error codes into "hard" and "soft" errors shall be done consistently via is_hard_arith_error(). PR fortran/103707 PR fortran/106987 gcc/fortran/ChangeLog: * arith.cc (is_hard_arith_error): New helper function to determine whether an arithmetic error is "hard" or not. (check_result): Use it. (gfc_arith_divide): Set "Division by zero" only for regular numerators of real and complex divisions. (reduce_unary): Use is_hard_arith_error to determine whether a hard or (recoverable) soft error was encountered. Terminate immediately on hard error, otherwise remember code of first soft error. (reduce_binary_ac): Likewise. (reduce_binary_ca): Likewise. (reduce_binary_aa): Likewise. gcc/testsuite/ChangeLog: * gfortran.dg/pr99350.f90: * gfortran.dg/arithmetic_overflow_3.f90: New test. --- gcc/fortran/arith.cc | 134 -- .../gfortran.dg/arithmetic_overflow_3.f90 | 48 +++ gcc/testsuite/gfortran.dg/pr99350.f90 | 2 +- 3 files changed, 143 insertions(+), 41 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/arithmetic_overflow_3.f90 diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index d17d1aaa1d9..b373c25e5e1 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -130,6 +130,30 @@ gfc_arith_error (arith code) } +/* Check if a certain arithmetic error code is severe enough to prevent + further simplification, as opposed to errors thrown by the range check + (e.g. overflow) or arithmetic exceptions that are tolerated with + -fno-range-check. */ + +static bool +is_hard_arith_error (arith code) +{ + switch (code) +{ +case ARITH_OK: +case ARITH_OVERFLOW: +case ARITH_UNDERFLOW: +case ARITH_NAN: +case ARITH_DIV0: +case ARITH_ASYMMETRIC: + return false; + +default: + return true; +} +} + + /* Get things ready to do math. */ void @@ -579,10 +603,10 @@ check_result (arith rc, gfc_expr *x, gfc_expr *r, gfc_expr **rp) val = ARITH_OK; } - if (val == ARITH_OK || val == ARITH_OVERFLOW) -*rp = r; - else + if (is_hard_arith_error (val)) gfc_free_expr (r); + else +*rp = r; return val; } @@ -792,23 +816,26 @@ gfc_arith_divide (gfc_expr *op1, gfc_expr *op2, gfc_expr **resultp) break; case BT_REAL: - if (mpfr_sgn (op2->value.real) == 0 && flag_range_check == 1) - { - rc = ARITH_DIV0; - break; - } + /* Set "Division by zero" only for regular numerator. */ + if (flag_range_check == 1 + && mpfr_zero_p (op2->value.real) + && mpfr_regular_p (op1->value.real)) + rc = ARITH_DIV0; mpfr_div (result->value.real, op1->value.real, op2->value.real, GFC_RND_MODE); break; case BT_COMPLEX: - if (mpc_cmp_si_si (op2->value.complex, 0, 0) == 0 - && flag_range_check == 1) - { - rc = ARITH_DIV0; - break; - } + /* Set "Division by zero" only for regular numerator. */ + if (flag_range_check == 1 + && mpfr_zero_p (mpc_realref (op2->value.complex)) + && mpfr_zero_p (mpc_imagref (op2->value.complex)) + && ((mpfr_regular_p (mpc_realref (op1->value.complex)) + && mpfr_number_p (mpc_imagref (op1->value.complex
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (&dtp->common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, on further thought, do we sanitize 'child_iomsg'? We pass it to snprintf as format. Wouldn't a strncpy be sufficient? Harald On 3/5/24 22:37, Harald Anlauf wrote: Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (&dtp->common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
Re: [PATCH] Fortran: error recovery while simplifying expressions [PR103707, PR106987]
Hi Paul, thanks for reviewing the patch, and your trust in me :-) Backporting to 13-branch seems easily feasible (needs another small queued backport on which this patch depends), but going further is definitely out of the question... Will wait a couple of weeks though. Harald On 3/6/24 11:51, Paul Richard Thomas wrote: Hi Harald, This all looks good to me. OK for mainline and, according to intestinal fortitude on your part, earlier branches. Thanks Paul On Tue, 5 Mar 2024 at 21:24, Harald Anlauf wrote: Dear all, error recovery on arithmetic errors during simplification has bugged me for a long time, especially since the occurence of ICEs depended on whether -frange-check is specified or not, whether array ctors were involved, etc. I've now come up with the attached patch that classifies the arithmetic result codes into "hard" and "soft" errors. A "soft" error means that it is an overflow or other exception (e.g. NaN) that is ignored with -fno-range-check. After the patch, a soft error will not stop simplification (a hard one will), and error status will be passed along. I took this opportunity to change the emitted error for division by zero for real and complex division dependent on whether the numerator is regular or not. This makes e.g. (0.)/0 a NaN and now says so, in accordance with some other brands. Regtested on x86_64-pc-linux-gnu. OK for mainline? Other comments? Thanks, Harald
Re: [patch, libgfortran] Part 2: PR105456 Child I/O does not propage iostat
Hi Jerry, can you please replace the user message in e.g. your new testcase pr105456-wf.f90 by say: piomsg="The users message containing % and %% and %s and other stuff" This behaves as expected with Intel, but dies horribly with gfortran after your patch! Cheers, Harald On 3/6/24 05:06, Jerry D wrote: On 3/5/24 1:51 PM, Harald Anlauf wrote: Hi Jerry, on further thought, do we sanitize 'child_iomsg'? We pass it to snprintf as format. Wouldn't a strncpy be sufficient? Harald Just to be safe I will bump char message[IOMSG_LEN] to char message[IOMSG_LEN + 1] This is like a C string vs a Fortran string length situation. snprintf guarantees we don't exceed the child_iomsg_len and null terminates it. I added 1 to: child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1 Because snprintf was chopping off the last character of the fortran string to put the null in. (zero based vs one based char array). I test this with a very long string which exceeded the length and then reduced it until I could see the desired end. I have not tried running a test case with sanitize. I did check with valgrind. I will try the sanitize flags to see if we get a problem. If not will push. Thanks for comments, Jerry - On 3/5/24 22:37, Harald Anlauf wrote: Hi Jerry, I think there is the risk of buffer overrun in the following places: + char message[IOMSG_LEN]; + child_iomsg_len = string_len_trim (IOMSG_LEN, child_iomsg) + 1; free_line (dtp); snprintf (message, child_iomsg_len, child_iomsg); generate_error (&dtp->common, dtp->u.p.child_saved_iostat, plus several more. Wouldn't it be better to increase the size of message by one? Thanks, Harald On 3/5/24 04:15, Jerry D wrote: On 3/1/24 11:24 AM, rep.dot@gmail.com wrote: Hi Jerry and Steve, On 29 February 2024 19:28:19 CET, Jerry D wrote: On 2/29/24 10:13 AM, Steve Kargl wrote: On Thu, Feb 29, 2024 at 09:36:43AM -0800, Jerry D wrote: On 2/29/24 1:47 AM, Bernhard Reutner-Fischer wrote: And, just for my own education, the length limitation of iomsg to 255 chars is not backed by the standard AFAICS, right? It's just our STRERR_MAXSZ? Yes, its what we have had for a long lone time. Once you throw an error things get very processor dependent. I found MSGLEN set to 100 and IOMSG_len to 256. Nothing magic about it. There is no restriction on the length for the iomsg-variable that receives the generated error message. In fact, if the iomsg-variable has a deferred-length type parameter, then (re)-allocation to the exact length is expected. F2023 12.11.6 IOMSG= specifier If an error, end-of-file, or end-of-record condition occurs during execution of an input/output statement, iomsg-variable is assigned an explanatory message, as if by intrinsic assignment. If no such condition occurs, the definition status and value of iomsg-variable are unchanged. character(len=23) emsg read(fd,*,iomsg=emsg) Here, the generated iomsg is either truncated to a length of 23 or padded with blanks to a length of 23. character(len=:), allocatable :: emsg read(fd,*,iomsg=emsg) Here, emsg should have the length of whatever error message was generated. HTH Well, currently, if someone uses a larger string than 256 we are going to chop it off. Do we want to process this differently now? Yes. There is some odd hunk about discrepancy of passed len and actual len afterwards in 22-007-r1, IIRC. Didn't look closely though. --- snip --- Attached is the revised patch using the already available string_len_trim function. This hunk is only executed if a user has not passed an iostat or iomsg variable in the parent I/O statement and an error is triggered which terminates execution of the program. In this case, the iomsg string is provided in the usual error message in a "processor defined" way. (F2023): 12.6.4.8.3 Executing defined input/output data transfers --- 11 If the iostat argument of the defined input/output procedure has a nonzero value when that procedure returns, and the processor therefore terminates execution of the program as described in 12.11, the processor shall make the value of the iomsg argument available in a processor-dependent manner. --- OK for trunk? Regards, Jerry
[PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
Dear all, after playing for some time with NAG and Intel, and an off-list discussion with Jerry, I am getting more and more convinced that simpler runtime error messages (also simpler to parse by a human) are superior to awkward solutions. This is also what Intel does: use only the name of the array (component) in the message whose indices are out of bounds. (NAG's solution appears also inconsistent for nested derived types.) So no x%z, or x%_data, etc. in runtime error messages any more. Please give it a spin... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 1/30/24 11:46, Mikael Morin wrote: Le 30/01/2024 à 11:38, Mikael Morin a écrit : Another (easier) way to clarify the data reference would be rephrasing the message so that the array part is separate from the scalar part, like so (there are too many 'of', but I lack inspiration): Index '0' of dimension 1 of component 'zz' of element from 'x1%vv' below lower bound of 1 This has the same number of 'of' but sounds better maybe: Out of bounds accessing component 'zz' of element from 'x1%yy': index '0' of dimension 1 below lower bound of 1 From cdf3b197beed0ce1649661b2132643b54cbade8d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 10 Mar 2024 22:14:30 +0100 Subject: [PATCH] Fortran: use name of array component in runtime error message [PR30802] gcc/fortran/ChangeLog: PR fortran/30802 * trans-array.cc (trans_array_bound_check): Find name of component to use in runtime error message. (array_bound_check_elemental): Likewise. (gfc_conv_array_ref): Likewise. gcc/testsuite/ChangeLog: PR fortran/30802 * gfortran.dg/bounds_check_17.f90: Adjust dg-pattern. * gfortran.dg/bounds_check_fail_6.f90: Likewise. * gfortran.dg/pr92050.f90: Likewise. * gfortran.dg/bounds_check_fail_8.f90: New test. --- gcc/fortran/trans-array.cc| 60 +-- gcc/testsuite/gfortran.dg/bounds_check_17.f90 | 2 +- .../gfortran.dg/bounds_check_fail_6.f90 | 7 ++- .../gfortran.dg/bounds_check_fail_8.f90 | 48 +++ gcc/testsuite/gfortran.dg/pr92050.f90 | 2 +- 5 files changed, 83 insertions(+), 36 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..9c62b070c50 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3497,6 +3497,8 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, tree descriptor; char *msg; const char * name = NULL; + gfc_expr *expr; + gfc_ref *ref; if (!(gfc_option.rtcheck & GFC_RTCHECK_BOUNDS)) return index; @@ -3509,6 +3511,24 @@ trans_array_bound_check (gfc_se * se, gfc_ss *ss, tree index, int n, name = ss->info->expr->symtree->n.sym->name; gcc_assert (name != NULL); + /* When we have a component ref, get name of the array section. + Note that there can only be one part ref. */ + expr = ss->info->expr; + if (expr->ref && !compname) +{ + for (ref = expr->ref; ref; ref = ref->next) + { + /* Remember component name. */ + if (ref->type == REF_COMPONENT) + { + name = ref->u.c.component->name; + continue; + } + if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION) + break; + } +} + if (VAR_P (descriptor)) name = IDENTIFIER_POINTER (DECL_NAME (descriptor)); @@ -3574,29 +3594,20 @@ array_bound_check_elemental (gfc_se * se, gfc_ss * ss, gfc_expr * expr) gfc_array_ref *ar; gfc_ref *ref; gfc_symbol *sym; - char *var_name = NULL; - size_t len; + const char *var_name = NULL; int dim; if (expr->expr_type == EXPR_VARIABLE) { sym = expr->symtree->n.sym; - len = strlen (sym->name) + 1; - - for (ref = expr->ref; ref; ref = ref->next) - if (ref->type == REF_COMPONENT) - len += 2 + strlen (ref->u.c.component->name); - - var_name = XALLOCAVEC (char, len); - strcpy (var_name, sym->name); + var_name = sym->name; for (ref = expr->ref; ref; ref = ref->next) { - /* Append component name. */ + /* Get component name. */ if (ref->type == REF_COMPONENT) { - strcat (var_name, "%%"); - strcat (var_name, ref->u.c.component->name); + var_name = ref->u.c.component->name; continue; } @@ -4001,7 +4012,7 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr, gfc_se indexse; gfc_se tmpse; gfc_symbol * sym = expr->symtree->n.sym; - char *var_name = NULL; + const char *var_name = NULL; if (ar->dimen == 0) { @@ -4035,30 +4046,17 @@ gfc_conv_array_ref (gfc_se * se, gfc_array_ref * ar, gfc_expr *expr, if (gfc_option.rtcheck & GFC_RTCHECK_BOUN
[PATCH] Fortran: handle procedure pointer component in DT array [PR110826]
Dear all, the attached patch fixes an ICE-on-valid code when assigning a procedure pointer that is a component of a DT array and the function in question is array-valued. (The procedure pointer itself cannot be an array.) Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From a9be17cf987b796c49684cde2f20dac3839c736c Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 11 Mar 2024 22:05:51 +0100 Subject: [PATCH] Fortran: handle procedure pointer component in DT array [PR110826] gcc/fortran/ChangeLog: PR fortran/110826 * array.cc (gfc_array_dimen_size): When walking the ref chain of an array and the ultimate component is a procedure pointer, do not try to figure out its dimension even if it is a array-valued function. gcc/testsuite/ChangeLog: PR fortran/110826 * gfortran.dg/proc_ptr_comp_53.f90: New test. --- gcc/fortran/array.cc | 7 .../gfortran.dg/proc_ptr_comp_53.f90 | 41 +++ 2 files changed, 48 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc index 3a6e3a7c95b..e9934f1491b 100644 --- a/gcc/fortran/array.cc +++ b/gcc/fortran/array.cc @@ -2597,6 +2597,13 @@ gfc_array_dimen_size (gfc_expr *array, int dimen, mpz_t *result) case EXPR_FUNCTION: for (ref = array->ref; ref; ref = ref->next) { + /* Ultimate component is a procedure pointer. */ + if (ref->type == REF_COMPONENT + && !ref->next + && ref->u.c.component->attr.function + && IS_PROC_POINTER (ref->u.c.component)) + return false; + if (ref->type != REF_ARRAY) continue; diff --git a/gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 b/gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 new file mode 100644 index 000..881ddd3558f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/proc_ptr_comp_53.f90 @@ -0,0 +1,41 @@ +! { dg-do compile } +! PR fortran/110826 - procedure pointer component in DT array + +module m + implicit none + + type pp +procedure(func_template), pointer, nopass :: f =>null() + end type pp + + abstract interface + function func_template(state) result(dstate) + implicit none + real, dimension(:,:), intent(in) :: state + real, dimension(size(state,1), size(state,2)) :: dstate + end function + end interface + +contains + + function zero_state(state) result(dstate) +real, dimension(:,:), intent(in) :: state +real, dimension(size(state,1), size(state,2)) :: dstate +dstate = 0. + end function zero_state + +end module m + +program test_func_array + use m + implicit none + + real, dimension(4,6) :: state + type(pp) :: func_scalar + type(pp) :: func_array(4) + + func_scalar %f => zero_state + func_array(1)%f => zero_state + print *, func_scalar %f(state) + print *, func_array(1)%f(state) +end program test_func_array -- 2.35.3
Re: [Patch, fortran PR89645/99065 No IMPLICIT type error with: ASSOCIATE( X => function() )
Hi Paul, On 3/12/24 15:54, Paul Richard Thomas wrote: Hi All, This is the last posting of this patch before I push it. Harald is OK with it on the grounds that the inferred_type flag guards the whole lot, except for the chunks in trans-stmt.cc. In spite of Harald's off-list admonition not to try to fix everything at once, this version fixes most of the inquiry reference bugs (associate_68.f90) with the exception of character(kind=4) function selectors. The reason for this is that I have some housekeeping to do before release on finalization and then I want to replace this patch in 15-branch with two pass parsing. My first attempts at the latter were a partial success. you wouldn't stop trying to fix everything, would you? ;-) It regtests OK on x86_64. Unless there are objections, I will commit on Thursday evening. No objections, just one wish: could you improve the text of the following comments so that mere mortals understand them? diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 12e7bf3c873..0ab69bb9dce 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc [...] + /* If there is a usable inquiry reference not there are no matching +derived types, force the inquiry reference by setting unknown the +type of the primary expression. */ I have a hard time parsing the first part of that sentence. diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index 5d9852c79e0..16adb2a7efb 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc [...] +/* Find all derived types in the uppermost namespace that have a component + a component called name and stash them in the assoc field of an + associate name variable. "a component" too much? Thanks, Harald Cheers Paul
[PATCH] Fortran: fix IS_CONTIGUOUS for polymorphic dummy arguments [PR114001]
Dear all, here's another small fix: IS_CONTIGUOUS did erroneously always return .true. for CLASS dummy arguments. The solution was to adjust the logic in gfc_is_simply_contiguous to also handle CLASS symbols. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 8f535b19bd0cb6a7c99ac9ba4c07778f86698a1c Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 12 Mar 2024 22:58:39 +0100 Subject: [PATCH] Fortran: fix IS_CONTIGUOUS for polymorphic dummy arguments [PR114001] gcc/fortran/ChangeLog: PR fortran/114001 * expr.cc (gfc_is_simply_contiguous): Adjust logic so that CLASS symbols are also handled. gcc/testsuite/ChangeLog: PR fortran/114001 * gfortran.dg/is_contiguous_4.f90: New test. --- gcc/fortran/expr.cc | 19 ++--- gcc/testsuite/gfortran.dg/is_contiguous_4.f90 | 81 +++ 2 files changed, 91 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/is_contiguous_4.f90 diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 37ea95d0185..82a642b01f7 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -6025,15 +6025,16 @@ gfc_is_simply_contiguous (gfc_expr *expr, bool strict, bool permit_element) } sym = expr->symtree->n.sym; - if (expr->ts.type != BT_CLASS - && ((part_ref - && !part_ref->u.c.component->attr.contiguous - && part_ref->u.c.component->attr.pointer) - || (!part_ref - && !sym->attr.contiguous - && (sym->attr.pointer - || (sym->as && sym->as->type == AS_ASSUMED_RANK) - || (sym->as && sym->as->type == AS_ASSUMED_SHAPE) + if ((part_ref + && part_ref->u.c.component + && !part_ref->u.c.component->attr.contiguous + && IS_POINTER (part_ref->u.c.component)) + || (!part_ref + && expr->ts.type != BT_CLASS + && !sym->attr.contiguous + && (sym->attr.pointer + || (sym->as && sym->as->type == AS_ASSUMED_RANK) + || (sym->as && sym->as->type == AS_ASSUMED_SHAPE return false; if (!ar || ar->type == AR_FULL) diff --git a/gcc/testsuite/gfortran.dg/is_contiguous_4.f90 b/gcc/testsuite/gfortran.dg/is_contiguous_4.f90 new file mode 100644 index 000..cb066f8836b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/is_contiguous_4.f90 @@ -0,0 +1,81 @@ +! { dg-do run } +! PR fortran/114001 - IS_CONTIGUOUS and polymorphic dummy + +program main + implicit none + integer :: i, cnt = 0 + logical :: expect + integer, target :: m(10) = [(i,i=1,size(m))] + integer, pointer :: p(:) + type t + integer :: j + end type t + type(t), pointer :: tt(:), tp(:) ! Type pointer + class(t), pointer :: ct(:), cp(:) ! Class pointer + + p => m(1:3) + expect = is_contiguous (p) + print *, "is_contiguous (p)=", expect + if (.not. expect) stop 91 + call sub_star (p, expect) + p => m(1::3) + expect = is_contiguous (p) + print *, "is_contiguous (p)=", expect + if (expect) stop 92 + call sub_star (p, expect) + + allocate (tt(10)) + tt(:)% j = m + tp => tt(4:6) + expect = is_contiguous (tp) + if (.not. expect) stop 96 + print *, "is_contiguous (tp)=", expect + call sub_t (tp, expect) + tp => tt(4::3) + expect = is_contiguous (tp) + if (expect) stop 97 + print *, "is_contiguous (tp)=", expect + call sub_t (tp, expect) + + allocate (ct(10)) + ct(:)% j = m + cp => ct(7:9) + expect = is_contiguous (cp) + print *, "is_contiguous (cp)=", expect + if (.not. expect) stop 98 + call sub_t (cp, expect) + cp => ct(4::3) + expect = is_contiguous (cp) + print *, "is_contiguous (cp)=", expect + if (expect) stop 99 + call sub_t (cp, expect) + +contains + + subroutine sub_star (x, expect) +class(*), intent(in) :: x(:) +logical, intent(in) :: expect +cnt = cnt + 10 +if (is_contiguous (x) .neqv. expect) then + print *, "sub_star(1): is_contiguous (x)=", is_contiguous (x), expect + stop (cnt + 1) +end if +select type (x) +type is (integer) + if (is_contiguous (x) .neqv. expect) then + print *, "sub_star(2): is_contiguous (x)=", is_contiguous (x), expect + stop (cnt + 2) + end if +end select + end + + subroutine sub_t (x, expect) +class(t), intent(in) :: x(:) +logical, intent(in) :: expect +cnt = cnt + 10 +if (is_contiguous (x) .neqv. expect) then + print *, "sub_t: is_contiguous (x)=", is_contiguous (x), expect + stop (cnt + 3) +end if + end +end -- 2.35.3
Re: [PATCH, v2] Fortran: use name of array component in runtime error message [PR30802]
Hi Mikael, On 3/15/24 17:31, Mikael Morin wrote: Le 10/03/2024 à 22:31, Harald Anlauf a écrit : Dear all, after playing for some time with NAG and Intel, and an off-list discussion with Jerry, I am getting more and more convinced that simpler runtime error messages (also simpler to parse by a human) are superior to awkward solutions. This is also what Intel does: use only the name of the array (component) in the message whose indices are out of bounds. (NAG's solution appears also inconsistent for nested derived types.) So no x%z, or x%_data, etc. in runtime error messages any more. That's a pity. What about providing the root variable and the failing component only? ... dimension 1 of array component 'z...%x' above array bound ... The data reference doesn't look great, but it provides valuable (in my opinion) information. OK, that sounds interesting. To clarify the options: - for ordinary array x it would stay 'x' - when z is a DT scalar, and z%x is the array in question, use 'z%x' (here z...%x would look strange to me) - when z is a DT array, and x some component further down, 'z...%x' I would rather not make the error message text vary too much to avoid to run into issues with translation. Would it be fine with you to have ... dimension 1 of array 'z...%x' above array bound ... only? Anything else? Cheers, Harald Please give it a spin... Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 1/30/24 11:46, Mikael Morin wrote: Le 30/01/2024 à 11:38, Mikael Morin a écrit : Another (easier) way to clarify the data reference would be rephrasing the message so that the array part is separate from the scalar part, like so (there are too many 'of', but I lack inspiration): Index '0' of dimension 1 of component 'zz' of element from 'x1%vv' below lower bound of 1 This has the same number of 'of' but sounds better maybe: Out of bounds accessing component 'zz' of element from 'x1%yy': index '0' of dimension 1 below lower bound of 1
[PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135]
Dear all, as there has been some good progress in the handling of optional dummy arguments, I looked again at this PR and a patch for it that I withdrew as it turned out incomplete. It turned out that it now needs only a minor adjustment for optional dummy arguments of procedures with bind(c) attribute so that ubsan checking does not trigger. Along this way I extended the previous testcase to exercise to some extent combinations of bind(c) and non-bind(c) procedures and found one failure (since at least gcc-9) that is genuine: passing a missing optional from a bind(c) procedure to an assumed-rank dummy, see PR114355. The corresponding test is commented in the testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald On 2/6/22 22:04, Harald Anlauf wrote: Hi Mikael, Am 04.02.22 um 11:45 schrieb Mikael Morin: Hello, Le 29/01/2022 à 22:41, Harald Anlauf via Fortran a écrit : The least invasive change - already pointed out by the reporter - is to check the presence of the argument before dereferencing the data pointer after the offset calculation. This requires adjusting the checking pattern for gfortran.dg/missing_optional_dummy_6a.f90. Regtesting reminded me that procedures with bind(c) attribute are doing their own stuff, which is why they need to be excluded here, otherwise testcase bind-c-contiguous-4.f90 would regress on the expected output. only after submitting the patch I figured that the patch is incomplete. When we have a call chain of procedures with and without bind(c), there are still cases left where the failure with the sanitizer is not fixed. Just add "bind(c)" to subroutine test_wrapper only in the original PR. I have added a corresponding comment in the PR. There is a potential alternative solution which I did not pursue, as I think it is more invasive, but also that I didn't succeed to implement: A non-present dummy array argument should not need to get its descriptor set up. Pursuing this is probably not the right thing to do during the current stage of development and could be implemented later. If somebody believes this is important, feel free to open a PR for this. I have an other (equally unimportant) concern that it may create an unnecessary conditional when passing a subobject of an optional argument. In that case we can assume that the optional is present. It’s not a correctness issue, so let’s not bother at this stage. Judging from the dump tree of the cases I looked at I did not see anything that would pose a problem to the optimizer. Regtested on x86_64-pc-linux-gnu. OK for mainline? OK. Given my latest observations I'd rather withdraw the current version of the patch and rethink. I also did not see an issue with bind(c) procedures calling alikes. It would help if one would not only know the properties of the actual argument, but also of the formal one, which is not available at that point in the code. I'll have another look and resubmit. Thanks. Thanks for the review! Harald From b3079a82a220477704f8156207239e4af4103ea9 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 15 Mar 2024 20:14:07 +0100 Subject: [PATCH] Fortran: fix for absent array argument passed to optional dummy [PR101135] gcc/fortran/ChangeLog: PR fortran/101135 * trans-array.cc (gfc_get_dataptr_offset): Check for optional arguments being present before dereferencing data pointer. gcc/testsuite/ChangeLog: PR fortran/101135 * gfortran.dg/missing_optional_dummy_6a.f90: Adjust diagnostic pattern. * gfortran.dg/ubsan/missing_optional_dummy_8.f90: New test. --- gcc/fortran/trans-array.cc| 11 ++ .../gfortran.dg/missing_optional_dummy_6a.f90 | 2 +- .../ubsan/missing_optional_dummy_8.f90| 108 ++ 3 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/ubsan/missing_optional_dummy_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..a7717a8107e 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -7526,6 +7526,17 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, /* Set the target data pointer. */ offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp); + + /* Check for optional dummy argument being present. Arguments of BIND(C) + procedures are excepted here since they are handled differently. */ + if (expr->expr_type == EXPR_VARIABLE + && expr->symtree->n.sym->attr.dummy + && expr->symtree->n.sym->attr.optional + && !is_CFI_desc (NULL, expr)) +offset = build3_loc (input_location, COND_EXPR, TREE_TYPE (offset), + gfc_conv_expr_present (expr->symtree->n.sym), offset, + fold_convert (TREE_TYPE (offset), gfc_index_zero_node)); + gfc_conv_descriptor_data_set (block, parm, offset); } diff --git a/gcc/tes
Re: [PATCH v2 2/2] fortran: Fix specification expression error with dummy procedures [PR111781]
Hi Mikael, thanks for the patch! Regarding the first part of the patch, I think that fixing bad testcases can be done at any time. Retaining identified, broken testcases means that one may hit bogus regressions, hindering progress. The second part of the patch looks at first glance fine to me. And as the patch changes less than its size suggests, in particular due to code refactoring, I don't see a reason to postpone it to stage 1. (On the contrary, deferring it to stage 1 might make future backports from 15 for later patches on top of that code harder. This is my opinion, and others might see this differently.) On 3/17/24 17:57, Mikael Morin wrote: * expr.cc (check_restricted): Remove the case where symbol is dummy and declared in the current ns. Use gfc_get_spec_ns to get the right namespace. Looking at the original and new code, I don't fully understand that part of the commit message: the changed check comes into play when the symbol is *not* in_common, ..., a dummy, ... So technically, we didn't access the (now removed) formal_arg_flag here in those cases. Or am I missing something? diff --git a/gcc/fortran/expr.cc b/gcc/fortran/expr.cc index 82a642b01f7..0852bc5f493 100644 --- a/gcc/fortran/expr.cc +++ b/gcc/fortran/expr.cc @@ -3509,19 +3509,13 @@ check_restricted (gfc_expr *e) if (!check_references (e->ref, &check_restricted)) break; - /* gfc_is_formal_arg broadcasts that a formal argument list is being -processed in resolve.cc(resolve_formal_arglist). This is done so -that host associated dummy array indices are accepted (PR23446). -This mechanism also does the same for the specification expressions -of array-valued functions. */ if (e->error || sym->attr.in_common || sym->attr.use_assoc || sym->attr.dummy || sym->attr.implied_index || sym->attr.flavor == FL_PARAMETER - || is_parent_of_current_ns (sym->ns) - || (gfc_is_formal_arg () && (sym->ns == gfc_current_ns))) + || is_parent_of_current_ns (gfc_get_spec_ns (sym))) { t = true; break; diff --git a/gcc/testsuite/gfortran.dg/spec_expr_8.f90 b/gcc/testsuite/gfortran.dg/spec_expr_8.f90 new file mode 100644 index 000..5885810d421 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/spec_expr_8.f90 @@ -0,0 +1,24 @@ +! { dg-do compile } +! +! PR fortran/111781 +! We used to reject the example below because the dummy procedure g was +! setting the current namespace without properly restoring it, which broke +! the specification expression check for the dimension of A later on. +! +! Contributed by Markus Vikhamar-Sandberg Is the reporter's first name Markus or Rasmus? Thanks, Harald
Re: [PATCH, v2] Fortran: fix for absent array argument passed to optional dummy [PR101135]
Hi Mikael, On 3/17/24 22:04, Mikael Morin wrote: diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 3673fa40720..a7717a8107e 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -7526,6 +7526,17 @@ gfc_get_dataptr_offset (stmtblock_t *block, tree parm, tree desc, tree offset, /* Set the target data pointer. */ offset = gfc_build_addr_expr (gfc_array_dataptr_type (desc), tmp); + + /* Check for optional dummy argument being present. Arguments of BIND(C) + procedures are excepted here since they are handled differently. */ + if (expr->expr_type == EXPR_VARIABLE + && expr->symtree->n.sym->attr.dummy + && expr->symtree->n.sym->attr.optional + && !is_CFI_desc (NULL, expr)) I think the condition could additionally check the lack of subreferences. But it's maybe not worth the trouble, and the patch is conservatively correct as is, so OK. I have thought about the conditions here for some time and did not find better ones. They need to be broad enough to catch the case in gfortran.dg/missing_optional_dummy_6a.f90 that (according to the tree-dump) was not properly handled previously and would have triggered ubsan at some point in the future when someone tried to change that testcase from currently dg-do compile to dg-do run... (After the patch it would pass, but I didn't dare to change the dg-do). I have pushed the patch as-is, but feel free to post testcases not covered (or improperly covered) to narrow this down further... Thanks for the patch. Thanks for the review! Harald + offset = build3_loc (input_location, COND_EXPR, TREE_TYPE (offset), + gfc_conv_expr_present (expr->symtree->n.sym), offset, + fold_convert (TREE_TYPE (offset), gfc_index_zero_node)); + gfc_conv_descriptor_data_set (block, parm, offset); }
[PATCH, committed] Fortran: error recovery in frontend optimization [PR103715]
Dear all, I've committed the attached simple & obvious patch for an ICE due to an invalid read in frontend optimization after regtesting and an OK by Jerry in the PR. Pushed: https://gcc.gnu.org/g:3be2b8f475f22c531d6cef1b041c0573b3ea5133 As this PR is marked as a regression, I plan to backport to open branches. Thanks, Harald From 3be2b8f475f22c531d6cef1b041c0573b3ea5133 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 18 Mar 2024 19:36:59 +0100 Subject: [PATCH] Fortran: error recovery in frontend optimization [PR103715] gcc/fortran/ChangeLog: PR fortran/103715 * frontend-passes.cc (check_externals_expr): Prevent invalid read in case of mismatch of external subroutine with function. gcc/testsuite/ChangeLog: PR fortran/103715 * gfortran.dg/pr103715.f90: New test. --- gcc/fortran/frontend-passes.cc | 3 +++ gcc/testsuite/gfortran.dg/pr103715.f90 | 12 2 files changed, 15 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr103715.f90 diff --git a/gcc/fortran/frontend-passes.cc b/gcc/fortran/frontend-passes.cc index 06dfa1a3232..3c06018fdbb 100644 --- a/gcc/fortran/frontend-passes.cc +++ b/gcc/fortran/frontend-passes.cc @@ -5807,6 +5807,9 @@ check_externals_expr (gfc_expr **ep, int *walk_subtrees ATTRIBUTE_UNUSED, if (e->expr_type != EXPR_FUNCTION) return 0; + if (e->symtree && e->symtree->n.sym->attr.subroutine) +return 0; + sym = e->value.function.esym; if (sym == NULL) return 0; diff --git a/gcc/testsuite/gfortran.dg/pr103715.f90 b/gcc/testsuite/gfortran.dg/pr103715.f90 new file mode 100644 index 000..72c5a31fb21 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr103715.f90 @@ -0,0 +1,12 @@ +! { dg-do compile } +! PR fortran/103715 - ICE in gfc_find_gsymbol +! +! valgrind did report an invalid read in check_externals_procedure + +program p + select type (y => g()) ! { dg-error "Selector shall be polymorphic" } + end select + call g() +end + +! { dg-prune-output "already being used as a FUNCTION" } -- 2.35.3
[PATCH, v3] Fortran: improve array component description in runtime error message [PR30802]
Hi Mikael, all, here's now the third version of the patch that implements the following scheme: On 3/15/24 20:29, Mikael Morin wrote: Le 15/03/2024 à 18:26, Harald Anlauf a écrit : OK, that sounds interesting. To clarify the options: - for ordinary array x it would stay 'x' - when z is a DT scalar, and z%x is the array in question, use 'z%x' (here z...%x would look strange to me) Yes, the ellipsis would look strange to me as well. - when z is a DT array, and x some component further down, 'z...%x' This case also applies when z is a DT scalar and x is more than one level deep. I would rather not make the error message text vary too much to avoid to run into issues with translation. Would it be fine with you to have ... dimension 1 of array 'z...%x' above array bound ... only? OK, let's drop "component". Anything else? No, I think you covered everything. I've created a new helper function that centralizes the generation of the abbreviated name of the array (component) and use it to simplify related code in multiple places. If we change our mind how a bounds violation error message should look like, it will be easier to adjust in the future. Is this OK for 14-mainline? Thanks, Harald From 30d7cef086d440262b206bc39bcbcac89491b792 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 20 Mar 2024 20:59:24 +0100 Subject: [PATCH] Fortran: improve array component description in runtime error message [PR30802] Runtime error messages for array bounds violation shall use the following scheme for a coherent, abridged description of arrays or array components of derived types: (1) If x is an ordinary array variable, use "x" (2) if z is a DT scalar and x an array component at level 1, use "z%x" (3) if z is a DT scalar and x an array component at level > 1, or if z is a DT array and x an array (at any level), use "z...%x" Use a new helper function abridged_ref_name for construction of that name. gcc/fortran/ChangeLog: PR fortran/30802 * trans-array.cc (abridged_ref_name): New helper function. (trans_array_bound_check): Use it. (array_bound_check_elemental): Likewise. (gfc_conv_array_ref): Likewise. gcc/testsuite/ChangeLog: PR fortran/30802 * gfortran.dg/bounds_check_17.f90: Adjust pattern. * gfortran.dg/bounds_check_fail_8.f90: New test. --- gcc/fortran/trans-array.cc| 132 +++--- gcc/testsuite/gfortran.dg/bounds_check_17.f90 | 2 +- .../gfortran.dg/bounds_check_fail_8.f90 | 56 3 files changed, 142 insertions(+), 48 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_fail_8.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 0a453828bad..30b84762346 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3485,6 +3485,78 @@ gfc_conv_array_ubound (tree descriptor, int dim) } +/* Generate abridged name of a part-ref for use in bounds-check message. + Cases: + (1) for an ordinary array variable x return "x" + (2) for z a DT scalar and array component x (at level 1) return "z%%x" + (3) for z a DT scalar and array component x (at level > 1) or + for z a DT array and array x (at any number of levels): "z...%%x" + */ + +static char * +abridged_ref_name (gfc_expr * expr, gfc_array_ref * ar) +{ + gfc_ref *ref; + gfc_symbol *sym; + char *ref_name = NULL; + const char *comp_name = NULL; + int len_sym, last_len = 0, level = 0; + bool sym_is_array; + + gcc_assert (expr->expr_type == EXPR_VARIABLE && expr->ref != NULL); + + sym = expr->symtree->n.sym; + sym_is_array = (sym->ts.type != BT_CLASS + ? sym->as != NULL + : IS_CLASS_ARRAY (sym)); + len_sym = strlen (sym->name); + + /* Scan ref chain to get name of the array component (when ar != NULL) or + array section, determine depth and remember its component name. */ + for (ref = expr->ref; ref; ref = ref->next) +{ + if (ref->type == REF_COMPONENT + && strcmp (ref->u.c.component->name, "_data") != 0) + { + level++; + comp_name = ref->u.c.component->name; + continue; + } + + if (ref->type != REF_ARRAY) + continue; + + if (ar) + { + if (&ref->u.ar == ar) + break; + } + else if (ref->u.ar.type == AR_SECTION) + break; +} + + if (level > 0) +last_len = strlen (comp_name); + + /* Provide a buffer sufficiently large to hold "x...%%z". */ + ref_name = XNEWVEC (char, len_sym + last_len + 6); + strcpy (ref_name, sym->name); + + if (level == 1 && !sym_is_array) +{ + strcat (ref_name, "%%"); + strcat (ref_name, comp_name); +} + else if (level > 0) +{ + strcat (ref_name, "...%%"); + strcat (ref_name, comp_name); +} + + return ref_name; +} + +
Re: [PATCH] fortran: Ignore use statements on error [PR107426]
Hi Mikael, this looks all good to me. I wouldn't mind the minor side-effects of better error recovery, as you are (successfully) trying hard to keep the namespaces sane. So OK for mainline. Thanks for the patch! Harald On 3/21/24 17:27, Mikael Morin wrote: Hello, here is a fix for an ICE caused by dangling pointers to ISO_C_BINDING's C_PTR symbol in the global intrinsic symbol for C_LOC. I tried to fix it by making the intrinsic symbol use its own copy of C_PTR, but it regressed heavily. Instead, I propose this which is based on a patch I attached to the PR one year ago. It's sufficient to remove the access to freed memory. However, an underlying problem remains that successive use-associations of ISO_C_BINDING's symbols in different scopes cause the return type of the C_LOC global intrinsic symbol to be set to the C_PTR from each scope successively, with the last one "winning". Not very pretty. Anyway, there are two changed messages in the testsuite as a side-effect of the proposed change. I regard them as acceptable, albeit slightly worse. No regression otherwise on x86_64-pc-linux-gnu. Ok for 14 master? Mikael -- >8 -- This fixes an access to freed memory on the testcase from the PR. The problem comes from an invalid subroutine statement in an interface, which is ignored and causes the following statements forming the procedure body to be rejected. One of them use-associates the intrinsic ISO_C_BINDING module, which imports new symbols in a namespace that is freed at the time the statement is rejected. However, this creates dangling pointers as ISO_C_BINDING is special and its import creates a reference to the imported C_PTR symbol in the return type of the global intrinsic symbol for C_LOC (see the function create_intrinsic_function). This change saves and restores the list of use statements, so that rejected use statements are removed before they have a chance to be applied to the current namespace and create dangling pointers. PR fortran/107426 gcc/fortran/ChangeLog: * gfortran.h (gfc_save_module_list, gfc_restore_old_module_list): New declarations. * module.cc (old_module_list_tail): New global variable. (gfc_save_module_list, gfc_restore_old_module_list): New functions. (gfc_use_modules): Set module_list and old_module_list_tail. * parse.cc (next_statement): Save module_list before doing any work. (reject_statement): Restore module_list to its saved value. gcc/testsuite/ChangeLog: * gfortran.dg/pr89943_3.f90: Update error pattern. * gfortran.dg/pr89943_4.f90: Likewise. * gfortran.dg/use_31.f90: New test. --- gcc/fortran/gfortran.h | 2 ++ gcc/fortran/module.cc | 31 + gcc/fortran/parse.cc| 4 gcc/testsuite/gfortran.dg/pr89943_3.f90 | 2 +- gcc/testsuite/gfortran.dg/pr89943_4.f90 | 2 +- gcc/testsuite/gfortran.dg/use_31.f90| 25 6 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/use_31.f90 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index c7039730fad..fec7b53ff1a 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -3926,6 +3926,8 @@ void gfc_module_done_2 (void); void gfc_dump_module (const char *, int); bool gfc_check_symbol_access (gfc_symbol *); void gfc_free_use_stmts (gfc_use_list *); +void gfc_save_module_list (); +void gfc_restore_old_module_list (); const char *gfc_dt_lower_string (const char *); const char *gfc_dt_upper_string (const char *); diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc index d1de53cbdb4..c565b84d61b 100644 --- a/gcc/fortran/module.cc +++ b/gcc/fortran/module.cc @@ -195,7 +195,12 @@ static const char *module_name; /* The name of the .smod file that the submodule will write to. */ static const char *submodule_name; +/* The list of use statements to apply to the current namespace + before parsing the non-use statements. */ static gfc_use_list *module_list; +/* The end of the MODULE_LIST list above at the time the recognition + of the current statement started. */ +static gfc_use_list **old_module_list_tail; /* If we're reading an intrinsic module, this is its ID. */ static intmod_id current_intmod; @@ -7561,6 +7566,8 @@ gfc_use_modules (void) gfc_use_module (module_list); free (module_list); } + module_list = NULL; + old_module_list_tail = &module_list; gfc_rename_list = NULL; } @@ -7584,6 +7591,30 @@ gfc_free_use_stmts (gfc_use_list *use_stmts) } +/* Remember the end of the MODULE_LIST list, so that the list can be restored + to its previous state if the current statement is erroneous. */ + +void +gfc_save_module_list () +{ + gfc_use_list **tail = &module_list; + while (*tail != NULL) +tail = &(*tail)->next; + old_module_list_tail = tail; +} + + +/* Restore the MODULE_LIST lis
[PATCH] Fortran: no size check passing NULL() without MOLD argument [PR55978]
Dear all, here's a simple and obvious patch for a rejects-valid case when we pass a NULL() actual to an optional dummy for variants where there is no MOLD argument and it is also not required. The testcase is an extended version of PR55978 comment#16 and cross-checked with Intel and NAG. Regtested on x86_64-pc-linux-gnu. I intend to commit soon unless there are objections. Thanks, Harald From e92244c5539a537cff338b781d15acd58d4c86f1 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 22 Mar 2024 18:17:15 +0100 Subject: [PATCH] Fortran: no size check passing NULL() without MOLD argument [PR55978] gcc/fortran/ChangeLog: PR fortran/55978 * interface.cc (gfc_compare_actual_formal): Skip size check for NULL() actual without MOLD argument. gcc/testsuite/ChangeLog: PR fortran/55978 * gfortran.dg/null_actual_5.f90: New test. --- gcc/fortran/interface.cc| 4 ++ gcc/testsuite/gfortran.dg/null_actual_5.f90 | 76 + 2 files changed, 80 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/null_actual_5.f90 diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index 64b90550be2..7b86a338bc1 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -3439,6 +3439,10 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, if (f->sym->ts.type == BT_CLASS) goto skip_size_check; + /* Skip size check for NULL() actual without MOLD argument. */ + if (a->expr->expr_type == EXPR_NULL && a->expr->ts.type == BT_UNKNOWN) + goto skip_size_check; + actual_size = get_expr_storage_size (a->expr); formal_size = get_sym_storage_size (f->sym); if (actual_size != 0 && actual_size < formal_size diff --git a/gcc/testsuite/gfortran.dg/null_actual_5.f90 b/gcc/testsuite/gfortran.dg/null_actual_5.f90 new file mode 100644 index 000..1198715b7c8 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/null_actual_5.f90 @@ -0,0 +1,76 @@ +! { dg-do compile } +! PR fortran/55978 +! +! Passing of NULL() with and without MOLD as actual argument +! +! Testcase derived from pr55978 comment#16 + +program pr55978_c16 + implicit none + + integer, pointer :: p(:) + integer, allocatable :: a(:) + character(10), pointer :: c + character(10), pointer :: cp(:) + + type t +integer, pointer :: p(:) +integer, allocatable :: a(:) + end type + + type(t) :: d + + ! (1) pointer + p => null() + call sub (p) + + ! (2) allocatable + call sub (a) + call sub (d%a) + + ! (3) pointer component + d%p => null () + call sub (d%p) + + ! (4) NULL + call sub (null (a)) ! OK + call sub (null (p)) ! OK + call sub (null (d%a)) ! OK + call sub (null (d%p)) ! OK + call sub (null ())! was erroneously rejected with: + ! Actual argument contains too few elements for dummy argument 'x' (1/4) + + call bla (null(c)) + call bla (null()) ! was erroneously rejected with: + ! Actual argument contains too few elements for dummy argument 'x' (1/10) + + call foo (null(cp)) + call foo (null()) + + call bar (null(cp)) + call bar (null()) ! was erroneously rejected with: + ! Actual argument contains too few elements for dummy argument 'x' (1/70) + +contains + + subroutine sub(x) +integer, intent(in), optional :: x(4) +if (present (x)) stop 1 + end + + subroutine bla(x) +character(len=10), intent(in), optional :: x +if (present (x)) stop 2 + end + + subroutine foo(x) +character(len=10), intent(in), optional :: x(:) +if (present (x)) stop 3 + end + + subroutine bar(x) +character(len=10), intent(in), optional :: x(7) +if (present (x)) stop 4 + end + +end -- 2.35.3
Re: [patch, libgfortran] PR107031 - endfile truncates file at wrong position
Hi Jerry, Am 26.03.24 um 04:18 schrieb Jerry D: Hi all, There has been a bit of discussio on which way to go on this. I took a look today and this trivial patch gives the behavior concluded on Fortran Discourse. See the bugzilla for all the relevant information. Regresion tested on x86-64. I will do the appropriate changelog. OK for trunk? Attached is a new test case and the patch here: diff --git a/libgfortran/io/file_pos.c b/libgfortran/io/file_pos.c index 2bc05b293f8..d169961f997 100644 --- a/libgfortran/io/file_pos.c +++ b/libgfortran/io/file_pos.c @@ -352,7 +352,6 @@ st_endfile (st_parameter_filepos *fpp) dtp.common = fpp->common; memset (&dtp.u.p, 0, sizeof (dtp.u.p)); dtp.u.p.current_unit = u; - next_record (&dtp, 1); } unit_truncate (u, stell (u->s), &fpp->common); this is OK from my side. Given the discussion on "dg-do run", wouldn't this be a perfect example where it is sufficient to run the testcase just once? The change is in libgfortran, not in the frontend or middle-end. Thanks for the patch! Harald
[PATCH] Fortran: fix DATA and derived types with pointer components [PR114474]
Dear all, the attached patch fixes a 10+ regression for cases where a derived type with a pointer component is used in a DATA statement. The failure looked obscure, see testcase comments, and pointed to a possible issue in the resolution (order). For the failing test, the target variable was seen with ts.type == BT_PROCEDURE instead of its actual type. For this reason, I restricted the fixup as much as possible. For details, please see the commit message. Testcase cross-checked with NAG. Regtested on x86_64-pc-linux-gnu. OK for mainline? If this fix survives broader testing, I would like to backport. Thanks, Harald P.S.: while trying to extend coverage of conforming code, I had much fun also with other compilers (e.g. NAG panicking...) From d5fda38243a22e1aef4367653d92521e53f2000d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 27 Mar 2024 21:18:04 +0100 Subject: [PATCH] Fortran: fix DATA and derived types with pointer components [PR114474] When matching actual arguments in match_actual_arg, these are initially treated as a possible dummy procedure, assuming that the correct type is determined later. This resolution could fail when the procedure is a derived type constructor with a pointer component and appears in a DATA statement, where the pointer shall be associated with an initial data target. Check for those cases where the type obviously has not been resolved yet, and which were missed because there was no component reference. gcc/fortran/ChangeLog: PR fortran/114474 * primary.cc (gfc_variable_attr): Catch variables used in structure constructors within DATA statements that are still tagged with a temporary type BT_PROCEDURE from match_actual_arg and which have the target attribute, and fix their typespec. gcc/testsuite/ChangeLog: PR fortran/114474 * gfortran.dg/data_pointer_3.f90: New test. --- gcc/fortran/primary.cc | 12 +++ gcc/testsuite/gfortran.dg/data_pointer_3.f90 | 77 2 files changed, 89 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/data_pointer_3.f90 diff --git a/gcc/fortran/primary.cc b/gcc/fortran/primary.cc index 0ab69bb9dce..5dd6875a4a6 100644 --- a/gcc/fortran/primary.cc +++ b/gcc/fortran/primary.cc @@ -2804,6 +2804,18 @@ gfc_variable_attr (gfc_expr *expr, gfc_typespec *ts) if (ts != NULL && expr->ts.type == BT_UNKNOWN) *ts = sym->ts; + /* Catch left-overs from match_actual_arg, where an actual argument of a + procedure is given a temporary ts.type == BT_PROCEDURE. The fixup is + needed for structure constructors in DATA statements, where a pointer + is associated with a data target, and the argument has not been fully + resolved yet. Components references are dealt with further below. */ + if (ts != NULL + && expr->ts.type == BT_PROCEDURE + && expr->ref == NULL + && attr.flavor != FL_PROCEDURE + && attr.target) +*ts = sym->ts; + has_inquiry_part = false; for (ref = expr->ref; ref; ref = ref->next) if (ref->type == REF_INQUIRY) diff --git a/gcc/testsuite/gfortran.dg/data_pointer_3.f90 b/gcc/testsuite/gfortran.dg/data_pointer_3.f90 new file mode 100644 index 000..f0325cd5bcb --- /dev/null +++ b/gcc/testsuite/gfortran.dg/data_pointer_3.f90 @@ -0,0 +1,77 @@ +! { dg-do compile } +! PR fortran/114474 - DATA and derived types with pointer components + +program pr114474 + implicit none + integer, target :: ii = 42! initial data target + + integer, target :: jj = 24 + integer, pointer:: qq => jj + ! ii and jj resolve slightly differently when the data statement below + ! is reached, as jj is resolved outside the structure constructor first + + type t + integer, pointer :: h + end type t + + integer, target :: kk(7) = 23 + integer, pointer:: ll(:) => kk + + type t1 + integer :: m(7) + end type t1 + + type(t) :: x1, x2, x3, x4, x5 + type(t), parameter :: z1 = t(null()) + + type(t1), target:: tt = t1([1,2,3,4,5,6,7]) + type(t1), parameter :: vv = t1(22) + type(t1):: w1, w2 + integer, pointer :: p1(:) => tt% m + + data x1 / t(null()) / + data x2 / t(ii) / ! ii is initial data target + data x3 / t(jj) / ! jj is resolved differently... + data x4 / t(tt%m(3)) / ! pointer association with 3rd element + + data w1 / t1(12) / + data w2 / t1(vv%m) / + + if ( associated (x1% h)) stop 1 + if (.not. associated (x2% h)) stop 2 + if (.not. associated (x3% h)) stop 3 + if (.not. associated (x4% h)) stop 4 + if (x2% h /= 42) stop 5 + if (x3% h /= 24) stop 6 + if (x4% h /= 3) stop 7 + + if (any (w1%m /= 12 )) stop 8 + if (any (w2%m /= vv%m)) stop 9 +end + + +subroutine sub + implicit none + + interface + real function myfun (x) + real, intent(in) :: x + end function myfun + end interface + + type u + procedure(myfun),
[PATCH] Fortran: fix NULL pointer dereference on overlapping initialization [PR50410]
Dear all, the attached simple, obvious and ancient patch from the PR fixes a NULL pointer dereference that occurs on overlapping initializations of derived types/DT components in DATA statements. Gfortran currently does not detect or report overlapping initializations in such cases, and some other compilers also do not (Intel) or give only a warning (e.g. Nvidia). For this reason I decided to add -std=legacy to the options in the testcase. Detecting the overlapping initializations appears to require deeper changes in the way we look up DT components when handling DATA statements, which is beyond the current stage. Regtested on x86_64-pc-linux-gnu. I intend to commit soon unless there are objections. Thanks, Harald From b3970a30679959eed159dffa816899e4430e9da5 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 28 Mar 2024 22:34:40 +0100 Subject: [PATCH] Fortran: fix NULL pointer dereference on overlapping initialization [PR50410] gcc/fortran/ChangeLog: PR fortran/50410 * trans-expr.cc (gfc_conv_structure): Check for NULL pointer. gcc/testsuite/ChangeLog: PR fortran/50410 * gfortran.dg/data_initialized_4.f90: New test. --- gcc/fortran/trans-expr.cc| 2 +- gcc/testsuite/gfortran.dg/data_initialized_4.f90 | 16 2 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/data_initialized_4.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 76bed9830c4..7ce798ab8a5 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -9650,7 +9650,7 @@ gfc_conv_structure (gfc_se * se, gfc_expr * expr, int init) cm = expr->ts.u.derived->components; for (c = gfc_constructor_first (expr->value.constructor); - c; c = gfc_constructor_next (c), cm = cm->next) + c && cm; c = gfc_constructor_next (c), cm = cm->next) { /* Skip absent members in default initializers and allocatable components. Although the latter have a default initializer diff --git a/gcc/testsuite/gfortran.dg/data_initialized_4.f90 b/gcc/testsuite/gfortran.dg/data_initialized_4.f90 new file mode 100644 index 000..156b6607edf --- /dev/null +++ b/gcc/testsuite/gfortran.dg/data_initialized_4.f90 @@ -0,0 +1,16 @@ +! { dg-do compile } +! { dg-additional-options "-std=legacy" } +! +! PR fortran/50410 +! +! Silently allow overlapping initialization in legacy mode (used to ICE) + +program p + implicit none + type t + integer :: g = 1 + end type t + type(t) :: u = t(2) + data u%g /3/ + print *, u! this might print "2" +end -- 2.35.3
Re: [Patch, fortran] PR110987 and PR113885 - gimplifier ICEs and wrong results in finalization
Hi Paul, Am 28.03.24 um 16:39 schrieb Paul Richard Thomas: Hi All, The attached patch has two elements: (i) A fix for gimplifier ICEs with derived type having no components. The reporter himself suggested (thanks Kirill!): - if (derived && derived->attr.zero_comp) + if (derived && (derived->components == NULL)) As far as I can tell, this is the correct fix. I tried setting attr.zero_comp in resolve.cc for all the OK types without components but this caused all sorts of fallout. (ii) Final calls were occurring in the wrong place for finalizable elemental function calls within scalarizer loops. This caused incorrect results even for derived types with components. This is also fixed. yes, this looks good here. It should be noted that finalizer calls from the rhs of an assignment are occurring at the wrong time, since F2018/24-7.5.6.3 requires: "If an executable construct references a nonpointer function, the result is finalized after execution of the innermost executable construct containing the reference.", while in the present implementation, this happening just before assignment to the lhs temporary. Fixing this is going to be really tough and invasive, so I decided that getting the right results and the correct number of finalization should be sufficient for the 14-branch release. As it happens, I had been mulling over how to do this for finalizations hidden in constructors and other contexts than assignment (eg. write statements or allocation with source). It's a few months away and will be appropriate for stage 1. Regtests on x86_64 - OK for mainline and then, after a bit, for backporting to 13-branch? The patch looks rather "conservative" (read: safe) and appears to fix the regressions very well, so go ahead as planned. Thanks for the patch! Harald Regards to all Paul Fortran: Fix a gimplifier ICE/wrong result with finalization [PR104555] 2024-03-28 Paul Thomas gcc/fortran PR fortran/36337 PR fortran/110987 PR fortran/113885 * trans-expr.cc (gfc_trans_assignment_1): Place finalization block before rhs post block for elemental rhs. * trans.cc (gfc_finalize_tree_expr): Check directly if a type has no components, rather than the zero components attribute. Treat elemental zero component expressions in the same way as scalars. gcc/testsuite/ PR fortran/113885 * gfortran.dg/finalize_54.f90: New test. * gfortran.dg/finalize_55.f90: New test. gcc/testsuite/ PR fortran/110987 * gfortran.dg/finalize_56.f90: New test.
Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab
Hi Paul, I had only a quick glance at your patch. I guess you unintentionally forgot to remove those parts that you already committed for PR110987, along with the finalize-testcases. I am still trying to find the precise paragraph in the standard you refer to regarding INTENT(OUT) and default initialization. While at it, I think I found a minor nit in testcase pr112407a.f90: component x%i appears undefined the first time it is printed. This can be verified by either adding an explicit x% i = -42 in the main after the allocate(x). Alternatively, running the code with Intel and using MALLOC_PERTURB_ shows a random arg1%i, but is otherwise fine. However, if by chance (random memory) x% i = +42 then the test would likely fail everywhere. Cheers, Harald Am 30.03.24 um 10:06 schrieb Paul Richard Thomas: Hi All, This bug emerged in a large code and involves possible recursion with a "hidden" module procedure; ie. where the symtree name starts with '@'. This throws the format decoder. As the last message in the PR shows, I have vacillated between silently passing on the possible recursion or adding an alternative warning message. In the end, as a conservative choice I went for emitting the message. In the course of trying to develop a compact test case, I found that type bound procedures were not being tested for recursion and that class dummies, with intent out, were being incorrectly initialized with an empty default initializer. Both of these have been fixed. Unfortunately, the most compact reproducer that Tomas was able to come up with required more than 100kbytes of module files. I tried from the bottom up but failed. Both the tests check the fixes for the other bugs. Regtests on x86_64 - OK for mainline and, in a couple of weeks, 13-branch? Paul Fortran: Fix wrong recursive errors and class initialization [PR112407] 2024-03-30 Paul Thomas gcc/fortran PR fortran/112407 *resolve.cc (resolve_procedure_expression): Change the test for for recursion in the case of hidden procedures from modules. (resolve_typebound_static): Add warning for possible recursive calls to typebound procedures. * trans-expr.cc (gfc_trans_class_init_assign): Do not apply default initializer to class dummy where component initializers are all null. gcc/testsuite/ PR fortran/112407 * gfortran.dg/pr112407a.f90: New test. * gfortran.dg/pr112407b.f90: New test.
Re: [Patch, fortran] PR112407 - [13/14 Regression] Fix for PR37336 triggers an ICE in gfc_format_decoder while constructing a vtab
Hi Paul! Am 31.03.24 um 14:08 schrieb Paul Richard Thomas: Hi Harald, I had only a quick glance at your patch. I guess you unintentionally forgot to remove those parts that you already committed for PR110987, along with the finalize-testcases. Guilty as charged. I guess I got out of the wrong side of the bed :-) I am still trying to find the precise paragraph in the standard you refer to regarding INTENT(OUT) and default initialization. Page 114 of the draft F2023 standard: "The INTENT (OUT) attribute for a nonpointer dummy argument specifies that the dummy argument becomes undefined on invocation of the procedure, except for any subcomponents that are default-initialized (7.5.4.6)." With the fix, gfortran behaves in the same way as ifort and nagfor. On rereading the patch, I think that s/"and use the passed value"/"and leave undefined"/ or some such is in order. Yes, something along this line is better. I also did test with NAG and Intel, and was surprised (confused?) at how the count of finalizer calls changes if component "i" gets a default value or not. Something one wouldn't do right after getting out of bed! So the patch looks good to me - except for one philosophical question: Fortran 2018 makes procedures recursive by default, but this is not yet implemented as such, and NON_RECURSIVE is not yet implemented. The new testcase pr112407b.f90 compiles with nagfor -f2018 without any warnings, and gives an error with nagfor -f2008. It appears that it works in the testsuite after the patch and when adding "-std=f2008" instead of using the default "-std=gnu". Would you mind adding "-std=f2008" as dg-option to that testcase? This would avoid one bogus regression when gfortran moves forward. Thanks for the patch! Harald While at it, I think I found a minor nit in testcase pr112407a.f90: component x%i appears undefined the first time it is printed. Fixed - thanks for pointing it out. A correct patch is attached. Thanks for looking at the previous, overloaded version. Paul 2024-03-30 Paul Thomas gcc/fortran PR fortran/112407 *resolve.cc (resolve_procedure_expression): Change the test for for recursion in the case of hidden procedures from modules. (resolve_typebound_static): Add warning for possible recursive calls to typebound procedures. * trans-expr.cc (gfc_trans_class_init_assign): Do not apply default initializer to class dummy where component initializers are all null. gcc/testsuite/ PR fortran/112407 * gfortran.dg/pr112407a.f90: New test. * gfortran.dg/pr112407b.f90: New test.
Re: [Patch, fortran] PR106999 [11/12/13/14 Regression] ICE tree check: expected record_type or union_type or qual_union_type, have function_type in gfc_class_data_get, at fortran/trans-expr.cc:233
Hi Paul, On 3/31/24 15:01, Paul Richard Thomas wrote: This regression has a relatively simple fix. The passing of a subroutine procedure pointer component to a dummy variable was being missed completely. The error has been added. Conversely, an error was generated for a procedure pointer variable but no use was being made of the interface, if one was available. This has been corrected. OK for mainline and backporting in a couple of weeks? this is all OK. Thanks for the patch! Harald Paul Fortran: Add error for subroutine passed to a variable dummy [PR106999] 2024-03-31 Paul Thomas gcc/fortran PR fortran/106999 *interface.cc (gfc_compare_interfaces): Add error for a subroutine proc pointer passed to a variable formal. (compare_parameter): If a procedure pointer is being passed to a non-procedure formal arg, and there is an an interface, use gfc_compare_interfaces to check and provide a more useful error message. gcc/testsuite/ PR fortran/106999 * gfortran.dg/pr106999.f90: New test.
[PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500]
Dear all, the attached patch fixes argument checking of: - C_SIZEOF - rejects-valid (see below) and ICE-on-valid - C_F_POINTER - ICE-on-invalid The interesting part is that C_SIZEOF was not well specified until after F2018, where an interp request lead to an edit that actually loosened restrictions and makes the checking much more straightforward, since expressions and function results are now allowed. I've added references to the relevant text and interp in the commit message. While updating the checking code shared between C_SIZEOF and C_F_POINTER, I figured that the latter missed a check preventing an ICE-on-invalid when a function returning a pointer was passed. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 6f412a6399a7e125db835584d3d2489a52150c27 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 8 Apr 2024 21:43:24 +0200 Subject: [PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500] The interpretation of the F2018 standard regarding valid arguments to the intrinsic C_SIZEOF(X) was clarified in an edit to 18-007r1: https://j3-fortran.org/doc/year/22/22-101r1.txt loosening restrictions and giving examples. The F2023 text has: ! F2023:18.2.3.8 C_SIZEOF (X) ! ! X shall be a data entity with interoperable type and type parameters, ! and shall not be an assumed-size array, an assumed-rank array that ! is associated with an assumed-size array, an unallocated allocatable ! variable, or a pointer that is not associated. where ! 3.41 data entity ! data object, result of the evaluation of an expression, or the ! result of the execution of a function reference Update the checking code for interoperable arguments accordingly, and extend to reject functions returning pointer as FPTR argument to C_F_POINTER. gcc/fortran/ChangeLog: PR fortran/106500 * check.cc (is_c_interoperable): Fix checks for C_SIZEOF. (gfc_check_c_f_pointer): Reject function returning a pointer as FPTR. gcc/testsuite/ChangeLog: PR fortran/106500 * gfortran.dg/c_sizeof_6.f90: Remove wrong dg-error. * gfortran.dg/c_f_pointer_tests_9.f90: New test. * gfortran.dg/c_sizeof_7.f90: New test. --- gcc/fortran/check.cc | 21 ++ .../gfortran.dg/c_f_pointer_tests_9.f90 | 21 ++ gcc/testsuite/gfortran.dg/c_sizeof_6.f90 | 2 +- gcc/testsuite/gfortran.dg/c_sizeof_7.f90 | 42 +++ 4 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_sizeof_7.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index db74dcf3f40..b7f60575c67 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5299,18 +5299,14 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr) return false; } - if (!c_loc && expr->rank > 0 && expr->expr_type != EXPR_ARRAY) + /* Checks for C_SIZEOF need to take into account edits to 18-007r1, see + https://j3-fortran.org/doc/year/22/22-101r1.txt . */ + if (!c_loc && !c_f_ptr && expr->rank > 0 && expr->expr_type == EXPR_VARIABLE) { gfc_array_ref *ar = gfc_find_array_ref (expr); - if (ar->type != AR_FULL) + if (ar->type == AR_FULL && ar->as->type == AS_ASSUMED_SIZE) { - *msg = "Only whole-arrays are interoperable"; - return false; - } - if (!c_f_ptr && ar->as->type != AS_EXPLICIT - && ar->as->type != AS_ASSUMED_SIZE) - { - *msg = "Only explicit-size and assumed-size arrays are interoperable"; + *msg = "Assumed-size arrays are not interoperable"; return false; } } @@ -5475,6 +5471,13 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) return false; } + if (attr.function) +{ + gfc_error ("FPTR at %L to C_F_POINTER is a function returning a pointer", + &fptr->where); + return false; +} + if (fptr->rank > 0 && !is_c_interoperable (fptr, &msg, false, true)) return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR " "at %L to C_F_POINTER: %s", &fptr->where, msg); diff --git a/gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 b/gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 new file mode 100644 index 000..bb6d3281b02 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 @@ -0,0 +1,21 @@ +! { dg-do compile } +! +! A function returning a pointer cannot be interoperable +! and cannot be used as FPTR argument to C_F_POINTER. + +subroutine s () + use, intrinsic :: iso_c_binding + implicit none + type(c_ptr) :: cPtr + call c_f_pointer (cPtr, p0)! { dg-error "function returning a pointer" } + call c_f_pointer (cPtr, p1, shape=[2
[PATCH, v2] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500]
Hi FX! On 4/9/24 09:32, FX Coudert wrote: Hi Harald, Thanks for the patch. + if (attr.function) +{ + gfc_error ("FPTR at %L to C_F_POINTER is a function returning a pointer", + &fptr->where); + return false; +} + if (fptr->rank > 0 && !is_c_interoperable (fptr, &msg, false, true)) return gfc_notify_std (GFC_STD_F2018, "Noninteroperable array FPTR " "at %L to C_F_POINTER: %s", &fptr->where, msg); In both of these gfc_error(), could we change our error message to say "FPTR argument” instead of “FPTR”? “FPTR to C_F_POINTER” does not really make sense to me. This would be more in line with what the generally do: Error: 'x' argument of 'sqrt' intrinsic at (1) must be REAL or COMPLEX So maybe “FPTR argument to C_F_POINTER at %L” ? That’s much more readable to me. Good point! I did indeed feel a little uncomfortable with the text and adjusted both messages accordingly to your suggestion. I also forgot to add one update of a pattern, and found a cornercase where the tightening of checks for C_F_POINTER was too strong. Corrected and now covered in an extension of the corresponding testcase. Otherwise, OK. FX Thanks for the review! If there are no further comments, I will commit tomorrow. Thanks, Harald From 5983a07f11c88d920241141732fa742735cdb8ea Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 9 Apr 2024 23:07:59 +0200 Subject: [PATCH] Fortran: fix argument checking of intrinsics C_SIZEOF, C_F_POINTER [PR106500] The interpretation of the F2018 standard regarding valid arguments to the intrinsic C_SIZEOF(X) was clarified in an edit to 18-007r1: https://j3-fortran.org/doc/year/22/22-101r1.txt loosening restrictions and giving examples. The F2023 text has: ! F2023:18.2.3.8 C_SIZEOF (X) ! ! X shall be a data entity with interoperable type and type parameters, ! and shall not be an assumed-size array, an assumed-rank array that ! is associated with an assumed-size array, an unallocated allocatable ! variable, or a pointer that is not associated. where ! 3.41 data entity ! data object, result of the evaluation of an expression, or the ! result of the execution of a function reference Update the checking code for interoperable arguments accordingly, and extend to reject functions returning pointer as FPTR argument to C_F_POINTER. gcc/fortran/ChangeLog: PR fortran/106500 * check.cc (is_c_interoperable): Fix checks for C_SIZEOF. (gfc_check_c_f_pointer): Reject function returning a pointer as FPTR, and improve an error message. gcc/testsuite/ChangeLog: PR fortran/106500 * gfortran.dg/c_sizeof_6.f90: Remove wrong dg-error. * gfortran.dg/sizeof_2.f90: Adjust pattern. * gfortran.dg/c_f_pointer_tests_9.f90: New test. * gfortran.dg/c_sizeof_7.f90: New test. --- gcc/fortran/check.cc | 26 +++- .../gfortran.dg/c_f_pointer_tests_9.f90 | 37 gcc/testsuite/gfortran.dg/c_sizeof_6.f90 | 2 +- gcc/testsuite/gfortran.dg/c_sizeof_7.f90 | 42 +++ gcc/testsuite/gfortran.dg/sizeof_2.f90| 2 +- 5 files changed, 96 insertions(+), 13 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/c_f_pointer_tests_9.f90 create mode 100644 gcc/testsuite/gfortran.dg/c_sizeof_7.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index db74dcf3f40..2f50d84b876 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -5299,18 +5299,14 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr) return false; } - if (!c_loc && expr->rank > 0 && expr->expr_type != EXPR_ARRAY) + /* Checks for C_SIZEOF need to take into account edits to 18-007r1, see + https://j3-fortran.org/doc/year/22/22-101r1.txt . */ + if (!c_loc && !c_f_ptr && expr->rank > 0 && expr->expr_type == EXPR_VARIABLE) { gfc_array_ref *ar = gfc_find_array_ref (expr); - if (ar->type != AR_FULL) + if (ar->type == AR_FULL && ar->as->type == AS_ASSUMED_SIZE) { - *msg = "Only whole-arrays are interoperable"; - return false; - } - if (!c_f_ptr && ar->as->type != AS_EXPLICIT - && ar->as->type != AS_ASSUMED_SIZE) - { - *msg = "Only explicit-size and assumed-size arrays are interoperable"; + *msg = "Assumed-size arrays are not interoperable"; return false; } } @@ -5475,9 +5471,17 @@ gfc_check_c_f_pointer (gfc_expr *cptr, gfc_expr *fptr, gfc_expr *shape) return false; } + if (fptr->ts.type == BT_PROCEDURE && attr.function) +{ + gfc_error ("FPTR argument to C_F_POINTER at %L is a function " + "returning a pointer", &fptr->where); + return false; +} + if (fptr->rank > 0 && !is_c_interoperable
Re: [Patch, fortran] PR113363 - ICE on ASSOCIATE and unlimited polymorphic function
Hi Paul! On 4/10/24 10:25, Paul Richard Thomas wrote: Hi All, This patch corrects incorrect results from assignment of unlimited polymorphic function results both in assignment statements and allocation with source. The first chunk in trans-array.cc ensures that the array dtype is set to the source dtype. The second chunk ensures that the lhs _len field does not default to zero and so is specific to dynamic types of character. The addition to trans-stmt.cc transforms the source expression, aka expr3, from a derived type of type "STAR" into a proper unlimited polymorphic expression ready for assignment to the newly allocated entity. I am wondering about the following snippet in trans-stmt.cc: + /* Copy over the lhs _data component ref followed by the +full array reference for source expressions with rank. +Otherwise, just copy the _data component ref. */ + if (code->expr3->rank + && ref && ref->next && !ref->next->next) + { + rhs->ref = gfc_copy_ref (ref); + rhs->ref->next = gfc_copy_ref (ref->next); + break; + } Why the two gfc_copy_ref? valgrind pointed my to the tail of gfc_copy_ref which already has: dest->next = gfc_copy_ref (src->next); so this looks redundant and leaks frontend memory? *** Playing with the testcase, I find several invalid writes with valgrind, or a heap buffer overflow with -fsanitize=address . It is sufficient to look at a mini-test where the class(*) function result is assigned to the class(*), allocatable in the main: x = foo () deallocate (x) The dump tree suggests that array bounds in foo() are read before they are properly set. These invalid writes do not occur with 13-branch, so this might be a regression. Can you have a look yourself? Thanks, Harald OK for mainline? Paul Fortran: Fix wrong code in unlimited polymorphic assignment [PR113363] 2024-04-10 Paul Thomas gcc/fortran PR fortran/113363 * trans-array.cc (gfc_array_init_size): Use the expr3 dtype so that the correct element size is used. (gfc_alloc_allocatable_for_assignment): Set the _len field for unlimited polymorphic assignments. * trans-stmt.cc (gfc_trans_allocate): Build a correct rhs for the assignment of an unlimited polymorphic 'source'. gcc/testsuite/ PR fortran/113363 * gfortran.dg/pr113363.f90: New test.
Re: [patch, libgfortran] PR111022 ES0.0E0 format gave ES0.dE0 output with d too high.
Jerry, Steve, Am 03.02.24 um 04:24 schrieb Steve Kargl: Jerry, The patch looks good to me, but please give Harald a chance to comment. I just tested it a little, and it looked good. We even get a runtime error on E0.0 now as required. :-) Thanks for the patch! Harald
[PATCH] Fortran: error recovery on arithmetic overflow on unary operations [PR113799]
Dear all, the attached patch improves error recovery when we encounter an array constructor where a unary operator (e.g. minus) is applied and -frange-check is active. The solution is not to terminate early in that case to avoid inconsistencies between check_result and reduce_unary when such a situation occurs. (There might be similar issues for binary operators, not treated here.) Regtested on x86_64-pc-linux-gnu. OK for mainline? The ICE/memory corruption is actually a 10+ regression. Do we need a backport? Thanks, Harald From eec039211e396e35204b55588013d74289a984cd Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 8 Feb 2024 21:51:38 +0100 Subject: [PATCH] Fortran: error recovery on arithmetic overflow on unary operations [PR113799] PR fortran/113799 gcc/fortran/ChangeLog: * arith.cc (reduce_unary): Remember any overflow encountered during reduction of unary arithmetic operations on array constructors and continue, and return error status, but terminate on serious errors. gcc/testsuite/ChangeLog: * gfortran.dg/arithmetic_overflow_2.f90: New test. --- gcc/fortran/arith.cc| 11 --- gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 | 12 2 files changed, 20 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 diff --git a/gcc/fortran/arith.cc b/gcc/fortran/arith.cc index 0598f6ac51b..d17d1aaa1d9 100644 --- a/gcc/fortran/arith.cc +++ b/gcc/fortran/arith.cc @@ -1323,6 +1323,7 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **), gfc_expr *op, gfc_constructor *c; gfc_expr *r; arith rc; + bool ov = false; if (op->expr_type == EXPR_CONSTANT) return eval (op, result); @@ -1336,13 +1337,17 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **), gfc_expr *op, { rc = reduce_unary (eval, c->expr, &r); - if (rc != ARITH_OK) + /* Remember any overflow encountered during reduction and continue, + but terminate on serious errors. */ + if (rc == ARITH_OVERFLOW) + ov = true; + else if (rc != ARITH_OK) break; gfc_replace_expr (c->expr, r); } - if (rc != ARITH_OK) + if (rc != ARITH_OK && rc != ARITH_OVERFLOW) gfc_constructor_free (head); else { @@ -1363,7 +1368,7 @@ reduce_unary (arith (*eval) (gfc_expr *, gfc_expr **), gfc_expr *op, *result = r; } - return rc; + return ov ? ARITH_OVERFLOW : rc; } diff --git a/gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 b/gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 new file mode 100644 index 000..6ca27f74215 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/arithmetic_overflow_2.f90 @@ -0,0 +1,12 @@ +! { dg-do compile } +! { dg-additional-options "-frange-check" } +! +! PR fortran/113799 - handle arithmetic overflow on unary minus + +program p + implicit none + real, parameter :: inf = real(z'7F80') + real, parameter :: someInf(*) = [inf, 0.] + print *, -someInf ! { dg-error "Arithmetic overflow" } + print *, minval(-someInf) ! { dg-error "Arithmetic overflow" } +end -- 2.35.3
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Hi Alex, I've been unable to apply your patch to my local trunk, likely due to whitespace issues my newsreader handles differently from your site. I see it inline instead of attached. A few general remarks: Please follow the general recommendation regarding style if possible, see https://www.gnu.org/prep/standards/standards.html#Formatting regarding formatting/whitespace use (5.1) and comments (5.2) Also, when an error message text spans multiple lines, please place the whitespace at the end of a line, not at the beginning of the new one: + if ( resolve_bindings_derived->attr.pdt_template && + !gfc_pdt_is_instance_of(resolve_bindings_derived, + CLASS_DATA(me_arg)->ts.u.derived)) +{ + gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of" +" the parametric derived-type %qs", me_arg->name, proc->name, gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of " "the parametric derived-type %qs", me_arg->name, proc->name, +me_arg->name, &where, resolve_bindings_derived->name); + goto error; +} The following change is almost unreadable: the lnegthy comment is split over three parts and almost hides the code. Couldn't this be combined into one comment before the function? diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index fddf68f8398..11f4bac0415 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc @@ -5172,6 +5172,35 @@ gfc_type_is_extension_of (gfc_symbol *t1, gfc_symbol *t2) return gfc_compare_derived_types (t1, t2); } +/* Check if a parameterized derived type t2 is an instance of a PDT template t1 */ + +bool +gfc_pdt_is_instance_of(gfc_symbol *t1, gfc_symbol *t2) +{ + if ( !t1->attr.pdt_template || !t2->attr.pdt_type ) +return false; + + /* +in decl.cc, gfc_get_pdt_instance, a pdt instance is given a 3 character prefix "Pdt", followed +by an underscore list of the kind parameters, up to a maximum of 8. + +So to check if a PDT Type corresponds to the template, extract the core derive_type name, +and then see if it is type compatible by name... + +For example: + +Pdtf_2_2 -> extract out the 'f' -> see if the derived type 'f' is compatible with symbol t1 + */ + + // Starting at index 3 of the string in order to skip past the 'Pdt' prefix + // Also, here the length of the template name is used in order to avoid the + // kind parameter suffixes that are placed at the end of PDT instance names. + if ( !(strncmp(&(t2->name[3]), t1->name, strlen(t1->name)) == 0) ) +return false; + + return true; +} + /* Check if two typespecs are type compatible (F03:5.1.1.2): If ts1 is nonpolymorphic, ts2 must be the same type. The following testcase tests for errors. I tried Intel and NAG on it after commenting the 'contains' section of the type desclaration. Both complained about subroutine deferred_len_param, e.g. Intel: A colon may only be used as a type parameter value in the declaration of an object that has the POINTER or ALLOCATABLE attribute. [THIS] class(param_deriv_type(:)), intent(inout) :: this NAG: Entity THIS of type PARAM_DERIV_TYPE(A=:) has a deferred length type parameter but is not a data pointer or allocatable Do we detect this after your patch? If the answer is yes, can we add another subroutine where we check for this error? (the dg-error suggests we only expect assumed len type parameters.) If no, maybe add a comment in the testcase that this subroutine may need updating later. diff --git a/gcc/testsuite/gfortran.dg/pdt_37.f03 b/gcc/testsuite/gfortran.dg/pdt_37.f03 new file mode 100644 index 000..68d376fad25 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pdt_37.f03 @@ -0,0 +1,34 @@ +! { dg-do compile } +! +! Tests the fixes for PR82943. +! +! This test focuses on the errors produced by incorrect LEN parameters for dummy +! arguments of PDT Typebound Procedures. +! +! Contributed by Alexander Westbrooks +! +module test_len_param + + type :: param_deriv_type(a) + integer, len :: a + contains + procedure :: assumed_len_param ! Good. No error expected. + procedure :: deferred_len_param ! { dg-error "All LEN type parameters of the passed dummy argument" } + procedure :: fixed_len_param ! { dg-error "All LEN type parameters of the passed dummy argument" } + end type + +contains +subroutine assumed_len_param(this) + class(param_deriv_type(*)), intent(inout) :: this +end subroutine + +subroutine deferred_len_param(this) +class(param_deriv_type(:)), intent(inout) :: this +end subroutine + +subroutine fixed_len_param(this) +class(param_deriv_type(10)), intent(inout) :: this +end subroutine + +end module +
[PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Dear all, the attached patch fixes a mis-handling of optional dummy arguments passed to optional dummy arguments of procedures with the bind(c) attribute. When those procedures are expecting CFI descriptors, there is no special treatment like a presence check necessary that by default passes a NULL pointer as default. The testcase tries to exercise various combinations of passing assumed-length character between bind(c) and non-bind(c), which apparently was insufficiently covered in the testsuite. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 87d1b973a4d6a561dc3f3a0c4c10f76d155fa000 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Mon, 12 Feb 2024 21:39:09 +0100 Subject: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866] PR fortran/113866 gcc/fortran/ChangeLog: * trans-expr.cc (gfc_conv_procedure_call): When passing an optional dummy argument to an optional dummy argument of a bind(c) procedure and the dummy argument is passed via a CFI descriptor, no special presence check and passing of a default NULL pointer is needed. gcc/testsuite/ChangeLog: * gfortran.dg/bind_c_optional-2.f90: New test. --- gcc/fortran/trans-expr.cc | 6 +- .../gfortran.dg/bind_c_optional-2.f90 | 104 ++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 67abca9f6ba..a0593b76f18 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -7269,8 +7269,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, with an interface indicating an optional argument. When we call an intrinsic subroutine, however, fsym is NULL, but we might still have an optional argument, so we proceed to the substitution - just in case. */ - if (e && (fsym == NULL || fsym->attr.optional)) + just in case. Arguments passed to bind(c) procedures via CFI + descriptors are handled elsewhere. */ + if (e && (fsym == NULL || fsym->attr.optional) + && !(sym->attr.is_bind_c && is_CFI_desc (fsym, NULL))) { /* If an optional argument is itself an optional dummy argument, check its presence and substitute a null if absent. This is diff --git a/gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 b/gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 new file mode 100644 index 000..b8b4c87775e --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bind_c_optional-2.f90 @@ -0,0 +1,104 @@ +! { dg-do run } +! PR fortran/113866 +! +! Check interoperability of assumed-length character (optional and +! non-optional) dummies between bind(c) and non-bind(c) procedures + +module bindcchar + implicit none + integer, parameter :: n = 100, l = 10 +contains + subroutine bindc_optional (c2, c4) bind(c) +character(*), optional :: c2, c4(n) +! print *, c2(1:3) +! print *, c4(5)(1:3) +if (.not. present (c2) .or. .not. present (c4)) stop 8 +if (c2(1:3)/= "a23") stop 1 +if (c4(5)(1:3) /= "bcd") stop 2 +if (len (c2) /= l .or. len (c4) /= l) stop 81 + end + + subroutine bindc (c2, c4) bind(c) +character(*) :: c2, c4(n) +if (c2(1:3)/= "a23") stop 3 +if (c4(5)(1:3) /= "bcd") stop 4 +if (len (c2) /= l .or. len (c4) /= l) stop 82 +call bindc_optional (c2, c4) + end + + subroutine not_bindc_optional (c1, c3) +character(*), optional :: c1, c3(n) +if (.not. present (c1) .or. .not. present (c3)) stop 5 +call bindc_optional (c1, c3) +call bindc (c1, c3) +if (len (c1) /= l .or. len (c3) /= l) stop 83 + end + + subroutine not_bindc_optional_deferred (c5, c6) +character(:), allocatable, optional :: c5, c6(:) +if (.not. present (c5) .or. .not. present (c6)) stop 6 +call not_bindc_optional (c5, c6) +call bindc_optional (c5, c6) +call bindc (c5, c6) +if (len (c5) /= l .or. len (c6) /= l) stop 84 + end + + subroutine not_bindc_optional2 (c7, c8) +character(*), optional :: c7, c8(:) +if (.not. present (c7) .or. .not. present (c8)) stop 7 +call bindc_optional (c7, c8) +call bindc (c7, c8) +if (len (c7) /= l .or. len (c8) /= l) stop 85 + end + + subroutine bindc_optional2 (c2, c4) bind(c) +character(*), optional :: c2, c4(n) +if (.not. present (c2) .or. .not. present (c4)) stop 8 +if (c2(1:3)/= "a23") stop 9 +if (c4(5)(1:3) /= "bcd") stop 10 +call bindc_optional (c2, c4) +call not_bindc_optional (c2, c4) +if (len (c2) /= l .or. len (c4) /= l) stop 86 + end + + subroutine bindc_optional_missing (c1, c2, c3, c4, c5) bind(c) +character(*), optional :: c1, c2(n), c3(:), c4(..), c5(*) +if (present (c1)) stop 11 +if (present (c2)) stop 12 +if (present (c3)) stop 13 +if (present (c4)) sto
Re: [patch, libgfortran] PR109358
Hi Jerry. Am 12.02.24 um 22:28 schrieb Jerry D: The attached patch fixes this PR by properly adjusting some variables When using stream io. See log below. New test case included. Regression tested on x86_64. OK for trunk and backport? the patch looks good to me. As it is simple and very local, feel free to backport at your discretion. Thanks for the patch! Harald Regards, Jerry ChangeLog: libgfortran: Adjust bytes_left and pos for access="STREAM". During tab edits, the pos (position) and bytes_used Variables were not being set correctly for stream I/O. Since stream I/O does not have 'real' records, the format buffer active length must be used instead of the record length variable. libgfortran/ChangeLog: PR libgfortran/109358 * io/transfer.c (formatted_transfer_scalar_write): Adjust bytes_used and pos variable for stream access. gcc/testsuite/ChangeLog: PR libgfortran/109358 * gfortran.dg/pr109358.f90: New test.
Re: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Hi Steve, Am 13.02.24 um 18:21 schrieb Steve Kargl: On Mon, Feb 12, 2024 at 09:57:08PM +0100, Harald Anlauf wrote: Dear all, the attached patch fixes a mis-handling of optional dummy arguments passed to optional dummy arguments of procedures with the bind(c) attribute. When those procedures are expecting CFI descriptors, there is no special treatment like a presence check necessary that by default passes a NULL pointer as default. The testcase tries to exercise various combinations of passing assumed-length character between bind(c) and non-bind(c), which apparently was insufficiently covered in the testsuite. Regtested on x86_64-pc-linux-gnu. OK for mainline? Yes. Thanks for filling out the more detailed testcase. indeed the new testcase just regressed due to commit r14-8947-g6caec7d9ec37e6 ... :-( Reduced testcase which fails on trunk: program p implicit none integer, parameter :: n = 100, l = 10 character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' character(:), allocatable :: d, e(:) allocate (d, source=a) allocate (e, source=b) print *, len (d), len (e), size (e) call not_bindc_optional_deferred (d, e) deallocate (d, e) contains subroutine not_bindc_optional_deferred (c5, c6) character(:), allocatable, optional :: c5, c6(:) if (.not. present (c5) .or. .not. present (c6)) stop 6 print *, len (c5), len (c6), size (c6) if (len (c5) /= l .or. len (c6) /= l) stop 84 end end Expected: 10 10 100 10 10 100 After above commit: 10 10 100 10 0 100 STOP 84 Will have to wait until the cause is found and fixed...
Re: [PATCH] Fortran: fix passing of optional dummies to bind(c) procedures [PR113866]
Am 13.02.24 um 19:13 schrieb Harald Anlauf: indeed the new testcase just regressed due to commit r14-8947-g6caec7d9ec37e6 ... :-( Reduced testcase which fails on trunk: program p implicit none integer, parameter :: n = 100, l = 10 character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' character(:), allocatable :: d, e(:) allocate (d, source=a) allocate (e, source=b) print *, len (d), len (e), size (e) call not_bindc_optional_deferred (d, e) deallocate (d, e) contains subroutine not_bindc_optional_deferred (c5, c6) character(:), allocatable, optional :: c5, c6(:) if (.not. present (c5) .or. .not. present (c6)) stop 6 print *, len (c5), len (c6), size (c6) if (len (c5) /= l .or. len (c6) /= l) stop 84 end end Expected: 10 10 100 10 10 100 After above commit: 10 10 100 10 0 100 STOP 84 This is now tracked as:: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113911 Will have to wait until the cause is found and fixed... As it is sufficient to disable the deferred-length test, I've done that and pushed the amended patch as https://gcc.gnu.org/g:f4935df217ad89f884f908f39086b322e80123d0 Thanks, Harald
Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
Hi Peter, thanks for your contribution to gfortran! You've found indeed a solution for a potentially annoying bug. Am 15.02.24 um 18:50 schrieb Peter Hill: Dear all, The attached patch fixes PR105658 by forcing an array temporary to be created. This is required when passing an array component, but this didn't happen if the dummy argument was an unlimited polymorphic type. The problem bit of code is in `gfc_conv_expr_descriptor`, near L7828: subref_array_target = (is_subref_array (expr) && (se->direct_byref || expr->ts.type == BT_CHARACTER)); need_tmp = (gfc_ref_needs_temporary_p (expr->ref) && !subref_array_target); where `need_tmp` is being evaluated to 0. The logic here isn't clear to me, and this function is used in several places, which is why I went with setting `parmse.force_tmp = 1` in `gfc_conv_procedure_call` and using the same conditional as the later branch for the non-polymorphic case (near the call to `gfc_conv_subref_array_arg`) If this patch is ok, please could someone commit it for me? This is my first patch for GCC, so apologies in advance if the commit message is missing something. Your patch mostly does the right thing. Note that when fsym is an unlimited polymorphic, some of its attributes are buried deep within its internal representation. I would also prefer to move the code to gfc_conv_intrinsic_to_class where it seems to fit better, like: diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..db906caa52e 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -1019,6 +1019,14 @@ gfc_conv_intrinsic_to_class (gfc_se *parmse, gfc_expr *e, tmp = gfc_typenode_for_spec (&class_ts); var = gfc_create_var (tmp, "class"); + /* Force a temporary for component or substring references. */ + if (unlimited_poly + && class_ts.u.derived->components->attr.dimension + && !class_ts.u.derived->components->attr.class_pointer + && !class_ts.u.derived->components->attr.allocatable + && is_subref_array (e)) +parmse->force_tmp = 1; + /* Set the vptr. */ ctree = gfc_class_vptr_get (var); (I am not entirely sure whether we need to exclude pointer and allocatable attributes here explicitly, given the constraints in F2023:15.5.2.6, but other may have an opinion, too. The above should be safe anyway.) Tested on x86_64-pc-linux-gnu. The bug is present in gfortran back to 4.9, so should it also be backported? I think we'll target 14-mainline and might consider a backport to 13-branch. Cheers, Peter PR fortran/105658 gcc/fortran/ChangeLog * trans-expr.cc (gfc_conv_procedure_call): When passing an array component reference of intrinsic type to a procedure with an unlimited polymorphic dummy argument, a temporary should be created. gcc/testsuite/ChangeLog * gfortran.dg/PR105658.f90: New test. --- gcc/fortran/trans-expr.cc | 8 gcc/testsuite/gfortran.dg/PR105658.f90 | 25 + 2 files changed, 33 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/PR105658.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index a0593b76f18..7fd3047c4e9 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6439,6 +6439,14 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, CLASS object for the unlimited polymorphic formal. */ gfc_find_vtab (&e->ts); gfc_init_se (&parmse, se); + /* The actual argument is a component reference to an array + of derived types, so we need to force creation of a + temporary */ + if (e->expr_type == EXPR_VARIABLE + && is_subref_array (e) + && !(fsym && fsym->attr.pointer)) + parmse.force_tmp = 1; + gfc_conv_intrinsic_to_class (&parmse, e, fsym->ts); } diff --git a/gcc/testsuite/gfortran.dg/PR105658.f90 b/gcc/testsuite/gfortran.dg/PR105658.f90 new file mode 100644 index 000..407ee25f77c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/PR105658.f90 @@ -0,0 +1,25 @@ +! { dg-do compile } +! { dg-options "-Warray-temporaries" } +! Test fix for incorrectly passing array component to unlimited polymorphic procedure + +module test_PR105658_mod + implicit none + type :: foo +integer :: member1 +integer :: member2 + end type foo +contains + subroutine print_poly(array) +class(*), dimension(:), intent(in) :: array +select type(array) +type is (integer) + print*, array +end select + end subroutine print_poly + + subroutine do_print(thing) +type(foo), dimension(3), intent(in) :: thing +call print_poly(thing%member1) ! { dg-warning "array temporary" } + end subroutine do_print + +end module test_PR105658_mod One could extend this testcase to cover substrings as well: module test_PR105658_mod implicit none type :: foo integer :: member1 integer :: member2 end type foo contains subroutine print_poly(arra
[PATCH] Fortran: deferred length of character variables shall not get lost [PR113911]
Dear all, this patch fixes a regression which was a side-effect of r14-8947, losing the length of a deferred-length character variable when passed as a dummy. The new testcase provides a workout for deferred length to improve coverage in the testsuite. Another temporarily disabled test was re-enabled. Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From 07fcdf7c9f9272d8e4752c23f04795d02d4ad440 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 16 Feb 2024 22:33:16 +0100 Subject: [PATCH] Fortran: deferred length of character variables shall not get lost [PR113911] PR fortran/113911 gcc/fortran/ChangeLog: * trans-array.cc (gfc_trans_deferred_array): Do not clobber deferred length for a character variable passed as dummy argument. gcc/testsuite/ChangeLog: * gfortran.dg/allocatable_length_2.f90: New test. * gfortran.dg/bind_c_optional-2.f90: Enable deferred-length test. --- gcc/fortran/trans-array.cc| 2 +- .../gfortran.dg/allocatable_length_2.f90 | 107 ++ .../gfortran.dg/bind_c_optional-2.f90 | 3 +- 3 files changed, 109 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/allocatable_length_2.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 2181990aa04..3673fa40720 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -11531,7 +11531,7 @@ gfc_trans_deferred_array (gfc_symbol * sym, gfc_wrapped_block * block) if (sym->ts.type == BT_CHARACTER && !INTEGER_CST_P (sym->ts.u.cl->backend_decl)) { - if (sym->ts.deferred && !sym->ts.u.cl->length) + if (sym->ts.deferred && !sym->ts.u.cl->length && !sym->attr.dummy) gfc_add_modify (&init, sym->ts.u.cl->backend_decl, build_zero_cst (TREE_TYPE (sym->ts.u.cl->backend_decl))); gfc_conv_string_length (sym->ts.u.cl, NULL, &init); diff --git a/gcc/testsuite/gfortran.dg/allocatable_length_2.f90 b/gcc/testsuite/gfortran.dg/allocatable_length_2.f90 new file mode 100644 index 000..2fd64efdc25 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocatable_length_2.f90 @@ -0,0 +1,107 @@ +! { dg-do run } +! PR fortran/113911 +! +! Test that deferred length is not lost + +module m + integer, parameter:: n = 100, l = 10 + character(l) :: a = 'a234567890', b(n) = 'bcdefghijk' + character(:), allocatable :: c1, c2(:) +end + +program p + use m, only : l, n, a, b, x => c1, y => c2 + implicit none + character(:), allocatable :: d, e(:) + allocate (d, source=a) + allocate (e, source=b) + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 12 + call plain_deferred (d, e) + call optional_deferred (d, e) + call optional_deferred_ar (d, e) + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 13 + deallocate (d, e) + call alloc (d, e) + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 14 + deallocate (d, e) + call alloc_host_assoc () + if (len (d) /= l .or. len (e) /= l .or. size (e) /= n) stop 15 + deallocate (d, e) + call alloc_use_assoc () + if (len (x) /= l .or. len (y) /= l .or. size (y) /= n) stop 16 + call indirect (x, y) + if (len (x) /= l .or. len (y) /= l .or. size (y) /= n) stop 17 + deallocate (x, y) +contains + subroutine plain_deferred (c1, c2) +character(:), allocatable :: c1, c2(:) +if (.not. allocated (c1) .or. .not. allocated (c2)) stop 1 +if (len (c1) /= l) stop 2 +if (len (c2) /= l) stop 3 +if (c1(1:3)/= "a23") stop 4 +if (c2(5)(1:3) /= "bcd") stop 5 + end + + subroutine optional_deferred (c1, c2) +character(:), allocatable, optional :: c1, c2(:) +if (.not. present (c1) .or. .not. present (c2)) stop 6 +if (.not. allocated (c1) .or. .not. allocated (c2)) stop 7 +if (len (c1) /= l) stop 8 +if (len (c2) /= l) stop 9 +if (c1(1:3)/= "a23") stop 10 +if (c2(5)(1:3) /= "bcd") stop 11 + end + + ! Assumed rank + subroutine optional_deferred_ar (c1, c2) +character(:), allocatable, optional :: c1(..) +character(:), allocatable, optional :: c2(..) +if (.not. present (c1) .or. & +.not. present (c2)) stop 21 +if (.not. allocated (c1) .or. & +.not. allocated (c2)) stop 22 + +select rank (c1) +rank (0) +if (len (c1) /= l) stop 23 + if (c1(1:3) /= "a23") stop 24 +rank default + stop 25 +end select + +select rank (c2) +rank (1) + if (len (c2) /= l) stop 26 + if (c2(5)(1:3) /= "bcd") stop 27 +rank default + stop 28 +end select + end + + ! Allocate dummy arguments + subroutine alloc (c1, c2) +character(:), allocatable :: c1, c2(:) +allocate (c1, source=a) +allocate (c2, source=b) + end + + ! Allocate host-associated variables + subroutine allo
Re: [PATCH] fortran: gfc_trans_subcomponent_assign fixes [PR113503]
Hi Jakub, On 2/17/24 10:02, Jakub Jelinek wrote: Hi! The r14-870 changes broke xtb package tests (reduced testcase is the first one below) and caused ICEs on a test derived from that (the second one). [...] thanks for your detailed analysis and for the patch, which puts things in straight order to actually fix two issues here! Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK for trunk, except for the minor nit below. --- gcc/testsuite/gfortran.dg/pr113503_1.f90.jj 2024-02-16 14:16:17.937153094 +0100 +++ gcc/testsuite/gfortran.dg/pr113503_1.f902024-02-16 14:16:10.124258815 +0100 @@ -0,0 +1,18 @@ +! PR fortran/113503 +! { dg-do compile } +! { dg-options "-O2 -fno-inline -Wuninitialized" } + +program pr113503 + implicit none + type :: T +character(len=:), allocatable :: u + end type + character(len=20) :: us(1) = 'foobar' + type(T) :: x + x = T(u = trim (us(1))) ! { dg-bogus "is used uninitialized" } tab here not allowed in Fortran My newsreader shows a tab here, giving a warning when running the test. Also, applying your patch on top of r14-9045 I do not see the uninitialized warning, which could have been fixed by r14-8947. Please recheck and adjust accordingly. + call foo +contains + subroutine foo +if (x%u /= 'foobar') stop 1 + end subroutine +end Thanks, Harald
Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
Hi Peter, On 2/19/24 16:19, Peter Hill wrote: Hi Harald, Thanks for your help, please see the updated and signed-off patch below. great! This is fine, and I'll commit it tomorrow unless others have further comments. It also occurred to me that array temporaries aren't _required_ here (for arrays of derived type components), but in the general case with a type with differently sized components, the stride wouldn't be a multiple of the component's type's size. Is it possible in principle to have an arbitrary stride? It is possible to have an arbitrary (fixed, non-unit) stride, but it is not always taken advantage of. If you take the last version of the testcase and compile with option -fdump-tree-original, you can see that the cases commented with "no temp needed" actually create a suitable descriptor. E.g. call print_poly (uu(2,2::2)) becomes: { struct __class__STAR_1_0t class.28; struct array01_integer(kind=4) parm.29; class.28._vptr = (struct __vtype__STAR * {ref-all}) &__vtab_INTEGER_4_; parm.29.span = 4; parm.29.dtype = {.elem_len=4, .version=0, .rank=1, .type=1}; parm.29.dim[0].lbound = 1; parm.29.dim[0].ubound = 3; parm.29.dim[0].stride = 10; parm.29.data = (void *) &uu[6]; parm.29.offset = -10; class.28._data = parm.29; class.28._len = 0; print_poly (&class.28); } Since we know that 'uu' is a contiguous array, we can calculate the stride (10) for the 1-d section. The case of the section of the character array is quite similar, but the variant with the substring reference would need further work to avoid the temporary. (It would be possible.) But as you say, the general case, which may involve types/classes, does not map to a simple descriptor. Thanks for your patch! Harald
Re: [PATCH] Fortran: fix passing array component to polymorphic argument [PR105658]
On 2/19/24 16:19, Peter Hill wrote: Hi Harald, Thanks for your help, please see the updated and signed-off patch below. Pushed: https://gcc.gnu.org/g:14ba8d5b87acd5f91ab8b8c02165a0fd53dcc2f2
Re: [PATCH] Fix fortran/PR114024
On 2/21/24 20:41, Jerry D wrote: On 2/21/24 10:30 AM, Steve Kargl wrote: I have attached a patch to PR114024, see https://gcc.gnu.org/pipermail/gcc-bugs/2024-February/854651.html The patch contains a new testcase and passes regression testing on x86_64-*-freebsd. Could someone castr an eye over the patch and commit it? Hi Steve, I looked it over and looks reasonable. I will try to apply it next few days and test here. If OK, I will commit. Jerry Actually the patch has two issues: - a minor one: a new front-end memleak which can be avoided by using either gfc_replace_expr (see its other uses) Hint: try valgrind on f951 - it still fails on the following code, because the traversal of the refs is incomplete / wrong: program foo implicit none complex :: cmp(3) real, pointer :: pp(:) class(*), allocatable :: uu(:) type t real :: re real :: im end type t type u type(t) :: tt(3) end type u type(u) :: cc cmp = (3.45,6.78) cc% tt% re = cmp% re cc% tt% im = cmp% im allocate (pp, source = cc% tt% im) ! ICE print *, pp allocate (uu, source = cc% tt% im) ! ICE end This still crashes for me for the indicated cases. Harald
Re: [PATCH] Fix fortran/PR114024
On 2/21/24 22:00, Steve Kargl wrote: Unfortunately, valgrind does not work on AMD FX-8350 cpu. Do you mean valgrind does not work at all? For gcc, you need to configure --enable-valgrind-annotations to not get bogus warnings. memleak vs ICE. I think I'll take one over the other. Probably need to free code->expr3 before the copy. Yep. I tried gfc_replace_expr in an earlier patch. It did not work. - it still fails on the following code, because the traversal of the refs is incomplete / wrong: program foo implicit none complex :: cmp(3) real, pointer :: pp(:) class(*), allocatable :: uu(:) type t real :: re real :: im end type t type u type(t) :: tt(3) end type u type(u) :: cc cmp = (3.45,6.78) cc% tt% re = cmp% re cc% tt% im = cmp% im allocate (pp, source = cc% tt% im) ! ICE cc%tt%im isn't a complex-part-ref, so this seems to be a different (maybe related) issue. Does the code compile with 'source = (cc%tt%im)'? If so, perhaps, detecting a component reference and doing the simply wrapping with parentheses can be done. Yes, that's why I tried to make up the above example. I think %re and %im are not too special, they work here pretty much like component refs elsewhere. print *, pp allocate (uu, source = cc% tt% im) ! ICE Ditto. Not to mention I know nothing about the implementation of CLASS in gfortran. You can ignore this one for now. It works if one places parens around the source expr as for the other cases. Harald
Re: [PATCH] Fix fortran/PR114024
Hi Steve! On 2/22/24 01:52, Steve Kargl wrote: On Wed, Feb 21, 2024 at 01:42:32PM -0800, Steve Kargl wrote: On Wed, Feb 21, 2024 at 10:20:43PM +0100, Harald Anlauf wrote: On 2/21/24 22:00, Steve Kargl wrote: memleak vs ICE. I think I'll take one over the other. Probably need to free code->expr3 before the copy. Yep. I tried gfc_replace_expr in an earlier patch. It did not work. I tried freeing code->expr3 before assigning the new expression. That leads to % gfcx -c ~/gcc/gccx/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 pid 69473 comm f951 has trashed its stack, killing gfortran: internal compiler error: Illegal instruction signal terminated program f951 Right. I also don't see what the lifetimes of the expressions are. But is the gfc_copy_expr really needed? Wouldn't the following suffice? code->expr3 = gfc_get_parentheses (code->expr3); If I don't free code->expr3 but simply assign the new expression from gfc_get_parentheses(), your example now compiles are executes are expected. It now allocate_with_source_28.f90. Caveat: I don't know how to test the CLASS uu. - it still fails on the following code, because the traversal of the refs is incomplete / wrong: program foo implicit none complex :: cmp(3) real, pointer :: pp(:) class(*), allocatable :: uu(:) type t real :: re real :: im end type t type u type(t) :: tt(3) end type u type(u) :: cc cmp = (3.45,6.78) cc% tt% re = cmp% re cc% tt% im = cmp% im allocate (pp, source = cc% tt% im) ! ICE cc%tt%im isn't a complex-part-ref, so this seems to be a different (maybe related) issue. Does the code compile with 'source = (cc%tt%im)'? If so, perhaps, detecting a component reference and doing the simply wrapping with parentheses can be done. Yes, that's why I tried to make up the above example. I think %re and %im are not too special, they work here pretty much like component refs elsewhere. I see. The %re and %im complex-part-ref correspond to ref->u.i == INQUIRY_RE and INQUIRY_IM, respectively. A part-ref for a user-defined type doesn't have an INQUIRY_xxx, so we'll need to see if there is a way to easily identify, e.g., cc%tt%re from your testcase. The attach patch uses ref->type == REF_COMPONENT to deal with the above code. I actually wanted to draw your attention away from the real/complex stuff, because that is not really the point. When do we actually need to enforce the parentheses? I tried the following, and it seems to work: if (code->expr3->expr_type == EXPR_VARIABLE && is_subref_array (code->expr3)) code->expr3 = gfc_get_parentheses (code->expr3); (Beware: this is not regtested!) On the positive side, it not only seems to fix the cases in question, but also substring references etc., like the following: program foo implicit none complex :: cmp(3) = (3.45,6.78) real, pointer :: pp(:) integer, allocatable :: aa(:) class(*), allocatable :: uu(:), vv(:) type t ! pseudo "complex" type real :: re real :: im end type t type ci ! "complex integer" type integer :: re integer :: im end type ci type u type(t) :: tt(3) type(ci) :: ii(3) end type u type(u) :: cc character(3) :: str(3) = ["abc","def","ghi"] character(:), allocatable :: ac(:) allocate (ac, source=str(1::2)(2:3)) print *, str(1::2)(2:3) call my_print (ac) cc% tt% re = cmp% re cc% tt% im = cmp% im cc% ii% re = nint (cmp% re) cc% ii% im = nint (cmp% im) print *, "re=", cc% tt% re print *, "im=", cc% tt% im allocate (pp, source = cc% tt% re) print *, pp allocate (uu, source = cc% tt% im) call my_print (uu) allocate (vv, source = cc% ii% im) call my_print (vv) contains subroutine my_print (x) class(*), intent(in) :: x(:) select type (x) type is (real) print *, "'real':", x type is (integer) print *, "'integer':", x type is (character(*)) print *, "'character':", x end select end subroutine my_print end Cheers, Harald
Re: [PATCH] Fix fortran/PR114024
On 2/22/24 22:01, Steve Kargl wrote: On Thu, Feb 22, 2024 at 09:22:37PM +0100, Harald Anlauf wrote: On the positive side, it not only seems to fix the cases in question, but also substring references etc., like the following: If the above passes a regression test, then by all means we should use it. I did not consider the substring case. Even if unneeded parentheses are inserted, which may cause generation of a temporary variable, I hope users are not using 'allocate(x,source=z%re)' is some deeply nested crazy loops structure. First thing is code correctness. There are cases where the allocation shall preserve the array bounds, which is where we must avoid the parentheses at all cost. But these cases should be very limited. (There are some code comments/TODOs regarding this and an open PR by Tobias(?)). The cases we are currently discussing are even requiring(!) the resetting of the lower bounds to 1, so your suggestion to enforce parentheses does not look unreasonable. BTW: If someone uses allocate in a tight loop, he/she deserves to be punished anyway... BTW, my patch and I suspect your improved patch also fixes 'allocate(x,mold=z%re)'. Consider, complex z(3) real, allocatable :: x(:) z = 42ha allocate(x, mold=z%re) print *, size(x) end % gfortran13 -o z a.f90 a.f90:9:25: 9 |allocate(x, mold=z%re) | 1 internal compiler error: in retrieve_last_ref, at fortran/trans-array.cc:6070 0x247d7a679 __libc_start1 /usr/src/lib/libc/csu/libc_start1.c:157 % gfcx -o z a.f90 && ./z 3 Nice! I completely forgot about MOLD... So the only missing pieces are a really comprehensive testcase and successful regtests... Cheers, Harald
[PATCH, v2] Fix fortran/PR114024
Hi Steve, all, here's an updated patch with an enhanced testcase that also checks MOLD= besides SOURCE=. Regtested on x86_64-pc-linux-gnu. Is it OK for mainline? Cheers, Harald On 2/22/24 22:32, Harald Anlauf wrote: On 2/22/24 22:01, Steve Kargl wrote: BTW, my patch and I suspect your improved patch also fixes 'allocate(x,mold=z%re)'. Consider, complex z(3) real, allocatable :: x(:) z = 42ha allocate(x, mold=z%re) print *, size(x) end % gfortran13 -o z a.f90 a.f90:9:25: 9 | allocate(x, mold=z%re) | 1 internal compiler error: in retrieve_last_ref, at fortran/trans-array.cc:6070 0x247d7a679 __libc_start1 /usr/src/lib/libc/csu/libc_start1.c:157 % gfcx -o z a.f90 && ./z 3 Nice! I completely forgot about MOLD... So the only missing pieces are a really comprehensive testcase and successful regtests... From a176c2f44f812d82aeb430fadf23ab4b6dd5bd65 Mon Sep 17 00:00:00 2001 From: Steve Kargl Date: Fri, 23 Feb 2024 22:05:04 +0100 Subject: [PATCH] Fortran: ALLOCATE statement, SOURCE/MOLD expressions with subrefs [PR114024] PR fortran/114024 gcc/fortran/ChangeLog: * trans-stmt.cc (gfc_trans_allocate): When a source expression has substring references, part-refs, or %re/%im inquiries, wrap the entity in parentheses to force evaluation of the expression. gcc/testsuite/ChangeLog: * gfortran.dg/allocate_with_source_27.f90: New test. * gfortran.dg/allocate_with_source_28.f90: New test. Co-Authored-By: Harald Anlauf --- gcc/fortran/trans-stmt.cc | 10 ++- .../gfortran.dg/allocate_with_source_27.f90 | 20 + .../gfortran.dg/allocate_with_source_28.f90 | 90 +++ 3 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 create mode 100644 gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc index 5247d3d39d7..e09828e218b 100644 --- a/gcc/fortran/trans-stmt.cc +++ b/gcc/fortran/trans-stmt.cc @@ -6355,8 +6355,14 @@ gfc_trans_allocate (gfc_code * code, gfc_omp_namelist *omp_allocate) vtab_needed = (al->expr->ts.type == BT_CLASS); gfc_init_se (&se, NULL); - /* When expr3 is a variable, i.e., a very simple expression, - then convert it once here. */ + /* When expr3 is a variable, i.e., a very simple expression, then + convert it once here. If one has a source expression that has + substring references, part-refs, or %re/%im inquiries, wrap the + entity in parentheses to force evaluation of the expression. */ + if (code->expr3->expr_type == EXPR_VARIABLE + && is_subref_array (code->expr3)) + code->expr3 = gfc_get_parentheses (code->expr3); + if (code->expr3->expr_type == EXPR_VARIABLE || code->expr3->expr_type == EXPR_ARRAY || code->expr3->expr_type == EXPR_CONSTANT) diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 new file mode 100644 index 000..d0f0f3c4a84 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocate_with_source_27.f90 @@ -0,0 +1,20 @@ +! +! { dg-do run } +! +! fortran/PR114024 +! https://github.com/fujitsu/compiler-test-suite +! Modified from Fortran/0093/0093_0130.f90 +! +program foo + implicit none + complex :: cmp(3) + real, allocatable :: xx(:), yy(:), zz(:) + cmp = (3., 6.78) + allocate(xx, source = cmp%re) ! This caused an ICE. + allocate(yy, source = cmp(1:3)%re) ! This caused an ICE. + allocate(zz, source = (cmp%re)) + if (any(xx /= [3., 3., 3.])) stop 1 + if (any(yy /= [3., 3., 3.])) stop 2 + if (any(zz /= [3., 3., 3.])) stop 3 +end program foo + diff --git a/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 new file mode 100644 index 000..976c567cf22 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/allocate_with_source_28.f90 @@ -0,0 +1,90 @@ +! { dg-do run } +! +! PR fortran/114024 + +program foo + implicit none + complex :: cmp(3) = (3.,4.) + type ci ! pseudo "complex integer" type + integer :: re + integer :: im + end type ci + type cr ! pseudo "complex" type + real :: re + real :: im + end type cr + type u + type(ci) :: ii(3) + type(cr) :: rr(3) + end type u + type(u) :: cc + + cc% ii% re = nint (cmp% re) + cc% ii% im = nint (cmp% im) + cc% rr% re = cmp% re + cc% rr% im = cmp% im + + call test_substring () + call test_int_real () + call test_poly () + +contains + + subroutine test_substring () +character(4) :: str(3) = ["abcd","efgh","ijkl"] +character(:), allocatable :: ac(:) +allocate (ac, source=str(1::2)(2:4)) +if (size (ac) /= 2 .or. len (ac) /= 3) stop 11 +if (ac(
[PATCH] Fortran: do not evaluate polymorphic functions twice in assignment [PR114012]
Dear all, the attached simple patch fixes an issue where we evaluated polymorphic functions twice in assignments: once for the _data component, and once for the _vptr. Using save_expr prevents the double evaluation. Regtested on x86_64-pc-linux-gnu. OK for mainline? And a backport to 13-branch after some delay? Thanks, Harald From 7a16143448ee21b716b54a94f83f9ee477af1b63 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sun, 25 Feb 2024 21:18:23 +0100 Subject: [PATCH] Fortran: do not evaluate polymorphic functions twice in assignment [PR114012] PR fortran/114012 gcc/fortran/ChangeLog: * trans-expr.cc (gfc_conv_procedure_call): Evaluate non-trivial arguments just once before assigning to an unlimited polymorphic dummy variable. gcc/testsuite/ChangeLog: * gfortran.dg/pr114012.f90: New test. --- gcc/fortran/trans-expr.cc | 4 ++ gcc/testsuite/gfortran.dg/pr114012.f90 | 81 ++ 2 files changed, 85 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/pr114012.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 118dfd7c9b2..d63c304661a 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6691,6 +6691,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, { tree efield; + /* Evaluate arguments just once. */ + if (e->expr_type != EXPR_VARIABLE) +parmse.expr = save_expr (parmse.expr); + /* Set the _data field. */ tmp = gfc_class_data_get (var); efield = fold_convert (TREE_TYPE (tmp), diff --git a/gcc/testsuite/gfortran.dg/pr114012.f90 b/gcc/testsuite/gfortran.dg/pr114012.f90 new file mode 100644 index 000..9dbb031c664 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pr114012.f90 @@ -0,0 +1,81 @@ +! { dg-do run } +! PR fortran/114012 +! +! Polymorphic functions were evaluated twice in assignment + +program test + implicit none + + type :: custom_int + integer :: val = 2 + end type + + interface assignment(=) + procedure assign + end interface + interface operator(-) + procedure neg + end interface + + type(custom_int) :: i + integer :: count_assign, count_neg + + count_assign = 0 + count_neg= 0 + + i = 1 + if (count_assign /= 1 .or. count_neg /= 0) stop 1 + + i = -i + if (count_assign /= 2 .or. count_neg /= 1) stop 2 + if (i% val /= -1) stop 3 + + i = neg(i) + if (count_assign /= 3 .or. count_neg /= 2) stop 4 + if (i% val /= 1) stop 5 + + i = (neg(i)) + if (count_assign /= 4 .or. count_neg /= 3) stop 6 + if (i% val /= -1) stop 7 + + i = - neg(i) + if (count_assign /= 5 .or. count_neg /= 5) stop 8 + if (i% val /= -1) stop 9 + +contains + + subroutine assign (field, val) +type(custom_int), intent(out) :: field +class(*), intent(in) :: val + +count_assign = count_assign + 1 + +select type (val) +type is (integer) +! print *, " in assign(integer)", field%val, val + field%val = val +type is (custom_int) +! print *, " in assign(custom)", field%val, val%val + field%val = val%val +class default + error stop +end select + + end subroutine assign + + function neg (input_field) result(output_field) +type(custom_int), intent(in), target :: input_field +class(custom_int), allocatable :: output_field +allocate (custom_int :: output_field) + +count_neg = count_neg + 1 + +select type (output_field) +type is (custom_int) +! print *, " in neg", output_field%val, input_field%val + output_field%val = -input_field%val +class default + error stop +end select + end function neg +end program test -- 2.35.3
Re: [patch, libgfortran] PR105456 Child I/O does not propage iostat
Hi Jerry, On 2/22/24 20:11, Jerry D wrote: Hi all, The attached fix adds a check for an error condition from a UDDTIO procedure in the case where there is no actual underlying error, but the user defines an error by setting the iostat variable manually before returning to the parent READ. the libgfortran fix LGTM. Regarding the testcase code, the following looks like you left some debugging code in it: + rewind (10) + read (10,*) x + print *, myerror, mymessage + write (*,'(10(A))') "Read: '",x%ch,"'" myerror and mymessage are never set and never tested. I suggest to either remove them or to enhance the testcase e.g. like rewind (10) read (10,*,iostat=myerror,iomsg=mymessage) x if (myerror /= 42 .or. mymessage /= "The users message") stop 1 rewind (10) read (10,*) x write (*,'(10(A))') "Read: '",x%ch,"'" I'll leave that up to you. I did not address the case of a formatted WRITE or unformatted READ/WRITE until I get some feedback on the approach. If this approach is OK I would like to commit and then do a separate patch for the cases I just mentioned. I haven't thought about this long enough, but I do not anything wrong with your patch. Feedback appreciated. Regression tested on x86_64. OK for trunk? This is OK. Thanks, Harald Jerry Author: Jerry DeLisle Date: Thu Feb 22 10:48:39 2024 -0800 libgfortran: Propagate user defined iostat and iomsg. PR libfortran/105456 libgfortran/ChangeLog: * io/list_read.c (list_formatted_read_scalar): Add checks for the case where a user defines their own error codes and error messages and generate the runtime error. gcc/testsuite/ChangeLog: * gfortran.dg/pr105456.f90: New test.
[PATCH] Fortran testsuite: fix invalid Fortran in testcase
Dear all, the attached patch fixes invalid Fortran in testcase gfortran.dg/pr101026.f, which might prohibit progress in fixing pr111781. (Note that the testcase was for a tree-optimizer issue, not the Fortran frontend.) OK for mainline? Will commit within 24h unless there are comments. Thanks, Harald From 75724b6b42a1c46383d8e6deedbfb8d2ebd0fa12 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Tue, 27 Feb 2024 21:51:53 +0100 Subject: [PATCH] Fortran testsuite: fix invalid Fortran in testcase gcc/testsuite/ChangeLog: * gfortran.dg/pr101026.f: Let variables used in specification expression be passed as dummy arguments --- gcc/testsuite/gfortran.dg/pr101026.f | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/gfortran.dg/pr101026.f b/gcc/testsuite/gfortran.dg/pr101026.f index 9576d8802ca..e05e21c898a 100644 --- a/gcc/testsuite/gfortran.dg/pr101026.f +++ b/gcc/testsuite/gfortran.dg/pr101026.f @@ -1,6 +1,6 @@ ! { dg-do compile } ! { dg-options "-Ofast -frounding-math" } - SUBROUTINE PASSB4 (CC,CH) + SUBROUTINE PASSB4 (CC,CH,IDO,L1) DIMENSION CC(IDO,4,L1), CH(IDO,L1,*) DO 103 I=2,IDO,2 TI4 = CC0-CC(I,4,K) -- 2.35.3
Re: [PATCH] Fortran - Error compiling PDT Type-bound Procedures [PR82943/86148/86268]
Hi Alex, this is now mostly correct, with the following exceptions: First, you should notice that the formatting of the commit message, when checked using "git gcc-verify", needs minor corrections. You will be guided how to fix this yourself. Second, testcase pdt_37.f03 has an undeclared dummy argument, which can be detected by adding "implicit none" (I usually use that whenever implicit typing is not wanted explicitly). I would get: pdt_37.f03:33:47: 33 | subroutine assumed_len_param_ptr(this, that) | 1 Error: Symbol 'that' at (1) has no IMPLICIT type; did you mean 'this'? I assume you want to uncomment the declaration of dummy 'that'. Third, I still see a - minor - indentation/tabbing/space issue here: diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc index 44f89f6afb4..852e0820e6a 100644 --- a/gcc/fortran/resolve.cc +++ b/gcc/fortran/resolve.cc [...] + if ( resolve_bindings_derived->attr.pdt_template + && gfc_pdt_is_instance_of (resolve_bindings_derived, + CLASS_DATA (me_arg)->ts.u.derived) + && (me_arg->param_list != NULL) + && (gfc_spec_list_type (me_arg->param_list, +CLASS_DATA(me_arg)->ts.u.derived) +!= SPEC_ASSUMED)) OK with the above fixed. Thanks for the patch! Harald On 2/28/24 07:24, Alexander Westbrooks wrote: Harald, Jerry helped me figure out my editor settings so that I could fix whitespace and formatting issues in my code. With my editor configured correctly, I saw that my code was not conforming to coding standards as I previously thought it was. I have fixed those things and updated my patch. Thank you for your patience. Let me know if this is okay to push to the trunk. Thanks, Alexander Westbrooks On Sun, Feb 25, 2024 at 2:40 PM Alexander Westbrooks wrote: Harald, Thank you for reviewing my code. I've been doing research and debugging to investigate the error thrown by Intel and NAG for the deferred parameter in the dummy variable declaration. I found where the problem was and added the fix as part of my patch. I've attached the patch as a file, which also includes your feedback and suggested fixes. I've updated the test case pdt_37.f03 to check for the POINTER or ALLOCATABLE error as you suggested. All regression tests pass, including the new ones, after including the fix for the POINTER or ALLOCATABLE error for CLASS declarations of PDTs when deferred length parameters are used. This was tested on WSL 2, with Ubuntu 20.04 distro. Is this okay to push to the trunk? Thanks, Alexander Westbrooks On Sun, Feb 11, 2024 at 2:11 PM Harald Anlauf wrote: Hi Alex, I've been unable to apply your patch to my local trunk, likely due to whitespace issues my newsreader handles differently from your site. I see it inline instead of attached. A few general remarks: Please follow the general recommendation regarding style if possible, see https://www.gnu.org/prep/standards/standards.html#Formatting regarding formatting/whitespace use (5.1) and comments (5.2) Also, when an error message text spans multiple lines, please place the whitespace at the end of a line, not at the beginning of the new one: + if ( resolve_bindings_derived->attr.pdt_template && + !gfc_pdt_is_instance_of(resolve_bindings_derived, + CLASS_DATA(me_arg)->ts.u.derived)) +{ + gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of" +" the parametric derived-type %qs", me_arg->name, proc->name, gfc_error ("Argument %qs of %qs with PASS(%s) at %L must be of " "the parametric derived-type %qs", me_arg->name, proc->name, +me_arg->name, &where, resolve_bindings_derived->name); + goto error; +} The following change is almost unreadable: the lnegthy comment is split over three parts and almost hides the code. Couldn't this be combined into one comment before the function? diff --git a/gcc/fortran/symbol.cc b/gcc/fortran/symbol.cc index fddf68f8398..11f4bac0415 100644 --- a/gcc/fortran/symbol.cc +++ b/gcc/fortran/symbol.cc @@ -5172,6 +5172,35 @@ gfc_type_is_extension_of (gfc_symbol *t1, gfc_symbol *t2) return gfc_compare_derived_types (t1, t2); } +/* Check if a parameterized derived type t2 is an instance of a PDT template t1 */ + +bool +gfc_pdt_is_instance_of(gfc_symbol *t1, gfc_symbol *t2) +{ + if ( !t1->attr.pdt_template || !t2->attr.pdt_type ) +return false; + + /* +in decl.cc, gfc_get_pdt_instance, a pdt instance is given a 3 character prefix "Pdt", followed +by an underscore list of the kind parameters, up to a maximum of 8. + +So to check if a PDT Type
[PATCH] Fortran: improve checks of NULL without MOLD as actual argument [PR104819]
Dear all, here's a first patch addressing issues with NULL as actual argument: if the dummy is assumed-rank or assumed length, MOLD shall be present. There is also an interp on interoperability of c_sizeof and NULL pointers, for which we have a partially incorrect testcase (gfortran.dg/pr101329.f90) which gets fixed. See https://j3-fortran.org/doc/year/22/22-101r1.txt for more. Furthermore, nested NULL()s are now handled. Regtested on x86_64-pc-linux-gnu. OK for mainline? I consider this part as safe and would like to backport to 13-branch. Objections? Thanks, Harald From ce7199b16872b3014be68744329a8f19ddd64b05 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Thu, 29 Feb 2024 21:43:53 +0100 Subject: [PATCH] Fortran: improve checks of NULL without MOLD as actual argument [PR104819] gcc/fortran/ChangeLog: PR fortran/104819 * check.cc (gfc_check_null): Handle nested NULL()s. (is_c_interoperable): Check for MOLD argument of NULL() as part of the interoperability check. * interface.cc (gfc_compare_actual_formal): Extend checks for NULL() actual arguments for presence of MOLD argument when required by Interp J3/22-146. gcc/testsuite/ChangeLog: PR fortran/104819 * gfortran.dg/pr101329.f90: Adjust testcase to conform to interp. * gfortran.dg/null_actual_4.f90: New test. --- gcc/fortran/check.cc| 5 ++- gcc/fortran/interface.cc| 30 ++ gcc/testsuite/gfortran.dg/null_actual_4.f90 | 35 + gcc/testsuite/gfortran.dg/pr101329.f90 | 4 +-- 4 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/null_actual_4.f90 diff --git a/gcc/fortran/check.cc b/gcc/fortran/check.cc index d661cf37f01..db74dcf3f40 100644 --- a/gcc/fortran/check.cc +++ b/gcc/fortran/check.cc @@ -4384,6 +4384,9 @@ gfc_check_null (gfc_expr *mold) if (mold == NULL) return true; + if (mold->expr_type == EXPR_NULL) +return true; + if (!variable_check (mold, 0, true)) return false; @@ -5216,7 +5219,7 @@ is_c_interoperable (gfc_expr *expr, const char **msg, bool c_loc, bool c_f_ptr) { *msg = NULL; - if (expr->expr_type == EXPR_NULL) + if (expr->expr_type == EXPR_NULL && expr->ts.type == BT_UNKNOWN) { *msg = "NULL() is not interoperable"; return false; diff --git a/gcc/fortran/interface.cc b/gcc/fortran/interface.cc index 231f2f252af..64b90550be2 100644 --- a/gcc/fortran/interface.cc +++ b/gcc/fortran/interface.cc @@ -3296,6 +3296,36 @@ gfc_compare_actual_formal (gfc_actual_arglist **ap, gfc_formal_arglist *formal, && a->expr->ts.type != BT_ASSUMED) gfc_find_vtab (&a->expr->ts); + /* Interp J3/22-146: + "If the context of the reference to NULL is an + corresponding to an dummy argument, MOLD shall be + present." */ + if (a->expr->expr_type == EXPR_NULL + && a->expr->ts.type == BT_UNKNOWN + && f->sym->as + && f->sym->as->type == AS_ASSUMED_RANK) + { + gfc_error ("Intrinsic % without % argument at %L " + "passed to assumed-rank dummy %qs", + &a->expr->where, f->sym->name); + ok = false; + goto match; + } + + if (a->expr->expr_type == EXPR_NULL + && a->expr->ts.type == BT_UNKNOWN + && f->sym->ts.type == BT_CHARACTER + && !f->sym->ts.deferred + && f->sym->ts.u.cl + && f->sym->ts.u.cl->length == NULL) + { + gfc_error ("Intrinsic % without % argument at %L " + "passed to assumed-length dummy %qs", + &a->expr->where, f->sym->name); + ok = false; + goto match; + } + if (a->expr->expr_type == EXPR_NULL && ((f->sym->ts.type != BT_CLASS && !f->sym->attr.pointer && (f->sym->attr.allocatable || !f->sym->attr.optional diff --git a/gcc/testsuite/gfortran.dg/null_actual_4.f90 b/gcc/testsuite/gfortran.dg/null_actual_4.f90 new file mode 100644 index 000..e03d5c8f7de --- /dev/null +++ b/gcc/testsuite/gfortran.dg/null_actual_4.f90 @@ -0,0 +1,35 @@ +! { dg-do compile } +! PR fortran/104819 +! +! Reject NULL without MOLD as actual to an assumed-rank dummy. +! See also interpretation request at +! https://j3-fortran.org/doc/year/22/22-101r1.txt +! +! Test nested NULL() + +program p + implicit none + integer, pointer :: a, a3(:,:,:) + character(10), pointer :: c + + call foo (a) + call foo (a3) + call foo (null (a)) + call foo (null (a3)) + call foo (null (null (a))) ! Valid: nested NULL()s + call foo (null (null (a3))) ! Valid: nested NULL()s + call foo (null ()) ! { dg-error "passed to assumed-rank dummy" } + + call str (null (c)) + call str (null (null (c))) + call str (null ()) ! { dg-error "passed to ass
[PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305]
Dear all, we are accepting loop annotations IVDEP, UNROLL n, VECTOR, and NOVECTOR for ordinary do loops, but ICE when such an annotation is specified before a DO CONCURRENT loop. Since at least the Intel compilers recognize some of the annotations also for DO CONCURRENT, it seems natural to extend gfortran instead of rejecting or ignoring the attributes. The attached patch handles the annotations as needed for the control structures of FORALL/DO CONCURRENT. Regarding the UNROLL directive, I don't have good references, so feedback is welcome. The current patch applies UNROLL only to the first loop control variable (for the case of loop nests), which translates into the innermost loop in gcc's representation. Regtested on x86_64-pc-linux-gnu. OK for mainline? Comments? Thanks, Harald From 0df60f02c399a6bf65850ecd5719b25b3de6676f Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Wed, 10 Jan 2024 23:10:02 +0100 Subject: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305] gcc/fortran/ChangeLog: PR fortran/113305 * gfortran.h: Add annotation controls to gfc_forall_iterator. * gfortran.texi: Document annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. * parse.cc (parse_do_block): Parse annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. Apply UNROLL only to first loop control variable. * trans-stmt.cc (gfc_trans_forall_loop): Annotate loops with IVDEP, UNROLL n, VECTOR, NOVECTOR as needed for DO CONCURRENT. (gfc_trans_forall_1): Handle annotations IVDEP, UNROLL n, VECTOR, NOVECTOR. gcc/testsuite/ChangeLog: PR fortran/113305 * gfortran.dg/do_concurrent_7.f90: New test. --- gcc/fortran/gfortran.h| 4 +++ gcc/fortran/gfortran.texi | 12 gcc/fortran/parse.cc | 26 - gcc/fortran/trans-stmt.cc | 29 ++- gcc/testsuite/gfortran.dg/do_concurrent_7.f90 | 26 + 5 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/do_concurrent_7.f90 diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 82f388c05f8..88502c1e3f0 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2926,6 +2926,10 @@ gfc_dt; typedef struct gfc_forall_iterator { gfc_expr *var, *start, *end, *stride; + unsigned short unroll; + bool ivdep; + bool vector; + bool novector; struct gfc_forall_iterator *next; } gfc_forall_iterator; diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 5615fee2897..371666dcbb6 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -3262,6 +3262,9 @@ It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. N is an integer constant specifying the unrolling factor. The values of 0 and 1 block any unrolling of the loop. +For @code{DO CONCURRENT} constructs the unrolling specification applies +only to the first loop control variable. + @node BUILTIN directive @subsection BUILTIN directive @@ -3300,6 +3303,9 @@ whether a particular loop is vectorizable due to potential dependencies between iterations. The purpose of the directive is to tell the compiler that vectorization is safe. +For @code{DO CONCURRENT} constructs this annotation is implicit to all +loop control variables. + This directive is intended for annotation of existing code. For new code it is recommended to consider OpenMP SIMD directives as potential alternative. @@ -3316,6 +3322,9 @@ This directive tells the compiler to vectorize the following loop. It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. +For @code{DO CONCURRENT} constructs this annotation applies to all loops +specified in the concurrent header. + @node NOVECTOR directive @subsection NOVECTOR directive @@ -3328,6 +3337,9 @@ This directive tells the compiler to not vectorize the following loop. It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. +For @code{DO CONCURRENT} constructs this annotation applies to all loops +specified in the concurrent header. + @node Non-Fortran Main Program @section Non-Fortran Main Program diff --git a/gcc/fortran/parse.cc b/gcc/fortran/parse.cc index d8b38cfb5ac..f41cc7d3510 100644 --- a/gcc/fortran/parse.cc +++ b/gcc/fortran/parse.cc @@ -5307,7 +5307,31 @@ parse_do_block (void) do_op = new_st.op; s.ext.end_do_label = new_st.label1; - if (new_st.ext.iterator != NULL) + if (do_op == EXEC_DO_CONCURRENT) +{ + gfc_forall_iterator *fa; + for (fa = new_st.ext.forall_iterator; fa; fa = fa->next) + { + /* Apply unroll only to innermost loop (first control + variable). */ + if (directive_unroll != -1) + { + fa->unroll = directive_unroll; + directive_unroll = -1; + } + if (directive_ivdep) + fa->ivde
[PATCH, v2] Fortran: annotations for DO CONCURRENT loops [PR113305]
Hi Bernhard, On 1/12/24 10:44, Bernhard Reutner-Fischer wrote: On Wed, 10 Jan 2024 23:24:22 +0100 Harald Anlauf wrote: diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 82f388c05f8..88502c1e3f0 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2926,6 +2926,10 @@ gfc_dt; typedef struct gfc_forall_iterator { gfc_expr *var, *start, *end, *stride; + unsigned short unroll; + bool ivdep; + bool vector; + bool novector; struct gfc_forall_iterator *next; } [] diff --git a/gcc/fortran/trans-stmt.cc b/gcc/fortran/trans-stmt.cc index a718dce237f..59a9cf99f9b 100644 --- a/gcc/fortran/trans-stmt.cc +++ b/gcc/fortran/trans-stmt.cc @@ -41,6 +41,10 @@ typedef struct iter_info tree start; tree end; tree step; + unsigned short unroll; + bool ivdep; + bool vector; + bool novector; struct iter_info *next; } Given that we already have in gfortran.h typedef struct { gfc_expr *var, *start, *end, *step; unsigned short unroll; bool ivdep; bool vector; bool novector; } gfc_iterator; would it make sense to break out these loop annotation flags into its own let's say struct gfc_iterator_flags and use pointers to that flags instead? I've created a struct gfc_loop_annot and use that directly as I think using pointers to it is probably not very efficient. Well, the struct is smaller than a pointer on a 64-bit system... LGTM otherwise. Thanks for the patch! Thanks for the review! If there are no further comments, I'll commit the attached version soon. Harald From 31d8957a95455663577a0e60109679d56aac234d Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 12 Jan 2024 19:51:11 +0100 Subject: [PATCH] Fortran: annotations for DO CONCURRENT loops [PR113305] gcc/fortran/ChangeLog: PR fortran/113305 * gfortran.h (gfc_loop_annot): New. (gfc_iterator, gfc_forall_iterator): Use for annotation control. * array.cc (gfc_copy_iterator): Adjust. * gfortran.texi: Document annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. * parse.cc (parse_do_block): Parse annotations IVDEP, UNROLL n, VECTOR, NOVECTOR as applied to DO CONCURRENT. Apply UNROLL only to first loop control variable. * trans-stmt.cc (iter_info): Use gfc_loop_annot. (gfc_trans_simple_do): Adjust. (gfc_trans_forall_loop): Annotate loops with IVDEP, UNROLL n, VECTOR, NOVECTOR as needed for DO CONCURRENT. (gfc_trans_forall_1): Handle loop annotations. gcc/testsuite/ChangeLog: PR fortran/113305 * gfortran.dg/do_concurrent_7.f90: New test. --- gcc/fortran/array.cc | 5 +- gcc/fortran/gfortran.h| 11 - gcc/fortran/gfortran.texi | 12 + gcc/fortran/parse.cc | 34 -- gcc/fortran/trans-stmt.cc | 46 ++- gcc/testsuite/gfortran.dg/do_concurrent_7.f90 | 26 +++ 6 files changed, 113 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/gfortran.dg/do_concurrent_7.f90 diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc index 19456baf103..81fa99d219f 100644 --- a/gcc/fortran/array.cc +++ b/gcc/fortran/array.cc @@ -2308,10 +2308,7 @@ gfc_copy_iterator (gfc_iterator *src) dest->start = gfc_copy_expr (src->start); dest->end = gfc_copy_expr (src->end); dest->step = gfc_copy_expr (src->step); - dest->unroll = src->unroll; - dest->ivdep = src->ivdep; - dest->vector = src->vector; - dest->novector = src->novector; + dest->annot = src->annot; return dest; } diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 82f388c05f8..fd73e4ce431 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2830,14 +2830,22 @@ gfc_case; #define gfc_get_case() XCNEW (gfc_case) +/* Annotations for loop constructs. */ typedef struct { - gfc_expr *var, *start, *end, *step; unsigned short unroll; bool ivdep; bool vector; bool novector; } +gfc_loop_annot; + + +typedef struct +{ + gfc_expr *var, *start, *end, *step; + gfc_loop_annot annot; +} gfc_iterator; #define gfc_get_iterator() XCNEW (gfc_iterator) @@ -2926,6 +2934,7 @@ gfc_dt; typedef struct gfc_forall_iterator { gfc_expr *var, *start, *end, *stride; + gfc_loop_annot annot; struct gfc_forall_iterator *next; } gfc_forall_iterator; diff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 5615fee2897..371666dcbb6 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -3262,6 +3262,9 @@ It must be placed immediately before a @code{DO} loop and applies only to the loop that follows. N is an integer constant specifying the unrolling factor. The values of 0 and 1 block any unrolling of the loop. +For @code{DO CONCURRENT} constructs the unrolling specification applies +only to the first loop control variable. + @node BUILTIN directive @subsection BUILTIN directive
[PATCH] Fortran: intrinsic ISHFTC and missing optional argument SIZE [PR67277]
Dear all, the testcase given in PR67277 actually consists of two separate issues: (1) passing an optional dummy argument to an elemental (intrinsic) procedure (2) a missing optional argument for SIZE to the ISHFTC intrinsic shall be equivalent to using BIT_SIZE(I). I've created a separate PR113377 for (1), as this looks like a more general issue with the scalarizer. The attached, rather simple and obvious patch thus fixes (2). Besides testing that the patch works as advertised, the testcase also contains variations that need fixing of PR113377 before they can be uncommented. Regtested on x86_64-pc-linux-gnu. OK for mainline? As I consider the patch safe, I'd like to backport to 13-branch later. Thanks, Harald P.S.: if someone out there feels familiar with the scalarizer, a look at PR113377 is appreciated. From 20da56165273c8814b3c53e6d71549ba6a37e0cd Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 13 Jan 2024 22:00:21 +0100 Subject: [PATCH] Fortran: intrinsic ISHFTC and missing optional argument SIZE [PR67277] gcc/fortran/ChangeLog: PR fortran/67277 * trans-intrinsic.cc (gfc_conv_intrinsic_ishftc): Handle optional dummy argument for SIZE passed to ISHFTC. Set default value to BIT_SIZE(I) when missing. gcc/testsuite/ChangeLog: PR fortran/67277 * gfortran.dg/ishftc_optional_size_1.f90: New test. --- gcc/fortran/trans-intrinsic.cc| 14 +++ .../gfortran.dg/ishftc_optional_size_1.f90| 97 +++ 2 files changed, 111 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 diff --git a/gcc/fortran/trans-intrinsic.cc b/gcc/fortran/trans-intrinsic.cc index 74139262657..0468dfae2b1 100644 --- a/gcc/fortran/trans-intrinsic.cc +++ b/gcc/fortran/trans-intrinsic.cc @@ -6863,9 +6863,23 @@ gfc_conv_intrinsic_ishftc (gfc_se * se, gfc_expr * expr) if (num_args == 3) { + gfc_expr *size = expr->value.function.actual->next->next->expr; + /* Use a library function for the 3 parameter version. */ tree int4type = gfc_get_int_type (4); + /* Treat optional SIZE argument when it is passed as an optional + dummy. If SIZE is absent, the default value is BIT_SIZE(I). */ + if (size->expr_type == EXPR_VARIABLE + && size->symtree->n.sym->attr.dummy + && size->symtree->n.sym->attr.optional) + { + tree type_of_size = TREE_TYPE (args[2]); + args[2] = build3_loc (input_location, COND_EXPR, type_of_size, +gfc_conv_expr_present (size->symtree->n.sym), +args[2], fold_convert (type_of_size, nbits)); + } + /* We convert the first argument to at least 4 bytes, and convert back afterwards. This removes the need for library functions for all argument sizes, and function will be diff --git a/gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 b/gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 new file mode 100644 index 000..1ccf4b38caa --- /dev/null +++ b/gcc/testsuite/gfortran.dg/ishftc_optional_size_1.f90 @@ -0,0 +1,97 @@ +! { dg-do run } +! +! PR fortran/67277 - ISHFTC and missing optional argument SIZE + +module m + implicit none +contains + ! Optional argument passed by reference + elemental function ishftc4_ref (i, shift, size_) result(r) +integer(4), intent(in) :: i +integer,intent(in) :: shift +integer,intent(in), optional :: size_ +integer :: r +r = ishftc (i, shift=shift, size=size_) + end + + elemental function ishftc1_ref (i, shift, size_) result(r) +integer(1), intent(in) :: i +integer,intent(in) :: shift +integer(1), intent(in), optional :: size_ +integer(1) :: r +r = ishftc (i, shift=shift, size=size_) + end + + ! Array valued argument i + function ishftc4_ref_4 (i, shift, size_) result(r) +integer(4), intent(in) :: i(4) +integer,intent(in) :: shift +integer,intent(in), optional :: size_ +integer :: r(size(i)) +r = ishftc (i, shift=shift, size=size_) + end + + ! Optional argument passed by value + elemental function ishftc4_val (i, shift, size_) result(r) +integer(4), intent(in) :: i +integer,intent(in) :: shift +integer,value, optional :: size_ +integer :: r +r = ishftc (i, shift=shift, size=size_) + end + + elemental function ishftc1_val (i, shift, size_) result(r) +integer(1), intent(in) :: i +integer,intent(in) :: shift +integer(1), value, optional :: size_ +integer(1) :: r +r = ishftc (i, shift=shift, size=size_) + end + + ! Array valued argument i + function ishftc4_val_4 (i, shift, size_) result(r) +integer(4), intent(in) :: i(4) +integer,intent(in) ::
[PATCH, committed] Fortran: fix wrong array bounds check [PR113471]
Dear all, I've pushed the attached obvious patch for a regression due to a wrong array bounds check after regtesting on x86_64-pc-linux-gnu and verification of the fix by the reporter in the PR. https://gcc.gnu.org/g:94b2e6cb1cc4feb122bf77f19a657c97bffa9b42 Thanks, Harald From 94b2e6cb1cc4feb122bf77f19a657c97bffa9b42 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Fri, 19 Jan 2024 21:20:44 +0100 Subject: [PATCH] Fortran: fix wrong array bounds check [PR113471] gcc/fortran/ChangeLog: PR fortran/113471 * trans-array.cc (array_bound_check_elemental): Array bounds check shall apply here to elemental dimensions of an array section only. gcc/testsuite/ChangeLog: PR fortran/113471 * gfortran.dg/bounds_check_24.f90: New test. --- gcc/fortran/trans-array.cc| 2 +- gcc/testsuite/gfortran.dg/bounds_check_24.f90 | 28 +++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 gcc/testsuite/gfortran.dg/bounds_check_24.f90 diff --git a/gcc/fortran/trans-array.cc b/gcc/fortran/trans-array.cc index 26e7adaa03f..878a92aff18 100644 --- a/gcc/fortran/trans-array.cc +++ b/gcc/fortran/trans-array.cc @@ -3600,7 +3600,7 @@ array_bound_check_elemental (gfc_se * se, gfc_ss * ss, gfc_expr * expr) continue; } - if (ref->type == REF_ARRAY && ref->u.ar.dimen > 0) + if (ref->type == REF_ARRAY && ref->u.ar.type == AR_SECTION) { ar = &ref->u.ar; for (dim = 0; dim < ar->dimen; dim++) diff --git a/gcc/testsuite/gfortran.dg/bounds_check_24.f90 b/gcc/testsuite/gfortran.dg/bounds_check_24.f90 new file mode 100644 index 000..d0251e8455b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/bounds_check_24.f90 @@ -0,0 +1,28 @@ +! { dg-do compile } +! { dg-additional-options "-fcheck=bounds -fdump-tree-original" } +! +! PR fortran/113471 - wrong array bounds check + +program pr113471 + implicit none + type t + integer, dimension(2) :: c1 = 0 + end type t + type(t) :: cc(7), bb(7) + integer :: kk = 1 + + ! no bounds check (can be determined at compile time): + call foo (cc(7)% c1) + + ! bounds check involving kk, but no "outside of expected range" + call foo (bb(kk)% c1) + +contains + subroutine foo (c) +integer, intent(in) :: c(:) + end +end + +! { dg-final { scan-tree-dump-times "below lower bound" 2 "original" } } +! { dg-final { scan-tree-dump-times "above upper bound" 2 "original" } } +! { dg-final { scan-tree-dump-not "outside of expected range" "original" } } -- 2.35.3
Re: PR82943 - Suggested patch to fix
Am 20.01.24 um 20:08 schrieb Jerry D: On 1/20/24 10:46 AM, Alexander Westbrooks wrote: Hello and Happy New Year! I wanted to follow up on this patch I made to address PR82943 for GFortran. Has anyone had a chance to review it? Thanks, Alexander Westbrooks Inserting myself in here just a little bit. I will apply and test today if I can. Paul is unavailable for a few weeks. Harald can chime in. Do you have commit rights for gcc? I am not aware of Alex having a copyright assignment on file, or a DCO certificate, and the patch is not signed off. But I might be wrong. Your efforts are appreciated. Regards, Jerry
Re: PR82943 - Suggested patch to fix
Am 20.01.24 um 21:37 schrieb Jerry D: On 1/20/24 12:08 PM, Harald Anlauf wrote: Am 20.01.24 um 20:08 schrieb Jerry D: On 1/20/24 10:46 AM, Alexander Westbrooks wrote: Hello and Happy New Year! I wanted to follow up on this patch I made to address PR82943 for GFortran. Has anyone had a chance to review it? Thanks, Alexander Westbrooks Inserting myself in here just a little bit. I will apply and test today if I can. Paul is unavailable for a few weeks. Harald can chime in. Do you have commit rights for gcc? I am not aware of Alex having a copyright assignment on file, or a DCO certificate, and the patch is not signed off. But I might be wrong. --- snip --- I do not mind committing this but need clarifications regarding the copyright (copyleft?) rules in this case. In the past we have allowed small contributions like this. This may be a little more than minor. It is. This is why I pointed to: https://gcc.gnu.org/dco.html Regardless it appears to do the job! Jerry
[PATCH] Fortran: passing of optional scalar arguments with VALUE attribute [PR113377]
Dear all, here's the first part of an attempt to fix issues with optional dummy arguments as actual arguments to optional dummies. This patch rectifies the case of scalar dummies with the VALUE attribute, which in gfortran's argument passing convention are passed on the stack when they are of intrinsic type, and have a hidden variable for the presence status. The testcase tries to cover valid combinations of actual and dummy argument. A few tests that are not standard-conforming but would still work with gfortran (due to the argument passing convention) are left there but commented out with a pointer to the standard (thanks, Mikael!). Regtested on x86_64-pc-linux-gnu. OK for mainline? Thanks, Harald From f6a65138391c902d2782973665059d7d059a50d1 Mon Sep 17 00:00:00 2001 From: Harald Anlauf Date: Sat, 20 Jan 2024 22:18:02 +0100 Subject: [PATCH] Fortran: passing of optional scalar arguments with VALUE attribute [PR113377] gcc/fortran/ChangeLog: PR fortran/113377 * trans-expr.cc (gfc_conv_procedure_call): Fix handling of optional scalar arguments of intrinsic type with the VALUE attribute. gcc/testsuite/ChangeLog: PR fortran/113377 * gfortran.dg/optional_absent_9.f90: New test. --- gcc/fortran/trans-expr.cc | 5 + .../gfortran.dg/optional_absent_9.f90 | 324 ++ 2 files changed, 329 insertions(+) create mode 100644 gcc/testsuite/gfortran.dg/optional_absent_9.f90 diff --git a/gcc/fortran/trans-expr.cc b/gcc/fortran/trans-expr.cc index 9dd1f4086f4..2f47a75955c 100644 --- a/gcc/fortran/trans-expr.cc +++ b/gcc/fortran/trans-expr.cc @@ -6526,6 +6526,10 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, gfc_init_se (&argse, NULL); argse.want_pointer = 1; gfc_conv_expr (&argse, e); + if (e->symtree->n.sym->attr.dummy +&& POINTER_TYPE_P (TREE_TYPE (argse.expr))) + argse.expr = gfc_build_addr_expr (NULL_TREE, +argse.expr); cond = fold_convert (TREE_TYPE (argse.expr), null_pointer_node); cond = fold_build2_loc (input_location, NE_EXPR, @@ -7256,6 +7260,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * sym, && e->symtree->n.sym->attr.optional && (((e->rank != 0 && elemental_proc) || e->representation.length || e->ts.type == BT_CHARACTER + || (e->rank == 0 && e->symtree->n.sym->attr.value) || (e->rank != 0 && (fsym == NULL || (fsym->as diff --git a/gcc/testsuite/gfortran.dg/optional_absent_9.f90 b/gcc/testsuite/gfortran.dg/optional_absent_9.f90 new file mode 100644 index 000..495a6c00d7f --- /dev/null +++ b/gcc/testsuite/gfortran.dg/optional_absent_9.f90 @@ -0,0 +1,324 @@ +! { dg-do run } +! PR fortran/113377 +! +! Test passing of missing optional scalar dummies of intrinsic type + +module m_int + implicit none +contains + subroutine test_int () +integer :: k = 1 +call one (k) +call one_val (k) +call one_all (k) +call one_ptr (k) + end + + subroutine one (i, j) +integer, intent(in) :: i +integer ,optional :: j +integer, allocatable :: aa +integer, pointer :: pp => NULL() +if (present (j)) error stop "j is present" +call two (i, j) +call two_val (i, j) +call two (i, aa) +call two (i, pp) + end + + subroutine one_val (i, j) +integer, intent(in) :: i +integer, value, optional :: j +if (present (j)) error stop "j is present" +call two (i, j) +call two_val (i, j) + end + + subroutine one_all (i, j) +integer, intent(in) :: i +integer, allocatable,optional :: j +if (present (j)) error stop "j is present" +! call two (i, j) ! invalid per F2018:15.5.2.12, par. 3, clause 8 +! call two_val (i, j) ! invalid (*) F2018:15.5.2.12, par. 3, clause 8 +call two_all (i, j) + end +! (*) gfortran argument passing conventions ("scalar dummy arguments of type +! INTEGER, LOGICAL, REAL, COMPLEX, and CHARACTER(len=1) with VALUE attribute +! pass the presence status separately") may still allow this case pass + + subroutine one_ptr (i, j) +integer, intent(in) :: i +integer, pointer,optional :: j +if (present (j)) error stop "j is present" +! call two (i, j) ! invalid per F2018:15.5.2.12, par. 3, clause 7 +! call two_val (i, j) ! invalid (*) F2018:15.5.2.12, par. 3, clause 7 +call two_ptr (i, j) + end + + subroutine two (i, j) +integer, intent(in) :: i +integer, intent(in), optional :: j +if (present (j)) error stop 11 + end + + subroutine two_val (i, j) +integer, intent(in) :: i +integer, value, optional :: j +if (present (j)) error stop 12 + end + + subroutine two_all (i, j) +integer, intent(in)