public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Chung-Lin Tang <cltang@codesourcery.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
	Catherine Moore <clm@codesourcery.com>,
	Andrew Stubbs <ams@codesourcery.com>
Subject: Re: [PATCH, OpenMP 5.0] More implementation of the requires directive
Date: Wed, 13 Jan 2021 16:27:28 +0100	[thread overview]
Message-ID: <20210113152728.GN1034503@tucnak> (raw)
In-Reply-To: <4273bf27-3f0e-0066-393b-2a561a7b9e12@codesourcery.com>

On Wed, Jan 13, 2021 at 11:07:44PM +0800, Chung-Lin Tang wrote:
> 2021-01-13  Chung-Lin Tang  <cltang@codesourcery.com>

...
Looks mostly ok, with some nits.

> 	* parse.c ("tree.h"): Add include.
> 	("omp-general.h"): Likewise.

I think the usual way is to write:
	* parse.c: Include "tree.h" and "omp-general.h".
 	(gfc_parse_file): Add code to merge omp_requires to omp_requires_mask.

Something I miss in the patch is that for the device API calls (I'd bother
only with direct calls) we should probably set OMP_REQUIRES_TARGET_USED
too, so perhaps do that during gimplification if flag_openmp and
in gimplify_call_expr there is fndecl and DECL_NAME of it is non-NULL and
starts with "omp_" it looks at DECL_ASSEMBLER_NAME and compares that to a
selected list of device APIs.

Also, would it be possible to diagnose .gnu.gomp_requires mismatches also
at link time through the linker plugin/mkoffload?
At least if we have LTO offloading bytecode in and the plugin is doing
something...

> +      if (flag_openmp && (omp_requires_mask & OMP_REQUIRES_TARGET_USED) != 0)
> +	{
> +	  const char *requires_section = ".gnu.gomp_requires";
> +	  tree maskvar = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> +				     get_identifier (".gomp_requires_mask"),
> +				     unsigned_type_node);
> +	  SET_DECL_ALIGN (maskvar, TYPE_ALIGN (unsigned_type_node));
> +	  TREE_STATIC (maskvar) = 1;
> +	  DECL_INITIAL (maskvar)
> +	    = build_int_cst (unsigned_type_node,
> +			     ((unsigned int) omp_requires_mask
> +			      & (OMP_REQUIRES_UNIFIED_ADDRESS
> +				 | OMP_REQUIRES_UNIFIED_SHARED_MEMORY
> +				 | OMP_REQUIRES_REVERSE_OFFLOAD)));
> +	  set_decl_section_name (maskvar, requires_section);

This probably needs to sorry if the target doesn't support named sections.
We probably don't support LTO in that case either though.

Also, the diagnostic of the mismatches on the library side should print
details, say that libfoobar is #pragma omp requires unified_shared_memory
while libbar is not.

	Jakub


  reply	other threads:[~2021-01-13 15:27 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-13 15:07 Chung-Lin Tang
2021-01-13 15:27 ` Jakub Jelinek [this message]
2021-03-25 11:18 ` Thomas Schwinge
2022-03-29 13:42 ` Andrew Stubbs
2022-06-08  3:56 ` [Patch] OpenMP: Move omp requires checks to libgomp Tobias Burnus
2022-06-09 11:40   ` Jakub Jelinek
2022-06-09 12:46     ` Tobias Burnus
2022-06-09 14:19       ` Jakub Jelinek
2022-06-29 14:33         ` [Patch][v4] " Tobias Burnus
2022-06-29 17:02           ` Jakub Jelinek
2022-06-29 18:10             ` Tobias Burnus
2022-06-29 20:18               ` Jakub Jelinek
2022-07-01 13:06                 ` [Patch][v5] " Tobias Burnus
2022-07-01 14:34                   ` Jakub Jelinek
2022-07-01 16:31                     ` Tobias Burnus
2022-07-01 16:55                       ` Jakub Jelinek
2022-07-01 21:08                         ` Tobias Burnus
2022-07-04  8:31                           ` Jakub Jelinek
2022-07-07 13:26                           ` Fix one issue in OpenMP 'requires' directive diagnostics (was: [Patch][v5] OpenMP: Move omp requires checks to libgomp) Thomas Schwinge
2022-07-07 13:56                             ` Tobias Burnus
2022-07-08  6:59                               ` Thomas Schwinge
2022-07-06 10:42                   ` Restore 'GOMP_offload_unregister_ver' functionality " Thomas Schwinge
2022-07-06 13:59                     ` Tobias Burnus
2022-07-06 21:08                       ` Thomas Schwinge
2022-08-17 11:45                       ` Jakub Jelinek
2023-09-15  9:41                   ` [Patch][v5] OpenMP: Move omp requires checks to libgomp Thomas Schwinge
2022-07-07  8:37           ` Adjust 'libgomp.c-c++-common/requires-3.c' (was: [Patch][v4] OpenMP: Move omp requires checks to libgomp) Thomas Schwinge
2022-07-07  9:02             ` Tobias Burnus
2022-07-07  8:42           ` Enhance 'libgomp.c-c++-common/requires-4.c', 'libgomp.c-c++-common/requires-5.c' testing " Thomas Schwinge
2022-07-07  9:36             ` Tobias Burnus
2022-07-07 10:42               ` Thomas Schwinge
2022-07-06 10:30   ` Define 'OMP_REQUIRES_[...]', 'GOMP_REQUIRES_[...]' in a single place (was: [Patch] " Thomas Schwinge
2022-07-06 13:40     ` Tobias Burnus
2022-07-06 11:04   ` Fix Intel MIC 'mkoffload' for OpenMP 'requires' " Thomas Schwinge
2022-07-06 11:29     ` Tobias Burnus
2022-07-06 12:38       ` Thomas Schwinge
2022-07-06 13:30         ` Tobias Burnus
2022-07-07 10:46           ` Thomas Schwinge
2022-07-06 14:19     ` Tobias Burnus
2024-03-07 12:38   ` nvptx: 'cuDeviceGetCount' failure is fatal " Thomas Schwinge
2024-03-07 14:28     ` nvptx: 'cuDeviceGetCount' failure is fatal Tobias Burnus
2024-03-08 15:58       ` Thomas Schwinge

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=20210113152728.GN1034503@tucnak \
    --to=jakub@redhat.com \
    --cc=ams@codesourcery.com \
    --cc=clm@codesourcery.com \
    --cc=cltang@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).