From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by sourceware.org (Postfix) with ESMTP id 734823857C6C for ; Tue, 8 Dec 2020 17:56:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 734823857C6C Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-41-pRs6QDKCOcaN7a1rZFoi6A-1; Tue, 08 Dec 2020 12:56:46 -0500 X-MC-Unique: pRs6QDKCOcaN7a1rZFoi6A-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 5CCC5107ACF5; Tue, 8 Dec 2020 17:56:45 +0000 (UTC) Received: from tucnak.zalov.cz (ovpn-112-11.ams2.redhat.com [10.36.112.11]) by smtp.corp.redhat.com (Postfix) with ESMTPS id EA1305D9DD; Tue, 8 Dec 2020 17:56:44 +0000 (UTC) Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.16.1/8.16.1) with ESMTPS id 0B8Hufkm500491 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NOT); Tue, 8 Dec 2020 18:56:42 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.16.1/8.16.1/Submit) id 0B8HueZ1500490; Tue, 8 Dec 2020 18:56:40 +0100 Date: Tue, 8 Dec 2020 18:56:40 +0100 From: Jakub Jelinek To: Tobias Burnus Cc: gcc-patches Subject: Re: [Patch] OpenMP: C/C++ parse 'omp allocate' Message-ID: <20201208175640.GI3788@tucnak> Reply-To: Jakub Jelinek References: MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-5.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: Tue, 08 Dec 2020 17:56:49 -0000 On Mon, Nov 23, 2020 at 03:50:33PM +0100, Tobias Burnus wrote: > Given that (at least for C/C++) there is some initial support for > OpenMP 5.0's allocators, it is likely that users will try it. Sadly at least the current implementation doesn't offer much benefits; I meant to add e.g. HBM support through dlopening of the memkind library, but I haven't found a box with hw I could test it on. Something that could be done is the memory pinning, we can use mlock for that at least on Linux, the question is how to handle small allocations. Larger allocations (say 2 or more pages) we could just round to number of pages with page alignment and mlock that part and munlock on freeing. Also, we should do something smarter for the NVPTX and AMDGCN offloading targets for the allocators, perhaps handle omp_alloc etc. with some constant allocator arguments directly through PTX etc. directives. > Also the release notes state: "the allocator routines of OpenMP 5.0, > including initial|allocate| clause support in C/C++." > > The latter does not include the omp allocate directive, still, > it can be expected that users will try: > > #pragma omp allocate(...) > > And that will fail at runtime. I think that's undesirable, > even if - like any unknown directive - -Wunknown-pragmas > (-Wall) warns about it. > > Thoughts? OK? > > Tobias > > PS: I have not tried to implement restrictions or additions > like 'allocate(a[5])', which is currently rejected. I also I think allocate(a[5]) is not valid, allocate can't mention parts of other variables (array elements, array sections, structure members). > did not check whether there are differences between the clause > ([partially] implemented) and the directive (this patch). I guess your patch is ok, but I should fine time to implement at least the rest of the restrictions; in particular e.g.: A declarative allocate directive must appear in the same scope as the declarations of each of its list items and must follow all such declarations. Check if the current scope is the scope that contains all the vars. Stick the allocator as an artificial attribute to the decls rather than throwing it away. I think we should implement also the 5.1 restriction: A declared variable may appear as a list item in at most one declarative allocate directive in a given compilation unit. because having multiple allocate directives for the same variable is just insane and unspecified what it would do. While the patch tests for C that the allocator has the right type, for C++ (for obvious reasons) it isn't checked, so we need the checking there later from the attributes or so, at least if it is dependent. For static storage vars, we need to verify the allocator is a constant expression, and most likely otherwise just ignore, unless we want to use say PTX etc. directives to allocate stuff in special kinds of memory. For automatic variables, we likely need to handle it during gimplification, that is the last time we can reasonably add the destructors easily (GOMP_free) such that it would be destructed on C++ exceptions, goto out of scope etc. > OpenMP: C/C++ parse 'omp allocate' > > gcc/c-family/ChangeLog: > > * c-pragma.c (omp_pragmas): Add 'allocate'. > * c-pragma.h (enum pragma_kind): Add PRAGMA_OMP_ALLOCATE. > > gcc/c/ChangeLog: > > * c-parser.c (c_parser_omp_allocate): New. > (c_parser_omp_construct): Call it. > > gcc/cp/ChangeLog: > > * parser.c (cp_parser_omp_allocate): New. > (cp_parser_omp_construct, cp_parser_pragma): Call it. > > gcc/testsuite/ChangeLog: > > * c-c++-common/gomp/allocate-5.c: New test. Ok, thanks. Jakub