From: Antoni Boucher <bouanto@zoho.com>
To: David Malcolm <dmalcolm@redhat.com>
Cc: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org
Subject: Re: [PATCH] libgccjit: add some reflection functions in the jit C api [PR96889]
Date: Fri, 2 Oct 2020 18:39:56 -0400 [thread overview]
Message-ID: <20201002223956.qj6eolrgnxx4lbop@bouanto-desktop.localdomain> (raw)
In-Reply-To: <9cd00fe5cb5c03184e3a5bd939447b30450a8ca7.camel@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 3967 bytes --]
Hi.
Thanks for the review.
I attached the updated patch file.
I don't have a copyright assignment, but I'll look at that.
On Fri, Oct 02, 2020 at 04:24:26PM -0400, David Malcolm wrote:
>On Fri, 2020-10-02 at 16:17 -0400, David Malcolm wrote:
>> On Tue, 2020-09-01 at 21:01 -0400, Antoni Boucher via Jit wrote:
>> > Hello.
>> > This WIP patch implements new reflection functions in the C API as
>> > mentioned in bug 96889.
>> > I'm looking forward for feedbacks on this patch.
>> > It's WIP because I'll probably add a few more reflection functions.
>> > Thanks.
>>
>> Sorry about the belated review, looks like I missed this one.
>>
>> At a high level, it seems reasonable.
>>
>> Do you have a copyright assignment in place for GCC contributions?
>> See https://gcc.gnu.org/contribute.html
>>
>> [...]
>
>diff --git a/gcc/jit/docs/topics/compatibility.rst
>> > b/gcc/jit/docs/topics/compatibility.rst
>> > index bb3387fa583..7e786194ded 100644
>> > --- a/gcc/jit/docs/topics/compatibility.rst
>> > +++ b/gcc/jit/docs/topics/compatibility.rst
>> > @@ -219,3 +219,14 @@ entrypoints:
>> > * :func:`gcc_jit_version_minor`
>> >
>> > * :func:`gcc_jit_version_patchlevel`
>> > +
>> > +.. _LIBGCCJIT_ABI_14:
>> > +
>> > +``LIBGCCJIT_ABI_14``
>> > +--------------------
>> > +``LIBGCCJIT_ABI_14`` covers the addition of reflection functions
>> > via API
>> > +entrypoints:
>> > +
>> > + * :func:`gcc_jit_function_get_return_type`
>> > +
>> > + * :func:`gcc_jit_function_get_param_count`
>>
>> This will now need bumping to 15; 14 covers the addition of
>> gcc_jit_global_set_initializer.
>>
>> [...]
>>
>> > +/* Public entrypoint. See description in libgccjit.h.
>> > +
>> > + After error-checking, the real work is done by the
>> > + gcc::jit::recording::function::get_return_type method, in
>> > + jit-recording.h. */
>> > +
>> > +gcc_jit_type *
>> > +gcc_jit_function_get_return_type (gcc_jit_function *func)
>> > +{
>>
>> This one is missing a:
>> RETURN_NULL_IF_FAIL (func, NULL, NULL, "NULL function");
>>
>>
>> > + return (gcc_jit_type *)func->get_return_type ();
>> > +}
>>
>> [...]
>>
>> > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
>> > index 1c5a12e9c01..6999ce25ca2 100644
>> > --- a/gcc/jit/libgccjit.h
>> > +++ b/gcc/jit/libgccjit.h
>>
>> [...]
>>
>> > @@ -1503,6 +1511,22 @@ gcc_jit_version_minor (void);
>> > extern int
>> > gcc_jit_version_patchlevel (void);
>> >
>> > +#define LIBGCCJIT_HAVE_gcc_jit_function_reflection
>> > +
>> > +/* Reflection functions to get the number of parameters and return
>> > types of
>> > + a function from the C API.
>>
>> "return type", is better grammar, I think, given that "function" is
>> singular.
>>
>> > +
>> > + "vec_type" should be a vector type, created using
>> > gcc_jit_type_get_vector.
>>
>> This line about "vec_type" seems to be a leftover from a copy&paste.
>>
>>
>> > + This API entrypoint was added in LIBGCCJIT_ABI_14; you can test
>> > for its
>> > + presence using
>> > + #ifdef LIBGCCJIT_HAVE_gcc_jit_function_reflection
>>
>> Version number will need bumping, as mentioned above.
>>
>> [...]
>>
>> > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
>> > index 6137dd4b4b0..b28f81a7a32 100644
>> > --- a/gcc/jit/libgccjit.map
>> > +++ b/gcc/jit/libgccjit.map
>> > @@ -186,4 +186,10 @@ LIBGCCJIT_ABI_13 {
>> > gcc_jit_version_major;
>> > gcc_jit_version_minor;
>> > gcc_jit_version_patchlevel;
>> > -} LIBGCCJIT_ABI_12;
>> > \ No newline at end of file
>> > +} LIBGCCJIT_ABI_12;
>> > +
>> > +LIBGCCJIT_ABI_14 {
>> > + global:
>> > + gcc_jit_function_get_return_type;
>> > + gcc_jit_function_get_param_count;
>> > +} LIBGCCJIT_ABI_13;
>>
>> Likewise.
>>
>> [...]
>>
>> Otherwise looks good to me.
>>
>> Bonus points for adding C++ bindings (and docs for them), but I don't
>> know of anyone using the C++ bindings.
>
>
>Also, please add "PR jit/96889" to the ChangeLog entries, and [PR96889]
>to the subject line.
>
>Dave
>
[-- Attachment #2: 0001-This-patch-add-some-reflection-functions-in-the-jit-.patch --]
[-- Type: text/plain, Size: 7546 bytes --]
From ef3b7d15622cc50dc4cae62fb7c31aeacc0f1ed9 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Sat, 1 Aug 2020 17:52:17 -0400
Subject: [PATCH] This patch add some reflection functions in the jit C api
[PR96889]
2020-09-1 Antoni Boucher <bouanto@zoho.com>
gcc/jit/
PR target/96889
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.
* docs/topics/functions.rst: Add documentation for the
functions gcc_jit_function_get_return_type and
gcc_jit_function_get_param_count
* libgccjit.c (gcc_jit_function_get_param_count and
gcc_jit_function_get_return_type): New functions.
* libgccjit.h
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
gcc/testsuite/
PR target/96889
* jit.dg/all-non-failing-tests.h: Add test-reflection.c.
* jit.dg/test-reflection.c: New test.
---
gcc/jit/docs/topics/compatibility.rst | 11 ++++++++
gcc/jit/docs/topics/functions.rst | 10 +++++++
gcc/jit/libgccjit.c | 29 ++++++++++++++++++++
gcc/jit/libgccjit.h | 22 +++++++++++++++
gcc/jit/libgccjit.map | 6 ++++
gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++++++
gcc/testsuite/jit.dg/test-reflection.c | 27 ++++++++++++++++++
7 files changed, 115 insertions(+)
create mode 100644 gcc/testsuite/jit.dg/test-reflection.c
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 6bfa101ed71..eb6acf91c44 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -226,3 +226,14 @@ entrypoints:
--------------------
``LIBGCCJIT_ABI_14`` covers the addition of
:func:`gcc_jit_global_set_initializer`
+
+.. _LIBGCCJIT_ABI_15:
+
+``LIBGCCJIT_ABI_15``
+--------------------
+``LIBGCCJIT_ABI_15`` covers the addition of reflection functions via API
+entrypoints:
+
+ * :func:`gcc_jit_function_get_return_type`
+
+ * :func:`gcc_jit_function_get_param_count`
diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/functions.rst
index eb40d64010e..9819c28cda2 100644
--- a/gcc/jit/docs/topics/functions.rst
+++ b/gcc/jit/docs/topics/functions.rst
@@ -171,6 +171,16 @@ Functions
underlying string, so it is valid to pass in a pointer to an on-stack
buffer.
+.. function:: size_t \
+ gcc_jit_function_get_param_count (gcc_jit_function *func)
+
+ Get the number of parameters of the function.
+
+.. function:: gcc_jit_type \*
+ gcc_jit_function_get_return_type (gcc_jit_function *func)
+
+ Get the return type of the function.
+
Blocks
------
.. type:: gcc_jit_block
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index a00aefc7108..352e1b84f3c 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -1012,6 +1012,35 @@ gcc_jit_function_get_param (gcc_jit_function *func, int index)
return static_cast <gcc_jit_param *> (func->get_param (index));
}
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::function::get_params method, in
+ jit-recording.h.
+ */
+
+size_t
+gcc_jit_function_get_param_count (gcc_jit_function *func)
+{
+ RETURN_NULL_IF_FAIL (func, NULL, NULL, "NULL function");
+ gcc::jit::recording::context *ctxt = func->m_ctxt;
+ JIT_LOG_FUNC (ctxt->get_logger ());
+ return func->get_params ().length ();
+}
+
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::function::get_return_type method, in
+ jit-recording.h. */
+
+gcc_jit_type *
+gcc_jit_function_get_return_type (gcc_jit_function *func)
+{
+ RETURN_NULL_IF_FAIL (func, NULL, NULL, "NULL function");
+ return (gcc_jit_type *)func->get_return_type ();
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 7134841bb07..38c04a2356f 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -740,6 +740,14 @@ gcc_jit_function_as_object (gcc_jit_function *func);
extern gcc_jit_param *
gcc_jit_function_get_param (gcc_jit_function *func, int index);
+/* Get the number of params of a function. */
+extern size_t
+gcc_jit_function_get_param_count (gcc_jit_function *func);
+
+/* Get the return type of a function. */
+extern gcc_jit_type *
+gcc_jit_function_get_return_type (gcc_jit_function *func);
+
/* Emit the function in graphviz format. */
extern void
gcc_jit_function_dump_to_dot (gcc_jit_function *func,
@@ -1518,6 +1526,20 @@ gcc_jit_version_minor (void);
extern int
gcc_jit_version_patchlevel (void);
+#define LIBGCCJIT_HAVE_gcc_jit_function_reflection
+
+/* Reflection functions to get the number of parameters and return type of
+ a function from the C API.
+
+ This API entrypoint was added in LIBGCCJIT_ABI_15; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_function_reflection
+*/
+extern gcc_jit_type *
+gcc_jit_function_get_return_type (gcc_jit_function *func);
+extern size_t
+gcc_jit_function_get_param_count (gcc_jit_function *func);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index a6e67e781a4..ce12fae626f 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -192,3 +192,9 @@ LIBGCCJIT_ABI_14 {
global:
gcc_jit_global_set_initializer;
} LIBGCCJIT_ABI_13;
+
+LIBGCCJIT_ABI_15 {
+ global:
+ gcc_jit_function_get_return_type;
+ gcc_jit_function_get_param_count;
+} LIBGCCJIT_ABI_14;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 4202eb7798b..a269144d2df 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -258,6 +258,13 @@
#undef create_code
#undef verify_code
+/* test-reflection.c */
+#define create_code create_code_reflection
+#define verify_code verify_code_reflection
+#include "test-reflection.c"
+#undef create_code
+#undef verify_code
+
/* test-string-literal.c */
#define create_code create_code_string_literal
#define verify_code verify_code_string_literal
@@ -424,6 +431,9 @@ const struct testcase testcases[] = {
{"reading_struct ",
create_code_reading_struct ,
verify_code_reading_struct },
+ {"reflection",
+ create_code_reflection ,
+ verify_code_reflection },
{"string_literal",
create_code_string_literal,
verify_code_string_literal},
diff --git a/gcc/testsuite/jit.dg/test-reflection.c b/gcc/testsuite/jit.dg/test-reflection.c
new file mode 100644
index 00000000000..b39e9abab33
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-reflection.c
@@ -0,0 +1,27 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Do nothing. */
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ /* Get the built-in functions. */
+ gcc_jit_function *builtin_sin =
+ gcc_jit_context_get_builtin_function (ctxt, "sin");
+
+ CHECK_VALUE (gcc_jit_function_get_param_count(builtin_sin), 1);
+
+ gcc_jit_type *double_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_DOUBLE);
+ CHECK_VALUE (gcc_jit_function_get_return_type(builtin_sin), double_type);
+}
+
--
2.28.0
next prev parent reply other threads:[~2020-10-02 22:40 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-02 1:01 [PATCH] libgccjit: add some reflection functions in the jit C api Antoni Boucher
2020-10-02 20:17 ` David Malcolm
2020-10-02 20:24 ` David Malcolm
2020-10-02 22:39 ` Antoni Boucher [this message]
2020-10-03 18:14 ` [PATCH] libgccjit: add some reflection functions in the jit C api [WIP] Antoni Boucher
2020-10-15 16:02 ` [PATCH] libgccjit: add some reflection functions in the jit C api Antoni Boucher
2020-10-15 16:23 ` Andrea Corallo
2020-10-15 17:39 ` Antoni Boucher
2020-10-15 18:04 ` Andrea Corallo
2020-10-15 21:52 ` David Malcolm
2020-10-17 0:41 ` Antoni Boucher
2020-11-03 22:13 ` Antoni Boucher
2021-05-13 8:33 ` Martin Liška
2021-05-13 21:30 ` David Malcolm
2021-05-14 2:11 ` Antoni Boucher
2021-05-26 0:19 ` Antoni Boucher
2021-05-27 22:19 ` David Malcolm
2021-05-28 1:51 ` Antoni Boucher
2021-06-10 22:41 ` David Malcolm
[not found] ` <bc9e81dc3f0a68d6389c9765b5901a5dbd1dcd71.camel@zoho.com>
[not found] ` <e962387aff72bce1ea29b0fc2cb04b84c26f9855.camel@redhat.com>
2021-06-18 15:55 ` Antoni Boucher
2021-06-18 16:09 ` David Malcolm
2021-06-18 19:41 ` Antoni Boucher
2021-06-18 20:37 ` David Malcolm
2021-07-19 16:10 ` Antoni Boucher
2021-07-29 12:59 ` Antoni Boucher
2021-08-31 12:34 ` Antoni Boucher
2021-09-28 0:53 ` Antoni Boucher
2021-10-13 2:09 ` Antoni Boucher
2021-10-23 21:20 ` SV: " Petter Tomner
2021-11-14 21:30 ` Antoni Boucher
2021-11-20 0:53 ` David Malcolm
2021-11-26 18:03 ` Gerald Pfeifer
2021-11-26 19:51 ` Gerald Pfeifer
2021-11-27 16:09 ` SV: " Petter Tomner
2021-12-02 18:04 ` Gerald Pfeifer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201002223956.qj6eolrgnxx4lbop@bouanto-desktop.localdomain \
--to=bouanto@zoho.com \
--cc=dmalcolm@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jit@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).