Patchwork [cilkplus] misc cleanups for <#pragma simd> implementation

login
register
mail settings
Submitter Aldy Hernandez
Date April 8, 2013, 3:09 p.m.
Message ID <5162DDBD.8030304@redhat.com>
Download mbox | patch
Permalink /patch/234803/
State New
Headers show

Comments

Aldy Hernandez - April 8, 2013, 3:09 p.m.
On 04/08/13 08:59, Iyer, Balaji V wrote:
> Hi Aldy,
> 	Here are the things I  found with the patch. All my comments have "BVI:" in front of them.

BTW, it would be nice if you could use standard mailer quotation when 
responding (">", etc).

> -  return;
>
> BVI: I am OK with removing this return, but the reason why I put it there is because it gets easier for me to set the break point there.

This is not standard practice in GCC source code.  It will have to be 
removed if we ever merge.

>       case PRAGMA_CILK_GRAINSIZE:
> -      if (context == pragma_external)
> -	{
> -	  c_parser_error (parser,"pragma grainsize must be inside a function");
> -	  return false;
> -	}
> -      if (flag_enable_cilk)
> -	c_parser_cilk_grainsize (parser);
> -      else
> -	{
> -	  warning (0, "pragma grainsize ignored");
> -	  c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
> -	}
> +      if (!c_parser_pragma_simd_ok_p (parser, context))
> +	return false;
> +      cilkplus_local_simd_values.loc = loc;
> +      c_parser_cilk_grainsize (parser);
>
> BVI: This is incorrect. #pragma grainsize is part of cilk keywords. It has no relation to the pragma simd and it will work wthout vectorization support.

Fixed, let me know if the current implementation is correct.

> +// FIXME: We should really rewrite all this psv* business to use vectors.
> +/* Given an index into the pragma simd list (PSV_INDEX), find its
> +   entry and return it.  */
>
> BVI: I am in the process of doing so. I will send out that patch as soon as I get some free time.

Ok, I have left all the FIXMEs so we don't miss any of them.

>     for (ps_iter = psv_head; ps_iter->ptr_next != NULL;
>          ps_iter = ps_iter->ptr_next)
> -    {
> -      ;
> -    }
> +    ;
>
> BVI: Are you sure the compiler let you get away this this? It gave me a warning once (in stage2 I believe).

Sure, I've done it for years.

> BVI: I have fixed these scripts already: the correct notation that I have used is "cilkplus_<type>_<language>_<compile/execute/errors>.exp

Ok, I have removed them from my patch.
> -
> +	
> BVI: Why did you replace a space with a tab?

Whoops, removed the space (and the tab).

How about this?
commit 1847c6c76ca2ed0da68cb7985fde4c0b4d634b65
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Mon Apr 8 09:59:38 2013 -0500

    Minor cleanups for pragma simd implementation.

Patch

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index f00d28d..a48b011 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -117,26 +117,11 @@  c_parse_init (void)
       ridpointers [(int) c_common_reswords[i].rid] = id;
     }
 
-  /* Here we initialize the simd_values structure. We only need it 
-     initialized the first time, after each consumptions, for-loop will 
-     automatically consume the values and delete the information.  */
-  cilkplus_local_simd_values.index              = 0;
-  cilkplus_local_simd_values.pragma_encountered = false;
-  cilkplus_local_simd_values.types              = P_SIMD_NOASSERT;
-  cilkplus_local_simd_values.vectorlength       = NULL_TREE;
-  cilkplus_local_simd_values.vec_length_list    = NULL;
-  cilkplus_local_simd_values.vec_length_size    = 0;
-  cilkplus_local_simd_values.private_vars       = NULL_TREE;
-  cilkplus_local_simd_values.priv_var_list      = NULL;
-  cilkplus_local_simd_values.priv_var_size      = 0;
-  cilkplus_local_simd_values.linear_vars        = NULL_TREE;
-  cilkplus_local_simd_values.linear_var_size    = 0;
-  cilkplus_local_simd_values.linear_var_list    = NULL;
-  cilkplus_local_simd_values.linear_steps       = NULL_TREE;
-  cilkplus_local_simd_values.linear_steps_list  = NULL;
-  cilkplus_local_simd_values.linear_steps_size  = 0;
-  cilkplus_local_simd_values.reduction_vals     = NULL;
-  cilkplus_local_simd_values.ptr_next           = NULL;
+  /* Only initialize the first time.  After each consumption, the
+     for-loop handling code (c_finish_loop) will automatically consume
+     the values and delete the information.  */
+  memset (&cilkplus_local_simd_values, 0,
+	  sizeof (cilkplus_local_simd_values));
 
   clear_pragma_simd_list ();
 }
@@ -1251,12 +1236,16 @@  static void c_parser_objc_at_synthesize_declaration (c_parser *);
 static void c_parser_objc_at_dynamic_declaration (c_parser *);
 static bool c_parser_objc_diagnose_bad_element_prefix
   (c_parser *, struct c_declspecs *);
+
+// FIXME: Re-work this so there are only prototypes for mutually
+// recursive functions.
+/* Cilk Plus supporting routines.  */
 static void c_parser_cilk_for_statement (c_parser *, tree);
-void c_parser_simd_linear (c_parser *);
-void c_parser_simd_private (c_parser *);
-void c_parser_simd_assert (c_parser *, bool);
-void c_parser_simd_vectorlength (c_parser *);
-void c_parser_simd_reduction (c_parser *);
+static void c_parser_simd_linear (c_parser *);
+static void c_parser_simd_private (c_parser *);
+static void c_parser_simd_assert (c_parser *, bool);
+static void c_parser_simd_vectorlength (c_parser *);
+static void c_parser_simd_reduction (c_parser *);
 static tree c_parser_array_notation (location_t, c_parser *, tree, tree);
 /* Parse a translation unit (C90 6.7, C99 6.9).
 
@@ -8799,7 +8788,7 @@  c_parser_objc_at_dynamic_declaration (c_parser *parser)
       #pragma simd assert
       #pragma simd noassert
  */
-void
+static void
 c_parser_simd_assert (c_parser *parser, bool is_assert)
 {
   c_token *token;
@@ -8856,7 +8845,7 @@  c_parser_simd_assert (c_parser *parser, bool is_assert)
       #pragma simd linear (<variable>:[<steps>], ...)
  */
 
-void
+static void
 c_parser_simd_linear (c_parser *parser)
 {
   tree linear_var_list = NULL_TREE, linear_steps_list = NULL_TREE;
@@ -8967,14 +8956,13 @@  c_parser_simd_linear (c_parser *parser)
 	}
     }
   cilkplus_local_simd_values.pragma_encountered = true;
-  return;
 }
 
 /* This function will parse the pragma simd private in the Cilkplus 
    language extension. The correct syntax is: 
 	#pragma simd private (<variable> [, <variable>])
  */
-void
+static void
 c_parser_simd_private (c_parser *parser)
 {
   tree private_var = NULL_TREE;
@@ -9068,15 +9056,13 @@  c_parser_simd_private (c_parser *parser)
 	  c_parser_for_statement (parser);
 	}
     }
-  
-  return;
 }
 
 /* This function will parse the pragma simd vectorlength in the Cilkplus 
    language extension. The correct syntax is: 
 	#pragma simd vectorlength (<INTEGER> [, <INTEGER>]*)
  */
-void
+static void
 c_parser_simd_vectorlength (c_parser *parser)
 {
   tree vec_length_list = NULL_TREE, v_length_value = NULL_TREE;
@@ -9168,8 +9154,6 @@  c_parser_simd_vectorlength (c_parser *parser)
 	  c_parser_for_statement (parser);
 	}
     }
-  
-  return;
 }
 
 /* This function will parser the Pragma SIMD Reduction in the Cilkplus language 
@@ -9177,7 +9161,7 @@  c_parser_simd_vectorlength (c_parser *parser)
 	  #pragma simd reduction (<operator>:<variable> [, <variable>]*)
  */
 
-void
+static void
 c_parser_simd_reduction (c_parser *parser)
 {
   c_token *token;
@@ -9299,7 +9283,6 @@  c_parser_simd_reduction (c_parser *parser)
 	   values given in the local_pragma_simd variable.  */ 
 	c_parser_for_statement (parser);
     }
-  return;
 }
 
 /* This function helps parse the grainsize pragma available in the Cilkplus 
@@ -9353,9 +9336,35 @@  c_parser_cilk_grainsize (c_parser *parser)
     }
   else
     c_parser_skip_to_pragma_eol (parser);
-  return;
 }
-	    
+
+/* Helper function for c_parser_pragma.  Perform some sanity checking
+   for <#pragma simd> constructs.  Returns FALSE if there was a
+   problem.  */
+
+static bool
+c_parser_pragma_simd_ok_p (c_parser *parser, enum pragma_context context)
+{
+  if (!flag_enable_cilk)
+    {
+      warning (0, "pragma simd ignored because -fcilkplus is not enabled");
+      c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
+      return false;
+    }
+  if (!flag_tree_vectorize)
+    {
+      warning (0, "pragma simd is useless without -ftree-vectorize");
+      c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
+      return false;
+    }
+  if (context == pragma_external)
+    {
+      c_parser_error (parser,"pragma simd must be inside a function");
+      return false;
+    }
+  return true;
+}
+
 /* Handle pragmas.  Some OpenMP pragmas are associated with, and therefore
    should be considered, statements.  ALLOW_STMT is true if we're within
    the context of a function and such pragmas are to be allowed.  Returns
@@ -9364,6 +9373,7 @@  c_parser_cilk_grainsize (c_parser *parser)
 static bool
 c_parser_pragma (c_parser *parser, enum pragma_context context)
 {
+  location_t loc = c_parser_peek_token (parser)->location;
   unsigned int id;
 
   id = c_parser_peek_token (parser)->pragma_kind;
@@ -9420,7 +9430,7 @@  c_parser_pragma (c_parser *parser, enum pragma_context context)
       return false;
 
     case PRAGMA_OMP_SECTION:
-      error_at (c_parser_peek_token (parser)->location,
+      error_at (loc,
 		"%<#pragma omp section%> may only be used in "
 		"%<#pragma omp sections%> construct");
       c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
@@ -9432,20 +9442,21 @@  c_parser_pragma (c_parser *parser, enum pragma_context context)
       return false;
       
     case PRAGMA_CILK_GRAINSIZE:
-      if (context == pragma_external)
+      cilkplus_local_simd_values.loc = loc;
+      if (!flag_enable_cilk)
 	{
-	  c_parser_error (parser,"pragma grainsize must be inside a function");
+	  warning_at (loc, 0, "pragma grainsize ignored because -fcilkplus "
+		      "is not enabled");
+	  c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
 	  return false;
 	}
-      if (flag_enable_cilk) 
-	c_parser_cilk_grainsize (parser);
-      else
+      if (context == pragma_external)
 	{
-	  warning (0, "pragma grainsize ignored");
-	  c_parser_skip_until_found (parser, CPP_PRAGMA_EOL, NULL);
+	  c_parser_error (parser,"pragma grainsize must be inside a function");
+	  return false;
 	}
+      c_parser_cilk_grainsize (parser);
       return false;
-      break;
 
     case PRAGMA_SIMD_ASSERT:
       flag_tree_vectorize = 1;
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 5c7e6cc..e74fd29 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -9115,15 +9115,7 @@  c_finish_loop (location_t start_locus, tree cond, tree incr, tree body,
 	      psv_head_insert (*cilkplus_ps_values);
 	  else
 	    LABEL_EXPR_PRAGMA_SIMD_INDEX (top) = INVALID_PRAGMA_SIMD_SLOT;
-	    
-	  /* Now we initialize them all to zeros.  */
-	  cilkplus_ps_values->pragma_encountered = false;
-	  cilkplus_ps_values->types              = P_SIMD_NOASSERT;
-	  cilkplus_ps_values->vectorlength       = NULL_TREE;
-	  cilkplus_ps_values->private_vars       = NULL_TREE;
-	  cilkplus_ps_values->linear_vars        = NULL_TREE;
-	  cilkplus_ps_values->linear_steps       = NULL_TREE;
-	  cilkplus_ps_values->reduction_vals     = NULL;
+	  memset (&cilkplus_ps_values, 0, sizeof (cilkplus_ps_values));
 	}  
       
       add_stmt (top);
diff --git a/gcc/gimple.c b/gcc/gimple.c
index 7516b46..83288df 100644
--- a/gcc/gimple.c
+++ b/gcc/gimple.c
@@ -574,7 +574,8 @@  gimple
 gimple_build_label (tree label)
 {
   gimple p = gimple_build_with_ops (GIMPLE_LABEL, ERROR_MARK, 1);
-  GIMPLE_PRAGMA_SIMD_INDEX (p) = PRAGMA_SIMD_INDEX (label);
+  if (flag_enable_cilk)
+    GIMPLE_PRAGMA_SIMD_INDEX (p) = PRAGMA_SIMD_INDEX (label);
   gimple_label_set_label (p, label);
   return p;
 }
@@ -2028,7 +2029,8 @@  gimple_set_bb (gimple stmt, basic_block bb)
 {
   stmt->gsbase.bb = bb;
 
-  if ( (bb != NULL) && (gimple_code (stmt) == GIMPLE_LABEL))
+  if (flag_enable_cilk
+      && bb && (gimple_code (stmt) == GIMPLE_LABEL))
     bb->pragma_simd_index = GIMPLE_PRAGMA_SIMD_INDEX (stmt);
 
   /* If the statement is a label, add the label to block-to-labels map
diff --git a/gcc/pragma_simd.c b/gcc/pragma_simd.c
index c39fd62..7381060 100644
--- a/gcc/pragma_simd.c
+++ b/gcc/pragma_simd.c
@@ -42,12 +42,65 @@  along with GCC; see the file COPYING3.  If not see
 
 struct pragma_simd_values *psv_head;
 
-/* This function Empties the pragma simd data structure.  */
+/* Verify that the <#pragma simd> clauses have been properly resolved.
+   INDEX is the pragma_simd_index into the global table.  */
+
+void
+pragma_simd_verify_clauses (int index)
+{
+  struct pragma_simd_values *vals = psv_find_node (index);
+  location_t loc = vals ? vals->loc : UNKNOWN_LOCATION;
+
+  if ((!clause_resolved_p (P_SIMD_VECTORLENGTH, index)))
+    {
+      if (pragma_simd_assert_requested_p (index))
+	{
+	  error_at (loc, "vectorlength in pragma simd not picked from list");
+	  exit (ICE_EXIT_CODE);
+	}
+      else 
+	warning_at (0, loc,
+		    "vectorlength in pragma simd not picked from list");
+    }
+  if (!clause_resolved_p (P_SIMD_PRIVATE, index))
+    { 
+      if (pragma_simd_assert_requested_p (index))
+	{ 
+	  error_at (loc, "unable to make all variables private");
+	  exit (ICE_EXIT_CODE);
+	} 
+      else
+	warning_at (0, loc,
+		    "unable to make all variables private in pragma simd");
+    }     
+  if (!clause_resolved_p (P_SIMD_LINEAR, index))
+    {
+      if (pragma_simd_assert_requested_p (index))
+	{
+	  error_at (loc, "unable to pick requested step-size in pragma simd");
+	  exit (ICE_EXIT_CODE);
+	}
+      else
+	warning (loc, "unable to pick requested step-size in pragma simd");
+    }
+  if (!clause_resolved_p (P_SIMD_REDUCTION, index))
+    {
+      if (pragma_simd_assert_requested_p (index))
+	{
+	  error_at (loc, "unable to satisfy all reductions in pragma simd");
+	  exit (ICE_EXIT_CODE);
+	}
+      else
+	warning_at (0, loc, "unable to satisfy all reductions in pragma simd");
+    }
+}
+
+/* Clear the pragma simd data structure.  */
+
 void
 clear_pragma_simd_list (void)
 {
   psv_head = NULL;
-  return;
 }
 
 /* this function will check and see if a certain clause is resolved
@@ -104,6 +157,7 @@  set_OK_for_certain_clause (enum pragma_simd_kind clause_type, bool set_value,
   if (!psv_head)
     return;
 
+  // FIXME: Why not use psv_find_node?
   for (ps_iter = psv_head; ps_iter != NULL; ps_iter = ps_iter->ptr_next)
     {
       if (ps_iter->pragma_encountered && (ps_iter->index == pragma_simd_index))
@@ -155,9 +209,10 @@  all_reductions_satisfied_p (int pragma_simd_index)
     }
   return true;
 }
-  
-/* This function will search the pragma simd list to see if a node for a 
-   certain loop exist (based on an index).  */
+
+// FIXME: We should really rewrite all this psv* business to use vectors.
+/* Given an index into the pragma simd list (PSV_INDEX), find its
+   entry and return it.  */
 
 struct pragma_simd_values *
 psv_find_node (int psv_index)
@@ -171,17 +226,15 @@  psv_find_node (int psv_index)
     return NULL;
   
   for (ps_iter = psv_head; ps_iter != NULL; ps_iter = ps_iter->ptr_next)
-    {
-      if ((ps_iter->index == psv_index) && ps_iter->pragma_encountered)
-	return ps_iter;
-    }
+    if ((ps_iter->index == psv_index) && ps_iter->pragma_encountered)
+      return ps_iter;
 
-  /* We should not get here.  */
+  gcc_unreachable ();
   return NULL;
 }
 
-/* this function will insert a value at the head of the list that holds
-   pragma simd information for the loops.  */
+/* Insert LOCAL_SIMD_VALUES into the global pragma simd table.  Return
+   the index into the table for the new entry.  */
 
 int
 psv_head_insert (struct pragma_simd_values local_simd_values)
@@ -194,13 +247,9 @@  psv_head_insert (struct pragma_simd_values local_simd_values)
   if (psv_head == NULL)
     {
       psv_head = (struct pragma_simd_values *)
-	xmalloc (sizeof (struct pragma_simd_values));
-      gcc_assert (psv_head != NULL);
+	xcalloc (1, sizeof (struct pragma_simd_values));
+      psv_head->loc = local_simd_values.loc;
       psv_head->pragma_encountered  = local_simd_values.pragma_encountered;
-      /* We keep the head pointer index to be invalid pragma simd slot + 1.
-       * This is done before fi we want to debug then we can set invalid pragma
-       * simd_slot value to some arbitary number and then see if we are
-       * catching the pragmas correctly.  */
       psv_head->index = INVALID_PRAGMA_SIMD_SLOT + 1;
       psv_head->types = local_simd_values.types;
       
@@ -244,15 +293,15 @@  psv_head_insert (struct pragma_simd_values local_simd_values)
   
   for (ps_iter = psv_head; ps_iter->ptr_next != NULL;
        ps_iter = ps_iter->ptr_next)
-    {
-      ;
-    }
+    ;
 
   ps_iter->ptr_next = (struct pragma_simd_values *)
-    xmalloc (sizeof (struct pragma_simd_values));
-  gcc_assert (ps_iter->ptr_next != NULL);
- 
+    xcalloc (1, sizeof (struct pragma_simd_values));
+
+  // FIXME: There are a bunch of fields not initialized here:
+  // i.e. vlength_OK, pvars_OK, linear_steps_size
   ps_iter->ptr_next->pragma_encountered = local_simd_values.pragma_encountered;
+  ps_iter->ptr_next->loc = local_simd_values.loc;
   ps_iter->ptr_next->index = ps_iter->index + 1;
   ps_iter->ptr_next->types = local_simd_values.types;
   ps_iter->ptr_next->vectorlength = local_simd_values.vectorlength;
@@ -282,6 +331,7 @@  pragma_simd_assert_requested_p (int ps_index)
   if (ps_index == 0) 
     return 0;
 
+  // FIXME: Why not use psv_find_node.
   for (ps_iter = psv_head; ps_iter; ps_iter = ps_iter->ptr_next)
     {
       if ((ps_iter->pragma_encountered == true) && (ps_iter->index == ps_index))
@@ -322,7 +372,8 @@  pragma_simd_acceptable_vlength_p (int ps_index,
   possible_vector_length = possible_vectorization_factor;
 
   vl_tree = build_int_cst (integer_type_node, possible_vector_length);
-  
+
+  // FIXME: Why not use psv_find_node?
   for (ps_iter = psv_head; ps_iter != NULL; ps_iter = ps_iter->ptr_next)
     {
       if ((ps_iter->pragma_encountered == true) && (ps_iter->index == ps_index))
@@ -359,6 +410,7 @@  pragma_simd_vectorize_loop_p (int ps_index)
   if (ps_index <= INVALID_PRAGMA_SIMD_SLOT) 
     return false;
 
+  // FIXME: Why not use psv_find_node?
   for (ps_iter = psv_head; ps_iter != NULL; ps_iter = ps_iter->ptr_next) 
     if (ps_iter->index == ps_index) 
       return ps_iter->pragma_encountered;
@@ -564,6 +616,7 @@  check_off_reduction_var (gimple reduc_stmt, int pragma_simd_index)
     }
 
 
+  // FIXME: Why not use psv_find_node?
   for (ps_iter = psv_head; ps_iter != NULL; ps_iter = ps_iter->ptr_next) 
     if (ps_iter->pragma_encountered && (ps_iter->index == pragma_simd_index)) 
       break;
diff --git a/gcc/tree-vectorizer.c b/gcc/tree-vectorizer.c
index 8fffffb..026405b 100644
--- a/gcc/tree-vectorizer.c
+++ b/gcc/tree-vectorizer.c
@@ -130,65 +130,12 @@  vectorize_loops (void)
 
   vect_location = UNKNOWN_LOC;
 
-  FOR_EACH_LOOP (li, loop, 0)
+  if (flag_enable_cilk)
     {
-      if (flag_enable_cilk)
-	{
-	  if ((!clause_resolved_p (P_SIMD_VECTORLENGTH, 
-				   loop->pragma_simd_index)))
-	    {
-	      if (pragma_simd_assert_requested_p (loop->pragma_simd_index))
-		{
-		  error ("Vectorlength not picked from the list."
-			 " ASSERT REQUESTED");
-		  exit (ICE_EXIT_CODE);
-		}
-	      else 
-		{
-		  warning (0, "Vectorlength not picked from list.");
-		}
-	    }
-	  if (!clause_resolved_p (P_SIMD_PRIVATE, loop->pragma_simd_index))
-	    { 
-	      if (pragma_simd_assert_requested_p (loop->pragma_simd_index))
-		{ 
-		  error ("Unable to make all variables private. "
-			  "ASSERT REQUESTED");
-		  exit(ICE_EXIT_CODE);
-		} 
-	      else
-		{
-		  warning (0, "Unable to make all variables private.");
-		} 
-	    }     
-	  if (!clause_resolved_p (P_SIMD_LINEAR, loop->pragma_simd_index))
-	    {
-	      if (pragma_simd_assert_requested_p (loop->pragma_simd_index))
-		{
-		  error ("Unable to pick requested step-size. "
-			 "ASSERT REQUESTED");
-		  exit(ICE_EXIT_CODE);
-		}
-	      else
-		{
-		  warning (0, "Unable to pick requested step-size.");
-		}
-	    }
-	  if (!clause_resolved_p (P_SIMD_REDUCTION, loop->pragma_simd_index))
-	    {
-	      if (pragma_simd_assert_requested_p (loop->pragma_simd_index))
-		{
-		  error ("Unable to satisfy all reductions.\nASSERT REQUESTED");
-		  exit(ICE_EXIT_CODE);
-		}
-	      else
-		{
-		  warning (0, "Unable to satisfy all reductions...continuing");
-		}
-	    }
-	}
+      FOR_EACH_LOOP (li, loop, 0)
+	pragma_simd_verify_clauses (loop->pragma_simd_index);
     }
-  
+
   statistics_counter_event (cfun, "Vectorized loops", num_vectorized_loops);
   if (dump_enabled_p ()
       || (num_vectorized_loops > 0 && dump_enabled_p ()))
diff --git a/gcc/tree.h b/gcc/tree.h
index cf6a135..dd4de3e 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -65,12 +65,22 @@  struct reduction_values
   struct reduction_values *ptr_next;
 };
 
-/* Since we can have multiple pragma simd, this will hold the values of 
-   each of the pragma simd and then as soon as it finds a for loop 
-   it will transfer those values into the loop tree structure.  */
+/* Since we can have multiple pragma simds, this holds the values of
+   each of the pragma simds as we are parsing them.  An index into
+   this table gets propagated to the tree structure for LABEL_DECL's
+   (as for loops are being parsed), then to the gimple structure for
+   GIMPLE_LABEL's, then to the BB structure, and finally to the loop
+   structure.  */
 struct pragma_simd_values
 {
   int index;
+
+  /* Location of the #pragma itself.  Ideally, we should keep the
+     location for each clause so we can give more detailed
+     diagnostics, but this will work for now.  */
+  location_t loc;
+
+  // FIXME: All these need to be commented.
   bool pragma_encountered;
   unsigned int types;
   tree vectorlength;
@@ -93,6 +103,8 @@  struct pragma_simd_values
   struct pragma_simd_values *ptr_next;
 };
 
+// FIXME: This should not be globally visible.  Instead we should have
+// accessor functions, with a more meaningful name.
 extern struct pragma_simd_values *psv_head;
 
 
@@ -3113,15 +3125,13 @@  struct GTY(()) tree_field_decl {
 #define EH_LANDING_PAD_NR(NODE) \
   (LABEL_DECL_CHECK (NODE)->label_decl.eh_landing_pad_nr)
 
-
+/* In a LABEL_DECL, the index into the pragma simd table.  */
 #define PRAGMA_SIMD_INDEX(NODE)					\
-  (LABEL_DECL_CHECK(NODE)->label_decl.pragma_simd_index)
+  (LABEL_DECL_CHECK (NODE)->label_decl.pragma_simd_index)
 
+/* As above, but for a LABEL_EXPR.  */
 #define LABEL_EXPR_PRAGMA_SIMD_INDEX(NODE)			\
-  (PRAGMA_SIMD_INDEX(TREE_OPERAND(LABEL_EXPR_CHECK(NODE), 0)))
-
-
-
+  (PRAGMA_SIMD_INDEX (TREE_OPERAND (LABEL_EXPR_CHECK (NODE), 0)))
 
 /* In LABEL_DECL nodes, nonzero means that an error message about
    jumping into such a binding contour has been printed for this label.  */
@@ -6623,14 +6633,9 @@  extern bool block_may_fallthru (const_tree);
 #define FOR_EXPR(NODE)		TREE_OPERAND (FOR_STMT_CHECK2 (NODE), 2)
 #define FOR_BODY(NODE)		TREE_OPERAND (FOR_STMT_CHECK2 (NODE), 3)
 
-/* Some cilk #defines */
-#define CILK_FOR_VAR(NODE)      TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 5)
-#define CILK_FOR_INIT(NODE)     TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 0)
-#define CILK_FOR_GRAIN(NODE)    TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 6)
-
-/* Here are the pragma simd specific files used by the parser and vectorizer 
-   available in pragma_simd.c.  */
+/* Cilk Plus supporting functions in pragma_simd.c.  */
 
+extern void pragma_simd_verify_clauses (int);
 extern struct pragma_simd_values *psv_find_node (int psv_index);
 extern int psv_head_insert (struct pragma_simd_values local_simd_values);
 extern bool pragma_simd_acceptable_vlength_p (int ps_index, 
@@ -6651,6 +6656,10 @@  extern void set_OK_for_certain_clause (enum pragma_simd_kind clause_type,
 				       int pragma_simd_index);
 extern HOST_WIDE_INT find_linear_step_size (int pragma_simd_index, tree var);
 
+#define CILK_FOR_VAR(NODE)      TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 5)
+#define CILK_FOR_INIT(NODE)     TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 0)
+#define CILK_FOR_GRAIN(NODE)    TREE_OPERAND (CILK_FOR_STMT_CHECK (NODE), 6)
+
 tree build_call_list (tree return_type, tree fn, tree arglist);
 bool is_elem_fn (tree);
 void elem_fn_create_fn (tree);