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 <andrea.cora...@arm.com>
>
> * 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 <andrea.cora...@arm.com>
>
> * jit.dg/all-non-failing-tests.h: Add test-blob.c.
>
> * jit.dg/test-blob.c: New test.
>
