Hi Tobias! On 2019-12-11T13:47:51+0100, Tobias Burnus <tob...@codesourcery.com> wrote: > This patch cleans up the public/private handling in libgomp/openacc.f90: > > * module openacc_kinds marked symbols explicitly as public and private > (but default is public). Make this explicit by a 'PUBLIC' and remove the > (redundant) explicit 'public :: <symb>' lines. > > * 'module openacc' had a bunch of 'public :: ' lines but the default was > already 'public'. Changed this to 'private' and marked the > use-associated 'openacc_kinds' symbols as 'public' and added 'public' > statements for the two missing items. (Net effect: this will hide all > openacc_internal symbols.) > > I think this patch is rather obvious. Nonetheless: are the comments?
Sounds good, don't have any comments on the Fortran details ;-) -- but I'll point out there also exists 'libgomp/config/accel/openacc.f90', which probably needs similar treatment? > (If not, I will commit it in the next days.) (Got committed in r279337.) > PS: I found the two missing symbols by looking at the 'interface <name>' > lines; 'module openacc' has only those + the version symbol. Ouch. Ah, no: you said "the default was already 'public'" -- so there's no need to backport that to gcc-9-branch, so that 'acc_copyout_finalize', 'acc_delete_finalize' will be available for OpenACC Fortran users. (These functions are, what would you expect, no covered by any test case in 'libgomp.oacc-fortran/'...) A few comments anyway: > --- a/libgomp/openacc.f90 > +++ b/libgomp/openacc.f90 > @@ -31,13 +31,12 @@ module openacc_kinds > use iso_fortran_env, only: int32 > implicit none > > + public > private :: int32 > - public :: acc_device_kind > > - integer, parameter :: acc_device_kind = int32 > + ! When adding items, also update 'public' setting in 'module openmp' below. This isn't "'module openmp'". ;-) > > - public :: acc_device_none, acc_device_default, acc_device_host > - public :: acc_device_not_host, acc_device_nvidia (So this was missing 'acc_device_gcn' -- but 'public' was the default.) > + integer, parameter :: acc_device_kind = int32 > > ! Keep in sync with include/gomp-constants.h. > integer (acc_device_kind), parameter :: acc_device_none = 0 > @@ -48,16 +47,11 @@ module openacc_kinds > integer (acc_device_kind), parameter :: acc_device_nvidia = 5 > integer (acc_device_kind), parameter :: acc_device_gcn = 8 > > - public :: acc_handle_kind > - > integer, parameter :: acc_handle_kind = int32 > > - public :: acc_async_noval, acc_async_sync > - > ! Keep in sync with include/gomp-constants.h. > integer (acc_handle_kind), parameter :: acc_async_noval = -1 > integer (acc_handle_kind), parameter :: acc_async_sync = -2 > - > end module > > module openacc_internal > @@ -717,6 +711,13 @@ module openacc > use openacc_internal > implicit none > > + private > + ! From openacc_kinds > + public :: acc_device_kind, acc_handle_kind > + public :: acc_device_none, acc_device_default, acc_device_host > + public :: acc_device_not_host, acc_device_nvidia, acc_device_gcn > + public :: acc_async_noval, acc_async_sync > + Some vertical space before the "From openacc_kinds" comment, and before the 'acc_async_*' ones? > public :: openacc_version > > public :: acc_get_num_devices, acc_set_device_type, acc_get_device_type > @@ -730,6 +731,7 @@ module openacc > public :: acc_update_device, acc_update_self, acc_is_present > public :: acc_copyin_async, acc_create_async, acc_copyout_async > public :: acc_delete_async, acc_update_device_async, acc_update_self_async > + public :: acc_copyout_finalize, acc_delete_finalize Put these into the place where they really belong, after 'acc_copyout, and 'acc_delete', respectively? Grüße Thomas
signature.asc
Description: PGP signature
