Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point

[Andrea Corallo <andrea.corallo@arm.com>]

David Malcolm <dmalcolm@redhat.com> writes:

> 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.

Hi Dave,

correct, if you can leak pointers into the generated code this entry
point is likely to be unnecessary.

> 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').

> Should it be a "const char[]" rather than just a "char[]"?

Good question, I believe depends on the usecase so I left out the const.
Perhaps should be a parameter of the entry point.

> One specific nit about the patch: in compatibility.rst, there should be
> a:
>
> .. _LIBGCCJIT_ABI_14:
>
> label before the heading.

Ops

> Hope this is constructive
> Dave

Sure it is.

Thanks

  Andrea