PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[Basile Starynkevitch]

[-- Attachment #1: Type: text/plain, Size: 1169 bytes --]

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.

It is bootstrapping, but I don't have any test cases ....

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

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: jitlonglongpatch-r223842.diff --]
[-- Type: text/x-patch, Size: 3988 bytes --]

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));
+}
+
+
 /* 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;

Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[David Malcolm]

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

Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[Basile Starynkevitch]

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} ***

Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[David Malcolm]

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
> 

PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[Basile Starynkevitch]

[-- 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);
 }

Re: PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[David Malcolm]

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

Re: PATCH (v2) trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[David Malcolm]

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)

Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[Basile Starynkevitch]

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} ***

Re: PATCH trunk GCCJIT: adding gcc_jit_context_new_rvalue_from_long_long, etc...

[David Malcolm]

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

[PATCH, committed] jit: Add guide for submitting patches to jit docs

[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