[patch] libgomp/openacc.f90 – clean-up public/private attributes

[Thomas Schwinge]

Hi Tobias!

On 2019-12-11T13:47:51+0100, Tobias Burnus <tobias@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 658 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20191216/8b9c1889/attachment.sig>