From: Tobias Burnus <tobias@codesourcery.com>
To: Andrew Stubbs <ams@codesourcery.com>, <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 08/17] openmp: -foffload-memory=pinned
Date: Fri, 8 Jul 2022 11:00:48 +0200 [thread overview]
Message-ID: <4dba87f8-2375-d6ad-1289-b8de95014b3a@codesourcery.com> (raw)
In-Reply-To: <397f6b84-9d45-379a-5402-76a8ac11f08e@codesourcery.com>
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.
|
(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.)
Tobias
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955
next prev parent reply other threads:[~2022-07-08 9:00 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 [this message]
2022-07-08 9:55 ` Andrew Stubbs
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=4dba87f8-2375-d6ad-1289-b8de95014b3a@codesourcery.com \
--to=tobias@codesourcery.com \
--cc=ams@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).