diff mbox

[PING] : [GOMP4] [PATCH] SIMD-Enabled Functions (formerly Elemental functions) for C

Message ID BF230D13CA30DD48930C31D4099330003A4AFEF8@FMSMSX101.amr.corp.intel.com
State New
Headers show

Commit Message

Iyer, Balaji V Dec. 6, 2013, 8:26 p.m. UTC 
Hi Aldy  & Jakub,
	I have made the fixes you have mentioned and have answered your questions below. Attached is the fixed patch and here are the ChangeLog entries.

Gcc/ChangeLog
2013-12-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * omp-low.c (expand_simd_clones): Added a new parameter called "type."
        (ipa_omp_simd_clone): Added a call to expand_simd_clones when Cilk Plus
        is enabled.
        (simd_clone_clauses_extract): Replaced the string "cilk simd elemental"
        with "cilk simd function."

Gcc/c-family/Changelog
2013-12-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-common.c (c_common_attribute_table): Added "cilk simd function"
        attribute.

Gcc/c/ChangeLog
2013-12-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-parser.c (struct c_parser::cilk_simd_fn_tokens): Added new field.
        (c_parser_declaration_or_fndef): Added a check if cilk_simd_fn_tokens
        field in parser is not empty.  If not-empty, call the function
        c_parser_finish_omp_declare_simd.
        (c_parser_cilk_clause_vectorlength): Modified function to be shared
        between SIMD-enabled functions and #pragma simd.  Changed return-type
        to bool and added new parameter.
        (c_parser_cilk_all_clauses): Modified the usage of the function
        c_parser_cilk_clause_vectorlength as mentioned above.
        (c_parser_cilk_simd_fn_expr_list): Likewise.
        (c_finish_cilk_simd_fn_tokens): Likewise.
        (c_parser_attributes): Added a cilk_simd_fn_tokens parameter.  Added a
        check for vector attribute and if so call the function
        c_parser_cilk_simd_fn_expr_list.  Also, when Cilk plus is enabled,
        called the function c_finish_cilk_simd_fn_tokens.
        (c_finish_omp_declare_simd): Added a check if cilk_simd_fn_tokens in
        parser field is non-empty.  If so, parse them as you would parse
        the omp declare simd pragma.

Gcc/testsuite/ChangeLog
2013-12-06  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-c++-common/cilk-plus/SE/ef_test.c: New test.
        * c-c++-common/cilk-plus/SE/ef_test2.c: Likewise.
        * c-c++-common/cilk-plus/SE/vlength_errors.c: Likewise.
        * c-c++-common/cilk-plus/SE/ef_error.c: Likewise.
        * c-c++-common/cilk-plus/SE/ef_error2.c: Likewise.
        * gcc.dg/cilk-plus/cilk-plus.exp: Added calls for the above tests.

Jakub, Is it Ok for branch?

Thanks,

Balaji V. Iyer.



> -----Original Message-----
> From: Aldy Hernandez [mailto:aldyh@redhat.com]
> Sent: Thursday, December 5, 2013 3:20 PM
> To: Iyer, Balaji V
> Cc: 'Jakub Jelinek'; 'gcc-patches@gcc.gnu.org'
> Subject: Re: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly
> Elemental functions) for C
> 
> On 11/30/13 20:38, Iyer, Balaji V wrote:
> > Hello Aldy,
> > 	Some of the middle end changes I made in the previous patch was
> not flying for the C++. Here is a fixed patch where the middle-end changes
> will work for both C and C++.
> > 	With this email, I am attaching the patch for C along with the middle
> end changes. Is this Ok for the branch?
> 
> Jakub and company ultimately have to approve your patch, but here are a
> few things.
> 
> > +
> > +  /* Cilk Plus specific parser/lexer information.  */
> > +
> > +  /* Buffer to hold all the tokens from parsing the vector attribute for the
> > +     SIMD Enabled functions (formerly known as elemental functions).
> > + */  vec <c_token, va_gc> *elem_fn_tokens;
> 
> If the name "elementals" is being phased out, then perhaps you need to
> come up with another name here.  Perhaps "cilk_simd_clone_tokens" or
> something that looks similar to the OpenMP variant
> "cilk_declare_simd_tokens" (akin to omp_declare_simd_clauses) in the rest
> of the patch.
> 

Fixed. I called it "cilk_simd_fn" instead of "elem_fn"

> Also, "Enabled" should not be capitalized.
> 

Fixed.


> > +/* Parses the vectorlength vector attribute for the SIMD Enabled
> functions
> > +   in Cilk Plus.
> > +   Syntax:
> > +   vectorlength (<integer constant expression>)  */
> > +
> > +static bool
> > +c_parser_elem_fn_vectorlength (c_parser *parser)
> 
> Similarly here.  Let's get rid of *elem* nomenclature throughout.
> Perhaps c_parser_cilk_declare_simd_vectorlength and similarly throughout
> the other parsing routines in the patch.  This will make it clearer that
> *cilk_declare_simd* is related to OpenMP's declare simd.
> 

Fixed.

> > +  if (TREE_CODE (value) != INTEGER_CST)
> > +    {
> > +      error_at (token->location, "vectorlength must be a constant integer");
> > +      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
> > +      return false;
> > +    }
> 
> I thought integer constant expressions were allowed here.  Shouldn't things
> like "sizeof (int)" be allowed?  See what is done in
> c_parser_cilk_clause_vectorlength() which handles constant expressions.
>   Also, you will need a corresponding test.
> 

Yes it is allowed. Fixed and added a test.

> For that matter... can't we combine the above function with
> c_parser_cilk_clause_vectorlength().  It doesn't make much sense having
> two functions parsing the exact same clause.  Perhaps add a flag to it and
> have it work in two modes: one mode to create OMP_CLAUSE_SAFELEN and
> one mode to fill the token vector.
> 

OK. Combined both the functions.

> > +              if (!c_parser_elem_fn_vectorlength (parser))
> > +                {
> > +                  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
> > +                  /* NO reason to keep any of these tokens if the
> > +                     vectorlength is messed up.  */
> > +		  vec_free (parser->elem_fn_tokens);
> > +                  return;
> 
> It may be cleaner to make the caller free the vector.

Well, the caller doesn't know if an error has occurred. I suppose I could do something like check for seen_error (), but this sounds a bit more clearer (to me altleast)

> 
> > +          /* linear and uniform are the same between SIMD
> > +             enabled functions and #pragma omp declare simd.  */
> 
> Capitalize first word.
> 

Fixed.

> > +      /* Do not push the last ')'  */
> > +      if (!(token->type == CPP_CLOSE_PAREN && paren_scope == 0))
> > +	vec_safe_push (parser->elem_fn_tokens, *token);
> 
> You're documenting the obvious.  Perhaps a better comment would be to
> explain why you do not push the last ')'.
> 

Fixed.

> > +/* Parses the vector attribute of SIMD enabled functions in Cilk Plus.
> > +   Syntax:
> > +   vector
> > +   vector (<vector attributes>).  */
> > +
> > +static void
> > +c_parser_elem_fn_expr_list (c_parser *parser, c_token vec_token)
> 
> Please document the second argument.
> 

Fixed.

> 
> > +  /* 2 EOF_token is added as a safety-net since the normal C front-end has
> > +     two token look-ahead.  */
> 
> "Two EOF tokens are added..."
> 

Fixed.

> >  static void
> > -expand_simd_clones (struct cgraph_node *node)
> > +expand_simd_clones (struct cgraph_node *node, const char *type)
> 
> Document second argument, but perhaps we don't even need the second
> argument.  See below.
> 
> > @@ -12337,7 +12336,10 @@
> >  {
> >    struct cgraph_node *node;
> >    FOR_EACH_FUNCTION (node)
> > -    expand_simd_clones (node);
> > +    expand_simd_clones (node, "omp declare simd");  if
> > + (flag_enable_cilkplus)
> > +    FOR_EACH_FUNCTION (node)
> > +      expand_simd_clones (node, "cilk plus elemental");
> 
> First of all, it's a really bad idea to scan all the functions twice.
> You should have adapted expand_simd_clones() to do the work for both.
> 

OK.  I included this in the first loop itself so we won't have to scan the functions twice.

> But even so, I don't think you need to do this at all.  Aren't Cilk Plus
> elementals supposed to be tagged as "omp declare simd" as well?  In which
> case expand_simd_clones() will DTRT.  It should...and then
> simd_clone_clauses_extract() already has the smarts to differentiate
> between the both variants.
> 

Yes, the thing is, there is a big do-while loop in the function and that needs to be replaced if we have to check for SIMD-enabled function and #pragma omp declare simd. If we pass it as a type, then it just needs to check for the type string. 

> Both OpenMP and Cilk Plus simd clones should have the "omp declare simd"
> attribute, and then Cilk Plus clones should *also* have the "cilk plus
> elemental" attribute.
> 
> Speak of which, we should probably rename the "cilk plus elemental"
> attribute throughout to "cilk plus declare simd" as I described earlier, but this
> is not your fault.  Bonus points if you want to do it as part of this patch :).
> 

I called it "cilk simd function" (since the name is SIMD-enabled function and it should have Cilk or Cilk Plus on it somewhere and Cilk simd enabled function seemed a bit too long)


> > +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/EF/*.c]] " -g" " "
> > +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/EF/*.c]] " -O1" " "
> > +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/EF/*.c]] " -O2 -std=c99" " "
> > +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/EF/*.c]] " -O2 -ftree-vectorize" " "
> > +dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-
> plus/EF/*.c]] " -O3 -g" " "
> > +

I narrowed this to 5 with the help of Jakub a while back. But now, I have replaced it with 3, "-O3 -g"," -O3 -g -std=c99"

> 
> As I've mentioned before, I am really against running Cilk tests with all these
> variants.  It's time consuming for all developers and the chances of catching
> an error that is not caught with say... "-O0" are pretty close to 0.  Do you have
> any particular errors you think will be better checked by trying all these
> variants?  The time spent doesn't seem to be worth it.  If you have a
> particular test in mind, perhaps you should add that flag to the test itself.
> 

OK. Fixes as I mentioned above.


> For that matter, we should probably get rid of all the variant testing in the
> rest of Cilk Plus.
> 
I will send this out as a different patch later for all the others.

> Furthermore, rename "EF" to something more readable and less dependent
> on "elementals" which is deprecated.  Perhaps "simd-clones" or something
> similar?
> 

Renamed "EF" to "SE" (Simd Enabled function)

> Aldy

Comments

Aldy Hernandez Dec. 6, 2013, 11:16 p.m. UTC | #1 
[Jakub, see below]

>>> +              if (!c_parser_elem_fn_vectorlength (parser)) +
>>> { +                  c_parser_skip_until_found (parser,
>>> CPP_CLOSE_PAREN, NULL); +                  /* NO reason to keep
>>> any of these tokens if the +                     vectorlength is
>>> messed up.  */ +		  vec_free (parser->elem_fn_tokens); +
>>> return;
>>
>> It may be cleaner to make the caller free the vector.
>
> Well, the caller doesn't know if an error has occurred. I suppose I
> could do something like check for seen_error (), but this sounds a
> bit more clearer (to me altleast)

Sorry, what I meant to say is that it may be cleaner if 
c_parser_elem_fn_vectorlength (or whatever it's called now) frees the 
vector upon error before returning.

>> First of all, it's a really bad idea to scan all the functions
>> twice. You should have adapted expand_simd_clones() to do the work
>> for both.
>>
>
> OK.  I included this in the first loop itself so we won't have to
> scan the functions twice.
>
>> But even so, I don't think you need to do this at all.  Aren't Cilk
>> Plus elementals supposed to be tagged as "omp declare simd" as
>> well?  In which case expand_simd_clones() will DTRT.  It
>> should...and then simd_clone_clauses_extract() already has the
>> smarts to differentiate between the both variants.
>>
>
> Yes, the thing is, there is a big do-while loop in the function and
> that needs to be replaced if we have to check for SIMD-enabled
> function and #pragma omp declare simd. If we pass it as a type, then
> it just needs to check for the type string.

But aren't both OpenMP and Cilk Plus simd clones marked as "omp declare 
simd"?  In which case you shouldn't have to do anything?  Are the Cilk 
Plus clones not being marked as "omp declare simd" in the front-ends?

> I narrowed this to 5 with the help of Jakub a while back. But now, I
> have replaced it with 3, "-O3 -g"," -O3 -g -std=c99"

I would prefer to get rid of -O3, since inlining may do interesting 
things to tests, and you'll have to use __attribute__((noinline)) to 
test some things.

Jakub, would you be ok with "-O0 -g" and "-O0 -std=c99"?  For that 
matter, I'd say pass no arguments at all (""), and let the test itself 
test something special if required.

>> For that matter, we should probably get rid of all the variant
>> testing in the rest of Cilk Plus.
>>
> I will send this out as a different patch later for all the others.

Thank you.  Do so after Jakub responds as to what he prefers.

> Renamed "EF" to "SE" (Simd Enabled function)

If you must, but I would still prefer something more meaningful (i.e., 
not an abbreviation).  I know there is precedence with the 
array-notation feature, but I dislike that too :).  Feel free to ignore 
me on this one.

> +/* Parses the vector attribute of SIMD enabled functions in Cilk Plus.
<insert space>
> +   The VEC_TOKEN is the "vector" token that is replaced with "simd" and
> +   pushed into the token list.
<insert space>
> +   Syntax:
> +   vector
> +   vector (<vector attributes>).  */

Also, s/The VEC_TOKEN/VEC_TOKEN/

> +static void
> +c_parser_cilk_simd_fn_expr_list (c_parser *parser, c_token vec_token)

This is a parsing routine for the vector attribute, let's call this 
"c_parser_cilk_simd_fn_vector" or "c_parser_cilk_simd_fn_vector_attrs". 
  The expr_list is confusing.

> +                  /* NO reason to keep any of these tokens if the
> +                     vectorlength is messed up.  */

Lower case "NO".

> +		  vec_free (parser->cilk_simd_fn_tokens);
> +                  // parser->cilk_simd_fn_tokens->release ();
> +                  return;

What's this commented out code?  If unnecessary, remove it.

> +  return;
> +}

Empty return at end of function.  Remove it.

> +  /* c_parser_attributes is called in several places, and so if these EOF

s/and so/so/

> +  /* Two EOF_token is added as a safety-net since the normal C front-end has
> +     two token look-ahead.  */

Shouldn't that be, "Two CPP_EOF tokens" ??

> +      error ("%<#pragma omp declare simd%> cannot be used in the same "
> +	     "function marked as a SIMD-enabled function");

Perhaps we should say "...as a Cilk Plus SIMD-enabled...", to make it 
absolutely clear that it is OpenMP and Cilk Plus that can't coexist.

>  /* Cilk Plus:
> -   vectorlength ( constant-expression ) */
> +   This function is shared by SIMD-enabled functions and #pragma simd.
> +   If IS_SIMD_FN is true then it is parsing a SIMD-enabled function and
> +   CLAUSES is unused.
> +   Syntax:
> +   vectorlength ( constant-expression )  */
>
> -static tree
> -c_parser_cilk_clause_vectorlength (c_parser *parser, tree clauses)
> +static bool
> +c_parser_cilk_clause_vectorlength (c_parser *parser, tree *clauses,
> +				   bool is_simd_fn)

Can you document what this function does?  Also, document the fact that 
when IS_SIMD_FN is true, we are merely caching the tokens, otherwise we 
are building the OMP_CLAUSE_SAFELEN.

>   /* if expr is error_mark_node, then the returning function would have
>      flagged the error.  No need to flag them twice.  */

s/if expr/If expr/

Also "flag it twice", in keeping with the tense in the previous sentence.

>       if (is_simd_fn)
> 	{
> 	  c_token new_token;
> 	  /* If we are here then it has be a number.  */
> 	  new_token.type = CPP_NUMBER;
> 	  new_token.keyword = RID_MAX;

I thought we agreed integral constants were allowed.  Why would we be 
expecting only a number?  Also, it should have been "it has TO be a number".

Aldy
diff mbox

Patch

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 205717)
+++ gcc/c-family/c-common.c	(working copy)
@@ -771,6 +771,8 @@ 
 			      handle_returns_nonnull_attribute, false },
   { "omp declare simd",       0, -1, true,  false, false,
 			      handle_omp_declare_simd_attribute, false },
+  { "cilk simd function",     0, -1, true,  false, false,
+			      handle_omp_declare_simd_attribute, false },
   { "omp declare target",     0, 0, true, false, false,
 			      handle_omp_declare_target_attribute, false },
   { "bnd_variable_size",      0, 0, true,  false, false,
Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 205717)
+++ gcc/c/c-parser.c	(working copy)
@@ -208,6 +208,12 @@ 
   /* True if we are in a context where the Objective-C "Property attribute"
      keywords are valid.  */
   BOOL_BITFIELD objc_property_attr_context : 1;
+
+  /* Cilk Plus specific parser/lexer information.  */
+
+  /* Buffer to hold all the tokens from parsing the vector attribute for the
+     SIMD-enabled functions (formerly known as elemental functions).  */
+  vec <c_token, va_gc> *cilk_simd_fn_tokens;
 } c_parser;
 
 
@@ -1246,6 +1252,7 @@ 
 static void c_parser_cilk_simd (c_parser *);
 static bool c_parser_cilk_verify_simd (c_parser *, enum pragma_context);
 static tree c_parser_array_notation (location_t, c_parser *, tree, tree);
+static bool c_parser_cilk_clause_vectorlength (c_parser *, tree *, bool);
 
 /* Parse a translation unit (C90 6.7, C99 6.9).
 
@@ -1647,7 +1654,8 @@ 
 					C_DTR_NORMAL, &dummy);
       if (declarator == NULL)
 	{
-	  if (omp_declare_simd_clauses.exists ())
+	  if (omp_declare_simd_clauses.exists ()
+	      || !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
 	    c_finish_omp_declare_simd (parser, NULL_TREE, NULL_TREE,
 				       omp_declare_simd_clauses);
 	  c_parser_skip_to_end_of_block_or_statement (parser);
@@ -1734,7 +1742,8 @@ 
 				  chainon (postfix_attrs, all_prefix_attrs));
 		  if (!d)
 		    d = error_mark_node;
-		  if (omp_declare_simd_clauses.exists ())
+		  if (omp_declare_simd_clauses.exists ()
+		      || !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
 		    c_finish_omp_declare_simd (parser, d, NULL_TREE,
 					       omp_declare_simd_clauses);
 		}
@@ -1746,7 +1755,8 @@ 
 				  chainon (postfix_attrs, all_prefix_attrs));
 		  if (!d)
 		    d = error_mark_node;
-		  if (omp_declare_simd_clauses.exists ())
+		  if (omp_declare_simd_clauses.exists ()
+		      || !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
 		    c_finish_omp_declare_simd (parser, d, NULL_TREE,
 					       omp_declare_simd_clauses);
 		  start_init (d, asm_name, global_bindings_p ());
@@ -1774,7 +1784,8 @@ 
 	      tree d = start_decl (declarator, specs, false,
 				   chainon (postfix_attrs,
 					    all_prefix_attrs));
-	      if (omp_declare_simd_clauses.exists ())
+	      if (omp_declare_simd_clauses.exists ()
+		  || !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
 		{
 		  tree parms = NULL_TREE;
 		  if (d && TREE_CODE (d) == FUNCTION_DECL)
@@ -1902,7 +1913,8 @@ 
 	c_parser_declaration_or_fndef (parser, false, false, false,
 				       true, false, NULL, vNULL);
       store_parm_decls ();
-      if (omp_declare_simd_clauses.exists ())
+      if (omp_declare_simd_clauses.exists ()
+	  || !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
 	c_finish_omp_declare_simd (parser, current_function_decl, NULL_TREE,
 				   omp_declare_simd_clauses);
       DECL_STRUCT_FUNCTION (current_function_decl)->function_start_locus
@@ -3765,6 +3777,98 @@ 
   return attr_name;
 }
 
+/* Parses the vector attribute of SIMD enabled functions in Cilk Plus.
+   The VEC_TOKEN is the "vector" token that is replaced with "simd" and
+   pushed into the token list. 
+   Syntax:
+   vector
+   vector (<vector attributes>).  */
+
+static void
+c_parser_cilk_simd_fn_expr_list (c_parser *parser, c_token vec_token)
+{
+  gcc_assert (simple_cst_equal (vec_token.value,
+                                get_identifier ("vector")) == 1);
+  int paren_scope = 0;
+  /* Replace the vector keyword with SIMD.  */
+  vec_token.value = get_identifier ("simd");
+  vec_safe_push (parser->cilk_simd_fn_tokens, vec_token);
+  /* Consume the "vector" token.  */
+  c_parser_consume_token (parser);
+
+  if (c_parser_next_token_is (parser, CPP_OPEN_PAREN))
+    {
+      c_parser_consume_token (parser);
+      paren_scope++;
+    }
+  while (paren_scope > 0)
+    {
+      c_token *token = c_parser_peek_token (parser);
+      if (token->type == CPP_OPEN_PAREN)
+        paren_scope++;
+      else if (token->type == CPP_CLOSE_PAREN)
+        paren_scope--;
+      if (token->type == CPP_NAME
+          && TREE_CODE (token->value) == IDENTIFIER_NODE)
+        {
+          tree value = token->value;
+          if (simple_cst_equal (value, get_identifier ("mask")) == 1)
+            token->value = get_identifier ("inbranch");
+          else if (simple_cst_equal (value, get_identifier ("nomask")) == 1)
+            token->value = get_identifier ("notinbranch");
+          else if (simple_cst_equal (value,
+                                     get_identifier ("vectorlength")) == 1)
+            {
+              if (!c_parser_cilk_clause_vectorlength (parser, NULL, true))
+                {
+                  c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
+                  /* NO reason to keep any of these tokens if the
+                     vectorlength is messed up.  */
+		  vec_free (parser->cilk_simd_fn_tokens);
+                  // parser->cilk_simd_fn_tokens->release ();
+                  return;
+                }
+              else
+                continue;
+            }
+          /* Linear and uniform are the same between SIMD
+             enabled functions and #pragma omp declare simd.  */
+        }
+      /* Do not push the last ')' since we are not pushing the '('.  */
+      if (!(token->type == CPP_CLOSE_PAREN && paren_scope == 0))
+	vec_safe_push (parser->cilk_simd_fn_tokens, *token);
+      c_parser_consume_token (parser);
+    }
+  
+  /* Since we are converting an attribute to a pragma, we need to end the
+     attribute with PRAGMA_EOL.  */
+  c_token eol_token;
+  memset (&eol_token, 0, sizeof (eol_token));
+  eol_token.type = CPP_PRAGMA_EOL;
+  vec_safe_push (parser->cilk_simd_fn_tokens, eol_token);
+  return;
+}
+
+/* Add 2 CPP_EOF at the end of PARSER->ELEM_FN_TOKENS vector.  */
+
+static void
+c_finish_cilk_simd_fn_tokens (c_parser *parser)
+{
+  c_token last_token = parser->cilk_simd_fn_tokens->last ();
+
+  /* c_parser_attributes is called in several places, and so if these EOF
+     tokens are already inserted, then don't do them again.  */
+  if (last_token.type == CPP_EOF)
+    return;
+  
+  /* Two EOF_token is added as a safety-net since the normal C front-end has
+     two token look-ahead.  */
+  c_token eof_token;
+  eof_token.type = CPP_EOF;
+  vec_safe_push (parser->cilk_simd_fn_tokens, eof_token);
+  vec_safe_push (parser->cilk_simd_fn_tokens, eof_token);
+}
+
 /* Parse (possibly empty) attributes.  This is a GNU extension.
 
    attributes:
@@ -3829,6 +3933,13 @@ 
 	  attr_name = c_parser_attribute_any_word (parser);
 	  if (attr_name == NULL)
 	    break;
+	  if (flag_enable_cilkplus
+	      && simple_cst_equal (attr_name, get_identifier ("vector")) == 1)
+	    {
+	      c_token *v_token = c_parser_peek_token (parser);
+	      c_parser_cilk_simd_fn_expr_list (parser, *v_token);
+	      continue;
+	    }
 	  c_parser_consume_token (parser);
 	  if (c_parser_next_token_is_not (parser, CPP_OPEN_PAREN))
 	    {
@@ -3909,6 +4020,9 @@ 
 	}
       parser->lex_untranslated_string = false;
     }
+
+  if (flag_enable_cilkplus && !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
+    c_finish_cilk_simd_fn_tokens (parser);
   return attrs;
 }
 
@@ -12754,10 +12868,20 @@ 
 c_finish_omp_declare_simd (c_parser *parser, tree fndecl, tree parms,
 			   vec<c_token> clauses)
 {
+
+  if (flag_enable_cilkplus
+      && clauses.exists () && !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
+    {
+      error ("%<#pragma omp declare simd%> cannot be used in the same "
+	     "function marked as a SIMD-enabled function");
+      vec_free (parser->cilk_simd_fn_tokens);
+      return;
+    }
+  
   /* Normally first token is CPP_NAME "simd".  CPP_EOF there indicates
      error has been reported and CPP_PRAGMA that c_finish_omp_declare_simd
      has already processed the tokens.  */
-  if (clauses[0].type == CPP_EOF)
+  if (clauses.exists () && clauses[0].type == CPP_EOF)
     return;
   if (fndecl == NULL_TREE || TREE_CODE (fndecl) != FUNCTION_DECL)
     {
@@ -12766,7 +12890,7 @@ 
       clauses[0].type = CPP_EOF;
       return;
     }
-  if (clauses[0].type != CPP_NAME)
+  if (clauses.exists () && clauses[0].type != CPP_NAME)
     {
       error_at (DECL_SOURCE_LOCATION (fndecl),
 		"%<#pragma omp declare simd%> not immediately followed by "
@@ -12780,9 +12904,20 @@ 
 
   unsigned int tokens_avail = parser->tokens_avail;
   gcc_assert (parser->tokens == &parser->tokens_buf[0]);
-  parser->tokens = clauses.address ();
-  parser->tokens_avail = clauses.length ();
-
+  bool is_cilkplus_cilk_simd_fn = false;
+  
+  if (flag_enable_cilkplus && !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
+    {
+      parser->tokens = parser->cilk_simd_fn_tokens->address ();
+      parser->tokens_avail = vec_safe_length (parser->cilk_simd_fn_tokens);
+      is_cilkplus_cilk_simd_fn = true;
+    }
+  else
+    {
+      parser->tokens = clauses.address ();
+      parser->tokens_avail = clauses.length ();
+    }
+  
   /* c_parser_omp_declare_simd pushed 2 extra CPP_EOF tokens at the end.  */
   while (parser->tokens_avail > 3)
     {
@@ -12792,19 +12927,31 @@ 
       c_parser_consume_token (parser);
       parser->in_pragma = true;
 
-      tree c = c_parser_omp_all_clauses (parser, OMP_DECLARE_SIMD_CLAUSE_MASK,
-					 "#pragma omp declare simd");
+      tree c = NULL_TREE;
+      if (is_cilkplus_cilk_simd_fn) 
+	c = c_parser_omp_all_clauses (parser, OMP_DECLARE_SIMD_CLAUSE_MASK,
+				      "SIMD-enabled functions attribute");
+      else 
+	c = c_parser_omp_all_clauses (parser, OMP_DECLARE_SIMD_CLAUSE_MASK,
+				      "#pragma omp declare simd");
       c = c_omp_declare_simd_clauses_to_numbers (parms, c);
       if (c != NULL_TREE)
 	c = tree_cons (NULL_TREE, c, NULL_TREE);
-      c = build_tree_list (get_identifier ("omp declare simd"), c);
+      if (is_cilkplus_cilk_simd_fn)
+	c = build_tree_list (get_identifier ("cilk simd function"), c);
+      else
+	c = build_tree_list (get_identifier ("omp declare simd"), c);
       TREE_CHAIN (c) = DECL_ATTRIBUTES (fndecl);
       DECL_ATTRIBUTES (fndecl) = c;
     }
 
   parser->tokens = &parser->tokens_buf[0];
   parser->tokens_avail = tokens_avail;
-  clauses[0].type = CPP_PRAGMA;
+  if (clauses.exists ())
+    clauses[0].type = CPP_PRAGMA;
+
+  if (!vec_safe_is_empty (parser->cilk_simd_fn_tokens))
+    vec_free (parser->cilk_simd_fn_tokens);
 }
 
 
@@ -13400,39 +13547,91 @@ 
 }
 
 /* Cilk Plus:
-   vectorlength ( constant-expression ) */
+   This function is shared by SIMD-enabled functions and #pragma simd.  
+   If IS_SIMD_FN is true then it is parsing a SIMD-enabled function and 
+   CLAUSES is unused. 
+   Syntax:
+   vectorlength ( constant-expression )  */
 
-static tree
-c_parser_cilk_clause_vectorlength (c_parser *parser, tree clauses)
+static bool
+c_parser_cilk_clause_vectorlength (c_parser *parser, tree *clauses, 
+				   bool is_simd_fn)
 {
-  /* The vectorlength clause behaves exactly like OpenMP's safelen
-     clause.  Represent it in OpenMP terms.  */
-  check_no_duplicate_clause (clauses, OMP_CLAUSE_SAFELEN, "vectorlength");
+  c_token *token = c_parser_peek_token (parser);
+  if (is_simd_fn)
+    {
+      gcc_assert (simple_cst_equal (token->value,
+				    get_identifier ("vectorlength")) == 1);
+      token->value = get_identifier ("simdlen");
+      vec_safe_push (parser->cilk_simd_fn_tokens, *token);
+      c_parser_consume_token (parser);
+    }
+  else
+    /* The vectorlength clause behaves exactly like OpenMP's safelen
+       clause.  Represent it in OpenMP terms.  */
+    check_no_duplicate_clause (*clauses, OMP_CLAUSE_SAFELEN, "vectorlength");
 
+  token = c_parser_peek_token (parser);
   if (!c_parser_require (parser, CPP_OPEN_PAREN, "expected %<(%>"))
-    return clauses;
+    return false;
 
+  if (is_simd_fn)
+    vec_safe_push (parser->cilk_simd_fn_tokens, *token);
+
+  *token = *c_parser_peek_token (parser);
   location_t loc = c_parser_peek_token (parser)->location;
   tree expr = c_parser_expr_no_commas (parser, NULL).value;
   expr = c_fully_fold (expr, false, NULL);
+  bool error_found = false;
 
-  if (!TREE_TYPE (expr)
+  /* if expr is error_mark_node, then the returning function would have
+     flagged the error.  No need to flag them twice.  */
+  if (expr == error_mark_node)
+    error_found = true;
+  else if (!TREE_TYPE (expr)
       || !TREE_CONSTANT (expr)
       || !INTEGRAL_TYPE_P (TREE_TYPE (expr)))
-    error_at (loc, "vectorlength must be an integer constant");
+    {
+      error_at (loc, "vectorlength must be an integer constant");
+      error_found = true;
+    }
   else if (exact_log2 (TREE_INT_CST_LOW (expr)) == -1)
-    error_at (loc, "vectorlength must be a power of 2");
+    {
+      error_at (loc, "vectorlength must be a power of 2");
+      error_found = true;
+    }
   else
     {
-      tree u = build_omp_clause (loc, OMP_CLAUSE_SAFELEN);
-      OMP_CLAUSE_SAFELEN_EXPR (u) = expr;
-      OMP_CLAUSE_CHAIN (u) = clauses;
-      clauses = u;
+      if (is_simd_fn)
+	{
+	  c_token new_token;
+	  /* If we are here then it has be a number.  */
+	  new_token.type = CPP_NUMBER;
+	  new_token.keyword = RID_MAX;
+	  new_token.value = expr;
+	  vec_safe_push (parser->cilk_simd_fn_tokens, new_token);
+	}
+      else
+	{
+	  tree u = build_omp_clause (loc, OMP_CLAUSE_SAFELEN);
+	  OMP_CLAUSE_SAFELEN_EXPR (u) = expr;
+	  OMP_CLAUSE_CHAIN (u) = *clauses;
+	  *clauses = u;
+	}
     }
+  if (error_found && is_simd_fn)
+    {
+      c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, NULL);
+      return false;
+    }
 
-  c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>");
+  token = c_parser_peek_token (parser);
+  if (!c_parser_require (parser, CPP_CLOSE_PAREN, "expected %<)%>"))
+    return false;
 
-  return clauses;
+  if (is_simd_fn)
+    vec_safe_push (parser->cilk_simd_fn_tokens, *token);
+  return true;
 }
 
 /* Cilk Plus:
@@ -13571,7 +13770,7 @@ 
       switch (c_kind)
 	{
 	case PRAGMA_CILK_CLAUSE_VECTORLENGTH:
-	  clauses = c_parser_cilk_clause_vectorlength (parser, clauses);
+	  c_parser_cilk_clause_vectorlength (parser, &clauses, false);
 	  break;
 	case PRAGMA_CILK_CLAUSE_LINEAR:
 	  clauses = c_parser_cilk_clause_linear (parser, clauses);
Index: gcc/omp-low.c
===================================================================
--- gcc/omp-low.c	(revision 205717)
+++ gcc/omp-low.c	(working copy)
@@ -11300,7 +11300,7 @@ 
      declare simd".  */
   bool cilk_clone
     = (flag_enable_cilkplus
-       && lookup_attribute ("cilk plus elemental",
+       && lookup_attribute ("cilk simd function",
 			    DECL_ATTRIBUTES (node->decl)));
 
   /* Allocate one more than needed just in case this is an in-branch
@@ -12238,16 +12238,17 @@ 
 }
 
 /* If the function in NODE is tagged as an elemental SIMD function,
-   create the appropriate SIMD clones.  */
+   create the appropriate SIMD clones.  TYPE string indicates the type of 
+   attribute whether we need to look for.  To date the choices are 
+   Cilk Plus SIMD-enabled function or #pragma OMP declare simd function.  */
 
 static void
-expand_simd_clones (struct cgraph_node *node)
+expand_simd_clones (struct cgraph_node *node, const char *type)
 {
   if (lookup_attribute ("noclone", DECL_ATTRIBUTES (node->decl)))
     return;
 
-  tree attr = lookup_attribute ("omp declare simd",
-				DECL_ATTRIBUTES (node->decl));
+  tree attr = lookup_attribute (type, DECL_ATTRIBUTES (node->decl));
   if (!attr || targetm.simd_clone.compute_vecsize_and_simdlen == NULL)
     return;
   /* Ignore
@@ -12327,7 +12328,7 @@ 
 	    }
 	}
     }
-  while ((attr = lookup_attribute ("omp declare simd", TREE_CHAIN (attr))));
+  while ((attr = lookup_attribute (type, TREE_CHAIN (attr))));
 }
 
 /* Entry point for IPA simd clone creation pass.  */
@@ -12337,7 +12338,11 @@ 
 {
   struct cgraph_node *node;
   FOR_EACH_FUNCTION (node)
-    expand_simd_clones (node);
+    { 
+      expand_simd_clones (node, "omp declare simd");
+      if (flag_enable_cilkplus)
+	expand_simd_clones (node, "cilk simd function");
+    }
   return 0;
 }
 
Index: gcc/testsuite/gcc.dg/cilk-plus/cilk-plus.exp
===================================================================
--- gcc/testsuite/gcc.dg/cilk-plus/cilk-plus.exp	(revision 205717)
+++ gcc/testsuite/gcc.dg/cilk-plus/cilk-plus.exp	(working copy)
@@ -59,6 +59,10 @@ 
     dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/CK/*.c]] " -O3 -flto -g -fcilkplus" " "
 }
 
+dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/SE/*.c]] " -g" " "
+dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/SE/*.c]] " -O3 -std=c99" " "
+dg-runtest [lsort [glob -nocomplain $srcdir/c-c++-common/cilk-plus/SE/*.c]] " -O3 -g" " "
+
 dg-finish
 
 unset TEST_EXTRA_LIBS
Index: gcc/testsuite/c-c++-common/cilk-plus/SE/ef_error.c
===================================================================
--- gcc/testsuite/c-c++-common/cilk-plus/SE/ef_error.c	(revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/SE/ef_error.c	(revision 0)
@@ -0,0 +1,18 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus -fopenmp" } */
+
+#pragma omp declare simd linear(y:1) simdlen(4) 
+__attribute__((vector (linear (y:1), vectorlength(4))))
+int func (int x, int y) { /* { dg-error "cannot be used in the same function marked as a SIMD-enabled" } */ 
+  return (x+y);
+}
+__attribute__((vector (linear (y:1), private (x)))) /* { dg-error "is not valid for" } */
+int func2 (int x, int y) {
+  return (x+y);
+}
+
+
+int main (void)
+{
+  return (func (5,6));
+}
Index: gcc/testsuite/c-c++-common/cilk-plus/SE/vlength_errors.c
===================================================================
--- gcc/testsuite/c-c++-common/cilk-plus/SE/vlength_errors.c	(revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/SE/vlength_errors.c	(revision 0)
@@ -0,0 +1,56 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus -Wunknown-pragmas" } */
+
+#define Q 4
+
+int z = Q;
+
+__attribute__ ((vector (uniform(x), vectorlength (), linear (y:1) ))) /* { dg-error "expected expression" } */
+int func2 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), linear (y:1), vectorlength (4.5) ))) /* { dg-error "vectorlength must be an integer" } */
+int func3 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), linear (y:1), vectorlength (z) ))) /* { dg-error "vectorlength must be an integer" } */
+int func4 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), linear (y:1), vectorlength (Q) ))) /* This is OK!  */
+int func5 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), vectorlength (z), linear (y:1)))) /* { dg-error "vectorlength must be an integer" } */
+int func6 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), linear (y:1), vectorlength (sizeof (int)) ))) /* This is OK too!  */
+int func7 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+int main (void)
+{
+  int ii = 0, q = 5;
+  for (ii = 0; ii < 10; ii++)
+    q += func2 (z, ii);
+  return q;
+}
Index: gcc/testsuite/c-c++-common/cilk-plus/SE/ef_error2.c
===================================================================
--- gcc/testsuite/c-c++-common/cilk-plus/SE/ef_error2.c	(revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/SE/ef_error2.c	(revision 0)
@@ -0,0 +1,14 @@ 
+/* { dg-do compile { target { i?86-*-* x86_64-*-* } } } */
+/* { dg-options "-fcilkplus -Wall" } */
+
+__attribute__((vector (vectorlength(32)))) 
+//#pragma omp simd simdlen (32)
+int func2 (int x, int y)  /* { dg-warning "unsupported simdlen" } */
+{
+  return (x+y);
+}
+
+int main (void)
+{
+  return (func2 (5,6));
+}
Index: gcc/testsuite/c-c++-common/cilk-plus/SE/ef_test.c
===================================================================
--- gcc/testsuite/c-c++-common/cilk-plus/SE/ef_test.c	(revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/SE/ef_test.c	(revision 0)
@@ -0,0 +1,78 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus -Wunknown-pragmas" } */
+
+/* Tests the clauses in several combinations put in different locations.  */
+/* This is mostly a parser test.  */
+#define Q 4
+
+int z = Q;
+
+ __attribute__ ((vector (uniform(x), linear (y:1), vectorlength (4) )))
+int func (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+ __attribute__ ((vector (uniform(x), vectorlength (2), linear (y:1) )))
+int func2 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(y), linear (x), vectorlength (4) )))
+int func3 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), linear (y:1), mask)))
+int func4 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), linear (y:1), nomask)))
+int func5 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform(x), mask, linear (y:1)))) 
+int func6 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform (x), mask, linear (y:1)), vector))
+int func7 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector (uniform (x), mask, linear (y:1)), vector (uniform (y), mask)))
+int func8 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+__attribute__ ((vector, vector (uniform (y), mask)))
+int func9 (int x, int y)
+{
+  int zq = 5;
+  return x + (y*zq);
+}
+
+int main (int argc, char *argv[])
+{
+  int ii = 0, q = 5;
+  for (ii = 0; ii < 10; ii++)
+    q += func (argc, ii);
+  return q;
+}
Index: gcc/testsuite/c-c++-common/cilk-plus/SE/ef_test2.c
===================================================================
--- gcc/testsuite/c-c++-common/cilk-plus/SE/ef_test2.c	(revision 0)
+++ gcc/testsuite/c-c++-common/cilk-plus/SE/ef_test2.c	(revision 0)
@@ -0,0 +1,16 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fcilkplus" } */
+void func (int x, int y) __attribute__((vector(linear(x:1), uniform (y)),
+					vector));
+
+int q;
+int main (void)
+{
+  int ii = 0;
+  q = 5; 
+  for (ii = 0; ii < 100; ii++) 
+    func (ii, q);
+
+  return 0;
+}
+