Re: [PATCH, OpenACC 2.7] Implement host_data must have use_device clause requirement

[Thomas Schwinge <thomas@codesourcery.com>]

Hi Chung-Lin!

On 2023-06-06T23:10:37+0800, Chung-Lin Tang <chunglin.tang@siemens.com> wrote:
> this patch implements the OpenACC 2.7 change requiring the host_data construct
> to have at least one use_device clause.

Thanks!

> This patch started out with a simple check during gimplify (much smaller patch),

Heh, thanks for the explanation -- would've been my first question
otherweise.  ;-)

> but turned out that front-ends removed use_device clauses when they have error,
> and the gimplify check started to echo a "no use_device clause" message in such
> cases, which seem confusing for the user. So ended up adding the check in each
> front-end instead.

I presume that's also the reason why you're doing this check before
'c_finish_omp_clauses' etc.?

I'll clarify with the OpenACC Technical Committee whether really those
diagnostics are intended as "error" or should instead just be "warning".
After all, there's no actual problem with an OpenACC 'host_data' without
'use_device' clause (or no data clause on OpenACC 'data', 'enter data',
'exit data', 'update', etc.) -- it's just likely that the user missed
something.  That is, the OpenACC 2.7: "At least one 'use_device' clause
must appear" is addressing the user, not at the implementation (in my
current interpretation).  Depending on the outcome of that, we can easily
adjust GCC.

Note for later, independently of your work here:
'c_parser_oacc_enter_exit_data' etc. for its corresponding "has no data
movement clause" diagnostic actually does 'c_finish_omp_clauses' etc.
first -- maybe that should be changed accordingly.  (Actually, I note
that it's only OpenACC 3.0 that "Required at least one data clause on a
'data' construct, an 'enter data' directive, or an 'exit data'
directive", heh...  Per his internal 2014-10-17 email, Cesar implemented
the code of 'c_parser_oacc_enter_exit_data' etc. "similar to that of acc
update", which indeed already back then did require "At least one 'self',
'host', or 'device' clause".  Fortran does have the diagnostic for
OpenACC 'update', but it's missing for OpenACC 'enter data', 'exit data'
without data clause (have not checked other constructs with similar
requirements).)

> Tested on powerpc64le-linux/nvptx, x86_64-linux/amdgcn tests in progress (expect
> no surprises). Is this okay for trunk?

OK with one small change, please -- unless there's a reason for doing it
this way:

> --- a/gcc/fortran/trans-openmp.cc
> +++ b/gcc/fortran/trans-openmp.cc
> @@ -4677,6 +4677,12 @@ gfc_trans_oacc_construct (gfc_code *code)
>       break;
>        case EXEC_OACC_HOST_DATA:
>       construct_code = OACC_HOST_DATA;
> +     if (code->ext.omp_clauses->lists[OMP_LIST_USE_DEVICE] == NULL)
> +       {
> +         error_at (gfc_get_location (&code->loc),
> +                   "%<host_data%> construct requires %<use_device%> clause");
> +         return NULL_TREE;
> +       }
>       break;
>        default:
>       gcc_unreachable ();

The OpenMP "must contain at least one [...] clause" checks are done in
'gcc/fortran/openmp.cc:resolve_omp_clauses'.  For consistency (or, to let
'gcc/fortran/trans-openmp.cc' continue to just deal with "directive
translation"), do similar for OpenACC 'host_data'?  (..., and we later
accordingly adjust 'gcc/fortran/openmp.cc:gfc_match_oacc_update', too?)


Grüße
 Thomas
-----------------
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