From patchwork Mon Dec 11 11:45:27 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 1874461 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=8.43.85.97; helo=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Spg2L0xK6z20Gs for ; Mon, 11 Dec 2023 22:46:50 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2A84B385AE47 for ; Mon, 11 Dec 2023 11:46:48 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from esa4.mentor.iphmx.com (esa4.mentor.iphmx.com [68.232.137.252]) by sourceware.org (Postfix) with ESMTPS id 9F3F1385842A; Mon, 11 Dec 2023 11:45:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9F3F1385842A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=codesourcery.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mentor.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9F3F1385842A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=68.232.137.252 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702295137; cv=none; b=kJhCbxs5CWlf6a/+MkiTLl+i4hSybWgfKQjjsCb04SXc5iEf5p+KaGgoFGKFv2oWv8cz8I6rlYLxqLbj29a3foekI3pcbKxKabxTMswRQ4sypHgtLGD7gGMzUjzbnBZ4KuJMvC/dd66/nFBawf0odNXUrgX5HjKiP17sq7smODI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1702295137; c=relaxed/simple; bh=ItTsJAHwshusrbluV19CxPwjLIvUrVTHjD2RHUGYbBk=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=MQ3aJtCoxgYiGDbhgAsH6+SnQ1O2Lk+O3uoOYk1H/hsGREy7jB4JAa9DYIYz3QX0YTpf4bGjza0H25/TWQEP805cwaqn2APbkbI+yDCFURAtRWhGsLe3J6HMIzBVQKYKMywlDn7rC6pbh6v8KIf9oBD3dlbY1cdrgHlYG4I+ZOw= ARC-Authentication-Results: i=1; server2.sourceware.org X-CSE-ConnectionGUID: Jfo5Li5CRau/va6pwzM8JQ== X-CSE-MsgGUID: cmvyG8QxQ3O+SAhL63JWjA== X-IronPort-AV: E=Sophos;i="6.04,267,1695715200"; d="diff'?scan'208";a="24959189" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa4.mentor.iphmx.com with ESMTP; 11 Dec 2023 03:45:32 -0800 IronPort-SDR: tTm26acWTK/FCp7GGEUJ1zv36pBMTEf1eDXO7aA3oTGTv67k1aJgODy58Nz7X0QDwjgwc43ydR DsFnt2XPARSlTHzD2ecv/K9u84T8i5YXjePAIqVXm3f56CTWrWyujGXlWi08VDaZq1xwn0YzRi vD6vxMuWmOhryB8M4pMmobjEpa+JPrePIlwHl+XjRLqfu5yygzDP95HMUPp+6khsacQrz0RLCm DK0Yh29/MEAWu9FtF763sr03+GwC0MXewQp7Au6k5UGGCl/JZqNgid3vJAv0ZJqd4lvcJyVGhq XIk= Message-ID: <16bc2d0c-7228-43f8-803b-74a980510370@codesourcery.com> Date: Mon, 11 Dec 2023 12:45:27 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: [Patch] OpenMP: Minor '!$omp allocators' cleanup - and still: Re: [patch] OpenMP/Fortran: Implement omp allocators/allocate for ptr/allocatables Content-Language: en-US To: Jakub Jelinek , Thomas Schwinge CC: , References: <60940754-edc6-4110-b7ba-5bed2133bbb6@codesourcery.com> <87fs0bsjwx.fsf@euler.schwinge.homeip.net> From: Tobias Burnus In-Reply-To: X-Originating-IP: [137.202.0.90] X-ClientProxiedBy: svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) To svr-ies-mbx-12.mgc.mentorg.com (139.181.222.12) X-Spam-Status: No, score=-11.3 required=5.0 tests=BAYES_00, GIT_PATCH_0, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Hi Thomas & Jakub, I included a minor cleanup patch - but the rest is really a bunch of RFC. I intent to commit that patch as obvious, unless there are further comments. On 09.12.23 16:14, Jakub Jelinek wrote: > There were 3 reasons to add GOMP_alloc (and 1 for GOMP_free): > 1) when it was added, omp_aligned_alloc was still not exported from the > library because we thought we shouldn't expose 5.1 features until we > have 5.0 implemented (then changed mind) > 2) unline omp_aligned_alloc, GOMP_alloc issues fatal error on allocation > failure > 3) the omp_* functions have omp_allocator_handle_t arguments, which is hard > to provide for builtins (I think this is the only reason for GOMP_free > addition, maybe together with wanting those to be paired) Is this a real issue? GOMP_{alloc,free} take uintptr_t as allocator while omp_* take omp_allocator_handle_t. But that contains a __omp_memspace_handle_t_max__ = __UINTPTR_MAX__ and >= C++ 11 ': __UINTPTR_TYPE__' for C/C++ and omp_allocator_handle_kind = c_intptr_t in Fortran (→ integer(c_intptr_t) = signed variant of uintptr_t). In Fortran, there is an explicit check that the allocator has that kind, which is a check whether it is kind=4 or kind=8, respectively, depending what kind matches intptr_t. Thus, for Fortran, there is already a mismatch. Thus, it seems to be at most a C/C++ issue. > Now, 1) is a non-issue anymore, I don't know what Fortran wants for > allocation failures, if it is better to have diagnostics on the libgomp side > or if wants to emit it inline. I think that's not quite clear, while for Fortran itself and for OpenMP's omp_alloc routine, the behavior is well defined, it is not for '!$omp allocators'. However, I think using omp_alloc is more Fortranish. I think it would be a tad cleaner to change it – but the question is how to do it best: Even when ignoring the uintptr_t vs. omp_allocator_handle_t issue, the question is still how to handle it best. Create another builtin - to make it possible to add it to gimple-ssa-warn-access.cc / tree-ssa-ccp.cc / tree-ssa-dce.cc? Or the alternative: Be more un-Fortranish and keep the current GOMP_alloc handling? Thoughts? > And yes, 3) would be an argument to add > GOMP_realloc. I am happy to add GOMP_realloc – passing 0 for old/new allocator in case of Fortran - if it really makes sense. Otherwise, I'd keep it as is. I think the next user would be the (pseudo-)USM patches, i.e. replacing all (re,c,m)alloc/free by calls to omp_(re,c,)alloc/omp_free + special allocator when -foffload-memory={none,pinned,unified} (well, not when =none). Those are a bit fragile but permit using pinned/USM on less capable offload devices. (Feature exists also with other compilers, i.e. the users seem to be used to the limitations of this feature.) Thus, one option would be to wait until that feature is ready. What would be the NULL behavior? As omp_alloc or as GOMP_alloc? I assume as omp_alloc would be fine? * * * On 09.12.23 12:19, Thomas Schwinge wrote: > Why not define 'GOMP_add_alloc', 'GOMP_is_alloc' via 'gcc/omp-builtins.def'? We could - but does it make sense to have it for a single caller? > Should this either be 'BUILT_IN_OMP_REALLOC' ('OMP' instead of 'GOMP'), > or otherwise a 'GOMP_realloc' be added to 'libgomp/allocator.c Cf. also above. but I am fine with a name change to OMP_ as well/in addition. > 'GOMP_add_alloc', 'GOMP_is_alloc' should get prototyped in > 'libgomp/libgomp_g.h'. I concur + added. 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 OpenMP: Minor '!$omp allocators' cleanup gcc/fortran/ChangeLog: * trans-openmp.cc (gfc_omp_call_add_alloc, gfc_omp_call_is_alloc): Set 'fn spec'. libgomp/ChangeLog: * libgomp_g.h (GOMP_add_alloc, GOMP_is_alloc): Add. gcc/fortran/trans-openmp.cc | 8 ++++++-- libgomp/libgomp_g.h | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc index 9e166c94f8e..95184920cf7 100644 --- a/gcc/fortran/trans-openmp.cc +++ b/gcc/fortran/trans-openmp.cc @@ -8361,8 +8361,10 @@ gfc_omp_call_add_alloc (tree ptr) if (fn == NULL_TREE) { fn = build_function_type_list (void_type_node, ptr_type_node, NULL_TREE); + tree att = build_tree_list (NULL_TREE, build_string (4, ". R ")); + att = tree_cons (get_identifier ("fn spec"), att, TYPE_ATTRIBUTES (fn)); + fn = build_type_attribute_variant (fn, att); fn = build_fn_decl ("GOMP_add_alloc", fn); -/* FIXME: attributes. */ } return build_call_expr_loc (input_location, fn, 1, ptr); } @@ -8380,7 +8382,9 @@ gfc_omp_call_is_alloc (tree ptr) fn = build_function_type_list (boolean_type_node, ptr_type_node, NULL_TREE); fn = build_fn_decl ("GOMP_is_alloc", fn); -/* FIXME: attributes. */ + tree att = build_tree_list (NULL_TREE, build_string (4, ". R ")); + att = tree_cons (get_identifier ("fn spec"), att, TYPE_ATTRIBUTES (fn)); + fn = build_type_attribute_variant (fn, att); } return build_call_expr_loc (input_location, fn, 1, ptr); } diff --git a/libgomp/libgomp_g.h b/libgomp/libgomp_g.h index 95046312ae9..ec619f255f2 100644 --- a/libgomp/libgomp_g.h +++ b/libgomp/libgomp_g.h @@ -366,6 +366,9 @@ extern void GOMP_teams_reg (void (*) (void *), void *, unsigned, unsigned, /* allocator.c */ +extern void GOMP_add_alloc (void *); +extern bool GOMP_is_alloc (void *); + extern void *GOMP_alloc (size_t, size_t, uintptr_t); extern void GOMP_free (void *, uintptr_t);