* Re: PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` David Malcolm
@ 2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
1 sibling, 0 replies; 11+ messages in thread
From: Basile Starynkevitch @ 2015-01-01 0:00 UTC (permalink / raw)
To: David Malcolm; +Cc: jit
On 07/17/2015 15:40, David Malcolm wrote:
>
>
> I'd prefer to just have:
>
> gcc_jit_context_new_rvalue_from_long_long
> gcc_jit_context_new_rvalue_from_unsigned_long_long
Ok, working on that. I'm not CC-ing gcc-patches@ because I have only
comments here... Will submit my v3 patch after you've had explained me
things...
>
> and not bother with the int32_t, int64_t, intptr_t.
>
> I think if we're adding "long long", we need "unsigned long long", given
> that there are values of the latter that aren't yet expressible without
> resorting to hacks.
Ok. working on that.
> Note that in:
> gcc_jit_context_new_rvalue_from_SOME_TYPE
> "SOME_TYPE" refers to a *host* type, whereas the gcc_jit_type parameter
> expresses the *target* type.
>
> So arguably we don't need lots of support for different host types: if
> you need to populate an int32_t constant, you can do:
>
> gcc_jit_type *t_int_32_t =
> gcc_jit_context_get_int_type (ctxt,
> sizeof (int32_t), /* int num_bytes */
> 1); /* int is_signed*/
>
>
> to get the target type (assuming sizeof (int32_t) on the host is the
> same as that on the target, which for these types is of course the
> case).
I am not sure to follow you... What make you believe that sizeof(int32_t)
is always the same for host and target? IIRC an hypothetical implementation
with char of 32 bits and sizeof(char) = sizeof(short) = sizeof(int) =
sizeof(long) = 1
and with sizeof(long long) = 2 would be conforming to the C99 standard.
BTW, I don't care about such pathological cases. My usage of GCCJIT is a
purely JIT thing on x86-64
with both target and host being the same architecture & ABI (x86-64 ...)....
>
> To build an rvalue for a specific int constant we can then simply use a
> big enough host type, say your new long long entrypoint:
>
> gcc_jit_rvalue *rval_const =
> gcc_jit_context_new_rvalue_from_long_long (ctxt,
> t_int32_t,
> INT32_MAX);
>
> given that INT32_MAX will fit in a host "long long".
>
> and indeed if you're using the C++ bindings you can get at integer types
> via some template "magic" like this:
>
> gccjit::type t_int_32_t = ctxt.get_int_type <int32_t> ();
>
> and then the rvalue like this:
>
> gccjit::rvalue rval_const = ctxt.new_rvalue (t_int_32_t,
> INT32_MAX);
>
>
> We have e.g.:
>
> extern gcc_jit_rvalue *
> gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
> gcc_jit_type *numeric_type,
> long long value);
>
> and these might be better described as:
>
>
> extern gcc_jit_rvalue *
> gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
> gcc_jit_type *target_numeric_type,
> long long host_value);
>
> I mention it now in the hope that it clarifies things. You don't need
> to actually make that change - that will be for when we add support for
> cross-compilation.
>
>> The patch is passing the test that I have added, on Debian/x86-64.
> Please run the full "make check-jit", not just the new test.
Will do.
>> Comments are welcome, including perhaps an Ok for trunk...
> Thanks for working on this.
>
> The patch isn't ready for trunk yet: I'd prefer you added "long long"
> and "unsigned long long", and dropped the other new types as noted
> above. Also the patch is at least missing documentation and some test
> coverage, plus some additional points noted below.
>
> FWIW I've fleshed out the notes on submitting jit patches somewhat, and
> they're in the docs now, see:
> https://gcc.gnu.org/onlinedocs/jit/internals/index.html#submitting-patches
>
>
>> Index: gcc/jit/jit-playback.c
>> ===================================================================
>> --- gcc/jit/jit-playback.c (revision 225860)
>> +++ gcc/jit/jit-playback.c (working copy)
>> @@ -1,4 +1,5 @@
>> -/* Internals of libgccjit: classes for playing back recorded API
>> calls.
>> +/* This jit-playback.c file is in -*- C++ -*- ...
>> + Internals of libgccjit: classes for playing back recorded API
>> calls.
> Is the format of these headers designed to be picked up by editors, e.g.
> emacs?
Yes, at least for me. But we really need a comment telling that it is a
C++ file.
(My opinion is that *.c extension for C++ code is insane, but I am in
the minority)
I might happen to pretend understanding that .c extension is a must-have
in GCC
(actually I don't believe that), but in new files which are C++ we
should mention that there are C++ code...., please! (at least for those
newbies who might look into that file)
Do you prefer a comment like
/* This jit-playback.c is in C++99. ...
> Please can you use C syntax for long long literals in the debug string
> (see
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66811 ).
Will try my best to do that....
>> /* The get_wide_int specialization for <long>. */
>>
>> template <>
>> @@ -4171,6 +4189,16 @@ memento_of_new_rvalue_from_const
>> <long>::get_wide_
>> return true;
>> }
>>
>> +/* The get_wide_int specialization for <long long>. */
>> +
>> +template <>
>> +bool
>> +memento_of_new_rvalue_from_const <long long>::get_wide_int (wide_int
>> *out) const
>> +{
>> + *out = wi::shwi (m_value, sizeof (m_value) * 8);
The <long> case that was already there contains the 8 as a magical
constant. I have no idea why. You probably wrote that for the existing
memento_of_new_rvalue_from_const <long>::get_wide_int
> Where does this "8" come from? (it looks like a "magic constant").
I have no idea. But the author of the <long> case should know. Ask him :-)
I guess it might be the number of bits in a char....
Cheers
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH, committed] jit: Add guide for submitting patches to jit docs
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc David Malcolm
@ 2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc Basile Starynkevitch
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: David Malcolm @ 2015-01-01 0:00 UTC (permalink / raw)
To: jit, gcc-patches; +Cc: David Malcolm
Committed to trunk as r225905.
gcc/jit/ChangeLog:
* docs/internals/index.rst (Overview of code structure): Add note
that the implementation is in C++, despite the .c extension.
(Submitting patches): New subsection.
* docs/_build/texinfo/libgccjit.texi: Regenerate.
---
gcc/jit/docs/internals/index.rst | 98 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 98 insertions(+)
diff --git a/gcc/jit/docs/internals/index.rst b/gcc/jit/docs/internals/index.rst
index d0852f9..6f28762 100644
--- a/gcc/jit/docs/internals/index.rst
+++ b/gcc/jit/docs/internals/index.rst
@@ -287,6 +287,9 @@ For example:
Overview of code structure
--------------------------
+The library is implemented in C++. The source files have the ``.c``
+extension for legacy reasons.
+
* ``libgccjit.c`` implements the API entrypoints. It performs error
checking, then calls into classes of the gcc::jit::recording namespace
within ``jit-recording.c`` and ``jit-recording.h``.
@@ -335,3 +338,98 @@ should be rejected via additional checking. The checking ideally should
be within the libgccjit API entrypoints in libgccjit.c, since this is as
close as possible to the error; failing that, a good place is within
``recording::context::validate ()`` in jit-recording.c.
+
+Submitting patches
+------------------
+Please read the contribution guidelines for gcc at
+https://gcc.gnu.org/contribute.html.
+
+Patches for the jit should be sent to both the
+gcc-patches@gcc.gnu.org and jit@gcc.gnu.org mailing lists,
+with "jit" and "PATCH" in the Subject line.
+
+You don't need to do a full bootstrap for code that just touches the
+``jit`` and ``testsuite/jit.dg`` subdirectories. However, please run
+``make check-jit`` before submitting the patch, and mention the results
+in your email (along with the host triple that the tests were run on).
+
+A good patch should contain the information listed in the
+gcc contribution guide linked to above; for a ``jit`` patch, the patch
+shold contain:
+
+ * the code itself (for example, a new API entrypoint will typically
+ touch ``libgccjit.h`` and ``.c``, along with support code in
+ ``jit-recording.[ch]`` and ``jit-playback.[ch]`` as appropriate)
+
+ * test coverage
+
+ * documentation for the C API
+
+ * documentation for the C++ API
+
+A patch that adds new API entrypoints should also contain:
+
+ * a feature macro in ``libgccjit.h`` so that client code that doesn't
+ use a "configure" mechanism can still easily detect the presence of
+ the entrypoint. See e.g. ``LIBGCCJIT_HAVE_SWITCH_STATEMENTS`` (for
+ a category of entrypoints) and
+ ``LIBGCCJIT_HAVE_gcc_jit_context_set_bool_allow_unreachable_blocks``
+ (for an individual entrypoint).
+
+ * a new ABI tag containing the new symbols (in ``libgccjit.map``), so
+ that we can detect client code that uses them
+
+ * Support for :c:func:`gcc_jit_context_dump_reproducer_to_file`. Most
+ jit testcases attempt to dump their contexts to a .c file; ``jit.exp``
+ then sanity-checks the generated c by compiling them (though
+ not running them). 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 entrypoints
+
+ * documentation for the new C++ entrypoints
+
+ * documentation for the new ABI tag (see ``topics/compatibility.rst``).
+
+Depending on the patch you can either extend an existing test case, or
+add a new test case. If you add an entirely new testcase: ``jit.exp``
+expects jit testcases to begin with ``test-``, or ``test-error-`` (for a
+testcase that generates an error on a :c:type:`gcc_jit_context`).
+
+Every new testcase that doesn't generate errors should also touch
+``gcc/testsuite/jit.dg/all-non-failing-tests.h``:
+
+ * Testcases that don't generate errors should ideally be added to the
+ ``testcases`` array in that file; 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 :c:type:`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 :c:type:`gcc_jit_context`).
+
+ .. note::
+
+ Given that exported functions within a :c:type:`gcc_jit_context`
+ must have unique names, and most testcases are run within
+ ``test-combination.c``, this means that every jit-compiled test
+ function typically needs a name that's unique across the entire
+ test suite.
+
+ * Testcases that aren't to be added to the ``testcases`` array should
+ instead add a comment to the file clarifying why they're not in that
+ array. See the file for examples.
+
+Typically a patch that touches the .rst documentation will also need the
+texinfo to be regenerated. You can do this with
+`Sphinx 1.0 <http://sphinx-doc.org/>`_ or later by
+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. You can regenerate it when
+committing to svn.
--
1.8.5.3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
[not found] <55A6A421.8060707@starynkevitch.net>
@ 2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` [PATCH, committed] jit: Add guide for submitting patches to jit docs David Malcolm
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: David Malcolm @ 2015-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: gcc-patches, jit
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc David Malcolm
2015-01-01 0:00 ` [PATCH, committed] jit: Add guide for submitting patches to jit docs David Malcolm
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc Basile Starynkevitch
@ 2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` Basile Starynkevitch
3 siblings, 0 replies; 11+ messages in thread
From: Basile Starynkevitch @ 2015-01-01 0:00 UTC (permalink / raw)
To: David Malcolm; +Cc: jit
[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]
On 07/15/2015 20:52, David Malcolm wrote:
> 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.
>
Here is the slightly improved but still incomplete patch against trunk
r225844. I don't send it yet to gcc-patches@ because I know it is incomplete
Perhaps it is useless to send it here, but at least it serves as backup
for me and to show the slow progress
I made so far.
Temporary gcc/jit/ChangeLog entry is
#######
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. Mention in comments that some of them are same as
others.
* libgccjit.map: New entries for above functions in
LIBGCCJIT_ABI_4/
* 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.
* jit-playback.c (new_rvalue_from_const <long long>): New
specialization.
* jit-recording.c (recording::memento_of_new_rvalue_from_const
<long long>::write_reproducer): New.
######
Sorry for this incomplete patch. But your comments (if you have a few
minutes to spend) are stil welcome.
Cheers.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
[-- Attachment #2: jitlonglongpatch-r225844.diff --]
[-- Type: text/x-patch, Size: 7875 bytes --]
Index: gcc/jit/jit-playback.c
===================================================================
--- gcc/jit/jit-playback.c (revision 225844)
+++ gcc/jit/jit-playback.c (working copy)
@@ -573,6 +573,30 @@ new_rvalue_from_const <long> (type *type,
}
}
+
+/* Specialization of making an rvalue from a const, for host <long long>. */
+
+template <>
+rvalue *
+context::
+new_rvalue_from_const <long long> (type *type,
+ long long value)
+{
+ // FIXME: type-checking, or coercion?
+ tree inner_type = type->as_tree ();
+ if (INTEGRAL_TYPE_P (inner_type))
+ {
+ tree inner = build_int_cst (inner_type, value);
+ return new rvalue (this, inner);
+ }
+ else
+ {
+ REAL_VALUE_TYPE real_value;
+ real_from_integer (&real_value, VOIDmode, value, SIGNED);
+ tree inner = build_real (inner_type, real_value);
+ return new rvalue (this, inner);
+ }
+}
/* Specialization of making an rvalue from a const, for host <double>. */
template <>
Index: gcc/jit/jit-recording.c
===================================================================
--- gcc/jit/jit-recording.c (revision 225844)
+++ gcc/jit/jit-recording.c (working copy)
@@ -4072,6 +4072,7 @@ recording::global::write_reproducer (reproducer &r
/* Explicit specialization of the various mementos we're interested in. */
template class recording::memento_of_new_rvalue_from_const <int>;
template class recording::memento_of_new_rvalue_from_const <long>;
+template class recording::memento_of_new_rvalue_from_const <long long>;
template class recording::memento_of_new_rvalue_from_const <double>;
template class recording::memento_of_new_rvalue_from_const <void *>;
@@ -4209,6 +4210,43 @@ recording::memento_of_new_rvalue_from_const <long>
m_value);
}
+
+/* The write_reproducer specialization for <long long>. */
+
+template <>
+void
+recording::memento_of_new_rvalue_from_const <long long>::write_reproducer (reproducer &r)
+{
+ const char *id = r.make_identifier (this, "rvalue");
+
+ /* We have to special-case LONG_LONG_MIN like in the previous specialization
+ and hence we'd get:
+ error: integer constant is so large that it is unsigned [-Werror]
+ Workaround this by writing (LONG_LONG_MIN + 1) - 1. */
+ if (m_value == LONG_LONG_MIN)
+ {
+ r.write (" gcc_jit_rvalue *%s =\n"
+ " gcc_jit_context_new_rvalue_from_long_long (%s, /* gcc_jit_context *ctxt */\n"
+ " %s, /* gcc_jit_type *numeric_type */\n"
+ " %lldLL - 1); /* long long value */\n",
+ id,
+ r.get_identifier (get_context ()),
+ r.get_identifier_as_type (m_type),
+ m_value + 1);;
+ return;
+ }
+
+ r.write (" gcc_jit_rvalue *%s =\n"
+ " gcc_jit_context_new_rvalue_from_long_long (%s, /* gcc_jit_context *ctxt */\n"
+ " %s, /* gcc_jit_type *numeric_type */\n"
+ " %lldLL); /* long long value */\n",
+ id,
+ r.get_identifier (get_context ()),
+ r.get_identifier_as_type (m_type),
+ m_value);
+ }
+
+
/* The make_debug_string specialization for <double>, rendering it as
(TARGET_TYPE)LITERAL
e.g.
Index: gcc/jit/libgccjit.c
===================================================================
--- gcc/jit/libgccjit.c (revision 225844)
+++ 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 <long> (numeric_type, (long) 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 <long long> (numeric_type, (long long) 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 <long long> (numeric_type, (long long) value));
+}
+
+
/* Public entrypoint. See description in libgccjit.h.
This is essentially equivalent to:
Index: gcc/jit/libgccjit.h
===================================================================
--- gcc/jit/libgccjit.h (revision 225844)
+++ gcc/jit/libgccjit.h (working copy)
@@ -752,6 +752,29 @@ 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);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long, but takes an int32_t */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int32 (gcc_jit_context *ctxt,
+ gcc_jit_type *numeric_type,
+ int32_t value);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but takes an int64_t */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int64 (gcc_jit_context *ctxt,
+ gcc_jit_type *numeric_type,
+ int64_t value);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but takes an intptr_t */
+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 225844)
+++ gcc/jit/libgccjit.map (working copy)
@@ -62,6 +62,7 @@ LIBGCCJIT_ABI_0
gcc_jit_context_new_rvalue_from_double;
gcc_jit_context_new_rvalue_from_int;
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;
@@ -128,3 +129,12 @@ LIBGCCJIT_ABI_3 {
gcc_jit_case_as_object;
gcc_jit_context_new_case;
} LIBGCCJIT_ABI_2;
+
+# Add support for long long & int32 & int64 & intptr
+LIBGCCJIT_ABI_4 {
+ global:
+ gcc_jit_context_new_rvalue_from_int32;
+ gcc_jit_context_new_rvalue_from_int64;
+ gcc_jit_context_new_rvalue_from_intptr;
+ gcc_jit_context_new_rvalue_from_long_long;
+} LIBGCCJIT_ABI_3;
\ No newline at end of file
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` Basile Starynkevitch
@ 2015-01-01 0:00 ` David Malcolm
1 sibling, 0 replies; 11+ messages in thread
From: David Malcolm @ 2015-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: gcc-patches, jit
On Fri, 2015-07-17 at 09:40 -0400, David Malcolm wrote:
> On Thu, 2015-07-16 at 11:00 +0200, Basile Starynkevitch wrote:
(snip)
> > +
> > +/* Specialization of making an rvalue from a const, for host <long
> > long>. */
> > +
> > +template <>
> > +rvalue *
> > +context::
> > +new_rvalue_from_const <long long> (type *type,
> > + long long value)
> > +{
> > + // FIXME: type-checking, or coercion?
> > + tree inner_type = type->as_tree ();
> > + if (INTEGRAL_TYPE_P (inner_type))
> > + {
> > + tree inner = build_int_cst (inner_type, value);
> > + return new rvalue (this, inner);
> > + }
> > + else
> > + {
> > + REAL_VALUE_TYPE real_value;
> > + real_from_integer (&real_value, VOIDmode, value, SIGNED);
> > + tree inner = build_real (inner_type, real_value);
> > + return new rvalue (this, inner);
> > + }
> > +}
>
> This is probably out-of-scope for this patch, but seeing this made me
> wonder if we ought to add a validation to the various
> "new_rvalue_from_TYPE" APIs to ensure that the host value will fit in
> the target type, and emit an error on the context if the given host
> value won't fit.
Filed as PR jit/66913 for now. The validation would happen in
libgccjit.c.
(snip)
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc Basile Starynkevitch
@ 2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` PATCH (v2) " Basile Starynkevitch
0 siblings, 1 reply; 11+ messages in thread
From: David Malcolm @ 2015-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: gcc-patches, jit
On Wed, 2015-07-15 at 21:15 +0200, Basile Starynkevitch wrote:
> On 07/15/2015 20:52, David Malcolm wrote:
> > 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.
> > 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.
>
> It does bootstrap (in the GCC sense). I suspect that C++ integral
> promotion or casting rules are enough to have something being linked,
> but probably not what is really needed.
Perhaps, but note that nothing in a regular gcc bootstrap uses
libgccjit, so you *might* still have a latent linking error that shows
up only at run time. Running the jit testsuite is the best way to be
sure.
> And I'm testing that on
> x86-64/Linux where the patch is almost useless.
>
> Thanks for your other comments. I'm trying to understand them and I am
> working on that.
>
> Cheers
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc David Malcolm
` (2 preceding siblings ...)
2015-01-01 0:00 ` PATCH " Basile Starynkevitch
@ 2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
3 siblings, 1 reply; 11+ messages in thread
From: Basile Starynkevitch @ 2015-01-01 0:00 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches, jit
On 07/15/2015 20:52, David Malcolm wrote:
> 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.
> * 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).
I'm sorry, but I can't understand the above comment. Where is the
"Implementation of recording::memento::write_reproducer for longs." I
can't find such comment in jit-recording.c!
BTW, it is really a pity that even a brand new subtree like gcc/jit/,
coded mostly in C++, uses *.c
as the file extension for newly introduced C++ files. There is no legacy
reason to use *.c extensions for new C++ files (as we had for source
files of twenty years of age). I really find that confusing. And no
comment mention that it is C++ not C!
It makes me almost cry :-)
Cheers.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` Basile Starynkevitch
@ 2015-01-01 0:00 ` David Malcolm
0 siblings, 0 replies; 11+ messages in thread
From: David Malcolm @ 2015-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: gcc-patches, jit
On Wed, 2015-07-15 at 21:37 +0200, Basile Starynkevitch wrote:
> On 07/15/2015 20:52, David Malcolm wrote:
> > 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.
>
> > * 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).
>
>
> I'm sorry, but I can't understand the above comment. Where is the
> "Implementation of recording::memento::write_reproducer for longs." I
> can't find such comment in jit-recording.c!
See approx line 4069 of jit-recording.c onwards:
/* The implementation of the various const-handling classes:
gcc::jit::recording::memento_of_new_rvalue_from_const <HOST_TYPE>. */
The specific code you refer to is here (approx line 4176 of
jit-recording.c):
/* The write_reproducer specialization for <long>. */
template <>
void
recording::memento_of_new_rvalue_from_const <long>::write_reproducer (reproducer &r)
{
/* etc */
> BTW, it is really a pity that even a brand new subtree like gcc/jit/,
> coded mostly in C++, uses *.c
> as the file extension for newly introduced C++ files. There is no legacy
> reason to use *.c extensions for new C++ files (as we had for source
> files of twenty years of age). I really find that confusing. And no
> comment mention that it is C++ not C!
> It makes me almost cry :-)
Sorry. I went with *.c for consistency with the rest of the source tree
(and it's somewhat easier to grep that way). I agree that this is
confusing, and merits at least a mention in docs/internals/index.rst.
Dave
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc David Malcolm
2015-01-01 0:00 ` [PATCH, committed] jit: Add guide for submitting patches to jit docs David Malcolm
@ 2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` PATCH " Basile Starynkevitch
2015-01-01 0:00 ` Basile Starynkevitch
3 siblings, 1 reply; 11+ messages in thread
From: Basile Starynkevitch @ 2015-01-01 0:00 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches, jit
On 07/15/2015 20:52, David Malcolm wrote:
> 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.
> 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.
It does bootstrap (in the GCC sense). I suspect that C++ integral
promotion or casting rules are enough to have something being linked,
but probably not what is really needed. And I'm testing that on
x86-64/Linux where the patch is almost useless.
Thanks for your other comments. I'm trying to understand them and I am
working on that.
Cheers
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` PATCH (v2) " Basile Starynkevitch
@ 2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
0 siblings, 2 replies; 11+ messages in thread
From: David Malcolm @ 2015-01-01 0:00 UTC (permalink / raw)
To: Basile Starynkevitch; +Cc: gcc-patches, jit
On Thu, 2015-07-16 at 11:00 +0200, Basile Starynkevitch wrote:
> On 07/15/2015 21:16, David Malcolm wrote:
> > Perhaps, but note that nothing in a regular gcc bootstrap uses
> > libgccjit, so you *might* still have a latent linking error that
> shows
> > up only at run time. Running the jit testsuite is the best way to
> be
> > sure.
> >
> >> And I'm testing that on
> >> x86-64/Linux where the patch is almost useless.
> >>
> >> Thanks for your other comments. I'm trying to understand them and I
> am
> >> working on that.
> >>
> >> Cheers
> >>
>
> Here (attached gcc-jitlonglong-r225860.diff)
> is an improved version of my patch against trunk r225860.
> Thanks to David Malcom for the kind help.
Thanks. Comments inline below thoughout.
> ### gcc/jit/ ChangeLog entry
>
> 2015-07-16 Basile Starynkevitch <basile@starynkevitch.net>
>
> * jit-playback.c: Mention that it is in C++.
> (new_rvalue_from_const <long>): New.
>
> * jit-recording.c: Mention that it is in C++.
> (recording::memento_of_new_rvalue_from_const <long long>): New
> instanciated template.
> (memento_of_new_rvalue_from_const <long
> long>::make_debug_string):
> New specialized function.
> (memento_of_new_rvalue_from_const <long long>::get_wide_int): New
> specialized function.
> (recording::memento_of_new_rvalue_from_const <long
> long>::write_reproducer): Likewise.
>
> * libgccjit.c: Mention that it is in C++.
> (gcc_jit_context_new_rvalue_from_long_long): New function.
> (gcc_jit_context_new_rvalue_from_int32): New function.
> (gcc_jit_context_new_rvalue_from_int64): New function.
> (gcc_jit_context_new_rvalue_from_intptr): New function.
>
> * libgccjit.h: #include <stdint.h>
> (gcc_jit_context_new_rvalue_from_long_long): New declaration.
> In the declarations of the functions below, a short comment
> explains that they are convenience functions.
> (gcc_jit_context_new_rvalue_from_int32): New declaration.
> (gcc_jit_context_new_rvalue_from_int64): New declaration.
> (gcc_jit_context_new_rvalue_from_intptr): New declaration.
>
> * libgccjit.map: Add LIBGCCJIT_ABI_4 for new functions
> e.g. gcc_jit_context_new_rvalue_from_long_long, ....
>
>
> ## gcc/testsuite/ChangeLog entry
> * test-constants.c (make_test_of_long_long_constant): New
> function.
> (make_tests_of_long_long_constants): New.
> (verify_long_long_constants): New.
> (create_code): Call make_tests_of_long_long_constants.
> (verify_code): Call verify_long_long_constants.
>
>
> I have mixed feelings about adding the
> gcc_jit_context_new_rvalue_from_int32
> gcc_jit_context_new_rvalue_from_int64 &
> gcc_jit_context_new_rvalue_from_intptr functions.
> On one hand, their name is very suggestive, and most programmers know
> about <stdint.h>. I should confess that I discovered only recently
> that
> long long is guaranteed by C99 standard to be at least 64 bits (I
> thought that the standard just required that long long is at least as
> big as long).
> On the other hand, we are adding more functions to the ABI, and
> indeed
> the gcc_jit_context_new_rvalue_from_long_long is in principle enough.
> Perhaps we should simply document that for int32_t, int64_t, intptr_t
> types, the GCCJIT user should test the sizeof intptr_t and call the
> appropriate function?
>
> BTW some bytecodes or VMs (in particular the JVM) are hardcoding the
> size of some integers, so dealing explicitly with int32_t & int64_t
> definitely makes sense.
I'd prefer to just have:
gcc_jit_context_new_rvalue_from_long_long
gcc_jit_context_new_rvalue_from_unsigned_long_long
and not bother with the int32_t, int64_t, intptr_t.
I think if we're adding "long long", we need "unsigned long long", given
that there are values of the latter that aren't yet expressible without
resorting to hacks.
Note that in:
gcc_jit_context_new_rvalue_from_SOME_TYPE
"SOME_TYPE" refers to a *host* type, whereas the gcc_jit_type parameter
expresses the *target* type.
So arguably we don't need lots of support for different host types: if
you need to populate an int32_t constant, you can do:
gcc_jit_type *t_int_32_t =
gcc_jit_context_get_int_type (ctxt,
sizeof (int32_t), /* int num_bytes */
1); /* int is_signed*/
to get the target type (assuming sizeof (int32_t) on the host is the
same as that on the target, which for these types is of course the
case).
To build an rvalue for a specific int constant we can then simply use a
big enough host type, say your new long long entrypoint:
gcc_jit_rvalue *rval_const =
gcc_jit_context_new_rvalue_from_long_long (ctxt,
t_int32_t,
INT32_MAX);
given that INT32_MAX will fit in a host "long long".
and indeed if you're using the C++ bindings you can get at integer types
via some template "magic" like this:
gccjit::type t_int_32_t = ctxt.get_int_type <int32_t> ();
and then the rvalue like this:
gccjit::rvalue rval_const = ctxt.new_rvalue (t_int_32_t,
INT32_MAX);
We have e.g.:
extern gcc_jit_rvalue *
gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
gcc_jit_type *numeric_type,
long long value);
and these might be better described as:
extern gcc_jit_rvalue *
gcc_jit_context_new_rvalue_from_long_long (gcc_jit_context *ctxt,
gcc_jit_type *target_numeric_type,
long long host_value);
I mention it now in the hope that it clarifies things. You don't need
to actually make that change - that will be for when we add support for
cross-compilation.
> The patch is passing the test that I have added, on Debian/x86-64.
Please run the full "make check-jit", not just the new test.
> Comments are welcome, including perhaps an Ok for trunk...
Thanks for working on this.
The patch isn't ready for trunk yet: I'd prefer you added "long long"
and "unsigned long long", and dropped the other new types as noted
above. Also the patch is at least missing documentation and some test
coverage, plus some additional points noted below.
FWIW I've fleshed out the notes on submitting jit patches somewhat, and
they're in the docs now, see:
https://gcc.gnu.org/onlinedocs/jit/internals/index.html#submitting-patches
> Index: gcc/jit/jit-playback.c
> ===================================================================
> --- gcc/jit/jit-playback.c (revision 225860)
> +++ gcc/jit/jit-playback.c (working copy)
> @@ -1,4 +1,5 @@
> -/* Internals of libgccjit: classes for playing back recorded API
> calls.
> +/* This jit-playback.c file is in -*- C++ -*- ...
> + Internals of libgccjit: classes for playing back recorded API
> calls.
Is the format of these headers designed to be picked up by editors, e.g.
emacs?
> Copyright (C) 2013-2015 Free Software Foundation, Inc.
> Contributed by David Malcolm <dmalcolm@redhat.com>.
>
> @@ -573,6 +574,30 @@ new_rvalue_from_const <long> (type *type,
> }
> }
>
> +
> +/* Specialization of making an rvalue from a const, for host <long
> long>. */
> +
> +template <>
> +rvalue *
> +context::
> +new_rvalue_from_const <long long> (type *type,
> + long long value)
> +{
> + // FIXME: type-checking, or coercion?
> + tree inner_type = type->as_tree ();
> + if (INTEGRAL_TYPE_P (inner_type))
> + {
> + tree inner = build_int_cst (inner_type, value);
> + return new rvalue (this, inner);
> + }
> + else
> + {
> + REAL_VALUE_TYPE real_value;
> + real_from_integer (&real_value, VOIDmode, value, SIGNED);
> + tree inner = build_real (inner_type, real_value);
> + return new rvalue (this, inner);
> + }
> +}
This is probably out-of-scope for this patch, but seeing this made me
wonder if we ought to add a validation to the various
"new_rvalue_from_TYPE" APIs to ensure that the host value will fit in
the target type, and emit an error on the context if the given host
value won't fit.
> /* Specialization of making an rvalue from a const, for host
> <double>. */
>
> template <>
> Index: gcc/jit/jit-recording.c
> ===================================================================
> --- gcc/jit/jit-recording.c (revision 225860)
> +++ gcc/jit/jit-recording.c (working copy)
> @@ -1,4 +1,5 @@
> -/* Internals of libgccjit: classes for recording calls made to the
> JIT API.
> +/* This jit-recording.c file is in -*- C++ -*- ...
> + Internals of libgccjit: classes for recording calls made to the
> JIT API.
> Copyright (C) 2013-2015 Free Software Foundation, Inc.
> Contributed by David Malcolm <dmalcolm@redhat.com>.
>
> @@ -4072,6 +4073,7 @@ recording::global::write_reproducer (reproducer
> &r
> /* Explicit specialization of the various mementos we're interested
> in. */
> template class recording::memento_of_new_rvalue_from_const <int>;
> template class recording::memento_of_new_rvalue_from_const <long>;
> +template class recording::memento_of_new_rvalue_from_const <long
> long>;
> template class recording::memento_of_new_rvalue_from_const <double>;
> template class recording::memento_of_new_rvalue_from_const <void *>;
>
> @@ -4161,6 +4163,22 @@ memento_of_new_rvalue_from_const
> <long>::make_debu
> m_value);
> }
>
> +
> +/* The make_debug_string specialization for <long long>, rendering it
> as
> + (TARGET_TYPE)LITERAL
> + e.g.
> + "(long long)42". */
> +
> +template <>
> +string *
> +memento_of_new_rvalue_from_const <long long>::make_debug_string ()
> +{
> + return string::from_printf (m_ctxt,
> + "(%s)%lli",
> + m_type->get_debug_string (),
> + m_value);
> +}
Please can you use C syntax for long long literals in the debug string
(see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66811 ).
> /* The get_wide_int specialization for <long>. */
>
> template <>
> @@ -4171,6 +4189,16 @@ memento_of_new_rvalue_from_const
> <long>::get_wide_
> return true;
> }
>
> +/* The get_wide_int specialization for <long long>. */
> +
> +template <>
> +bool
> +memento_of_new_rvalue_from_const <long long>::get_wide_int (wide_int
> *out) const
> +{
> + *out = wi::shwi (m_value, sizeof (m_value) * 8);
Where does this "8" come from? (it looks like a "magic constant").
> + return true;
> +}
> +
> /* The write_reproducer specialization for <long>. */
>
> template <>
> @@ -4209,6 +4237,43 @@ recording::memento_of_new_rvalue_from_const
> <long>
> m_value);
> }
>
> +
> +/* The write_reproducer specialization for <long long>. */
> +
> +template <>
> +void
> +recording::memento_of_new_rvalue_from_const <long
> long>::write_reproducer (reproducer &r)
> +{
> + const char *id = r.make_identifier (this, "rvalue");
> +
> + /* We have to special-case LONG_LONG_MIN like in the previous
> specialization
> + and hence we'd get:
> + error: integer constant is so large that it is unsigned
> [-Werror]
> + Workaround this by writing (LONG_LONG_MIN + 1) - 1. */
> + if (m_value == LONG_LONG_MIN)
> + {
> + r.write (" gcc_jit_rvalue *%s =\n"
> + " gcc_jit_context_new_rvalue_from_long_long (%s, /*
> gcc_jit_context *ctxt */\n"
> + " %s, /*
> gcc_jit_type *numeric_type */\n"
> + " %lldLL -
> 1); /* long long value */\n",
> + id,
> + r.get_identifier (get_context ()),
> + r.get_identifier_as_type (m_type),
> + m_value + 1);;
> + return;
> + }
> +
> + r.write (" gcc_jit_rvalue *%s =\n"
> + " gcc_jit_context_new_rvalue_from_long_long (%s, /*
> gcc_jit_context *ctxt */\n"
> + " %s, /*
> gcc_jit_type *numeric_type */\n"
> + " %lldLL); /* long
> long value */\n",
> + id,
> + r.get_identifier (get_context ()),
> + r.get_identifier_as_type (m_type),
> + m_value);
> + }
> +
> +
> /* The make_debug_string specialization for <double>, rendering it as
> (TARGET_TYPE)LITERAL
> e.g.
> Index: gcc/jit/libgccjit.c
> ===================================================================
> --- gcc/jit/libgccjit.c (revision 225860)
> +++ gcc/jit/libgccjit.c (working copy)
> @@ -1,4 +1,5 @@
> -/* Implementation of the C API; all wrappers into the internal C++
> API
> +/* This libgccjit.c file is in -*- C++ -*- ...
> + Implementation of the C API; all wrappers into the internal C++
> API
> Copyright (C) 2013-2015 Free Software Foundation, Inc.
> Contributed by David Malcolm <dmalcolm@redhat.com>.
>
> @@ -1154,6 +1155,77 @@ 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 <long> (numeric_type, (long)
> value));
> +}
As noted above, I'd prefer to drop these int32/int64 entrypoints in
favor of "unsigned long long", for the reasons discussed above.
> +
> +/* 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 <long long> (numeric_type, (long
> long) 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);
> +
> + if (sizeof(intptr_t) == sizeof(int))
> + return ((gcc_jit_rvalue *)ctxt
> + ->new_rvalue_from_const <int> (numeric_type, (int)
> value));
> + else if (sizeof(intptr_t) == sizeof(long))
> + return ((gcc_jit_rvalue *)ctxt
> + ->new_rvalue_from_const <long> (numeric_type, (long)
> value));
> + else
> + return ((gcc_jit_rvalue *)ctxt
> + ->new_rvalue_from_const <long long> (numeric_type, (long
> long) value));
> +}
> +
> +
> /* Public entrypoint. See description in libgccjit.h.
>
> This is essentially equivalent to:
> Index: gcc/jit/libgccjit.h
> ===================================================================
> --- gcc/jit/libgccjit.h (revision 225860)
> +++ gcc/jit/libgccjit.h (working copy)
> @@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see
> #define LIBGCCJIT_H
>
> #include <stdio.h>
> +#include <stdint.h>
>
> #ifdef __cplusplus
> extern "C" {
> @@ -752,6 +753,32 @@ 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);
> +
> +/* Internally same as gcc_jit_context_new_rvalue_from_long, but takes
> + an int32_t for convenience. */
> +extern gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_int32 (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + int32_t value);
> +
> +/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but
> + takes an int64_t for convenience. */
> +extern gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_int64 (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + int64_t value);
> +
> +/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but
> + takes an intptr_t for convenience. */
> +extern gcc_jit_rvalue *
> +gcc_jit_context_new_rvalue_from_intptr (gcc_jit_context *ctxt,
> + gcc_jit_type *numeric_type,
> + intptr_t value);
Same comments about types as above.
> +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 225860)
> +++ gcc/jit/libgccjit.map (working copy)
> @@ -128,3 +128,12 @@ LIBGCCJIT_ABI_3 {
> gcc_jit_case_as_object;
> gcc_jit_context_new_case;
> } LIBGCCJIT_ABI_2;
> +
> +# Add support for long long & int32 & int64 & intptr
> +LIBGCCJIT_ABI_4 {
> + global:
> + gcc_jit_context_new_rvalue_from_int32;
> + gcc_jit_context_new_rvalue_from_int64;
> + gcc_jit_context_new_rvalue_from_intptr;
> + gcc_jit_context_new_rvalue_from_long_long;
> +} LIBGCCJIT_ABI_3;
Likewise.
> \ No newline at end of file
> Index: gcc/testsuite/jit.dg/test-constants.c
> ===================================================================
> --- gcc/testsuite/jit.dg/test-constants.c (revision 225860)
> +++ gcc/testsuite/jit.dg/test-constants.c (working copy)
> @@ -173,6 +173,79 @@ verify_long_constants (gcc_jit_result *result)
> }
>
> /**********************************************************************
> + Tests of gcc_jit_context_new_rvalue_from_long_long.
> +
> **********************************************************************/
> +
> +static const char *
> +make_test_of_long_long_constant (gcc_jit_context *ctxt,
> + gcc_jit_type *type,
> + long long value,
> + const char *funcname)
> +{
> + /* Make a test function of the form:
> + long long funcname (void)
> + {
> + return VALUE;
> + }
> + and return a debug dump of VALUE so that
> + the caller can sanity-check the debug dump implementation.
> + */
> + gcc_jit_rvalue *rvalue =
> + gcc_jit_context_new_rvalue_from_long_long (ctxt, type, value);
> + make_test_of_constant (ctxt, type, rvalue, funcname);
> + return gcc_jit_object_get_debug_string (
> + gcc_jit_rvalue_as_object (rvalue));
> +}
> +
> +static void
> +make_tests_of_long_long_constants (gcc_jit_context *ctxt)
> +{
> + gcc_jit_type *long_long_type =
> + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_LONG_LONG);
> +
> + CHECK_STRING_VALUE
> + (
> + make_test_of_long_long_constant (ctxt,
> + long_long_type,
> + 0,
> + "test_long_long_constant_0"),
> + "(long long)0");
> + make_test_of_long_long_constant (ctxt,
> + long_long_type,
> + LLONG_MAX,
> +
> "test_long_long_constant_LLONG_MAX");
> + make_test_of_long_long_constant (ctxt,
> + long_long_type,
> + LLONG_MIN,
> +
> "test_long_long_constant_LLONG_MIN");
> +}
> +
> +static void
> +verify_long_long_constants (gcc_jit_result *result)
> +{
> + typedef long long (*test_fn) (void);
> +
> + test_fn test_long_long_constant_0 =
> + (test_fn)gcc_jit_result_get_code (result,
> + "test_long_long_constant_0");
> + CHECK_NON_NULL (test_long_long_constant_0);
> + CHECK_VALUE (test_long_long_constant_0 (), 0);
> +
> + test_fn test_long_long_constant_LLONG_MAX =
> + (test_fn)gcc_jit_result_get_code (result,
> +
> "test_long_long_constant_LLONG_MAX");
> + CHECK_NON_NULL (test_long_long_constant_LLONG_MAX);
> + CHECK_VALUE (test_long_long_constant_LLONG_MAX (), LLONG_MAX);
> +
> + test_fn test_long_long_constant_LLONG_MIN =
> + (test_fn)gcc_jit_result_get_code (result,
> +
> "test_long_long_constant_LLONG_MIN");
> + CHECK_NON_NULL (test_long_long_constant_LLONG_MIN);
> + CHECK_VALUE (test_long_long_constant_LLONG_MIN (), LLONG_MIN);
> +}
> +
> +
> +/**********************************************************************
> Tests of gcc_jit_context_new_rvalue_from_double.
>
> **********************************************************************/
>
> @@ -323,6 +396,7 @@ create_code (gcc_jit_context *ctxt, void *user_dat
> {
> make_tests_of_int_constants (ctxt);
> make_tests_of_long_constants (ctxt);
> + make_tests_of_long_long_constants (ctxt);
> make_tests_of_double_constants (ctxt);
> make_tests_of_ptr_constants (ctxt);
> }
> @@ -334,6 +408,7 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result
>
> verify_int_constants (result);
> verify_long_constants (result);
> + verify_long_long_constants (result);
> verify_double_constants (result);
> verify_ptr_constants (result);
> }
Please add test coverage for both new API entrypoints, this only adds it
for gcc_jit_context_new_rvalue_from_long_long (and there wasn't any test
coverage for the int32_t etc entrypoints, but we're dropping those).
The patch also needs:
* to document the new API entrypoints (see
gcc/jit/docs/topics/expressions.rst)
* to document the new ABI tag (see
gcc/jit/docs/topics/compatibility.rst)
* feature macros for the new API entrypoints (e.g.
#define LIBGCCJIT_HAVE_gcc_jit_context_new_rvalue_from_long_long
#define LIBGCCJIT_HAVE_gcc_jit_context_new_rvalue_from_unsigned_long_long
* C++ bindings for the new API entrypoints (and documentation, in
gcc/jit/docs/cp/topics/expressions.rst)
Thanks again
Dave
^ permalink raw reply [flat|nested] 11+ messages in thread
* PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...
2015-01-01 0:00 ` David Malcolm
@ 2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
0 siblings, 1 reply; 11+ messages in thread
From: Basile Starynkevitch @ 2015-01-01 0:00 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches, jit
[-- Attachment #1: Type: text/plain, Size: 3686 bytes --]
On 07/15/2015 21:16, David Malcolm wrote:
> Perhaps, but note that nothing in a regular gcc bootstrap uses
> libgccjit, so you *might* still have a latent linking error that shows
> up only at run time. Running the jit testsuite is the best way to be
> sure.
>
>> And I'm testing that on
>> x86-64/Linux where the patch is almost useless.
>>
>> Thanks for your other comments. I'm trying to understand them and I am
>> working on that.
>>
>> Cheers
>>
Here (attached gcc-jitlonglong-r225860.diff)
is an improved version of my patch against trunk r225860.
Thanks to David Malcom for the kind help.
### gcc/jit/ ChangeLog entry
2015-07-16 Basile Starynkevitch <basile@starynkevitch.net>
* jit-playback.c: Mention that it is in C++.
(new_rvalue_from_const <long>): New.
* jit-recording.c: Mention that it is in C++.
(recording::memento_of_new_rvalue_from_const <long long>): New
instanciated template.
(memento_of_new_rvalue_from_const <long long>::make_debug_string):
New specialized function.
(memento_of_new_rvalue_from_const <long long>::get_wide_int): New
specialized function.
(recording::memento_of_new_rvalue_from_const <long
long>::write_reproducer): Likewise.
* libgccjit.c: Mention that it is in C++.
(gcc_jit_context_new_rvalue_from_long_long): New function.
(gcc_jit_context_new_rvalue_from_int32): New function.
(gcc_jit_context_new_rvalue_from_int64): New function.
(gcc_jit_context_new_rvalue_from_intptr): New function.
* libgccjit.h: #include <stdint.h>
(gcc_jit_context_new_rvalue_from_long_long): New declaration.
In the declarations of the functions below, a short comment
explains that they are convenience functions.
(gcc_jit_context_new_rvalue_from_int32): New declaration.
(gcc_jit_context_new_rvalue_from_int64): New declaration.
(gcc_jit_context_new_rvalue_from_intptr): New declaration.
* libgccjit.map: Add LIBGCCJIT_ABI_4 for new functions
e.g. gcc_jit_context_new_rvalue_from_long_long, ....
## gcc/testsuite/ChangeLog entry
* test-constants.c (make_test_of_long_long_constant): New function.
(make_tests_of_long_long_constants): New.
(verify_long_long_constants): New.
(create_code): Call make_tests_of_long_long_constants.
(verify_code): Call verify_long_long_constants.
I have mixed feelings about adding the
gcc_jit_context_new_rvalue_from_int32
gcc_jit_context_new_rvalue_from_int64 &
gcc_jit_context_new_rvalue_from_intptr functions.
On one hand, their name is very suggestive, and most programmers know
about <stdint.h>. I should confess that I discovered only recently that
long long is guaranteed by C99 standard to be at least 64 bits (I
thought that the standard just required that long long is at least as
big as long).
On the other hand, we are adding more functions to the ABI, and indeed
the gcc_jit_context_new_rvalue_from_long_long is in principle enough.
Perhaps we should simply document that for int32_t, int64_t, intptr_t
types, the GCCJIT user should test the sizeof intptr_t and call the
appropriate function?
BTW some bytecodes or VMs (in particular the JVM) are hardcoding the
size of some integers, so dealing explicitly with int32_t & int64_t
definitely makes sense.
The patch is passing the test that I have added, on Debian/x86-64.
Comments are welcome, including perhaps an Ok for trunk...
Regards.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basile<at>starynkevitch<dot>net mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mine, sont seulement les miennes} ***
[-- Attachment #2: gcc-jitlonglong-r225860.diff --]
[-- Type: text/x-patch, Size: 13373 bytes --]
Index: gcc/jit/jit-playback.c
===================================================================
--- gcc/jit/jit-playback.c (revision 225860)
+++ gcc/jit/jit-playback.c (working copy)
@@ -1,4 +1,5 @@
-/* Internals of libgccjit: classes for playing back recorded API calls.
+/* This jit-playback.c file is in -*- C++ -*- ...
+ Internals of libgccjit: classes for playing back recorded API calls.
Copyright (C) 2013-2015 Free Software Foundation, Inc.
Contributed by David Malcolm <dmalcolm@redhat.com>.
@@ -573,6 +574,30 @@ new_rvalue_from_const <long> (type *type,
}
}
+
+/* Specialization of making an rvalue from a const, for host <long long>. */
+
+template <>
+rvalue *
+context::
+new_rvalue_from_const <long long> (type *type,
+ long long value)
+{
+ // FIXME: type-checking, or coercion?
+ tree inner_type = type->as_tree ();
+ if (INTEGRAL_TYPE_P (inner_type))
+ {
+ tree inner = build_int_cst (inner_type, value);
+ return new rvalue (this, inner);
+ }
+ else
+ {
+ REAL_VALUE_TYPE real_value;
+ real_from_integer (&real_value, VOIDmode, value, SIGNED);
+ tree inner = build_real (inner_type, real_value);
+ return new rvalue (this, inner);
+ }
+}
/* Specialization of making an rvalue from a const, for host <double>. */
template <>
Index: gcc/jit/jit-recording.c
===================================================================
--- gcc/jit/jit-recording.c (revision 225860)
+++ gcc/jit/jit-recording.c (working copy)
@@ -1,4 +1,5 @@
-/* Internals of libgccjit: classes for recording calls made to the JIT API.
+/* This jit-recording.c file is in -*- C++ -*- ...
+ Internals of libgccjit: classes for recording calls made to the JIT API.
Copyright (C) 2013-2015 Free Software Foundation, Inc.
Contributed by David Malcolm <dmalcolm@redhat.com>.
@@ -4072,6 +4073,7 @@ recording::global::write_reproducer (reproducer &r
/* Explicit specialization of the various mementos we're interested in. */
template class recording::memento_of_new_rvalue_from_const <int>;
template class recording::memento_of_new_rvalue_from_const <long>;
+template class recording::memento_of_new_rvalue_from_const <long long>;
template class recording::memento_of_new_rvalue_from_const <double>;
template class recording::memento_of_new_rvalue_from_const <void *>;
@@ -4161,6 +4163,22 @@ memento_of_new_rvalue_from_const <long>::make_debu
m_value);
}
+
+/* The make_debug_string specialization for <long long>, rendering it as
+ (TARGET_TYPE)LITERAL
+ e.g.
+ "(long long)42". */
+
+template <>
+string *
+memento_of_new_rvalue_from_const <long long>::make_debug_string ()
+{
+ return string::from_printf (m_ctxt,
+ "(%s)%lli",
+ m_type->get_debug_string (),
+ m_value);
+}
+
/* The get_wide_int specialization for <long>. */
template <>
@@ -4171,6 +4189,16 @@ memento_of_new_rvalue_from_const <long>::get_wide_
return true;
}
+/* The get_wide_int specialization for <long long>. */
+
+template <>
+bool
+memento_of_new_rvalue_from_const <long long>::get_wide_int (wide_int *out) const
+{
+ *out = wi::shwi (m_value, sizeof (m_value) * 8);
+ return true;
+}
+
/* The write_reproducer specialization for <long>. */
template <>
@@ -4209,6 +4237,43 @@ recording::memento_of_new_rvalue_from_const <long>
m_value);
}
+
+/* The write_reproducer specialization for <long long>. */
+
+template <>
+void
+recording::memento_of_new_rvalue_from_const <long long>::write_reproducer (reproducer &r)
+{
+ const char *id = r.make_identifier (this, "rvalue");
+
+ /* We have to special-case LONG_LONG_MIN like in the previous specialization
+ and hence we'd get:
+ error: integer constant is so large that it is unsigned [-Werror]
+ Workaround this by writing (LONG_LONG_MIN + 1) - 1. */
+ if (m_value == LONG_LONG_MIN)
+ {
+ r.write (" gcc_jit_rvalue *%s =\n"
+ " gcc_jit_context_new_rvalue_from_long_long (%s, /* gcc_jit_context *ctxt */\n"
+ " %s, /* gcc_jit_type *numeric_type */\n"
+ " %lldLL - 1); /* long long value */\n",
+ id,
+ r.get_identifier (get_context ()),
+ r.get_identifier_as_type (m_type),
+ m_value + 1);;
+ return;
+ }
+
+ r.write (" gcc_jit_rvalue *%s =\n"
+ " gcc_jit_context_new_rvalue_from_long_long (%s, /* gcc_jit_context *ctxt */\n"
+ " %s, /* gcc_jit_type *numeric_type */\n"
+ " %lldLL); /* long long value */\n",
+ id,
+ r.get_identifier (get_context ()),
+ r.get_identifier_as_type (m_type),
+ m_value);
+ }
+
+
/* The make_debug_string specialization for <double>, rendering it as
(TARGET_TYPE)LITERAL
e.g.
Index: gcc/jit/libgccjit.c
===================================================================
--- gcc/jit/libgccjit.c (revision 225860)
+++ gcc/jit/libgccjit.c (working copy)
@@ -1,4 +1,5 @@
-/* Implementation of the C API; all wrappers into the internal C++ API
+/* This libgccjit.c file is in -*- C++ -*- ...
+ Implementation of the C API; all wrappers into the internal C++ API
Copyright (C) 2013-2015 Free Software Foundation, Inc.
Contributed by David Malcolm <dmalcolm@redhat.com>.
@@ -1154,6 +1155,77 @@ 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 <long> (numeric_type, (long) 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 <long long> (numeric_type, (long long) 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);
+
+ if (sizeof(intptr_t) == sizeof(int))
+ return ((gcc_jit_rvalue *)ctxt
+ ->new_rvalue_from_const <int> (numeric_type, (int) value));
+ else if (sizeof(intptr_t) == sizeof(long))
+ return ((gcc_jit_rvalue *)ctxt
+ ->new_rvalue_from_const <long> (numeric_type, (long) value));
+ else
+ return ((gcc_jit_rvalue *)ctxt
+ ->new_rvalue_from_const <long long> (numeric_type, (long long) value));
+}
+
+
/* Public entrypoint. See description in libgccjit.h.
This is essentially equivalent to:
Index: gcc/jit/libgccjit.h
===================================================================
--- gcc/jit/libgccjit.h (revision 225860)
+++ gcc/jit/libgccjit.h (working copy)
@@ -21,6 +21,7 @@ along with GCC; see the file COPYING3. If not see
#define LIBGCCJIT_H
#include <stdio.h>
+#include <stdint.h>
#ifdef __cplusplus
extern "C" {
@@ -752,6 +753,32 @@ 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);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long, but takes
+ an int32_t for convenience. */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int32 (gcc_jit_context *ctxt,
+ gcc_jit_type *numeric_type,
+ int32_t value);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but
+ takes an int64_t for convenience. */
+extern gcc_jit_rvalue *
+gcc_jit_context_new_rvalue_from_int64 (gcc_jit_context *ctxt,
+ gcc_jit_type *numeric_type,
+ int64_t value);
+
+/* Internally same as gcc_jit_context_new_rvalue_from_long_long, but
+ takes an intptr_t for convenience. */
+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 225860)
+++ gcc/jit/libgccjit.map (working copy)
@@ -128,3 +128,12 @@ LIBGCCJIT_ABI_3 {
gcc_jit_case_as_object;
gcc_jit_context_new_case;
} LIBGCCJIT_ABI_2;
+
+# Add support for long long & int32 & int64 & intptr
+LIBGCCJIT_ABI_4 {
+ global:
+ gcc_jit_context_new_rvalue_from_int32;
+ gcc_jit_context_new_rvalue_from_int64;
+ gcc_jit_context_new_rvalue_from_intptr;
+ gcc_jit_context_new_rvalue_from_long_long;
+} LIBGCCJIT_ABI_3;
\ No newline at end of file
Index: gcc/testsuite/jit.dg/test-constants.c
===================================================================
--- gcc/testsuite/jit.dg/test-constants.c (revision 225860)
+++ gcc/testsuite/jit.dg/test-constants.c (working copy)
@@ -173,6 +173,79 @@ verify_long_constants (gcc_jit_result *result)
}
/**********************************************************************
+ Tests of gcc_jit_context_new_rvalue_from_long_long.
+ **********************************************************************/
+
+static const char *
+make_test_of_long_long_constant (gcc_jit_context *ctxt,
+ gcc_jit_type *type,
+ long long value,
+ const char *funcname)
+{
+ /* Make a test function of the form:
+ long long funcname (void)
+ {
+ return VALUE;
+ }
+ and return a debug dump of VALUE so that
+ the caller can sanity-check the debug dump implementation.
+ */
+ gcc_jit_rvalue *rvalue =
+ gcc_jit_context_new_rvalue_from_long_long (ctxt, type, value);
+ make_test_of_constant (ctxt, type, rvalue, funcname);
+ return gcc_jit_object_get_debug_string (
+ gcc_jit_rvalue_as_object (rvalue));
+}
+
+static void
+make_tests_of_long_long_constants (gcc_jit_context *ctxt)
+{
+ gcc_jit_type *long_long_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_LONG_LONG);
+
+ CHECK_STRING_VALUE
+ (
+ make_test_of_long_long_constant (ctxt,
+ long_long_type,
+ 0,
+ "test_long_long_constant_0"),
+ "(long long)0");
+ make_test_of_long_long_constant (ctxt,
+ long_long_type,
+ LLONG_MAX,
+ "test_long_long_constant_LLONG_MAX");
+ make_test_of_long_long_constant (ctxt,
+ long_long_type,
+ LLONG_MIN,
+ "test_long_long_constant_LLONG_MIN");
+}
+
+static void
+verify_long_long_constants (gcc_jit_result *result)
+{
+ typedef long long (*test_fn) (void);
+
+ test_fn test_long_long_constant_0 =
+ (test_fn)gcc_jit_result_get_code (result,
+ "test_long_long_constant_0");
+ CHECK_NON_NULL (test_long_long_constant_0);
+ CHECK_VALUE (test_long_long_constant_0 (), 0);
+
+ test_fn test_long_long_constant_LLONG_MAX =
+ (test_fn)gcc_jit_result_get_code (result,
+ "test_long_long_constant_LLONG_MAX");
+ CHECK_NON_NULL (test_long_long_constant_LLONG_MAX);
+ CHECK_VALUE (test_long_long_constant_LLONG_MAX (), LLONG_MAX);
+
+ test_fn test_long_long_constant_LLONG_MIN =
+ (test_fn)gcc_jit_result_get_code (result,
+ "test_long_long_constant_LLONG_MIN");
+ CHECK_NON_NULL (test_long_long_constant_LLONG_MIN);
+ CHECK_VALUE (test_long_long_constant_LLONG_MIN (), LLONG_MIN);
+}
+
+
+/**********************************************************************
Tests of gcc_jit_context_new_rvalue_from_double.
**********************************************************************/
@@ -323,6 +396,7 @@ create_code (gcc_jit_context *ctxt, void *user_dat
{
make_tests_of_int_constants (ctxt);
make_tests_of_long_constants (ctxt);
+ make_tests_of_long_long_constants (ctxt);
make_tests_of_double_constants (ctxt);
make_tests_of_ptr_constants (ctxt);
}
@@ -334,6 +408,7 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result
verify_int_constants (result);
verify_long_constants (result);
+ verify_long_long_constants (result);
verify_double_constants (result);
verify_ptr_constants (result);
}
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-07-17 17:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <55A6A421.8060707@starynkevitch.net>
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc David Malcolm
2015-01-01 0:00 ` [PATCH, committed] jit: Add guide for submitting patches to jit docs David Malcolm
2015-01-01 0:00 ` PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` PATCH (v2) " Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
2015-01-01 0:00 ` PATCH " Basile Starynkevitch
2015-01-01 0:00 ` Basile Starynkevitch
2015-01-01 0:00 ` David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).