public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Stubbs <ams@codesourcery.com>
To: Tobias Burnus <tobias@codesourcery.com>, <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 08/17] openmp: -foffload-memory=pinned
Date: Fri, 8 Jul 2022 10:55:07 +0100	[thread overview]
Message-ID: <2d973ffb-6e31-b8c6-f32f-86fac369c6eb@codesourcery.com> (raw)
In-Reply-To: <4dba87f8-2375-d6ad-1289-b8de95014b3a@codesourcery.com>

On 08/07/2022 10:00, Tobias Burnus wrote:
> On 08.07.22 00:18, Andrew Stubbs wrote:
>>> Likewise, the 'requires' mechanism could then also be used in '[PATCH 
>>> 16/17] amdgcn, openmp: Auto-detect USM mode and set HSA_XNACK'.
>>
>> No, I don't think so; that environment variable needs to be set before 
>> the libraries are loaded or it's too late.  There are other ways to 
>> achieve the same thing, by leaving messages for the libgomp plugin to 
>> pick up, perhaps, but it's all extra complexity for no real gain. 
> 
> I think we talk about two different things:
> 
> 
> (a) where (and when) to check/set the environment variable. I think this 
> part is fine. You could consider moving the generated code for 
> 'configure_xnack' code into the existing 'init' constructor function, 
> but it does not really matter. (Nor does the order in which the 
> constructor function runs.)
> 
> (I also do not see any benefit of moving it to libgomp. The message 
> could then be suppressed if no device available or similar tricky, but I 
> do not see any real advantage of moving it.)
> 
> Longer side note: I think the message "error: HSA_XNACK=%%s is 
> incompatible; please unset" could be clearer. Both in terms who issues 
> it and that it talks about an environment variable. Maybe:
> 
> "|libgomp: fatal error: Environment variable HSA_XNACK=%s is 
> incompatible with GCN offloading; please unset"|
> 
> |or something like that. (I did misuse 'libgomp:' for this; I am not 
> sure that makes sense or is even more misleading.) – I am also not sure 
> GCN fits that well, given that CDNA is not GCN. But that is a general
> problem. But in any case, adding "fatal", "environment variable" and ... 
> offloading makes surely sense, IMHO.

It's not incompatible with GCN offloading, only with the XNACK mode in 
which the binary was compiled (i.e. USM on or off).

The message could be less terse, indeed. I went through a variety of 
messages for this and couldn't find one that I liked. How about...

   fatal error: HSA_XNACK=%s is set but this program was compiled for 
HSA_XNACK=%s; please unset your environment variable.

> (b) How the value is made available inside both gcc/config/gcn/gcn.cc 
> and in mkoffload.cc.
> 
> I was talking about (b). Namely:
> 
> omp_requires_mask is already available in gcc/config/gcn/gcn.cc and 
> mkoffload.cc. Thus, there is no reason to reinvent the wheel and coming 
> up with another means to pass the same kind of data to the very same files.
> 
> (You still might want to add another flag to it (assuming 'omp requires 
> unified_shared_memory' alias OMP_REQUIRES_UNIFIED_SHARED_MEMORY is 
> insufficient.)

OK, this is a new feature that I probably should investigate.

Thanks

Andrew


  reply	other threads:[~2022-07-08  9:55 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-07 10:34 [PATCH 00/17] openmp, nvptx, amdgcn: 5.0 Memory Allocators Andrew Stubbs
2022-07-07 10:34 ` [PATCH 01/17] libgomp, nvptx: low-latency memory allocator Andrew Stubbs
2022-12-08 11:40   ` Jakub Jelinek
2022-07-07 10:34 ` [PATCH 02/17] libgomp: pinned memory Andrew Stubbs
2022-12-08 12:11   ` Jakub Jelinek
2022-12-08 12:51     ` Andrew Stubbs
2022-12-08 14:02       ` Tobias Burnus
2022-12-08 14:35         ` Andrew Stubbs
2022-12-08 15:02           ` Tobias Burnus
2022-07-07 10:34 ` [PATCH 03/17] libgomp, openmp: Add ompx_pinned_mem_alloc Andrew Stubbs
2022-07-07 10:34 ` [PATCH 04/17] openmp, nvptx: low-lat memory access traits Andrew Stubbs
2022-07-07 10:34 ` [PATCH 05/17] openmp, nvptx: ompx_unified_shared_mem_alloc Andrew Stubbs
2022-07-07 10:34 ` [PATCH 06/17] openmp: Add -foffload-memory Andrew Stubbs
2022-07-07 10:34 ` [PATCH 07/17] openmp: allow requires unified_shared_memory Andrew Stubbs
2022-07-07 10:34 ` [PATCH 08/17] openmp: -foffload-memory=pinned Andrew Stubbs
2022-07-07 11:54   ` Tobias Burnus
2022-07-07 22:18     ` Andrew Stubbs
2022-07-08  9:00       ` Tobias Burnus
2022-07-08  9:55         ` Andrew Stubbs [this message]
2022-07-08  9:57           ` Tobias Burnus
2023-02-20 14:59       ` Prototype 'GOMP_enable_pinned_mode' (was: [PATCH 08/17] openmp: -foffload-memory=pinned) Thomas Schwinge
2022-07-07 10:34 ` [PATCH 09/17] openmp: Use libgomp memory allocation functions with unified shared memory Andrew Stubbs
2022-07-07 10:34 ` [PATCH 10/17] Add parsing support for allocate directive (OpenMP 5.0) Andrew Stubbs
2022-07-07 10:34 ` [PATCH 11/17] Translate " Andrew Stubbs
2022-07-07 10:34 ` [PATCH 12/17] Handle cleanup of omp allocated variables " Andrew Stubbs
2022-07-07 10:34 ` [PATCH 13/17] Gimplify allocate directive " Andrew Stubbs
2022-07-07 10:34 ` [PATCH 14/17] Lower " Andrew Stubbs
2022-07-07 10:34 ` [PATCH 15/17] amdgcn: Support XNACK mode Andrew Stubbs
2022-07-07 10:34 ` [PATCH 16/17] amdgcn, openmp: Auto-detect USM mode and set HSA_XNACK Andrew Stubbs
2022-07-07 10:34 ` [PATCH 17/17] amdgcn: libgomp plugin USM implementation Andrew Stubbs

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=2d973ffb-6e31-b8c6-f32f-86fac369c6eb@codesourcery.com \
    --to=ams@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=tobias@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).