diff mbox

OpenACC routines -- middle end

Message ID 5a660e56-dfd1-cb7a-644d-9b8e688bdae8@codesourcery.com
State New
Headers show

Commit Message

Cesar Philippidis Nov. 22, 2016, 7:53 p.m. UTC 
On 11/18/2016 04:14 AM, Jakub Jelinek wrote:
> On Fri, Nov 11, 2016 at 03:43:02PM -0800, Cesar Philippidis wrote:

>> +	    error_at (OMP_CLAUSE_LOCATION (c),

>> +		      "%qs specifies a conflicting level of parallelism",

>> +		      omp_clause_code_name[OMP_CLAUSE_CODE (c)]);

>> +	    inform (OMP_CLAUSE_LOCATION (c_level),

>> +		    "... to the previous %qs clause here",

> 

> I think the '... ' part is unnecessary.

> Perhaps word it better like we word errors/warnings for mismatched

> attributes etc.?

> 

>> +    incompatible:

>> +      if (c_diag != NULL_TREE)

>> +	error_at (OMP_CLAUSE_LOCATION (c_diag),

>> +		  "incompatible %qs clause when applying"

>> +		  " %<%s%> to %qD, which has already been"

>> +		  " marked as an accelerator routine",

>> +		  omp_clause_code_name[OMP_CLAUSE_CODE (c_diag)],

>> +		  routine_str, fndecl);

>> +      else if (c_diag_p != NULL_TREE)

>> +	error_at (loc,

>> +		  "missing %qs clause when applying"

>> +		  " %<%s%> to %qD, which has already been"

>> +		  " marked as an accelerator routine",

>> +		  omp_clause_code_name[OMP_CLAUSE_CODE (c_diag_p)],

>> +		  routine_str, fndecl);

>> +      else

>> +	gcc_unreachable ();

>> +      if (c_diag_p != NULL_TREE)

>> +	inform (OMP_CLAUSE_LOCATION (c_diag_p),

>> +		"... with %qs clause here",

>> +		omp_clause_code_name[OMP_CLAUSE_CODE (c_diag_p)]);

> 

> Again, I think this usually would be something like "previous %qs clause"

> or similar in the inform.  Generally, I think the error message should

> be self-contained and infom should be just extra information, rather than

> error containing first half of the diagnostic message and inform the second

> one.  E.g. for translations, while such a sentence crossing the two

> diagnostic routines might make sense in english, it might look terrible in

> other languages.

> 

>> +      else

>> +	{

>> +	  /* In the front ends, we don't preserve location information for the

>> +	     OpenACC routine directive itself.  However, that of c_level_p

>> +	     should be close.  */

>> +	  location_t loc_routine = OMP_CLAUSE_LOCATION (c_level_p);

>> +	  inform (loc_routine, "... without %qs clause near to here",

>> +		  omp_clause_code_name[OMP_CLAUSE_CODE (c_diag)]);

>> +	}

>> +      /* Incompatible.  */

>> +      return -1;

>> +    }

>> +

>> +  return 0;


I've incorporated those changes in this patch. Is it ok for trunk?

Cesar

Comments

Jakub Jelinek Nov. 22, 2016, 7:58 p.m. UTC | #1 
On Tue, Nov 22, 2016 at 11:53:50AM -0800, Cesar Philippidis wrote:
> I've incorporated those changes in this patch. Is it ok for trunk?


The ChangeLog mentions omp-low.[ch] changes, but the patch doesn't include
them.
Have they been dropped, or moved to another patch?

> 2016-11-22  Cesar Philippidis  <cesar@codesourcery.com>

> 	    Thomas Schwinge  <thomas@codesourcery.com>

> 

> 	gcc/c-family/

> 	* c-attribs.c (c_common_attribute_table): Adjust "omp declare target".

> 	* c-pragma.h (enum pragma_omp_clause): Add PRAGMA_OACC_CLAUSE_BIND

> 	and PRAGMA_OACC_CLAUSE_NOHOST.

> 

> 	gcc/

> 	* gimplify.c (gimplify_scan_omp_clauses): Handle OMP_CLAUSE_BIND and

> 	OMP_CLAUSE_NOHOST.

> 	(gimplify_adjust_omp_clauses): Likewise.

> 	* omp-low.c (scan_sharing_clauses): Likewise.

> 	(verify_oacc_routine_clauses): New function.

> 	(maybe_discard_oacc_function): New function.

> 	(execute_oacc_device_lower): Don't generate code for NOHOST.

> 	* omp-low.h (verify_oacc_routine_clauses): Declare.

> 	* tree-core.h (enum omp_clause_code): Add OMP_CLAUSE_BIND and

> 	OMP_CLAUSE_NOHOST.

> 	* tree-pretty-print.c (dump_omp_clause): Likewise.

> 	* tree.c (omp_clause_num_ops): Likewise.

> 	(omp_clause_code_name): Likewise.

> 	(walk_tree_1): Handle OMP_CLAUSE_BIND, OMP_CLAUSE_NOHOST.

> 	* tree.h (OMP_CLAUSE_BIND_NAME): Define.


	Jakub
diff mbox

Patch

2016-11-22  Cesar Philippidis  <cesar@codesourcery.com>
	    Thomas Schwinge  <thomas@codesourcery.com>

	gcc/c-family/
	* c-attribs.c (c_common_attribute_table): Adjust "omp declare target".
	* c-pragma.h (enum pragma_omp_clause): Add PRAGMA_OACC_CLAUSE_BIND
	and PRAGMA_OACC_CLAUSE_NOHOST.

	gcc/
	* gimplify.c (gimplify_scan_omp_clauses): Handle OMP_CLAUSE_BIND and
	OMP_CLAUSE_NOHOST.
	(gimplify_adjust_omp_clauses): Likewise.
	* omp-low.c (scan_sharing_clauses): Likewise.
	(verify_oacc_routine_clauses): New function.
	(maybe_discard_oacc_function): New function.
	(execute_oacc_device_lower): Don't generate code for NOHOST.
	* omp-low.h (verify_oacc_routine_clauses): Declare.
	* tree-core.h (enum omp_clause_code): Add OMP_CLAUSE_BIND and
	OMP_CLAUSE_NOHOST.
	* tree-pretty-print.c (dump_omp_clause): Likewise.
	* tree.c (omp_clause_num_ops): Likewise.
	(omp_clause_code_name): Likewise.
	(walk_tree_1): Handle OMP_CLAUSE_BIND, OMP_CLAUSE_NOHOST.
	* tree.h (OMP_CLAUSE_BIND_NAME): Define.

diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c
index 964efe9..49999b8 100644
--- a/gcc/c-family/c-attribs.c
+++ b/gcc/c-family/c-attribs.c
@@ -322,7 +322,7 @@  const struct attribute_spec c_common_attribute_table[] =
 			      handle_omp_declare_simd_attribute, false },
   { "simd",		      0, 1, true,  false, false,
 			      handle_simd_attribute, false },
-  { "omp declare target",     0, 0, true, false, false,
+  { "omp declare target",     0, -1, true, false, false,
 			      handle_omp_declare_target_attribute, false },
   { "omp declare target link", 0, 0, true, false, false,
 			      handle_omp_declare_target_attribute, false },
diff --git a/gcc/c-family/c-pragma.h b/gcc/c-family/c-pragma.h
index 6d9cb08..dd2722a 100644
--- a/gcc/c-family/c-pragma.h
+++ b/gcc/c-family/c-pragma.h
@@ -149,6 +149,7 @@  enum pragma_omp_clause {
   /* Clauses for OpenACC.  */
   PRAGMA_OACC_CLAUSE_ASYNC = PRAGMA_CILK_CLAUSE_VECTORLENGTH + 1,
   PRAGMA_OACC_CLAUSE_AUTO,
+  PRAGMA_OACC_CLAUSE_BIND,
   PRAGMA_OACC_CLAUSE_COPY,
   PRAGMA_OACC_CLAUSE_COPYOUT,
   PRAGMA_OACC_CLAUSE_CREATE,
@@ -158,6 +159,7 @@  enum pragma_omp_clause {
   PRAGMA_OACC_CLAUSE_GANG,
   PRAGMA_OACC_CLAUSE_HOST,
   PRAGMA_OACC_CLAUSE_INDEPENDENT,
+  PRAGMA_OACC_CLAUSE_NOHOST,
   PRAGMA_OACC_CLAUSE_NUM_GANGS,
   PRAGMA_OACC_CLAUSE_NUM_WORKERS,
   PRAGMA_OACC_CLAUSE_PRESENT,
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8611060..04b591e 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -8373,6 +8373,8 @@  gimplify_scan_omp_clauses (tree *list_p, gimple_seq *pre_p,
 	  ctx->default_kind = OMP_CLAUSE_DEFAULT_KIND (c);
 	  break;
 
+	case OMP_CLAUSE_BIND:
+	case OMP_CLAUSE_NOHOST:
 	default:
 	  gcc_unreachable ();
 	}
@@ -9112,6 +9114,8 @@  gimplify_adjust_omp_clauses (gimple_seq *pre_p, gimple_seq body, tree *list_p,
 	  remove = true;
 	  break;
 
+	case OMP_CLAUSE_BIND:
+	case OMP_CLAUSE_NOHOST:
 	default:
 	  gcc_unreachable ();
 	}
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index a3d220d..bd0e254 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -465,7 +465,13 @@  enum omp_clause_code {
 
   /* OpenMP internal-only clause to specify grid dimensions of a gridified
      kernel.  */
-  OMP_CLAUSE__GRIDDIM_
+  OMP_CLAUSE__GRIDDIM_,
+
+  /* OpenACC clause: bind (string).  */
+  OMP_CLAUSE_BIND,
+
+  /* OpenACC clause: nohost.  */
+  OMP_CLAUSE_NOHOST
 };
 
 #undef DEFTREESTRUCT
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 096eefd..5494441 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -1022,6 +1022,15 @@  dump_omp_clause (pretty_printer *pp, tree clause, int spc, int flags)
 			 spc, flags, false);
       pp_right_paren (pp);
       break;
+    case OMP_CLAUSE_NOHOST:
+      pp_string (pp, "nohost");
+      break;
+    case OMP_CLAUSE_BIND:
+      pp_string (pp, "bind(");
+      dump_generic_node (pp, OMP_CLAUSE_BIND_NAME (clause),
+			 spc, flags, false);
+      pp_string (pp, ")");
+      break;
 
     case OMP_CLAUSE__GRIDDIM_:
       pp_string (pp, "_griddim_(");
diff --git a/gcc/tree.c b/gcc/tree.c
index a4c5b1b..521fb22 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -329,6 +329,8 @@  unsigned const char omp_clause_num_ops[] =
   1, /* OMP_CLAUSE_VECTOR_LENGTH  */
   1, /* OMP_CLAUSE_TILE  */
   2, /* OMP_CLAUSE__GRIDDIM_  */
+  1, /* OMP_CLAUSE_BIND  */
+  0, /* OMP_CLAUSE_NOHOST  */
 };
 
 const char * const omp_clause_code_name[] =
@@ -399,7 +401,9 @@  const char * const omp_clause_code_name[] =
   "num_workers",
   "vector_length",
   "tile",
-  "_griddim_"
+  "_griddim_",
+  "bind",
+  "nohost",
 };
 
 
@@ -11871,6 +11875,7 @@  walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
 	case OMP_CLAUSE__LOOPTEMP_:
 	case OMP_CLAUSE__SIMDUID_:
 	case OMP_CLAUSE__CILK_FOR_COUNT_:
+	case OMP_CLAUSE_BIND:
 	  WALK_SUBTREE (OMP_CLAUSE_OPERAND (*tp, 0));
 	  /* FALLTHRU */
 
@@ -11892,6 +11897,7 @@  walk_tree_1 (tree *tp, walk_tree_fn func, void *data,
 	case OMP_CLAUSE_DEFAULTMAP:
 	case OMP_CLAUSE_AUTO:
 	case OMP_CLAUSE_SEQ:
+	case OMP_CLAUSE_NOHOST:
 	case OMP_CLAUSE_TILE:
 	  WALK_SUBTREE_TAIL (OMP_CLAUSE_CHAIN (*tp));
 
diff --git a/gcc/tree.h b/gcc/tree.h
index b4ec3fd..a17606f 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -1526,6 +1526,9 @@  extern void protected_set_expr_location (tree, location_t);
 #define OMP_CLAUSE_VECTOR_LENGTH_EXPR(NODE) \
   OMP_CLAUSE_OPERAND ( \
     OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_VECTOR_LENGTH), 0)
+#define OMP_CLAUSE_BIND_NAME(NODE) \
+  OMP_CLAUSE_OPERAND ( \
+    OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_BIND), 0)
 
 #define OMP_CLAUSE_DEPEND_KIND(NODE) \
   (OMP_CLAUSE_SUBCODE_CHECK (NODE, OMP_CLAUSE_DEPEND)->omp_clause.subcode.depend_kind)