[prev in list] [next in list] [prev in thread] [next in thread] 

List:       gcc-fortran
Subject:    Re: [PATCH 4/5, OpenACC] Allow optional arguments to be used in the use_device OpenACC clause
From:       Jakub Jelinek <jakub () redhat ! com>
Date:       2019-07-31 12:54:38
Message-ID: 20190731125438.GB15878 () tucnak
[Download RAW message or body]

On Mon, Jul 29, 2019 at 10:00:53PM +0100, Kwok Cheung Yeung wrote:
> --- a/gcc/omp-low.c
> +++ b/gcc/omp-low.c
> @@ -11636,9 +11636,40 @@ lower_omp_target (gimple_stmt_iterator *gsi_p,
> omp_context *ctx)
>  	      tkind = GOMP_MAP_FIRSTPRIVATE_INT;
>  	    type = TREE_TYPE (ovar);
>  	    if (TREE_CODE (type) == ARRAY_TYPE)
> -	      var = build_fold_addr_expr (var);
> +	      {
> +		var = build_fold_addr_expr (var);
> +		gimplify_assign (x, var, &ilist);
> +	      }
>  	    else
>  	      {
> +		tree opt_arg_label;
> +		bool optional_arg_p
> +		  = TREE_CODE (TREE_TYPE (ovar)) == POINTER_TYPE
> +		    && omp_is_optional_argument (ovar);

This should be also wrapped in ()s for emacs, like:

		bool optional_arg_p
		  = (TREE_CODE (TREE_TYPE (ovar)) == POINTER_TYPE
		     && omp_is_optional_argument (ovar));

> +
> +		if (optional_arg_p)
> +		  {
> +		    tree null_label
> +		      = create_artificial_label (UNKNOWN_LOCATION);
> +		    tree notnull_label
> +		      = create_artificial_label (UNKNOWN_LOCATION);
> +		    opt_arg_label
> +		      = create_artificial_label (UNKNOWN_LOCATION);
> +		    tree new_x = copy_node (x);

Please use unshare_expr instead of copy_node here.

Otherwise LGTM.

On the OpenMP side this isn't sufficient, because it only
handles mapping clauses, not the data sharing, so I guess I'll need to go
through all data sharing clauses on all constructs, write testcases and see
if what OpenMP spec says (just a general rule):
"If a list item that appears in a directive or clause is an optional dummy argument that is not present,
the directive or clause for that list item is ignored.

If the variable referenced inside a construct is an optional dummy argument that is not present, any
explicitly determined, implicitly determined, or predetermined data-sharing and data-mapping
attribute rules for that variable are ignored. Otherwise, if the variable is an optional dummy
argument that is present, it is present inside the construct."
is handled right.

	Jakub
[prev in list] [next in list] [prev in thread] [next in thread]