PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
David Malcolm
dmalcolm@redhat.com
Wed Jul 15 19:04:00 GMT 2015
On Wed, 2015-07-15 at 20:19 +0200, Basile Starynkevitch wrote:
> Hello All and David Malcolm
>
> The attached patch (relative to trunk r224842) is adding
> gcc_jit_context_new_rvalue_from_long_long and similar functions to
> GCCJIT.
Thanks.
[CCing the jit mailing list; please CC patches affecting the jit there.]
Various comments inline throughout.
> It is bootstrapping, but I don't have any test cases ....
You don't need to do a full bootstrap for code that just touches the
"jit" subdirectory.
You can use "make check-jit" to run just the jit test suite. It's
mostly parallelized, so
make -jN check-jit
for some N is worthwhile.
Currently you ought to get about 8000 "PASS" results in jit.sum, and no
failures.
Please add test coverage for the new API entrypoints to
gcc/testsuite/jit.dg/test-constants.c (which is what tests the analogous
pre-existing entrypoints).
You can run just this one testcase by running:
make check-jit RUNTESTFLAGS="-v -v -v jit.exp=test-constants.c"
Aside: this isn't the case here, but if you were adding an entirely new
testcase, here are some notes: jit.exp expects jit testcases to begin
with "test-" or "test-error-" (for an testcase that generates an error
on a gcc_jit_context). New testcases that don't generate errors should
ideally be added to the "testcases" array in
testsuite/jit.dg/all-non-failing-tests.h; this means that, in addition
to being run standalone, they also get run within test-combination.c
(which runs all successful tests inside one big gcc_jit_context), and
test-threads.c (which runs all successful tests in one process, each one
running in a different thread on a different gcc_jit-context).
> ## gcc/jit/ChangeLog entry:
>
> 2015-07-15 Basile Starynkevitch <basile@starynkevitch.net>
>
> * libgccjit.h (gcc_jit_context_new_rvalue_from_long_long)
> (gcc_jit_context_new_rvalue_from_int32)
> (gcc_jit_context_new_rvalue_from_int64)
> (gcc_jit_context_new_rvalue_from_intptr): New function
> declarations.
>
> * libgccjit.map: New entries for above functions.
>
> * libgccjit.c (gcc_jit_context_new_rvalue_from_long_long)
> (gcc_jit_context_new_rvalue_from_int32)
> (gcc_jit_context_new_rvalue_from_int64)
> (gcc_jit_context_new_rvalue_from_intptr): New functions.
>
> ###
>
> Comments are welcome. Ok for trunk?
> see https://gcc.gnu.org/ml/jit/2015-q3/msg00085.html
Note that these are *host* types; the target type is expressed by the
(gcc_jit_type *) parameter.
Do we actually need all of them? I suspect that these:
gcc_jit_context_new_rvalue_from_long_long
gcc_jit_context_new_rvalue_from_unsigned_long_long
ought to suffice, assuming we can guarantee that
sizeof (long long) >= sizeof (int64)
and
sizeof (long long) >= sizeof (intptr_t)
on every host that we care about.
[snip]
> Index: gcc/jit/libgccjit.c
> ===================================================================
> --- gcc/jit/libgccjit.c (revision 225842)
> +++ gcc/jit/libgccjit.c (working copy)
> @@ -1154,6 +1154,70 @@ gcc_jit_context_new_rvalue_from_long
> (gcc_jit_cont
> ->new_rvalue_from_const <long> (numeric_type, value));
> }
>
> +/* Public entrypoint. See description in libgccjit.h. */
> +
> +gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + long long value)
> +{
> + RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
> + JIT_LOG_FUNC (ctxt->get_logger ());
> + RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
> +
> + return ((gcc_jit_rvalue *)ctxt
> + ->new_rvalue_from_const <long long> (numeric_type, value));
> +}
> +
> +
> +/* Public entrypoint. See description in libgccjit.h. */
> +
> +gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_int32 (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + int32_t value)
> +{
> + RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
> + JIT_LOG_FUNC (ctxt->get_logger ());
> + RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
> +
> + return ((gcc_jit_rvalue *)ctxt
> + ->new_rvalue_from_const <int32_t> (numeric_type, value));
> +}
> +
> +
> +/* Public entrypoint. See description in libgccjit.h. */
> +
> +gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_int64 (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + int64_t value)
> +{
> + RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
> + JIT_LOG_FUNC (ctxt->get_logger ());
> + RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
> +
> + return ((gcc_jit_rvalue *)ctxt
> + ->new_rvalue_from_const <int64_t> (numeric_type, value));
> +}
> +
> +
> +/* Public entrypoint. See description in libgccjit.h. */
> +
> +gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_intptr (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + intptr_t value)
> +{
> + RETURN_NULL_IF_FAIL (ctxt, NULL, NULL, "NULL context");
> + JIT_LOG_FUNC (ctxt->get_logger ());
> + RETURN_NULL_IF_FAIL_NONNULL_NUMERIC_TYPE (ctxt, numeric_type);
> +
> + return ((gcc_jit_rvalue *)ctxt
> + ->new_rvalue_from_const <intptr_t> (numeric_type, value));
> +}
Does this actually link and run? This appears to be missing some
implementations of the template specializations in jit/jit-recording.c
for the new specializations of new_rvalue_from_const.
If these are missing, I'd expect to see a linker error at run-time when
attempting to run client code that links against such a libgccjit.so.
> /* Public entrypoint. See description in libgccjit.h.
>
> This is essentially equivalent to:
> Index: gcc/jit/libgccjit.h
> ===================================================================
> --- gcc/jit/libgccjit.h (revision 225842)
> +++ gcc/jit/libgccjit.h (working copy)
> @@ -752,6 +752,26 @@ gcc_jit_context_new_rvalue_from_long
> (gcc_jit_cont
> long value);
>
> extern gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + long long value);
> +
> +extern gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_int32 (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + int32_t value);
> +
> +extern gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_int64 (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + int64_t value);
> +
> +extern gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_intptr (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + intptr_t value);
> +
> +extern gcc_jit_rvalue *
> gcc_jit_context_zero (gcc_jit_context *ctxt,
> gcc_jit_type *numeric_type);
>
> Index: gcc/jit/libgccjit.map
> ===================================================================
> --- gcc/jit/libgccjit.map (revision 225842)
> +++ gcc/jit/libgccjit.map (working copy)
> @@ -61,7 +61,10 @@ LIBGCCJIT_ABI_0
> gcc_jit_context_new_param;
> gcc_jit_context_new_rvalue_from_double;
> gcc_jit_context_new_rvalue_from_int;
> + gcc_jit_context_new_rvalue_from_int32;
> + gcc_jit_context_new_rvalue_from_int64;
> gcc_jit_context_new_rvalue_from_long;
> + gcc_jit_context_new_rvalue_from_long_long;
> gcc_jit_context_new_rvalue_from_ptr;
> gcc_jit_context_new_string_literal;
> gcc_jit_context_new_struct_type;
Please create a new ABI tag for the new symbols (probably
"LIBGCCJIT_ABI_4") and put them in there. See the notes at:
https://gcc.gnu.org/onlinedocs/jit/topics/compatibility.html
I realize we don't yet have a checklist for what a patch to add a new
libgccjit API entrypoint ought to contain, so here goes:
* the new entrypoints themselves (in libgccjit.h and .c, along with
relevant support code in jit-recording.[ch] and jit-playback.[ch] as
appropriate.
* a new ABI tag containing the new symbols (in libgccjit.map), so that
we can detect client code that uses them
* test cases
* dump_to_reproducer support (most testcases attempt to dump their
contexts to a .c file and then sanity-check the generated c by compiling
them, though not running them; see jit.exp). A new API entrypoint
needs to "know" how to write itself back out to C (by implementing
gcc::jit::recording::memento::write_reproducer for the appropriate
memento subclass).
* C++ bindings for the new entrypoints (see libgccjit++.h); ideally with
test coverage, though the C++ API test coverage is admittedly spotty at
the moment
* documentation for the new C entrypoint
* documentation for the new C++ entrypoint
* documentation for the new ABI tag (see topics/compatibility.rst).
(the length of this list is one reason why I'm somewhat hesitant about
adding new API entrypoints)
Typically a patch that touches the .rst documentation will also need the
texinfo to be regenerated. You can do this via running "make texinfo"
within SRCDIR/gcc/jit/docs. Don't do this within the patch sent to the
mailing list; it can often be relatively large and inconsequential (e.g.
anchor renumbering), rather like generated "configure" changes from
configure.ac. I regenerate it when committing to svn.
Thanks
Dave
More information about the Gcc-patches
mailing list