[PING][GOMP4][PATCH] SIMD-enabled functions (formerly Elemental functions) for C++

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

Hello Everyone,
	Did anyone get a chance to look into this?

Thanks,

Balaji V. Iyer.

> -----Original Message-----
> From: Iyer, Balaji V
> Sent: Monday, December 23, 2013 11:51 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Jakub Jelinek
> Subject: [PING][GOMP4][PATCH] SIMD-enabled functions (formerly
> Elemental functions) for C++
> 
> Ping!
> 
> -Balaji V. Iyer.
> 
> > -----Original Message-----
> > From: Iyer, Balaji V
> > Sent: Thursday, December 19, 2013 1:12 PM
> > To: Jakub Jelinek
> > Cc: 'Aldy Hernandez (aldyh@redhat.com)'; 'gcc-patches@gcc.gnu.org'
> > Subject: RE: [GOMP4][PATCH] SIMD-enabled functions (formerly
> Elemental
> > functions) for C++
> >
> > Hi Jakub,
> > 	Attached, please find a fixed patch. I have answered your questions
> > below. Is this OK for trunk?
> >
> > Here are the ChangeLog entries:
> > Gcc/cp/ChangeLog
> > 2013-12-19  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >
> >         * parser.c (cp_parser_direct_declarator): When Cilk Plus is enabled
> >         see if there is an attribute after function decl.  If so, then
> >         parse them now.
> >         (cp_parser_late_return_type_opt): Handle parsing of Cilk Plus SIMD
> >         enabled function late parsing.
> >         (cp_parser_gnu_attribute_list): Parse all the tokens for the vector
> >         attribute for a SIMD-enabled function.
> >         (cp_parser_omp_all_clauses): Skip parsing to the end of pragma when
> >         the function is used by SIMD-enabled function (indicated by NULL
> >         pragma token).   Added 3 new clauses: PRAGMA_CILK_CLAUSE_MASK,
> >         PRAGMA_CILK_CLAUSE_NOMASK and
> > PRAGMA_CILK_CLAUSE_VECTORLENGTH
> >         (cp_parser_cilk_simd_vectorlength): Modified this function to handle
> >         vectorlength clause in SIMD-enabled function and #pragma SIMD's
> >         vectorlength clause.  Added a new bool parameter to differentiate
> >         between the two.
> >         (cp_parser_cilk_simd_fn_vector_attrs): New function.
> >         (is_cilkplus_vector_p): Likewise.
> >         (cp_parser_late_parsing_elem_fn_info): Likewise.
> >         (cp_parser_omp_clause_name): Added a check for "mask," "nomask"
> >         and "vectorlength" clauses when Cilk Plus is enabled.
> >         (cp_parser_omp_clause_linear): Added a new parameter of type bool
> >         and emit a sorry message when step size is a parameter.
> >         * parser.h (cp_parser::cilk_simd_fn_info): New field.
> >
> > Testsuite/ChangeLog
> > 2013-12-19  Balaji V. Iyer  <balaji.v.iyer@intel.com>
> >
> >         * g++.dg/cilk-plus/cilk-plus.exp: Called the C/C++ common tests for
> >         SIMD enabled function.
> >         * g++.dg/cilk-plus/ef_test.C: New test.
> >         * c-c++-common/cilk-plus/vlength_errors.c: Added new dg-error tags
> >         to differenciate C error messages from C++ ones.
> >
> > Thanks,
> >
> > Balaji V. Iyer.
> >
> > > -----Original Message-----
> > > From: Jakub Jelinek [mailto:jakub@redhat.com]
> > > Sent: Thursday, December 19, 2013 2:23 AM
> > > To: Iyer, Balaji V
> > > Cc: 'Aldy Hernandez (aldyh@redhat.com)'; 'gcc-patches@gcc.gnu.org'
> > > Subject: Re: [GOMP4][PATCH] SIMD-enabled functions (formerly
> > Elemental
> > > functions) for C++
> > >
> > > On Wed, Dec 18, 2013 at 11:36:04PM +0000, Iyer, Balaji V wrote:
> > > > --- a/gcc/cp/decl2.c
> > > > +++ b/gcc/cp/decl2.c
> > > > @@ -1124,6 +1124,10 @@ is_late_template_attribute (tree attr, tree
> > decl)
> > > >        && is_attribute_p ("omp declare simd", name))
> > > >      return true;
> > > >
> > > > +  /* Ditto as above for Cilk Plus SIMD-enabled function attributes.
> > > > + */  if (flag_enable_cilkplus && is_attribute_p ("cilk simd
> > > > + function",
> > > name))
> > > > +    return true;
> > >
> > > Why?  It doesn't have any argument, why it should be processed late?
> > >
> >
> > Fixed.
> >
> > > > @@ -17097,6 +17102,14 @@ cp_parser_direct_declarator (cp_parser*
> > > > parser,
> > > >
> > > >  		  attrs = cp_parser_std_attribute_spec_seq (parser);
> > > >
> > > > +		  /* In here, we handle cases where attribute is used after
> > > > +		     the function declaration.  For example:
> > > > +		     void func (int x) __attribute__((vector(..)));  */
> > > > +		  if (flag_enable_cilkplus
> > > > +		      && cp_lexer_next_token_is_keyword (parser->lexer,
> > > > +							 RID_ATTRIBUTE))
> > > > +		    attrs = chainon (cp_parser_gnu_attributes_opt (parser),
> > > > +				     attrs);
> > > >  		  late_return = (cp_parser_late_return_type_opt
> > > >  				 (parser, declarator,
> > > >  				  memfn ? cv_quals : -1));
> > >
> > > Doesn't this change the grammar (for all attributes, not just Cilk+
> > > specific
> > > ones) just based on whether -fcilkplus has been specified or not?
> > >
> >
> > OK. Fixed this by making it parse tentatively (sort of similar to how
> > you parse attributes after labels (line #9584))
> >
> > > > @@ -17820,10 +17833,14 @@ cp_parser_late_return_type_opt
> > > (cp_parser* parser, cp_declarator *declarator,
> > > >  			 && declarator
> > > >  			 && declarator->kind == cdk_id);
> > > >
> > > > +  bool cilk_simd_fn_vector_p = (parser->cilk_simd_fn_info
> > > > +			   && declarator
> > > > +			   && declarator->kind == cdk_id);
> > >
> > > Formatting looks wrong, put = on the next line and align && right
> > > below parser.
> >
> > Fixed.
> >
> > > > +
> > > > +cp_omp_declare_simd_data info;
> > >
> > > Global var?  Why?  Isn't heap or GC allocation better?
> >
> > Fixed. Replaced it with XNEW and XDELETE combinations instead of
> > setting the address of a global value.
> >
> > > > +  /* The vectorlength clause in #pragma simdbehaves exactly like
> > > > + OpenMP's
> > >
> > > Missing space after simd
> > >
> >
> > Fixed.
> >
> > > > @@ -8602,9 +8602,12 @@ apply_late_template_attributes (tree
> > > > *decl_p,
> > > tree attributes, int attr_flags,
> > > >  	    {
> > > >  	      *p = TREE_CHAIN (t);
> > > >  	      TREE_CHAIN (t) = NULL_TREE;
> > > > -	      if (flag_openmp
> > > > -		  && is_attribute_p ("omp declare simd",
> > > > -				     get_attribute_name (t))
> > > > +	      if (((flag_openmp
> > > > +		    && is_attribute_p ("omp declare simd",
> > > > +				       get_attribute_name (t)))
> > > > +		   || (flag_enable_cilkplus
> > > > +		       && is_attribute_p ("cilk simd function",
> > > > +					  get_attribute_name (t))))
> > >
> > > Again, why this?
> > >
> >
> > SIMD enabled function is marked with omp declare simd and cilk simd
> > function attributes. So I should handle them both...
> >
> > > 	Jakub

Attachment: diff2.txt
Description: diff2.txt