[PATCH] Allow `gcc_jit_type_get_size` to work with pointers
Hi,
Here's a little fix to allow the `gcc_jit_type_get_size` function to
work on pointer types as well.
Cordially.
From 21e6e2d5ea897fc74d0e3194973093c58157e6fa Mon Sep 17 00:00:00 2001
From: Guillaume Gomez
Date: Tue, 26 Mar 2024 17:56:36 +0100
Subject: [PATCH] [PATH] Allow `gcc_jit_type_get_size` to work with pointers
gcc/jit/ChangeLog:
* libgccjit.cc (gcc_jit_type_get_size): Add pointer support
---
gcc/jit/libgccjit.cc | 4 ++--
gcc/testsuite/jit.dg/test-pointer_size.c | 27
2 files changed, 29 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-pointer_size.c
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index a2cdc01a3a4..58d47723e38 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -575,8 +575,8 @@ gcc_jit_type_get_size (gcc_jit_type *type)
{
RETURN_VAL_IF_FAIL (type, -1, NULL, NULL, "NULL type");
RETURN_VAL_IF_FAIL
-(type->is_int () || type->is_float (), -1, NULL, NULL,
- "only getting the size of integer or floating-point types is supported for now");
+(type->is_int () || type->is_float () || type->is_pointer (), -1, NULL, NULL,
+ "only getting the size of integer or floating-point or pointer types is supported for now");
return type->get_size ();
}
diff --git a/gcc/testsuite/jit.dg/test-pointer_size.c b/gcc/testsuite/jit.dg/test-pointer_size.c
new file mode 100644
index 000..337796acc2a
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-pointer_size.c
@@ -0,0 +1,27 @@
+/* { dg-do compile { target x86_64-*-* } } */
+
+#include
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ gcc_jit_type *int_type =
+gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+ gcc_jit_type *int_ptr_type = gcc_jit_type_get_pointer (int_type);
+
+ int int_ptr_size = gcc_jit_type_get_size (int_ptr_type);
+ CHECK_VALUE (int_ptr_size, 8);
+
+ gcc_jit_type *void_type =
+gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+ gcc_jit_type *void_ptr_type = gcc_jit_type_get_pointer (void_type);
+
+ CHECK_VALUE (int_ptr_size, gcc_jit_type_get_size (void_ptr_type));
+}
--
2.24.1.2762.gfe2e4819b8
Re: [PATCH] Allow `gcc_jit_type_get_size` to work with pointers
I can push it. Thanks for the (very quick) review! Le ven. 29 mars 2024 à 18:48, David Malcolm a écrit : > > On Thu, 2024-03-28 at 23:47 +0100, Guillaume Gomez wrote: > > Hi, > > > > Here's a little fix to allow the `gcc_jit_type_get_size` function to > > work on pointer types as well. > > > Thanks, looks good to me. > > Are you able to push this, or do you want me to? > > Dave >
Re: [PATCH] Allow `gcc_jit_type_get_size` to work with pointers
Pushed as commit 4c18ace1cb69a31af4ac719850a66de79ed12e93. Le ven. 29 mars 2024 à 18:50, Guillaume Gomez a écrit : > > I can push it. Thanks for the (very quick) review! > > Le ven. 29 mars 2024 à 18:48, David Malcolm a écrit : > > > > On Thu, 2024-03-28 at 23:47 +0100, Guillaume Gomez wrote: > > > Hi, > > > > > > Here's a little fix to allow the `gcc_jit_type_get_size` function to > > > work on pointer types as well. > > > > > Thanks, looks good to me. > > > > Are you able to push this, or do you want me to? > > > > Dave > >
Re: [PATCH] Add support for function attributes and variable attributes
Hi David.
Thanks for the review!
> > +.. function:: void\
> > + gcc_jit_lvalue_add_string_attribute (gcc_jit_lvalue
> > *variable,
> > +enum
> > gcc_jit_fn_attribute attribute,
>^^
>
> This got out of sync with the declaration in the header file; it should
> be enum gcc_jit_variable_attribute attribute
Indeed, good catch!
> I took a brief look through the handler functions and with the above
> caveat I didn't see anything obviously wrong. I'm going to assume this
> code is OK given that presumably you've been testing it within rustc,
> right?
Both in rustc and in the JIT tests we added.
[..snip...]
I added all the missing `RETURN_IF_FAIL` you mentioned. None of the
arguments should be `NULL` so it was a mistake not to check it.
[..snip...]
I removed the tests comments as you mentioned.
> Please update jit.dg/all-non-failing-tests.h for the new tests; it's
> meant to list all of the (non failing) tests alphabetically.
It's not always correctly sorted. Might be worth sending a patch after this
one gets merged to fix that.
> I *think* all of the new tests aren't suitable to be run as part of a
> shared context (e.g. due to touching the optimization level or
> examining generated asm), so they should be listed in that header with
> comments explaining why.
I added them with a comment on top of each of them.
I joined the new patch version.
Thanks again for the review!
Le mar. 9 janv. 2024 à 20:59, David Malcolm a écrit :
>
> On Wed, 2023-11-15 at 17:53 +0100, Guillaume Gomez wrote:
> > Hi,
> >
> > This patch adds the (incomplete) support for function and variable
> > attributes. The added attributes are the ones we're using in
> > rustc_codegen_gcc but all the groundwork is done to add more (and we
> > will very likely add more as we didn't add all the ones we use in
> > rustc_codegen_gcc yet).
> >
> > The only big question with this patch is about `inline`. We currently
> > handle it as an attribute because it is more convenient for us but is
> > it ok or should we create a separate function to mark a function as
> > inlined?
> >
> > Thanks in advance for the review.
>
> Thanks for the patch; sorry for the delay in reviewing.
>
> At a high-level I think the API is OK as-is, but I have some nitpicks
> with the implementation:
>
> [...snip...]
>
> > diff --git a/gcc/jit/docs/topics/types.rst b/gcc/jit/docs/topics/types.rst
> > index d8c1d15d69d..6c72c99cbd9 100644
> > --- a/gcc/jit/docs/topics/types.rst
> > +++ b/gcc/jit/docs/topics/types.rst
>
> [...snip...]
>
> > +.. function:: void\
> > + gcc_jit_lvalue_add_string_attribute (gcc_jit_lvalue
> > *variable,
> > +enum
> > gcc_jit_fn_attribute attribute,
> ^^
>
> This got out of sync with the declaration in the header file; it should
> be
> enum gcc_jit_variable_attribute attribute
>
> [...snip...]
>
> > diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
> > index a729086bafb..898b4d6e7f8 100644
> > --- a/gcc/jit/dummy-frontend.cc
> > +++ b/gcc/jit/dummy-frontend.cc
>
> It's unfortunate that jit/dummy-frontend.cc has its own copy of the
> material in c-common/c-attribs.cc. I glanced through this code, and it
> seems that there are already various differences between the two copies
> in the existing code, and the patch adds more such differences.
>
> Bother - but I think this part of the patch is inevitable (and OK)
> given the existing state of attribute handling here.
>
> [...snip...]
>
> I took a brief look through the handler functions and with the above
> caveat I didn't see anything obviously wrong. I'm going to assume this
> code is OK given that presumably you've been testing it within rustcc,
> right?
>
> [..snip...]
>
> > diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
> > index 0451b4df7f9..337d4ea3b95 100644
> > --- a/gcc/jit/libgccjit.cc
> > +++ b/gcc/jit/libgccjit.cc
> > @@ -3965,6 +3965,51 @@ gcc_jit_type_get_aligned (gcc_jit_type *type,
> >return (gcc_jit_type *)type->get_aligned (alignment_in_bytes);
> > }
> >
> > +void
> > +gcc_jit_function_add_attribute (gcc_jit_function *func,
> > + gcc_jit_fn_attribute attribute)
> > +{
> > + RETURN_IF_FAIL (func, NULL, NULL, "NULL func");
> > +
> > +
Re: [PATCH] Add support for function attributes and variable attributes
Hi David, > The above looks correct, but the patch adds the entrypoint descriptions > to topics/types.rst, which seems like the wrong place. The function- > related ones should be in topics/functions.rst in the "Functions" > section and the lvalue/variable one in topics/expression.rst after the > "Global variables" section. Ah indeed. Mix-up on my end. Fixed it. > test-restrict.c is a pre-existing testcase, so please don't delete its > entry. Ah indeed, I went too quickly and thought it was a test I renamed... > BTW, the ChangeLog entry mentions adding test-restrict.c, but the patch > doesn't add it, so that part of the proposed ChangeLog is wrong. > > Does the patch pass ./contrib/gcc-changelog/git_check_commit.py ? I messed up a bit, fixed it thanks to you. I didn't run the script in my last update but just did: ``` $ contrib/gcc-changelog/git_check_commit.py $(git log -1 --format=%h) Checking 3849ee2eadf0eeec2b0080a5142ced00be96a60d: OK ``` > Otherwise, looks good, assuming that the patch has been tested with the > full jit testsuite. When rebasing on upstream yesterday I discovered that two tests were not working anymore. For the first one, it was simply because of the changes in `dummy-frontend.cc`. For the second one (test-noinline-attribute.c), it was because the rules for inlining changed since we wrote this patch apparently (our fork is very late). Antoni discovered that we could just add a call to `asm` to prevent this from happening so I added it. So yes, all jit tests are passing as expected. :) Le jeu. 11 janv. 2024 à 19:46, David Malcolm a écrit : > > On Thu, 2024-01-11 at 01:00 +0100, Guillaume Gomez wrote: > > Hi David. > > > > Thanks for the review! > > > > > > +.. function:: void\ > > > > + gcc_jit_lvalue_add_string_attribute > > > > (gcc_jit_lvalue *variable, > > > > +enum > > > > gcc_jit_fn_attribute attribute, > > > > > > ^^ > > > > > > This got out of sync with the declaration in the header file; it > > > should > > > be enum gcc_jit_variable_attribute attribute > > > > Indeed, good catch! > > > > > I took a brief look through the handler functions and with the > > > above > > > caveat I didn't see anything obviously wrong. I'm going to assume > > > this > > > code is OK given that presumably you've been testing it within > > > rustc, > > > right? > > > > Both in rustc and in the JIT tests we added. > > > > [..snip...] > > > > I added all the missing `RETURN_IF_FAIL` you mentioned. None of the > > arguments should be `NULL` so it was a mistake not to check it. > > > > [..snip...] > > > > I removed the tests comments as you mentioned. > > > > > Please update jit.dg/all-non-failing-tests.h for the new tests; > > > it's > > > meant to list all of the (non failing) tests alphabetically. > > > > It's not always correctly sorted. Might be worth sending a patch > > after this > > one gets merged to fix that. > > > > > I *think* all of the new tests aren't suitable to be run as part of > > > a > > > shared context (e.g. due to touching the optimization level or > > > examining generated asm), so they should be listed in that header > > > with > > > comments explaining why. > > > > I added them with a comment on top of each of them. > > > > I joined the new patch version. > > > > Thanks again for the review! > > Thanks for the updated patch. I noticed a few minor issues: > > > diff --git a/gcc/jit/docs/topics/types.rst b/gcc/jit/docs/topics/types.rst > > index bb51f037b7e..b1aedc03787 100644 > > --- a/gcc/jit/docs/topics/types.rst > > +++ b/gcc/jit/docs/topics/types.rst > > @@ -553,3 +553,80 @@ Reflection API > > .. code-block:: c > > > >#ifdef LIBGCCJIT_HAVE_gcc_jit_type_get_restrict > > + > > +.. function:: void\ > > + gcc_jit_function_add_attribute (gcc_jit_function *func, > > + enum gcc_jit_fn_attribute > > attribute) > > + > > + Add an attribute ``attribute`` to a function ``func``. > > + > > + This is equivalent to the following code: > > + > > + .. code-block:: c > > + > > +__attribute__((always_inline)) > > + > > + This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; you can test for > > + its presence using > > + &
Re: [PATCH] Add support for function attributes and variable attributes
> It sounds like the patch you have locally is ready, but it has some > nontrivial changes compared to the last version you posted to the list. > Please post your latest version to the list. Sure! This patch adds the support for attributes on functions and variables. It does so by adding the following functions: * gcc_jit_function_add_attribute * gcc_jit_function_add_string_attribute * gcc_jit_function_add_integer_array_attribute * gcc_jit_lvalue_add_string_attribute It adds the following types: * gcc_jit_fn_attribute * gcc_jit_variable_attribute It adds tests to ensure that the attributes are correctly applied. > Do you have push rights, or do you need me to push it for you? I have push rights so I'll merge the patch myself. But thanks for offering to do it. Le jeu. 11 janv. 2024 à 23:38, David Malcolm a écrit : > > On Thu, 2024-01-11 at 22:40 +0100, Guillaume Gomez wrote: > > Hi David, > > > > > The above looks correct, but the patch adds the entrypoint > > > descriptions > > > to topics/types.rst, which seems like the wrong place. The > > > function- > > > related ones should be in topics/functions.rst in the "Functions" > > > section and the lvalue/variable one in topics/expression.rst after > > > the > > > "Global variables" section. > > > > Ah indeed. Mix-up on my end. Fixed it. > > > > > test-restrict.c is a pre-existing testcase, so please don't delete > > > its > > > entry. > > > > Ah indeed, I went too quickly and thought it was a test I renamed... > > > > > BTW, the ChangeLog entry mentions adding test-restrict.c, but the > > > patch > > > doesn't add it, so that part of the proposed ChangeLog is wrong. > > > > > > Does the patch pass ./contrib/gcc-changelog/git_check_commit.py ? > > > > I messed up a bit, fixed it thanks to you. I didn't run the script in > > my last > > update but just did: > > > > ``` > > $ contrib/gcc-changelog/git_check_commit.py $(git log -1 --format=%h) > > Checking 3849ee2eadf0eeec2b0080a5142ced00be96a60d: OK > > ``` > > > > > Otherwise, looks good, assuming that the patch has been tested with > > > the > > > full jit testsuite. > > > > When rebasing on upstream yesterday I discovered that two tests > > were not working anymore. For the first one, it was simply because of > > the changes in `dummy-frontend.cc`. For the second one > > (test-noinline-attribute.c), it was because the rules for inlining > > changed > > since we wrote this patch apparently (our fork is very late). Antoni > > discovered > > that we could just add a call to `asm` to prevent this from happening > > so I > > added it. > > > > So yes, all jit tests are passing as expected. :) > > Good. > > It sounds like the patch you have locally is ready, but it has some > nontrivial changes compared to the last version you posted to the list. > Please post your latest version to the list. > > Do you have push rights, or do you need me to push it for you? > > Thanks > Dave > > > > > Le jeu. 11 janv. 2024 à 19:46, David Malcolm a > > écrit : > > > > > > On Thu, 2024-01-11 at 01:00 +0100, Guillaume Gomez wrote: > > > > Hi David. > > > > > > > > Thanks for the review! > > > > > > > > > > +.. function:: void\ > > > > > > + gcc_jit_lvalue_add_string_attribute > > > > > > (gcc_jit_lvalue *variable, > > > > > > +enum > > > > > > gcc_jit_fn_attribute attribute, > > > > > > > > > > ^^ > > > > > > > > > > This got out of sync with the declaration in the header file; > > > > > it > > > > > should > > > > > be enum gcc_jit_variable_attribute attribute > > > > > > > > Indeed, good catch! > > > > > > > > > I took a brief look through the handler functions and with the > > > > > above > > > > > caveat I didn't see anything obviously wrong. I'm going to > > > > > assume > > > > > this > > > > > code is OK given that presumably you've been testing it within > > > > > rustc, > > > > > right? > > > > > > > > Both in rustc and in the JIT tests we added. > > > > > > > > [..snip...] > > > > > > >
Re: [PATCH] Add support for function attributes and variable attributes
Just realized that you were asking for the patch I forgot to join... Here it is. Le ven. 12 janv. 2024 à 11:09, Guillaume Gomez a écrit : > > > It sounds like the patch you have locally is ready, but it has some > > nontrivial changes compared to the last version you posted to the list. > > Please post your latest version to the list. > > Sure! > > This patch adds the support for attributes on functions and variables. It does > so by adding the following functions: > > * gcc_jit_function_add_attribute > * gcc_jit_function_add_string_attribute > * gcc_jit_function_add_integer_array_attribute > * gcc_jit_lvalue_add_string_attribute > > It adds the following types: > > * gcc_jit_fn_attribute > * gcc_jit_variable_attribute > > It adds tests to ensure that the attributes are correctly applied. > > > Do you have push rights, or do you need me to push it for you? > > I have push rights so I'll merge the patch myself. But thanks for offering to > do it. > > Le jeu. 11 janv. 2024 à 23:38, David Malcolm a écrit : > > > > On Thu, 2024-01-11 at 22:40 +0100, Guillaume Gomez wrote: > > > Hi David, > > > > > > > The above looks correct, but the patch adds the entrypoint > > > > descriptions > > > > to topics/types.rst, which seems like the wrong place. The > > > > function- > > > > related ones should be in topics/functions.rst in the "Functions" > > > > section and the lvalue/variable one in topics/expression.rst after > > > > the > > > > "Global variables" section. > > > > > > Ah indeed. Mix-up on my end. Fixed it. > > > > > > > test-restrict.c is a pre-existing testcase, so please don't delete > > > > its > > > > entry. > > > > > > Ah indeed, I went too quickly and thought it was a test I renamed... > > > > > > > BTW, the ChangeLog entry mentions adding test-restrict.c, but the > > > > patch > > > > doesn't add it, so that part of the proposed ChangeLog is wrong. > > > > > > > > Does the patch pass ./contrib/gcc-changelog/git_check_commit.py ? > > > > > > I messed up a bit, fixed it thanks to you. I didn't run the script in > > > my last > > > update but just did: > > > > > > ``` > > > $ contrib/gcc-changelog/git_check_commit.py $(git log -1 --format=%h) > > > Checking 3849ee2eadf0eeec2b0080a5142ced00be96a60d: OK > > > ``` > > > > > > > Otherwise, looks good, assuming that the patch has been tested with > > > > the > > > > full jit testsuite. > > > > > > When rebasing on upstream yesterday I discovered that two tests > > > were not working anymore. For the first one, it was simply because of > > > the changes in `dummy-frontend.cc`. For the second one > > > (test-noinline-attribute.c), it was because the rules for inlining > > > changed > > > since we wrote this patch apparently (our fork is very late). Antoni > > > discovered > > > that we could just add a call to `asm` to prevent this from happening > > > so I > > > added it. > > > > > > So yes, all jit tests are passing as expected. :) > > > > Good. > > > > It sounds like the patch you have locally is ready, but it has some > > nontrivial changes compared to the last version you posted to the list. > > Please post your latest version to the list. > > > > Do you have push rights, or do you need me to push it for you? > > > > Thanks > > Dave > > > > > > > > Le jeu. 11 janv. 2024 à 19:46, David Malcolm a > > > écrit : > > > > > > > > On Thu, 2024-01-11 at 01:00 +0100, Guillaume Gomez wrote: > > > > > Hi David. > > > > > > > > > > Thanks for the review! > > > > > > > > > > > > +.. function:: void\ > > > > > > > + gcc_jit_lvalue_add_string_attribute > > > > > > > (gcc_jit_lvalue *variable, > > > > > > > +enum > > > > > > > gcc_jit_fn_attribute attribute, > > > > > > > > > > > > ^^ > > > > > > > > > > > > This got out of sync with the declaration in the header file; > > > > > > it > > > > > > should > > > > > > be enum gcc_jit_variable_at
Re: [PATCH] Add support for function attributes and variable attributes
Ping David. :) Le sam. 9 déc. 2023 à 12:12, Guillaume Gomez a écrit : > > Added it. > > Le jeu. 7 déc. 2023 à 18:13, Antoni Boucher a écrit : > > > > It seems like you forgot to prefix the commit message with "libgccjit: > > ". > > > > On Thu, 2023-11-30 at 10:55 +0100, Guillaume Gomez wrote: > > > Ping David. :) > > > > > > Le jeu. 23 nov. 2023 à 22:59, Antoni Boucher a > > > écrit : > > > > David: I found back the comment you made. Here it is: > > > > > > > >I see you have patches to add function and variable attributes; > > > > I > > > >wonder if this would be cleaner internally if there was a > > > >recording::attribute class, rather than the std::pair currently > > > > in > > > >use > > > >(some attributes have int arguments rather than string, others > > > > have > > > >multiple args). > > > > > > > >I also wondered if a "gcc_jit_attribute" type could be exposed > > > > to > > > >the > > > >user, e.g.: > > > > > > > > attr1 = gcc_jit_context_new_attribute (ctxt, "noreturn"); > > > > attr2 = gcc_jit_context_new_attribute_with_string (ctxt, > > > > "alias", > > > >"__foo"); > > > > gcc_jit_function_add_attribute (ctxt, attr1); > > > > gcc_jit_function_add_attribute (ctxt, attr2); > > > > > > > >or somesuch? But I think the API you currently have is OK. > > > > > > > > On Thu, 2023-11-23 at 22:52 +0100, Guillaume Gomez wrote: > > > > > Ping David. :) > > > > > > > > > > Le mer. 15 nov. 2023 à 17:56, Antoni Boucher a > > > > > écrit : > > > > > > > > > > > > David: another thing I remember you mentioned when you reviewed > > > > > > an > > > > > > earlier version of this patch is the usage of `std::pair`. > > > > > > I can't find where you said that, but I remember you mentioned > > > > > > that > > > > > > we > > > > > > should use a struct instead. > > > > > > Can you please elaborate again? > > > > > > Thanks. > > > > > > > > > > > > On Wed, 2023-11-15 at 17:53 +0100, Guillaume Gomez wrote: > > > > > > > Hi, > > > > > > > > > > > > > > This patch adds the (incomplete) support for function and > > > > > > > variable > > > > > > > attributes. The added attributes are the ones we're using in > > > > > > > rustc_codegen_gcc but all the groundwork is done to add more > > > > > > > (and > > > > > > > we > > > > > > > will very likely add more as we didn't add all the ones we > > > > > > > use in > > > > > > > rustc_codegen_gcc yet). > > > > > > > > > > > > > > The only big question with this patch is about `inline`. We > > > > > > > currently > > > > > > > handle it as an attribute because it is more convenient for > > > > > > > us > > > > > > > but is > > > > > > > it ok or should we create a separate function to mark a > > > > > > > function > > > > > > > as > > > > > > > inlined? > > > > > > > > > > > > > > Thanks in advance for the review. > > > > > > > > > > > >
Re: [PATCH] Add support for function attributes and variable attributes
Ping David. :) Le lun. 18 déc. 2023 à 23:27, Guillaume Gomez a écrit : > > Ping David. :) > > Le sam. 9 déc. 2023 à 12:12, Guillaume Gomez > a écrit : > > > > Added it. > > > > Le jeu. 7 déc. 2023 à 18:13, Antoni Boucher a écrit : > > > > > > It seems like you forgot to prefix the commit message with "libgccjit: > > > ". > > > > > > On Thu, 2023-11-30 at 10:55 +0100, Guillaume Gomez wrote: > > > > Ping David. :) > > > > > > > > Le jeu. 23 nov. 2023 à 22:59, Antoni Boucher a > > > > écrit : > > > > > David: I found back the comment you made. Here it is: > > > > > > > > > >I see you have patches to add function and variable attributes; > > > > > I > > > > >wonder if this would be cleaner internally if there was a > > > > >recording::attribute class, rather than the std::pair currently > > > > > in > > > > >use > > > > >(some attributes have int arguments rather than string, others > > > > > have > > > > >multiple args). > > > > > > > > > >I also wondered if a "gcc_jit_attribute" type could be exposed > > > > > to > > > > >the > > > > >user, e.g.: > > > > > > > > > > attr1 = gcc_jit_context_new_attribute (ctxt, "noreturn"); > > > > > attr2 = gcc_jit_context_new_attribute_with_string (ctxt, > > > > > "alias", > > > > >"__foo"); > > > > > gcc_jit_function_add_attribute (ctxt, attr1); > > > > > gcc_jit_function_add_attribute (ctxt, attr2); > > > > > > > > > >or somesuch? But I think the API you currently have is OK. > > > > > > > > > > On Thu, 2023-11-23 at 22:52 +0100, Guillaume Gomez wrote: > > > > > > Ping David. :) > > > > > > > > > > > > Le mer. 15 nov. 2023 à 17:56, Antoni Boucher a > > > > > > écrit : > > > > > > > > > > > > > > David: another thing I remember you mentioned when you reviewed > > > > > > > an > > > > > > > earlier version of this patch is the usage of `std::pair`. > > > > > > > I can't find where you said that, but I remember you mentioned > > > > > > > that > > > > > > > we > > > > > > > should use a struct instead. > > > > > > > Can you please elaborate again? > > > > > > > Thanks. > > > > > > > > > > > > > > On Wed, 2023-11-15 at 17:53 +0100, Guillaume Gomez wrote: > > > > > > > > Hi, > > > > > > > > > > > > > > > > This patch adds the (incomplete) support for function and > > > > > > > > variable > > > > > > > > attributes. The added attributes are the ones we're using in > > > > > > > > rustc_codegen_gcc but all the groundwork is done to add more > > > > > > > > (and > > > > > > > > we > > > > > > > > will very likely add more as we didn't add all the ones we > > > > > > > > use in > > > > > > > > rustc_codegen_gcc yet). > > > > > > > > > > > > > > > > The only big question with this patch is about `inline`. We > > > > > > > > currently > > > > > > > > handle it as an attribute because it is more convenient for > > > > > > > > us > > > > > > > > but is > > > > > > > > it ok or should we create a separate function to mark a > > > > > > > > function > > > > > > > > as > > > > > > > > inlined? > > > > > > > > > > > > > > > > Thanks in advance for the review. > > > > > > > > > > > > > > >
Re: [PATCH] Add support for function attributes and variable attributes
Added it. Le jeu. 7 déc. 2023 à 18:13, Antoni Boucher a écrit : > > It seems like you forgot to prefix the commit message with "libgccjit: > ". > > On Thu, 2023-11-30 at 10:55 +0100, Guillaume Gomez wrote: > > Ping David. :) > > > > Le jeu. 23 nov. 2023 à 22:59, Antoni Boucher a > > écrit : > > > David: I found back the comment you made. Here it is: > > > > > >I see you have patches to add function and variable attributes; > > > I > > >wonder if this would be cleaner internally if there was a > > >recording::attribute class, rather than the std::pair currently > > > in > > >use > > >(some attributes have int arguments rather than string, others > > > have > > >multiple args). > > > > > >I also wondered if a "gcc_jit_attribute" type could be exposed > > > to > > >the > > >user, e.g.: > > > > > > attr1 = gcc_jit_context_new_attribute (ctxt, "noreturn"); > > > attr2 = gcc_jit_context_new_attribute_with_string (ctxt, > > > "alias", > > >"__foo"); > > > gcc_jit_function_add_attribute (ctxt, attr1); > > > gcc_jit_function_add_attribute (ctxt, attr2); > > > > > >or somesuch? But I think the API you currently have is OK. > > > > > > On Thu, 2023-11-23 at 22:52 +0100, Guillaume Gomez wrote: > > > > Ping David. :) > > > > > > > > Le mer. 15 nov. 2023 à 17:56, Antoni Boucher a > > > > écrit : > > > > > > > > > > David: another thing I remember you mentioned when you reviewed > > > > > an > > > > > earlier version of this patch is the usage of `std::pair`. > > > > > I can't find where you said that, but I remember you mentioned > > > > > that > > > > > we > > > > > should use a struct instead. > > > > > Can you please elaborate again? > > > > > Thanks. > > > > > > > > > > On Wed, 2023-11-15 at 17:53 +0100, Guillaume Gomez wrote: > > > > > > Hi, > > > > > > > > > > > > This patch adds the (incomplete) support for function and > > > > > > variable > > > > > > attributes. The added attributes are the ones we're using in > > > > > > rustc_codegen_gcc but all the groundwork is done to add more > > > > > > (and > > > > > > we > > > > > > will very likely add more as we didn't add all the ones we > > > > > > use in > > > > > > rustc_codegen_gcc yet). > > > > > > > > > > > > The only big question with this patch is about `inline`. We > > > > > > currently > > > > > > handle it as an attribute because it is more convenient for > > > > > > us > > > > > > but is > > > > > > it ok or should we create a separate function to mark a > > > > > > function > > > > > > as > > > > > > inlined? > > > > > > > > > > > > Thanks in advance for the review. > > > > > > > > > From df75f0eb8aacba249b6e791603752e35778951a4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Jun 2022 14:34:39 -0400 Subject: [PATCH] libgccjit: Add support for function attributes and variable attributes. gcc/jit/ChangeLog: * dummy-frontend.cc (handle_alias_attribute): New function. (handle_always_inline_attribute): New function. (handle_cold_attribute): New function. (handle_fnspec_attribute): New function. (handle_format_arg_attribute): New function. (handle_format_attribute): New function. (handle_noinline_attribute): New function. (handle_target_attribute): New function. (handle_used_attribute): New function. (handle_visibility_attribute): New function. (handle_weak_attribute): New function. (handle_alias_ifunc_attribute): New function. * jit-playback.cc (fn_attribute_to_string): New function. (variable_attribute_to_string): New function. (global_new_decl): Add attributes support. (set_variable_attribute): New function. (new_global): Add attributes support. (new_global_initialized): Add attributes support. (new_local): Add attributes support. * jit-playback.h (fn_attribute_to_string): New function. (set_variable_attribute): New function. * jit-recording.cc (recording::lvalue::add_attribute): New function. (recording::function::function): New function. (recording:
[PATCH] Add missing declaration of get_restrict in C++ interface
Hi, This patch adds the `get_restrict` method declaration for the C++ interface as it was forgotten. Thanks in advance for the review. From e819fd01cd3e79bfab28a77f4ce78f34156e7a83 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 9 Nov 2023 17:53:08 +0100 Subject: [PATCH] Add missing declaration of get_restrict in C++ interface gcc/jit/ChangeLog: * libgccjit++.h: --- gcc/jit/libgccjit++.h | 1 + 1 file changed, 1 insertion(+) diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h index 4a04db386e6..f9a0017cae5 100644 --- a/gcc/jit/libgccjit++.h +++ b/gcc/jit/libgccjit++.h @@ -360,6 +360,7 @@ namespace gccjit type get_volatile (); type get_aligned (size_t alignment_in_bytes); type get_vector (size_t num_units); +type get_restrict (); // Shortcuts for getting values of numeric types: rvalue zero (); -- 2.34.1
Re: [PATCH] Add missing declaration of get_restrict in C++ interface
I confirm it does. I realized it when finalizing our patch for attributes support. Le jeu. 9 nov. 2023 à 21:49, David Malcolm a écrit : > > On Thu, 2023-11-09 at 21:03 +0100, Guillaume Gomez wrote: > > Hi, > > > > This patch adds the `get_restrict` method declaration for > > the C++ interface as it was forgotten. > > > > Thanks in advance for the review. > > Looking at my jit.sum results, it looks like the .cc files are indeed > FAILing on initial compilation, with errors such as: > > In file included from gcc/testsuite/jit.dg/test-alignment.cc:4: > gcc/testsuite/../jit/libgccjit++.h:1414:1: error: no declaration matches > 'gccjit::type gccjit::type::get_restrict()' > gcc/testsuite/../jit/libgccjit++.h:1414:1: note: no functions named > 'gccjit::type gccjit::type::get_restrict()' > gcc/testsuite/../jit/libgccjit++.h:350:9: note: 'class gccjit::type' defined > here > > which presumably started with r14-3552-g29763b002459cb. > > Hence the patch looks good to me - thanks! > > Does this patch fix those test cases? > > Dave >
[PATCH] Add support for function attributes and variable attributes
Hi, This patch adds the (incomplete) support for function and variable attributes. The added attributes are the ones we're using in rustc_codegen_gcc but all the groundwork is done to add more (and we will very likely add more as we didn't add all the ones we use in rustc_codegen_gcc yet). The only big question with this patch is about `inline`. We currently handle it as an attribute because it is more convenient for us but is it ok or should we create a separate function to mark a function as inlined? Thanks in advance for the review. From df75f0eb8aacba249b6e791603752e35778951a4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Jun 2022 14:34:39 -0400 Subject: [PATCH] [PATCH] Add support for function attributes and variable attributes. gcc/jit/ChangeLog: * dummy-frontend.cc (handle_alias_attribute): New function. (handle_always_inline_attribute): New function. (handle_cold_attribute): New function. (handle_fnspec_attribute): New function. (handle_format_arg_attribute): New function. (handle_format_attribute): New function. (handle_noinline_attribute): New function. (handle_target_attribute): New function. (handle_used_attribute): New function. (handle_visibility_attribute): New function. (handle_weak_attribute): New function. (handle_alias_ifunc_attribute): New function. * jit-playback.cc (fn_attribute_to_string): New function. (variable_attribute_to_string): New function. (global_new_decl): Add attributes support. (set_variable_attribute): New function. (new_global): Add attributes support. (new_global_initialized): Add attributes support. (new_local): Add attributes support. * jit-playback.h (fn_attribute_to_string): New function. (set_variable_attribute): New function. * jit-recording.cc (recording::lvalue::add_attribute): New function. (recording::function::function): New function. (recording::function::write_to_dump): Add attributes support. (recording::function::add_attribute): New function. (recording::function::add_string_attribute): New function. (recording::function::add_integer_array_attribute): New function. (recording::global::replay_into): Add attributes support. (recording::local::replay_into): Add attributes support. * libgccjit.cc (gcc_jit_function_add_attribute): New function. (gcc_jit_function_add_string_attribute): New function. (gcc_jit_function_add_integer_array_attribute): New function. (gcc_jit_lvalue_add_attribute): New function. * libgccjit.h (enum gcc_jit_fn_attribute): New enum. (gcc_jit_function_add_attribute): New function. (gcc_jit_function_add_string_attribute): New function. (gcc_jit_function_add_integer_array_attribute): New function. (enum gcc_jit_variable_attribute): New function. (gcc_jit_lvalue_add_string_attribute): New function. * libgccjit.map: Declare new functions. gcc/testsuite/ChangeLog: * jit.dg/jit.exp: Add `jit-verify-assembler-output-not` test command. * jit.dg/test-restrict.c: New test. * jit.dg/test-restrict-attribute.c: New test. * jit.dg/test-alias-attribute.c: New test. * jit.dg/test-always_inline-attribute.c: New test. * jit.dg/test-cold-attribute.c: New test. * jit.dg/test-const-attribute.c: New test. * jit.dg/test-noinline-attribute.c: New test. * jit.dg/test-nonnull-attribute.c: New test. * jit.dg/test-pure-attribute.c: New test. * jit.dg/test-used-attribute.c: New test. * jit.dg/test-variable-attribute.c: New test. * jit.dg/test-weak-attribute.c: New test. gcc/jit/ChangeLog: * docs/topics/compatibility.rst: Add documentation for LIBGCCJIT_ABI_26. * docs/topics/types.rst: Add documentation for new functions. Co-authored-by: Antoni Boucher Signed-off-by: Guillaume Gomez --- gcc/jit/docs/topics/compatibility.rst | 12 + gcc/jit/docs/topics/types.rst | 77 +++ gcc/jit/dummy-frontend.cc | 504 -- gcc/jit/jit-playback.cc | 165 +- gcc/jit/jit-playback.h| 37 +- gcc/jit/jit-recording.cc | 166 +- gcc/jit/jit-recording.h | 19 +- gcc/jit/libgccjit.cc | 45 ++ gcc/jit/libgccjit.h | 49 ++ gcc/jit/libgccjit.map | 8 + gcc/testsuite/jit.dg/jit.exp | 33 ++ gcc/testsuite/jit.dg/test-alias-attribute.c | 50 ++ .../jit.dg/test-always_inline-attribute.c | 153 ++ gcc/testsuite/jit.dg/test-cold-attribute.c| 54 ++ gcc/testsuite/jit.dg/test-const-attribute.c | 134 + .../jit.dg/test-noinline-attribute.c | 114 gcc/testsuite/jit.dg/test-nonnull-attribute.c | 94 gcc/testsuite/jit.dg/test-pure-attribute.c| 134 + ...t-restrict.c => test-restrict-attribute.c} | 4 +- gcc/testsuite/jit.dg/test-used-attribute.c| 112 .../jit.dg/test-variable-attribute.c | 46 ++ gcc/testsuite/jit.dg/test-weak-attribute.c| 41 ++ 22 files changed, 1986 inse
Re: [PATCH] Add support for function attributes and variable attributes
Ping David. :) Le mer. 15 nov. 2023 à 17:56, Antoni Boucher a écrit : > > David: another thing I remember you mentioned when you reviewed an > earlier version of this patch is the usage of `std::pair`. > I can't find where you said that, but I remember you mentioned that we > should use a struct instead. > Can you please elaborate again? > Thanks. > > On Wed, 2023-11-15 at 17:53 +0100, Guillaume Gomez wrote: > > Hi, > > > > This patch adds the (incomplete) support for function and variable > > attributes. The added attributes are the ones we're using in > > rustc_codegen_gcc but all the groundwork is done to add more (and we > > will very likely add more as we didn't add all the ones we use in > > rustc_codegen_gcc yet). > > > > The only big question with this patch is about `inline`. We currently > > handle it as an attribute because it is more convenient for us but is > > it ok or should we create a separate function to mark a function as > > inlined? > > > > Thanks in advance for the review. >
Re: [PATCH] Add support for function attributes and variable attributes
Ping David. :) Le jeu. 23 nov. 2023 à 22:59, Antoni Boucher a écrit : > David: I found back the comment you made. Here it is: > >I see you have patches to add function and variable attributes; I >wonder if this would be cleaner internally if there was a >recording::attribute class, rather than the std::pair currently in >use >(some attributes have int arguments rather than string, others have >multiple args). > >I also wondered if a "gcc_jit_attribute" type could be exposed to >the >user, e.g.: > > attr1 = gcc_jit_context_new_attribute (ctxt, "noreturn"); > attr2 = gcc_jit_context_new_attribute_with_string (ctxt, "alias", >"__foo"); > gcc_jit_function_add_attribute (ctxt, attr1); > gcc_jit_function_add_attribute (ctxt, attr2); > > or somesuch? But I think the API you currently have is OK. > > On Thu, 2023-11-23 at 22:52 +0100, Guillaume Gomez wrote: > > Ping David. :) > > > > Le mer. 15 nov. 2023 à 17:56, Antoni Boucher a > > écrit : > > > > > > David: another thing I remember you mentioned when you reviewed an > > > earlier version of this patch is the usage of `std::pair`. > > > I can't find where you said that, but I remember you mentioned that > > > we > > > should use a struct instead. > > > Can you please elaborate again? > > > Thanks. > > > > > > On Wed, 2023-11-15 at 17:53 +0100, Guillaume Gomez wrote: > > > > Hi, > > > > > > > > This patch adds the (incomplete) support for function and > > > > variable > > > > attributes. The added attributes are the ones we're using in > > > > rustc_codegen_gcc but all the groundwork is done to add more (and > > > > we > > > > will very likely add more as we didn't add all the ones we use in > > > > rustc_codegen_gcc yet). > > > > > > > > The only big question with this patch is about `inline`. We > > > > currently > > > > handle it as an attribute because it is more convenient for us > > > > but is > > > > it ok or should we create a separate function to mark a function > > > > as > > > > inlined? > > > > > > > > Thanks in advance for the review. > > > > >
[PATCH] Add rvalue::get_name method (and its C equivalent)
Hi,
I just encountered the need to retrieve the name of an `rvalue` (if
there is one) while working on the Rust GCC backend.
This patch adds a getter to retrieve the information.
Cordially.
From d2ddeec950f23533e5e18bc0c10c4b49eef3cda3 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez
Date: Sat, 20 Apr 2024 01:02:20 +0200
Subject: [PATCH] [PATCH] Add rvalue::get_name method
gcc/jit/ChangeLog:
* jit-recording.h: Add rvalue::get_name method
* libgccjit.cc (gcc_jit_rvalue_get_name): Likewise
* libgccjit.h (gcc_jit_rvalue_get_name): Likewise
* libgccjit.map: Likewise
gcc/testsuite/ChangeLog:
* jit.dg/test-tls.c: Add test for gcc_jit_rvalue_get_name
---
gcc/jit/jit-recording.h | 8
gcc/jit/libgccjit.cc| 16
gcc/jit/libgccjit.h | 4
gcc/jit/libgccjit.map | 5 +
gcc/testsuite/jit.dg/test-tls.c | 3 +++
5 files changed, 36 insertions(+)
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index d8d16f4fe29..3ae87c146ac 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1213,6 +1213,8 @@ public:
virtual bool is_constant () const { return false; }
virtual bool get_wide_int (wide_int *) const { return false; }
+ virtual string * get_name () { return NULL; }
+
private:
virtual enum precedence get_precedence () const = 0;
@@ -1305,6 +1307,8 @@ public:
const char *access_as_rvalue (reproducer &r) final override;
const char *access_as_lvalue (reproducer &r) final override;
+ string * get_name () final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
@@ -1558,6 +1562,8 @@ public:
void set_rvalue_init (rvalue *val) { m_rvalue_init = val; }
+ string * get_name () final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
template
@@ -2148,6 +2154,8 @@ public:
void write_to_dump (dump &d) final override;
+ string * get_name () final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 445c0d0e0e3..2b8706dc7fd 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -4377,3 +4377,19 @@ gcc_jit_context_add_top_level_asm (gcc_jit_context *ctxt,
RETURN_IF_FAIL (asm_stmts, ctxt, NULL, "NULL asm_stmts");
ctxt->add_top_level_asm (loc, asm_stmts);
}
+
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, this calls the trivial
+ gcc::jit::recording::rvalue::get_name method, in jit-recording.h. */
+
+extern const char *
+gcc_jit_rvalue_get_name (gcc_jit_rvalue *rvalue)
+{
+ RETURN_NULL_IF_FAIL (rvalue, NULL, NULL, "NULL rvalue");
+ auto name = rvalue->get_name ();
+
+ if (!name)
+return NULL;
+ return name->c_str ();
+}
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 74e847b2dec..d4094610a16 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -2066,6 +2066,10 @@ gcc_jit_lvalue_add_string_attribute (gcc_jit_lvalue *variable,
enum gcc_jit_variable_attribute attribute,
const char* value);
+/* Returns the name of the `rvalue`, if any. Returns NULL otherwise. */
+extern const char *
+gcc_jit_rvalue_get_name (gcc_jit_rvalue *rvalue);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 99aa5970be1..bbed8024263 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -289,3 +289,8 @@ LIBGCCJIT_ABI_27 {
global:
gcc_jit_context_new_sizeof;
} LIBGCCJIT_ABI_26;
+
+LIBGCCJIT_ABI_28 {
+ global:
+gcc_jit_rvalue_get_name;
+} LIBGCCJIT_ABI_27;
diff --git a/gcc/testsuite/jit.dg/test-tls.c b/gcc/testsuite/jit.dg/test-tls.c
index 3b20182ac10..b651eb09b44 100644
--- a/gcc/testsuite/jit.dg/test-tls.c
+++ b/gcc/testsuite/jit.dg/test-tls.c
@@ -28,6 +28,9 @@ create_code (gcc_jit_context *ctxt, void *user_data)
ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "foo");
gcc_jit_lvalue_set_tls_model (foo, GCC_JIT_TLS_MODEL_GLOBAL_DYNAMIC);
+ CHECK_STRING_VALUE (
+gcc_jit_rvalue_get_name (gcc_jit_lvalue_as_rvalue (foo)), "foo");
+
/* Build the test_fn. */
gcc_jit_function *test_fn =
gcc_jit_context_new_function (ctxt, NULL,
--
2.24.1.2762.gfe2e4819b8
Re: [PATCH] Add rvalue::get_name method (and its C equivalent)
Hey Arthur :)
> Is there any reason for that getter to return a mutable pointer to the
> name? Would something like this work instead if you're just looking at
> getting the name?
>
> + virtual string * get_name () const { return NULL; }
>
> With of course adequate modifications to the inheriting classes.
Good catch, thanks!
Updated the patch and attached the new version to this email.
Cordially.
Le lun. 22 avr. 2024 à 11:51, Arthur Cohen a écrit :
>
> Hey Guillaume :)
>
> On 4/20/24 01:05, Guillaume Gomez wrote:
> > Hi,
> >
> > I just encountered the need to retrieve the name of an `rvalue` (if
> > there is one) while working on the Rust GCC backend.
> >
> > This patch adds a getter to retrieve the information.
> >
> > Cordially.
>
> >virtual bool get_wide_int (wide_int *) const { return false; }
> >
> > + virtual string * get_name () { return NULL; }
> > +
> > private:
> >virtual enum precedence get_precedence () const = 0;
>
> Is there any reason for that getter to return a mutable pointer to the
> name? Would something like this work instead if you're just looking at
> getting the name?
>
> + virtual string * get_name () const { return NULL; }
>
> With of course adequate modifications to the inheriting classes.
>
> Best,
>
> Arthur
From a8ae68e337bec3e55a60997c5325e2270fd75962 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez
Date: Sat, 20 Apr 2024 01:02:20 +0200
Subject: [PATCH] [PATCH] Add rvalue::get_name method
gcc/jit/ChangeLog:
* jit-recording.h: Add rvalue::get_name method
* libgccjit.cc (gcc_jit_rvalue_get_name): Likewise
* libgccjit.h (gcc_jit_rvalue_get_name): Likewise
* libgccjit.map: Likewise
gcc/testsuite/ChangeLog:
* jit.dg/test-tls.c: Add test for gcc_jit_rvalue_get_name
---
gcc/jit/jit-recording.h | 8
gcc/jit/libgccjit.cc| 16
gcc/jit/libgccjit.h | 4
gcc/jit/libgccjit.map | 5 +
gcc/testsuite/jit.dg/test-tls.c | 3 +++
5 files changed, 36 insertions(+)
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index d8d16f4fe29..2190b68 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1213,6 +1213,8 @@ public:
virtual bool is_constant () const { return false; }
virtual bool get_wide_int (wide_int *) const { return false; }
+ virtual string * get_name () const { return NULL; }
+
private:
virtual enum precedence get_precedence () const = 0;
@@ -1305,6 +1307,8 @@ public:
const char *access_as_rvalue (reproducer &r) final override;
const char *access_as_lvalue (reproducer &r) final override;
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
@@ -1558,6 +1562,8 @@ public:
void set_rvalue_init (rvalue *val) { m_rvalue_init = val; }
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
template
@@ -2148,6 +2154,8 @@ public:
void write_to_dump (dump &d) final override;
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 445c0d0e0e3..2b8706dc7fd 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -4377,3 +4377,19 @@ gcc_jit_context_add_top_level_asm (gcc_jit_context *ctxt,
RETURN_IF_FAIL (asm_stmts, ctxt, NULL, "NULL asm_stmts");
ctxt->add_top_level_asm (loc, asm_stmts);
}
+
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, this calls the trivial
+ gcc::jit::recording::rvalue::get_name method, in jit-recording.h. */
+
+extern const char *
+gcc_jit_rvalue_get_name (gcc_jit_rvalue *rvalue)
+{
+ RETURN_NULL_IF_FAIL (rvalue, NULL, NULL, "NULL rvalue");
+ auto name = rvalue->get_name ();
+
+ if (!name)
+return NULL;
+ return name->c_str ();
+}
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 74e847b2dec..d4094610a16 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -2066,6 +2066,10 @@ gcc_jit_lvalue_add_string_attribute (gcc_jit_lvalue *variable,
enum gcc_jit_variable_attribute attribute,
const char* value);
+/* Returns the name of the `rvalue`, if any. Returns NULL otherwise. */
+extern const char *
+gcc_jit_rvalue_get_name (gcc_jit_rvalue *rvalue);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 99aa5970be1..bbed8024263 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -289,3 +
Re: [PATCH] Add rvalue::get_name method (and its C equivalent)
... I most definitely forgot. Thanks! ^^'
Le lun. 22 avr. 2024 à 15:19, Antoni Boucher a écrit :
> I believe you forgot to regenerate the ChangeLog.
>
> Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit :
> > Good point!
> >
> > New patch attached.
> >
> > Le lun. 22 avr. 2024 à 15:13, Antoni Boucher a écrit
> :
> >>
> >> Please move the function to be on lvalue since there are no rvalue types
> >> that are not lvalues that have a name.
> >>
> >> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
> >>> Hey Arthur :)
> >>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> + virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>
> >>> Good catch, thanks!
> >>>
> >>> Updated the patch and attached the new version to this email.
> >>>
> >>> Cordially.
> >>>
> >>> Le lun. 22 avr. 2024 à 11:51, Arthur Cohen
> a écrit :
> >>>>
> >>>> Hey Guillaume :)
> >>>>
> >>>> On 4/20/24 01:05, Guillaume Gomez wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I just encountered the need to retrieve the name of an `rvalue` (if
> >>>>> there is one) while working on the Rust GCC backend.
> >>>>>
> >>>>> This patch adds a getter to retrieve the information.
> >>>>>
> >>>>> Cordially.
> >>>>
> >>>>> virtual bool get_wide_int (wide_int *) const { return false; }
> >>>>>
> >>>>> + virtual string * get_name () { return NULL; }
> >>>>> +
> >>>>>private:
> >>>>> virtual enum precedence get_precedence () const = 0;
> >>>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> + virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>>
> >>>> Best,
> >>>>
> >>>> Arthur
>
From 79a5af70787509f0f23dc131a39ed32a88d8f8fc Mon Sep 17 00:00:00 2001
From: Guillaume Gomez
Date: Sat, 20 Apr 2024 01:02:20 +0200
Subject: [PATCH] [PATCH] Add lvalue::get_name method
gcc/jit/ChangeLog:
* jit-recording.h: Add lvalue::get_name method
* libgccjit.cc (gcc_jit_lvalue_get_name): Likewise
* libgccjit.h (gcc_jit_lvalue_get_name): Likewise
* libgccjit.map: Likewise
gcc/testsuite/ChangeLog:
* jit.dg/test-tls.c: Add test for gcc_jit_lvalue_get_name
---
gcc/jit/jit-recording.h | 7 +++
gcc/jit/libgccjit.cc| 16
gcc/jit/libgccjit.h | 4
gcc/jit/libgccjit.map | 5 +
gcc/testsuite/jit.dg/test-tls.c | 2 ++
5 files changed, 34 insertions(+)
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index d8d16f4fe29..a80327b26ba 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1267,6 +1267,7 @@ public:
void set_register_name (const char *reg_name);
void set_alignment (unsigned bytes);
unsigned get_alignment () const { return m_alignment; }
+ virtual string * get_name () const { return NULL; }
protected:
string *m_link_section;
@@ -1305,6 +1306,8 @@ public:
const char *access_as_rvalue (reproducer &r) final override;
const char *access_as_lvalue (reproducer &r) final override;
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
@@ -1558,6 +1561,8 @@ public:
void set_rvalue_init (rvalue *val) { m_rvalue_init = val; }
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
template
@@ -2148,6 +2153,8 @@ public:
void write_to_dump (dump &d) final override;
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
diff --git a/gcc/jit/libgccjit.cc b/gcc/j
Re: [PATCH] Add rvalue::get_name method (and its C equivalent)
Good point!
New patch attached.
Le lun. 22 avr. 2024 à 15:13, Antoni Boucher a écrit :
>
> Please move the function to be on lvalue since there are no rvalue types
> that are not lvalues that have a name.
>
> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
> > Hey Arthur :)
> >
> >> Is there any reason for that getter to return a mutable pointer to the
> >> name? Would something like this work instead if you're just looking at
> >> getting the name?
> >>
> >> + virtual string * get_name () const { return NULL; }
> >>
> >> With of course adequate modifications to the inheriting classes.
> >
> > Good catch, thanks!
> >
> > Updated the patch and attached the new version to this email.
> >
> > Cordially.
> >
> > Le lun. 22 avr. 2024 à 11:51, Arthur Cohen a
> > écrit :
> >>
> >> Hey Guillaume :)
> >>
> >> On 4/20/24 01:05, Guillaume Gomez wrote:
> >>> Hi,
> >>>
> >>> I just encountered the need to retrieve the name of an `rvalue` (if
> >>> there is one) while working on the Rust GCC backend.
> >>>
> >>> This patch adds a getter to retrieve the information.
> >>>
> >>> Cordially.
> >>
> >>> virtual bool get_wide_int (wide_int *) const { return false; }
> >>>
> >>> + virtual string * get_name () { return NULL; }
> >>> +
> >>> private:
> >>> virtual enum precedence get_precedence () const = 0;
> >>
> >> Is there any reason for that getter to return a mutable pointer to the
> >> name? Would something like this work instead if you're just looking at
> >> getting the name?
> >>
> >> + virtual string * get_name () const { return NULL; }
> >>
> >> With of course adequate modifications to the inheriting classes.
> >>
> >> Best,
> >>
> >> Arthur
From 79a5af70787509f0f23dc131a39ed32a88d8f8fc Mon Sep 17 00:00:00 2001
From: Guillaume Gomez
Date: Sat, 20 Apr 2024 01:02:20 +0200
Subject: [PATCH] [PATCH] Add rvalue::get_name method
gcc/jit/ChangeLog:
* jit-recording.h: Add rvalue::get_name method
* libgccjit.cc (gcc_jit_rvalue_get_name): Likewise
* libgccjit.h (gcc_jit_rvalue_get_name): Likewise
* libgccjit.map: Likewise
gcc/testsuite/ChangeLog:
* jit.dg/test-tls.c: Add test for gcc_jit_rvalue_get_name
---
gcc/jit/jit-recording.h | 7 +++
gcc/jit/libgccjit.cc| 16
gcc/jit/libgccjit.h | 4
gcc/jit/libgccjit.map | 5 +
gcc/testsuite/jit.dg/test-tls.c | 2 ++
5 files changed, 34 insertions(+)
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index d8d16f4fe29..a80327b26ba 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1267,6 +1267,7 @@ public:
void set_register_name (const char *reg_name);
void set_alignment (unsigned bytes);
unsigned get_alignment () const { return m_alignment; }
+ virtual string * get_name () const { return NULL; }
protected:
string *m_link_section;
@@ -1305,6 +1306,8 @@ public:
const char *access_as_rvalue (reproducer &r) final override;
const char *access_as_lvalue (reproducer &r) final override;
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
@@ -1558,6 +1561,8 @@ public:
void set_rvalue_init (rvalue *val) { m_rvalue_init = val; }
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
template
@@ -2148,6 +2153,8 @@ public:
void write_to_dump (dump &d) final override;
+ string * get_name () const final override { return m_name; }
+
private:
string * make_debug_string () final override { return m_name; }
void write_reproducer (reproducer &r) final override;
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 445c0d0e0e3..ea03afcd2c5 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -4377,3 +4377,19 @@ gcc_jit_context_add_top_level_asm (gcc_jit_context *ctxt,
RETURN_IF_FAIL (asm_stmts, ctxt, NULL, "NULL asm_stmts");
ctxt->add_top_level_asm (loc, asm_stmts);
}
+
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, this calls the trivial
+ gcc::jit::recording::lvalue::get_name method, in jit-recording.h. */
+
+extern const char *
+gcc_jit_lvalue_get_name (gcc_jit_lvalue *lvalue)
+{
+ RETURN_NULL_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
+ auto name = lvalue->get_name ();
+
+ if (!name)
+retu
Re: [PATCH] Add rvalue::get_name method (and its C equivalent)
`param` is also inheriting from `lvalue`. I don't think adding this
check is a good idea
because it will not evolve nicely if more changes are done in libgccjit.
Le lun. 22 avr. 2024 à 17:19, Antoni Boucher a écrit :
>
> For your new API endpoint, please add a check like:
>
>RETURN_IF_FAIL (lvalue->is_global () || lvalue->is_local (),
> NULL,
> NULL,
> "lvalue should be a variable");
>
>
> Le 2024-04-22 à 09 h 16, Guillaume Gomez a écrit :
> > Good point!
> >
> > New patch attached.
> >
> > Le lun. 22 avr. 2024 à 15:13, Antoni Boucher a écrit :
> >>
> >> Please move the function to be on lvalue since there are no rvalue types
> >> that are not lvalues that have a name.
> >>
> >> Le 2024-04-22 à 09 h 04, Guillaume Gomez a écrit :
> >>> Hey Arthur :)
> >>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> + virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>
> >>> Good catch, thanks!
> >>>
> >>> Updated the patch and attached the new version to this email.
> >>>
> >>> Cordially.
> >>>
> >>> Le lun. 22 avr. 2024 à 11:51, Arthur Cohen a
> >>> écrit :
> >>>>
> >>>> Hey Guillaume :)
> >>>>
> >>>> On 4/20/24 01:05, Guillaume Gomez wrote:
> >>>>> Hi,
> >>>>>
> >>>>> I just encountered the need to retrieve the name of an `rvalue` (if
> >>>>> there is one) while working on the Rust GCC backend.
> >>>>>
> >>>>> This patch adds a getter to retrieve the information.
> >>>>>
> >>>>> Cordially.
> >>>>
> >>>>> virtual bool get_wide_int (wide_int *) const { return false; }
> >>>>>
> >>>>> + virtual string * get_name () { return NULL; }
> >>>>> +
> >>>>>private:
> >>>>> virtual enum precedence get_precedence () const = 0;
> >>>>
> >>>> Is there any reason for that getter to return a mutable pointer to the
> >>>> name? Would something like this work instead if you're just looking at
> >>>> getting the name?
> >>>>
> >>>> + virtual string * get_name () const { return NULL; }
> >>>>
> >>>> With of course adequate modifications to the inheriting classes.
> >>>>
> >>>> Best,
> >>>>
> >>>> Arthur
[PATCH] libgccjit: Add support for `restrict` attribute on function parameters
Hi,
This patch adds the possibility to specify the __restrict__ attribute
for function parameters. It is used by the Rust GCC backend.
Thanks in advance for the review.
From 8cafadb8409094c7fc66a1073397942a60cb27b3 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez
Date: Fri, 11 Aug 2023 22:48:11 +0200
Subject: [PATCH] Add support for `restrict` attribute on function parameters
gcc/jit/Changelog:
* jit-playback.cc: Remove trailing whitespace characters.
* jit-playback.h: Add get_restrict method.
* jit-recording.cc: Add get_restrict methods.
* jit-recording.h: Add get_restrict methods.
* libgccjit++.h: Add get_restrict methods.
* libgccjit.cc: Add gcc_jit_type_get_restrict.
* libgccjit.h: Declare gcc_jit_type_get_restrict.
* libgccjit.map: Declare gcc_jit_type_get_restrict.
gcc/testsuite/ChangeLog:
* jit.dg/test-restrict.c: Add test for __restrict__ attribute.
Signed-off-by: Guillaume Gomez
---
gcc/jit/jit-playback.cc | 2 +-
gcc/jit/jit-playback.h | 5 ++
gcc/jit/jit-recording.cc | 47 +
gcc/jit/jit-recording.h | 37 -
gcc/jit/libgccjit++.h| 6 +++
gcc/jit/libgccjit.cc | 14 +
gcc/jit/libgccjit.h | 4 ++
gcc/jit/libgccjit.map| 1 +
gcc/testsuite/jit.dg/test-restrict.c | 77
9 files changed, 191 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-restrict.c
diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
index 88e1b212030..0eb4e94fdc4 100644
--- a/gcc/jit/jit-playback.cc
+++ b/gcc/jit/jit-playback.cc
@@ -3793,7 +3793,7 @@ if (t) \
NAME_TYPE (complex_float_type_node, "complex float");
NAME_TYPE (complex_double_type_node, "complex double");
NAME_TYPE (complex_long_double_type_node, "complex long double");
-
+
m_const_char_ptr = build_pointer_type(
build_qualified_type (char_type_node, TYPE_QUAL_CONST));
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index d153f4945d8..fb4f7b8b65b 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -490,6 +490,11 @@ public:
return new type (build_qualified_type (m_inner, TYPE_QUAL_VOLATILE));
}
+ type *get_restrict () const
+ {
+return new type (build_qualified_type (m_inner, TYPE_QUAL_RESTRICT));
+ }
+
type *get_aligned (size_t alignment_in_bytes) const;
type *get_vector (size_t num_units) const;
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index f962c9748c4..c5f50349311 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -2380,6 +2380,19 @@ recording::type::get_const ()
return result;
}
+/* Given a type T, get the type restrict T.
+
+ Implements the post-error-checking part of
+ gcc_jit_type_get_restrict. */
+
+recording::type *
+recording::type::get_restrict ()
+{
+ recording::type *result = new memento_of_get_restrict (this);
+ m_ctxt->record (result);
+ return result;
+}
+
/* Given a type T, get the type volatile T.
Implements the post-error-checking part of
@@ -3090,6 +3103,40 @@ recording::memento_of_get_volatile::write_reproducer (reproducer &r)
r.get_identifier_as_type (m_other_type));
}
+/* The implementation of class gcc::jit::recording::memento_of_get_restrict. */
+
+/* Implementation of pure virtual hook recording::memento::replay_into
+ for recording::memento_of_get_restrict. */
+
+void
+recording::memento_of_get_restrict::replay_into (replayer *)
+{
+ set_playback_obj (m_other_type->playback_type ()->get_restrict ());
+}
+
+/* Implementation of recording::memento::make_debug_string for
+ results of get_restrict, prepending "restrict ". */
+
+recording::string *
+recording::memento_of_get_restrict::make_debug_string ()
+{
+ return string::from_printf (m_ctxt,
+ "restrict %s", m_other_type->get_debug_string ());
+}
+
+/* Implementation of recording::memento::write_reproducer for restrict
+ types. */
+
+void
+recording::memento_of_get_restrict::write_reproducer (reproducer &r)
+{
+ const char *id = r.make_identifier (this, "type");
+ r.write (" gcc_jit_type *%s =\n"
+ "gcc_jit_type_get_restrict (%s);\n",
+ id,
+ r.get_identifier_as_type (m_other_type));
+}
+
/* The implementation of class gcc::jit::recording::memento_of_get_aligned. */
/* Implementation of pure virtual hook recording::memento::replay_into
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 929bbe37c3f..1aff22ff689 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -555,6 +555,7 @@ public:
type *get_pointer ();
type *get_const ();
type *get_volatile ();
+ type *get_restrict ();
type *get_aligned (size_t alignment_in_bytes);
type *get_vector (size_t num_units);
@@ -603,6 +604,7 @@ public:
virtual bool is_bool () const = 0;
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
My apologies, forgot to run the commit checkers. Here's the commit
with the errors fixed.
Le mer. 16 août 2023 à 18:32, Guillaume Gomez
a écrit :
>
> Hi,
>
> This patch adds the possibility to specify the __restrict__ attribute
> for function parameters. It is used by the Rust GCC backend.
>
> Thanks in advance for the review.
From 9d3a06d5c6062aa1652a28305471d7af901e8922 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez
Date: Fri, 11 Aug 2023 22:48:11 +0200
Subject: [PATCH] [PATCH] Add support for `restrict` attribute on function
parameters
gcc/jit/Changelog:
* jit-playback.cc: Remove trailing whitespace characters.
* jit-playback.h: Add get_restrict method.
* jit-recording.cc: Add get_restrict methods.
* jit-recording.h: Add get_restrict methods.
* libgccjit++.h: Add get_restrict methods.
* libgccjit.cc: Add gcc_jit_type_get_restrict.
* libgccjit.h: Declare gcc_jit_type_get_restrict.
* libgccjit.map: Declare gcc_jit_type_get_restrict.
gcc/testsuite/ChangeLog:
* jit.dg/test-restrict.c: Add test for __restrict__ attribute.
Signed-off-by: Guillaume Gomez
---
gcc/jit/jit-playback.cc | 2 +-
gcc/jit/jit-playback.h | 5 ++
gcc/jit/jit-recording.cc | 47 +
gcc/jit/jit-recording.h | 39 +-
gcc/jit/libgccjit++.h| 6 +++
gcc/jit/libgccjit.cc | 14 +
gcc/jit/libgccjit.h | 4 ++
gcc/jit/libgccjit.map| 1 +
gcc/testsuite/jit.dg/test-restrict.c | 77
9 files changed, 192 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-restrict.c
diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
index 88e1b212030..0eb4e94fdc4 100644
--- a/gcc/jit/jit-playback.cc
+++ b/gcc/jit/jit-playback.cc
@@ -3793,7 +3793,7 @@ if (t) \
NAME_TYPE (complex_float_type_node, "complex float");
NAME_TYPE (complex_double_type_node, "complex double");
NAME_TYPE (complex_long_double_type_node, "complex long double");
-
+
m_const_char_ptr = build_pointer_type(
build_qualified_type (char_type_node, TYPE_QUAL_CONST));
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index d153f4945d8..fb4f7b8b65b 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -490,6 +490,11 @@ public:
return new type (build_qualified_type (m_inner, TYPE_QUAL_VOLATILE));
}
+ type *get_restrict () const
+ {
+return new type (build_qualified_type (m_inner, TYPE_QUAL_RESTRICT));
+ }
+
type *get_aligned (size_t alignment_in_bytes) const;
type *get_vector (size_t num_units) const;
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index f962c9748c4..f1ac8084522 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -2380,6 +2380,19 @@ recording::type::get_const ()
return result;
}
+/* Given a type T, get the type restrict T.
+
+ Implements the post-error-checking part of
+ gcc_jit_type_get_restrict. */
+
+recording::type *
+recording::type::get_restrict ()
+{
+ recording::type *result = new memento_of_get_restrict (this);
+ m_ctxt->record (result);
+ return result;
+}
+
/* Given a type T, get the type volatile T.
Implements the post-error-checking part of
@@ -3090,6 +3103,40 @@ recording::memento_of_get_volatile::write_reproducer (reproducer &r)
r.get_identifier_as_type (m_other_type));
}
+/* The implementation of class gcc::jit::recording::memento_of_get_restrict. */
+
+/* Implementation of pure virtual hook recording::memento::replay_into
+ for recording::memento_of_get_restrict. */
+
+void
+recording::memento_of_get_restrict::replay_into (replayer *)
+{
+ set_playback_obj (m_other_type->playback_type ()->get_restrict ());
+}
+
+/* Implementation of recording::memento::make_debug_string for
+ results of get_restrict, prepending "restrict ". */
+
+recording::string *
+recording::memento_of_get_restrict::make_debug_string ()
+{
+ return string::from_printf (m_ctxt,
+ "restrict %s", m_other_type->get_debug_string ());
+}
+
+/* Implementation of recording::memento::write_reproducer for restrict
+ types. */
+
+void
+recording::memento_of_get_restrict::write_reproducer (reproducer &r)
+{
+ const char *id = r.make_identifier (this, "type");
+ r.write (" gcc_jit_type *%s =\n"
+ "gcc_jit_type_get_restrict (%s);\n",
+ id,
+ r.get_identifier_as_type (m_other_type));
+}
+
/* The implementation of class gcc::jit::recording::memento_of_get_aligned. */
/* Implementation of pure virtual hook recording::memento::replay_into
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 929bbe37c3f..0f20bbacff2 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -555,6 +555,7 @@ public:
type *get_pointer ();
type *get_const ();
type *get_
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
Hi Dave,
> What kind of testing has the patch had? (e.g. did you run "make check-
> jit" ? Has this been in use on real Rust code?)
I tested it as Rust backend directly on this code:
```
pub fn foo(a: &mut i32, b: &mut i32, c: &i32) {
*a += *c;
*b += *c;
}
```
I ran it with `rustc` (and the GCC backend) with the following flags:
`-C link-args=-lc --emit=asm -O --crate-type=lib` which gave the diff
you can see in the attached file. Explanations: the diff on the right
has the `__restrict__` attribute used whereas on the left it is the
current version where we don't handle it.
As for C testing, I used this code:
```
void t(int *__restrict__ a, int *__restrict__ b, char *__restrict__ c) {
*a += *c;
*b += *c;
}
```
(without the `__restrict__` of course when I need to have a witness
ASM). I attached the diff as well, this time the file with the use of
`__restrict__` in on the left. I compiled with the following flags:
`-S -O3`.
> Please add a feature macro:
> #define LIBGCCJIT_HAVE_gcc_jit_type_get_restrict
> (see the similar ones in the header).
I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` and extended the
documentation as well to mention the ABI change.
> Please add a new ABI tag (LIBGCCJIT_ABI_25 ?), rather than adding this
> to ABI_0.
I added `LIBGCCJIT_ABI_34` as `LIBGCCJIT_ABI_33` was the last one.
> This refers to a "cold attribute"; is this a vestige of a copy-and-
> paste from a different test case?
It is a vestige indeed... Missed this one.
> I see that the test scans the generated assembler. Does the test
> actually verify that restrict has an effect, or was that another
> vestige from a different test case?
No, this time it's what I wanted. Please see the C diff I provided
above to see that the ASM has a small diff that allowed me to confirm
that the `__restrict__` attribute was correctly set.
> If this test is meant to run at -O3 and thus can't be part of test-
> combination.c, please add a comment about it to
> gcc/testsuite/jit.dg/all-non-failing-tests.h (in the alphabetical
> place).
Below `-O3`, this ASM difference doesn't appear unfortunately.
> The patch also needs to add documentation for the new entrypoint (in
> topics/types.rst), and for the new ABI tag (in
> topics/compatibility.rst).
Added!
> Thanks again for the patch; hope the above is constructive
It was incredibly useful! Thanks for taking time to writing down the
explanations.
The new patch is attached to this email.
Cordially.
Le jeu. 17 août 2023 à 01:06, David Malcolm a écrit :
>
> On Wed, 2023-08-16 at 22:06 +0200, Guillaume Gomez via Jit wrote:
> > My apologies, forgot to run the commit checkers. Here's the commit
> > with the errors fixed.
> >
> > Le mer. 16 août 2023 à 18:32, Guillaume Gomez
> > a écrit :
> > >
> > > Hi,
>
> Hi Guillaume, thanks for the patch.
>
> > >
> > > This patch adds the possibility to specify the __restrict__
> > > attribute
> > > for function parameters. It is used by the Rust GCC backend.
>
> What kind of testing has the patch had? (e.g. did you run "make check-
> jit" ? Has this been in use on real Rust code?)
>
> Overall, this patch looks close to being ready, but some nits below...
>
> [...]
>
> > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
> > index 60eaf39bff6..2e0d08a06d8 100644
> > --- a/gcc/jit/libgccjit.h
> > +++ b/gcc/jit/libgccjit.h
> > @@ -635,6 +635,10 @@ gcc_jit_type_get_const (gcc_jit_type *type);
> > extern gcc_jit_type *
> > gcc_jit_type_get_volatile (gcc_jit_type *type);
> >
> > +/* Given type "T", get type "restrict T". */
> > +extern gcc_jit_type *
> > +gcc_jit_type_get_restrict (gcc_jit_type *type);
> > +
> > #define LIBGCCJIT_HAVE_SIZED_INTEGERS
> >
> > /* Given types LTYPE and RTYPE, return non-zero if they are
> compatible.
>
> Please add a feature macro:
> #define LIBGCCJIT_HAVE_gcc_jit_type_get_restrict
> (see the similar ones in the header).
>
> > diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
> > index e52de0057a5..b7289b13845 100644
> > --- a/gcc/jit/libgccjit.map
> > +++ b/gcc/jit/libgccjit.map
> > @@ -104,6 +104,7 @@ LIBGCCJIT_ABI_0
> > gcc_jit_type_as_object;
> > gcc_jit_type_get_const;
> > gcc_jit_type_get_pointer;
> > +gcc_jit_type_get_restrict;
> > gcc_jit_type_get_volatile;
>
> Please add a new ABI tag (LIBGCCJIT_ABI_25 ?), rather than adding this
> to ABI_0.
>
> > diff --git a/gcc/testsuite/jit.dg/test-restrict.c
> b/gcc/testsuite/jit.dg/test-restrict.c
> > new file mode 100644
> > index 000..4
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
Antoni spot a typo I made:
I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` instead of
`LIBGCCJIT_HAVE_gcc_jit_type_get_restrict`. Fixed in this patch, sorry
for the noise.
Le jeu. 17 août 2023 à 11:30, Guillaume Gomez
a écrit :
>
> Hi Dave,
>
> > What kind of testing has the patch had? (e.g. did you run "make check-
> > jit" ? Has this been in use on real Rust code?)
>
> I tested it as Rust backend directly on this code:
>
> ```
> pub fn foo(a: &mut i32, b: &mut i32, c: &i32) {
> *a += *c;
> *b += *c;
> }
> ```
>
> I ran it with `rustc` (and the GCC backend) with the following flags:
> `-C link-args=-lc --emit=asm -O --crate-type=lib` which gave the diff
> you can see in the attached file. Explanations: the diff on the right
> has the `__restrict__` attribute used whereas on the left it is the
> current version where we don't handle it.
>
> As for C testing, I used this code:
>
> ```
> void t(int *__restrict__ a, int *__restrict__ b, char *__restrict__ c) {
> *a += *c;
> *b += *c;
> }
> ```
>
> (without the `__restrict__` of course when I need to have a witness
> ASM). I attached the diff as well, this time the file with the use of
> `__restrict__` in on the left. I compiled with the following flags:
> `-S -O3`.
>
> > Please add a feature macro:
> > #define LIBGCCJIT_HAVE_gcc_jit_type_get_restrict
> > (see the similar ones in the header).
>
> I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` and extended the
> documentation as well to mention the ABI change.
>
> > Please add a new ABI tag (LIBGCCJIT_ABI_25 ?), rather than adding this
> > to ABI_0.
>
> I added `LIBGCCJIT_ABI_34` as `LIBGCCJIT_ABI_33` was the last one.
>
> > This refers to a "cold attribute"; is this a vestige of a copy-and-
> > paste from a different test case?
>
> It is a vestige indeed... Missed this one.
>
> > I see that the test scans the generated assembler. Does the test
> > actually verify that restrict has an effect, or was that another
> > vestige from a different test case?
>
> No, this time it's what I wanted. Please see the C diff I provided
> above to see that the ASM has a small diff that allowed me to confirm
> that the `__restrict__` attribute was correctly set.
>
> > If this test is meant to run at -O3 and thus can't be part of test-
> > combination.c, please add a comment about it to
> > gcc/testsuite/jit.dg/all-non-failing-tests.h (in the alphabetical
> > place).
>
> Below `-O3`, this ASM difference doesn't appear unfortunately.
>
> > The patch also needs to add documentation for the new entrypoint (in
> > topics/types.rst), and for the new ABI tag (in
> > topics/compatibility.rst).
>
> Added!
>
> > Thanks again for the patch; hope the above is constructive
>
> It was incredibly useful! Thanks for taking time to writing down the
> explanations.
>
> The new patch is attached to this email.
>
> Cordially.
>
> Le jeu. 17 août 2023 à 01:06, David Malcolm a écrit :
> >
> > On Wed, 2023-08-16 at 22:06 +0200, Guillaume Gomez via Jit wrote:
> > > My apologies, forgot to run the commit checkers. Here's the commit
> > > with the errors fixed.
> > >
> > > Le mer. 16 août 2023 à 18:32, Guillaume Gomez
> > > a écrit :
> > > >
> > > > Hi,
> >
> > Hi Guillaume, thanks for the patch.
> >
> > > >
> > > > This patch adds the possibility to specify the __restrict__
> > > > attribute
> > > > for function parameters. It is used by the Rust GCC backend.
> >
> > What kind of testing has the patch had? (e.g. did you run "make check-
> > jit" ? Has this been in use on real Rust code?)
> >
> > Overall, this patch looks close to being ready, but some nits below...
> >
> > [...]
> >
> > > diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
> > > index 60eaf39bff6..2e0d08a06d8 100644
> > > --- a/gcc/jit/libgccjit.h
> > > +++ b/gcc/jit/libgccjit.h
> > > @@ -635,6 +635,10 @@ gcc_jit_type_get_const (gcc_jit_type *type);
> > > extern gcc_jit_type *
> > > gcc_jit_type_get_volatile (gcc_jit_type *type);
> > >
> > > +/* Given type "T", get type "restrict T". */
> > > +extern gcc_jit_type *
> > > +gcc_jit_type_get_restrict (gcc_jit_type *type);
> > > +
> > > #define LIBGCCJIT_HAVE_SIZED_INTEGERS
> > >
> > > /* Given types LTYPE and RTYPE, return non-zero if they are
> > compatible.
> >
> > Please
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
And now I just discovered that a lot of commits from Antoni's fork
haven't been sent upstream which is why the ABI count is so high in
his repository. Fixed that as well.
Le jeu. 17 août 2023 à 17:26, Guillaume Gomez
a écrit :
>
> Antoni spot a typo I made:
>
> I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` instead of
> `LIBGCCJIT_HAVE_gcc_jit_type_get_restrict`. Fixed in this patch, sorry
> for the noise.
>
> Le jeu. 17 août 2023 à 11:30, Guillaume Gomez
> a écrit :
> >
> > Hi Dave,
> >
> > > What kind of testing has the patch had? (e.g. did you run "make check-
> > > jit" ? Has this been in use on real Rust code?)
> >
> > I tested it as Rust backend directly on this code:
> >
> > ```
> > pub fn foo(a: &mut i32, b: &mut i32, c: &i32) {
> > *a += *c;
> > *b += *c;
> > }
> > ```
> >
> > I ran it with `rustc` (and the GCC backend) with the following flags:
> > `-C link-args=-lc --emit=asm -O --crate-type=lib` which gave the diff
> > you can see in the attached file. Explanations: the diff on the right
> > has the `__restrict__` attribute used whereas on the left it is the
> > current version where we don't handle it.
> >
> > As for C testing, I used this code:
> >
> > ```
> > void t(int *__restrict__ a, int *__restrict__ b, char *__restrict__ c) {
> > *a += *c;
> > *b += *c;
> > }
> > ```
> >
> > (without the `__restrict__` of course when I need to have a witness
> > ASM). I attached the diff as well, this time the file with the use of
> > `__restrict__` in on the left. I compiled with the following flags:
> > `-S -O3`.
> >
> > > Please add a feature macro:
> > > #define LIBGCCJIT_HAVE_gcc_jit_type_get_restrict
> > > (see the similar ones in the header).
> >
> > I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` and extended the
> > documentation as well to mention the ABI change.
> >
> > > Please add a new ABI tag (LIBGCCJIT_ABI_25 ?), rather than adding this
> > > to ABI_0.
> >
> > I added `LIBGCCJIT_ABI_34` as `LIBGCCJIT_ABI_33` was the last one.
> >
> > > This refers to a "cold attribute"; is this a vestige of a copy-and-
> > > paste from a different test case?
> >
> > It is a vestige indeed... Missed this one.
> >
> > > I see that the test scans the generated assembler. Does the test
> > > actually verify that restrict has an effect, or was that another
> > > vestige from a different test case?
> >
> > No, this time it's what I wanted. Please see the C diff I provided
> > above to see that the ASM has a small diff that allowed me to confirm
> > that the `__restrict__` attribute was correctly set.
> >
> > > If this test is meant to run at -O3 and thus can't be part of test-
> > > combination.c, please add a comment about it to
> > > gcc/testsuite/jit.dg/all-non-failing-tests.h (in the alphabetical
> > > place).
> >
> > Below `-O3`, this ASM difference doesn't appear unfortunately.
> >
> > > The patch also needs to add documentation for the new entrypoint (in
> > > topics/types.rst), and for the new ABI tag (in
> > > topics/compatibility.rst).
> >
> > Added!
> >
> > > Thanks again for the patch; hope the above is constructive
> >
> > It was incredibly useful! Thanks for taking time to writing down the
> > explanations.
> >
> > The new patch is attached to this email.
> >
> > Cordially.
> >
> > Le jeu. 17 août 2023 à 01:06, David Malcolm a écrit :
> > >
> > > On Wed, 2023-08-16 at 22:06 +0200, Guillaume Gomez via Jit wrote:
> > > > My apologies, forgot to run the commit checkers. Here's the commit
> > > > with the errors fixed.
> > > >
> > > > Le mer. 16 août 2023 à 18:32, Guillaume Gomez
> > > > a écrit :
> > > > >
> > > > > Hi,
> > >
> > > Hi Guillaume, thanks for the patch.
> > >
> > > > >
> > > > > This patch adds the possibility to specify the __restrict__
> > > > > attribute
> > > > > for function parameters. It is used by the Rust GCC backend.
> > >
> > > What kind of testing has the patch had? (e.g. did you run "make check-
> > > jit" ? Has this been in use on real Rust code?)
> > >
> > > Overall, this patch looks close to being ready, but some nits below...
> > >
> > > [...]
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
Thanks for the review!
Le jeu. 17 août 2023 à 17:50, David Malcolm a écrit :
>
> On Thu, 2023-08-17 at 17:41 +0200, Guillaume Gomez wrote:
> > And now I just discovered that a lot of commits from Antoni's fork
> > haven't been sent upstream which is why the ABI count is so high in
> > his repository. Fixed that as well.
>
> Thanks for the updated patch; I was about to comment on that.
>
> This version is good for gcc trunk.
>
> Dave
>
> >
> > Le jeu. 17 août 2023 à 17:26, Guillaume Gomez
> > a écrit :
> > >
> > > Antoni spot a typo I made:
> > >
> > > I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` instead of
> > > `LIBGCCJIT_HAVE_gcc_jit_type_get_restrict`. Fixed in this patch,
> > > sorry
> > > for the noise.
> > >
> > > Le jeu. 17 août 2023 à 11:30, Guillaume Gomez
> > > a écrit :
> > > >
> > > > Hi Dave,
> > > >
> > > > > What kind of testing has the patch had? (e.g. did you run "make
> > > > > check-
> > > > > jit" ? Has this been in use on real Rust code?)
> > > >
> > > > I tested it as Rust backend directly on this code:
> > > >
> > > > ```
> > > > pub fn foo(a: &mut i32, b: &mut i32, c: &i32) {
> > > > *a += *c;
> > > > *b += *c;
> > > > }
> > > > ```
> > > >
> > > > I ran it with `rustc` (and the GCC backend) with the following
> > > > flags:
> > > > `-C link-args=-lc --emit=asm -O --crate-type=lib` which gave the
> > > > diff
> > > > you can see in the attached file. Explanations: the diff on the
> > > > right
> > > > has the `__restrict__` attribute used whereas on the left it is
> > > > the
> > > > current version where we don't handle it.
> > > >
> > > > As for C testing, I used this code:
> > > >
> > > > ```
> > > > void t(int *__restrict__ a, int *__restrict__ b, char
> > > > *__restrict__ c) {
> > > > *a += *c;
> > > > *b += *c;
> > > > }
> > > > ```
> > > >
> > > > (without the `__restrict__` of course when I need to have a
> > > > witness
> > > > ASM). I attached the diff as well, this time the file with the
> > > > use of
> > > > `__restrict__` in on the left. I compiled with the following
> > > > flags:
> > > > `-S -O3`.
> > > >
> > > > > Please add a feature macro:
> > > > > #define LIBGCCJIT_HAVE_gcc_jit_type_get_restrict
> > > > > (see the similar ones in the header).
> > > >
> > > > I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` and extended the
> > > > documentation as well to mention the ABI change.
> > > >
> > > > > Please add a new ABI tag (LIBGCCJIT_ABI_25 ?), rather than
> > > > > adding this
> > > > > to ABI_0.
> > > >
> > > > I added `LIBGCCJIT_ABI_34` as `LIBGCCJIT_ABI_33` was the last
> > > > one.
> > > >
> > > > > This refers to a "cold attribute"; is this a vestige of a copy-
> > > > > and-
> > > > > paste from a different test case?
> > > >
> > > > It is a vestige indeed... Missed this one.
> > > >
> > > > > I see that the test scans the generated assembler. Does the
> > > > > test
> > > > > actually verify that restrict has an effect, or was that
> > > > > another
> > > > > vestige from a different test case?
> > > >
> > > > No, this time it's what I wanted. Please see the C diff I
> > > > provided
> > > > above to see that the ASM has a small diff that allowed me to
> > > > confirm
> > > > that the `__restrict__` attribute was correctly set.
> > > >
> > > > > If this test is meant to run at -O3 and thus can't be part of
> > > > > test-
> > > > > combination.c, please add a comment about it to
> > > > > gcc/testsuite/jit.dg/all-non-failing-tests.h (in the
> > > > > alphabetical
> > > > > place).
> > > >
> > > > Below `-O3`, this ASM difference doesn't appear unfortunately.
> > > >
> > > > > The patch also needs to add documentation for the new
> &
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
Quick question: do you plan to make the merge or should I ask Antoni?
Le jeu. 17 août 2023 à 17:59, Guillaume Gomez
a écrit :
> Thanks for the review!
>
> Le jeu. 17 août 2023 à 17:50, David Malcolm a écrit
> :
> >
> > On Thu, 2023-08-17 at 17:41 +0200, Guillaume Gomez wrote:
> > > And now I just discovered that a lot of commits from Antoni's fork
> > > haven't been sent upstream which is why the ABI count is so high in
> > > his repository. Fixed that as well.
> >
> > Thanks for the updated patch; I was about to comment on that.
> >
> > This version is good for gcc trunk.
> >
> > Dave
> >
> > >
> > > Le jeu. 17 août 2023 à 17:26, Guillaume Gomez
> > > a écrit :
> > > >
> > > > Antoni spot a typo I made:
> > > >
> > > > I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` instead of
> > > > `LIBGCCJIT_HAVE_gcc_jit_type_get_restrict`. Fixed in this patch,
> > > > sorry
> > > > for the noise.
> > > >
> > > > Le jeu. 17 août 2023 à 11:30, Guillaume Gomez
> > > > a écrit :
> > > > >
> > > > > Hi Dave,
> > > > >
> > > > > > What kind of testing has the patch had? (e.g. did you run "make
> > > > > > check-
> > > > > > jit" ? Has this been in use on real Rust code?)
> > > > >
> > > > > I tested it as Rust backend directly on this code:
> > > > >
> > > > > ```
> > > > > pub fn foo(a: &mut i32, b: &mut i32, c: &i32) {
> > > > > *a += *c;
> > > > > *b += *c;
> > > > > }
> > > > > ```
> > > > >
> > > > > I ran it with `rustc` (and the GCC backend) with the following
> > > > > flags:
> > > > > `-C link-args=-lc --emit=asm -O --crate-type=lib` which gave the
> > > > > diff
> > > > > you can see in the attached file. Explanations: the diff on the
> > > > > right
> > > > > has the `__restrict__` attribute used whereas on the left it is
> > > > > the
> > > > > current version where we don't handle it.
> > > > >
> > > > > As for C testing, I used this code:
> > > > >
> > > > > ```
> > > > > void t(int *__restrict__ a, int *__restrict__ b, char
> > > > > *__restrict__ c) {
> > > > > *a += *c;
> > > > > *b += *c;
> > > > > }
> > > > > ```
> > > > >
> > > > > (without the `__restrict__` of course when I need to have a
> > > > > witness
> > > > > ASM). I attached the diff as well, this time the file with the
> > > > > use of
> > > > > `__restrict__` in on the left. I compiled with the following
> > > > > flags:
> > > > > `-S -O3`.
> > > > >
> > > > > > Please add a feature macro:
> > > > > > #define LIBGCCJIT_HAVE_gcc_jit_type_get_restrict
> > > > > > (see the similar ones in the header).
> > > > >
> > > > > I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` and extended the
> > > > > documentation as well to mention the ABI change.
> > > > >
> > > > > > Please add a new ABI tag (LIBGCCJIT_ABI_25 ?), rather than
> > > > > > adding this
> > > > > > to ABI_0.
> > > > >
> > > > > I added `LIBGCCJIT_ABI_34` as `LIBGCCJIT_ABI_33` was the last
> > > > > one.
> > > > >
> > > > > > This refers to a "cold attribute"; is this a vestige of a copy-
> > > > > > and-
> > > > > > paste from a different test case?
> > > > >
> > > > > It is a vestige indeed... Missed this one.
> > > > >
> > > > > > I see that the test scans the generated assembler. Does the
> > > > > > test
> > > > > > actually verify that restrict has an effect, or was that
> > > > > > another
> > > > > > vestige from a different test case?
> > > > >
> > > > > No, this time it's what I wanted. Please see the C diff I
> > > > > provided
> > > > > above to see that the ASM has a small diff that allowed me to
> > > > > confirm
&
Re: [PATCH] libgccjit: Fix a failing test
Ping David. Le lun. 16 janv. 2023 à 15:08, Guillaume Gomez a écrit : > Ping David. > > Le jeu. 5 janv. 2023 à 23:37, Guillaume Gomez > a écrit : > >> Ping David. >> >> Le sam. 24 déc. 2022 à 21:01, Guillaume Gomez >> a écrit : >> >>> Ping David >>> >>> Le jeu. 15 déc. 2022 à 11:34, Guillaume Gomez < >>> guillaume1.go...@gmail.com> a écrit : >>> >>>> Forgot it indeed, thanks for notifying me! >>>> >>>> I modified the commit message to add it and added it into this email. >>>> >>>> Le mer. 14 déc. 2022 à 16:12, Antoni Boucher a >>>> écrit : >>>> >>>>> Thanks! >>>>> >>>>> In your patch, you're missing this line at the end of the commit >>>>> message: >>>>> >>>>>Signed-off-by: Guillaume Gomez >>>>> >>>>> On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit wrote: >>>>> > Hi, >>>>> > >>>>> > This fixes bug 107999. >>>>> > >>>>> > Thanks in advance for the review. >>>>> >>>>>
Re: [PATCH] libgccjit: Fix a failing test
No problem, thanks for the explanations. I joined the patch with the fixed commit message. Le jeu. 2 mars 2023 à 22:58, David Malcolm a écrit : > On Thu, 2022-12-15 at 08:34 +0100, Guillaume Gomez via Jit wrote: > > Forgot it indeed, thanks for notifying me! > > > > I modified the commit message to add it and added it into this email. > > Sorry about the delay in reviewing this; for some reason I didn't see > the mail. > > The patch looks good for trunk, but please add a reference to > PR jit/107999 > to the subject line and ChangeLog message. > > Dave > > > > > Le mer. 14 déc. 2022 à 16:12, Antoni Boucher a > > écrit : > > > > > Thanks! > > > > > > In your patch, you're missing this line at the end of the commit > > > message: > > > > > >Signed-off-by: Guillaume Gomez > > > > > > On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit wrote: > > > > Hi, > > > > > > > > This fixes bug 107999. > > > > > > > > Thanks in advance for the review. > > > > > > > > From 985228a76feecf16658b95a012e0b531e7e5c750 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Dec 2022 14:28:22 +0100 Subject: [PATCH] [PATCH] Fix a failing test by updating its error string [PR107999] gcc/testsuite/ChangeLog: PR jit/107999 * jit.dg/test-error-array-bounds.c: Update test. Signed-off-by: Guillaume Gomez --- gcc/testsuite/jit.dg/test-error-array-bounds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/jit.dg/test-error-array-bounds.c b/gcc/testsuite/jit.dg/test-error-array-bounds.c index b6c0ee526d4..a0dead13cb7 100644 --- a/gcc/testsuite/jit.dg/test-error-array-bounds.c +++ b/gcc/testsuite/jit.dg/test-error-array-bounds.c @@ -70,5 +70,5 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) /* ...and that the message was captured by the API. */ CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), "array subscript 10 is above array bounds of" - " 'char[10]' [-Warray-bounds]"); + " 'char[10]' [-Warray-bounds=]"); } -- 2.34.1
Re: [PATCH] libgccjit: Fix a failing test
Just realized I used whitespace and not a tab. Sorry about that. Here's the fixed version... Le jeu. 2 mars 2023 à 23:19, Guillaume Gomez a écrit : > No problem, thanks for the explanations. > > I joined the patch with the fixed commit message. > > Le jeu. 2 mars 2023 à 22:58, David Malcolm a écrit : > >> On Thu, 2022-12-15 at 08:34 +0100, Guillaume Gomez via Jit wrote: >> > Forgot it indeed, thanks for notifying me! >> > >> > I modified the commit message to add it and added it into this email. >> >> Sorry about the delay in reviewing this; for some reason I didn't see >> the mail. >> >> The patch looks good for trunk, but please add a reference to >> PR jit/107999 >> to the subject line and ChangeLog message. >> >> Dave >> >> > >> > Le mer. 14 déc. 2022 à 16:12, Antoni Boucher a >> > écrit : >> > >> > > Thanks! >> > > >> > > In your patch, you're missing this line at the end of the commit >> > > message: >> > > >> > >Signed-off-by: Guillaume Gomez >> > > >> > > On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit wrote: >> > > > Hi, >> > > > >> > > > This fixes bug 107999. >> > > > >> > > > Thanks in advance for the review. >> > > >> > > >> >> From 0835c7ba8bdf4090c7fb102206e70c1ed235808e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Dec 2022 14:28:22 +0100 Subject: [PATCH] [PATCH] Fix a failing test by updating its error string [PR107999] gcc/testsuite/ChangeLog: PR jit/107999 * jit.dg/test-error-array-bounds.c: Update test. Signed-off-by: Guillaume Gomez --- gcc/testsuite/jit.dg/test-error-array-bounds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/jit.dg/test-error-array-bounds.c b/gcc/testsuite/jit.dg/test-error-array-bounds.c index b6c0ee526d4..a0dead13cb7 100644 --- a/gcc/testsuite/jit.dg/test-error-array-bounds.c +++ b/gcc/testsuite/jit.dg/test-error-array-bounds.c @@ -70,5 +70,5 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) /* ...and that the message was captured by the API. */ CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), "array subscript 10 is above array bounds of" - " 'char[10]' [-Warray-bounds]"); + " 'char[10]' [-Warray-bounds=]"); } -- 2.34.1
Re: [PATCH] libgccjit: Fix a failing test
I don't have push rights so if you could push it, it'd be super appreciated! Le jeu. 2 mars 2023 à 23:33, David Malcolm a écrit : > On Thu, 2023-03-02 at 23:29 +0100, Guillaume Gomez wrote: > > Just realized I used whitespace and not a tab. Sorry about that. > > Here's the > > fixed version... > > Looks great. Do you have push rights, or do you want me to push this? > > Thanks > Dave > > > > > Le jeu. 2 mars 2023 à 23:19, Guillaume Gomez > > a > > écrit : > > > > > No problem, thanks for the explanations. > > > > > > I joined the patch with the fixed commit message. > > > > > > Le jeu. 2 mars 2023 à 22:58, David Malcolm a > > > écrit : > > > > > > > On Thu, 2022-12-15 at 08:34 +0100, Guillaume Gomez via Jit wrote: > > > > > Forgot it indeed, thanks for notifying me! > > > > > > > > > > I modified the commit message to add it and added it into this > > > > > email. > > > > > > > > Sorry about the delay in reviewing this; for some reason I didn't > > > > see > > > > the mail. > > > > > > > > The patch looks good for trunk, but please add a reference to > > > > PR jit/107999 > > > > to the subject line and ChangeLog message. > > > > > > > > Dave > > > > > > > > > > > > > > Le mer. 14 déc. 2022 à 16:12, Antoni Boucher > > > > > a > > > > > écrit : > > > > > > > > > > > Thanks! > > > > > > > > > > > > In your patch, you're missing this line at the end of the > > > > > > commit > > > > > > message: > > > > > > > > > > > >Signed-off-by: Guillaume Gomez > > > > > > > > > > > > > > > > > > On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit > > > > > > wrote: > > > > > > > Hi, > > > > > > > > > > > > > > This fixes bug 107999. > > > > > > > > > > > > > > Thanks in advance for the review. > > > > > > > > > > > > > > > > > > > > > >
Re: [PATCH] libgccjit: Fix a failing test
Thanks! Le jeu. 2 mars 2023 à 23:54, David Malcolm a écrit : > On Thu, 2023-03-02 at 23:35 +0100, Guillaume Gomez wrote: > > I don't have push rights so if you could push it, it'd be super > > appreciated! > > Done, as r13-6425-g6b432c0f777ab9; I took the liberty of slightly > tweaking the subject line to add a "jit, testsuite: " prefix. > > Thanks again for the patch > Dave > >
PATCH: c++tools: fix compilation
Hi, This patch fixes the following compilation error: ../.././c++tools/server.cc: In function ‘void server(bool, int, module_resolver*)’: ../.././c++tools/server.cc:756:69: error: ‘readers’ was not declared in this scope; did you mean ‘read’? 756 | if (active < 0 && sock_fd >= 0 && FD_ISSET (sock_fd, &readers)) | ^~~ It was missing a preprocessor condition around this code to work as the "readers" variable is created only in a preprocessor condition. Signed-off-by: Guillaume Gomez From 39279d8b37287c09708d910921ce5cfa5b87ac01 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Thu, 20 Oct 2022 18:18:52 +0200 Subject: [PATCH] Add missing preprocessor condition to fix c++tools/server.cc file compilation --- c++tools/server.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/c++tools/server.cc b/c++tools/server.cc index 00154a05925..693aec6820a 100644 --- a/c++tools/server.cc +++ b/c++tools/server.cc @@ -753,8 +753,10 @@ server (bool ipv6, int sock_fd, module_resolver *resolver) } } +#if defined (HAVE_PSELECT) || defined (HAVE_SELECT) if (active < 0 && sock_fd >= 0 && FD_ISSET (sock_fd, &readers)) active = -1; +#endif } if (active >= 0) -- 2.34.1
[PATCH] libgccjit: Fix a failing test
Hi, This fixes bug 107999. Thanks in advance for the review. From e6db0cb107e54789095f4585dd279a2c984d2ca1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Dec 2022 14:28:22 +0100 Subject: [PATCH] Fix a failing test by updating its error string. gcc/testsuite/ChangeLog: * jit.dg/test-error-array-bounds.c: Update test. --- gcc/testsuite/jit.dg/test-error-array-bounds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/jit.dg/test-error-array-bounds.c b/gcc/testsuite/jit.dg/test-error-array-bounds.c index b6c0ee526d4..a0dead13cb7 100644 --- a/gcc/testsuite/jit.dg/test-error-array-bounds.c +++ b/gcc/testsuite/jit.dg/test-error-array-bounds.c @@ -70,5 +70,5 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) /* ...and that the message was captured by the API. */ CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), "array subscript 10 is above array bounds of" - " 'char[10]' [-Warray-bounds]"); + " 'char[10]' [-Warray-bounds=]"); } -- 2.34.1
Re: [PATCH] libgccjit: Fix a failing test
Forgot it indeed, thanks for notifying me! I modified the commit message to add it and added it into this email. Le mer. 14 déc. 2022 à 16:12, Antoni Boucher a écrit : > Thanks! > > In your patch, you're missing this line at the end of the commit > message: > >Signed-off-by: Guillaume Gomez > > On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit wrote: > > Hi, > > > > This fixes bug 107999. > > > > Thanks in advance for the review. > > From 30f049d4f39de392dbb3cff4b64779f2507fc914 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Wed, 14 Dec 2022 14:28:22 +0100 Subject: [PATCH] Fix a failing test by updating its error string. gcc/testsuite/ChangeLog: * jit.dg/test-error-array-bounds.c: Update test. Signed-off-by: Guillaume Gomez --- gcc/testsuite/jit.dg/test-error-array-bounds.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/testsuite/jit.dg/test-error-array-bounds.c b/gcc/testsuite/jit.dg/test-error-array-bounds.c index b6c0ee526d4..a0dead13cb7 100644 --- a/gcc/testsuite/jit.dg/test-error-array-bounds.c +++ b/gcc/testsuite/jit.dg/test-error-array-bounds.c @@ -70,5 +70,5 @@ verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) /* ...and that the message was captured by the API. */ CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), "array subscript 10 is above array bounds of" - " 'char[10]' [-Warray-bounds]"); + " 'char[10]' [-Warray-bounds=]"); } -- 2.34.1
Re: [PATCH] libgccjit: Fix a failing test
Ping David Le jeu. 15 déc. 2022 à 11:34, Guillaume Gomez a écrit : > Forgot it indeed, thanks for notifying me! > > I modified the commit message to add it and added it into this email. > > Le mer. 14 déc. 2022 à 16:12, Antoni Boucher a écrit : > >> Thanks! >> >> In your patch, you're missing this line at the end of the commit >> message: >> >>Signed-off-by: Guillaume Gomez >> >> On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit wrote: >> > Hi, >> > >> > This fixes bug 107999. >> > >> > Thanks in advance for the review. >> >>
Re: [PATCH] libgccjit: Fix a failing test
Ping David. Le sam. 24 déc. 2022 à 21:01, Guillaume Gomez a écrit : > Ping David > > Le jeu. 15 déc. 2022 à 11:34, Guillaume Gomez > a écrit : > >> Forgot it indeed, thanks for notifying me! >> >> I modified the commit message to add it and added it into this email. >> >> Le mer. 14 déc. 2022 à 16:12, Antoni Boucher a écrit : >> >>> Thanks! >>> >>> In your patch, you're missing this line at the end of the commit >>> message: >>> >>>Signed-off-by: Guillaume Gomez >>> >>> On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit wrote: >>> > Hi, >>> > >>> > This fixes bug 107999. >>> > >>> > Thanks in advance for the review. >>> >>>
Re: [PATCH] libgccjit: Fix a failing test
Ping David. Le jeu. 5 janv. 2023 à 23:37, Guillaume Gomez a écrit : > Ping David. > > Le sam. 24 déc. 2022 à 21:01, Guillaume Gomez > a écrit : > >> Ping David >> >> Le jeu. 15 déc. 2022 à 11:34, Guillaume Gomez >> a écrit : >> >>> Forgot it indeed, thanks for notifying me! >>> >>> I modified the commit message to add it and added it into this email. >>> >>> Le mer. 14 déc. 2022 à 16:12, Antoni Boucher a >>> écrit : >>> >>>> Thanks! >>>> >>>> In your patch, you're missing this line at the end of the commit >>>> message: >>>> >>>>Signed-off-by: Guillaume Gomez >>>> >>>> On Wed, 2022-12-14 at 14:39 +0100, Guillaume Gomez via Jit wrote: >>>> > Hi, >>>> > >>>> > This fixes bug 107999. >>>> > >>>> > Thanks in advance for the review. >>>> >>>>
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
After more investigations, it seems like the fault was on our side as
we were calling `gcc_jit_type_get_restrict` on types that weren't
pointers.
I added a check for that in `gcc_jit_type_get_restrict` directly as well.
We will continue our investigation to be sure we didn't miss anything.
Le mar. 22 août 2023 à 17:26, Antoni Boucher a écrit :
>
> Since the tests in the PR for rustc_codegen_gcc
> (https://github.com/rust-lang/rustc_codegen_gcc/pull/312) currently
> fails, let's wait a bit before merging the patch, in case it would need
> some fixes.
>
> On Thu, 2023-08-17 at 20:09 +0200, Guillaume Gomez via Jit wrote:
> > Quick question: do you plan to make the merge or should I ask Antoni?
> >
> > Le jeu. 17 août 2023 à 17:59, Guillaume Gomez
> >
> > a écrit :
> >
> > > Thanks for the review!
> > >
> > > Le jeu. 17 août 2023 à 17:50, David Malcolm a
> > > écrit
> > > :
> > > >
> > > > On Thu, 2023-08-17 at 17:41 +0200, Guillaume Gomez wrote:
> > > > > And now I just discovered that a lot of commits from Antoni's
> > > > > fork
> > > > > haven't been sent upstream which is why the ABI count is so
> > > > > high in
> > > > > his repository. Fixed that as well.
> > > >
> > > > Thanks for the updated patch; I was about to comment on that.
> > > >
> > > > This version is good for gcc trunk.
> > > >
> > > > Dave
> > > >
> > > > >
> > > > > Le jeu. 17 août 2023 à 17:26, Guillaume Gomez
> > > > > a écrit :
> > > > > >
> > > > > > Antoni spot a typo I made:
> > > > > >
> > > > > > I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` instead of
> > > > > > `LIBGCCJIT_HAVE_gcc_jit_type_get_restrict`. Fixed in this
> > > > > > patch,
> > > > > > sorry
> > > > > > for the noise.
> > > > > >
> > > > > > Le jeu. 17 août 2023 à 11:30, Guillaume Gomez
> > > > > > a écrit :
> > > > > > >
> > > > > > > Hi Dave,
> > > > > > >
> > > > > > > > What kind of testing has the patch had? (e.g. did you run
> > > > > > > > "make
> > > > > > > > check-
> > > > > > > > jit" ? Has this been in use on real Rust code?)
> > > > > > >
> > > > > > > I tested it as Rust backend directly on this code:
> > > > > > >
> > > > > > > ```
> > > > > > > pub fn foo(a: &mut i32, b: &mut i32, c: &i32) {
> > > > > > > *a += *c;
> > > > > > > *b += *c;
> > > > > > > }
> > > > > > > ```
> > > > > > >
> > > > > > > I ran it with `rustc` (and the GCC backend) with the
> > > > > > > following
> > > > > > > flags:
> > > > > > > `-C link-args=-lc --emit=asm -O --crate-type=lib` which
> > > > > > > gave the
> > > > > > > diff
> > > > > > > you can see in the attached file. Explanations: the diff on
> > > > > > > the
> > > > > > > right
> > > > > > > has the `__restrict__` attribute used whereas on the left
> > > > > > > it is
> > > > > > > the
> > > > > > > current version where we don't handle it.
> > > > > > >
> > > > > > > As for C testing, I used this code:
> > > > > > >
> > > > > > > ```
> > > > > > > void t(int *__restrict__ a, int *__restrict__ b, char
> > > > > > > *__restrict__ c) {
> > > > > > > *a += *c;
> > > > > > > *b += *c;
> > > > > > > }
> > > > > > > ```
> > > > > > >
> > > > > > > (without the `__restrict__` of course when I need to have a
> > > > > > > witness
> > > > > > > ASM). I attached the diff as well, this time the file with
> > > > > > > the
> > > > > > > use of
> > > > > > > `__restrict__` in on the left. I compiled with the
> > > > > > > followi
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
We finished the investigation and found out the issue: when passing
arguments by value to functions, rustc still provides "NoAlias" as
attribute to the argument whereas it should never be passed in this
case. Luckily for us, in case the argument is a function pointer
coming from a struct field, it crashes GCC, which is what allowed us
to figure out about this. A code which reproduces this bug:
```
use std::mem;
#[repr(C)]
pub struct Buffer {
data: *mut u8,
len: usize,
drop: extern "C" fn(Buffer),
}
impl Default for Buffer {
#[inline]
fn default() -> Self {
Self::from(vec![])
}
}
impl Buffer {
#[inline]
pub fn new() -> Self {
Self::default()
}
#[inline]
pub fn push(&mut self, v: u8) {}
}
impl From> for Buffer {
fn from(mut v: Vec) -> Self {
let (data, len) = (v.as_mut_ptr(), v.len());
mem::forget(v);
extern "C" fn drop(b: Buffer) {}
Buffer { data, len, drop }
}
}
fn main() {
let mut buffer = Buffer::new();
}
```
So in short: the patch in the previous mail which added this check:
```
RETURN_NULL_IF_FAIL (type->is_pointer (), NULL, NULL, "not a pointer type");
```
is correct and ready. We fixed the bug on our side in
https://github.com/rust-lang/rustc_codegen_gcc/pull/312.
For more information about the "ByValue" attributes issue, there were
some discussions about it in LLVM:
*
https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20171218/512066.html
* https://reviews.llvm.org/D40118
Le ven. 25 août 2023 à 22:47, Guillaume Gomez
a écrit :
>
> After more investigations, it seems like the fault was on our side as
> we were calling `gcc_jit_type_get_restrict` on types that weren't
> pointers.
> I added a check for that in `gcc_jit_type_get_restrict` directly as well.
>
> We will continue our investigation to be sure we didn't miss anything.
>
> Le mar. 22 août 2023 à 17:26, Antoni Boucher a écrit :
> >
> > Since the tests in the PR for rustc_codegen_gcc
> > (https://github.com/rust-lang/rustc_codegen_gcc/pull/312) currently
> > fails, let's wait a bit before merging the patch, in case it would need
> > some fixes.
> >
> > On Thu, 2023-08-17 at 20:09 +0200, Guillaume Gomez via Jit wrote:
> > > Quick question: do you plan to make the merge or should I ask Antoni?
> > >
> > > Le jeu. 17 août 2023 à 17:59, Guillaume Gomez
> > >
> > > a écrit :
> > >
> > > > Thanks for the review!
> > > >
> > > > Le jeu. 17 août 2023 à 17:50, David Malcolm a
> > > > écrit
> > > > :
> > > > >
> > > > > On Thu, 2023-08-17 at 17:41 +0200, Guillaume Gomez wrote:
> > > > > > And now I just discovered that a lot of commits from Antoni's
> > > > > > fork
> > > > > > haven't been sent upstream which is why the ABI count is so
> > > > > > high in
> > > > > > his repository. Fixed that as well.
> > > > >
> > > > > Thanks for the updated patch; I was about to comment on that.
> > > > >
> > > > > This version is good for gcc trunk.
> > > > >
> > > > > Dave
> > > > >
> > > > > >
> > > > > > Le jeu. 17 août 2023 à 17:26, Guillaume Gomez
> > > > > > a écrit :
> > > > > > >
> > > > > > > Antoni spot a typo I made:
> > > > > > >
> > > > > > > I added `LIBGCCJIT_HAVE_gcc_jit_type_get_size` instead of
> > > > > > > `LIBGCCJIT_HAVE_gcc_jit_type_get_restrict`. Fixed in this
> > > > > > > patch,
> > > > > > > sorry
> > > > > > > for the noise.
> > > > > > >
> > > > > > > Le jeu. 17 août 2023 à 11:30, Guillaume Gomez
> > > > > > > a écrit :
> > > > > > > >
> > > > > > > > Hi Dave,
> > > > > > > >
> > > > > > > > > What kind of testing has the patch had? (e.g. did you run
> > > > > > > > > "make
> > > > > > > > > check-
> > > > > > > > > jit" ? Has this been in use on real Rust code?)
> > > > > > > >
> > > > > > > > I tested it as Rust backend directly on this code:
> > > > > > > >
> > > > > > > > ```
> > > > > > > > pub fn foo(a: &
Re: [PATCH] libgccjit: Add support for `restrict` attribute on function parameters
Thanks a lot! Le mar. 29 août 2023 à 17:35, David Malcolm a écrit : > > On Tue, 2023-08-29 at 17:15 +0200, Guillaume Gomez wrote: > > We finished the investigation and found out the issue: when passing > > arguments by value to functions, rustc still provides "NoAlias" as > > attribute to the argument whereas it should never be passed in this > > case. Luckily for us, in case the argument is a function pointer > > coming from a struct field, it crashes GCC, which is what allowed us > > to figure out about this. A code which reproduces this bug: > > [...snip...] > > > So in short: the patch in the previous mail which added this check: > > > > ``` > > RETURN_NULL_IF_FAIL (type->is_pointer (), NULL, NULL, "not a pointer > > type"); > > ``` > > > > is correct and ready. > > Thanks. I've gone ahead and pushed it to gcc trunk for you as r14- > 3552-g29763b002459cb. > > [...snip...] > > Dave >
Re: [PATH] [CLEANUP] Remove trailing whitespace characters
Hi David. Thanks for the feedback! Well, I think at this point, better to get "global approval" on the idea and on what you suggested (which I agree with) before I make any additional changes considering how big the patch is. Let's wait for others to also express their opinion here.
