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

["Iyer, Balaji V" <balaji dot v dot iyer at intel dot com>]

Hi Jakub,
	Attached, please find a fixed patch. I have fixed all the issues you have mentioned below. I have also answered your questions below. Is this OK for trunk/branch?

Here are the ChangeLog entries:
gcc/ChangeLog
2013-12-17  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * omp-low.c (simd_clone_clauses_extract): Replaced the string
        "cilk simd elemental" with "cilk simd function."

gcc/c-family/ChangeLog
2013-12-17  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * c-common.c (c_common_attribute_table): Added "cilk simd function"
        attribute.
        * c-pragma.h (enum pragma_cilk_clause): Remove.
        (enum pragma_omp_clause):  Added the following fields:
        PRAGMA_CILK_CLAUSE_NOMASK, PRAGMA_CILK_CLAUSE_MASK,
        PRAGMA_CILK_CLAUSE_VECTORLENGTH, PRAGMA_CILK_CLAUSE_NONE,
        PRAGMA_CILK_CLAUSE_LINEAR, PRAGMA_CILK_CLAUSE_PRIVATE,
        PRAGMA_CILK_CLAUSE_FIRSTPRIVATE, PRAGMA_CILK_CLAUSE_LASTPRIVATE,
        PRAGMA_CILK_CLAUSE_UNIFORM.

gcc/c/ChangeLog
2013-12-17  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.  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_vector_attrs): New function.
        (c_finish_cilk_simd_fn_tokens): Likewise.
        (is_cilkplus_vector_p): Likewise.
        (c_parser_omp_clause_name): Added checking for "vectorlength,"
        "nomask," and "mask" strings in clause name.
        (c_parser_omp_all_clauses): Added 3 new case statements:
        PRAGMA_CILK_CLAUSE_VECTORLENGTH, PRAGMA_CILK_CLAUSE_MASK and
        PRAGMA_CILK_CLAUSE_NOMASK.
        (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_vector_attrs.  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.
        (c_parser_omp_clause_linear): Added a new bool parm. is_cilk_simd_fn.
        Added a check when step is a parameter and flag it as error.
        (CILK_SIMD_FN_CLAUSE_MASK): New #define.
        (c_parser_cilk_clause_name): Changed pragma_cilk_clause to
        pragma_omp_clause.

gcc/cp/ChangeLog
2013-12-17  Balaji V. Iyer  <balaji.v.iyer@intel.com>

        * parser.c (cp_parser_cilk_simd_clause_name): Changed cilk_clause_name
        to omp_clause_name.

gcc/testsuite/ChangeLog 
2013-12-17  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.
        * c-c++-common/cilk-plus/SE/ef_error3.c: Likewise.
        * gcc.dg/cilk-plus/cilk-plus.exp: Added calls for the above tests.


Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: Jakub Jelinek [mailto:jakub@redhat.com]
> Sent: Tuesday, December 17, 2013 1:25 PM
> To: Iyer, Balaji V
> Cc: Joseph S. Myers; Aldy Hernandez (aldyh@redhat.com); 'gcc-
> patches@gcc.gnu.org'
> Subject: Re: [PING]: [GOMP4] [PATCH] SIMD-Enabled Functions (formerly
> Elemental functions) for C
> 
> Hi!
> 
> On Tue, Dec 17, 2013 at 05:23:43PM +0000, Iyer, Balaji V wrote:
> > +/* Returns name of the next clause in Cilk Plus SIMD-enabled function's
> > +   attribute.
> > +   If the clause is not recognized PRAGMA_OMP_CLAUSE_NONE is
> returned and
> > +   the token is not consumed.  Otherwise appropriate
> pragma_omp_clause is
> > +   returned and the token is consumed.  */
> > +
> > +static pragma_omp_clause
> > +c_parser_cilk_simd_fn_clause_name (c_parser *parser) {
> > +  pragma_omp_clause result = PRAGMA_OMP_CLAUSE_NONE;
> > +
> > +  if (c_parser_next_token_is_not (parser, CPP_NAME))
> > +    return result;
> > +
> > +  const char *p = IDENTIFIER_POINTER (c_parser_peek_token
> > + (parser)->value);  if (!strcmp (p, "vectorlength"))
> > +    result = PRAGMA_CILK_CLAUSE_VECTORLENGTH;  else if (!strcmp (p,
> > + "uniform"))
> > +    result = PRAGMA_CILK_CLAUSE_UNIFORM;  else if (!strcmp (p,
> > + "linear"))
> > +    result = PRAGMA_CILK_CLAUSE_LINEAR;  else if (!strcmp (p,
> > + "mask"))
> > +    result = PRAGMA_CILK_CLAUSE_MASK;  else if (!strcmp (p,
> > + "nomask"))
> > +    result = PRAGMA_CILK_CLAUSE_UNMASK;
> > +
> > +  if (result != PRAGMA_OMP_CLAUSE_NONE)
> > +    c_parser_consume_token (parser);
> > +  return result;
> > +}
> 
> No, this isn't what I meant.  I meant that you add the 3 new clause names to
> c_parser_omp_clause_name (and use PRAGMA_CILK_* for those).
> 

OK. Fixed as you suggested.

> > +      if (token->type == CPP_NAME
> > +	  && TREE_CODE (token->value) == IDENTIFIER_NODE)
> > +	if (!strcmp (IDENTIFIER_POINTER (token->value), "vectorlength"))
> > +	  {
> > +	    if (!c_parser_cilk_clause_vectorlength (parser, NULL, true))
> > +	      {
> > +		c_parser_skip_until_found (parser, CPP_CLOSE_PAREN,
> NULL);
> > +		return;
> > +	      }
> > +	    else
> > +	      continue;
> > +	  }
> 
> Why do you need this at all?  I'd expect you just remove this whole if and the
> c_parser_cilk_clause_vectorlength function, and instead just parse
> vectorlength normally when you see
> PRAGMA_CILK_CLAUSE_VECTORLENGTH.
> 

OK. Fixed. I called c_parser_cilk_clause_vectorlength in the c_parser_omp_all_clauses.

> > +	  sorry ("using parameters for %<linear%> step is not supported "
> > +		 "in this release");
> 
> ... is not supported yet".
> 

Fixed.

> > -      c_kind = c_parser_omp_clause_name (parser);
> > +
> > +      if (mask == CILK_SIMD_FN_CLAUSE_MASK)
> > +	c_kind = c_parser_cilk_simd_fn_clause_name (parser);
> > +      else
> > +	c_kind = c_parser_omp_clause_name (parser);
> 
> Please revert this.
> 

Done.

> > @@ -10933,7 +11088,8 @@
> >  	  c_name = "aligned";
> >  	  break;
> >  	case PRAGMA_OMP_CLAUSE_LINEAR:
> > -	  clauses = c_parser_omp_clause_linear (parser, clauses);
> > +	  clauses = c_parser_omp_clause_linear
> > +	    (parser, clauses, mask == CILK_SIMD_FN_CLAUSE_MASK);
> 
> Please test for some particular bit in the mask, not == on the masks.
> 

Done. Tested for vectorlength as you suggested in a previous patch.

> 	Jakub

Attachment: diff8.txt
Description: diff8.txt