From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from esa3.mentor.iphmx.com (esa3.mentor.iphmx.com [68.232.137.180]) by sourceware.org (Postfix) with ESMTPS id 57B9E3858C2C for ; Fri, 8 Jul 2022 09:55:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 57B9E3858C2C Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com X-IronPort-AV: E=Sophos;i="5.92,255,1650960000"; d="scan'208";a="78403970" Received: from orw-gwy-02-in.mentorg.com ([192.94.38.167]) by esa3.mentor.iphmx.com with ESMTP; 08 Jul 2022 01:55:12 -0800 IronPort-SDR: QhPiBvowRZGel6jQcvx+64IUgqfOCtweKsSUiQnuNwaGby+e1tzvhZmCjv9InwIikHmNP+4BRZ yZW0dctDRPKkykav61afPA+jjoJ3rI4dniIa1vePSm3hb9ZIWCcxppb74HnFPDiLQaY0F2BgR9 ApJX37dXr+kmP92eaRQENb9Z0jkxrT7Wk861IRFMajrk9VZ+ne/McZ9AsprcncwUd7ZzwHvuND 48fbRSwhVH2Ea/VHE/3zuMlPrdyQIjUUYYFDw0LqJMk2VDYDpKxuE5tZKUy+1OJleB5VznXMw5 6fg= Message-ID: <2d973ffb-6e31-b8c6-f32f-86fac369c6eb@codesourcery.com> Date: Fri, 8 Jul 2022 10:55:07 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH 08/17] openmp: -foffload-memory=pinned Content-Language: en-GB To: Tobias Burnus , References: <8011a994bb38db60f37127880b0fc682564f6e8d.1657188329.git.ams@codesourcery.com> <397f6b84-9d45-379a-5402-76a8ac11f08e@codesourcery.com> <4dba87f8-2375-d6ad-1289-b8de95014b3a@codesourcery.com> From: Andrew Stubbs In-Reply-To: <4dba87f8-2375-d6ad-1289-b8de95014b3a@codesourcery.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-09.mgc.mentorg.com (139.181.222.9) To svr-ies-mbx-11.mgc.mentorg.com (139.181.222.11) X-Spam-Status: No, score=-5.5 required=5.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Jul 2022 09:55:18 -0000 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