Re: [PATCH, v2] PR fortran/104573 - ICE in resolve_structure_cons, at fortran/resolve.cc:1299

[Mikael Morin]

Le 01/03/2022 à 23:18, Harald Anlauf via Fortran a écrit :


I do hope I got you right.  The attached patch fixes your variant
as well as the original testcase, and regtests fine.
Just to be sure: is this what you were thinking of?


Indeed, that’s what I had in mind.
Nice, I didn’t expect that the requested change would be enough to fix 
my testcase, as there was a different ICE.


Thanks.

Re: OpenACC 'kernels' decomposition: Mark variables used in synthesized data clauses as addressable [PR100280]

[Jakub Jelinek via Fortran]

On Tue, Mar 01, 2022 at 05:46:20PM +0100, Thomas Schwinge wrote:
> OK to proceed in this way?

With a suitable ChangeLog entry and one nit fixed yes.

> --- gcc/omp-low.cc
> +++ gcc/omp-low.cc
> @@ -188,7 +188,7 @@ struct omp_context
>  static splay_tree all_contexts;
>  static int taskreg_nesting_level;
>  static int target_nesting_level;
> -static bitmap task_shared_vars;
> +static bitmap make_addressable_vars;
>  static bitmap global_nonaddressable_vars;
>  static vec taskreg_contexts;
>  static vec task_cpyfns;
> @@ -572,9 +572,9 @@ use_pointer_for_field (tree decl, omp_context *shared_ctx)
>   /* Taking address of OUTER in lower_send_shared_vars
>  might need regimplification of everything that uses the
>  variable.  */
> - if (!task_shared_vars)
> -   task_shared_vars = BITMAP_ALLOC (NULL);
> - bitmap_set_bit (task_shared_vars, DECL_UID (outer));
> + if (!make_addressable_vars)
> +   make_addressable_vars = BITMAP_ALLOC (NULL);
> + bitmap_set_bit (make_addressable_vars, DECL_UID (outer));

Has the MUA replaced tabs with spaces?

> --- gcc/omp-oacc-kernels-decompose.cc
> +++ gcc/omp-oacc-kernels-decompose.cc
> @@ -845,7 +845,11 @@ maybe_build_inner_data_region (location_t loc, gimple 
> *body,
>   prev_mapped_var = v;
> 
>   /* See .  */
> - TREE_ADDRESSABLE (v) = 1;
> + if (!TREE_ADDRESSABLE (v))
> +   {
> + /* Request that OMP lowering make 'v' addressable.  */
> + OMP_CLAUSE_MAP_DECL_MAKE_ADDRESSABLE (new_clause) = 1;
> +   }

That is a single statement body, so shouldn't have {}s around it.

Jakub

[PATCH][v2] openmp, fortran: Check that the type of an event handle in a detach clause is suitable [PR104131]

[Kwok Cheung Yeung]

Hello

I have updated the patch to catch array elements and structure 
components as additional checks, in addition to checking that the 
variable is a scalar.


The check has been moved to the end of resolve_omp_clauses as it is more 
appropriate there. This gets rid of the additional 'Unexpected !$OMP END 
TASK statement' error, since the type error is now caught after the 
matching phase.


Coarrays (with the testcases in pr104131-2.f90) can be dealt with in a 
separate patch. Is this part okay for trunk?


Thanks

Kwok

On 01/03/2022 3:37 pm, Mikael Morin wrote:

So, if I try to sum up what has been gathered in this thread:

  - pr104131.f90 is invalid, as x is not scalar.
    Checks are better done in resolve_omp_clauses after a call
    to gfc_resolve_expr.
    Checking expr->sym->attr.dimension seems to cover more cases than
    expr->rank > 0.

  - pr104131-2.f90 is valid and should be accepted.

  - Some other cases should be rejected, including x[1] (coindexed
    variable), x(1) (array element), x%comp (structure component).

Is that correct? Anything else?

Regarding the expr->rank vs expr->sym->attr.dimension controversy, my 
take is that it should stick to the error message.  Use expr->rank is 
the error is about scalar vs array, use expr->sym->attr.dimension if 
it’s about subobject-ness of an array variable.


Coming back to the PR, the ICE backtraces for pr104131.f90 and 
pr104131-2.f90 are different and should probably be treated separatedly.
I don’t know how difficult the bullet 2 above would be, but bullet 1 and 
3 seem quite doable.From 3ed6eb1e38ad2a25c6eca18f9ff4d05d3f227db3 Mon Sep 17 00:00:00 2001
From: Kwok Cheung Yeung 
Date: Wed, 2 Mar 2022 17:09:45 +
Subject: [PATCH] openmp, fortran: Check that the type of an event handle in a
 detach clause is suitable [PR104131]

This rejects variables that are array types, array elements or derived type
members when used as the event handle inside a detach clause (in accordance
with the OpenMP specification).  This would previously lead to an ICE.

2022-03-02  Kwok Cheung Yeung  

gcc/fortran/

PR fortran/104131
* openmp.cc (gfc_match_omp_detach): Move check for type of event
handle to...
(resolve_omp_clauses) ...here.  Also check that the event handle is
not an array, or an array access or structure element access.

gcc/testsuite/

PR fortran/104131
* gfortran.dg/gomp/pr104131.f90: New.
* gfortran.dg/gomp/task-detach-1.f90: Update expected error message.
---
 gcc/fortran/openmp.cc | 34 +--
 gcc/testsuite/gfortran.dg/gomp/pr104131.f90   | 26 ++
 .../gfortran.dg/gomp/task-detach-1.f90|  4 +--
 3 files changed, 51 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr104131.f90

diff --git a/gcc/fortran/openmp.cc b/gcc/fortran/openmp.cc
index 19142c4d8d0..16cd03a3d67 100644
--- a/gcc/fortran/openmp.cc
+++ b/gcc/fortran/openmp.cc
@@ -531,14 +531,6 @@ gfc_match_omp_detach (gfc_expr **expr)
   if (gfc_match_variable (expr, 0) != MATCH_YES)
 goto syntax_error;
 
-  if ((*expr)->ts.type != BT_INTEGER || (*expr)->ts.kind != gfc_c_intptr_kind)
-{
-  gfc_error ("%qs at %L should be of type "
-"integer(kind=omp_event_handle_kind)",
-(*expr)->symtree->n.sym->name, &(*expr)->where);
-  return MATCH_ERROR;
-}
-
   if (gfc_match_char (')') != MATCH_YES)
 goto syntax_error;
 
@@ -7581,9 +7573,29 @@ resolve_omp_clauses (gfc_code *code, gfc_omp_clauses 
*omp_clauses,
gfc_error ("%s must contain at least one MAP clause at %L",
   p, &code->loc);
 }
-  if (!openacc && omp_clauses->mergeable && omp_clauses->detach)
-gfc_error ("% clause at %L must not be used together with "
-  "% clause", &omp_clauses->detach->where);
+
+  if (!openacc && omp_clauses->detach)
+{
+  if (!gfc_resolve_expr (omp_clauses->detach)
+ || omp_clauses->detach->ts.type != BT_INTEGER
+ || omp_clauses->detach->ts.kind != gfc_c_intptr_kind
+ || omp_clauses->detach->rank != 0)
+   gfc_error ("%qs at %L should be a scalar of type "
+  "integer(kind=omp_event_handle_kind)",
+  omp_clauses->detach->symtree->n.sym->name,
+  &omp_clauses->detach->where);
+  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->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 "
+  "a derived type or class", &omp_clauses->detach->where);
+
+  if (omp_clauses->mergeable)
+   gfc_error ("% clause at %L must not be used together with "
+  "% cla

Re: [PATCH][v2] openmp, fortran: Check that the type of an event handle in a detach clause is suitable [PR104131]

[Jakub Jelinek via Fortran]

On Wed, Mar 02, 2022 at 05:22:21PM +, Kwok Cheung Yeung wrote:
> I have updated the patch to catch array elements and structure components as
> additional checks, in addition to checking that the variable is a scalar.
> 
> The check has been moved to the end of resolve_omp_clauses as it is more
> appropriate there. This gets rid of the additional 'Unexpected !$OMP END
> TASK statement' error, since the type error is now caught after the matching
> phase.
> 
> Coarrays (with the testcases in pr104131-2.f90) can be dealt with in a
> separate patch. Is this part okay for trunk?

LGTM, thanks.
Yes, coarrays is something that we need to deal with in all the clauses,
not just this one, so before we claim OpenMP 5.0 support we need to go
through it fully.

Jakub