Re: [PATCH, Fortran] Fixes for F2018 C838 (PR fortran/101334)
On 20.09.21 06:01, Sandra Loosemore wrote: This patch fixes some bugs in handling of assumed-rank arguments revealed by the TS29113 testsuite, allowing xfails to be removed from those testcases. It was previously failing to diagnose an error when passing an assumed-rank argument to a procedure via a non-assumed-rank dummy, and giving a bogus error when passing one as the first argument to the ASSOCIATED intrinsic. Both fixes turned out to be 1-liners. OK to commit? OK - however, I think you should first commit your follow-up/second patch (fix testsuite) to avoid intermittent test-suite fails. Additionally, if I try the following testcase, which is now permitted, I get two ICEs. Can you check? * The first one seems to be a bug in gfc_conv_intrinsic_function, which assumes also for assumed rank that if the first argument is an array, the second argument must also be an array. * For the second one, I see in the dump: p->dim[p->dtype.rank + -1].stride is seems as '-1' is gfc_array_index_type while 'dtype.rank' is signed_char_type_node. (Disclaimer: I don't have a clean tree, but I think this issue not affected by my patches.) subroutine foo(p, lp, lpd) use iso_c_binding implicit none (type, external) real, pointer :: p(..) real, pointer :: lp real, pointer :: lpd(:,:) ! gfc_conv_expr_descriptor, at fortran/trans-array.c:7324 if (associated(p, lp)) stop 1 ! verify_gimple: type mismatch in binary expression - signed char, signed char, integer(kind=8), _4 = _3 + -1; if (associated(p, lpd)) stop 1 end 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] Fix testcases that violate C838, + revealed ICE
On 20.09.21 06:02, Sandra Loosemore wrote: This patch fixes 3 testcases that violate F2018 C838 by passing an assumed-rank argument to a procedure via an assumed-sized dummy, by wrapping the call in a SELECT RANK construct. But wait, there's more! This triggered an ICE due to a null pointer dereference in the code that handles the associated variable in the SELECT RANK. I fixed that by copying the idiom used in other places for GFC_DECL_SAVED_DESCRIPTOR, so now all the tests pass again. Is this OK to commit? LGTM – as mentioned in the other patch, please commit this patch first to avoid intermittent testsuite fails. Tobias I confess I'm not certain whether adding the SELECT RANK causes the testcases now to do something different from what they were originally trying to test, but they never should have worked as originally written anyway. We were just not previously diagnosing the C838 violations without the other patch I just posted to do that. -Sandra - 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/OpenMP: unconstrained/reproducible ordered modifier
On Fri, Sep 17, 2021 at 11:42:13PM +0200, Tobias Burnus wrote: > Fortran/OpenMP: unconstrained/reproducible ordered modifier > > gcc/fortran/ChangeLog: > > * gfortran.h (gfc_omp_clauses): Add order_unconstrained. > * dump-parse-tree.c (show_omp_clauses): Dump it. > * openmp.c (gfc_match_omp_clauses): Match unconstrained/reproducible > modifiers to ordered(concurrent). > (OMP_DISTRIBUTE_CLAUSES): Accept ordered clause. > (resolve_omp_clauses): Reject ordered + order on same directive. > * trans-openmp.c (gfc_trans_omp_clauses, gfc_split_omp_clauses): Pass > on unconstrained modifier of ordered(concurrent). > > gcc/testsuite/ChangeLog: > > * gfortran.dg/gomp/order-5.f90: New test. > * gfortran.dg/gomp/order-6.f90: New test. > * gfortran.dg/gomp/order-7.f90: New test. > * gfortran.dg/gomp/order-8.f90: New test. > * gfortran.dg/gomp/order-9.f90: New test. Ok, thanks. > @@ -5892,6 +5893,8 @@ gfc_split_omp_clauses (gfc_code *code, > = code->ext.omp_clauses->collapse; > clausesa[GFC_OMP_SPLIT_DISTRIBUTE].order_concurrent > = code->ext.omp_clauses->order_concurrent; So the FE was splitting the order clause to distribute already before, perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1 support. The difference is e.g. the distribute parallel do order(concurrent) copyin(thr) case which used to be ok in 5.0 and is not in 5.1. Jakub
Re: [PATCH, Fortran] Fixes for F2018 C838 (PR fortran/101334)
Hi Sandra, This patch fixes some bugs in handling of assumed-rank arguments revealed by the TS29113 testsuite, allowing xfails to be removed from those testcases. It was previously failing to diagnose an error when passing an assumed-rank argument to a procedure via a non-assumed-rank dummy, and giving a bogus error when passing one as the first argument to the ASSOCIATED intrinsic. Both fixes turned out to be 1-liners. OK to commit? OK. Thanks for the patch! Best regards Thomas
[Patch]GCC11 - Fortran: combined directives - order(concurrent) not on distribute (was: Re: [Patch] Fortran/OpenMP: unconstrained/reproducible ordered modifier)
On 20.09.21 11:55, Jakub Jelinek via Fortran wrote:
So the FE was splitting the order clause to distribute already before,
perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1
support.
The difference is e.g. the distribute parallel do order(concurrent) copyin(thr)
case which used to be ok in 5.0 and is not in 5.1.
Well, if I try with GCC 11:
void f(int *a)
{
int i;
static int thr;
#pragma omp threadprivate (thr)
#pragma omp distribute parallel for order(concurrent) copyin(thr)
for (i = 0; i < 10; ++i)
{
thr = 5;
a[i] = thr;
}
}
I get with gcc (+ gfortran):
error: threadprivate variable ‘thr’ used in a region with ‘order(concurrent)’
clause
I might have misunderstood the example.
* * *
In any case, for GCC 11, I have now fixed the splitting and added a testcase
which
relies on -fdump-tree-original scanning and does not use threadprivate.
OK for GCC 11, only?
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
GCC11 - Fortran: combined directives - order(concurrent) not on distribute
While OpenMP 5.1 and GCC 12 permits 'order(concurrent)' on distribute,
OpenMP 5.0 and GCC 11 don't. This patch for GCC 11 ensures the clause also
does not end up on 'distribute' when splitting combined directives.
gcc/fortran/ChangeLog:
* trans-openmp.c (gfc_split_omp_clauses): Don't put 'order(concurrent)'
on 'distribute' for combined directives, matching OpenMP 5.0
gcc/testsuite/ChangeLog:
* gfortran.dg/gomp/distribute-order-concurrent.f90: New test.
gcc/fortran/trans-openmp.c | 2 --
.../gomp/distribute-order-concurrent.f90 | 25 ++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index 7e931bf4bc7..973d916b4a2 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -5176,8 +5176,6 @@ gfc_split_omp_clauses (gfc_code *code,
/* Duplicate collapse. */
clausesa[GFC_OMP_SPLIT_DISTRIBUTE].collapse
= code->ext.omp_clauses->collapse;
- clausesa[GFC_OMP_SPLIT_DISTRIBUTE].order_concurrent
- = code->ext.omp_clauses->order_concurrent;
}
if (mask & GFC_OMP_MASK_PARALLEL)
{
diff --git a/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90 b/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90
new file mode 100644
index 000..9597d913684
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/distribute-order-concurrent.f90
@@ -0,0 +1,25 @@
+! { dg-additional-options "-fdump-tree-original" }
+!
+! In OpenMP 5.0, 'order(concurrent)' does not apply to distribute
+! Ensure that it is rejected in GCC 11.
+!
+! Note: OpenMP 5.1 allows it; the GCC 12 testcase for it is gfortran.dg/gomp/order-5.f90
+
+subroutine f(a)
+implicit none
+integer :: i, thr
+!save :: thr
+integer :: a(:)
+
+!$omp distribute parallel do order(concurrent) private(thr)
+ do i = 1, 10
+thr = 5
+a(i) = thr
+ end do
+!$omp end distribute parallel do
+end
+
+! { dg-final { scan-tree-dump-not "omp distribute\[^\n\r]*order" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp distribute\[\n\r\]" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp parallel private\\(thr\\)" "original" } }
+! { dg-final { scan-tree-dump "#pragma omp for nowait order\\(concurrent\\)" "original" } }
Re: [Patch]GCC11 - Fortran: combined directives - order(concurrent) not on distribute (was: Re: [Patch] Fortran/OpenMP: unconstrained/reproducible ordered modifier)
On Mon, Sep 20, 2021 at 05:01:32PM +0200, Tobias Burnus wrote:
> On 20.09.21 11:55, Jakub Jelinek via Fortran wrote:
> > So the FE was splitting the order clause to distribute already before,
> > perhaps we should undo that for gcc 11 which doesn't claim any OpenMP 5.1
> > support.
> > The difference is e.g. the distribute parallel do order(concurrent)
> > copyin(thr)
> > case which used to be ok in 5.0 and is not in 5.1.
>
> Well, if I try with GCC 11:
>
> void f(int *a)
> {
> int i;
> static int thr;
> #pragma omp threadprivate (thr)
> #pragma omp distribute parallel for order(concurrent) copyin(thr)
> for (i = 0; i < 10; ++i)
>{
> thr = 5;
> a[i] = thr;
>}
> }
>
> I get with gcc (+ gfortran):
> error: threadprivate variable ‘thr’ used in a region with
> ‘order(concurrent)’ clause
> I might have misunderstood the example.
Sure, even before you couldn't use it in the region body, because
order(concurrent) was split to worksharing-loop.
The testcase that used to be accepted and is now rejected is e.g.
int thr;
#pragma omp threadprivate (thr)
void
foo (void)
{
int i;
#pragma omp distribute parallel for order(concurrent) copyin(thr)
for (i = 0; i < 10; ++i)
;
}
While copyin without actually using the threadprivate var in the body
might look weird, in some cases it might be useful if the threadprivate
variable is used in some following parallel region.
> OK for GCC 11, only?
>
> 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
> GCC11 - Fortran: combined directives - order(concurrent) not on distribute
>
> While OpenMP 5.1 and GCC 12 permits 'order(concurrent)' on distribute,
> OpenMP 5.0 and GCC 11 don't. This patch for GCC 11 ensures the clause also
> does not end up on 'distribute' when splitting combined directives.
>
> gcc/fortran/ChangeLog:
>
> * trans-openmp.c (gfc_split_omp_clauses): Don't put 'order(concurrent)'
> on 'distribute' for combined directives, matching OpenMP 5.0
>
> gcc/testsuite/ChangeLog:
>
> * gfortran.dg/gomp/distribute-order-concurrent.f90: New test.
Ok, thanks.
Jakub
Harald Anlauf appointed Fortran Reviewer
I am pleased to announce that the GCC Steering Committee has appointed Harald Anlauf as a Fortran Reviewer. Please join me in congratulating Harald on his new role. Harald, please update your listing in the MAINTAINERS file. Happy hacking! David
Re: [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]
And v3 – I realized that testcases would be useful. Thus, now with added testcases. :-) Tobias On 17.09.21 20:45, Tobias Burnus wrote: I seemingly messed up a bit in previous patch – corrected version attached. OK? Tobias PS: Due to now enabling the missing-include-dir warning also for cpp,the following warning show up during build. This seems to be specific to libgfortran building, libgomp works and real-world code also does not seem to be affected: : Error: /x86_64-pc-linux-gnu/include: No such file or directory [-Werror=missing-include-dirs] : Error: /x86_64-pc-linux-gnu/sys-include: No such file or directory [-Werror=missing-include-dirs] : Error: finclude: No such file or directory [-Werror=missing-include-dirs] The latter is due to the driver adding '-fintrinsic-modules-path finclude' when calling f951. I think the rest is a side effect of running with -B and other build trickery. The warnings do not trigger when compiling the Fortran file in libgomp nor for a quick real-world case (which uses gfortran in a normal way not with -B etc.). Thus, I think it should be fine. Alternatively, we could think of reducing the noisiness. Thoughts? PPS: Besides this, the following still applies: On 17.09.21 15:02, Tobias Burnus wrote: Short version: * -Wno-missing-include-dirs had no effect as the warning was always on * For CPP-only options like -idirafter, no warning was shown. This patch tries to address both, i.e. cpp's include-dir diagnostics are shown as well – and silencing the diagnostic works as well. OK for mainline? Tobias PS: BACKGROUND and LONG DESCRIPTION C/C++ by default have disabled the -Wmissing-include-dirs warning. Fortran by default has that warning enabled. The value is actually stored at two places (cf. c-family/c.opt): Wmissing-include-dirs ... CPP(warn_missing_include_dirs) Var(cpp_warn_missing_include_dirs) Init(0) For CPP, that value always needs to initialized – and it is used in gcc/incpath.c as cpp_options *opts = cpp_get_options (pfile); if (opts->warn_missing_include_dirs && cur->user_supplied_p) cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s: %s", Additionally, there is cpp_warn_missing_include_dirs which is used by Fortran – and which consists of global_options.x_cpp_warn_missing_include_dirs global_options_set._cpp_warn_missing_include_dirs The flag processing happens as described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11 in short: - First gfc_init_options is called - Then for reach command-line option gfc_handle_option - Finally gfc_post_options Currently: - gfc_init_options: Sets cpp_warn_missing_include_dirs (unconditionally as unset) - gfc_handle_option: Always warns about the missing include dir - before gfc_post_options: set_option is called, which sets cpp_warn_missing_include_dirs – but that's too late. Additionally, as mentioned above – pfile's warn_missing_include_dirs is never properly set. * * * This patch fixes those issues: * Now -Wno-missing-include-dirs does silence the warnings * CPP now also properly does warn. Example (initial version): $ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/ -Wmissing-include-dirs f951: Warning: Nonexistent include directory ‘bar//’ [-Wmissing-include-dirs] : Warning: /fdaf/: No such file or directory : Warning: bar/: No such file or directory In order to avoid the double output for -I, I disabled the Fortran output if CPP is enabled. Additionally, I had to use the cpp_reason_option_codes to print the flag in brackets. Fixed/final output is: : Warning: /fdaf/: No such file or directory [-Wmissing-include-dirs] : Warning: bar/: No such file or directory [-Wmissing-include-dirs] - 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 Fortran: Fix -Wno-missing-include-dirs handling [PR55534] gcc/fortran/ChangeLog: PR fortran/55534 * cpp.c: Define GCC_C_COMMON_C for #include "options.h" to make cpp_reason_option_codes available. (gfc_cpp_register_include_paths): Make static, set pfile's warn_missing_include_dirs and move before caller. (gfc_cpp_init_cb): New, cb code moved from ... (gfc_cpp_init_0): ... here. (gfc_cpp_post_options): Call gfc_cpp_init_cb. (cb_cpp_diagnostic_cpp_option): New. As implemented in c-family to match CppReason flags to -W... names. (cb_cpp_diagnostic): Use it to replace single special case. * cpp.h (gfc_cpp_register_include_paths): Remove as now static. * gfortran.h (gfc_check_include_dirs): New prototype. (gfc_add_include_path): Add new bool arg. * options.c (gfc_init_options): Don't set -Wmissing-include-dirs. (gfc_post_options): Set it here after commandline processing. Call gfc_add_include_path with defer_warn=false. (gfc_handle_option): Call it with defer_w
Re: [Patch] Fortran: Fix -Wno-missing-include-dirs handling [PR55534]
Hi Tobias, Am 20.09.21 um 22:33 schrieb Tobias Burnus: And v3 – I realized that testcases would be useful. Thus, now with added testcases. :-) I was about to recommend a testcase I prepared when your revised patch arrived. Will not bother you with it, as you seem to provide really good coverage. LGTM. Thanks for the patch! Harald Tobias On 17.09.21 20:45, Tobias Burnus wrote: I seemingly messed up a bit in previous patch – corrected version attached. OK? Tobias PS: Due to now enabling the missing-include-dir warning also for cpp,the following warning show up during build. This seems to be specific to libgfortran building, libgomp works and real-world code also does not seem to be affected: : Error: /x86_64-pc-linux-gnu/include: No such file or directory [-Werror=missing-include-dirs] : Error: /x86_64-pc-linux-gnu/sys-include: No such file or directory [-Werror=missing-include-dirs] : Error: finclude: No such file or directory [-Werror=missing-include-dirs] The latter is due to the driver adding '-fintrinsic-modules-path finclude' when calling f951. I think the rest is a side effect of running with -B and other build trickery. The warnings do not trigger when compiling the Fortran file in libgomp nor for a quick real-world case (which uses gfortran in a normal way not with -B etc.). Thus, I think it should be fine. Alternatively, we could think of reducing the noisiness. Thoughts? PPS: Besides this, the following still applies: On 17.09.21 15:02, Tobias Burnus wrote: Short version: * -Wno-missing-include-dirs had no effect as the warning was always on * For CPP-only options like -idirafter, no warning was shown. This patch tries to address both, i.e. cpp's include-dir diagnostics are shown as well – and silencing the diagnostic works as well. OK for mainline? Tobias PS: BACKGROUND and LONG DESCRIPTION C/C++ by default have disabled the -Wmissing-include-dirs warning. Fortran by default has that warning enabled. The value is actually stored at two places (cf. c-family/c.opt): Wmissing-include-dirs ... CPP(warn_missing_include_dirs) Var(cpp_warn_missing_include_dirs) Init(0) For CPP, that value always needs to initialized – and it is used in gcc/incpath.c as cpp_options *opts = cpp_get_options (pfile); if (opts->warn_missing_include_dirs && cur->user_supplied_p) cpp_warning (pfile, CPP_W_MISSING_INCLUDE_DIRS, "%s: %s", Additionally, there is cpp_warn_missing_include_dirs which is used by Fortran – and which consists of global_options.x_cpp_warn_missing_include_dirs global_options_set._cpp_warn_missing_include_dirs The flag processing happens as described in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55534#c11 in short: - First gfc_init_options is called - Then for reach command-line option gfc_handle_option - Finally gfc_post_options Currently: - gfc_init_options: Sets cpp_warn_missing_include_dirs (unconditionally as unset) - gfc_handle_option: Always warns about the missing include dir - before gfc_post_options: set_option is called, which sets cpp_warn_missing_include_dirs – but that's too late. Additionally, as mentioned above – pfile's warn_missing_include_dirs is never properly set. * * * This patch fixes those issues: * Now -Wno-missing-include-dirs does silence the warnings * CPP now also properly does warn. Example (initial version): $ gfortran-trunk ../empty.f90 -c -cpp -idirafter /fdaf/ -I bar/ -Wmissing-include-dirs f951: Warning: Nonexistent include directory ‘bar//’ [-Wmissing-include-dirs] : Warning: /fdaf/: No such file or directory : Warning: bar/: No such file or directory In order to avoid the double output for -I, I disabled the Fortran output if CPP is enabled. Additionally, I had to use the cpp_reason_option_codes to print the flag in brackets. Fixed/final output is: : Warning: /fdaf/: No such file or directory [-Wmissing-include-dirs] : Warning: bar/: No such file or directory [-Wmissing-include-dirs] - 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: Missing finalizations
Hi Paul, I'm following up on this as you requested. I won't be able to start looking into this seriously for a week or two anyway. Thanks, Andrew On Tuesday, September 14, 2021 11:51:00 PM PDT Paul Richard Thomas wrote: > Hi Andrew, > > Not long before I had to step aside (temporarily, I hope) from gfortran > maintenance, I made quite a lot of progress on missing finalizations. I'll > dig out the in-progress patch for you and remind myself of the remaining > issues. One of these was a standards problem, where the patched gfortran > differed from a leading brand and I thought that the leading brand was > wrong. > > Give me a day or two and prod me if I do not come up with the goods by the > end of the week. > > Paul > > > On Wed, 15 Sept 2021 at 01:31, Andrew Benson via Fortran < > > fortran@gcc.gnu.org> wrote: > > I will (hopefully) have some time in the next few months to work on > > gfortran. > > I could pick up a few easy PRs to fix, but a more ambitious (and more > > useful) > > task would be to work on some of the missing finalizations. For my own > > work > > finalization of function results and stay constructors would be most > > useful. > > But, I don't have any real idea of how difficult this would be to do. Does > > any > > one here have any guidance on how to do this, or how challenging it might > > be? > > > > Thanks, > > Andrew -- * Andrew Benson: https://abensonca.github.io * Galacticus: https://github.com/galacticusorg/galacticus
