Re: [PATCH] PR fortran/101565 - ICE in gfc_simplify_image_index, at fortran/simplify.c:8234
Hello, Le 29/11/2021 à 22:31, Harald Anlauf via Fortran a écrit : Dear all, a trivial one: we need to check the type of the SUB argument to the coarray IMAGE_INDEX intrinsic. It has to be an array of type integer. Patch by Steve Kargl. I hope at some point he’ll finally come to a working git workflow. Regtested on x86_64-pc-linux-gnu. OK for mainline? Sure.
Re: [PATCH] PR fortran/103473 - [11/12 Regression] ICE in simplify_minmaxloc_nodim, at fortran/simplify.c:5287
Le 29/11/2021 à 23:01, Harald Anlauf via Fortran a écrit : Dear all, another trivial and obvious one, discovered by Gerhard. We can have a NULL pointer dereference simplifying MINLOC/MAXLOC on an array that was not properly declared. OK for mainline / affected 11-branch after regtesting completes? Yes, fine as well. I have the impression that there are quite a number of bugs of this kind, and that maybe we could take a more systematic approach to not try to simplify something with errors. Thanks.
Re: [gomp4] Make OpenACC orphan gang reductions errors
Hi!
On 2017-05-01T18:27:59-0700, Cesar Philippidis wrote:
> This patch promotes all OpenACC gang reductions on orphan loops as
> errors. Accord to the spec, orphan loops are those which are not
> lexically nested inside an OpenACC parallel or kernels regions. I.e.,
> acc loops inside acc routines.
>
> At first I thought this could be a warning because the gang reduction
> finalizer uses an atomic update. However, because there is no
> synchronization between gangs, there is way to guarantee that reduction
> will have completed once a single gang entity returns from the acc
> routine call.
>
> I've applied this patch to gomp-4_0-branch.
... which I've now adapted (with several things to be fixed in follow-up
commits) and pushed to master branch in
commit 2b7dac2c0dcb087da9e4018943c023c0678234a3
"Make OpenACC orphan gang reductions errors", see attached.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From 2b7dac2c0dcb087da9e4018943c023c0678234a3 Mon Sep 17 00:00:00 2001
From: Cesar Philippidis
Date: Mon, 1 May 2017 18:27:59 -0700
Subject: [PATCH] Make OpenACC orphan gang reductions errors
This patch promotes all OpenACC gang reductions on orphan loops as
errors. Accord to the spec, orphan loops are those which are not
lexically nested inside an OpenACC parallel or kernels regions. I.e.,
acc loops inside acc routines.
At first I thought this could be a warning because the gang reduction
finalizer uses an atomic update. However, because there is no
synchronization between gangs, there is way to guarantee that reduction
will have completed once a single gang entity returns from the acc
routine call.
gcc/c/
* c-typeck.c (c_finish_omp_clauses): Emit an error on orphan
OpenACC gang reductions.
gcc/cp/
* semantics.c (finish_omp_clauses): Emit an error on orphan
OpenACC gang reductions.
gcc/fortran/
* openmp.c (oacc_is_parallel, oacc_is_kernels): New 'static'
functions.
(resolve_oacc_loop_blocks): Emit an error on orphan OpenACC gang
reductions.
gcc/
* omp-general.h (enum oacc_loop_flags): Add OLF_REDUCTION enum.
* omp-low.c (lower_oacc_head_mark): Use it to mark OpenACC
reductions.
* omp-offload.c (oacc_loop_auto_partitions): Don't assign gang
level parallelism to orphan reductions.
gcc/testsuite/
* c-c++-common/goacc/nested-reductions-1-routine.c: Adjust.
* c-c++-common/goacc/nested-reductions-2-routine.c: Likewise.
* gcc.dg/goacc/loop-processing-1.c: Likewise.
* gfortran.dg/goacc/nested-reductions-1-routine.f90: Likewise.
* gfortran.dg/goacc/nested-reductions-2-routine.f90: Likewise.
* c-c++-common/goacc/orphan-reductions-1.c: New test.
* c-c++-common/goacc/orphan-reductions-2.c: New test.
* gfortran.dg/goacc/orphan-reductions-1.f90: New test.
* gfortran.dg/goacc/orphan-reductions-2.f90: New test.
libgomp/
* testsuite/libgomp.oacc-fortran/parallel-dims.f90: Temporarily
skip.
Co-Authored-By: Thomas Schwinge
---
gcc/c/c-typeck.c | 8 +
gcc/cp/semantics.c| 8 +
gcc/fortran/openmp.c | 24 ++
gcc/omp-general.h | 3 +-
gcc/omp-low.c | 4 +
gcc/omp-offload.c | 7 +
.../goacc/nested-reductions-1-routine.c | 3 +
.../goacc/nested-reductions-2-routine.c | 9 +
.../c-c++-common/goacc/orphan-reductions-1.c | 56 +
.../c-c++-common/goacc/orphan-reductions-2.c | 87
.../gcc.dg/goacc/loop-processing-1.c | 2 +-
.../goacc/nested-reductions-1-routine.f90 | 3 +
.../goacc/nested-reductions-2-routine.f90 | 9 +
.../gfortran.dg/goacc/orphan-reductions-1.f90 | 206 ++
.../gfortran.dg/goacc/orphan-reductions-2.f90 | 89
.../libgomp.oacc-fortran/parallel-dims.f90| 1 +
16 files changed, 517 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/goacc/orphan-reductions-1.c
create mode 100644 gcc/testsuite/c-c++-common/goacc/orphan-reductions-2.c
create mode 100644 gcc/testsuite/gfortran.dg/goacc/orphan-reductions-1.f90
create mode 100644 gcc/testsuite/gfortran.dg/goacc/orphan-reductions-2.f90
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 7524304f2bd..a025740e618 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -14135,6 +14135,14 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
goto check_dup_generic;
case OMP_CLAUSE_REDUCTION:
+ if (ort == C_ORT_ACC && oacc_get_fn_attrib (current_function_decl)
+ && omp_find_clause (clauses, OMP_CLAUSE_GANG))
+ {
+ error_at (OMP_CLAUSE_LOCATION (c),
+ "gang reduction on an orphan loop");
+ remove = true;
+ break;
+ }
if (reduction_seen == 0)
reduction_seen =
Re: [gomp4] Make OpenACC orphan gang reductions errors
Hi!
On 2017-05-01T18:27:59-0700, Cesar Philippidis wrote:
> --- a/gcc/fortran/openmp.c
> +++ b/gcc/fortran/openmp.c
> @@ -6090,6 +6090,18 @@ resolve_oacc_loop_blocks (gfc_code *code)
> + if (code->op == EXEC_OACC_LOOP
> + && code->ext.omp_clauses->lists[OMP_LIST_REDUCTION]
> + && code->ext.omp_clauses->gang)
> +{
> + for (c = omp_current_ctx; c; c = c->previous)
> + if (!oacc_is_loop (c->code))
> + break;
> + if (c == NULL || !(oacc_is_parallel (c->code)
> + || oacc_is_kernels (c->code)))
> + gfc_error ("gang reduction on an orphan loop at %L", &code->loc);
> +}
To avoid erroneous diagnostics, we also need to handle the OpenACC
'serial' construct here. I've adapted Kwok's relevant patch, and pushed
to master branch commit f1a58ab0db20c0862e8b5039bd448fc8c9799cac
"[OpenACC] Allow gang reductions inside serial constructs", see attached.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From f1a58ab0db20c0862e8b5039bd448fc8c9799cac Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung
Date: Fri, 13 Mar 2020 11:13:49 -0700
Subject: [PATCH] [OpenACC] Allow gang reductions inside serial constructs
... fixing a regression introduced in the preceding
commit 2b7dac2c0dcb087da9e4018943c023c0678234a3
"Make OpenACC orphan gang reductions errors".
gcc/fortran/
* openmp.c (oacc_is_serial, oacc_is_parallel_or_serial): New.
(resolve_oacc_loop_blocks): Use oacc_is_parallel_or_serial instead of
oacc_is_parallel.
libgomp/
* testsuite/libgomp.oacc-fortran/parallel-dims.f90: Remove
temporary skip.
Co-Authored-By: Thomas Schwinge
---
gcc/fortran/openmp.c | 14 +-
.../libgomp.oacc-fortran/parallel-dims.f90 | 1 -
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 4fa38691c01..b4100577e51 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -8334,6 +8334,18 @@ oacc_is_kernels (gfc_code *code)
return code->op == EXEC_OACC_KERNELS || code->op == EXEC_OACC_KERNELS_LOOP;
}
+static bool
+oacc_is_serial (gfc_code *code)
+{
+ return code->op == EXEC_OACC_SERIAL || code->op == EXEC_OACC_SERIAL_LOOP;
+}
+
+static bool
+oacc_is_parallel_or_serial (gfc_code *code)
+{
+ return oacc_is_parallel (code) || oacc_is_serial (code);
+}
+
static gfc_statement
omp_code_to_statement (gfc_code *code)
{
@@ -8644,7 +8656,7 @@ resolve_oacc_loop_blocks (gfc_code *code)
for (c = omp_current_ctx; c; c = c->previous)
if (!oacc_is_loop (c->code))
break;
- if (c == NULL || !(oacc_is_parallel (c->code)
+ if (c == NULL || !(oacc_is_parallel_or_serial (c->code)
|| oacc_is_kernels (c->code)))
gfc_error ("gang reduction on an orphan loop at %L", &code->loc);
}
diff --git a/libgomp/testsuite/libgomp.oacc-fortran/parallel-dims.f90 b/libgomp/testsuite/libgomp.oacc-fortran/parallel-dims.f90
index 80d64030414..fad3d9d6a80 100644
--- a/libgomp/testsuite/libgomp.oacc-fortran/parallel-dims.f90
+++ b/libgomp/testsuite/libgomp.oacc-fortran/parallel-dims.f90
@@ -3,7 +3,6 @@
! { dg-additional-sources parallel-dims-aux.c }
! { dg-do run }
- ! { dg-skip-if TODO { *-*-* } }
! { dg-prune-output "command-line option '-fintrinsic-modules-path=.*' is valid for Fortran but not for C" }
! { dg-additional-options "-fopt-info-note-omp" }
--
2.33.0
Re: [PATCH] [og10] libgomp, Fortran: Fix OpenACC "gang reduction on an orphan loop" error message
Hi!
On 2020-07-20T12:26:48+0200, Frederik Harwath wrote:
> Thomas Schwinge writes:
>>> Can I include the patch in OG10?
> This has been delayed a bit by my vacation, but I have now committed
> the patch.
>> (Ideally, we'd also test 'serial' construct in addition to 'kernels',
>> 'parallel'
> I have included the test cases for the "serial construct".
I've adapted the remaining relevant changes and pushed to master branch
commit c4f4c60457d1657cbd72015de3d818eb6462a0e9
'Re OpenACC "gang reduction on an orphan loop" error message', see
attached.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From c4f4c60457d1657cbd72015de3d818eb6462a0e9 Mon Sep 17 00:00:00 2001
From: Frederik Harwath
Date: Mon, 20 Jul 2020 11:24:21 +0200
Subject: [PATCH] Re OpenACC "gang reduction on an orphan loop" error message
Follow-up to preceding commit 2b7dac2c0dcb087da9e4018943c023c0678234a3
"Make OpenACC orphan gang reductions errors".
gcc/fortran/
* openmp.c (oacc_is_parallel_or_serial): Evolve into...
(oacc_is_compute_construct): ... this function.
(resolve_oacc_loop_blocks): Use "oacc_is_compute_construct"
instead of "oacc_is_parallel_or_serial" for checking that a
loop is not orphaned.
gcc/testsuite/
* gfortran.dg/goacc/orphan-reductions-3.f90: New test
verifying that the "gang reduction on an orphan loop" error message
is not emitted for non-orphaned loops.
* c-c++-common/goacc/orphan-reductions-3.c: Likewise for C and C++.
Co-Authored-By: Thomas Schwinge
---
gcc/fortran/openmp.c | 9 +-
.../c-c++-common/goacc/orphan-reductions-3.c | 102 ++
.../gfortran.dg/goacc/orphan-reductions-3.f90 | 89 +++
3 files changed, 196 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/goacc/orphan-reductions-3.c
create mode 100644 gcc/testsuite/gfortran.dg/goacc/orphan-reductions-3.f90
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index b4100577e51..7950c7fb43d 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -8341,9 +8341,11 @@ oacc_is_serial (gfc_code *code)
}
static bool
-oacc_is_parallel_or_serial (gfc_code *code)
+oacc_is_compute_construct (gfc_code *code)
{
- return oacc_is_parallel (code) || oacc_is_serial (code);
+ return (oacc_is_parallel (code)
+ || oacc_is_kernels (code)
+ || oacc_is_serial (code));
}
static gfc_statement
@@ -8656,8 +8658,7 @@ resolve_oacc_loop_blocks (gfc_code *code)
for (c = omp_current_ctx; c; c = c->previous)
if (!oacc_is_loop (c->code))
break;
- if (c == NULL || !(oacc_is_parallel_or_serial (c->code)
- || oacc_is_kernels (c->code)))
+ if (c == NULL || !(oacc_is_compute_construct (c->code)))
gfc_error ("gang reduction on an orphan loop at %L", &code->loc);
}
diff --git a/gcc/testsuite/c-c++-common/goacc/orphan-reductions-3.c b/gcc/testsuite/c-c++-common/goacc/orphan-reductions-3.c
new file mode 100644
index 000..cd8ad274ebb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/goacc/orphan-reductions-3.c
@@ -0,0 +1,102 @@
+/* Verify that the error message for gang reduction on orphaned OpenACC loops
+ is not reported for non-orphaned loops. */
+
+/* { dg-additional-options "-Wopenacc-parallelism" } */
+
+int
+kernels (int n)
+{
+ int i, s1 = 0, s2 = 0;
+#pragma acc kernels
+ {
+#pragma acc loop gang reduction(+:s1) /* { dg-bogus "gang reduction on an orphan loop" } */
+ for (i = 0; i < n; i++)
+s1 = s1 + 2;
+
+#pragma acc loop gang reduction(+:s2) /* { dg-bogus "gang reduction on an orphan loop" } */
+ for (i = 0; i < n; i++)
+s2 = s2 + 2;
+ }
+ return s1 + s2;
+}
+
+int
+parallel (int n)
+{
+ int i, s1 = 0, s2 = 0;
+#pragma acc parallel
+ {
+#pragma acc loop gang reduction(+:s1) /* { dg-bogus "gang reduction on an orphan loop" } */
+ for (i = 0; i < n; i++)
+s1 = s1 + 2;
+
+#pragma acc loop gang reduction(+:s2) /* { dg-bogus "gang reduction on an orphan loop" } */
+ for (i = 0; i < n; i++)
+s2 = s2 + 2;
+ }
+ return s1 + s2;
+}
+
+int
+serial (int n)
+{
+ int i, s1 = 0, s2 = 0;
+#pragma acc serial /* { dg-warning "region contains gang partitioned code but is not gang partitioned" } */
+ {
+#pragma acc loop gang reduction(+:s1) /* { dg-bogus "gang reduction on an orphan loop" } */
+ for (i = 0; i < n; i++)
+s1 = s1 + 2;
+
+#pragma acc loop gang reduction(+:s2) /* { dg-bogus "gang reduction on an orphan loop" } */
+ for (i = 0; i < n; i++)
+s2 = s2 + 2;
+ }
+ return s1 + s2;
+}
+
+int
+serial_combined (int n)
+{
+ int i, s1 = 0, s2 = 0;
+#pragma acc serial loop gang reduction(+:s1) /* { dg-bogus "gang reduction on an orphan loop" } */
+ /* { dg-warning "region contains gang partitioned code but is not gang partitioned" "" { target *-*-* } .-1 } */
+ for (i
Re: [gomp4] Make OpenACC orphan gang reductions errors
Hi!
On 2017-05-01T18:27:59-0700, Cesar Philippidis wrote:
> gcc/c/
> * c-typeck.c (c_finish_omp_clauses): Emit an error on orphan OpenACC
> gang reductions.
>
> gcc/cp/
> * semantics.c (finish_omp_clauses): Emit an error on orphan OpenACC
> gang reductions.
>
> gcc/fortran/
> * openmp.c (resolve_oacc_loop_blocks): Emit an error on orphan OpenACC
> gang reductions.
As a follow-up, I've pushed to master branch
commit 77d24d43644909852998043335b5a0e09d1e8f02
'Consolidate OpenACC "gang reduction on an orphan loop" checking',
see attached.
Grüße
Thomas
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
>From 77d24d43644909852998043335b5a0e09d1e8f02 Mon Sep 17 00:00:00 2001
From: Thomas Schwinge
Date: Fri, 26 Nov 2021 12:29:26 +0100
Subject: [PATCH] Consolidate OpenACC "gang reduction on an orphan loop"
checking
No need to implement separately in all front ends what we may implement in the
middle end, once for all.
Follow-up to preceding commit 2b7dac2c0dcb087da9e4018943c023c0678234a3
"Make OpenACC orphan gang reductions errors".
gcc/
* omp-offload.c (oacc_loop_process): Implement "gang reduction on
an orphan loop" checking.
gcc/c/
* c-typeck.c (c_finish_omp_clauses): Remove "gang reduction on an
orphan loop" checking.
gcc/cp/
* semantics.c (finish_omp_clauses): Remove "gang reduction on an
orphan loop" checking.
gcc/fortran/
* openmp.c (resolve_oacc_loop_blocks): Remove "gang reduction on
an orphan loop" checking.
(oacc_is_parallel, oacc_is_kernels, oacc_is_serial)
(oacc_is_compute_construct): Remove.
gcc/testsuite/
* gfortran.dg/goacc/orphan-reductions-1.f90: Adjust.
---
gcc/c/c-typeck.c | 8
gcc/cp/semantics.c| 8
gcc/fortran/openmp.c | 37 ---
gcc/omp-offload.c | 20 --
.../gfortran.dg/goacc/orphan-reductions-1.f90 | 8 ++--
5 files changed, 20 insertions(+), 61 deletions(-)
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index a025740e618..7524304f2bd 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -14135,14 +14135,6 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
goto check_dup_generic;
case OMP_CLAUSE_REDUCTION:
- if (ort == C_ORT_ACC && oacc_get_fn_attrib (current_function_decl)
- && omp_find_clause (clauses, OMP_CLAUSE_GANG))
- {
- error_at (OMP_CLAUSE_LOCATION (c),
- "gang reduction on an orphan loop");
- remove = true;
- break;
- }
if (reduction_seen == 0)
reduction_seen = OMP_CLAUSE_REDUCTION_INSCAN (c) ? -1 : 1;
else if (reduction_seen != -2
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index c84caf43251..cd1956497f8 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -6667,14 +6667,6 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
field_ok = ((ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP);
goto check_dup_generic;
case OMP_CLAUSE_REDUCTION:
- if (ort == C_ORT_ACC && oacc_get_fn_attrib (current_function_decl)
- && omp_find_clause (clauses, OMP_CLAUSE_GANG))
- {
- error_at (OMP_CLAUSE_LOCATION (c),
- "gang reduction on an orphan loop");
- remove = true;
- break;
- }
if (reduction_seen == 0)
reduction_seen = OMP_CLAUSE_REDUCTION_INSCAN (c) ? -1 : 1;
else if (reduction_seen != -2
diff --git a/gcc/fortran/openmp.c b/gcc/fortran/openmp.c
index 7950c7fb43d..d120be81467 100644
--- a/gcc/fortran/openmp.c
+++ b/gcc/fortran/openmp.c
@@ -8322,31 +8322,6 @@ resolve_omp_do (gfc_code *code)
}
}
-static bool
-oacc_is_parallel (gfc_code *code)
-{
- return code->op == EXEC_OACC_PARALLEL || code->op == EXEC_OACC_PARALLEL_LOOP;
-}
-
-static bool
-oacc_is_kernels (gfc_code *code)
-{
- return code->op == EXEC_OACC_KERNELS || code->op == EXEC_OACC_KERNELS_LOOP;
-}
-
-static bool
-oacc_is_serial (gfc_code *code)
-{
- return code->op == EXEC_OACC_SERIAL || code->op == EXEC_OACC_SERIAL_LOOP;
-}
-
-static bool
-oacc_is_compute_construct (gfc_code *code)
-{
- return (oacc_is_parallel (code)
- || oacc_is_kernels (code)
- || oacc_is_serial (code));
-}
static gfc_statement
omp_code_to_statement (gfc_code *code)
@@ -8650,18 +8625,6 @@ resolve_oacc_loop_blocks (gfc_code *code)
if (!oacc_is_loop (code))
return;
- if (code->op == EXEC_OACC_LOOP
- && code->ext.omp_clauses->lists[OMP_LIST_REDUCTION]
- && code->ext.omp_clauses->gang)
-{
- fortran_omp_context *c;
- for (c = omp_current_ctx; c; c = c->previous)
- if (!oacc_is_loop (c->code))
- break;
- if (c == NULL || !(oacc_is_compute_construct (c->code)))
- gfc_error ("gang reduction on an orphan loop at %L", &code->
Re: [PATCH] Avoid some -Wunreachable-code-ctrl
Le 29/11/2021 à 16:03, Richard Biener via Gcc-patches a écrit : diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c index f5ba7cecd54..16ee2afc9c0 100644 --- a/gcc/fortran/frontend-passes.c +++ b/gcc/fortran/frontend-passes.c @@ -5229,7 +5229,6 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t exprfn, void *data) case EXPR_OP: WALK_SUBEXPR ((*e)->value.op.op1); WALK_SUBEXPR_TAIL ((*e)->value.op.op2); - break; case EXPR_FUNCTION: for (a = (*e)->value.function.actual; a; a = a->next) WALK_SUBEXPR (a->expr); I’m uncomfortable with the above change. It makes it look like there is a fall through, but there is not. Maybe inline the macro to make the continue explicit, or use WALK_SUBEXPR instead of WALK_SUBEXPR_TAIL and hope the compiler will do the tail call optimization. Mikael
Re: [PATCH] Avoid some -Wunreachable-code-ctrl
On Tue, 30 Nov 2021, Mikael Morin wrote: > Le 29/11/2021 à 16:03, Richard Biener via Gcc-patches a écrit : > > diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c > > index f5ba7cecd54..16ee2afc9c0 100644 > > --- a/gcc/fortran/frontend-passes.c > > +++ b/gcc/fortran/frontend-passes.c > > @@ -5229,7 +5229,6 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t exprfn, > > void *data) > > case EXPR_OP: > >WALK_SUBEXPR ((*e)->value.op.op1); > >WALK_SUBEXPR_TAIL ((*e)->value.op.op2); > > - break; > > case EXPR_FUNCTION: > >for (a = (*e)->value.function.actual; a; a = a->next) > > WALK_SUBEXPR (a->expr); > > I’m uncomfortable with the above change. > It makes it look like there is a fall through, but there is not. > Maybe inline the macro to make the continue explicit, or use WALK_SUBEXPR > instead of WALK_SUBEXPR_TAIL and hope the compiler will do the tail call > optimization. Ah, it follows the style in tree.c:walk_tree_1 where break was used inconsistently after WALK_SUBTREE_TAIL which was then more obvious to me to clean up. I didn't realize the fortran FE only had a single WALK_SUBEXPR_TAIL. I'm not sure inlining will make the situation more clear, for sure using WALK_SUBEXPR would but it might loose the tailcall. Would you accept an additional comment after WALK_SUBEXPR_TAIL like case EXPR_OP: WALK_SUBEXPR ((*e)->value.op.op1); WALK_SUBEXPR_TAIL ((*e)->value.op.op2); /* tail-recurse */ ? Btw, a fallthru would be diagnosed by GCC unless we put /* Fallthru */ here. Maybe renaming WALK_SUBEXPR_TAIL to WALK_SUBEXPR_WITH_CONTINUE or WALK_SUBEXPR_BY_TAIL_RECURSING or WALK_SUBEXPR_TAILRECURSE would be more obvious? Thanks, Richard.
Re: [PATCH] Avoid some -Wunreachable-code-ctrl
On 30/11/2021 14:25, Richard Biener wrote: On Tue, 30 Nov 2021, Mikael Morin wrote: Le 29/11/2021 à 16:03, Richard Biener via Gcc-patches a écrit : diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c index f5ba7cecd54..16ee2afc9c0 100644 --- a/gcc/fortran/frontend-passes.c +++ b/gcc/fortran/frontend-passes.c @@ -5229,7 +5229,6 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t exprfn, void *data) case EXPR_OP: WALK_SUBEXPR ((*e)->value.op.op1); WALK_SUBEXPR_TAIL ((*e)->value.op.op2); - break; case EXPR_FUNCTION: for (a = (*e)->value.function.actual; a; a = a->next) WALK_SUBEXPR (a->expr); I’m uncomfortable with the above change. It makes it look like there is a fall through, but there is not. Maybe inline the macro to make the continue explicit, or use WALK_SUBEXPR instead of WALK_SUBEXPR_TAIL and hope the compiler will do the tail call optimization. Ah, it follows the style in tree.c:walk_tree_1 where break was used inconsistently after WALK_SUBTREE_TAIL which was then more obvious to me to clean up. I didn't realize the fortran FE only had a single WALK_SUBEXPR_TAIL. I'm not sure inlining will make the situation more clear, for sure using WALK_SUBEXPR would but it might loose the tailcall. Would you accept an additional comment after WALK_SUBEXPR_TAIL like case EXPR_OP: WALK_SUBEXPR ((*e)->value.op.op1); WALK_SUBEXPR_TAIL ((*e)->value.op.op2); /* tail-recurse */ My preference would be a gcc_unreachable() or something similar, but I understand it would get a warning as well? Without better idea, I’m fine with an even more explicit comment: /* No fallthru because of the tail recursion above. */ ? Btw, a fallthru would be diagnosed by GCC unless we put /* Fallthru */ here. Sure, but my main concern was misreading from programmers (including me), which is not diagnosed by compilers. Maybe renaming WALK_SUBEXPR_TAIL to WALK_SUBEXPR_WITH_CONTINUE or WALK_SUBEXPR_BY_TAIL_RECURSING or WALK_SUBEXPR_TAILRECURSE would be more obvious? I think the comment above would be enough. Thanks.
Re: [PATCH] Avoid some -Wunreachable-code-ctrl
On Tue, 30 Nov 2021, Mikael Morin wrote: > On 30/11/2021 14:25, Richard Biener wrote: > > On Tue, 30 Nov 2021, Mikael Morin wrote: > > > >> Le 29/11/2021 ? 16:03, Richard Biener via Gcc-patches a ?crit : > >>> diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c > >>> index f5ba7cecd54..16ee2afc9c0 100644 > >>> --- a/gcc/fortran/frontend-passes.c > >>> +++ b/gcc/fortran/frontend-passes.c > >>> @@ -5229,7 +5229,6 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t > >>> exprfn, > >>> void *data) > >>> case EXPR_OP: > >>> WALK_SUBEXPR ((*e)->value.op.op1); > >>> WALK_SUBEXPR_TAIL ((*e)->value.op.op2); > >>> - break; > >>> case EXPR_FUNCTION: > >>> for (a = (*e)->value.function.actual; a; a = a->next) > >>> WALK_SUBEXPR (a->expr); > >> > >> I?m uncomfortable with the above change. > >> It makes it look like there is a fall through, but there is not. > >> Maybe inline the macro to make the continue explicit, or use WALK_SUBEXPR > >> instead of WALK_SUBEXPR_TAIL and hope the compiler will do the tail call > >> optimization. > > > > Ah, it follows the style in tree.c:walk_tree_1 where break was used > > inconsistently after WALK_SUBTREE_TAIL which was then more obvious > > to me to clean up. I didn't realize the fortran FE only had a > > single WALK_SUBEXPR_TAIL. > > > > I'm not sure inlining will make the situation more clear, for > > sure using WALK_SUBEXPR would but it might loose the tailcall. > > > > Would you accept an additional comment after WALK_SUBEXPR_TAIL like > > > >case EXPR_OP: > > WALK_SUBEXPR ((*e)->value.op.op1); > > WALK_SUBEXPR_TAIL ((*e)->value.op.op2); > > /* tail-recurse */ > > > My preference would be a gcc_unreachable() or something similar, but I > understand it would get a warning as well? > > Without better idea, I?m fine with an even more explicit comment: > > /* No fallthru because of the tail recursion above. */ > > > ? Btw, a fallthru would be diagnosed by GCC unless we put > > > > /* Fallthru */ > > > > here. > Sure, but my main concern was misreading from programmers (including me), > which is not diagnosed by compilers. > > > Maybe renaming WALK_SUBEXPR_TAIL to WALK_SUBEXPR_WITH_CONTINUE > > or WALK_SUBEXPR_BY_TAIL_RECURSING or WALK_SUBEXPR_TAILRECURSE would > > be more obvious? > > > I think the comment above would be enough. Installed as follows. Richard. >From e5c2a436ef7596d254ffefd279742382b1ff546b Mon Sep 17 00:00:00 2001 From: Richard Biener Date: Tue, 30 Nov 2021 15:25:17 +0100 Subject: [PATCH] Add comment to indicate tail recursion To: gcc-patc...@gcc.gnu.org My previous change removed an unreachable break; there (an unreachable continue; would have been more to the point). The following re-adds a comment explaining that WALK_SUBEXPR_TAIL does not fall through but tail recurses. 2021-11-30 Richard Biener gcc/fortran/ * frontend-passes.c (gfc_expr_walker): Add comment to indicate tail recursion. --- gcc/fortran/frontend-passes.c | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c index 16ee2afc9c0..4764c834f4f 100644 --- a/gcc/fortran/frontend-passes.c +++ b/gcc/fortran/frontend-passes.c @@ -5229,6 +5229,7 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t exprfn, void *data) case EXPR_OP: WALK_SUBEXPR ((*e)->value.op.op1); WALK_SUBEXPR_TAIL ((*e)->value.op.op2); + /* No fallthru because of the tail recursion above. */ case EXPR_FUNCTION: for (a = (*e)->value.function.actual; a; a = a->next) WALK_SUBEXPR (a->expr); -- 2.31.1
[power-ieee128] What should the math functions be annotated with?
Hi, looking at the head of a generated gfortran library math function, for example bessel_r16.c, #if defined(GFC_REAL_16_IS_FLOAT128) #define MATHFUNC(funcname) funcname ## q #else #define MATHFUNC(funcname) funcname ## l #endif So (I suppose I can unravel the m4 code to generate this:-) what which function name should be called for the REAL(KIND=17) function to be generated? Is, for example, jnq what we want to have? Regards Thomas
Re: [PATCH, Fortran] Fix setting of array lower bound for named arrays
On 29.11.21 22:11, Harald Anlauf wrote: "A whole array is a named array or a structure component whose final part-ref is an array component name; no subscript list is appended." I think in "h(3)" there is not really a named array – thus I read it as if the "Otherwise ... result value is 1" applies. If you read on in the standard: "The appearance of a whole array variable in an executable construct specifies all the elements of the array ..." which might make you/makes me think that the sentence before that one could need an official interpretation... I am not sure whether I understand what part of the spec you wonder about. (I mean besides that 'variable' can also mean referencing a data-pointer-returning function.) Question: What do NAG/flang/... report for lbound(h(3)) - also [3] – or [1] as gfortran? I've submitted a reduced example to the Intel Fortran Forum: https://community.intel.com/t5/Intel-Fortran-Compiler/Allocate-with-SOURCE-and-bounds/m-p/1339992#M158535 There are good chances that Steve Lionel reads and comments on it. So far only "FortranFan" has replied – and he comes to the same conclusion as my reading, albeit without referring to the standard. 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] PR fortran/101565 - ICE in gfc_simplify_image_index, at fortran/simplify.c:8234
Hi Mikael, Am 30.11.21 um 12:25 schrieb Mikael Morin: Hello, Le 29/11/2021 à 22:31, Harald Anlauf via Fortran a écrit : Dear all, a trivial one: we need to check the type of the SUB argument to the coarray IMAGE_INDEX intrinsic. It has to be an array of type integer. Patch by Steve Kargl. I hope at some point he’ll finally come to a working git workflow. Initially I had to rethink my workflow habits when switching from svn to git. But after a steep learning curve I wouldn't want to go back. One day Steve might see it same way. Regtested on x86_64-pc-linux-gnu. OK for mainline? Sure. Thanks, Harald
Re: [PATCH, Fortran] Fix setting of array lower bound for named arrays
Hi Tobias, Am 30.11.21 um 18:24 schrieb Tobias Burnus: On 29.11.21 22:11, Harald Anlauf wrote: "A whole array is a named array or a structure component whose final part-ref is an array component name; no subscript list is appended." I think in "h(3)" there is not really a named array – thus I read it as if the "Otherwise ... result value is 1" applies. If you read on in the standard: "The appearance of a whole array variable in an executable construct specifies all the elements of the array ..." which might make you/makes me think that the sentence before that one could need an official interpretation... I am not sure whether I understand what part of the spec you wonder about. (I mean besides that 'variable' can also mean referencing a data-pointer-returning function.) strictly speaking you're now talking about the text for LBOUND, and your quote is not from the standard section about the ALLOCATE statement. And there are several places in the standard document where there is an explicit reference to LBOUND when talking about what the bounds should be. This is why I am unhappy with the text about ALLOCATE, not about LBOUND. Question: What do NAG/flang/... report for lbound(h(3)) - also [3] – or [1] as gfortran? I've submitted a reduced example to the Intel Fortran Forum: https://community.intel.com/t5/Intel-Fortran-Compiler/Allocate-with-SOURCE-and-bounds/m-p/1339992#M158535 There are good chances that Steve Lionel reads and comments on it. So far only "FortranFan" has replied – and he comes to the same conclusion as my reading, albeit without referring to the standard. You seem to be quite convinced with your interpretation, while I am simply confused. So go ahead and apply to mainline. Let's see if we learn more. I do hope I will. 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
Re: [PATCH, Fortran] Fix setting of array lower bound for named arrays
On 11/30/21 8:54 PM, Harald Anlauf via Fortran wrote: Hi Tobias, You seem to be quite convinced with your interpretation, while I am simply confused. If both compiler developers are confused, and actual compiler implementations differ in their outcomes of the test case, IMNSHO it is time to ask the Fortran Standardization Committee for an interpretation (of the standard's text). Kind regards, -- Toon Moene - e-mail: t...@moene.org - phone: +31 346 214290 Saturnushof 14, 3738 XG Maartensdijk, The Netherlands
Re: [PATCH, fortran] Improve expansion of constant array expressions within constructors
Hello,
On 27/11/2021 21:56, Harald Anlauf via Fortran wrote:
diff --git a/gcc/fortran/array.c b/gcc/fortran/array.c
index 6552eaf3b0c..fbc66097c80 100644
--- a/gcc/fortran/array.c
+++ b/gcc/fortran/array.c
@@ -1804,6 +1804,12 @@ expand_constructor (gfc_constructor_base base)
if (empty_constructor)
empty_ts = e->ts;
+ /* Simplify constant array expression/section within constructor. */
+ if (e->expr_type == EXPR_VARIABLE && e->rank > 0 && e->ref
+ && e->symtree && e->symtree->n.sym
+ && e->symtree->n.sym->attr.flavor == FL_PARAMETER)
+ gfc_simplify_expr (e, 0);
+
if (e->expr_type == EXPR_ARRAY)
{
if (!expand_constructor (e->value.constructor))
There is another simplification call just a few lines below, that I
thought could just be moved up.
But it works on a copy of the expression, and managing the copy makes it
complex as well, so let’s do it your way.
OK.
