FW: [GOMP4][PATCH] SIMD-enabled functions (formerly Elemental functions) for C++
Iyer, Balaji V
balaji.v.iyer@intel.com
Thu Jan 23 05:24:00 GMT 2014
Hi Jakub,
Did you get a chance to look at this? Is it OK to install to trunk?
Thanks,
Balaji V. Iyer.
> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Iyer, Balaji V
> Sent: Sunday, January 19, 2014 10:37 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,
> I have fixed all the issues you have mentioned below. I am also
> attaching a fixed patch along with ChangeLog entries. I have also rebased the
> patch to the trunk revision r206786.
>
> Is this Ok to install?
>
> Thanks,
>
> Balaji V. Iyer.
>
> > -----Original Message-----
> > From: Jakub Jelinek [mailto:jakub@redhat.com]
> > Sent: Friday, January 17, 2014 12:46 PM
> > 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 Thu, Dec 19, 2013 at 06:12:29PM +0000, Iyer, Balaji V wrote:
> > > 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"
> >
> > The comma should have been after " .
> >
>
> Fixed.
>
> > > + /* 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_next_tokens_can_be_attribute_p (parser))
> >
> > As you are just calling cp_parser_gnu_attributes_opt here and not
> > ..._std_..., I'd say the above should be
> > cp_next_tokens_can_be_gnu_attribute_p
> > rather than cp_next_tokens_can_be_attribute_p. I think [[...]]
> > attributes at this position are ignored, so no need to handle them,
> > not sure about whether we allow e.g. combination of GNU and std
> attributes or vice versa.
> >
>
> Fixed.
>
> > > + {
> > > + cp_parser_parse_tentatively (parser);
> > > + tree attr = cp_parser_gnu_attributes_opt (parser);
> > > + if (cp_lexer_next_token_is_not (parser->lexer,
> > > + CPP_SEMICOLON)
> > > + && cp_lexer_next_token_is_not (parser->lexer,
> > > + CPP_OPEN_BRACE))
> > > + cp_parser_abort_tentative_parse (parser);
> > > + else if (!cp_parser_parse_definitely (parser))
> > > + ;
> > > + else
> > > + attrs = chainon (attr, attrs);
> > > + }
> > > late_return = (cp_parser_late_return_type_opt
> > > (parser, declarator,
> > > memfn ? cv_quals : -1));
> >
> > > @@ -17842,6 +17868,10 @@ cp_parser_late_return_type_opt
> (cp_parser*
> > parser, cp_declarator *declarator,
> > > type = cp_parser_trailing_type_id (parser);
> > > }
> > >
> > > + if (cilk_simd_fn_vector_p)
> > > + declarator->std_attributes
> > > + = cp_parser_late_parsing_cilk_simd_fn_info (parser,
> > > + declarator->std_attributes);
> >
> > Please make sure declarator is aligned below parser.
> >
>
> Fixed.
>
> > > + token->type = CPP_PRAGMA_EOL;
> > > + parser->lexer->next_token = token; cp_lexer_consume_token
> > > + (parser->lexer);
> > > +
> > > + struct cp_token_cache *cp =
> > > + cp_token_cache_new (v_token, cp_lexer_peek_token
> > > + (parser->lexer));
> >
> > The = should already go on the next line.
> >
>
> Fixed.
>
> > > +/* Handles the delayed parsing of the Cilk Plus SIMD-enabled function.
> > > + This function is modelled similar to the late parsing of omp declare
> > > + simd. */
> > > +
> > > +static tree
> > > +cp_parser_late_parsing_cilk_simd_fn_info (cp_parser *parser, tree
> > > +attrs) {
> > > + struct cp_token_cache *ce;
> > > + cp_omp_declare_simd_data *info = parser->cilk_simd_fn_info;
> > > + int ii = 0;
> > > +
> > > + if (parser->omp_declare_simd != NULL)
> > > + {
> > > + error ("%<#pragma omp declare simd%> cannot be used in the
> > > + same
> > function"
> > > + " marked as a Cilk Plus SIMD-enabled function");
> > > + parser->cilk_simd_fn_info = NULL;
> >
> > This will leak parser->cilk_simd_fn_info memory. Please XDELETE it first.
> >
>
> Fixed
>
> > > + return attrs;
> > > + }
> > > + if (!info->error_seen && info->fndecl_seen)
> > > + {
> > > + error ("vector attribute not immediately followed by a single
> function"
> > > + " declaration or definition");
> > > + info->error_seen = true;
> > > + }
> > > + if (info->error_seen)
> > > + return attrs;
> > > +
> > > + /* Vector attributes are converted to #pragma omp declare simd
> > > + values
> > and
> > > + so we need them enabled. */
> > > + flag_openmp = 1;
> >
> > The C FE doesn't do this. I thought all the omp-low.c spots are now
> > guarded by flag_openmp || flag_enable_cilkplus etc. conditions.
>
> Fixed. The above "orring" was omitted in decl.c and pt.c . I have put them in.
>
> >
> > > + c = build_tree_list (get_identifier ("cilk simd function"),
> > > + cl);
> >
> > Please use NULL_TREE instead of cl here and please fix up the C FE as
> > well, the "cilk simd function" attribute should be just an attribute
> > without value, serving only as a boolean, if present, decl with "omp
> > declare simd" attribute is Cilk+, otherwise OpenMP.
> >
> > > + TREE_CHAIN (c) = attrs;
> > > + if (processing_template_decl)
> > > + ATTR_IS_DEPENDENT (c) = 1;
> >
>
> Fixed.
>
> > But, as a boolean attribute it doesn't and shouldn't be
> > ATTR_IS_DEPENDENT, there is nothing to adjust in it.
> > > --- a/gcc/cp/pt.c
> > > +++ b/gcc/cp/pt.c
> > > @@ -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))))
> > > && TREE_VALUE (t))
> > > {
> > > tree clauses = TREE_VALUE (TREE_VALUE (t));
> >
> > And this change is unnecessary after the above suggested change.
> >
>
> Fixed.
>
> > Jakub
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: diff.txt
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20140123/1b15b7f3/attachment.txt>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ChangeLog.elem_fn
Type: application/octet-stream
Size: 1952 bytes
Desc: ChangeLog.elem_fn
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20140123/1b15b7f3/attachment.obj>
More information about the Gcc-patches
mailing list