[Patch] OpenMP/Fortran: Reject nonintrinsic assignments in OMP WORKSHARE [PR100633]

2021-05-17 Thread Tobias Burnus

OK for mainline?
It is an ice-on-invalid; does a GCC 11 backport nonetheless make sense?

Tobias

-
Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
Thürauf
OpenMP/Fortran: Reject nonintrinsic assignments in OMP WORKSHARE [PR100633]

	PR fortran/100633

gcc/fortran/ChangeLog:

	* resolve.c (gfc_resolve_code): Reject nonintrinsic assignments in
	OMP WORKSHARE.

gcc/testsuite/ChangeLog:

	* gfortran.dg/gomp/workshare-59.f90: New test.

 gcc/fortran/resolve.c   |  6 ++
 gcc/testsuite/gfortran.dg/gomp/workshare-59.f90 | 26 +
 2 files changed, 32 insertions(+)

diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index c02bbed8739..747516fbc1d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -11940,6 +11940,12 @@ start:
 
 	  if (resolve_ordinary_assign (code, ns))
 	{
+	  if (omp_workshare_flag)
+		{
+		  gfc_error ("Expected intrinsic assignment in OMP WORKSHARE "
+			 "at %L", &code->loc);
+		  break;
+		}
 	  if (code->op == EXEC_COMPCALL)
 		goto compcall;
 	  else
diff --git a/gcc/testsuite/gfortran.dg/gomp/workshare-59.f90 b/gcc/testsuite/gfortran.dg/gomp/workshare-59.f90
new file mode 100644
index 000..65d04c2b55d
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/workshare-59.f90
@@ -0,0 +1,26 @@
+! PR fortran/100633
+
+module defined_assign
+  interface assignment(=)
+module procedure work_assign
+  end interface
+
+  contains
+subroutine work_assign(a,b)
+  integer, intent(out) :: a
+  logical, intent(in) :: b(:)
+end subroutine work_assign
+end module defined_assign
+
+program omp_workshare
+  use defined_assign
+
+  integer :: a
+  logical :: l(10)
+  l = .TRUE.
+
+  !$omp workshare
+  a = l   ! { dg-error "Expected intrinsic assignment in OMP WORKSHARE" }
+  !$omp end workshare
+
+end program omp_workshare


Re: [Patch] OpenMP/Fortran: Reject nonintrinsic assignments in OMP WORKSHARE [PR100633]

2021-05-17 Thread Jakub Jelinek via Fortran
On Mon, May 17, 2021 at 12:27:22PM +0200, Tobias Burnus wrote:
> OK for mainline?
> It is an ice-on-invalid; does a GCC 11 backport nonetheless make sense?
> 
> Tobias
> 
> -
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München 
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank 
> Thürauf

> OpenMP/Fortran: Reject nonintrinsic assignments in OMP WORKSHARE [PR100633]
> 
>   PR fortran/100633
> 
> gcc/fortran/ChangeLog:
> 
>   * resolve.c (gfc_resolve_code): Reject nonintrinsic assignments in
>   OMP WORKSHARE.
> 
> gcc/testsuite/ChangeLog:
> 
>   * gfortran.dg/gomp/workshare-59.f90: New test.

LGTM for both trunk and 11.
Thanks.

Jakub



Re: [PATCH 4/5] Rework indirect struct handling for OpenACC/OpenMP in gimplify.c

2021-05-17 Thread Bernd Edlinger
On 5/14/21 11:26 PM, Julian Brown wrote:
> This patch reworks indirect struct handling in gimplify.c (i.e. for struct
> components mapped with "mystruct->a[0:n]", "mystruct->b", etc.), for
> both OpenACC and OpenMP.  The key observation leading to these changes
> was that component mappings of references-to-structures is already
> implemented and working, and indirect struct component handling via a
> pointer can work quite similarly.  That lets us remove some earlier,
> special-case handling for mapping indirect struct component accesses
> for OpenACC, which required the pointed-to struct to be manually mapped
> before the indirect component mapping.
> 
> With this patch, you can map struct components directly (e.g. an array
> slice "mystruct->a[0:n]") just like you can map a non-indirect struct
> component slice ("mystruct.a[0:n]"). Both references-to-pointers (with
> the former syntax) and references to structs (with the latter syntax)
> work now.
> 
> For Fortran class pointers, we no longer re-use GOMP_MAP_TO_PSET for the
> class metadata (the structure that points to the class data and vptr)
> -- it is instead treated as any other struct.
> 
> For C++, the struct handling also works for class members ("this->foo"),
> without having to explicitly map "this[:1]" first.
> 
> For OpenACC, we permit chained indirect component references
> ("mystruct->a->b[0:n]"), though only the last part of such mappings will
> trigger an attach/detach operation.  To properly use such a construct
> on the target, you must still manually map "mystruct->a[:1]" first --
> but there's no need to map "mystruct[:1]" explicitly before that.
> 
> This patch incorporates parts of Chung-Lin's patch "Recommit "Enable
> gimplify GOMP_MAP_STRUCT handling of (COMPONENT_REF (INDIRECT_REF
> ...)) map clauses"." from the og10 branch.
> 
> OK for trunk?
> 
> Thanks,
> 
> Julian
> 
> 2021-05-14  Julian Brown  
>   Chung-Lin Tang  
> 
> gcc/fortran/
>   * trans-openmp.c (gfc_trans_omp_clauses): Don't create GOMP_MAP_TO_PSET
>   mappings for class metadata, nor GOMP_MAP_POINTER mappings for
>   POINTER_TYPE_P decls.
> 
> gcc/
>   * gimplify.c (tree-hash-traits.h): Include.
>   (extract_base_bit_offset): Add BASE_IND parameter.  Handle
>   pointer-typed indirect references alongside reference-typed ones.
>   (strip_components_and_deref, aggregate_base_p): New functions.
>   (build_struct_group): Update struct_map_to_clause type.  Add pointer
>   type indirect ref handling, including chained references.  Handle
>   pointers and references to structs in OpenACC regions as well as
>   OpenMP ones.
>   (gimplify_scan_omp_clauses): Remove struct_deref_set handling.  Rework
>   pointer-type indirect structure access handling to work more like
>   the reference-typed handling.
>   * omp-low.c (scan_sharing_clauses): Handle pointer-type indirect struct
>   references, and references to pointers to structs also.
> 
> gcc/testsuite/
>   * g++.dg/goacc/member-array-acc.C: New test (XFAILed for now).
>   * g++.dg/gomp/member-array-omp.C: New test (XFAILed for now).
> 
> libgomp/
>   * testsuite/libgomp.oacc-c-c++-common/deep-copy-15.c: New test.
>   * testsuite/libgomp.oacc-c-c++-common/deep-copy-16.c: New test.
>   * testsuite/libgomp.oacc-c++/deep-copy-17.C: New test.
> ---
>  gcc/fortran/trans-openmp.c|  20 +-
>  gcc/gimplify.c| 285 ++
>  gcc/omp-low.c |  16 +-
>  gcc/testsuite/g++.dg/goacc/member-array-acc.C |  14 +
>  gcc/testsuite/g++.dg/gomp/member-array-omp.C  |  14 +
>  .../testsuite/libgomp.oacc-c++/deep-copy-17.C | 101 +++
>  .../libgomp.oacc-c-c++-common/deep-copy-15.c  |  71 +
>  .../libgomp.oacc-c-c++-common/deep-copy-16.c  | 231 ++
>  8 files changed, 612 insertions(+), 140 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/goacc/member-array-acc.C
>  create mode 100644 gcc/testsuite/g++.dg/gomp/member-array-omp.C
>  create mode 100644 libgomp/testsuite/libgomp.oacc-c++/deep-copy-17.C
>  create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-15.c
>  create mode 100644 libgomp/testsuite/libgomp.oacc-c-c++-common/deep-copy-16.c
> 
> diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
> index 5666cd68c7e..ff614ffe744 100644
> --- a/gcc/fortran/trans-openmp.c
> +++ b/gcc/fortran/trans-openmp.c
> @@ -2721,30 +2721,16 @@ gfc_trans_omp_clauses (stmtblock_t *block, 
> gfc_omp_clauses *clauses,
> tree present = gfc_omp_check_optional_argument (decl, true);
> if (openacc && n->sym->ts.type == BT_CLASS)
>   {
> -   tree type = TREE_TYPE (decl);
> if (n->sym->attr.optional)
>   sorry ("optional class parameter");
> -   if (POINTER_TYPE_P (type))
> - {
> -

Re: [PATCH 5/5] Mapping of components of references to pointers to structs for OpenMP/OpenACC

2021-05-17 Thread Chung-Lin Tang

Hi Julian,

On 2021/5/15 5:27 AM, Julian Brown wrote:

GCC currently raises a parse error for indirect accesses to struct
members, where the base of the access is a reference to a pointer.
This patch fixes that case.



gcc/cp/
* semantics.c (finish_omp_clauses): Handle components of references to
pointers to structs.

libgomp/
* testsuite/libgomp.oacc-c++/deep-copy-17.C: Update test.



--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -7670,7 +7670,12 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type 
ort)
  if ((ort == C_ORT_ACC || ort == C_ORT_OMP)
  && TREE_CODE (t) == COMPONENT_REF
  && TREE_CODE (TREE_OPERAND (t, 0)) == INDIRECT_REF)
-   t = TREE_OPERAND (TREE_OPERAND (t, 0), 0);
+   {
+ t = TREE_OPERAND (TREE_OPERAND (t, 0), 0);
+ /* References to pointers have a double indirection here.  */
+ if (TREE_CODE (t) == INDIRECT_REF)
+   t = TREE_OPERAND (t, 0);
+   }
  if (TREE_CODE (t) == COMPONENT_REF
  && ((ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP
  || ort == C_ORT_ACC)


There is already a large plethora of such modifications in this patch:
"[PATCH, OG10, OpenMP 5.0, committed] Remove array section base-pointer mapping 
semantics, and other front-end adjustments."
https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570075.html

I am in the process of taking that patch to mainline, so are you sure this is 
not already handled there?


diff --git a/libgomp/testsuite/libgomp.oacc-c++/deep-copy-17.C 
b/libgomp/testsuite/libgomp.oacc-c++/deep-copy-17.C
index dacbb520f3d..e038e9e3802 100644
--- a/libgomp/testsuite/libgomp.oacc-c++/deep-copy-17.C
+++ b/libgomp/testsuite/libgomp.oacc-c++/deep-copy-17.C
@@ -83,7 +83,7 @@ void strrp (void)
a[0] = 8;
c[0] = 10;
e[0] = 12;
-  #pragma acc parallel copy(n->a[0:10], n->c[0:10], n->e[0:10])
+  #pragma acc parallel copy(n->a[0:10], n->b, n->c[0:10], n->d, n->e[0:10])
{
  n->a[0] = n->c[0] + n->e[0];
}


This testcase can be added.

Chung-Lin






Re: [PATCH 7/7] [og10] WIP GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION changes

2021-05-17 Thread Chung-Lin Tang

On 2021/5/11 4:57 PM, Julian Brown wrote:

This work-in-progress patch tries to get
GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION to behave more like
GOMP_MAP_ATTACH_DETACH -- in that the mapping is made to form groups
to be processed by build_struct_group/build_struct_comp_map.  I think
that's important to integrate with how groups of mappings for array
sections are handled in other cases.

This patch isn't sufficient by itself to fix a couple of broken test cases
at present (libgomp.c++/target-lambda-1.C, libgomp.c++/target-this-4.C),
though.


No, GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION is supposed to be just a slightly
different behavior version of GOMP_MAP_ATTACH; it tolerates an unmapped
pointer-target and assigns NULL on the device, instead of just gomp_fatal().
(see its handling in libgomp/target.c)

In case OpenACC can have the same such zero-length array section behavior,
we can just share one GOMP_MAP_ATTACH map. For now it is treated as separate
cases.

Chung-Lin


2021-05-11  Julian Brown  

gcc/
* gimplify.c (build_struct_comp_nodes): Add
GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION handling.
(build_struct_group): Process GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION
as part of pointer group.
(gimplify_scan_omp_clauses): Update prev_list_p such that
GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION will form part of pointer
group.
---
  gcc/gimplify.c | 16 
  1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 6d204908c82..c5cb486aa23 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -8298,7 +8298,9 @@ build_struct_comp_nodes (enum tree_code code, tree 
grp_start, tree grp_end,
if (grp_mid
&& OMP_CLAUSE_CODE (grp_mid) == OMP_CLAUSE_MAP
&& (OMP_CLAUSE_MAP_KIND (grp_mid) == GOMP_MAP_ALWAYS_POINTER
- || OMP_CLAUSE_MAP_KIND (grp_mid) == GOMP_MAP_ATTACH_DETACH))
+ || OMP_CLAUSE_MAP_KIND (grp_mid) == GOMP_MAP_ATTACH_DETACH
+ || (OMP_CLAUSE_MAP_KIND (grp_mid)
+ == GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION)))
  {
tree c3
= build_omp_clause (OMP_CLAUSE_LOCATION (grp_end), OMP_CLAUSE_MAP);
@@ -8774,12 +8776,14 @@ build_struct_group (struct gimplify_omp_ctx *ctx,
 ? splay_tree_lookup (ctx->variables, (splay_tree_key) decl)
 : NULL);
bool ptr = (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_ALWAYS_POINTER);
-  bool attach_detach = (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_ATTACH_DETACH);
+  bool attach_detach = (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_ATTACH_DETACH
+   || (OMP_CLAUSE_MAP_KIND (c)
+   == GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION));
bool attach = (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_ATTACH
 || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_DETACH);
bool has_attachments = false;
/* For OpenACC, pointers in structs should trigger an attach action.  */
-  if (attach_detach
+  if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_ATTACH_DETACH
&& ((region_type & (ORT_ACC | ORT_TARGET | ORT_TARGET_DATA))
  || code == OMP_TARGET_ENTER_DATA
  || code == OMP_TARGET_EXIT_DATA))
@@ -9784,6 +9788,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
*pre_p,
  if (!remove
  && OMP_CLAUSE_MAP_KIND (c) != GOMP_MAP_ALWAYS_POINTER
  && OMP_CLAUSE_MAP_KIND (c) != GOMP_MAP_ATTACH_DETACH
+ && (OMP_CLAUSE_MAP_KIND (c)
+ != GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION)
  && OMP_CLAUSE_MAP_KIND (c) != GOMP_MAP_TO_PSET
  && OMP_CLAUSE_CHAIN (c)
  && OMP_CLAUSE_CODE (OMP_CLAUSE_CHAIN (c)) == OMP_CLAUSE_MAP
@@ -9792,7 +9798,9 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq 
*pre_p,
  || (OMP_CLAUSE_MAP_KIND (OMP_CLAUSE_CHAIN (c))
  == GOMP_MAP_ATTACH_DETACH)
  || (OMP_CLAUSE_MAP_KIND (OMP_CLAUSE_CHAIN (c))
- == GOMP_MAP_TO_PSET)))
+ == GOMP_MAP_TO_PSET)
+ || (OMP_CLAUSE_MAP_KIND (OMP_CLAUSE_CHAIN (c))
+ == GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION)))
prev_list_p = list_p;
  
  	  break;




Re: [PATCH 4/5] Rework indirect struct handling for OpenACC/OpenMP in gimplify.c

2021-05-17 Thread Julian Brown
On Mon, 17 May 2021 14:12:00 +0200
Bernd Edlinger  wrote:

> >  */ @@ -8715,19 +8770,26 @@ static tree
> >  build_struct_group (struct gimplify_omp_ctx *ctx,
> > enum omp_region_type region_type, enum
> > tree_code code, tree decl, unsigned int *flags, tree c,
> > -   hash_map *&struct_map_to_clause,
> > +   hash_map
> > *&struct_map_to_clause, tree *&prev_list_p, tree *&list_p, bool
> > *cont) {
> >poly_offset_int coffset;
> >poly_int64 cbitpos;
> > -  tree base_ref;
> > +  tree base_ind, base_ref;
> > +  tree *list_in_p = list_p, *prev_list_in_p = prev_list_p;
> >
> 
> Is this a kind of debug code?
> This fails to compile:
> 
> ../../gcc-trunk/gcc/gimplify.c: In function ‘tree_node*
> build_struct_group(gimplify_omp_ctx*, omp_region_type, tree_code,
> tree, unsigned int*, tree, hash_map*&,
> tree_node**&, tree_node**&, bool*)’:
> ../../gcc-trunk/gcc/gimplify.c:8779:9: error: unused variable
> ‘list_in_p’ [-Werror=unused-variable] 8779 |   tree *list_in_p =
> list_p, *prev_list_in_p = prev_list_p; | ^
> ../../gcc-trunk/gcc/gimplify.c:8779:30: error: unused variable
> ‘prev_list_in_p’ [-Werror=unused-variable] 8779 |   tree *list_in_p =
> list_p, *prev_list_in_p = prev_list_p; |
> ^~

Oops, that's left over from an earlier iteration of the patch, and
indeed isn't needed any more. I'll be sure to bootstrap the next
iteration of these patches I send upstream.

Thanks,

Julian


Re: [PATCH 5/5] Mapping of components of references to pointers to structs for OpenMP/OpenACC

2021-05-17 Thread Julian Brown
On Mon, 17 May 2021 21:07:19 +0800
Chung-Lin Tang  wrote:

> Hi Julian,
> 
> On 2021/5/15 5:27 AM, Julian Brown wrote:
> > GCC currently raises a parse error for indirect accesses to struct
> > members, where the base of the access is a reference to a pointer.
> > This patch fixes that case.  
> 
> > gcc/cp/
> > * semantics.c (finish_omp_clauses): Handle components of
> > references to pointers to structs.
> > 
> > libgomp/
> > * testsuite/libgomp.oacc-c++/deep-copy-17.C: Update test.  
> 
> > --- a/gcc/cp/semantics.c
> > +++ b/gcc/cp/semantics.c
> > @@ -7670,7 +7670,12 @@ finish_omp_clauses (tree clauses, enum
> > c_omp_region_type ort) if ((ort == C_ORT_ACC || ort == C_ORT_OMP)
> >   && TREE_CODE (t) == COMPONENT_REF
> >   && TREE_CODE (TREE_OPERAND (t, 0)) == INDIRECT_REF)
> > -   t = TREE_OPERAND (TREE_OPERAND (t, 0), 0);
> > +   {
> > + t = TREE_OPERAND (TREE_OPERAND (t, 0), 0);
> > + /* References to pointers have a double indirection
> > here.  */
> > + if (TREE_CODE (t) == INDIRECT_REF)
> > +   t = TREE_OPERAND (t, 0);
> > +   }
> >   if (TREE_CODE (t) == COMPONENT_REF
> >   && ((ort & C_ORT_OMP_DECLARE_SIMD) == C_ORT_OMP
> >   || ort == C_ORT_ACC)  
> 
> There is already a large plethora of such modifications in this patch:
> "[PATCH, OG10, OpenMP 5.0, committed] Remove array section
> base-pointer mapping semantics, and other front-end adjustments."
> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570075.html
> 
> I am in the process of taking that patch to mainline, so are you sure
> this is not already handled there?

Hmm, it might be -- thanks. Consider this patch withdrawn if so. (But
yeah, keep the test case by all means!)

Julian


Re: [PATCH 7/7] [og10] WIP GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION changes

2021-05-17 Thread Julian Brown
On Mon, 17 May 2021 21:14:26 +0800
Chung-Lin Tang  wrote:

> On 2021/5/11 4:57 PM, Julian Brown wrote:
> > This work-in-progress patch tries to get
> > GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION to behave more like
> > GOMP_MAP_ATTACH_DETACH -- in that the mapping is made to form groups
> > to be processed by build_struct_group/build_struct_comp_map.  I
> > think that's important to integrate with how groups of mappings for
> > array sections are handled in other cases.
> > 
> > This patch isn't sufficient by itself to fix a couple of broken
> > test cases at present (libgomp.c++/target-lambda-1.C,
> > libgomp.c++/target-this-4.C), though.  
> 
> No, GOMP_MAP_ATTACH_ZERO_LENGTH_ARRAY_SECTION is supposed to be just
> a slightly different behavior version of GOMP_MAP_ATTACH; it
> tolerates an unmapped pointer-target and assigns NULL on the device,
> instead of just gomp_fatal(). (see its handling in libgomp/target.c)
> 
> In case OpenACC can have the same such zero-length array section
> behavior, we can just share one GOMP_MAP_ATTACH map. For now it is
> treated as separate cases.

OK, understood. But, I'm a bit concerned that we're ignoring some
"hidden rules" with regards to OMP pointer clause ordering/grouping that
certain code (at least the bit that creates GOMP_MAP_STRUCT node
groups, and parts of omp-low.c) relies on. I believe those rules are as
follows:

 - an array slice is mapped using two or three pointers -- two for a
   normal (non-reference) base pointer, and three if we have a
   reference to a pointer (i.e. in C++) or an array descriptor (i.e. in
   Fortran). So we can have e.g.

   GOMP_MAP_TO
   GOMP_MAP_ALWAYS_POINTER

   GOMP_MAP_TO
   GOMP_MAP_.*_POINTER
   GOMP_MAP_ALWAYS_POINTER

   GOMP_MAP_TO
   GOMP_MAP_TO_PSET
   GOMP_MAP_ALWAYS_POINTER

 - for OpenACC, we extend this to allow (up to and including
   gimplify.c) the GOMP_MAP_ATTACH_DETACH mapping. So we can have (for
   component refs):

   GOMP_MAP_TO
   GOMP_MAP_ATTACH_DETACH

   GOMP_MAP_TO
   GOMP_MAP_TO_PSET
   GOMP_MAP_ATTACH_DETACH

   GOMP_MAP_TO
   GOMP_MAP_.*_POINTER
   GOMP_MAP_ATTACH_DETACH

For the scanning in insert_struct_comp_map (as it is at present) to
work right, these groups must stay intact.  I think the current
behaviour of omp_target_reorder_clauses on the og10 branch can break
those groups apart though!

(The "prev_list_p" stuff in the loop in question in gimplify.c just
keeps track of the first node in these groups.)

For OpenACC, the GOMP_MAP_ATTACH_DETACH code does *not* depend on the
previous clause when lowering in omp-low.c. But GOMP_MAP_ALWAYS_POINTER
does! And in one case ("update" directive), GOMP_MAP_ATTACH_DETACH is
rewritten to GOMP_MAP_ALWAYS_POINTER, so for that case at least, the
dependency on the preceding mapping node must stay intact.

OpenACC also allows "bare" GOMP_MAP_ATTACH and GOMP_MAP_DETACH nodes
(corresponding to the "attach" and "detach" clauses). Those are handled
a bit differently to GOMP_MAP_ATTACH_DETACH in gimplify.c -- but
GOMP_MAP_ATTACH_Z_L_A_S doesn't quite behave like that either, I don't
think?

Anyway: I've not entirely understood what omp_target_reorder_clauses is
doing, but I think it may need to try harder to keep the groups
mentioned above together.  What do you think?

Thanks,

Julian


Re: [EXTERNAL] Aw: Help with long compile time of all-USE module

2021-05-17 Thread Thompson, Matt (GSFC-610.1)[SCIENCE SYSTEMS AND APPLICATIONS INC] via Fortran
Martin,

Thanks. I'll try to give it a try soon...

...though that might be a while. Our preliminary tests with GCC 11 seem to show 
some issues with our code (other than slowness). Our devs (including Tom Clune 
who might be known to some here) are looking to see why GCC 10.3 is happy, but 
GCC 11 isn't. The changelog didn't seem to exciting but you never know when 
things like polymorphism, strings, et al are all happening at once!

Matt

--
Matt Thompson, SSAI, Ld Scientific Programmer/Analyst
NASA GSFC,Global Modeling and Assimilation Office
Code 610.1,  8800 Greenbelt Rd,  Greenbelt,  MD 20771
Phone: 301-614-6712 Fax: 301-614-6246
http://science.gsfc.nasa.gov/sed/bio/matthew.thompson

From: Martin Stein 
Date: Monday, May 17, 2021 at 6:15 AM
To: "Thompson, Matt (GSFC-610.1)[SCIENCE SYSTEMS AND APPLICATIONS INC]" 

Cc: "fortran@gcc.gnu.org" 
Subject: [EXTERNAL] Aw: Help with long compile time of all-USE module

PS: I am currently using (with 11.1) the following simple patch to 
significantly speeding up my compilation. The patch is not correct, but I could 
not find any examples, where this makes a difference. Even by looking at the 
orignal commit introducing the find_symbol call I could not come up with an 
example where this matters. I would be interested to know whether this is the 
cause for the long compilation times in your case as well.

gcc/fortran/module.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gcc/fortran/module.c b/gcc/fortran/module.c
index 089453caa03..a29865c7412 100644
--- a/gcc/fortran/module.c
+++ b/gcc/fortran/module.c
@@ -5501,11 +5501,12 @@ read_module (void)
this symbol, which is not in an ONLY clause, must not be
added to the namespace(11.3.2).  Note that find_symbol
only returns the first occurrence that it finds.  */
- if (!only_flag && !info->u.rsym.renamed
+/*   if (!only_flag && !info->u.rsym.renamed
   && strcmp (name, module_name) != 0
   && find_symbol (gfc_current_ns->sym_root, name,
   module_name, 0))
   continue;
+*/

 st = gfc_find_symtree (gfc_current_ns->sym_root, p);


Best regards
Martin