Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-12-02 Thread Gerald Pfeifer
On Sat, 27 Nov 2021, Petter Tomner wrote:
> Ye it is supposed to compile cleanly for 32bit too. 
> 
> I pushed a patch for it as a "free for all". With %zu specifiers.

Thank you, Petter.  I just updated the lang/gcc12-devel port in FreeBSD
to Sunday's snapshot that has those changes, so we shall learn soon if
there's still some problem (though I expect we'll be good).

Gerald


Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-11-26 Thread Gerald Pfeifer
On Fri, 26 Nov 2021, Gerald Pfeifer wrote:
> I have received a report of GCC builds now failing on FreeBSD/i386:

Ah, and here are the logs (IPv6 required, unfortunately):

Log URL:
http://beefy5.nyi.freebsd.org/data/122i386-default/9e1bda400030/logs/gcc12-devel-12.0.0.s20211121.log
Build URL:
http://beefy5.nyi.freebsd.org/build.html?mastername=122i386-default=9e1bda400030

Gerald


Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-11-26 Thread Gerald Pfeifer
On Fri, 19 Nov 2021, David Malcolm via Gcc-patches wrote:
> On Mon, 2021-09-27 at 20:53 -0400, Antoni Boucher wrote:
>> I fixed an issue (it would show an error message when
>> gcc_jit_type_dyncast_function_ptr_type was called on a type different
>> than a function pointer type).
> The updated patch is good for trunk, assuming you re-ran the regression 
> tests successfully.

I have received a report of GCC builds now failing on FreeBSD/i386:

In function 'gcc_jit_type*
gcc_jit_function_type_get_param_type(gcc_jit_function_type*, size_t)':
/wrkdirs/usr/ports/lang/gcc12-devel/work/gcc-12-20211121/gcc/jit/libgccjit.c:
184:35: error: format '%ld' expects argument of type 'long int', but argument 5 
has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
  184 | jit_error ((CTXT), (LOC), "%s: " ERR_FMT, \
  |   ^
  185 |__func__, (A0), (A1), (A2)); \
  |    
/wrkdirs/usr/ports/lang/gcc12-devel/work/gcc-12-20211121/gcc/jit/libgccjit.c:
230:3: note: in expansion of macro 'RETURN_VAL_IF_FAIL_PRINTF3'
  230 |   RETURN_VAL_IF_FAIL_PRINTF3 (TEST_EXPR, NULL, CTXT, LOC, ERR_FMT, A0, 
A1, A2)
  |   ^~
/wrkdirs/usr/ports/lang/gcc12-devel/work/gcc-12-20211121/gcc/jit/libgccjit.c: 
708:3: note: in expansion of macro 'RETURN_NULL_IF_FAIL_PRINTF3'
  708 |   RETURN_NULL_IF_FAIL_PRINTF3 (index < num_params,
  |   ^~~
/wrkdirs/usr/ports/lang/gcc12-devel/work/gcc-12-20211121/gcc/jit/libgccjit.c: 
710:44: note: format string is defined here
  710 |"index of %ld is too large (%s has %ld 
params)",
  |  ~~^
  ||
  |long int 
  |  %d
/wrkdirs/usr/ports/lang/gcc12-devel/work/gcc-12-20211121/gcc/jit/libgccjit.c:
184:35: error: format '%ld' expects argument of type 'long int', but argument 7 
has type 'size_t' {aka 'unsigned int'} [-Werror=format=]


My regular testers are x86-64 and do not show this, but if I'm right
it should also should on 32-bit GNU/Linux?

Gerald


Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-11-19 Thread David Malcolm via Gcc-patches
On Mon, 2021-09-27 at 20:53 -0400, Antoni Boucher wrote:
> I fixed an issue (it would show an error message when
> gcc_jit_type_dyncast_function_ptr_type was called on a type different
> than a function pointer type).
> 
> Here's the updated patch.

Sorry about the delay in responding.

The updated patch is good for trunk, assuming you re-ran the regression
tests successfully.

Dave





Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-11-14 Thread Antoni Boucher via Gcc-patches
David: PING

Le mardi 12 octobre 2021 à 22:09 -0400, Antoni Boucher a écrit :
> David: PING
> 
> Le lundi 27 septembre 2021 à 20:53 -0400, Antoni Boucher a écrit :
> > I fixed an issue (it would show an error message when
> > gcc_jit_type_dyncast_function_ptr_type was called on a type
> > different
> > than a function pointer type).
> > 
> > Here's the updated patch.
> > 
> > Le vendredi 18 juin 2021 à 16:37 -0400, David Malcolm a écrit :
> > > On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> > > > I have write access now.
> > > 
> > > Great.
> > > 
> > > > I'm not sure how I'm supposed to send my patches:
> > > > should I put it in personal branches and you'll merge them?
> > > 
> > > Please send them to this mailing list for review; once they're
> > > approved
> > > you can merge them.
> > > 
> > > > 
> > > > And for the MAINTAINERS file, should I just push to master
> > > > right
> > > > away,
> > > > after sending it to the mailing list?
> > > 
> > > I think people just push the MAINTAINERS change and then let the
> > > list
> > > know, since it makes a good test that write access is working
> > > correctly.
> > > 
> > > Dave
> > > 
> > > > 
> > > > Thanks for your help!
> > > > 
> > > > Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > > > > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > > > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a
> > > > > > écrit :
> > > > > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > > > > Thank you for your answer.
> > > > > > > > I attached the updated patch.
> > > > > > > 
> > > > > > > BTW you (or possibly me) dropped the mailing lists; was
> > > > > > > that
> > > > > > > deliberate?
> > > > > > 
> > > > > > Oh, my bad.
> > > > > > 
> > > > > 
> > > > > [...]
> > > > > 
> > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > I have signed the FSF copyright attribution.
> > > > > > > 
> > > > > > > I can push changes on your behalf, but I'd prefer it if
> > > > > > > you
> > > > > > > did
> > > > > > > it,
> > > > > > > especially given that you have various other patches you
> > > > > > > want
> > > > > > > to
> > > > > > > get
> > > > > > > in.
> > > > > > > 
> > > > > > > Instructions on how to get push rights to the git repo
> > > > > > > are
> > > > > > > here:
> > > > > > >   https://gcc.gnu.org/gitwrite.html
> > > > > > > 
> > > > > > > I can sponsor you.
> > > > > > 
> > > > > > Thanks.
> > > > > > I did sign up to get push rights.
> > > > > > Have you accepted my request to get those?
> > > > > 
> > > > > I did, but I didn't see any kind of notification.  Did you
> > > > > get
> > > > > an
> > > > > email
> > > > > about it?
> > > > > 
> > > > > 
> > > > > Dave
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> 
> 



SV: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-10-23 Thread Petter Tomner via Gcc-patches
Hi!

+gcc_jit_type *
+gcc_jit_type_unqualified (gcc_jit_type *type)
+{
+  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
+
+  return (gcc_jit_type *)type->unqualified ();
+}

I think there is a problem with the current implementation of unqualified() 
that might be kinda surprising to
users, since it only removes one qualifier and also not "pointed to" qualifiers.

Unrelated to this patch that behavior should probably be documented, since it 
effects many entry-points
and makes cast to base type kinda mandatory when qualified types are mixed. 
Casts uses a recursive
check of types and is happy to cast any pointer to any pointer (no matter the 
level of pointerness) so
they work.

I guess the casts should get some shortcircuit for casts between same base 
types really,
to not bloat the GENERIC tree.

I made a utility function in another patch for comparing types that do full 
stripping (see bellow).

Maybe something like that could be usefull?

jit-recording.h:
+/* Strip all qualifiers and count pointer depth, returning true
+   if the types and pointer depth are the same, otherwise false. */
+static inline bool
+types_kinda_same (recording::type *a, recording::type *b)
+{
+  /* Handle trivial case here, to allow for inlining. */
+  return a == b || types_kinda_same_internal (a, b);
+}
jit-recording.c:
+/* Strip qualifiers and count pointer depth, returning true
+   if the types' base type and pointer depth are
+   the same, otherwise false.
+
+   Do not call this directly. Call 'types_kinda_same' */
+bool
+types_kinda_same_internal (recording::type *a, recording::type *b)
+{
+  int ptr_depth[2] = {};
+  recording::type *base_types[2];
+  recording::type *types[2] = {a, b};
+
+  /* Strip qualifiers and count pointerness */
+  for (int i = 0; i < 2; i++)
+{
+  recording::type *t = types[i];
+  while (true)
+   {
+ if (!t)
+   return false; /* Should only happen on bad input */
+
+ recording::type *pointed_to_type = t->is_pointer();
+ if (pointed_to_type != NULL)
+   {
+ ptr_depth[i]++;
+ t = pointed_to_type;
+ continue;
+   }
+
+ /* unqualified() returns 'this' on base types */
+ recording::type *next = t->unqualified ();
+ if (next == t)
+   {
+ base_types[i] = t;
+ break;
+   }
+ t = next;
+   }
+}
+
+  return base_types[0] == base_types[1] &&
+(ptr_depth[0] == ptr_depth[1]);
+}



Från: Jit  för Antoni Boucher via Jit 

Skickat: den 13 oktober 2021 04:09
Till: David Malcolm
Kopia: Antoni Boucher via Jit; gcc-patches@gcc.gnu.org
Ämne: Re: [PATCH] libgccjit: add some reflection functions in the jit C api
    
David: PING

Le lundi 27 septembre 2021 à 20:53 -0400, Antoni Boucher a écrit :
> I fixed an issue (it would show an error message when
> gcc_jit_type_dyncast_function_ptr_type was called on a type different
> than a function pointer type).
> 
> Here's the updated patch.
> 
> Le vendredi 18 juin 2021 à 16:37 -0400, David Malcolm a écrit :
> > On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> > > I have write access now.
> > 
> > Great.
> > 
> > > I'm not sure how I'm supposed to send my patches:
> > > should I put it in personal branches and you'll merge them?
> > 
> > Please send them to this mailing list for review; once they're
> > approved
> > you can merge them.
> > 
> > > 
> > > And for the MAINTAINERS file, should I just push to master right
> > > away,
> > > after sending it to the mailing list?
> > 
> > I think people just push the MAINTAINERS change and then let the
> > list
> > know, since it makes a good test that write access is working
> > correctly.
> > 
> > Dave
> > 
> > > 
> > > Thanks for your help!
> > > 
> > > Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > > > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a
> > > > > écrit :
> > > > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > > > Thank you for your answer.
> > > > > > > I attached the updated patch.
> > > > > > 
> > > > > > BTW you (or possibly me) dropped the mailing lists; was
> > > > > > that
> > > > > > deliberate?
> > > > > 
> > > > > Oh, my bad.
> > > > > 
> > > > 
> > > > [...]
> > > > 
> > > > 
> > > > > > 
> > > > > > 
> > > > > >

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-10-12 Thread Antoni Boucher via Gcc-patches
David: PING

Le lundi 27 septembre 2021 à 20:53 -0400, Antoni Boucher a écrit :
> I fixed an issue (it would show an error message when
> gcc_jit_type_dyncast_function_ptr_type was called on a type different
> than a function pointer type).
> 
> Here's the updated patch.
> 
> Le vendredi 18 juin 2021 à 16:37 -0400, David Malcolm a écrit :
> > On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> > > I have write access now.
> > 
> > Great.
> > 
> > > I'm not sure how I'm supposed to send my patches:
> > > should I put it in personal branches and you'll merge them?
> > 
> > Please send them to this mailing list for review; once they're
> > approved
> > you can merge them.
> > 
> > > 
> > > And for the MAINTAINERS file, should I just push to master right
> > > away,
> > > after sending it to the mailing list?
> > 
> > I think people just push the MAINTAINERS change and then let the
> > list
> > know, since it makes a good test that write access is working
> > correctly.
> > 
> > Dave
> > 
> > > 
> > > Thanks for your help!
> > > 
> > > Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > > > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a
> > > > > écrit :
> > > > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > > > Thank you for your answer.
> > > > > > > I attached the updated patch.
> > > > > > 
> > > > > > BTW you (or possibly me) dropped the mailing lists; was
> > > > > > that
> > > > > > deliberate?
> > > > > 
> > > > > Oh, my bad.
> > > > > 
> > > > 
> > > > [...]
> > > > 
> > > > 
> > > > > > 
> > > > > > 
> > > > > > > I have signed the FSF copyright attribution.
> > > > > > 
> > > > > > I can push changes on your behalf, but I'd prefer it if you
> > > > > > did
> > > > > > it,
> > > > > > especially given that you have various other patches you
> > > > > > want
> > > > > > to
> > > > > > get
> > > > > > in.
> > > > > > 
> > > > > > Instructions on how to get push rights to the git repo are
> > > > > > here:
> > > > > >   https://gcc.gnu.org/gitwrite.html
> > > > > > 
> > > > > > I can sponsor you.
> > > > > 
> > > > > Thanks.
> > > > > I did sign up to get push rights.
> > > > > Have you accepted my request to get those?
> > > > 
> > > > I did, but I didn't see any kind of notification.  Did you get
> > > > an
> > > > email
> > > > about it?
> > > > 
> > > > 
> > > > Dave
> > > > 
> > > 
> > > 
> > 
> > 
> 




Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-09-27 Thread Antoni Boucher via Gcc-patches
I fixed an issue (it would show an error message when
gcc_jit_type_dyncast_function_ptr_type was called on a type different
than a function pointer type).

Here's the updated patch.

Le vendredi 18 juin 2021 à 16:37 -0400, David Malcolm a écrit :
> On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> > I have write access now.
> 
> Great.
> 
> > I'm not sure how I'm supposed to send my patches:
> > should I put it in personal branches and you'll merge them?
> 
> Please send them to this mailing list for review; once they're
> approved
> you can merge them.
> 
> > 
> > And for the MAINTAINERS file, should I just push to master right
> > away,
> > after sending it to the mailing list?
> 
> I think people just push the MAINTAINERS change and then let the list
> know, since it makes a good test that write access is working
> correctly.
> 
> Dave
> 
> > 
> > Thanks for your help!
> > 
> > Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a écrit :
> > > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > > Thank you for your answer.
> > > > > > I attached the updated patch.
> > > > > 
> > > > > BTW you (or possibly me) dropped the mailing lists; was that
> > > > > deliberate?
> > > > 
> > > > Oh, my bad.
> > > > 
> > > 
> > > [...]
> > > 
> > > 
> > > > > 
> > > > > 
> > > > > > I have signed the FSF copyright attribution.
> > > > > 
> > > > > I can push changes on your behalf, but I'd prefer it if you
> > > > > did
> > > > > it,
> > > > > especially given that you have various other patches you want
> > > > > to
> > > > > get
> > > > > in.
> > > > > 
> > > > > Instructions on how to get push rights to the git repo are
> > > > > here:
> > > > >   https://gcc.gnu.org/gitwrite.html
> > > > > 
> > > > > I can sponsor you.
> > > > 
> > > > Thanks.
> > > > I did sign up to get push rights.
> > > > Have you accepted my request to get those?
> > > 
> > > I did, but I didn't see any kind of notification.  Did you get an
> > > email
> > > about it?
> > > 
> > > 
> > > Dave
> > > 
> > 
> > 
> 
> 

From 95f8b85bcc7b1259eef1e9916de824c752b2f2c0 Mon Sep 17 00:00:00 2001
From: Antoni Boucher 
Date: Sat, 1 Aug 2020 17:52:17 -0400
Subject: [PATCH] libgccjit: Add some reflection functions [PR96889]

2021-07-19  Antoni Boucher  

gcc/jit/
	PR target/96889
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_16): 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
	* docs/topics/types.rst: Add documentation for the functions
	gcc_jit_function_type_get_return_type,
	gcc_jit_function_type_get_param_count,
	gcc_jit_function_type_get_param_type,
	gcc_jit_type_unqualified, gcc_jit_type_dyncast_array,
	gcc_jit_type_is_bool,
	gcc_jit_type_dyncast_function_ptr_type,
	gcc_jit_type_is_integral, gcc_jit_type_is_pointer,
	gcc_jit_type_dyncast_vector,
	gcc_jit_vector_type_get_element_type,
	gcc_jit_vector_type_get_num_units,
	gcc_jit_struct_get_field, gcc_jit_type_is_struct,
	and gcc_jit_struct_get_field_count
	* libgccjit.c:
	(gcc_jit_function_get_return_type, gcc_jit_function_get_param_count,
	gcc_jit_function_type_get_return_type,
	gcc_jit_function_type_get_param_count,
	gcc_jit_function_type_get_param_type, gcc_jit_type_unqualified,
	gcc_jit_type_dyncast_array, gcc_jit_type_is_bool,
	gcc_jit_type_dyncast_function_ptr_type, gcc_jit_type_is_integral,
	gcc_jit_type_is_pointer, gcc_jit_type_dyncast_vector,
	gcc_jit_vector_type_get_element_type,
	gcc_jit_vector_type_get_num_units, gcc_jit_struct_get_field,
	gcc_jit_type_is_struct, gcc_jit_struct_get_field_count): New
	functions.
	(struct gcc_jit_function_type, struct gcc_jit_vector_type):
	New types.
	* libgccjit.h:
	(gcc_jit_function_get_return_type, gcc_jit_function_get_param_count,
	gcc_jit_function_type_get_return_type,
	gcc_jit_function_type_get_param_count,
	gcc_jit_function_type_get_param_type, gcc_jit_type_unqualified,
	gcc_jit_type_dyncast_array, gcc_jit_type_is_bool,
	gcc_jit_type_dyncast_function_ptr_type, gcc_jit_type_is_integral,
	gcc_jit_type_is_pointer, gcc_jit_type_dyncast_vector,
	gcc_jit_vector_type_get_element_type,
	gcc_jit_vector_type_get_num_units, gcc_jit_struct_get_field,
	gcc_jit_type_is_struct, gcc_jit_struct_get_field_count): New
	function declarations.
	(struct gcc_jit_function_type, struct gcc_jit_vector_type):
	New types.
	* jit-recording.h: New functions (is_struct and is_vector)
	* libgccjit.map (LIBGCCJIT_ABI_16): 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|  43 ++-
 gcc/jit/docs/topics/functions.rst|  26 ++
 gcc/jit/docs/topics/types.rst| 122 +
 gcc/jit/jit-recording.h  |   7 +
 gcc/jit/libgccjit.c   

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-08-31 Thread Antoni Boucher via Gcc-patches
David: PING

Le jeudi 29 juillet 2021 à 08:59 -0400, Antoni Boucher a écrit :
> David: PING
> 
> Le lundi 19 juillet 2021 à 12:10 -0400, Antoni Boucher a écrit :
> > I'm sending the patch once again for review/approval.
> > 
> > I fixed the doc to use the new function names.
> > 
> > Le vendredi 18 juin 2021 à 16:37 -0400, David Malcolm a écrit :
> > > On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> > > > I have write access now.
> > > 
> > > Great.
> > > 
> > > > I'm not sure how I'm supposed to send my patches:
> > > > should I put it in personal branches and you'll merge them?
> > > 
> > > Please send them to this mailing list for review; once they're
> > > approved
> > > you can merge them.
> > > 
> > > > 
> > > > And for the MAINTAINERS file, should I just push to master
> > > > right
> > > > away,
> > > > after sending it to the mailing list?
> > > 
> > > I think people just push the MAINTAINERS change and then let the
> > > list
> > > know, since it makes a good test that write access is working
> > > correctly.
> > > 
> > > Dave
> > > 
> > > > 
> > > > Thanks for your help!
> > > > 
> > > > Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > > > > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > > > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a
> > > > > > écrit :
> > > > > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > > > > Thank you for your answer.
> > > > > > > > I attached the updated patch.
> > > > > > > 
> > > > > > > BTW you (or possibly me) dropped the mailing lists; was
> > > > > > > that
> > > > > > > deliberate?
> > > > > > 
> > > > > > Oh, my bad.
> > > > > > 
> > > > > 
> > > > > [...]
> > > > > 
> > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > > I have signed the FSF copyright attribution.
> > > > > > > 
> > > > > > > I can push changes on your behalf, but I'd prefer it if
> > > > > > > you
> > > > > > > did
> > > > > > > it,
> > > > > > > especially given that you have various other patches you
> > > > > > > want
> > > > > > > to
> > > > > > > get
> > > > > > > in.
> > > > > > > 
> > > > > > > Instructions on how to get push rights to the git repo
> > > > > > > are
> > > > > > > here:
> > > > > > >   https://gcc.gnu.org/gitwrite.html
> > > > > > > 
> > > > > > > I can sponsor you.
> > > > > > 
> > > > > > Thanks.
> > > > > > I did sign up to get push rights.
> > > > > > Have you accepted my request to get those?
> > > > > 
> > > > > I did, but I didn't see any kind of notification.  Did you
> > > > > get
> > > > > an
> > > > > email
> > > > > about it?
> > > > > 
> > > > > 
> > > > > Dave
> > > > > 
> > > > 
> > > > 
> > > 
> > > 
> > 
> 
> 




Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-07-29 Thread Antoni Boucher via Gcc-patches
David: PING

Le lundi 19 juillet 2021 à 12:10 -0400, Antoni Boucher a écrit :
> I'm sending the patch once again for review/approval.
> 
> I fixed the doc to use the new function names.
> 
> Le vendredi 18 juin 2021 à 16:37 -0400, David Malcolm a écrit :
> > On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> > > I have write access now.
> > 
> > Great.
> > 
> > > I'm not sure how I'm supposed to send my patches:
> > > should I put it in personal branches and you'll merge them?
> > 
> > Please send them to this mailing list for review; once they're
> > approved
> > you can merge them.
> > 
> > > 
> > > And for the MAINTAINERS file, should I just push to master right
> > > away,
> > > after sending it to the mailing list?
> > 
> > I think people just push the MAINTAINERS change and then let the
> > list
> > know, since it makes a good test that write access is working
> > correctly.
> > 
> > Dave
> > 
> > > 
> > > Thanks for your help!
> > > 
> > > Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > > > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a
> > > > > écrit :
> > > > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > > > Thank you for your answer.
> > > > > > > I attached the updated patch.
> > > > > > 
> > > > > > BTW you (or possibly me) dropped the mailing lists; was
> > > > > > that
> > > > > > deliberate?
> > > > > 
> > > > > Oh, my bad.
> > > > > 
> > > > 
> > > > [...]
> > > > 
> > > > 
> > > > > > 
> > > > > > 
> > > > > > > I have signed the FSF copyright attribution.
> > > > > > 
> > > > > > I can push changes on your behalf, but I'd prefer it if you
> > > > > > did
> > > > > > it,
> > > > > > especially given that you have various other patches you
> > > > > > want
> > > > > > to
> > > > > > get
> > > > > > in.
> > > > > > 
> > > > > > Instructions on how to get push rights to the git repo are
> > > > > > here:
> > > > > >   https://gcc.gnu.org/gitwrite.html
> > > > > > 
> > > > > > I can sponsor you.
> > > > > 
> > > > > Thanks.
> > > > > I did sign up to get push rights.
> > > > > Have you accepted my request to get those?
> > > > 
> > > > I did, but I didn't see any kind of notification.  Did you get
> > > > an
> > > > email
> > > > about it?
> > > > 
> > > > 
> > > > Dave
> > > > 
> > > 
> > > 
> > 
> > 
> 




Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-07-19 Thread Antoni Boucher via Gcc-patches
I'm sending the patch once again for review/approval.

I fixed the doc to use the new function names.

Le vendredi 18 juin 2021 à 16:37 -0400, David Malcolm a écrit :
> On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> > I have write access now.
> 
> Great.
> 
> > I'm not sure how I'm supposed to send my patches:
> > should I put it in personal branches and you'll merge them?
> 
> Please send them to this mailing list for review; once they're
> approved
> you can merge them.
> 
> > 
> > And for the MAINTAINERS file, should I just push to master right
> > away,
> > after sending it to the mailing list?
> 
> I think people just push the MAINTAINERS change and then let the list
> know, since it makes a good test that write access is working
> correctly.
> 
> Dave
> 
> > 
> > Thanks for your help!
> > 
> > Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a écrit :
> > > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > > Thank you for your answer.
> > > > > > I attached the updated patch.
> > > > > 
> > > > > BTW you (or possibly me) dropped the mailing lists; was that
> > > > > deliberate?
> > > > 
> > > > Oh, my bad.
> > > > 
> > > 
> > > [...]
> > > 
> > > 
> > > > > 
> > > > > 
> > > > > > I have signed the FSF copyright attribution.
> > > > > 
> > > > > I can push changes on your behalf, but I'd prefer it if you
> > > > > did
> > > > > it,
> > > > > especially given that you have various other patches you want
> > > > > to
> > > > > get
> > > > > in.
> > > > > 
> > > > > Instructions on how to get push rights to the git repo are
> > > > > here:
> > > > >   https://gcc.gnu.org/gitwrite.html
> > > > > 
> > > > > I can sponsor you.
> > > > 
> > > > Thanks.
> > > > I did sign up to get push rights.
> > > > Have you accepted my request to get those?
> > > 
> > > I did, but I didn't see any kind of notification.  Did you get an
> > > email
> > > about it?
> > > 
> > > 
> > > Dave
> > > 
> > 
> > 
> 
> 

From aaf77d109d74afb0f46d3e5d4d094914cb1b21de Mon Sep 17 00:00:00 2001
From: Antoni Boucher 
Date: Sat, 1 Aug 2020 17:52:17 -0400
Subject: [PATCH] libgccjit: Add some reflection functions [PR96889]

2021-07-19  Antoni Boucher  

gcc/jit/
	PR target/96889
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_16): 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
	* docs/topics/types.rst: Add documentation for the functions
	gcc_jit_function_type_get_return_type,
	gcc_jit_function_type_get_param_count,
	gcc_jit_function_type_get_param_type,
	gcc_jit_type_unqualified, gcc_jit_type_dyncast_array,
	gcc_jit_type_is_bool,
	gcc_jit_type_dyncast_function_ptr_type,
	gcc_jit_type_is_integral, gcc_jit_type_is_pointer,
	gcc_jit_type_dyncast_vector,
	gcc_jit_vector_type_get_element_type,
	gcc_jit_vector_type_get_num_units,
	gcc_jit_struct_get_field, gcc_jit_type_is_struct,
	and gcc_jit_struct_get_field_count
	* libgccjit.c:
	(gcc_jit_function_get_return_type, gcc_jit_function_get_param_count,
	gcc_jit_function_type_get_return_type,
	gcc_jit_function_type_get_param_count,
	gcc_jit_function_type_get_param_type, gcc_jit_type_unqualified,
	gcc_jit_type_dyncast_array, gcc_jit_type_is_bool,
	gcc_jit_type_dyncast_function_ptr_type, gcc_jit_type_is_integral,
	gcc_jit_type_is_pointer, gcc_jit_type_dyncast_vector,
	gcc_jit_vector_type_get_element_type,
	gcc_jit_vector_type_get_num_units, gcc_jit_struct_get_field,
	gcc_jit_type_is_struct, gcc_jit_struct_get_field_count): New
	functions.
	(struct gcc_jit_function_type, struct gcc_jit_vector_type):
	New types.
	* libgccjit.h:
	(gcc_jit_function_get_return_type, gcc_jit_function_get_param_count,
	gcc_jit_function_type_get_return_type,
	gcc_jit_function_type_get_param_count,
	gcc_jit_function_type_get_param_type, gcc_jit_type_unqualified,
	gcc_jit_type_dyncast_array, gcc_jit_type_is_bool,
	gcc_jit_type_dyncast_function_ptr_type, gcc_jit_type_is_integral,
	gcc_jit_type_is_pointer, gcc_jit_type_dyncast_vector,
	gcc_jit_vector_type_get_element_type,
	gcc_jit_vector_type_get_num_units, gcc_jit_struct_get_field,
	gcc_jit_type_is_struct, gcc_jit_struct_get_field_count): New
	function declarations.
	(struct gcc_jit_function_type, struct gcc_jit_vector_type):
	New types.
	* jit-recording.h: New functions (is_struct and is_vector)
	* libgccjit.map (LIBGCCJIT_ABI_16): 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|  43 ++-
 gcc/jit/docs/topics/functions.rst|  26 ++
 gcc/jit/docs/topics/types.rst| 122 +
 gcc/jit/jit-recording.h  |   7 +
 gcc/jit/libgccjit.c  | 261 +++
 gcc/jit/libgccjit.h   

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-06-18 Thread David Malcolm via Gcc-patches
On Fri, 2021-06-18 at 15:41 -0400, Antoni Boucher wrote:
> I have write access now.

Great.

> I'm not sure how I'm supposed to send my patches:
> should I put it in personal branches and you'll merge them?

Please send them to this mailing list for review; once they're approved
you can merge them.

> 
> And for the MAINTAINERS file, should I just push to master right
> away,
> after sending it to the mailing list?

I think people just push the MAINTAINERS change and then let the list
know, since it makes a good test that write access is working
correctly.

Dave

> 
> Thanks for your help!
> 
> Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> > On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a écrit :
> > > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > > Thank you for your answer.
> > > > > I attached the updated patch.
> > > > 
> > > > BTW you (or possibly me) dropped the mailing lists; was that
> > > > deliberate?
> > > 
> > > Oh, my bad.
> > > 
> > 
> > [...]
> > 
> > 
> > > > 
> > > > 
> > > > > I have signed the FSF copyright attribution.
> > > > 
> > > > I can push changes on your behalf, but I'd prefer it if you did
> > > > it,
> > > > especially given that you have various other patches you want
> > > > to
> > > > get
> > > > in.
> > > > 
> > > > Instructions on how to get push rights to the git repo are
> > > > here:
> > > >   https://gcc.gnu.org/gitwrite.html
> > > > 
> > > > I can sponsor you.
> > > 
> > > Thanks.
> > > I did sign up to get push rights.
> > > Have you accepted my request to get those?
> > 
> > I did, but I didn't see any kind of notification.  Did you get an
> > email
> > about it?
> > 
> > 
> > Dave
> > 
> 
> 




Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-06-18 Thread Antoni Boucher via Gcc-patches
I have write access now.
I'm not sure how I'm supposed to send my patches:
should I put it in personal branches and you'll merge them?

And for the MAINTAINERS file, should I just push to master right away,
after sending it to the mailing list?

Thanks for your help!

Le vendredi 18 juin 2021 à 12:09 -0400, David Malcolm a écrit :
> On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> > Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a écrit :
> > > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > > Thank you for your answer.
> > > > I attached the updated patch.
> > > 
> > > BTW you (or possibly me) dropped the mailing lists; was that
> > > deliberate?
> > 
> > Oh, my bad.
> > 
> 
> [...]
> 
> 
> > > 
> > > 
> > > > I have signed the FSF copyright attribution.
> > > 
> > > I can push changes on your behalf, but I'd prefer it if you did
> > > it,
> > > especially given that you have various other patches you want to
> > > get
> > > in.
> > > 
> > > Instructions on how to get push rights to the git repo are here:
> > >   https://gcc.gnu.org/gitwrite.html
> > > 
> > > I can sponsor you.
> > 
> > Thanks.
> > I did sign up to get push rights.
> > Have you accepted my request to get those?
> 
> I did, but I didn't see any kind of notification.  Did you get an
> email
> about it?
> 
> 
> Dave
> 




Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-06-18 Thread David Malcolm via Gcc-patches
On Fri, 2021-06-18 at 11:55 -0400, Antoni Boucher wrote:
> Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a écrit :
> > On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > > Thank you for your answer.
> > > I attached the updated patch.
> > 
> > BTW you (or possibly me) dropped the mailing lists; was that
> > deliberate?
> 
> Oh, my bad.
> 

[...]


> > 
> > 
> > > I have signed the FSF copyright attribution.
> > 
> > I can push changes on your behalf, but I'd prefer it if you did it,
> > especially given that you have various other patches you want to
> > get
> > in.
> > 
> > Instructions on how to get push rights to the git repo are here:
> >   https://gcc.gnu.org/gitwrite.html
> > 
> > I can sponsor you.
> 
> Thanks.
> I did sign up to get push rights.
> Have you accepted my request to get those?

I did, but I didn't see any kind of notification.  Did you get an email
about it?


Dave



Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-06-18 Thread Antoni Boucher via Gcc-patches
Le vendredi 11 juin 2021 à 14:00 -0400, David Malcolm a écrit :
> On Fri, 2021-06-11 at 08:15 -0400, Antoni Boucher wrote:
> > Thank you for your answer.
> > I attached the updated patch.
> 
> BTW you (or possibly me) dropped the mailing lists; was that
> deliberate?

Oh, my bad.

> > 
> > See my answers below.
> > 
> > Le jeudi 10 juin 2021 à 18:41 -0400, David Malcolm a écrit :
> > > On Thu, 2021-05-27 at 21:51 -0400, Antoni Boucher wrote:
> > > > I chose option A, so everything is a size_t, now.
> > > > I also renamed the dyncast functions.
> > > > Here's the new patch.
> > > 
> > > Thanks, sorry again about the delays in reviewing your work.
> > > 
> > > You didn't specify how you tested the patch; are you running the
> > > full
> > > jit regression test suite?
> > 
> > Yes, I run the full jit test suite.
> 
> Great.
> 
> [...snip...]
> 
> > > 
> > > I can't remember, sorry, do you have push rights to the gcc git
> > > repository?
> > 
> > I don't have push rights; it's actually my first contribution to
> > gcc.
> 
> Congratulations!  I'm excited about having Rust support for GCC (two
> different ways, in fact!)

Yup. Exciting times!

> 
> > I have signed the FSF copyright attribution.
> 
> I can push changes on your behalf, but I'd prefer it if you did it,
> especially given that you have various other patches you want to get
> in.
> 
> Instructions on how to get push rights to the git repo are here:
>   https://gcc.gnu.org/gitwrite.html
> 
> I can sponsor you.

Thanks.
I did sign up to get push rights.
Have you accepted my request to get those?

> 
> Note that your patches would still need review/approval ("Write After
> Approval").
> 
> > > Do you have a preference as to which patch you want me to look at
> > > next?
> > > Otherwise I'll go through them in the order in
> > > https://github.com/antoyo/rustc_codegen_gcc/tree/master/gcc-patches
> > 
> > You can indeed look in this directory in the order they were added.
> > There was one patch before the reflection one (0001-This-patch-
> > handles-
> > truncation-and-extension-for-cast.patch), but since it's only a
> > bugfix,
> > it doesn't matter if it's merged after.
> 
> That one looks good to me now too (I've replied on that patch).
> 
> > 
> > This one is ready for review, but I believe the other one needs
> > correction on my end since the last review you made. I'll make sure
> > to
> > fix them soon.
> 
> Now I'm confused as to which patches you're referring to, sorry. 
> I'll
> hold off for now on further reviews; let me know when you want me to
> look at them, and which ones.
> 
> Thanks
> Dave
> 




Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-06-10 Thread David Malcolm via Gcc-patches
On Thu, 2021-05-27 at 21:51 -0400, Antoni Boucher wrote:
> I chose option A, so everything is a size_t, now.
> I also renamed the dyncast functions.
> Here's the new patch.

Thanks, sorry again about the delays in reviewing your work.

You didn't specify how you tested the patch; are you running the full
jit regression test suite?

This is looking close to done; a few nits inline below:

[...snip...]

> diff --git a/gcc/jit/docs/topics/functions.rst 
> b/gcc/jit/docs/topics/functions.rst
> index b2d9239aa0a..1d20e3045a0 100644
> --- a/gcc/jit/docs/topics/functions.rst
> +++ b/gcc/jit/docs/topics/functions.rst

[...snip...]

> +   The API entrypoints relating to getting info about parameters and return
> +   types:
> +
> +  * :c:func:`gcc_jit_function_get_return_type`
> +
> +  * :c:func:`gcc_jit_function_get_param_count`
> +
> +   were added in :ref:`LIBGCCJIT_ABI_15`; you can test for their presence
> +   using

16, rather than 15.

> +   The API entrypoints related to the reflection API:
> +
> +  * :c:func:`gcc_jit_function_type_get_return_type`
> +

[...snip...]

> +
> +  * :c:func:`gcc_jit_struct_get_field_count`
> +
> +   were added in :ref:`LIBGCCJIT_ABI_15`; you can test for their presence
> +   using

16, rather than 15 again.

> +
> +   .. code-block:: c
> +
> +  #ifdef LIBGCCJIT_HAVE_REFLECTION
> +
> +   .. type:: gcc_jit_case

[...snip...]

> +gcc_jit_type *
> +gcc_jit_function_type_get_param_type (gcc_jit_function_type *function_type,
> + size_t index)
> +{
> +  RETURN_NULL_IF_FAIL (function_type, NULL, NULL, "NULL function_type");
> +  size_t num_params = function_type->get_param_types ().length ();
> +  gcc::jit::recording::context *ctxt = function_type->m_ctxt;
> +  RETURN_NULL_IF_FAIL_PRINTF3 (index < num_params,
> +ctxt, NULL,
> +"index of %ld is too large (%s has %ld params)",
> +index,
> +function_type->get_debug_string (),
> +num_params);
> +  return (gcc_jit_type *)function_type->get_param_types ()[index];
> +}
> +

I'm retaining the above, since...

[...snip...]

>  
> +
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::fields::get_field method in
> +   jit-recording.c.  */
> +extern gcc_jit_field *
> +gcc_jit_struct_get_field (gcc_jit_struct *struct_type,
> +size_t index)
> +{
> +  RETURN_NULL_IF_FAIL (struct_type, NULL, NULL, "NULL struct type");
> +  RETURN_NULL_IF_FAIL (struct_type->get_fields (), NULL, NULL,
> + "NULL struct fields");
> +  RETURN_NULL_IF_FAIL ((int) index < struct_type->get_fields ()->length (),
> + NULL, NULL, "NULL struct type");

...copy error here; the message for this kind of failure needs
updating.  Do it in a similar way to how you did
gcc_jit_function_type_get_param_type above, using the ctxt of the
struct_type.

[...snip...]

> +#define LIBGCCJIT_HAVE_REFLECTION
> +
> +/* Reflection functions to get the number of parameters, return type of
> +   a function and whether a type is a bool from the C API.
> +
> +   This API entrypoint was added in LIBGCCJIT_ABI_15; you can test for its

16, rather than 15, again.

> +   presence using
> + #ifdef LIBGCCJIT_HAVE_REFLECTION

[...snip...]

OK for trunk with the above nits fixed, assuming that you have run the
full regression test suite (or do you need help with that?)

I can't remember, sorry, do you have push rights to the gcc git
repository?

Do you have a preference as to which patch you want me to look at next?
Otherwise I'll go through them in the order in
https://github.com/antoyo/rustc_codegen_gcc/tree/master/gcc-patches

Dave




Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-27 Thread Antoni Boucher via Gcc-patches
I chose option A, so everything is a size_t, now.
I also renamed the dyncast functions.
Here's the new patch.

Le jeudi 27 mai 2021 à 18:19 -0400, David Malcolm a écrit :
> On Tue, 2021-05-25 at 20:19 -0400, Antoni Boucher wrote:
> > @David: PING
> > 
> > As far as I know, the only remaining question is about using
> > `ssize_t`
> > for the return type of some functions.
> > Here's why I use this type:
> > 
> > That seemed off to return NULL for the functions returning a 
> > size_t to indicate an error. So I changed it to return -1 (and
> > return
> > type to ssize_t). Is that the proper way to indicate an error?
> > 
> > Once I know the answer for this error handling question, I'll fix
> > the
> > types.
> > 
> > Thanks!
> 
> I think the things that accept params for indexes should be size_t
> (i.e. unsigned).
> 
> As for the error-handling, I've been going back and forth.
> 
> The various return of -1 in the patch all occur when NULL is passed
> in
> as the gcc_jit_foo*, for the various foo. Unfortunately this means
> that
> we can't get at a jit context to emit the error on, so jit_error is
> called with a NULL context, and hence merely prints to stderr; it
> can't
> set state on any jit context.
> 
> Presumably if the code is passing in NULL for one of these things,
> then
> something's gone wrong already.
> 
> Option (A): returning a size_t (unsigned):
> - No way to indicate error if the user passes in NULL, or a bogus
> value, beyond output on stderr; we have to defensively return 0,
> which
> the user's code is likely to gracefully handle
> 
> Option (B): returning a ssize_t:
> - Can return -1 if the user passes in NULL or a bogus value.  But if
> the user doesn't check for -1 and blithely casts to unsigned (e.g.
> allocating a buffer), their code is likely to explode with a very
> large
> allocation.
> 
> Consider e.g. gcc_jit_function_type_get_param_count.  If the user
> uses 
> gcc_jit_type_is_function_ptr_type to dynamically cast some type to
> function_type and forgets to check, and passes in NULL, then...
> 
> ...with option (A), it returns 0, as if the function has no params;
> eventually a type-checking error occurs.
> 
> ...with option (B), it returns -1, and the user who didn't check
> before
> has to check again
> 
> So I think I prefer option (A), but I'm willing to be convinced on
> the
> alternative.
> 
> I also now think that the dynamic cast functions (e.g.
> gcc_jit_type_is_function_ptr_type) should be "_dyncast_" rather than
> "_is_" (e.g. gcc_jit_type_dyncast_function_ptr_type) to make it clear
> that this is a dynamic cast that can fail, and to contrast them with
> the "_as_" functions which are static casts.
> 
> Hope this makes sense and is constructive.
> Dave
> 
> 
> > 
> > Le jeudi 13 mai 2021 à 17:30 -0400, David Malcolm a écrit :
> > > On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote:
> > > > I was missing a check in gcc_jit_struct_get_field, I added it
> > > > in
> > > > this
> > > > new patch.
> > > > 
> > > 
> > > Sorry about the long delay in reviewing this patch.
> > > 
> > > The main high-level points are:
> > > - currently the get_*_count functions return "ssize_t" - why? 
> > > Only
> > > unsigned values are meaningful; shouldn't they return "size_t"
> > > instead?
> > > 
> > > - the various "lookup by index" functions take "int" i.e. signed,
> > > but
> > > only >= 0 is meaningful.  I think it makes sense to make them
> > > take
> > > size_t instead.
> > > 
> > > Sorry if we covered that before in the review, it's been a while.
> > > 
> > > Various nitpicks inline below...
> > > 
> > > [...snip...]
> > >  
> > > > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > > > b/gcc/jit/docs/topics/compatibility.rst
> > > > index 6bfa101ed71..236e5c72d81 100644
> > > > --- a/gcc/jit/docs/topics/compatibility.rst
> > > > +++ b/gcc/jit/docs/topics/compatibility.rst
> > > > @@ -226,3 +226,44 @@ 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:
> > > 
> > > This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm.
> > > 
> > > [...snip...]
> > > 
> > > > diff --git a/gcc/jit/docs/topics/functions.rst
> > > > b/gcc/jit/docs/topics/functions.rst
> > > > index eb40d64010e..aa6de87282d 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::  ssize_t \
> > > > +   gcc_jit_function_get_param_count
> > > > (gcc_jit_function
> > > > *func)
> > > > +
> > > > +   Get the number of 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-27 Thread David Malcolm via Gcc-patches
On Tue, 2021-05-25 at 20:19 -0400, Antoni Boucher wrote:
> @David: PING
> 
> As far as I know, the only remaining question is about using
> `ssize_t`
> for the return type of some functions.
> Here's why I use this type:
> 
> That seemed off to return NULL for the functions returning a 
> size_t to indicate an error. So I changed it to return -1 (and return
> type to ssize_t). Is that the proper way to indicate an error?
> 
> Once I know the answer for this error handling question, I'll fix the
> types.
> 
> Thanks!

I think the things that accept params for indexes should be size_t
(i.e. unsigned).

As for the error-handling, I've been going back and forth.

The various return of -1 in the patch all occur when NULL is passed in
as the gcc_jit_foo*, for the various foo. Unfortunately this means that
we can't get at a jit context to emit the error on, so jit_error is
called with a NULL context, and hence merely prints to stderr; it can't
set state on any jit context.

Presumably if the code is passing in NULL for one of these things, then
something's gone wrong already.

Option (A): returning a size_t (unsigned):
- No way to indicate error if the user passes in NULL, or a bogus
value, beyond output on stderr; we have to defensively return 0, which
the user's code is likely to gracefully handle

Option (B): returning a ssize_t:
- Can return -1 if the user passes in NULL or a bogus value.  But if
the user doesn't check for -1 and blithely casts to unsigned (e.g.
allocating a buffer), their code is likely to explode with a very large
allocation.

Consider e.g. gcc_jit_function_type_get_param_count.  If the user uses 
gcc_jit_type_is_function_ptr_type to dynamically cast some type to
function_type and forgets to check, and passes in NULL, then...

...with option (A), it returns 0, as if the function has no params;
eventually a type-checking error occurs.

...with option (B), it returns -1, and the user who didn't check before
has to check again

So I think I prefer option (A), but I'm willing to be convinced on the
alternative.

I also now think that the dynamic cast functions (e.g.
gcc_jit_type_is_function_ptr_type) should be "_dyncast_" rather than
"_is_" (e.g. gcc_jit_type_dyncast_function_ptr_type) to make it clear
that this is a dynamic cast that can fail, and to contrast them with
the "_as_" functions which are static casts.

Hope this makes sense and is constructive.
Dave


> 
> Le jeudi 13 mai 2021 à 17:30 -0400, David Malcolm a écrit :
> > On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote:
> > > I was missing a check in gcc_jit_struct_get_field, I added it in
> > > this
> > > new patch.
> > > 
> > 
> > Sorry about the long delay in reviewing this patch.
> > 
> > The main high-level points are:
> > - currently the get_*_count functions return "ssize_t" - why?  Only
> > unsigned values are meaningful; shouldn't they return "size_t"
> > instead?
> > 
> > - the various "lookup by index" functions take "int" i.e. signed, but
> > only >= 0 is meaningful.  I think it makes sense to make them take
> > size_t instead.
> > 
> > Sorry if we covered that before in the review, it's been a while.
> > 
> > Various nitpicks inline below...
> > 
> > [...snip...]
> >  
> > > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > > b/gcc/jit/docs/topics/compatibility.rst
> > > index 6bfa101ed71..236e5c72d81 100644
> > > --- a/gcc/jit/docs/topics/compatibility.rst
> > > +++ b/gcc/jit/docs/topics/compatibility.rst
> > > @@ -226,3 +226,44 @@ 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:
> > 
> > This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm.
> > 
> > [...snip...]
> > 
> > > diff --git a/gcc/jit/docs/topics/functions.rst
> > > b/gcc/jit/docs/topics/functions.rst
> > > index eb40d64010e..aa6de87282d 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::  ssize_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.
> > 
> > As noted before, this doesn't yet document all the new entrypoints; I
> > think you wanted to hold off until all the details were thrashed out,
> > but hopefully we're close.
> > 
> > The documentation for an entrypoint should specify which ABI it was
> > added in.
> > 
> > [...snip...]

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-25 Thread Antoni Boucher via Gcc-patches
@David: PING

As far as I know, the only remaining question is about using `ssize_t`
for the return type of some functions.
Here's why I use this type:

That seemed off to return NULL for the functions returning a 
size_t to indicate an error. So I changed it to return -1 (and return
type to ssize_t). Is that the proper way to indicate an error?

Once I know the answer for this error handling question, I'll fix the
types.

Thanks!

Le jeudi 13 mai 2021 à 17:30 -0400, David Malcolm a écrit :
> On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote:
> > I was missing a check in gcc_jit_struct_get_field, I added it in this
> > new patch.
> > 
> 
> Sorry about the long delay in reviewing this patch.
> 
> The main high-level points are:
> - currently the get_*_count functions return "ssize_t" - why?  Only
> unsigned values are meaningful; shouldn't they return "size_t" instead?
> 
> - the various "lookup by index" functions take "int" i.e. signed, but
> only >= 0 is meaningful.  I think it makes sense to make them take
> size_t instead.
> 
> Sorry if we covered that before in the review, it's been a while.
> 
> Various nitpicks inline below...
> 
> [...snip...]
>  
> > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > b/gcc/jit/docs/topics/compatibility.rst
> > index 6bfa101ed71..236e5c72d81 100644
> > --- a/gcc/jit/docs/topics/compatibility.rst
> > +++ b/gcc/jit/docs/topics/compatibility.rst
> > @@ -226,3 +226,44 @@ 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:
> 
> This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm.
> 
> [...snip...]
> 
> > diff --git a/gcc/jit/docs/topics/functions.rst
> > b/gcc/jit/docs/topics/functions.rst
> > index eb40d64010e..aa6de87282d 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::  ssize_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.
> 
> As noted before, this doesn't yet document all the new entrypoints; I
> think you wanted to hold off until all the details were thrashed out,
> but hopefully we're close.
> 
> The documentation for an entrypoint should specify which ABI it was
> added in.
> 
> [...snip...]
> 
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::type::is_struct method, in
> > +   jit-recording.c.  */
> > +
> > +gcc_jit_struct *
> > +gcc_jit_type_is_struct (gcc_jit_type *type)
> > +{
> > +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
> > +  gcc::jit::recording::struct_ *struct_type = type->is_struct ();
> > +  return (gcc_jit_struct *)struct_type;
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::vector_type::get_num_units method, in
> > +   jit-recording.c.  */
> > +
> > +ssize_t
> > +gcc_jit_vector_type_get_num_units (gcc_jit_vector_type *vector_type)
> > +{
> > +  RETURN_VAL_IF_FAIL (vector_type, -1, NULL, NULL, "NULL
> > vector_type");
> > +  return vector_type->get_num_units ();
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::vector_type::get_element_type method, in
> > +   jit-recording.c.  */
> > +
> > +gcc_jit_type *
> > +gcc_jit_vector_type_get_element_type (gcc_jit_vector_type
> > *vector_type)
> > +{
> > +  RETURN_NULL_IF_FAIL (vector_type, NULL, NULL, "NULL vector_type");
> > +  return (gcc_jit_type *)vector_type->get_element_type ();
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::type::unqualified method, in
> > +   jit-recording.c.  */
> > +
> > +gcc_jit_type *
> > +gcc_jit_type_unqualified (gcc_jit_type *type)
> > +{
> > +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
> > +
> > +  return (gcc_jit_type *)type->unqualified ();
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::type::dyn_cast_function_type method, in
> > +   jit-recording.c.  */
> > +
> > +gcc_jit_function_type *
> > 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-13 Thread Antoni Boucher via Gcc-patches
Thanks for your reviews.

I attached the new patch to this email.

See answers below:

Le jeudi 13 mai 2021 à 17:30 -0400, David Malcolm a écrit :
> On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote:
> > I was missing a check in gcc_jit_struct_get_field, I added it in
> > this
> > new patch.
> > 
> 
> Sorry about the long delay in reviewing this patch.
> 
> The main high-level points are:
> - currently the get_*_count functions return "ssize_t" - why?  Only
> unsigned values are meaningful; shouldn't they return "size_t"
> instead?

For those, I had this question in a previous email:

That seemed off to return NULL for the functions returning a 
size_t to indicate an error. So I changed it to return -1 (and return
type to ssize_t). Is that the proper way to indicate an error?

Once I know the answer for this error handling question, I'll fix the
types.

> - the various "lookup by index" functions take "int" i.e. signed, but
> only >= 0 is meaningful.  I think it makes sense to make them take
> size_t instead.

That's fixed in the new patch.

> Sorry if we covered that before in the review, it's been a while.
> 
> Various nitpicks inline below...
> 
> [...snip...]
>  
> > diff --git a/gcc/jit/docs/topics/compatibility.rst
> > b/gcc/jit/docs/topics/compatibility.rst
> > index 6bfa101ed71..236e5c72d81 100644
> > --- a/gcc/jit/docs/topics/compatibility.rst
> > +++ b/gcc/jit/docs/topics/compatibility.rst
> > @@ -226,3 +226,44 @@ 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:
> 
> This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm.

This was updated for the new patch.

> [...snip...]
> 
> > diff --git a/gcc/jit/docs/topics/functions.rst
> > b/gcc/jit/docs/topics/functions.rst
> > index eb40d64010e..aa6de87282d 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::  ssize_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.
> 
> As noted before, this doesn't yet document all the new entrypoints; I
> think you wanted to hold off until all the details were thrashed out,
> but hopefully we're close.
> 
> The documentation for an entrypoint should specify which ABI it was
> added in.

The documentation was added in the new patch.

> [...snip...]
> 
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::type::is_struct method, in
> > +   jit-recording.c.  */
> > +
> > +gcc_jit_struct *
> > +gcc_jit_type_is_struct (gcc_jit_type *type)
> > +{
> > +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
> > +  gcc::jit::recording::struct_ *struct_type = type->is_struct ();
> > +  return (gcc_jit_struct *)struct_type;
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::vector_type::get_num_units method, in
> > +   jit-recording.c.  */
> > +
> > +ssize_t
> > +gcc_jit_vector_type_get_num_units (gcc_jit_vector_type
> > *vector_type)
> > +{
> > +  RETURN_VAL_IF_FAIL (vector_type, -1, NULL, NULL, "NULL
> > vector_type");
> > +  return vector_type->get_num_units ();
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::vector_type::get_element_type method, in
> > +   jit-recording.c.  */
> > +
> > +gcc_jit_type *
> > +gcc_jit_vector_type_get_element_type (gcc_jit_vector_type
> > *vector_type)
> > +{
> > +  RETURN_NULL_IF_FAIL (vector_type, NULL, NULL, "NULL
> > vector_type");
> > +  return (gcc_jit_type *)vector_type->get_element_type ();
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::type::unqualified method, in
> > +   jit-recording.c.  */
> > +
> > +gcc_jit_type *
> > +gcc_jit_type_unqualified (gcc_jit_type *type)
> > +{
> > +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
> > +
> > +  return (gcc_jit_type *)type->unqualified ();
> > +}
> > +
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::type::dyn_cast_function_type method, in

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-13 Thread David Malcolm via Gcc-patches
On Tue, 2020-11-03 at 17:13 -0500, Antoni Boucher wrote:
> I was missing a check in gcc_jit_struct_get_field, I added it in this
> new patch.
> 

Sorry about the long delay in reviewing this patch.

The main high-level points are:
- currently the get_*_count functions return "ssize_t" - why?  Only
unsigned values are meaningful; shouldn't they return "size_t" instead?

- the various "lookup by index" functions take "int" i.e. signed, but
only >= 0 is meaningful.  I think it makes sense to make them take
size_t instead.

Sorry if we covered that before in the review, it's been a while.

Various nitpicks inline below...

[...snip...]
 
> diff --git a/gcc/jit/docs/topics/compatibility.rst 
> b/gcc/jit/docs/topics/compatibility.rst
> index 6bfa101ed71..236e5c72d81 100644
> --- a/gcc/jit/docs/topics/compatibility.rst
> +++ b/gcc/jit/docs/topics/compatibility.rst
> @@ -226,3 +226,44 @@ 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:

This needs updating, as I used LIBGCCJIT_ABI_15 for inline asm.

[...snip...]

> diff --git a/gcc/jit/docs/topics/functions.rst 
> b/gcc/jit/docs/topics/functions.rst
> index eb40d64010e..aa6de87282d 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::  ssize_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.

As noted before, this doesn't yet document all the new entrypoints; I
think you wanted to hold off until all the details were thrashed out,
but hopefully we're close.

The documentation for an entrypoint should specify which ABI it was
added in.

[...snip...]

> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::type::is_struct method, in
> +   jit-recording.c.  */
> +
> +gcc_jit_struct *
> +gcc_jit_type_is_struct (gcc_jit_type *type)
> +{
> +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
> +  gcc::jit::recording::struct_ *struct_type = type->is_struct ();
> +  return (gcc_jit_struct *)struct_type;
> +}
> +
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::vector_type::get_num_units method, in
> +   jit-recording.c.  */
> +
> +ssize_t
> +gcc_jit_vector_type_get_num_units (gcc_jit_vector_type *vector_type)
> +{
> +  RETURN_VAL_IF_FAIL (vector_type, -1, NULL, NULL, "NULL vector_type");
> +  return vector_type->get_num_units ();
> +}
> +
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::vector_type::get_element_type method, in
> +   jit-recording.c.  */
> +
> +gcc_jit_type *
> +gcc_jit_vector_type_get_element_type (gcc_jit_vector_type *vector_type)
> +{
> +  RETURN_NULL_IF_FAIL (vector_type, NULL, NULL, "NULL vector_type");
> +  return (gcc_jit_type *)vector_type->get_element_type ();
> +}
> +
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::type::unqualified method, in
> +   jit-recording.c.  */
> +
> +gcc_jit_type *
> +gcc_jit_type_unqualified (gcc_jit_type *type)
> +{
> +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
> +
> +  return (gcc_jit_type *)type->unqualified ();
> +}
> +
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::type::dyn_cast_function_type method, in
> +   jit-recording.c.  */
> +
> +gcc_jit_function_type *
> +gcc_jit_type_is_function_ptr_type (gcc_jit_type *type)
> +{
> +  RETURN_NULL_IF_FAIL (type, NULL, NULL, "NULL type");
> +  gcc::jit::recording::type *func_ptr_type = type->dereference ();
> +  RETURN_NULL_IF_FAIL (func_ptr_type, NULL, NULL, "NULL type");
> +  gcc::jit::recording::function_type *func_type =
> +func_ptr_type->dyn_cast_function_type ();
> +  RETURN_NULL_IF_FAIL (func_type, NULL, NULL, "NULL type");

I notice that the other new "*_is_*" functions don't fail if the
dyncast returns NULL, whereas this one does.

RETURN_NULL_IF_FAIL calls jit_error; do we really want that?  It seems
more consistent to have it return NULL without an error for the case
where "type" isn't a function ptr type.

> +
> +  return (gcc_jit_function_type *)func_type;
> +}
> +
> +/* Public entrypoint.  See 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2021-05-13 Thread Martin Liška

@David: PING

On 11/3/20 11:13 PM, Antoni Boucher via Gcc-patches wrote:

I was missing a check in gcc_jit_struct_get_field, I added it in this new patch.

On Thu, Oct 15, 2020 at 05:52:33PM -0400, David Malcolm wrote:

On Thu, 2020-10-15 at 13:39 -0400, Antoni Boucher wrote:

Thanks. I updated the patch with these changes.


Thanks for patch; review below.  Sorry if it seems excessively nitpicky
in places.


2020-09-1  Antoni Boucher  

    gcc/jit/
    PR target/96889
    * docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.


15 now.


    * docs/topics/functions.rst: Add documentation for the
    functions gcc_jit_function_get_return_type and
    gcc_jit_function_get_param_count
    * libgccjit.c: New functions:
  * gcc_jit_function_get_return_type;
  * gcc_jit_function_get_param_count;
  * gcc_jit_function_type_get_return_type;
  * gcc_jit_function_type_get_param_count;
  * gcc_jit_function_type_get_param_type;
  * gcc_jit_type_unqualified;
  * gcc_jit_type_is_array;
  * gcc_jit_type_is_bool;
  * gcc_jit_type_is_function_ptr_type;
  * gcc_jit_type_is_int;
  * gcc_jit_type_is_pointer;
  * gcc_jit_type_is_vector;
  * gcc_jit_vector_type_get_element_type;
  * gcc_jit_vector_type_get_num_units;
  * gcc_jit_struct_get_field;
  * gcc_jit_type_is_struct;
  * gcc_jit_struct_get_field_count;


This isn't valid ChangeLog format; it will fail the git hooks.


    * libgccjit.h


Likewise.


    * jit-recording.h: New functions (is_struct and is_vector)
    * libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.


15 now.



    gcc/testsuite/
    PR target/96889
    * jit.dg/all-non-failing-tests.h: Add test-reflection.c.
    * jit.dg/test-reflection.c: New test.


[...]



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.
+


The documentation part of the patch is incomplete: it hasn't been
updated to add all the new entrypoints.
Also, the return type of gcc_jit_function_get_param_count is
inconsistent (size_t above, but ssize_t below).



diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 30e37aff387..525b8bc921d 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -538,7 +538,9 @@ public:
   virtual bool is_bool () const = 0;
   virtual type *is_pointer () = 0;
   virtual type *is_array () = 0;
+  virtual struct_ *is_struct () { return NULL; }


Can't you use dyn_cast_struct for this?
Or is this about looking through decorated_type? e.g. for const and
volatile variants?

I guess my question is, what is the purpose of gcc_jit_type_is_struct?


   virtual bool is_void () const { return false; }
+  virtual vector_type *is_vector () { return NULL; }


Likewise, can't you use dyn_cast_vector_type for this?


   virtual bool has_known_size () const { return true; }

   bool is_numeric () const
@@ -595,6 +597,8 @@ public:
   bool is_bool () const FINAL OVERRIDE;
   type *is_pointer () FINAL OVERRIDE { return dereference (); }
   type *is_array () FINAL OVERRIDE { return NULL; }
+  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
+  struct_ *is_struct () FINAL OVERRIDE { return NULL; }


Likewise, and this is redundant, as it's merely copying the base class
implementation.


   bool is_void () const FINAL OVERRIDE { return m_kind == GCC_JIT_TYPE_VOID; }

 public:
@@ -629,6 +633,8 @@ public:
   bool is_bool () const FINAL OVERRIDE { return false; }
   type *is_pointer () FINAL OVERRIDE { return m_other_type; }
   type *is_array () FINAL OVERRIDE { return NULL; }
+  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
+  struct_ *is_struct () FINAL OVERRIDE { return NULL; }


Likewise.



@@ -655,6 +661,7 @@ public:
   bool is_bool () const FINAL OVERRIDE { return m_other_type->is_bool (); }
   type *is_pointer () FINAL OVERRIDE { return m_other_type->is_pointer (); }
   type *is_array () FINAL OVERRIDE { return m_other_type->is_array (); }
+  struct_ *is_struct () FINAL OVERRIDE { return m_other_type->is_struct (); }


Aha: with a decorated type you look through the decoration.


 protected:
   type *m_other_type;
@@ -737,6 +744,8 @@ public:

   void replay_into (replayer *) FINAL OVERRIDE;

+  vector_type 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-11-03 Thread Antoni Boucher via Gcc-patches
I was missing a check in gcc_jit_struct_get_field, I added it in this 
new patch.


On Thu, Oct 15, 2020 at 05:52:33PM -0400, David Malcolm wrote:

On Thu, 2020-10-15 at 13:39 -0400, Antoni Boucher wrote:

Thanks. I updated the patch with these changes.


Thanks for patch; review below.  Sorry if it seems excessively nitpicky
in places.


2020-09-1  Antoni Boucher  

gcc/jit/
PR target/96889
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.


15 now.


* docs/topics/functions.rst: Add documentation for the
functions gcc_jit_function_get_return_type and
gcc_jit_function_get_param_count
* libgccjit.c: New functions:
  * gcc_jit_function_get_return_type;
  * gcc_jit_function_get_param_count;
  * gcc_jit_function_type_get_return_type;
  * gcc_jit_function_type_get_param_count;
  * gcc_jit_function_type_get_param_type;
  * gcc_jit_type_unqualified;
  * gcc_jit_type_is_array;
  * gcc_jit_type_is_bool;
  * gcc_jit_type_is_function_ptr_type;
  * gcc_jit_type_is_int;
  * gcc_jit_type_is_pointer;
  * gcc_jit_type_is_vector;
  * gcc_jit_vector_type_get_element_type;
  * gcc_jit_vector_type_get_num_units;
  * gcc_jit_struct_get_field;
  * gcc_jit_type_is_struct;
  * gcc_jit_struct_get_field_count;


This isn't valid ChangeLog format; it will fail the git hooks.


* libgccjit.h


Likewise.


* jit-recording.h: New functions (is_struct and is_vector)
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.


15 now.



gcc/testsuite/
PR target/96889
* jit.dg/all-non-failing-tests.h: Add test-reflection.c.
* jit.dg/test-reflection.c: New test.


[...]



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.
+


The documentation part of the patch is incomplete: it hasn't been
updated to add all the new entrypoints.
Also, the return type of gcc_jit_function_get_param_count is
inconsistent (size_t above, but ssize_t below).



diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 30e37aff387..525b8bc921d 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -538,7 +538,9 @@ public:
   virtual bool is_bool () const = 0;
   virtual type *is_pointer () = 0;
   virtual type *is_array () = 0;
+  virtual struct_ *is_struct () { return NULL; }


Can't you use dyn_cast_struct for this?
Or is this about looking through decorated_type? e.g. for const and
volatile variants?

I guess my question is, what is the purpose of gcc_jit_type_is_struct?


   virtual bool is_void () const { return false; }
+  virtual vector_type *is_vector () { return NULL; }


Likewise, can't you use dyn_cast_vector_type for this?


   virtual bool has_known_size () const { return true; }

   bool is_numeric () const
@@ -595,6 +597,8 @@ public:
   bool is_bool () const FINAL OVERRIDE;
   type *is_pointer () FINAL OVERRIDE { return dereference (); }
   type *is_array () FINAL OVERRIDE { return NULL; }
+  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
+  struct_ *is_struct () FINAL OVERRIDE { return NULL; }


Likewise, and this is redundant, as it's merely copying the base class
implementation.


   bool is_void () const FINAL OVERRIDE { return m_kind == GCC_JIT_TYPE_VOID; }

 public:
@@ -629,6 +633,8 @@ public:
   bool is_bool () const FINAL OVERRIDE { return false; }
   type *is_pointer () FINAL OVERRIDE { return m_other_type; }
   type *is_array () FINAL OVERRIDE { return NULL; }
+  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
+  struct_ *is_struct () FINAL OVERRIDE { return NULL; }


Likewise.



@@ -655,6 +661,7 @@ public:
   bool is_bool () const FINAL OVERRIDE { return m_other_type->is_bool (); }
   type *is_pointer () FINAL OVERRIDE { return m_other_type->is_pointer (); }
   type *is_array () FINAL OVERRIDE { return m_other_type->is_array (); }
+  struct_ *is_struct () FINAL OVERRIDE { return m_other_type->is_struct (); }


Aha: with a decorated type you look through the decoration.


 protected:
   type *m_other_type;
@@ -737,6 +744,8 @@ public:

   void replay_into (replayer *) FINAL OVERRIDE;

+  vector_type *is_vector () FINAL OVERRIDE { return this; }
+
 private:
   string * 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-16 Thread Antoni Boucher via Gcc-patches

Hi.
Thanks for the review. See the comments below.
I attached the updated patch.

On Thu, Oct 15, 2020 at 05:52:33PM -0400, David Malcolm wrote:

On Thu, 2020-10-15 at 13:39 -0400, Antoni Boucher wrote:

Thanks. I updated the patch with these changes.


Thanks for patch; review below.  Sorry if it seems excessively nitpicky
in places.


2020-09-1  Antoni Boucher  

gcc/jit/
PR target/96889
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.


15 now.


* docs/topics/functions.rst: Add documentation for the
functions gcc_jit_function_get_return_type and
gcc_jit_function_get_param_count
* libgccjit.c: New functions:
  * gcc_jit_function_get_return_type;
  * gcc_jit_function_get_param_count;
  * gcc_jit_function_type_get_return_type;
  * gcc_jit_function_type_get_param_count;
  * gcc_jit_function_type_get_param_type;
  * gcc_jit_type_unqualified;
  * gcc_jit_type_is_array;
  * gcc_jit_type_is_bool;
  * gcc_jit_type_is_function_ptr_type;
  * gcc_jit_type_is_int;
  * gcc_jit_type_is_pointer;
  * gcc_jit_type_is_vector;
  * gcc_jit_vector_type_get_element_type;
  * gcc_jit_vector_type_get_num_units;
  * gcc_jit_struct_get_field;
  * gcc_jit_type_is_struct;
  * gcc_jit_struct_get_field_count;


This isn't valid ChangeLog format; it will fail the git hooks.


Is there any way to run that locally to check the format automatically?




* libgccjit.h


Likewise.


* jit-recording.h: New functions (is_struct and is_vector)
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.


15 now.



gcc/testsuite/
PR target/96889
* jit.dg/all-non-failing-tests.h: Add test-reflection.c.
* jit.dg/test-reflection.c: New test.


[...]



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.
+


The documentation part of the patch is incomplete: it hasn't been
updated to add all the new entrypoints.


Indeed, I wanted some feedback before I write the documentation. I'll 
write it soon.



Also, the return type of gcc_jit_function_get_param_count is
inconsistent (size_t above, but ssize_t below).



diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 30e37aff387..525b8bc921d 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -538,7 +538,9 @@ public:
   virtual bool is_bool () const = 0;
   virtual type *is_pointer () = 0;
   virtual type *is_array () = 0;
+  virtual struct_ *is_struct () { return NULL; }


Can't you use dyn_cast_struct for this?


We could, but wouldn't that be inconvenient for the user to call 
gcc_jit_type_unqualified every time they want to call a function such as 
gcc_jit_type_is_struct?
(I don't mind if you prefer to make the call to gcc_jit_type_unqualified 
explicit in user code.)



Or is this about looking through decorated_type? e.g. for const and
volatile variants?

I guess my question is, what is the purpose of gcc_jit_type_is_struct?


The purpose is to be able to get information about the fields of the 
struct, if the type is a struct.





   virtual bool is_void () const { return false; }
+  virtual vector_type *is_vector () { return NULL; }


Likewise, can't you use dyn_cast_vector_type for this?


Same as for is_struct().


   virtual bool has_known_size () const { return true; }

   bool is_numeric () const
@@ -595,6 +597,8 @@ public:
   bool is_bool () const FINAL OVERRIDE;
   type *is_pointer () FINAL OVERRIDE { return dereference (); }
   type *is_array () FINAL OVERRIDE { return NULL; }
+  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
+  struct_ *is_struct () FINAL OVERRIDE { return NULL; }


Likewise, and this is redundant, as it's merely copying the base class
implementation.


   bool is_void () const FINAL OVERRIDE { return m_kind == GCC_JIT_TYPE_VOID; }

 public:
@@ -629,6 +633,8 @@ public:
   bool is_bool () const FINAL OVERRIDE { return false; }
   type *is_pointer () FINAL OVERRIDE { return m_other_type; }
   type *is_array () FINAL OVERRIDE { return NULL; }
+  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
+  struct_ *is_struct () FINAL OVERRIDE { return NULL; }


Likewise.



@@ -655,6 +661,7 @@ public:
   bool 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-15 Thread David Malcolm via Gcc-patches
On Thu, 2020-10-15 at 13:39 -0400, Antoni Boucher wrote:
> Thanks. I updated the patch with these changes.

Thanks for patch; review below.  Sorry if it seems excessively nitpicky
in places.

> 2020-09-1  Antoni Boucher  
> 
> gcc/jit/
> PR target/96889
> * docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.

15 now.

> * docs/topics/functions.rst: Add documentation for the
> functions gcc_jit_function_get_return_type and
> gcc_jit_function_get_param_count
> * libgccjit.c: New functions:
>   * gcc_jit_function_get_return_type;
>   * gcc_jit_function_get_param_count;
>   * gcc_jit_function_type_get_return_type;
>   * gcc_jit_function_type_get_param_count;
>   * gcc_jit_function_type_get_param_type;
>   * gcc_jit_type_unqualified;
>   * gcc_jit_type_is_array;
>   * gcc_jit_type_is_bool;
>   * gcc_jit_type_is_function_ptr_type;
>   * gcc_jit_type_is_int;
>   * gcc_jit_type_is_pointer;
>   * gcc_jit_type_is_vector;
>   * gcc_jit_vector_type_get_element_type;
>   * gcc_jit_vector_type_get_num_units;
>   * gcc_jit_struct_get_field;
>   * gcc_jit_type_is_struct;
>   * gcc_jit_struct_get_field_count;

This isn't valid ChangeLog format; it will fail the git hooks.

> * libgccjit.h

Likewise.

> * jit-recording.h: New functions (is_struct and is_vector)
> * libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.

15 now.

> 
> gcc/testsuite/
> PR target/96889
> * jit.dg/all-non-failing-tests.h: Add test-reflection.c.
> * jit.dg/test-reflection.c: New test.

[...]


> 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.
> +

The documentation part of the patch is incomplete: it hasn't been
updated to add all the new entrypoints.
Also, the return type of gcc_jit_function_get_param_count is
inconsistent (size_t above, but ssize_t below).


> diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
> index 30e37aff387..525b8bc921d 100644
> --- a/gcc/jit/jit-recording.h
> +++ b/gcc/jit/jit-recording.h
> @@ -538,7 +538,9 @@ public:
>virtual bool is_bool () const = 0;
>virtual type *is_pointer () = 0;
>virtual type *is_array () = 0;
> +  virtual struct_ *is_struct () { return NULL; }

Can't you use dyn_cast_struct for this?
Or is this about looking through decorated_type? e.g. for const and
volatile variants?

I guess my question is, what is the purpose of gcc_jit_type_is_struct?

>virtual bool is_void () const { return false; }
> +  virtual vector_type *is_vector () { return NULL; }

Likewise, can't you use dyn_cast_vector_type for this?

>virtual bool has_known_size () const { return true; }
>  
>bool is_numeric () const
> @@ -595,6 +597,8 @@ public:
>bool is_bool () const FINAL OVERRIDE;
>type *is_pointer () FINAL OVERRIDE { return dereference (); }
>type *is_array () FINAL OVERRIDE { return NULL; }
> +  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
> +  struct_ *is_struct () FINAL OVERRIDE { return NULL; }

Likewise, and this is redundant, as it's merely copying the base class
implementation.

>bool is_void () const FINAL OVERRIDE { return m_kind == GCC_JIT_TYPE_VOID; 
> }
>  
>  public:
> @@ -629,6 +633,8 @@ public:
>bool is_bool () const FINAL OVERRIDE { return false; }
>type *is_pointer () FINAL OVERRIDE { return m_other_type; }
>type *is_array () FINAL OVERRIDE { return NULL; }
> +  vector_type *is_vector () FINAL OVERRIDE { return NULL; }
> +  struct_ *is_struct () FINAL OVERRIDE { return NULL; }

Likewise.


> @@ -655,6 +661,7 @@ public:
>bool is_bool () const FINAL OVERRIDE { return m_other_type->is_bool (); }
>type *is_pointer () FINAL OVERRIDE { return m_other_type->is_pointer (); }
>type *is_array () FINAL OVERRIDE { return m_other_type->is_array (); }
> +  struct_ *is_struct () FINAL OVERRIDE { return m_other_type->is_struct (); }

Aha: with a decorated type you look through the decoration.

>  protected:
>type *m_other_type;
> @@ -737,6 +744,8 @@ public:
>  
>void replay_into (replayer *) FINAL OVERRIDE;
>  
> +  vector_type *is_vector () FINAL OVERRIDE { return this; }
> +

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-15 Thread Andrea Corallo via Gcc-patches
Antoni Boucher  writes:

> Thanks. I updated the patch with these changes.
>
> Is there any tool to automatically check the style?

Yes, we have in contrib check_GNU_style.sh and check_GNU_style.py.

  Andrea


Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-15 Thread Antoni Boucher via Gcc-patches

Thanks. I updated the patch with these changes.

Is there any tool to automatically check the style?

On Thu, Oct 15, 2020 at 06:23:18PM +0200, Andrea Corallo wrote:

Antoni Boucher via Jit  writes:

Hi Antoni,

Just had a quick look, please find some quite minor comments in line.


From b0edc9eb8e8d3ba9e1c6a8d061a8627c0b0cf102 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: New functions:
  * gcc_jit_function_get_return_type;
  * gcc_jit_function_get_param_count;
  * gcc_jit_function_type_get_return_type;
  * gcc_jit_function_type_get_param_count;
  * gcc_jit_function_type_get_param_type;
  * gcc_jit_type_unqualified;
  * gcc_jit_type_is_array;
  * gcc_jit_type_is_bool;
  * gcc_jit_type_is_function_ptr_type;
  * gcc_jit_type_is_int;
  * gcc_jit_type_is_pointer;
  * gcc_jit_type_is_vector;
  * gcc_jit_vector_type_get_element_type;
  * gcc_jit_vector_type_get_num_units;
  * gcc_jit_struct_get_field;
  * gcc_jit_type_is_struct;
  * gcc_jit_struct_get_field_count;
* libgccjit.h
* jit-recording.h: New functions (is_struct and is_vector)
* 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|  41 +++
 gcc/jit/docs/topics/functions.rst|  10 +
 gcc/jit/jit-recording.h  |  16 ++
 gcc/jit/libgccjit.c  | 254 +++
 gcc/jit/libgccjit.h  | 117 +
 gcc/jit/libgccjit.map|  21 ++
 gcc/testsuite/jit.dg/all-non-failing-tests.h |  10 +
 gcc/testsuite/jit.dg/test-reflection.c   |  89 +++
 8 files changed, 558 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..16f24d20a75 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -226,3 +226,44 @@ 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`
+
+  * :func:`gcc_jit_type_is_array`
+
+  * :func:`gcc_jit_type_is_bool`
+
+  * :func:`gcc_jit_type_is_int`
+
+  * :func:`gcc_jit_type_is_pointer`
+
+  * :func:`gcc_jit_type_is_struct`
+
+  * :func:`gcc_jit_type_is_vector`
+
+  * :func:`gcc_jit_type_unqualified`
+
+  * :func:`gcc_jit_type_is_function_ptr_type`
+
+  * :func:`gcc_jit_function_type_get_return_type`
+
+  * :func:`gcc_jit_function_type_get_param_count`
+
+  * :func:`gcc_jit_function_type_get_param_type`
+
+  * :func:`gcc_jit_vector_type_get_num_units`
+
+  * :func:`gcc_jit_vector_type_get_element_type`
+
+  * :func:`gcc_jit_struct_get_field`
+
+  * :func:`gcc_jit_struct_get_field_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/jit-recording.h b/gcc/jit/jit-recording.h
index 30e37aff387..525b8bc921d 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -538,7 +538,9 @@ public:
   virtual bool is_bool () const = 0;
   virtual type *is_pointer () = 0;
   virtual type *is_array () = 0;
+  virtual struct_ *is_struct () { return NULL; }
   virtual bool is_void () const { return false; }
+  virtual vector_type *is_vector () { return NULL; }
   virtual bool has_known_size () const { return true; }

   bool 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-15 Thread Andrea Corallo via Gcc-patches
Antoni Boucher via Jit  writes:

Hi Antoni,

Just had a quick look, please find some quite minor comments in line.

> From b0edc9eb8e8d3ba9e1c6a8d061a8627c0b0cf102 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: New functions:
>   * gcc_jit_function_get_return_type;
>   * gcc_jit_function_get_param_count;
>   * gcc_jit_function_type_get_return_type;
>   * gcc_jit_function_type_get_param_count;
>   * gcc_jit_function_type_get_param_type;
>   * gcc_jit_type_unqualified;
>   * gcc_jit_type_is_array;
>   * gcc_jit_type_is_bool;
>   * gcc_jit_type_is_function_ptr_type;
>   * gcc_jit_type_is_int;
>   * gcc_jit_type_is_pointer;
>   * gcc_jit_type_is_vector;
>   * gcc_jit_vector_type_get_element_type;
>   * gcc_jit_vector_type_get_num_units;
>   * gcc_jit_struct_get_field;
>   * gcc_jit_type_is_struct;
>   * gcc_jit_struct_get_field_count;
> * libgccjit.h
> * jit-recording.h: New functions (is_struct and is_vector)
> * 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|  41 +++
>  gcc/jit/docs/topics/functions.rst|  10 +
>  gcc/jit/jit-recording.h  |  16 ++
>  gcc/jit/libgccjit.c  | 254 +++
>  gcc/jit/libgccjit.h  | 117 +
>  gcc/jit/libgccjit.map|  21 ++
>  gcc/testsuite/jit.dg/all-non-failing-tests.h |  10 +
>  gcc/testsuite/jit.dg/test-reflection.c   |  89 +++
>  8 files changed, 558 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..16f24d20a75 100644
> --- a/gcc/jit/docs/topics/compatibility.rst
> +++ b/gcc/jit/docs/topics/compatibility.rst
> @@ -226,3 +226,44 @@ 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`
> +
> +  * :func:`gcc_jit_type_is_array`
> +
> +  * :func:`gcc_jit_type_is_bool`
> +
> +  * :func:`gcc_jit_type_is_int`
> +
> +  * :func:`gcc_jit_type_is_pointer`
> +
> +  * :func:`gcc_jit_type_is_struct`
> +
> +  * :func:`gcc_jit_type_is_vector`
> +
> +  * :func:`gcc_jit_type_unqualified`
> +
> +  * :func:`gcc_jit_type_is_function_ptr_type`
> +
> +  * :func:`gcc_jit_function_type_get_return_type`
> +
> +  * :func:`gcc_jit_function_type_get_param_count`
> +
> +  * :func:`gcc_jit_function_type_get_param_type`
> +
> +  * :func:`gcc_jit_vector_type_get_num_units`
> +
> +  * :func:`gcc_jit_vector_type_get_element_type`
> +
> +  * :func:`gcc_jit_struct_get_field`
> +
> +  * :func:`gcc_jit_struct_get_field_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/jit-recording.h b/gcc/jit/jit-recording.h
> index 30e37aff387..525b8bc921d 100644
> --- a/gcc/jit/jit-recording.h
> +++ b/gcc/jit/jit-recording.h
> @@ -538,7 +538,9 @@ public:
>virtual bool is_bool () const = 0;
>virtual type *is_pointer () = 0;
>virtual type *is_array () = 0;
> +  virtual struct_ *is_struct () { return NULL; }
>virtual bool is_void () const { return false; }
> +  virtual vector_type 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-15 Thread Antoni Boucher via Gcc-patches

Hi.
I added all the functions I need in this new patch.
Please tell me if that looks good and I'll add documentation for those 
functions.

Thanks.

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


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

>From b0edc9eb8e8d3ba9e1c6a8d061a8627c0b0cf102 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: New functions:
  * gcc_jit_function_get_return_type;
  * gcc_jit_function_get_param_count;
  * gcc_jit_function_type_get_return_type;
  * gcc_jit_function_type_get_param_count;
  * gcc_jit_function_type_get_param_type;
  * gcc_jit_type_unqualified;
  * gcc_jit_type_is_array;
  * gcc_jit_type_is_bool;
  * gcc_jit_type_is_function_ptr_type;
  * gcc_jit_type_is_int;
  * gcc_jit_type_is_pointer;
  * gcc_jit_type_is_vector;
  * gcc_jit_vector_type_get_element_type;
  * gcc_jit_vector_type_get_num_units;
  * gcc_jit_struct_get_field;
  * gcc_jit_type_is_struct;
  * 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api [WIP]

2020-10-03 Thread Antoni Boucher via Gcc-patches
By the way, that seemed off to return NULL for the function returning a 
size_t to indicate an error. So I changed it to return -1 (and return 
type to ssize_t).  Is that the proper way to indicate an error?


I also wanted to mention that this patch is still a work-in-progress as 
I'm adding a few other functions.


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


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



Re: [PATCH] libgccjit: add some reflection functions in the jit C api [PR96889]

2020-10-02 Thread Antoni Boucher via Gcc-patches

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


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

>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 

Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-02 Thread David Malcolm via Gcc-patches
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
> 
> 
> > +   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



Re: [PATCH] libgccjit: add some reflection functions in the jit C api

2020-10-02 Thread David Malcolm via Gcc-patches
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


> +   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.

Dave



[PATCH] libgccjit: add some reflection functions in the jit C api

2020-09-01 Thread Antoni Boucher via Gcc-patches

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.
>From 23ab738c0d9202f6798a38fb4aa15edfcc67d11c 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.

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  | 28 
 gcc/jit/libgccjit.h  | 24 +
 gcc/jit/libgccjit.map|  8 +-
 gcc/testsuite/jit.dg/all-non-failing-tests.h | 10 +++
 gcc/testsuite/jit.dg/test-reflection.c   | 27 +++
 7 files changed, 117 insertions(+), 1 deletion(-)
 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 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`
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 50130fbbe00..e3f8dd33665 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -1012,6 +1012,34 @@ 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 (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 1c5a12e9c01..6999ce25ca2 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,
@@ -1503,6 +1511,22 @@ gcc_jit_version_minor (void);
 extern int
 gcc_jit_version_patchlevel (void);
 
+#define LIBGCCJIT_HAVE_gcc_jit_function_reflection
+
+/*