[2/7] OpenMP: Pass a 3-way flag to omp_check_context_selector instead of a bool.

Message ID 20250210180725.2076678-3-sloosemore@baylibre.com
State Committed
Commit 9a2116f91150cf872e7d4a66036a81ecd2526b48
Headers
Series OpenMP: Implement "begin declare variant" |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap fail Patch failed to apply

Commit Message

Sandra Loosemore Feb. 10, 2025, 6:07 p.m. UTC
  The OpenMP "begin declare variant" directive has slightly different
requirements for context selectors than regular "declare variant", so
something more than a bool is required to tell the error-checking routine
what to check.

gcc/ChangeLog
	* omp-general.cc (omp_check_context_selector): Change
	metadirective_p argument to a 3-way flag.  Add extra check for
	OMP_CTX_BEGIN_DECLARE_VARIANT.
	* omp-general.h (enum omp_ctx_directive): New.
	(omp_check_context_selector): Adjust declaration.

gcc/c/ChangeLog
	* c-parser.cc (c_finish_omp_declare_variant): Update call to
	omp_check_context_selector.
	(c_parser_omp_metadirective): Likewise.

gcc/cp/ChangeLog
	* parser.cc (cp_finish_omp_declare_variant): Update call to
	omp_check_context_selector.
	(cp_parser_omp_metadirective): Likewise.

gcc/fortran/ChangeLog
	* trans-openmp.cc (gfc_trans_omp_declare_variant): Update call to
	omp_check_context_selector.
	(gfc_trans_omp_metadirective): Likewise.
---
 gcc/c/c-parser.cc           |  6 ++++--
 gcc/cp/parser.cc            |  6 ++++--
 gcc/fortran/trans-openmp.cc |  5 +++--
 gcc/omp-general.cc          | 18 +++++++++++++++---
 gcc/omp-general.h           |  6 +++++-
 5 files changed, 31 insertions(+), 10 deletions(-)
  

Comments

Tobias Burnus Feb. 10, 2025, 6:21 p.m. UTC | #1
Namely, this patch replaces bool by the newly added enum 
'OMP_CTX_{METADIRECTIVE,{BEGIN_,}DECLARE_VARIANT}'.

Sandra Loosemore wrote:
> The OpenMP "begin declare variant" directive has slightly different
> requirements for context selectors than regular "declare variant", so
> something more than a bool is required to tell the error-checking routine
> what to check.
... admittedly, I find the OMP_CTX_.* also more readable than just 
true/false in the function call.
> gcc/ChangeLog
> 	* omp-general.cc (omp_check_context_selector): Change
> 	metadirective_p argument to a 3-way flag.  Add extra check for
> 	OMP_CTX_BEGIN_DECLARE_VARIANT.
> 	* omp-general.h (enum omp_ctx_directive): New.
> 	(omp_check_context_selector): Adjust declaration.
>
> gcc/c/ChangeLog
> 	* c-parser.cc (c_finish_omp_declare_variant): Update call to
> 	omp_check_context_selector.
> 	(c_parser_omp_metadirective): Likewise.
>
> gcc/cp/ChangeLog
> 	* parser.cc (cp_finish_omp_declare_variant): Update call to
> 	omp_check_context_selector.
> 	(cp_parser_omp_metadirective): Likewise.
>
> gcc/fortran/ChangeLog
> 	* trans-openmp.cc (gfc_trans_omp_declare_variant): Update call to
> 	omp_check_context_selector.
> 	(gfc_trans_omp_metadirective): Likewise.

LGTM for GCC 15.

Tobias
  

Patch

diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
index 106a5b48093..62c6bc031d6 100644
--- a/gcc/c/c-parser.cc
+++ b/gcc/c/c-parser.cc
@@ -26982,7 +26982,8 @@  c_finish_omp_declare_variant (c_parser *parser, tree fndecl, tree parms)
 	  ctx  = c_parser_omp_context_selector_specification (parser, parms);
 	  if (ctx == error_mark_node)
 	    goto fail;
-	  ctx = omp_check_context_selector (match_loc, ctx, false);
+	  ctx = omp_check_context_selector (match_loc, ctx,
+					    OMP_CTX_DECLARE_VARIANT);
 	  if (ctx != error_mark_node && variant != error_mark_node)
 	    {
 	      if (TREE_CODE (variant) != FUNCTION_DECL)
@@ -29099,7 +29100,8 @@  c_parser_omp_metadirective (c_parser *parser, bool *if_p)
 							     NULL_TREE);
 	  if (ctx == error_mark_node)
 	    goto error;
-	  ctx = omp_check_context_selector (match_loc, ctx, true);
+	  ctx = omp_check_context_selector (match_loc, ctx,
+					    OMP_CTX_METADIRECTIVE);
 	  if (ctx == error_mark_node)
 	    goto error;
 
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index bd43f885ebe..9d0970b4d83 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -50437,7 +50437,8 @@  cp_finish_omp_declare_variant (cp_parser *parser, cp_token *pragma_tok,
 	  ctx = cp_parser_omp_context_selector_specification (parser, true);
 	  if (ctx == error_mark_node)
 	    goto fail;
-	  ctx = omp_check_context_selector (match_loc, ctx, false);
+	  ctx = omp_check_context_selector (match_loc, ctx,
+					    OMP_CTX_DECLARE_VARIANT);
 	  if (ctx != error_mark_node && variant != error_mark_node)
 	    {
 	      tree match_loc_node
@@ -51424,7 +51425,8 @@  cp_parser_omp_metadirective (cp_parser *parser, cp_token *pragma_tok,
 	  ctx = cp_parser_omp_context_selector_specification (parser, false);
 	  if (ctx == error_mark_node)
 	    goto fail;
-	  ctx = omp_check_context_selector (match_loc, ctx, true);
+	  ctx = omp_check_context_selector (match_loc, ctx,
+					    OMP_CTX_METADIRECTIVE);
 	  if (ctx == error_mark_node)
 	    goto fail;
 
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index e9103cd3bac..580d5837bd5 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -8776,7 +8776,8 @@  gfc_trans_omp_declare_variant (gfc_namespace *ns)
 	  continue;
 	}
       set_selectors = omp_check_context_selector
-	(gfc_get_location (&odv->where), set_selectors, false);
+	(gfc_get_location (&odv->where), set_selectors,
+	 OMP_CTX_DECLARE_VARIANT);
       if (set_selectors != error_mark_node)
 	{
 	  if (!variant_proc_sym->attr.implicit_type
@@ -9082,7 +9083,7 @@  gfc_trans_omp_metadirective (gfc_code *code)
       tree ctx = gfc_trans_omp_set_selector (variant->selectors,
 					     variant->where);
       ctx = omp_check_context_selector (gfc_get_location (&variant->where),
-					ctx, true);
+					ctx, OMP_CTX_METADIRECTIVE);
       if (ctx == error_mark_node)
 	return error_mark_node;
 
diff --git a/gcc/omp-general.cc b/gcc/omp-general.cc
index 0d6f02ece31..0a2dd6b5be7 100644
--- a/gcc/omp-general.cc
+++ b/gcc/omp-general.cc
@@ -1302,7 +1302,8 @@  expr_uses_parm_decl (tree *tp, int *walk_subtrees ATTRIBUTE_UNUSED,
    it is correct or error_mark_node otherwise.  */
 
 tree
-omp_check_context_selector (location_t loc, tree ctx, bool metadirective_p)
+omp_check_context_selector (location_t loc, tree ctx,
+			    enum omp_ctx_directive directive)
 {
   bool tss_seen[OMP_TRAIT_SET_LAST], ts_seen[OMP_TRAIT_LAST];
 
@@ -1398,7 +1399,7 @@  omp_check_context_selector (location_t loc, tree ctx, bool metadirective_p)
 
 	  /* This restriction is documented in the spec in the section
 	     for the metadirective "when" clause (7.4.1 in the 5.2 spec).  */
-	  if (metadirective_p
+	  if (directive == OMP_CTX_METADIRECTIVE
 	      && ts_code == OMP_TRAIT_CONSTRUCT_SIMD
 	      && OMP_TS_PROPERTIES (ts))
 	    {
@@ -1408,10 +1409,21 @@  omp_check_context_selector (location_t loc, tree ctx, bool metadirective_p)
 	      return error_mark_node;
 	    }
 
+	  /* "simd" is not allowed at all in "begin declare variant"
+	     selectors.  */
+	  if (directive == OMP_CTX_BEGIN_DECLARE_VARIANT
+	      && ts_code == OMP_TRAIT_CONSTRUCT_SIMD)
+	    {
+	      error_at (loc,
+			"the %<simd%> selector is not permitted in a "
+			"%<begin declare variant%> context selector");
+	      return error_mark_node;
+	    }
+
 	  /* Reject expressions that reference parameter variables in
 	     "declare variant", as this is not yet implemented.  FIXME;
 	     see PR middle-end/113904.  */
-	  if (!metadirective_p
+	  if (directive != OMP_CTX_METADIRECTIVE
 	      && (ts_code == OMP_TRAIT_DEVICE_NUM
 		  || ts_code == OMP_TRAIT_USER_CONDITION))
 	    {
diff --git a/gcc/omp-general.h b/gcc/omp-general.h
index 4e143ed586b..5d44ff979f0 100644
--- a/gcc/omp-general.h
+++ b/gcc/omp-general.h
@@ -194,8 +194,12 @@  extern tree find_combined_omp_for (tree *, int *, void *);
 extern poly_uint64 omp_max_vf (bool);
 extern int omp_max_simt_vf (void);
 extern const char *omp_context_name_list_prop (tree);
+enum omp_ctx_directive
+  { OMP_CTX_DECLARE_VARIANT,
+    OMP_CTX_BEGIN_DECLARE_VARIANT,
+    OMP_CTX_METADIRECTIVE };
 extern tree omp_check_context_selector (location_t loc, tree ctx,
-					bool metadirective_p);
+					enum omp_ctx_directive directive);
 extern void omp_mark_declare_variant (location_t loc, tree variant,
 				      tree construct);
 extern int omp_context_selector_matches (tree, tree, bool);