Re: [PATCH] openacc: Fix up C++ #pragma acc routine handling [PR101731]

[Thomas Schwinge <thomas@codesourcery.com>]

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