From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-2.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 39ABB3851C15 for ; Wed, 3 Jun 2020 15:03:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 39ABB3851C15 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-205-K3AMzfAaPI-xDk3wp9snNw-1; Wed, 03 Jun 2020 11:03:12 -0400 X-MC-Unique: K3AMzfAaPI-xDk3wp9snNw-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id F1D3A80B709; Wed, 3 Jun 2020 15:03:10 +0000 (UTC) Received: from ovpn-112-11.phx2.redhat.com (ovpn-112-11.phx2.redhat.com [10.3.112.11]) by smtp.corp.redhat.com (Postfix) with ESMTP id 39E595C240; Wed, 3 Jun 2020 15:03:10 +0000 (UTC) Message-ID: <59a8d345c642d49281a601278946e087a4bbe3e2.camel@redhat.com> Subject: Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point From: David Malcolm To: Andrea Corallo , gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Cc: nd@arm.com Date: Wed, 03 Jun 2020 11:03:09 -0400 In-Reply-To: References: User-Agent: Evolution 3.32.5 (3.32.5-1.fc30) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.7 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_H4, 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: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Jun 2020 15:03:15 -0000 On Wed, 2020-06-03 at 12:11 +0200, Andrea Corallo wrote: > Hi all, > > I'd like to submit this patch to extend libgccjit to allow for > storing > binary blobs (equivalent to initialized char arrays). > > The use-case is when libgccjit is used as a back-end for dynamic > programming languages. In this case is often necessary to store > serialized objects into the compilation unit as support to the > generated code. > > The proposed entry point is the following: > > gcc_jit_lvalue * > gcc_jit_context_new_blob (gcc_jit_context *ctxt, > gcc_jit_location *loc, > enum gcc_jit_global_kind kind, > const void *ptr, > size_t size, > const char *name); > > This create a special kind of libgccjit global that will be > initialized > with the memory content pointed by 'ptr'. > > I've added a testcase and the regression is clean. I've also tested > it with the libgccjit based Emacs disabling the current workaround we > have > for this. > > Feedback is very welcome. Thanks for the patch. Is this entrypoint only needed for the ahead-of-time use case? If the client code is instead going to do an in-memory compilation, then I believe it can simply build the buffer of data in memory and expose it to the jitted code via gcc_jit_context_new_rvalue_from_ptr. It occurred to me that the entrypoint is combining two things: - creating a global char[] - creating an initializer for that global which got me wondering if we should instead have a way to add initializers for globals. My first thought was something like: gcc_jit_context_new_global_with_initializer which would be like gcc_jit_context_new_global but would have an additional gcc_jit_rvalue *init_value param? The global would have to be of kind GCC_JIT_GLOBAL_EXPORTED or GCC_JIT_GLOBAL_INTERNAL, not GCC_JIT_GLOBAL_IMPORTED. Alternatively, maybe it would be better to have gcc_jit_global_set_initializer (gcc_jit_lvalue *global, gcc_jit_rvalue *init_val); to make the API more flexible. But even if we had this, we'd still need some way to create the rvalue for that initial value. Also, maybe there ought to be a distinction between rvalues that can vary at runtime vs those that can be computed at compile-time (and are thus suitable for use in static initialization). I suspect you may have gone through the same thought process and come up with a simpler approach. (I'm mostly just "thinking out loud" here, sorry if it isn't very coherent). Should it be a "const char[]" rather than just a "char[]"? One specific nit about the patch: in compatibility.rst, there should be a: .. _LIBGCCJIT_ABI_14: label before the heading. Hope this is constructive Dave > Andrea > > gcc/jit/ChangeLog > > 2020-06-02 Andrea Corallo > > * docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI > tag. > > * docs/topics/expressions.rst (Binary Blobs): New section > documenting gcc_jit_context_new_blob. > > * jit-common.h: Document class blob. > > * jit-playback.c (playback::context::global_new_decl) > (playback::context::global_finalize_lvalue): New methods. > (playback::context::new_global): Make use of global_new_decl, > global_finalize_lvalue. > (playback::context::new_blob): New method. > > * jit-playback.h (new_blob, global_new_decl): New method > declarations. > > * jit-recording.c (recording::context::new_blob) > (recording::blob::replay_into) > (recording::global::write_qualifier_to_dump): New methods. > (recording::global::write_to_dump): Use > write_qualifier_to_dump. > (recording::blob::write_to_dump): New method. > > * jit-recording.h (class blob): New class. > (class global): Have m_kind and m_name as protected. > (class global): Remove FINAL qualifier to replay_into and > write_to_dump. > (class global): New method write_qualifier_to_dump decl. > (class context): New method new_blob decl. > > * libgccjit++.h (context::new_blob): New method. > > * libgccjit.c (gcc_jit_lvalue_as_rvalue): New function. > > * libgccjit.h (gcc_jit_context_new_blob): New function > declaration. > (LIBGCCJIT_HAVE_gcc_jit_context_new_blob): New macro. > > * libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag. > > gcc/testsuite/ChangeLog > > 2020-06-02 Andrea Corallo > > * jit.dg/all-non-failing-tests.h: Add test-blob.c. > > * jit.dg/test-blob.c: New test. >