From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by sourceware.org (Postfix) with ESMTP id 8C3103851C2E for ; Fri, 24 Jul 2020 22:05:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 8C3103851C2E 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-195-TJSM6kVbOWKZTlPvY5swAg-1; Fri, 24 Jul 2020 18:05:45 -0400 X-MC-Unique: TJSM6kVbOWKZTlPvY5swAg-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 109D7800460; Fri, 24 Jul 2020 22:05:44 +0000 (UTC) Received: from ovpn-113-239.phx2.redhat.com (ovpn-113-239.phx2.redhat.com [10.3.113.239]) by smtp.corp.redhat.com (Postfix) with ESMTP id 54EF110013D7; Fri, 24 Jul 2020 22:05:43 +0000 (UTC) Message-ID: <93e3d65a0b04b13a5d5c9970a2058d167357ed6c.camel@redhat.com> Subject: Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point From: David Malcolm To: Andrea Corallo Cc: jit@gcc.gnu.org, nd@arm.com, gcc-patches@gcc.gnu.org Date: Fri, 24 Jul 2020 18:05:42 -0400 In-Reply-To: References: <59a8d345c642d49281a601278946e087a4bbe3e2.camel@redhat.com> <87zh9kumwi.fsf@arm.com> User-Agent: Evolution 3.32.5 (3.32.5-1.fc30) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 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=-4.1 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: 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: Fri, 24 Jul 2020 22:05:51 -0000 On Wed, 2020-07-01 at 18:14 +0200, Andrea Corallo wrote: > Andrea Corallo writes: > > > > 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). > > > > Yes I had kind of similar thoughs. > > > > Ideally would be good to have a generic solution, the complication > > is > > that as you mentioned not every rvalue is suitable for initializing > > every global, but rather the opposite. My fear was that the space > > to be > > covered would be non trivial for a robust solution in this case. > > > > Also I believe we currently have no way to express in libgccjit > > rvalues > > an array with some content, so how to use this as > > initializer? Perhaps > > also we should have a new type gcc_jit_initializer? > > > > On the other hand I thought that for simple things like integers > > the > > problem is tipically already solved with an assignment in some init > > code > > (infact I think so far nobody complained) while the real standing > > limitation is for blobs (perhaps I'm wrong). And in the end if you > > can > > stuff some data in, you can use it later for any scope. > > > > Another "hybrid" solution would be to have specific entry point for > > each > > type of the subset we want to allow for static > > initialization. This way > > we still control the creation of the initializer internally so it's > > safe. In this view this blob entry point would be just one of > > these > > (probably with a different name like > > 'gcc_jit_context_new_glob_init_char_array'). > > > > Hi Dave, > > wanted to ask if you formed an opinion about the patch and/or more in > general the problem of static initialize data. Sorry for the delay in responding. Another idea that occurred to me is, as before, to generalize the entrypoint to cover creating globals with arbitrary types and to support initializers (either with a new gcc_jit_context_new_global_with_initializer or a new: gcc_jit_global_set_initializer as above, but instead of passing in a gcc_jit_rvalue *init_val, to pass in a const void *initializer size_t num_bytes pair pointing to the initializer buffer, with the behavior that these are the bytes that will be used for the initial value of the global, whatever that means for the given type. Something like either: extern gcc_jit_lvalue * gcc_jit_context_new_global_with_initializer (gcc_jit_context *ctxt, gcc_jit_location *loc, enum gcc_jit_global_kind kind, gcc_jit_type *type, const char *name, const void *initializer, size_t num_bytes); or: extern void gcc_jit_global_set_initializer (gcc_jit_lvalue *global, const void *initializer, size_t num_bytes); or somesuch. The former is similar to the proposed new entrypoint from your patch, but gains a type argument (and is modeled more closely on the existing entrypoint for creating globals). I haven't thought this through in detail, and I'm not sure exactly how it would work for arbitrary types, but I thought it worth sharing. (For example I can think of nasty issues if we ever want to support cross-compilation, e.g. where sizeof types or endianness differs between host and target). Maybe we can make it work initially for char[] types, rejecting others, assuming that that's the most pressing need in your work, and we can add support for other types internally as followups without needing to add new entrypoints? Thoughts? Dave