From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from sender4-pp-o91.zoho.com (sender4-pp-o91.zoho.com [136.143.188.91]) by sourceware.org (Postfix) with ESMTPS id B0C5F388187D; Fri, 2 Oct 2020 22:40:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B0C5F388187D ARC-Seal: i=1; a=rsa-sha256; t=1601678399; cv=none; d=zohomail.com; s=zohoarc; b=SyfRznMngswm3JVAXVgXufXFvyCBv4D0mCnj8wm1XVn89ea/EVoDsmm0HVsSpnNUKnFbj1g+vEq82fsYmiGT6vOueQS4XbJNSRR0gI9iTVi2JPLDIGa1X5LFLQSpUiHNZ1pZBpvg0VqCUnhBXqVaMdQmai3SOR3AE3LDzWC1EOc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1601678399; h=Content-Type:Cc:Date:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:To; bh=tGpaPlLSlKZjVMKtNY92EAn5qWvCsAAojv/JS4uXsD4=; b=Y+0LpW5azQSs2iHOcolcdJorTjTQNPSlluE7oUveepkPaTbaBoWwzIYsb+Rf2g1obzRsgGeaMs/UxisTEv3LA1sJnqtdrffI4AxBjRYaKGU1ECT3YXVDBJOJxt5MMN3x75+NU2A5KKRr6E1FRq6jrQ4StamSI/aJiOjGnAANjfw= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=zoho.com; spf=pass smtp.mailfrom=bouanto@zoho.com; dmarc=pass header.from= header.from= Received: from localhost (38.87.1.99 [38.87.1.99]) by mx.zohomail.com with SMTPS id 160167839859563.01493829262495; Fri, 2 Oct 2020 15:39:58 -0700 (PDT) Date: Fri, 2 Oct 2020 18:39:56 -0400 From: Antoni Boucher To: David Malcolm Cc: gcc-patches@gcc.gnu.org, jit@gcc.gnu.org Subject: Re: [PATCH] libgccjit: add some reflection functions in the jit C api [PR96889] Message-ID: <20201002223956.qj6eolrgnxx4lbop@bouanto-desktop.localdomain> References: <20200902010120.crnx55ev635ceiey@bouanto-desktop.localdomain> <6fe2ea355403eb177c9632e076c11c792f23c791.camel@redhat.com> <9cd00fe5cb5c03184e3a5bd939447b30450a8ca7.camel@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="qgfl6eh3dximnaas" Content-Disposition: inline In-Reply-To: <9cd00fe5cb5c03184e3a5bd939447b30450a8ca7.camel@redhat.com> X-Zoho-Virus-Status: 1 X-ZohoMailClient: External X-Spam-Status: No, score=-9.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: jit@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Jit mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 02 Oct 2020 22:40:05 -0000 --qgfl6eh3dximnaas Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline 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 > --qgfl6eh3dximnaas Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0001-This-patch-add-some-reflection-functions-in-the-jit-.patch" >From ef3b7d15622cc50dc4cae62fb7c31aeacc0f1ed9 Mon Sep 17 00:00:00 2001 From: Antoni Boucher 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 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 (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 +#include + +#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 --qgfl6eh3dximnaas--