public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, OpenMP, C++] Allow classes with static members to be mappable
@ 2022-03-09 11:04 Chung-Lin Tang
  2022-05-05  9:12 ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Chung-Lin Tang @ 2022-03-09 11:04 UTC (permalink / raw)
  To: gcc-patches, Jakub Jelinek, Catherine Moore

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]

Hi Jakub,
Now in OpenMP 5.x, static members are supposed to be not a barrier for a class
to be target-mapped.

There is the related issue of actually providing access to static const/constexpr
members on the GPU (probably a case of https://github.com/OpenMP/spec/issues/2158)
but that is for later.

This patch basically just removes the check for static members inside
cp_omp_mappable_type_1, and adjusts a testcase. Not sure if more tests are needed.
Tested on trunk without regressions, okay when stage1 reopens?

Thanks,
Chung-Lin

2022-03-09  Chung-Lin Tang  <cltang@codesourcery.com>

gcc/cp/ChangeLog:

	* decl2.cc (cp_omp_mappable_type_1): Remove requirement that all
	members must be non-static; remove check for static fields.

gcc/testsuite/ChangeLog:

	* g++.dg/gomp/unmappable-1.C: Adjust testcase.

[-- Attachment #2: g.diff --]
[-- Type: text/plain, Size: 1409 bytes --]

diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index c53acf4546d..ace7783d9bd 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -1544,21 +1544,14 @@ cp_omp_mappable_type_1 (tree type, bool notes)
   /* Arrays have mappable type if the elements have mappable type.  */
   while (TREE_CODE (type) == ARRAY_TYPE)
     type = TREE_TYPE (type);
-  /* All data members must be non-static.  */
+
   if (CLASS_TYPE_P (type))
     {
       tree field;
       for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
-	if (VAR_P (field))
-	  {
-	    if (notes)
-	      inform (DECL_SOURCE_LOCATION (field),
-		      "static field %qD is not mappable", field);
-	    result = false;
-	  }
 	/* All fields must have mappable types.  */
-	else if (TREE_CODE (field) == FIELD_DECL
-		 && !cp_omp_mappable_type_1 (TREE_TYPE (field), notes))
+	if (TREE_CODE (field) == FIELD_DECL
+	    && !cp_omp_mappable_type_1 (TREE_TYPE (field), notes))
 	  result = false;
     }
   return result;
diff --git a/gcc/testsuite/g++.dg/gomp/unmappable-1.C b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
index 364f884500c..1532b9c73f1 100644
--- a/gcc/testsuite/g++.dg/gomp/unmappable-1.C
+++ b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
@@ -4,7 +4,7 @@
 class C
 {
 public:
-  static int static_member; /* { dg-message "static field .C::static_member. is not mappable" } */
+  static int static_member;
   virtual void f() {}
 };
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, OpenMP, C++] Allow classes with static members to be mappable
  2022-03-09 11:04 [PATCH, OpenMP, C++] Allow classes with static members to be mappable Chung-Lin Tang
@ 2022-05-05  9:12 ` Jakub Jelinek
  2022-07-27 11:45   ` Tobias Burnus
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-05-05  9:12 UTC (permalink / raw)
  To: Chung-Lin Tang; +Cc: gcc-patches, Catherine Moore

On Wed, Mar 09, 2022 at 07:04:24PM +0800, Chung-Lin Tang wrote:
> Now in OpenMP 5.x, static members are supposed to be not a barrier for a class
> to be target-mapped.
> 
> There is the related issue of actually providing access to static const/constexpr
> members on the GPU (probably a case of https://github.com/OpenMP/spec/issues/2158)
> but that is for later.
> 
> This patch basically just removes the check for static members inside
> cp_omp_mappable_type_1, and adjusts a testcase. Not sure if more tests are needed.
> Tested on trunk without regressions, okay when stage1 reopens?
> 
> Thanks,
> Chung-Lin
> 
> 2022-03-09  Chung-Lin Tang  <cltang@codesourcery.com>
> 
> gcc/cp/ChangeLog:
> 
> 	* decl2.cc (cp_omp_mappable_type_1): Remove requirement that all
> 	members must be non-static; remove check for static fields.

I don't see anything useful left in cp_omp_mappable_type{,_1}.
In particular, starting with OpenMP 5.0, for both C and C++ we just say
that a mappable type is a complete type.  True, for C++ there is also the
"All member functions accessed in any target region must appear in a
declare target directive."
and similarly for Fortran, but that isn't something we really can check when
we ask whether something is a mappable type, that isn't a property of a
type, but a property of the target region.
In OpenMP 4.5 the special C++ mappable_type langhooks was useful, both for
the non-static data members and for virtual methods.

So, I think instead of your patch, we should just throw away
cp_omp_mappable_type{,_1}, and as C++ was the only one that had a special
langhook, I think we should kill the langhook altogether too, move
lhd_omp_mappable_type from langhooks.cc to omp-general.cc or so
as omp_mappable_type and use that instead of the langhooks or
cp_omp_mappable_type.
Now, the C++ FE has also cp_omp_emit_unmappable_type_notes while other FEs
don't, either we can just say that type doesn't have mappable type
like the C FE does, or perhaps just can emit a note that it isn't a mappable
type because it is incomplete (but then it would be nice to do the same
thing in the C FE too).

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, OpenMP, C++] Allow classes with static members to be mappable
  2022-05-05  9:12 ` Jakub Jelinek
@ 2022-07-27 11:45   ` Tobias Burnus
  2022-08-17 12:13     ` Jakub Jelinek
  0 siblings, 1 reply; 4+ messages in thread
From: Tobias Burnus @ 2022-07-27 11:45 UTC (permalink / raw)
  To: Jakub Jelinek, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2881 bytes --]

Hi all,

On 05.05.22 11:12, Jakub Jelinek via Gcc-patches wrote:
→ https://gcc.gnu.org/pipermail/gcc-patches/2022-May/594082.html
> On Wed, Mar 09, 2022 at 07:04:24PM +0800, Chung-Lin Tang wrote:
https://gcc.gnu.org/pipermail/gcc-patches/2022-March/591449.html
>> Now in OpenMP 5.x, static members are supposed to be not a barrier for a class
>> to be target-mapped.
>>
>> There is the related issue of actually providing access to static const/constexpr
>> members on the GPU (probably a case of https://github.com/OpenMP/spec/issues/2158)
>> but that is for later.
>>
>> This patch basically just removes the check for static members inside
>> cp_omp_mappable_type_1, and adjusts a testcase. Not sure if more tests are needed.
>> Tested on trunk without regressions, okay when stage1 reopens?
> I don't see anything useful left in cp_omp_mappable_type{,_1}.
> In particular, starting with OpenMP 5.0, for both C and C++ we just say
> that a mappable type is a complete type.  True, for C++ there is also the
> "All member functions accessed in any target region must appear in a
> declare target directive."
> and similarly for Fortran, but that isn't something we really can check [...]
I have now removed the hook as suggested.
> Now, the C++ FE has also cp_omp_emit_unmappable_type_notes while other FEs
> don't, either we can just say that type doesn't have mappable type
> like the C FE does, or perhaps just can emit a note that it isn't a mappable
> type because it is incomplete (but then it would be nice to do the same
> thing in the C FE too).

I looked into this – but I think only C++ has TYPE_MAIN_DECL; without being
able to get the DECL_SOURCE_LOCATION, adding an "inform" does not really help.

Instead of the omp-specific emit function, I now use cxx_incomplete_type_inform,
which avoids printing a pointless "note:"/inform if no source location is
available and uses common wording. (Visible with the added example, which
printed before for some errors the omp-specific inform and for others the
cp-generic inform.)

(Actually, the testcase shows some overdiagnostic - for the single
incomplete-type issue, there are 3 error message (two identical) and with
C++ trice the same inform/"note:". This is unchanged with this patch.)

The code could be simplified by removing the COMPLETE_TYPE_P condition before
cxx_incomplete_type_inform in cp/{decl,semantics}.cc, given that in
omp_mappable_type(), the only check besides error_node is !COMPLETE_TYPE_P.

The patch passes bootstrapping and regtesting on x86-64-gnu-linux.
OK?

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

[-- Attachment #2: omp-static-class.diff --]
[-- Type: text/x-patch, Size: 18896 bytes --]

OpenMP/C++: Allow classes with static members to be mappable

As this is the last lang-specific user of the omp_mappable_type hook,
the hook is removed, keeping only a generic omp_mappable_type for
incomplete types (or error_node).

gcc/c/ChangeLog:

	* c-decl.cc (c_decl_attributes, finish_decl): Call omp_mappable_type
	instead of removed langhook.
	* c-typeck.cc (c_finish_omp_clauses): Likewise.

gcc/cp/ChangeLog:

	* cp-objcp-common.h (LANG_HOOKS_OMP_MAPPABLE_TYPE): Remove.
	* cp-tree.h (cp_omp_mappable_type, cp_omp_emit_unmappable_type_notes):
	Remove.
	* decl2.cc (cp_omp_mappable_type_1, cp_omp_mappable_type,
	cp_omp_emit_unmappable_type_notes): Remove.
	(cplus_decl_attributes): Call omp_mappable_type instead of
	removed langhook.
	* decl.cc (cp_finish_decl): Likewise; call cxx_incomplete_type_inform
	in lieu of cp_omp_emit_unmappable_type_notes.
	* semantics.cc (finish_omp_clauses): Likewise.

gcc/ChangeLog:

	* gimplify.cc (omp_notice_variable): Call omp_mappable_type
        instead of removed langhook.
	* omp-general.h (omp_mappable_type): New prototype.
	* omp-general.cc (omp_mappable_type):  New; moved from ...
	* langhooks.cc (lhd_omp_mappable_type): ... here.
	* langhooks-def.h (lhd_omp_mappable_type,
	LANG_HOOKS_OMP_MAPPABLE_TYPE): Remove.
	(LANG_HOOKS_FOR_TYPES_INITIALIZER): Remote the latter.
	* langhooks.h (struct lang_hooks_for_types): Remove
	omp_mappable_type.

gcc/testsuite/ChangeLog:

	* g++.dg/gomp/unmappable-1.C: Remove dg-error; remove dg-note no
	longer shown as TYPE_MAIN_DECL is NULL.
	* c-c++-common/gomp/map-incomplete-type.c: New test.

Co-authored-by: Chung-Lin Tang <cltang@codesourcery.com>

 gcc/c/c-decl.cc                                    |  5 +-
 gcc/c/c-typeck.cc                                  |  8 +--
 gcc/cp/cp-objcp-common.h                           |  2 -
 gcc/cp/cp-tree.h                                   |  2 -
 gcc/cp/decl.cc                                     |  6 ++-
 gcc/cp/decl2.cc                                    | 59 +---------------------
 gcc/cp/semantics.cc                                | 28 ++++++----
 gcc/gimplify.cc                                    |  2 +-
 gcc/langhooks-def.h                                |  3 --
 gcc/langhooks.cc                                   | 11 ----
 gcc/langhooks.h                                    |  3 --
 gcc/omp-general.cc                                 | 11 ++++
 gcc/omp-general.h                                  |  1 +
 .../c-c++-common/gomp/map-incomplete-type.c        | 17 +++++++
 gcc/testsuite/g++.dg/gomp/unmappable-1.C           |  3 +-
 15 files changed, 61 insertions(+), 100 deletions(-)

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index ae8990c138f..9e590c66dae 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -5074,8 +5074,7 @@ c_decl_attributes (tree *node, tree attributes, int flags)
       && ((VAR_P (*node) && is_global_var (*node))
 	  || TREE_CODE (*node) == FUNCTION_DECL))
     {
-      if (VAR_P (*node)
-	  && !lang_hooks.types.omp_mappable_type (TREE_TYPE (*node)))
+      if (VAR_P (*node) && !omp_mappable_type (TREE_TYPE (*node)))
 	attributes = tree_cons (get_identifier ("omp declare target implicit"),
 				NULL_TREE, attributes);
       else
@@ -5701,7 +5700,7 @@ finish_decl (tree decl, location_t init_loc, tree init,
       DECL_ATTRIBUTES (decl)
 	= remove_attribute ("omp declare target implicit",
 			    DECL_ATTRIBUTES (decl));
-      if (!lang_hooks.types.omp_mappable_type (TREE_TYPE (decl)))
+      if (!omp_mappable_type (TREE_TYPE (decl)))
 	error ("%q+D in declare target directive does not have mappable type",
 	       decl);
       else if (!lookup_attribute ("omp declare target",
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index fd0a7f81a7a..d084746b7c3 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -14944,7 +14944,7 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 	      else
 		{
 		  t = OMP_CLAUSE_DECL (c);
-		  if (!lang_hooks.types.omp_mappable_type (TREE_TYPE (t)))
+		  if (!omp_mappable_type (TREE_TYPE (t)))
 		    {
 		      error_at (OMP_CLAUSE_LOCATION (c),
 				"array section does not have mappable type "
@@ -15081,7 +15081,7 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 			    t, omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
 		  remove = true;
 		}
-	      else if (!lang_hooks.types.omp_mappable_type (TREE_TYPE (t)))
+	      else if (!omp_mappable_type (TREE_TYPE (t)))
 		{
 		  error_at (OMP_CLAUSE_LOCATION (c),
 			    "%qE does not have a mappable type in %qs clause",
@@ -15162,7 +15162,7 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 			 || (OMP_CLAUSE_MAP_KIND (c)
 			     == GOMP_MAP_FORCE_DEVICEPTR)))
 		   && t == OMP_CLAUSE_DECL (c)
-		   && !lang_hooks.types.omp_mappable_type (TREE_TYPE (t)))
+		   && !omp_mappable_type (TREE_TYPE (t)))
 	    {
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
@@ -15279,7 +15279,7 @@ c_finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 			cname);
 	      remove = true;
 	    }
-	  else if (!lang_hooks.types.omp_mappable_type (TREE_TYPE (t)))
+	  else if (!omp_mappable_type (TREE_TYPE (t)))
 	    {
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
diff --git a/gcc/cp/cp-objcp-common.h b/gcc/cp/cp-objcp-common.h
index 3c04e5c0265..1a67f14d9b3 100644
--- a/gcc/cp/cp-objcp-common.h
+++ b/gcc/cp/cp-objcp-common.h
@@ -187,8 +187,6 @@ extern tree cxx_simulate_record_decl (location_t, const char *,
 #define LANG_HOOKS_OMP_FINISH_CLAUSE cxx_omp_finish_clause
 #undef LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE
 #define LANG_HOOKS_OMP_PRIVATIZE_BY_REFERENCE cxx_omp_privatize_by_reference
-#undef LANG_HOOKS_OMP_MAPPABLE_TYPE
-#define LANG_HOOKS_OMP_MAPPABLE_TYPE cp_omp_mappable_type
 #undef LANG_HOOKS_OMP_DISREGARD_VALUE_EXPR
 #define LANG_HOOKS_OMP_DISREGARD_VALUE_EXPR cxx_omp_disregard_value_expr
 
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3278b4114bd..cb23975484f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6959,8 +6959,6 @@ extern bool possibly_inlined_p			(tree);
 extern int parm_index                           (tree);
 extern tree vtv_start_verification_constructor_init_function (void);
 extern tree vtv_finish_verification_constructor_init_function (tree);
-extern bool cp_omp_mappable_type		(tree);
-extern bool cp_omp_emit_unmappable_type_notes	(tree);
 extern void cp_check_const_attributes (tree);
 
 /* in error.cc */
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 70ad681467e..7f8bfa31674 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8625,11 +8625,13 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
 	= remove_attribute ("omp declare target implicit",
 			    DECL_ATTRIBUTES (decl));
       complete_type (TREE_TYPE (decl));
-      if (!cp_omp_mappable_type (TREE_TYPE (decl)))
+      if (!omp_mappable_type (TREE_TYPE (decl)))
 	{
 	  error ("%q+D in declare target directive does not have mappable"
 		 " type", decl);
-	  cp_omp_emit_unmappable_type_notes (TREE_TYPE (decl));
+	  if (TREE_TYPE (decl) != error_mark_node
+	      && !COMPLETE_TYPE_P (TREE_TYPE (decl)))
+	    cxx_incomplete_type_inform (TREE_TYPE (decl));
 	}
       else if (!lookup_attribute ("omp declare target",
 				  DECL_ATTRIBUTES (decl))
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 3737e5f010c..89ab2545d64 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -49,6 +49,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "c-family/c-ada-spec.h"
 #include "asan.h"
 #include "optabs-query.h"
+#include "omp-general.h"
 
 /* Id for dumping the raw trees.  */
 int raw_dump_id;
@@ -1578,62 +1579,6 @@ cp_check_const_attributes (tree attributes)
     }
 }
 
-/* Return true if TYPE is an OpenMP mappable type.
-   If NOTES is non-zero, emit a note message for each problem.  */
-static bool
-cp_omp_mappable_type_1 (tree type, bool notes)
-{
-  bool result = true;
-
-  /* Mappable type has to be complete.  */
-  if (type == error_mark_node || !COMPLETE_TYPE_P (type))
-    {
-      if (notes && type != error_mark_node)
-	{
-	  tree decl = TYPE_MAIN_DECL (type);
-	  inform ((decl ? DECL_SOURCE_LOCATION (decl) : input_location),
-		  "incomplete type %qT is not mappable", type);
-	}
-      result = false;
-    }
-  /* Arrays have mappable type if the elements have mappable type.  */
-  while (TREE_CODE (type) == ARRAY_TYPE)
-    type = TREE_TYPE (type);
-  /* All data members must be non-static.  */
-  if (CLASS_TYPE_P (type))
-    {
-      tree field;
-      for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
-	if (VAR_P (field))
-	  {
-	    if (notes)
-	      inform (DECL_SOURCE_LOCATION (field),
-		      "static field %qD is not mappable", field);
-	    result = false;
-	  }
-	/* All fields must have mappable types.  */
-	else if (TREE_CODE (field) == FIELD_DECL
-		 && !cp_omp_mappable_type_1 (TREE_TYPE (field), notes))
-	  result = false;
-    }
-  return result;
-}
-
-/* Return true if TYPE is an OpenMP mappable type.  */
-bool
-cp_omp_mappable_type (tree type)
-{
-  return cp_omp_mappable_type_1 (type, false);
-}
-
-/* Return true if TYPE is an OpenMP mappable type.
-   Emit an error messages if not.  */
-bool
-cp_omp_emit_unmappable_type_notes (tree type)
-{
-  return cp_omp_mappable_type_1 (type, true);
-}
-
 /* Return the last pushed declaration for the symbol DECL or NULL
    when no such declaration exists.  */
 
@@ -1709,7 +1654,7 @@ cplus_decl_attributes (tree *decl, tree attributes, int flags)
 	       *decl);
       else if (VAR_P (*decl)
 	       && (processing_template_decl
-		   || !cp_omp_mappable_type (TREE_TYPE (*decl))))
+		   || !omp_mappable_type (TREE_TYPE (*decl))))
 	attributes = tree_cons (get_identifier ("omp declare target implicit"),
 				NULL_TREE, attributes);
       else
diff --git a/gcc/cp/semantics.cc b/gcc/cp/semantics.cc
index 16dea0593b0..ae7c8ea7b1f 100644
--- a/gcc/cp/semantics.cc
+++ b/gcc/cp/semantics.cc
@@ -7992,13 +7992,15 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 		  t = OMP_CLAUSE_DECL (c);
 		  if (TREE_CODE (t) != TREE_LIST
 		      && !type_dependent_expression_p (t)
-		      && !cp_omp_mappable_type (TREE_TYPE (t)))
+		      && !omp_mappable_type (TREE_TYPE (t)))
 		    {
 		      error_at (OMP_CLAUSE_LOCATION (c),
 				"array section does not have mappable type "
 				"in %qs clause",
 				omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
-		      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
+		      if (TREE_TYPE (t) != error_mark_node
+			  && !COMPLETE_TYPE_P (TREE_TYPE (t)))
+			cxx_incomplete_type_inform (TREE_TYPE (t));
 		      remove = true;
 		    }
 		  while (TREE_CODE (t) == ARRAY_REF)
@@ -8134,12 +8136,14 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 			    t, omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
 		  remove = true;
 		}
-	      else if (!cp_omp_mappable_type (TREE_TYPE (t)))
+	      else if (!omp_mappable_type (TREE_TYPE (t)))
 		{
 		  error_at (OMP_CLAUSE_LOCATION (c),
 			    "%qE does not have a mappable type in %qs clause",
 			    t, omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
-		  cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
+		  if (TREE_TYPE (t) != error_mark_node
+		      && !COMPLETE_TYPE_P (TREE_TYPE (t)))
+		    cxx_incomplete_type_inform (TREE_TYPE (t));
 		  remove = true;
 		}
 	      while (TREE_CODE (t) == COMPONENT_REF)
@@ -8232,14 +8236,16 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 			     == GOMP_MAP_FIRSTPRIVATE_POINTER)))
 		   && t == OMP_CLAUSE_DECL (c)
 		   && !type_dependent_expression_p (t)
-		   && !cp_omp_mappable_type (TYPE_REF_P (TREE_TYPE (t))
-					     ? TREE_TYPE (TREE_TYPE (t))
-					     : TREE_TYPE (t)))
+		   && !omp_mappable_type (TYPE_REF_P (TREE_TYPE (t))
+					  ? TREE_TYPE (TREE_TYPE (t))
+					  : TREE_TYPE (t)))
 	    {
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
 			omp_clause_code_name[OMP_CLAUSE_CODE (c)]);
-	      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
+	      if (TREE_TYPE (t) != error_mark_node
+		  && !COMPLETE_TYPE_P (TREE_TYPE (t)))
+		cxx_incomplete_type_inform (TREE_TYPE (t));
 	      remove = true;
 	    }
 	  else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
@@ -8409,12 +8415,14 @@ finish_omp_clauses (tree clauses, enum c_omp_region_type ort)
 			cname);
 	      remove = true;
 	    }
-	  else if (!cp_omp_mappable_type (TREE_TYPE (t)))
+	  else if (!omp_mappable_type (TREE_TYPE (t)))
 	    {
 	      error_at (OMP_CLAUSE_LOCATION (c),
 			"%qD does not have a mappable type in %qs clause", t,
 			cname);
-	      cp_omp_emit_unmappable_type_notes (TREE_TYPE (t));
+	      if (TREE_TYPE (t) != error_mark_node
+		  && !COMPLETE_TYPE_P (TREE_TYPE (t)))
+		cxx_incomplete_type_inform (TREE_TYPE (t));
 	      remove = true;
 	    }
 	  if (remove)
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 2ac7ca0855e..5e340a7afb5 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -7882,7 +7882,7 @@ omp_notice_variable (struct gimplify_omp_ctx *ctx, tree decl, bool in_code)
 	      if (gimplify_omp_ctxp->target_firstprivatize_array_bases
 		  && omp_privatize_by_reference (decl))
 		type = TREE_TYPE (type);
-	      if (!lang_hooks.types.omp_mappable_type (type))
+	      if (!omp_mappable_type (type))
 		{
 		  error ("%qD referenced in target region does not have "
 			 "a mappable type", decl);
diff --git a/gcc/langhooks-def.h b/gcc/langhooks-def.h
index 4e17915e8b3..0f1bfbf255c 100644
--- a/gcc/langhooks-def.h
+++ b/gcc/langhooks-def.h
@@ -88,7 +88,6 @@ extern tree lhd_omp_array_size (tree, gimple_seq *);
 struct gimplify_omp_ctx;
 extern void lhd_omp_firstprivatize_type_sizes (struct gimplify_omp_ctx *,
 					       tree);
-extern bool lhd_omp_mappable_type (tree);
 extern bool lhd_omp_scalar_p (tree, bool);
 extern tree *lhd_omp_get_decl_init (tree);
 extern void lhd_omp_finish_decl_inits ();
@@ -206,7 +205,6 @@ extern tree lhd_unit_size_without_reusable_padding (tree);
 #define LANG_HOOKS_TYPE_MAX_SIZE	lhd_return_null_const_tree
 #define LANG_HOOKS_OMP_FIRSTPRIVATIZE_TYPE_SIZES \
   lhd_omp_firstprivatize_type_sizes
-#define LANG_HOOKS_OMP_MAPPABLE_TYPE	lhd_omp_mappable_type
 #define LANG_HOOKS_TYPE_HASH_EQ		NULL
 #define LANG_HOOKS_COPY_LANG_QUALIFIERS NULL
 #define LANG_HOOKS_GET_ARRAY_DESCR_INFO	NULL
@@ -235,7 +233,6 @@ extern tree lhd_unit_size_without_reusable_padding (tree);
   LANG_HOOKS_INCOMPLETE_TYPE_ERROR, \
   LANG_HOOKS_TYPE_MAX_SIZE, \
   LANG_HOOKS_OMP_FIRSTPRIVATIZE_TYPE_SIZES, \
-  LANG_HOOKS_OMP_MAPPABLE_TYPE, \
   LANG_HOOKS_TYPE_HASH_EQ, \
   LANG_HOOKS_COPY_LANG_QUALIFIERS, \
   LANG_HOOKS_GET_ARRAY_DESCR_INFO, \
diff --git a/gcc/langhooks.cc b/gcc/langhooks.cc
index a93340799f3..7d975675990 100644
--- a/gcc/langhooks.cc
+++ b/gcc/langhooks.cc
@@ -686,17 +686,6 @@ lhd_omp_firstprivatize_type_sizes (struct gimplify_omp_ctx *c ATTRIBUTE_UNUSED,
 {
 }
 
-/* Return true if TYPE is an OpenMP mappable type.  */
-
-bool
-lhd_omp_mappable_type (tree type)
-{
-  /* Mappable type has to be complete.  */
-  if (type == error_mark_node || !COMPLETE_TYPE_P (type))
-    return false;
-  return true;
-}
-
 /* Common function for add_builtin_function, add_builtin_function_ext_scope
    and simulate_builtin_function_decl.  */
 
diff --git a/gcc/langhooks.h b/gcc/langhooks.h
index 97aa9e0d070..b1b2b0e10f0 100644
--- a/gcc/langhooks.h
+++ b/gcc/langhooks.h
@@ -128,9 +128,6 @@ struct lang_hooks_for_types
      firstprivate variables.  */
   void (*omp_firstprivatize_type_sizes) (struct gimplify_omp_ctx *, tree);
 
-  /* Return true if TYPE is a mappable type.  */
-  bool (*omp_mappable_type) (tree type);
-
   /* Return TRUE if TYPE1 and TYPE2 are identical for type hashing purposes.
      Called only after doing all language independent checks.
      At present, this function is only called when both TYPE1 and TYPE2 are
diff --git a/gcc/omp-general.cc b/gcc/omp-general.cc
index a406c578f33..33792adc4de 100644
--- a/gcc/omp-general.cc
+++ b/gcc/omp-general.cc
@@ -80,6 +80,17 @@ omp_check_optional_argument (tree decl, bool for_present_check)
   return lang_hooks.decls.omp_check_optional_argument (decl, for_present_check);
 }
 
+/* Return true if TYPE is an OpenMP mappable type.  */
+
+bool
+omp_mappable_type (tree type)
+{
+  /* Mappable type has to be complete.  */
+  if (type == error_mark_node || !COMPLETE_TYPE_P (type))
+    return false;
+  return true;
+}
+
 /* True if OpenMP should privatize what this DECL points to rather
    than the DECL itself.  */
 
diff --git a/gcc/omp-general.h b/gcc/omp-general.h
index 74e90e1a71a..1b5455a0a8f 100644
--- a/gcc/omp-general.h
+++ b/gcc/omp-general.h
@@ -94,6 +94,7 @@ struct omp_for_data
 extern tree omp_find_clause (tree clauses, enum omp_clause_code kind);
 extern bool omp_is_allocatable_or_ptr (tree decl);
 extern tree omp_check_optional_argument (tree decl, bool for_present_check);
+extern bool omp_mappable_type (tree type);
 extern bool omp_privatize_by_reference (tree decl);
 extern void omp_adjust_for_condition (location_t loc, enum tree_code *cond_code,
 				      tree *n2, tree v, tree step);
diff --git a/gcc/testsuite/c-c++-common/gomp/map-incomplete-type.c b/gcc/testsuite/c-c++-common/gomp/map-incomplete-type.c
new file mode 100644
index 00000000000..29702defa6a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/gomp/map-incomplete-type.c
@@ -0,0 +1,17 @@
+struct incomplete_t;
+/* { dg-note "forward declaration of 'struct incomplete_t'" "" { target c++ } .-1 } */
+
+/* Note: This note is only printed with C++ (trice); the loc is available due to TYPE_MAIN_DECL.  */
+
+struct incomplete_t *ptr;
+int i;
+
+void
+foo (void)
+{
+  #pragma omp target enter data map(to: i) map(to: ptr[0])
+    /* All apply to the line above.  The first error is printed twice.  */
+    /* { dg-error "invalid use of undefined type 'struct incomplete_t'" "" { target c } .-2 } */
+    /* { dg-error "invalid use of incomplete type 'struct incomplete_t'" "" { target c++ } .-3 } */
+    /* { dg-error "array section does not have mappable type in 'map' clause" "" { target *-*-* } .-4 } */
+}
diff --git a/gcc/testsuite/g++.dg/gomp/unmappable-1.C b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
index 364f884500c..28ba184a57a 100644
--- a/gcc/testsuite/g++.dg/gomp/unmappable-1.C
+++ b/gcc/testsuite/g++.dg/gomp/unmappable-1.C
@@ -4,7 +4,7 @@
 class C
 {
 public:
-  static int static_member; /* { dg-message "static field .C::static_member. is not mappable" } */
+  static int static_member;
   virtual void f() {}
 };
 
@@ -14,7 +14,6 @@ int
 main ()
 {
 #pragma omp target map(v) /* { dg-error ".v. does not have a mappable type in .map. clause" } */
-  /* { dg-message "incomplete type .C \\\[\\\]. is not mappable" "" { target *-*-* } .-1 } */
   {
   }
 }

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH, OpenMP, C++] Allow classes with static members to be mappable
  2022-07-27 11:45   ` Tobias Burnus
@ 2022-08-17 12:13     ` Jakub Jelinek
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Jelinek @ 2022-08-17 12:13 UTC (permalink / raw)
  To: Tobias Burnus; +Cc: gcc-patches

On Wed, Jul 27, 2022 at 01:45:30PM +0200, Tobias Burnus wrote:
> OpenMP/C++: Allow classes with static members to be mappable
> 
> As this is the last lang-specific user of the omp_mappable_type hook,
> the hook is removed, keeping only a generic omp_mappable_type for
> incomplete types (or error_node).
> 
> gcc/c/ChangeLog:
> 
> 	* c-decl.cc (c_decl_attributes, finish_decl): Call omp_mappable_type
> 	instead of removed langhook.
> 	* c-typeck.cc (c_finish_omp_clauses): Likewise.
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-objcp-common.h (LANG_HOOKS_OMP_MAPPABLE_TYPE): Remove.
> 	* cp-tree.h (cp_omp_mappable_type, cp_omp_emit_unmappable_type_notes):
> 	Remove.
> 	* decl2.cc (cp_omp_mappable_type_1, cp_omp_mappable_type,
> 	cp_omp_emit_unmappable_type_notes): Remove.
> 	(cplus_decl_attributes): Call omp_mappable_type instead of
> 	removed langhook.
> 	* decl.cc (cp_finish_decl): Likewise; call cxx_incomplete_type_inform
> 	in lieu of cp_omp_emit_unmappable_type_notes.
> 	* semantics.cc (finish_omp_clauses): Likewise.
> 
> gcc/ChangeLog:
> 
> 	* gimplify.cc (omp_notice_variable): Call omp_mappable_type
>         instead of removed langhook.
> 	* omp-general.h (omp_mappable_type): New prototype.
> 	* omp-general.cc (omp_mappable_type):  New; moved from ...
> 	* langhooks.cc (lhd_omp_mappable_type): ... here.
> 	* langhooks-def.h (lhd_omp_mappable_type,
> 	LANG_HOOKS_OMP_MAPPABLE_TYPE): Remove.
> 	(LANG_HOOKS_FOR_TYPES_INITIALIZER): Remote the latter.
> 	* langhooks.h (struct lang_hooks_for_types): Remove
> 	omp_mappable_type.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/gomp/unmappable-1.C: Remove dg-error; remove dg-note no
> 	longer shown as TYPE_MAIN_DECL is NULL.
> 	* c-c++-common/gomp/map-incomplete-type.c: New test.
> 
> Co-authored-by: Chung-Lin Tang <cltang@codesourcery.com>

Ok, thanks.

	Jakub


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-08-17 12:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 11:04 [PATCH, OpenMP, C++] Allow classes with static members to be mappable Chung-Lin Tang
2022-05-05  9:12 ` Jakub Jelinek
2022-07-27 11:45   ` Tobias Burnus
2022-08-17 12:13     ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).