diff mbox

default omp clause

Message ID 55B25583.4060407@acm.org
State New
Headers show

Commit Message

Nathan Sidwell July 24, 2015, 3:10 p.m. UTC 
Jakub,
in working on openacc default clause handling, I found the logic in 
omp_notice_variable a little hard to follow, due to the size of function and 
depth of nesting.

I broke out the existing default handling to a separate function, and made the 
control flow a bit clearer (IMHO).

ok for trunk?

nathan

Comments

Jakub Jelinek July 24, 2015, 3:22 p.m. UTC | #1 
On Fri, Jul 24, 2015 at 11:10:59AM -0400, Nathan Sidwell wrote:
> 2015-07-24  Nathan Sidwell  <nathan@codesourcery.com>
> 
> 	* gimplify.c (omp_default_clause): New function.  Reorganize flow
> 	for clarity. Broken out of ...
> 	(omp_notice_variable): ... here.

Ok, except:

> +    case OMP_CLAUSE_DEFAULT_NONE:
> +      {
> +	const char *accn;

Can you rename this variable to something that makes sense?
kind, or construct, or name, or directive?

> +
> +	if (ctx->region_type & ORT_PARALLEL)
> +	  accn = "parallel";
> +	else if (ctx->region_type & ORT_TASK)
> +	  accn = "task";
> +	else if (ctx->region_type & ORT_TEAMS)
> +	  accn = "teams";
> +	else
> +	  gcc_unreachable ();
> +	
> +	error ("%qE not specified in enclosing %s",
> +	       DECL_NAME (lang_hooks.decls.omp_report_decl (decl)), accn);
> +	error_at (ctx->location, "enclosing %s", accn);

	Jakub
Nathan Sidwell July 24, 2015, 4:13 p.m. UTC | #2 
On 07/24/15 11:22, Jakub Jelinek wrote:
> On Fri, Jul 24, 2015 at 11:10:59AM -0400, Nathan Sidwell wrote:

>> +    case OMP_CLAUSE_DEFAULT_NONE:
>> +      {
>> +	const char *accn;
>
> Can you rename this variable to something that makes sense?
> kind, or construct, or name, or directive?

Hm, yeah, that was a rather pathetic name.  went with 'rtype'.

nathan
diff mbox

Patch

2015-07-24  Nathan Sidwell  <nathan@codesourcery.com>

	* gimplify.c (omp_default_clause): New function.  Reorganize flow
	for clarity. Broken out of ...
	(omp_notice_variable): ... here.

Index: gcc/gimplify.c
===================================================================
--- gcc/gimplify.c	(revision 226154)
+++ gcc/gimplify.c	(working copy)
@@ -5764,6 +5764,96 @@  omp_notice_threadprivate_variable (struc
   return false;
 }
 
+/* Determine outer default flags for DECL mentioned in an OMP region
+   but not declared in an enclosing clause.
+
+   ??? Some compiler-generated variables (like SAVE_EXPRs) could be
+   remapped firstprivate instead of shared.  To some extent this is
+   addressed in omp_firstprivatize_type_sizes, but not
+   effectively.  */
+
+static unsigned
+omp_default_clause (struct gimplify_omp_ctx *ctx, tree decl,
+		    bool in_code, unsigned flags)
+{
+  enum omp_clause_default_kind default_kind = ctx->default_kind;
+  enum omp_clause_default_kind kind;
+
+  kind = lang_hooks.decls.omp_predetermined_sharing (decl);
+  if (kind != OMP_CLAUSE_DEFAULT_UNSPECIFIED)
+    default_kind = kind;
+
+  switch (default_kind)
+    {
+    case OMP_CLAUSE_DEFAULT_NONE:
+      {
+	const char *accn;
+
+	if (ctx->region_type & ORT_PARALLEL)
+	  accn = "parallel";
+	else if (ctx->region_type & ORT_TASK)
+	  accn = "task";
+	else if (ctx->region_type & ORT_TEAMS)
+	  accn = "teams";
+	else
+	  gcc_unreachable ();
+	
+	error ("%qE not specified in enclosing %s",
+	       DECL_NAME (lang_hooks.decls.omp_report_decl (decl)), accn);
+	error_at (ctx->location, "enclosing %s", accn);
+      }
+      /* FALLTHRU */
+    case OMP_CLAUSE_DEFAULT_SHARED:
+      flags |= GOVD_SHARED;
+      break;
+    case OMP_CLAUSE_DEFAULT_PRIVATE:
+      flags |= GOVD_PRIVATE;
+      break;
+    case OMP_CLAUSE_DEFAULT_FIRSTPRIVATE:
+      flags |= GOVD_FIRSTPRIVATE;
+      break;
+    case OMP_CLAUSE_DEFAULT_UNSPECIFIED:
+      /* decl will be either GOVD_FIRSTPRIVATE or GOVD_SHARED.  */
+      gcc_assert ((ctx->region_type & ORT_TASK) != 0);
+      if (struct gimplify_omp_ctx *octx = ctx->outer_context)
+	{
+	  omp_notice_variable (octx, decl, in_code);
+	  for (; octx; octx = octx->outer_context)
+	    {
+	      splay_tree_node n2;
+
+	      if ((octx->region_type & (ORT_TARGET_DATA | ORT_TARGET)) != 0)
+		continue;
+	      n2 = splay_tree_lookup (octx->variables, (splay_tree_key) decl);
+	      if (n2 && (n2->value & GOVD_DATA_SHARE_CLASS) != GOVD_SHARED)
+		{
+		  flags |= GOVD_FIRSTPRIVATE;
+		  goto found_outer;
+		}
+	      if ((octx->region_type & (ORT_PARALLEL | ORT_TEAMS)) != 0)
+		{
+		  flags |= GOVD_SHARED;
+		  goto found_outer;
+		}
+	    }
+	}
+      
+      if (TREE_CODE (decl) == PARM_DECL
+	  || (!is_global_var (decl)
+	      && DECL_CONTEXT (decl) == current_function_decl))
+	flags |= GOVD_FIRSTPRIVATE;
+      else
+	flags |= GOVD_SHARED;
+    found_outer:
+      break;
+
+    default:
+      gcc_unreachable ();
+    }
+
+  return flags;
+}
+
 /* Record the fact that DECL was used within the OMP context CTX.
    IN_CODE is true when real code uses DECL, and false when we should
    merely emit default(none) errors.  Return true if DECL is going to
@@ -5822,90 +5912,12 @@  omp_notice_variable (struct gimplify_omp
 
   if (n == NULL)
     {
-      enum omp_clause_default_kind default_kind, kind;
-      struct gimplify_omp_ctx *octx;
-
       if (ctx->region_type == ORT_WORKSHARE
 	  || ctx->region_type == ORT_SIMD
 	  || ctx->region_type == ORT_TARGET_DATA)
 	goto do_outer;
 
-      /* ??? Some compiler-generated variables (like SAVE_EXPRs) could be
-	 remapped firstprivate instead of shared.  To some extent this is
-	 addressed in omp_firstprivatize_type_sizes, but not effectively.  */
-      default_kind = ctx->default_kind;
-      kind = lang_hooks.decls.omp_predetermined_sharing (decl);
-      if (kind != OMP_CLAUSE_DEFAULT_UNSPECIFIED)
-	default_kind = kind;
-
-      switch (default_kind)
-	{
-	case OMP_CLAUSE_DEFAULT_NONE:
-	  if ((ctx->region_type & ORT_PARALLEL) != 0)
-	    {
-	      error ("%qE not specified in enclosing parallel",
-		     DECL_NAME (lang_hooks.decls.omp_report_decl (decl)));
-	      error_at (ctx->location, "enclosing parallel");
-	    }
-	  else if ((ctx->region_type & ORT_TASK) != 0)
-	    {
-	      error ("%qE not specified in enclosing task",
-		     DECL_NAME (lang_hooks.decls.omp_report_decl (decl)));
-	      error_at (ctx->location, "enclosing task");
-	    }
-	  else if (ctx->region_type & ORT_TEAMS)
-	    {
-	      error ("%qE not specified in enclosing teams construct",
-		     DECL_NAME (lang_hooks.decls.omp_report_decl (decl)));
-	      error_at (ctx->location, "enclosing teams construct");
-	    }
-	  else
-	    gcc_unreachable ();
-	  /* FALLTHRU */
-	case OMP_CLAUSE_DEFAULT_SHARED:
-	  flags |= GOVD_SHARED;
-	  break;
-	case OMP_CLAUSE_DEFAULT_PRIVATE:
-	  flags |= GOVD_PRIVATE;
-	  break;
-	case OMP_CLAUSE_DEFAULT_FIRSTPRIVATE:
-	  flags |= GOVD_FIRSTPRIVATE;
-	  break;
-	case OMP_CLAUSE_DEFAULT_UNSPECIFIED:
-	  /* decl will be either GOVD_FIRSTPRIVATE or GOVD_SHARED.  */
-	  gcc_assert ((ctx->region_type & ORT_TASK) != 0);
-	  if (ctx->outer_context)
-	    omp_notice_variable (ctx->outer_context, decl, in_code);
-	  for (octx = ctx->outer_context; octx; octx = octx->outer_context)
-	    {
-	      splay_tree_node n2;
-
-	      if ((octx->region_type & (ORT_TARGET_DATA | ORT_TARGET)) != 0)
-		continue;
-	      n2 = splay_tree_lookup (octx->variables, (splay_tree_key) decl);
-	      if (n2 && (n2->value & GOVD_DATA_SHARE_CLASS) != GOVD_SHARED)
-		{
-		  flags |= GOVD_FIRSTPRIVATE;
-		  break;
-		}
-	      if ((octx->region_type & (ORT_PARALLEL | ORT_TEAMS)) != 0)
-		break;
-	    }
-	  if (flags & GOVD_FIRSTPRIVATE)
-	    break;
-	  if (octx == NULL
-	      && (TREE_CODE (decl) == PARM_DECL
-		  || (!is_global_var (decl)
-		      && DECL_CONTEXT (decl) == current_function_decl)))
-	    {
-	      flags |= GOVD_FIRSTPRIVATE;
-	      break;
-	    }
-	  flags |= GOVD_SHARED;
-	  break;
-	default:
-	  gcc_unreachable ();
-	}
+      flags = omp_default_clause (ctx, decl, in_code, flags);
 
       if ((flags & GOVD_PRIVATE)
 	  && lang_hooks.decls.omp_private_outer_ref (decl))