* [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731] @ 2021-11-20 8:39 Jakub Jelinek 2021-11-22 14:49 ` Thomas Schwinge 0 siblings, 1 reply; 4+ messages in thread From: Jakub Jelinek @ 2021-11-20 8:39 UTC (permalink / raw) To: Thomas Schwinge; +Cc: gcc-patches, Tobias Burnus Hi! The following testcase ICEs because two function declarations are nested in each other and the acc routine handling code isn't prepared to put the pragma on both. The fix is similar to what #pragma omp declare {simd,variant} does, in particular set the fndecl_seen flag already in cp_parser_late_parsing* when we encounter it rather than only after we finalize it. In cp_finalize_oacc_routine I had to move the fndecl_seen diagnostics to non-FUNCTION_DECL block, because for FUNCTION_DECLs the flag is already known to be set from cp_parser_late_parsing_oacc_routine, but can't be removed altogether, because that regresses quality of 2 goacc/routine-5.c diagnostics - we drop "a single " from the '#pragma acc routine' not immediately followed by a single function declaration or definition diagnostic say on #pragma acc routine int foo (), b; if we drop it altogether. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2021-11-20 Jakub Jelinek <jakub@redhat.com> PR c++/101731 * parser.c (cp_parser_late_parsing_oacc_routine): Set parser->oacc_routine->fndecl_seen here, rather than ... (cp_finalize_oacc_routine): ... here. Don't error if parser->oacc_routine->fndecl_seen is set for FUNCTION_DECLs. * c-c++-common/goacc/routine-6.c: New test. --- gcc/cp/parser.c.jj 2021-11-19 16:39:51.534595887 +0100 +++ gcc/cp/parser.c 2021-11-19 22:14:41.209591009 +0100 @@ -46852,8 +46852,8 @@ cp_parser_late_parsing_oacc_routine (cp_ emission easier. */ parser->oacc_routine->clauses = nreverse (parser->oacc_routine->clauses); cp_parser_pop_lexer (parser); - /* Later, cp_finalize_oacc_routine will process the clauses, and then set - fndecl_seen. */ + /* Later, cp_finalize_oacc_routine will process the clauses. */ + parser->oacc_routine->fndecl_seen = true; return attrs; } @@ -46871,16 +46871,17 @@ cp_finalize_oacc_routine (cp_parser *par || fndecl == error_mark_node) return; - if (parser->oacc_routine->fndecl_seen) - { - error_at (parser->oacc_routine->loc, - "%<#pragma acc routine%> not immediately followed by" - " a single function declaration or definition"); - parser->oacc_routine = NULL; - return; - } if (TREE_CODE (fndecl) != FUNCTION_DECL) { + if (parser->oacc_routine->fndecl_seen) + { + error_at (parser->oacc_routine->loc, + "%<#pragma acc routine%> not immediately followed by" + " a single function declaration or definition"); + parser->oacc_routine = NULL; + return; + } + cp_ensure_no_oacc_routine (parser); return; } @@ -46921,11 +46922,6 @@ cp_finalize_oacc_routine (cp_parser *par parser->oacc_routine->clauses, DECL_ATTRIBUTES (fndecl)); } - - /* Don't unset parser->oacc_routine here: we may still need it to - diagnose wrong usage. But, remember that we've used this "#pragma acc - routine". */ - parser->oacc_routine->fndecl_seen = true; } } --- gcc/testsuite/c-c++-common/goacc/routine-6.c.jj 2021-11-19 22:13:11.150864445 +0100 +++ gcc/testsuite/c-c++-common/goacc/routine-6.c 2021-11-19 22:13:11.150864445 +0100 @@ -0,0 +1,4 @@ +/* PR c++/101731 */ + +#pragma acc routine /* { dg-error "not immediately followed by a single function declaration or definition" "" { target c++ } } */ +int foo (int bar ()); Jakub ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731] 2021-11-20 8:39 [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731] Jakub Jelinek @ 2021-11-22 14:49 ` Thomas Schwinge 2021-11-22 15:02 ` Jakub Jelinek 0 siblings, 1 reply; 4+ messages in thread From: Thomas Schwinge @ 2021-11-22 14:49 UTC (permalink / raw) To: Jakub Jelinek; +Cc: Tobias Burnus, gcc-patches Hi Jakub! On 2021-11-20T09:39:53+0100, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > The following testcase ICEs because two function declarations are nested in > each other and the acc routine handling code isn't prepared to put the > pragma on both. Ha. Many things covered in 'c-c++-common/goacc/routine-5.c' -- but not that. > The fix is similar to what #pragma omp declare {simd,variant} does, > in particular set the fndecl_seen flag already in cp_parser_late_parsing* > when we encounter it rather than only after we finalize it. > > In cp_finalize_oacc_routine I had to move the fndecl_seen diagnostics to > non-FUNCTION_DECL block, because for FUNCTION_DECLs the flag is already > known to be set from cp_parser_late_parsing_oacc_routine, but can't be > removed altogether, because that regresses quality of 2 goacc/routine-5.c > diagnostics - we drop "a single " from the > '#pragma acc routine' not immediately followed by a single function declaration or definition > diagnostic say on > #pragma acc routine > int foo (), b; > if we drop it altogether. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Thanks for looking into this. I now don't recall my exact thinking when reworking this code five years ago... Please push your change, fixing the ICE. Then, regarding the user-visible behavior: > +#pragma acc routine /* { dg-error "not immediately followed by a single function declaration or definition" "" { target c++ } } */ > +int foo (int bar ()); So in C++ we now refuse, but in C we do accept this. I suppose I shall look into making C behave the same way -- unless there is a reason for the different behavior? And/or, is it actually is useful to allow such nested usage? Per its associated clauses, an OpenACC 'routine' directive really is meant to apply to one function only, in contrast to OpenMP 'target declare'. But the question is whether we should raise an error for the example above, or whether the 'routine' shall just apply to 'foo' but not 'bar', but without an error diagnostic? OpenACC 3.2, 2.15.1 "Routine Directive" states that "the 'routine' directive without a name may appear immediately before a function definition, a C++ _lambda_, or just before a function prototype and applies to that immediately following function or prototype". Grüße Thomas > 2021-11-20 Jakub Jelinek <jakub@redhat.com> > > PR c++/101731 > * parser.c (cp_parser_late_parsing_oacc_routine): Set > parser->oacc_routine->fndecl_seen here, rather than ... > (cp_finalize_oacc_routine): ... here. Don't error if > parser->oacc_routine->fndecl_seen is set for FUNCTION_DECLs. > > * c-c++-common/goacc/routine-6.c: New test. > > --- gcc/cp/parser.c.jj 2021-11-19 16:39:51.534595887 +0100 > +++ gcc/cp/parser.c 2021-11-19 22:14:41.209591009 +0100 > @@ -46852,8 +46852,8 @@ cp_parser_late_parsing_oacc_routine (cp_ > emission easier. */ > parser->oacc_routine->clauses = nreverse (parser->oacc_routine->clauses); > cp_parser_pop_lexer (parser); > - /* Later, cp_finalize_oacc_routine will process the clauses, and then set > - fndecl_seen. */ > + /* Later, cp_finalize_oacc_routine will process the clauses. */ > + parser->oacc_routine->fndecl_seen = true; > > return attrs; > } > @@ -46871,16 +46871,17 @@ cp_finalize_oacc_routine (cp_parser *par > || fndecl == error_mark_node) > return; > > - if (parser->oacc_routine->fndecl_seen) > - { > - error_at (parser->oacc_routine->loc, > - "%<#pragma acc routine%> not immediately followed by" > - " a single function declaration or definition"); > - parser->oacc_routine = NULL; > - return; > - } > if (TREE_CODE (fndecl) != FUNCTION_DECL) > { > + if (parser->oacc_routine->fndecl_seen) > + { > + error_at (parser->oacc_routine->loc, > + "%<#pragma acc routine%> not immediately followed by" > + " a single function declaration or definition"); > + parser->oacc_routine = NULL; > + return; > + } > + > cp_ensure_no_oacc_routine (parser); > return; > } > @@ -46921,11 +46922,6 @@ cp_finalize_oacc_routine (cp_parser *par > parser->oacc_routine->clauses, > DECL_ATTRIBUTES (fndecl)); > } > - > - /* Don't unset parser->oacc_routine here: we may still need it to > - diagnose wrong usage. But, remember that we've used this "#pragma acc > - routine". */ > - parser->oacc_routine->fndecl_seen = true; > } > } > > --- gcc/testsuite/c-c++-common/goacc/routine-6.c.jj 2021-11-19 22:13:11.150864445 +0100 > +++ gcc/testsuite/c-c++-common/goacc/routine-6.c 2021-11-19 22:13:11.150864445 +0100 > @@ -0,0 +1,4 @@ > +/* PR c++/101731 */ > + > +#pragma acc routine /* { dg-error "not immediately followed by a single function declaration or definition" "" { target c++ } } */ > +int foo (int bar ()); > > Jakub ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731] 2021-11-22 14:49 ` Thomas Schwinge @ 2021-11-22 15:02 ` Jakub Jelinek 2022-01-13 12:07 ` Merge 'c-c++-common/goacc/routine-6.c' into 'c-c++-common/goacc/routine-5.c', and document current C/C++ difference (was: [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731]) Thomas Schwinge 0 siblings, 1 reply; 4+ messages in thread From: Jakub Jelinek @ 2021-11-22 15:02 UTC (permalink / raw) To: Thomas Schwinge; +Cc: Tobias Burnus, gcc-patches On Mon, Nov 22, 2021 at 03:49:42PM +0100, Thomas Schwinge wrote: > Then, regarding the user-visible behavior: > > > +#pragma acc routine /* { dg-error "not immediately followed by a single function declaration or definition" "" { target c++ } } */ > > +int foo (int bar ()); > > So in C++ we now refuse, but in C we do accept this. I suppose I shall > look into making C behave the same way -- unless there is a reason for > the different behavior? And/or, is it actually is useful to allow such > nested usage? Per its associated clauses, an OpenACC 'routine' directive > really is meant to apply to one function only, in contrast to OpenMP > 'target declare'. But the question is whether we should raise an error > for the example above, or whether the 'routine' shall just apply to 'foo' > but not 'bar', but without an error diagnostic? All I've verified is that our OpenMP code handles it the same way, i.e. #pragma omp declare simd int foo (int bar ()); is accepted in C and rejected in C++. I guess one question is to check if it is in both languages actually the same thing. If we want to accept it in C++ and let the pragma apply only to the outer declaration, I guess we'd need to temporarily set to NULL parser->omp_declare_simd and parser->oacc_routine while parsing the parameters of a function declaration or definition. At least OpenMP is fairly fuzzy here, the reason we error on #pragma omp declare simd int foo (), i; has been mainly some discussions in the lang committee and the fact that it talks about a single declaration, not all affected declarations. Whether int foo (int bar ()); should be in that light treated as two function declarations or one with another one nested in it and irrelevant for it is unclear. Jakub ^ permalink raw reply [flat|nested] 4+ messages in thread
* Merge 'c-c++-common/goacc/routine-6.c' into 'c-c++-common/goacc/routine-5.c', and document current C/C++ difference (was: [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731]) 2021-11-22 15:02 ` Jakub Jelinek @ 2022-01-13 12:07 ` Thomas Schwinge 0 siblings, 0 replies; 4+ messages in thread From: Thomas Schwinge @ 2022-01-13 12:07 UTC (permalink / raw) To: gcc-patches; +Cc: Jakub Jelinek, Tobias Burnus [-- Attachment #1: Type: text/plain, Size: 2491 bytes --] Hi! On 2021-11-22T16:02:31+0100, Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > On Mon, Nov 22, 2021 at 03:49:42PM +0100, Thomas Schwinge wrote: >> Then, regarding the user-visible behavior: >> >> > +#pragma acc routine /* { dg-error "not immediately followed by a single function declaration or definition" "" { target c++ } } */ >> > +int foo (int bar ()); >> >> So in C++ we now refuse, but in C we do accept this. I suppose I shall >> look into making C behave the same way -- unless there is a reason for >> the different behavior? And/or, is it actually is useful to allow such >> nested usage? Per its associated clauses, an OpenACC 'routine' directive >> really is meant to apply to one function only, in contrast to OpenMP >> 'target declare'. But the question is whether we should raise an error >> for the example above, or whether the 'routine' shall just apply to 'foo' >> but not 'bar', but without an error diagnostic? > > All I've verified is that our OpenMP code handles it the same way, Thanks for the explanation. Pushed to master branch commit 67fdcc8835665b5bc13652205e815e498d65c5a1 "Merge 'c-c++-common/goacc/routine-6.c' into 'c-c++-common/goacc/routine-5.c', and document current C/C++ difference", see attached. Grüße Thomas > i.e. > #pragma omp declare simd > int foo (int bar ()); > is accepted in C and rejected in C++. > I guess one question is to check if it is in both languages actually > the same thing. If we want to accept it in C++ and let the pragma > apply only to the outer declaration, I guess we'd need to temporarily > set to NULL parser->omp_declare_simd and parser->oacc_routine while > parsing the parameters of a function declaration or definition. > At least OpenMP is fairly fuzzy here, the reason we error on > #pragma omp declare simd > int foo (), i; > has been mainly some discussions in the lang committee and the fact > that it talks about a single declaration, not all affected declarations. > Whether int foo (int bar ()); should be in that light treated as two > function declarations or one with another one nested in it and irrelevant > for it is unclear. > > Jakub ----------------- Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Merge-c-c-common-goacc-routine-6.c-into-c-c-common-g.patch --] [-- Type: text/x-diff, Size: 2266 bytes --] From 67fdcc8835665b5bc13652205e815e498d65c5a1 Mon Sep 17 00:00:00 2001 From: Thomas Schwinge <thomas@codesourcery.com> Date: Mon, 22 Nov 2021 16:09:09 +0100 Subject: [PATCH] Merge 'c-c++-common/goacc/routine-6.c' into 'c-c++-common/goacc/routine-5.c', and document current C/C++ difference gcc/testsuite/ * c-c++-common/goacc/routine-6.c: Merge into... * c-c++-common/goacc/routine-5.c: ... this, and document current C/C++ difference. --- gcc/testsuite/c-c++-common/goacc/routine-5.c | 8 ++++++++ gcc/testsuite/c-c++-common/goacc/routine-6.c | 4 ---- 2 files changed, 8 insertions(+), 4 deletions(-) delete mode 100644 gcc/testsuite/c-c++-common/goacc/routine-6.c diff --git a/gcc/testsuite/c-c++-common/goacc/routine-5.c b/gcc/testsuite/c-c++-common/goacc/routine-5.c index e3fbd6573b8..94678f2bf5b 100644 --- a/gcc/testsuite/c-c++-common/goacc/routine-5.c +++ b/gcc/testsuite/c-c++-common/goacc/routine-5.c @@ -94,6 +94,14 @@ typedef struct c_2 c_2; #pragma acc routine /* { dg-error ".#pragma acc routine. not immediately followed by function declaration or definition" } */ struct d_2 {} d_2; +/* PR c++/101731 */ +/* Regarding the current C/C++ difference, see + <http://mid.mail-archive.com/20211122150231.GP2646553@tucnak>. */ +#pragma acc routine /* { dg-error "not immediately followed by a single function declaration or definition" "" { target c++ } } */ +int pr101731_foo (int pr101731_bar ()); +#pragma acc routine (pr101731_foo) vector /* { dg-error "has already been marked with an OpenACC 'routine' directive" "" { target c } } */ +#pragma acc routine (pr101731_bar) vector /* { dg-error "'pr101731_bar' has not been declared" } */ + #pragma acc routine /* { dg-error ".#pragma acc routine. not immediately followed by function declaration or definition" } */ #pragma acc routine int fn4 (void); diff --git a/gcc/testsuite/c-c++-common/goacc/routine-6.c b/gcc/testsuite/c-c++-common/goacc/routine-6.c deleted file mode 100644 index 0a231a015a7..00000000000 --- a/gcc/testsuite/c-c++-common/goacc/routine-6.c +++ /dev/null @@ -1,4 +0,0 @@ -/* PR c++/101731 */ - -#pragma acc routine /* { dg-error "not immediately followed by a single function declaration or definition" "" { target c++ } } */ -int foo (int bar ()); -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-13 12:07 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-11-20 8:39 [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731] Jakub Jelinek 2021-11-22 14:49 ` Thomas Schwinge 2021-11-22 15:02 ` Jakub Jelinek 2022-01-13 12:07 ` Merge 'c-c++-common/goacc/routine-6.c' into 'c-c++-common/goacc/routine-5.c', and document current C/C++ difference (was: [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731]) Thomas Schwinge
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).