| Message ID | 5165E691.8020907@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 10, 2013 at 3:24 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > Hi Tom. Hi folks. > > We've asked Balaji to rewrite the <#pragma simd> handling for cilkplus as we > currently do for OMP, etc, in init_pragma(). > > The cilkplus branch currently has something like: > > cpp_register_deferred_pragma (parse_in, "simd", "", > PRAGMA_SIMD_EMPTY, true, false); > cpp_register_deferred_pragma (parse_in, "simd", "assert", > PRAGMA_SIMD_ASSERT, true, false); > cpp_register_deferred_pragma (parse_in, "simd", "noassert", > PRAGMA_SIMD_NOASSERT, true, false); > cpp_register_deferred_pragma (parse_in, "simd", "vectorlength", > PRAGMA_SIMD_VECTORLENGTH, true, false); What about just registering simd as the pragma and then look for the right keyword after that? Like diagnostic is handled? Thanks, Andrew Pinski > > Notice that #pragma simd can be both a pragma name space, and also a lone > pragma with no arguments: > > #pragma simd assert > -or- > #pragma simd > > It seems like the code in libcpp's do_pragma(), specifically disallows this. > If we're looking at a possible pragma name space, the next expected token is > a CPP_NAME. > > Is there a way to handle this scenario with the current infrastructure? If > not, is something like the attached (untested) patch reasonable? > > Aldy On Wed, Apr 10, 2013 at 3:24 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > Hi Tom. Hi folks. > > We've asked Balaji to rewrite the <#pragma simd> handling for cilkplus as we > currently do for OMP, etc, in init_pragma(). > > The cilkplus branch currently has something like: > > cpp_register_deferred_pragma (parse_in, "simd", "", > PRAGMA_SIMD_EMPTY, true, false); > cpp_register_deferred_pragma (parse_in, "simd", "assert", > PRAGMA_SIMD_ASSERT, true, false); > cpp_register_deferred_pragma (parse_in, "simd", "noassert", > PRAGMA_SIMD_NOASSERT, true, false); > cpp_register_deferred_pragma (parse_in, "simd", "vectorlength", > PRAGMA_SIMD_VECTORLENGTH, true, false); > > Notice that #pragma simd can be both a pragma name space, and also a lone > pragma with no arguments: > > #pragma simd assert > -or- > #pragma simd > > It seems like the code in libcpp's do_pragma(), specifically disallows this. > If we're looking at a possible pragma name space, the next expected token is > a CPP_NAME. > > Is there a way to handle this scenario with the current infrastructure? If > not, is something like the attached (untested) patch reasonable? > > Aldy
On Wed, Apr 10, 2013 at 05:16:17PM -0700, Andrew Pinski wrote: > On Wed, Apr 10, 2013 at 3:24 PM, Aldy Hernandez <aldyh@redhat.com> wrote: > > Hi Tom. Hi folks. > > > > We've asked Balaji to rewrite the <#pragma simd> handling for cilkplus as we > > currently do for OMP, etc, in init_pragma(). > > > > The cilkplus branch currently has something like: > > > > cpp_register_deferred_pragma (parse_in, "simd", "", > > PRAGMA_SIMD_EMPTY, true, false); > > cpp_register_deferred_pragma (parse_in, "simd", "assert", > > PRAGMA_SIMD_ASSERT, true, false); > > cpp_register_deferred_pragma (parse_in, "simd", "noassert", > > PRAGMA_SIMD_NOASSERT, true, false); > > cpp_register_deferred_pragma (parse_in, "simd", "vectorlength", > > PRAGMA_SIMD_VECTORLENGTH, true, false); > > What about just registering simd as the pragma and then look for the > right keyword after that? Like diagnostic is handled? Yeah, the above is definitely wrong. Just if (flag_cilkplus) cpp_register_deferred_pragma (parse_in, NULL, "simd", PRAGMA_SIMD, true, false); and parse the clauses in c/c-parser.c and cp/parser.c, look at how OpenMP pragmas are parsed (those have the "omp" space, you just use NULL, otherwise it is not any different). Also check the standard whether cpp expansion is allowed or not and on what exactly. Like: #define S simd #define V vectorlength #pragma simd vectorlength(8) for (i = 0; i < 16; i++) ; vs. #pragma simd V(8) for (i = 0; i < 16; i++) ; vs. #pragma S V(8) for (i = 0; i < 16; i++) ; (that's the two last arguments to cpp_register_deferred_pragma). Jakub
On 04/11/13 00:32, Jakub Jelinek wrote: > On Wed, Apr 10, 2013 at 05:16:17PM -0700, Andrew Pinski wrote: >> On Wed, Apr 10, 2013 at 3:24 PM, Aldy Hernandez <aldyh@redhat.com> wrote: >>> Hi Tom. Hi folks. >>> >>> We've asked Balaji to rewrite the <#pragma simd> handling for cilkplus as we >>> currently do for OMP, etc, in init_pragma(). >>> >>> The cilkplus branch currently has something like: >>> >>> cpp_register_deferred_pragma (parse_in, "simd", "", >>> PRAGMA_SIMD_EMPTY, true, false); >>> cpp_register_deferred_pragma (parse_in, "simd", "assert", >>> PRAGMA_SIMD_ASSERT, true, false); >>> cpp_register_deferred_pragma (parse_in, "simd", "noassert", >>> PRAGMA_SIMD_NOASSERT, true, false); >>> cpp_register_deferred_pragma (parse_in, "simd", "vectorlength", >>> PRAGMA_SIMD_VECTORLENGTH, true, false); >> >> What about just registering simd as the pragma and then look for the >> right keyword after that? Like diagnostic is handled? > > Yeah, the above is definitely wrong. Just > if (flag_cilkplus) > cpp_register_deferred_pragma (parse_in, NULL, "simd", PRAGMA_SIMD, true, false); Well, the thing is that you can't just use NULL for "#pragma simd" as a pragma and then define "#pragma simd assert" to use "simd" as a pragma namespace. I had already tried that: if (flag_enable_cilk) { cpp_register_deferred_pragma (parse_in, NULL, "simd", PRAGMA_SIMD_EMPTY, true, false); cpp_register_deferred_pragma (parse_in, "simd", "assert", PRAGMA_SIMD_ASSERT, true, false); // etc } In which case, we ICE because we're registering simd as both a pragma and a pragma namespace: cpp_error (pfile, CPP_DL_ICE, "registering \"%s\" as both a pragma and a pragma namespace", NODE_NAME (node)); > and parse the clauses in c/c-parser.c and cp/parser.c, look at how OpenMP > pragmas are parsed (those have the "omp" space, you just use NULL, otherwise > it is not any different). I see Balaji is already parsing the simd pragmas in c_parser_pragma.
On Thu, Apr 11, 2013 at 07:10:46AM -0500, Aldy Hernandez wrote: > >Yeah, the above is definitely wrong. Just > > if (flag_cilkplus) > > cpp_register_deferred_pragma (parse_in, NULL, "simd", PRAGMA_SIMD, true, false); > > Well, the thing is that you can't just use NULL for "#pragma simd" > as a pragma and then define "#pragma simd assert" to use "simd" as a > pragma namespace. I had already tried that: My understanding is that the pragma is #pragma simd, it isn't a namespace, and assert, vectorlength etc. are clauses. http://software.intel.com/sites/products/documentation/studio/composer/en-us/2011Update/compiler_c/cref_cls/common/cppref_pragma_simd.htm #pragma simd [clause[ [,] clause]...] Thus, you'd parse it as PRAGMA_SIMD, then you'd just parse all the (optional) clauses for it. Say for OpenMP 4.0 #pragma omp simd (which is IMHO different just by using a non-NULL namespace). Upon seeing PRAGMA_OMP_SIMD (in your case PRAGMA_SIMD), the C++ parser in cp_parser_pragma just calls a function to handle parsing of it, for PRAGMA_OMP_SIMD on gomp-4_0-branch that is cp_parser_omp_construct (after checking that the pragma doesn't appear outside of function context (== pragma_external). That then just calls cp_parser_omp_simd which uses a helper function (cp_parser_omp_all_clauses) to parse the clauses and newline at the end of the pragma line, then whatever else needs to be parsed after it (for cycle nest). Jakub
On 04/11/13 07:36, Jakub Jelinek wrote: > On Thu, Apr 11, 2013 at 07:10:46AM -0500, Aldy Hernandez wrote: >>> Yeah, the above is definitely wrong. Just >>> if (flag_cilkplus) >>> cpp_register_deferred_pragma (parse_in, NULL, "simd", PRAGMA_SIMD, true, false); >> >> Well, the thing is that you can't just use NULL for "#pragma simd" >> as a pragma and then define "#pragma simd assert" to use "simd" as a >> pragma namespace. I had already tried that: > > My understanding is that the pragma is #pragma simd, it isn't a namespace, > and assert, vectorlength etc. are clauses. > http://software.intel.com/sites/products/documentation/studio/composer/en-us/2011Update/compiler_c/cref_cls/common/cppref_pragma_simd.htm > #pragma simd [clause[ [,] clause]...] > > Thus, you'd parse it as PRAGMA_SIMD, then you'd just parse all the > (optional) clauses for it. Ughh, you are correct. Sorry for the noise. Balaji, I assume you have enough to continue? Thanks y'all. Aldy > > Say for OpenMP 4.0 #pragma omp simd (which is IMHO different just by using > a non-NULL namespace). Upon seeing PRAGMA_OMP_SIMD (in your case > PRAGMA_SIMD), the C++ parser in cp_parser_pragma just calls a function to > handle parsing of it, for PRAGMA_OMP_SIMD on gomp-4_0-branch that is > cp_parser_omp_construct (after checking that the pragma doesn't appear > outside of function context (== pragma_external). That then just > calls cp_parser_omp_simd which uses a helper function > (cp_parser_omp_all_clauses) to parse the clauses and newline at the end > of the pragma line, then whatever else needs to be parsed after it (for > cycle nest). > > Jakub >
> -----Original Message----- > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches- > owner@gcc.gnu.org] On Behalf Of Aldy Hernandez > Sent: Thursday, April 11, 2013 8:55 AM > To: Jakub Jelinek > Cc: Andrew Pinski; Tom Tromey; Iyer, Balaji V; gcc-patches > Subject: Re: libcpp: registering both a pragma and a pragma namespace with the > same name > > On 04/11/13 07:36, Jakub Jelinek wrote: > > On Thu, Apr 11, 2013 at 07:10:46AM -0500, Aldy Hernandez wrote: > >>> Yeah, the above is definitely wrong. Just > >>> if (flag_cilkplus) > >>> cpp_register_deferred_pragma (parse_in, NULL, "simd", > >>> PRAGMA_SIMD, true, false); > >> > >> Well, the thing is that you can't just use NULL for "#pragma simd" > >> as a pragma and then define "#pragma simd assert" to use "simd" as a > >> pragma namespace. I had already tried that: > > > > My understanding is that the pragma is #pragma simd, it isn't a > > namespace, and assert, vectorlength etc. are clauses. > > http://software.intel.com/sites/products/documentation/studio/composer > > /en-us/2011Update/compiler_c/cref_cls/common/cppref_pragma_simd.htm > > #pragma simd [clause[ [,] clause]...] > > > > Thus, you'd parse it as PRAGMA_SIMD, then you'd just parse all the > > (optional) clauses for it. > > Ughh, you are correct. Sorry for the noise. > > Balaji, I assume you have enough to continue? I believe so. Thank you Jakub, Andrew and Aldy! Sincerely, Balaji V. Iyer. > > Thanks y'all. > Aldy > > > > > Say for OpenMP 4.0 #pragma omp simd (which is IMHO different just by > > using a non-NULL namespace). Upon seeing PRAGMA_OMP_SIMD (in your > > case PRAGMA_SIMD), the C++ parser in cp_parser_pragma just calls a > > function to handle parsing of it, for PRAGMA_OMP_SIMD on > > gomp-4_0-branch that is cp_parser_omp_construct (after checking that > > the pragma doesn't appear outside of function context (== > > pragma_external). That then just calls cp_parser_omp_simd which uses > > a helper function > > (cp_parser_omp_all_clauses) to parse the clauses and newline at the > > end of the pragma line, then whatever else needs to be parsed after it > > (for cycle nest). > > > > Jakub > >
diff --git a/libcpp/directives.c b/libcpp/directives.c index 65b2034..d09d2a4 100644 --- a/libcpp/directives.c +++ b/libcpp/directives.c @@ -1373,7 +1373,26 @@ do_pragma (cpp_reader *pfile) if (token->type == CPP_NAME) p = lookup_pragma_entry (p->u.space, token->val.node.node); else - p = NULL; + { + /* See if we can handle pragmas that are defined both as + a pragma namespace, and as an argumentless pragma. + For example: + + #pragma simd vectorlength + #pragma simd // empty argument + */ + if (token->type == CPP_EOF) + { + const cpp_hashnode *node; + node = cpp_lookup (pfile, UC "", 0); + if (node) + p = lookup_pragma_entry (p->u.space, node); + else + p = NULL; + } + else + p = NULL; + } if (allow_name_expansion) pfile->state.prevent_expansion++; count = 2;
Hi Tom. Hi folks. We've asked Balaji to rewrite the <#pragma simd> handling for cilkplus as we currently do for OMP, etc, in init_pragma(). The cilkplus branch currently has something like: cpp_register_deferred_pragma (parse_in, "simd", "", PRAGMA_SIMD_EMPTY, true, false); cpp_register_deferred_pragma (parse_in, "simd", "assert", PRAGMA_SIMD_ASSERT, true, false); cpp_register_deferred_pragma (parse_in, "simd", "noassert", PRAGMA_SIMD_NOASSERT, true, false); cpp_register_deferred_pragma (parse_in, "simd", "vectorlength", PRAGMA_SIMD_VECTORLENGTH, true, false); Notice that #pragma simd can be both a pragma name space, and also a lone pragma with no arguments: #pragma simd assert -or- #pragma simd It seems like the code in libcpp's do_pragma(), specifically disallows this. If we're looking at a possible pragma name space, the next expected token is a CPP_NAME. Is there a way to handle this scenario with the current infrastructure? If not, is something like the attached (untested) patch reasonable? Aldy