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

[David Malcolm <dmalcolm@redhat.com>]

On Wed, 2020-07-01 at 18:14 +0200, Andrea Corallo wrote:
> Andrea Corallo <andrea.corallo@arm.com> 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