public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Kwok Cheung Yeung <kcy@codesourcery.com>
Cc: gcc-patches@gcc.gnu.org, fortran@gcc.gnu.org, thomas@codesourcery.com
Subject: Re: [PATCH 4/5, OpenACC] Allow optional arguments to be used in the use_device OpenACC clause
Date: Wed, 31 Jul 2019 13:13:00 -0000	[thread overview]
Message-ID: <20190731125438.GB15878@tucnak> (raw)
In-Reply-To: <7e1a7bf1-0f72-f024-b1eb-296a696f768b@codesourcery.com>

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

  reply	other threads:[~2019-07-31 12:54 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <75222f0b-b55d-dcd9-b4f4-8e218675e8d7@mentor.com>
2019-07-12 11:35 ` [PATCH 0/5, OpenACC] Add support for Fortran optional arguments in OpenACC Kwok Cheung Yeung
2019-07-12 11:36   ` [PATCH 1/5, OpenACC] Allow NULL as an argument to OpenACC 2.6 directives Kwok Cheung Yeung
2019-11-29 14:42     ` [PR92726] OpenACC: 'NULL'-in -> no-op, and/or 'NULL'-out (was: [PATCH 1/5, OpenACC] Allow NULL as an argument to OpenACC 2.6 directives) Thomas Schwinge
2019-11-20 13:11       ` [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments Tobias Burnus
2019-11-29 12:17         ` Tobias Burnus
2019-12-05 15:16         ` Jakub Jelinek
2019-12-05 15:47           ` [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments) Thomas Schwinge
2019-12-05 16:04             ` Jakub Jelinek
2019-12-05 20:21               ` Segher Boessenkool
2019-12-05 16:17             ` Joseph Myers
2019-12-05 16:24               ` Paul Koning
2019-12-05 16:40                 ` Jeff Law
2019-12-05 16:55                 ` [RFC] Characters per line: from punch card (80) to line printer (132) Florian Weimer
2019-12-05 17:55               ` Andrew Stubbs
2019-12-05 18:12                 ` Eric Gallager
2019-12-05 18:22                 ` Robin Curtis
2019-12-05 19:16                   ` James Secan
2019-12-06  9:22                   ` Andrew Stubbs
2019-12-05 16:44             ` [RFC] Characters per line: from punch card (80) to line printer (132) (was: [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments) Michael Matz
2019-12-05 17:03               ` Jonathan Wakely
2019-12-05 18:07                 ` Marek Polacek
2019-12-05 20:06                 ` Segher Boessenkool
2019-12-05 20:38                   ` Marek Polacek
2019-12-05 22:02                     ` Segher Boessenkool
2019-12-05 20:56                   ` Jonathan Wakely
2019-12-05 22:19                     ` Segher Boessenkool
2019-12-05 22:34                       ` Jonathan Wakely
2019-12-05 22:37                       ` Jonathan Wakely
2019-12-05 23:02                         ` Segher Boessenkool
2019-12-05 17:29               ` N.M. Maclaren
2019-12-05 20:12               ` Segher Boessenkool
2019-12-05 20:41               ` Jason Merrill
2019-12-05 18:54             ` [RFC] Characters per line: from punch card (80) to line printer (132) Martin Sebor
2019-12-05 20:32               ` Segher Boessenkool
2019-12-07 14:49         ` [Patch][OpenMP/OpenACC/Fortran] Fix mapping of optional (present|absent) arguments Thomas Schwinge
2019-12-11 10:52           ` Tobias Burnus
2019-12-10 17:54             ` [Patch, Fortran] OpenMP/OpenACC – fix more issues with OPTIONAL Tobias Burnus
2019-12-16  9:25               ` *ping* " Tobias Burnus
2019-12-29 23:46                 ` *ping**2 " Tobias Burnus
2019-12-30  4:33                   ` Jerry
2020-01-03 11:29               ` Jakub Jelinek
2020-01-08  8:33               ` Thomas Schwinge
2020-01-08  8:55                 ` Tobias Burnus
2020-04-29 10:00                   ` Thomas Schwinge
2020-04-29 14:01                     ` Tobias Burnus
2020-04-30  6:24                       ` Richard Biener
2020-04-30  7:17                         ` Jakub Jelinek
2020-05-05  9:08                           ` Tobias Burnus
2020-06-17 22:22                       ` Thomas Schwinge
2019-12-02 16:59       ` [PR92726] OpenACC: 'NULL'-in -> no-op, and/or 'NULL'-out Tobias Burnus
2019-07-12 11:37   ` [PATCH 2/5, OpenACC] Support Fortran optional arguments in the firstprivate clause Kwok Cheung Yeung
2019-07-12 11:42     ` Jakub Jelinek
2019-07-17 18:08       ` Kwok Cheung Yeung
2019-07-17 20:49         ` Tobias Burnus
2019-07-18  9:30           ` Tobias Burnus
2019-07-18 11:45             ` Kwok Cheung Yeung
2019-07-29 21:01       ` Kwok Cheung Yeung
2019-07-31 12:54         ` Jakub Jelinek
2019-10-07  9:26         ` Thomas Schwinge
2019-10-07 13:16           ` Kwok Cheung Yeung
2019-07-12 11:39   ` [PATCH 4/5, OpenACC] Allow optional arguments to be used in the use_device OpenACC clause Kwok Cheung Yeung
2019-07-29 22:42     ` Kwok Cheung Yeung
2019-07-31 13:13       ` Jakub Jelinek [this message]
2019-07-12 11:39   ` [PATCH 3/5, OpenACC] Add support for allocatable arrays as optional arguments Kwok Cheung Yeung
2019-07-12 11:41   ` [PATCH 5/5, OpenACC] Add tests for Fortran optional arguments in OpenACC 2.6 Kwok Cheung Yeung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190731125438.GB15878@tucnak \
    --to=jakub@redhat.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=kcy@codesourcery.com \
    --cc=thomas@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).