Re: [Patch, fortran] PR fortran/100120/100816/100818/100819/100821 problems raised by aggregate data types
Hi José, Patch tested only on x86_64-pc-linux-gnu. Also tested on darwin20. The patch is OK for me. Thanks for the work, Dominique
Re: [Patch] Fortran/OpenMP: Add omp loop [PR99928]
On Thu, Jun 03, 2021 at 12:30:50AM +0200, Tobias Burnus wrote:
> This patch adds support for 'omp loop' to gfortran including the combined
> constructs. It also fixes some splitting issues with clauses in
> combined constructs.
>
> It does not attempt to clean up all remaining Fortran issues with
> clauses in combined constructs (cf. below + PR).
>
> * * *
>
> Since 'parallel loop' is now supported, using
> !$omp parallel &
> !$acc& loop
> no longer gave an error. (Same result before: '!$acc& do'.)
I think best would be just to remove that part of the testcase
in the loop patch and handle the !$omp with !$acc continuations
and vice versa in a separate change. That seems to be a preexisting
bug not really related to whether we support loop or not.
fatal_error is certainly not ideal, but I can understand fixing
it otherwise might be hard.
Wonder if we just shouldn't treat the incorrect continuations
as if they were simple comments.
> The check does (for Fortran – and alike for C):
> ! { dg-final { scan-tree-dump-not "omp
> distribute\[^\n\r]*lastprivate\\(j00\\)" "gimple" } }
> ! { dg-final { scan-tree-dump-not "omp
> parallel\[^\n\r]*lastprivate\\(j00\\)" "gimple" } }
> ! { dg-final { scan-tree-dump-not "omp for\[^\n\r]*lastprivate\\(j00\\)"
> "gimple" } }
> ! { dg-final { scan-tree-dump "omp simd\[^\n\r]*linear\\(j00:1\\)" "gimple"
> } }
> !$omp distribute parallel do simd default(none)
> do j00 = 1, 64
> end do
>
> While C generates (original dump)
>
> #pragma omp distribute
> #pragma omp parallel default(none)
> #pragma omp for nowait
> #pragma omp simd
>
> Fortran generates the same except for:
> #pragma omp simd linear(j00:1)
The *-7 testcase is not appropriate for Fortran, it tests the
IV declared in the construct cases which Fortran has no counterpart for,
even if the IV is first seen within the construct, that would implicitly
declare it outside of the construct.
> + if (gfc_match ("teams )") == MATCH_YES)
> + c->bind = OMP_BIND_TEAMS;
> + else if (gfc_match ("parallel )") == MATCH_YES)
> + c->bind = OMP_BIND_PARALLEL;
> + else if (gfc_match ("thread )") == MATCH_YES)
> + c->bind = OMP_BIND_THREAD;
> + else
> + {
> + gfc_error ("Expected TEAMS, PARALLEL or THEAD as binding in "
Typo, s/THEAD/THREAD/
> + if (clauses->bind == OMP_BIND_TEAMS)
> +OMP_CLAUSE_BIND_KIND (c) = OMP_CLAUSE_BIND_TEAMS;
> + else if (clauses->bind == OMP_BIND_PARALLEL)
> +OMP_CLAUSE_BIND_KIND (c) = OMP_CLAUSE_BIND_PARALLEL;
> + else if (clauses->bind == OMP_BIND_THREAD)
> +OMP_CLAUSE_BIND_KIND (c) = OMP_CLAUSE_BIND_THREAD;
> + else
> + gcc_unreachable ();
Wouldn't a switch (clauses->bind) be cleaner?
Otherwise LGTM.
Jakub
PING [PATCH] PR fortran/99839 - [9/10/11/12 Regression] ICE in inline_matmul_assign, at fortran/frontend-passes.c:4234
*PING* > Gesendet: Donnerstag, 27. Mai 2021 um 22:20 Uhr > Von: "Harald Anlauf" > An: "fortran" , "gcc-patches" > Betreff: [PATCH] PR fortran/99839 - [9/10/11/12 Regression] ICE in > inline_matmul_assign, at fortran/frontend-passes.c:4234 > > Dear Fortranners, > > frontend optimization tries to inline matmul, but then it also needs > to take care of the assignment to the result array. If that one is > not of canonical type, we currently get an ICE. The straightforward > solution is to simply punt in those cases and avoid inlining. > > Regtested on x86_64-pc-linux-gnu. > > OK for mainline? Backport to affected branches? > > Thanks, > Harald > > > Fortran - ICE in inline_matmul_assign > > Restrict inlining of matmul to those cases where assignment to the > result array does not need special treatment. > > gcc/fortran/ChangeLog: > > PR fortran/99839 > * frontend-passes.c (inline_matmul_assign): Do not inline matmul > if the assignment to the resulting array if it is not of canonical > type (real/integer/complex/logical). > > gcc/testsuite/ChangeLog: > > PR fortran/99839 > * gfortran.dg/inline_matmul_25.f90: New test. > >
[PATCH] [og11] OpenMP/OpenACC: Move array_ref/indirect_ref handling code out of extract_base_bit_offset
At Richard Biener's suggestion, this patch undoes the following patch:
https://gcc.gnu.org/pipermail/gcc-patches/2021-June/571712.html
and moves the stripping of ARRAY_REFS/INDIRECT_REFS out of
extract_base_bit_offset and back into the (two) call sites of the
function. The difference between the two ways of looking through these
nodes comes down to (I think) what processing has been done on the
clause in question already: in the case where BASE_REF is non-NULL,
we are processing an OMP_CLAUSE_DECL for the first time. Conversely,
when BASE_REF is NULL, we are processing a node from the sorted list
that is being constructed after a GOMP_MAP_STRUCT node.
In practice, this appears to have no effect on test results (and I
haven't come up with a new test where it makes a difference), though
I will fold this version into the next iteration of these patches sent
upstream in order to avoid potentially introducing a bug.
Tested with offloading to NVPTX. I will apply to the og11 branch after
the weekend.
2021-06-03 Julian Brown
gcc/
* gimplify.c (extract_base_bit_offset): Don't look through ARRAY_REFs or
INDIRECT_REFs here.
(build_struct_group): Reinstate previous behaviour for handling
ARRAY_REFs/INDIRECT_REFs.
---
gcc/gimplify.c | 46 ++
1 file changed, 26 insertions(+), 20 deletions(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index c6ebef8e41c..1742a2cb564 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -8526,20 +8526,6 @@ extract_base_bit_offset (tree base, tree *base_ind, tree
*base_ref,
if (base_ref)
*base_ref = NULL_TREE;
- if (TREE_CODE (base) == ARRAY_REF)
-{
- while (TREE_CODE (base) == ARRAY_REF)
- base = TREE_OPERAND (base, 0);
- if (TREE_CODE (base) != COMPONENT_REF
- || TREE_CODE (TREE_TYPE (base)) != ARRAY_TYPE)
- return NULL_TREE;
-}
- else if (TREE_CODE (base) == INDIRECT_REF
- && TREE_CODE (TREE_OPERAND (base, 0)) == COMPONENT_REF
- && (TREE_CODE (TREE_TYPE (TREE_OPERAND (base, 0)))
- == REFERENCE_TYPE))
-base = TREE_OPERAND (base, 0);
-
base = get_inner_reference (base, &bitsize, &bitpos, &offset, &mode,
&unsignedp, &reversep, &volatilep);
@@ -9116,11 +9102,17 @@ build_struct_group (struct gimplify_omp_ctx *ctx,
poly_offset_int coffset;
poly_int64 cbitpos;
tree base_ind, base_ref, tree_coffset;
+ tree ocd = OMP_CLAUSE_DECL (c);
bool openmp = !(region_type & ORT_ACC);
- tree base = extract_base_bit_offset (OMP_CLAUSE_DECL (c), &base_ind,
- &base_ref, &cbitpos, &coffset,
- &tree_coffset, openmp);
+ while (TREE_CODE (ocd) == ARRAY_REF)
+ocd = TREE_OPERAND (ocd, 0);
+
+ if (TREE_CODE (ocd) == INDIRECT_REF)
+ocd = TREE_OPERAND (ocd, 0);
+
+ tree base = extract_base_bit_offset (ocd, &base_ind, &base_ref, &cbitpos,
+ &coffset, &tree_coffset, openmp);
bool do_map_struct = (base == decl && !tree_coffset);
@@ -9347,9 +9339,23 @@ build_struct_group (struct gimplify_omp_ctx *ctx,
poly_offset_int offset;
poly_int64 bitpos;
tree tree_offset;
- tree base = extract_base_bit_offset (sc_decl, NULL, NULL,
-&bitpos, &offset,
-&tree_offset, openmp);
+
+ if (TREE_CODE (sc_decl) == ARRAY_REF)
+ {
+ while (TREE_CODE (sc_decl) == ARRAY_REF)
+ sc_decl = TREE_OPERAND (sc_decl, 0);
+ if (TREE_CODE (sc_decl) != COMPONENT_REF
+ || TREE_CODE (TREE_TYPE (sc_decl)) != ARRAY_TYPE)
+ break;
+ }
+ else if (TREE_CODE (sc_decl) == INDIRECT_REF
+&& TREE_CODE (TREE_OPERAND (sc_decl, 0)) == COMPONENT_REF
+&& (TREE_CODE (TREE_TYPE (TREE_OPERAND (sc_decl, 0)))
+== REFERENCE_TYPE))
+ sc_decl = TREE_OPERAND (sc_decl, 0);
+
+ tree base = extract_base_bit_offset (sc_decl, NULL, NULL, &bitpos,
+&offset, &tree_offset, openmp);
if (!base || !operand_equal_p (base, decl, 0))
break;
if (scp)
--
2.29.2
Re: [Patch] Fortran/OpenMP: Add omp loop [PR99928]
On Thu, Jun 03, 2021 at 05:07:22PM +0200, Jakub Jelinek via Gcc-patches wrote: > I think best would be just to remove that part of the testcase > in the loop patch and handle the !$omp with !$acc continuations > and vice versa in a separate change. That seems to be a preexisting > bug not really related to whether we support loop or not. > fatal_error is certainly not ideal, but I can understand fixing > it otherwise might be hard. > Wonder if we just shouldn't treat the incorrect continuations > as if they were simple comments. Perhaps can't it gfc_error_now for the mixing of OMP and ACC line continuations and then act as if the incorrect ones were comments (like for -fopenmp -fno-openacc or -fopenacc -fno-openmp)? Jakub
