Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-04-12 Thread David Malcolm via Gcc-patches
On Wed, 2022-02-09 at 20:33 -0500, Antoni Boucher wrote:
> Here's the updated patch.

Thanks.

I updated the patch somewhat:
  * fixed up some hunks that didn't quite apply
  * tweaked the comment in all-non-failing-tests.h
  * fixed continuation lines in .rst ". function::" clause
  * test-error-register* was failing: I forcibly disabled colorization
in diagnostic, by adding:
   gcc_jit_context_add_command_line_option
 (ctxt, "-fdiagnostics-color=never");
to harness.h, to avoid possible difference in output based on whether
stderr is connected to a tty
  * regenerated .texinfo from .rst

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.

I've pushed the patch to trunk for GCC 12 as r12-8118-g5780ff348ad443

https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=5780ff348ad4430383fd67c6f0c572d8c3e721ad

Dave

> 
> Le mardi 25 janvier 2022 à 12:13 -0500, Antoni Boucher via Jit a
> écrit :
> > See answers below.
> > 
> > Le lundi 24 janvier 2022 à 18:20 -0500, David Malcolm a écrit :
> > > On Sat, 2022-01-22 at 19:29 -0500, Antoni Boucher wrote:
> > > > Hi.
> > > > 
> > > > Le mardi 18 janvier 2022 à 18:49 -0500, David Malcolm a écrit :
> > > > > On Mon, 2022-01-17 at 19:46 -0500, Antoni Boucher via Gcc-
> > > > > patches
> > > > > wrote:
> > > > > > I missed the comment about the new define, so here's the
> > > > > > updated
> > > > > > patch.
> > > > > 
> > > > > Thanks for the patch.
> > > > > > 
> > > > > > Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via
> > > > > > Jit
> > > > > > a
> > > > > > écrit :
> > > > > > > Hi.
> > > > > > > This patch add supports for register variables in
> > > > > > > libgccjit.
> > > > > > > 
> > > > > > > It passes the JIT tests, but since I added a function in
> > > > > > > reginfo.c,
> > > > > > > I
> > > > > > > wonder if I should run the whole testsuite.
> > > > > 
> > > > > We're in stage 4 for gcc 12, so we should be especially careful
> > > > > about
> > > > > changes right now, and we're not meant to be adding new GCC 12
> > > > > features.
> > > > > 
> > > > > How close is gcc 12's libgccjit to being usable with your rustc
> > > > > backend?  If we're almost there, I'm willing to make our case
> > > > > for
> > > > > late-
> > > > > breaking libgccjit changes to the release managers, but if you
> > > > > think
> > > > > rustc users are going to need to build a patched libgccjit,
> > > > > then
> > > > > let's
> > > > > queue this up for gcc 13.
> > > > 
> > > > As I mentioned in my other email, if the 4 patches currently
> > > > being
> > > > reviewed (and listed here:
> > > > https://github.com/antoyo/libgccjit-patches) were included in gcc
> > > > 12,
> > > > I'd be able to build rustc_codegen_gcc with an unpatched gcc.
> > > 
> > > Thanks.  Once the relevant patches look good to me, I'll approach
> > > the
> > > release managers with the proposal.
> > > 
> > > > 
> > > > It is to be noted however, that I'll need more patches for future
> > > > work.
> > > > Off the top of my head, I'll at least need a patch for the inline
> > > > attribute, try/catch and target-specific builtins.
> > > > The last 2 features will probably take some time to implement, so
> > > > I'll
> > > > let you judge if you think it's worth merging the 4 patches
> > > > currently
> > > > being reviewed for gcc 12.
> > > 
> > > Thanks, though I don't know enough about your project's features to
> > > make the judgement call.  Does rustc_codegen_gcc have releases yet,
> > > or
> > > are you just pointing people at the trunk of your repo?  I guess
> > > the
> > > question is - are you hoping to be able to point people at distro
> > > installs of gcc 12's libgccjit and have some version of
> > > rustc_codegen_gcc "just work" with that, or are they going to have
> > > to
> > > rebuild their own libgccjit to meaningfully use it?
> > 
> > rustc_codegen_gcc does not have releases. It is merged from time to
> > time into rustc, so we can as well say that I point them to trunk.
> > 
> > I can feature gate the future features/patches I will need so that
> > people can still use the project with an unpatched gcc.
> > 
> > There's some interest in having it work with an unpatched gcc because
> > that would allow people to start working on the infra to get the
> > project distributed via rustup so that it could be distributed as a
> > preview component.
> > 
> > > 
> > > [...snip various corrections...]
> > > 
> > > > 
> > > > > > diff --git a/gcc/testsuite/jit.dg/test-register-variable.c
> > > > > > b/gcc/testsuite/jit.dg/test-register-variable.c
> > > > > > new file mode 100644
> > > > > > index 000..3cea3f1668f
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/jit.dg/test-register-variable.c
> > > > > > +
> > > 
> > > [...snip...]
> > > 
> > > > > > +/* { dg-final { jit-verify-output-file-was-created "" } } */
> > > > > > +/* { dg-final { jit-verify-assembler-output
> > > > > > "movl  \\\$1,
> > > > > > %r12d" } } */
> > > > > > +/* { dg-final { 

Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-02-09 Thread Antoni Boucher via Gcc-patches
" } } */
> > > > > +/* { dg-final { jit-verify-assembler-output
> > > > > "movl  \\\$2,
> > > > > %r13d" } } */
> > > > 
> > > > How target-specific is this test?
> > > 
> > > It will only work on x86-64. Should I feature-gate the test
> > > somehow?
> > 
> > 
> > Yes; I think you can do this by adding this to the top of the test:
> > 
> >    /* { dg-do compile { target x86_64-*-* } } */
> > 
> > like test-asm.c does.
> 
> Thanks. I'll update the patch to do this.
> 
> > 
> > > > 
> > > > We should have test coverage for at least these two errors:
> > > > 
> > > > - gcc_jit_lvalue_set_register_name(global_variable,
> > > > "this_is_not_a_register");
> > > > - attempting to set the name for a var that doesn't fit in the
> > > > given
> > > > register (e.g. trying to use a register for an array that's way
> > > > too
> > > > big)
> > > 
> > > Done.
> > 
> > Thanks.
> > 
> > Is the updated patch available for review? It looks like you didn't
> > attach it.
> > 
> > Dave
> > 
> 

From 542b7af61292a04d61480388d425a1057ecd66d4 Mon Sep 17 00:00:00 2001
From: Antoni Boucher 
Date: Sun, 29 Aug 2021 10:54:55 -0400
Subject: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-17  Antoni Boucher 

gcc/jit/
	PR jit/104072
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_22): New ABI tag.
	* docs/topics/expressions.rst: Add documentation for the
	function gcc_jit_lvalue_set_register_name.
	* dummy-frontend.cc: Clear the global_regs cache to avoid an
	issue where compiling multiple times the same code gives an
	error about assigning the same register to 2 global variables.
	* jit-playback.h: New function (set_register_name).
	* jit-recording.cc: New function (set_register_name) and add
	support for register variables.
	* jit-recording.h: New field (m_reg_name) and new function
	(set_register_name).
	* libgccjit.cc: New function (gcc_jit_lvalue_set_register_name).
	* libgccjit.h: New function (gcc_jit_lvalue_set_register_name).
	* libgccjit.map (LIBGCCJIT_ABI_22): New ABI tag.

gcc/
	PR jit/104072
	* reginfo.cc: New functions (clear_global_regs_cache, reginfo_cc_finalize).
	* rtl.h: New function (reginfo_cc_finalize).

gcc/testsuite/
	PR jit/104072
	* jit.dg/all-non-failing-tests.h: Add new
	test-register-variable.
	* jit.dg/test-error-register-variable-bad-name.c: New test.
	* jit.dg/test-error-register-variable-size-mismatch.c: New test.
	* jit.dg/test-register-variable.c: New test.
---
 gcc/jit/docs/topics/compatibility.rst |  9 +++
 gcc/jit/docs/topics/expressions.rst   | 20 +++
 gcc/jit/jit-playback.h|  9 +++
 gcc/jit/jit-recording.cc  | 18 +-
 gcc/jit/jit-recording.h   |  9 ++-
 gcc/jit/libgccjit.cc  | 14 +
 gcc/jit/libgccjit.h   | 12 
 gcc/jit/libgccjit.map | 11 
 gcc/reginfo.cc| 18 ++
 gcc/rtl.h |  1 +
 gcc/testsuite/jit.dg/all-non-failing-tests.h  |  3 +
 .../test-error-register-variable-bad-name.c   | 35 
 ...st-error-register-variable-size-mismatch.c | 40 +
 gcc/testsuite/jit.dg/test-register-variable.c | 56 +++
 gcc/toplev.cc |  1 +
 15 files changed, 250 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-error-register-variable-bad-name.c
 create mode 100644 gcc/testsuite/jit.dg/test-error-register-variable-size-mismatch.c
 create mode 100644 gcc/testsuite/jit.dg/test-register-variable.c

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 16cebe31a10..689c94c4fac 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -302,3 +302,12 @@ thread-local storage model of a variable:
 section of a variable:
 
   * :func:`gcc_jit_lvalue_set_link_section`
+
+.. _LIBGCCJIT_ABI_22:
+
+``LIBGCCJIT_ABI_22``
+---
+``LIBGCCJIT_ABI_22`` covers the addition of an API entrypoint to set the
+register name of a variable:
+
+  * :func:`gcc_jit_lvalue_set_register_name`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 791a20398ca..5cf780e6349 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -738,6 +738,26 @@ where the rvalue is computed by reading from the storage area.
 
   #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
 
+.. function:: void
+  gcc_jit_lvalue_set_register_name (gcc_jit_lvalue

Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-25 Thread Antoni Boucher via Gcc-patches
See answers below.

Le lundi 24 janvier 2022 à 18:20 -0500, David Malcolm a écrit :
> On Sat, 2022-01-22 at 19:29 -0500, Antoni Boucher wrote:
> > Hi.
> > 
> > Le mardi 18 janvier 2022 à 18:49 -0500, David Malcolm a écrit :
> > > On Mon, 2022-01-17 at 19:46 -0500, Antoni Boucher via Gcc-patches
> > > wrote:
> > > > I missed the comment about the new define, so here's the
> > > > updated
> > > > patch.
> > > 
> > > Thanks for the patch.
> > > > 
> > > > Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit
> > > > a
> > > > écrit :
> > > > > Hi.
> > > > > This patch add supports for register variables in libgccjit.
> > > > > 
> > > > > It passes the JIT tests, but since I added a function in
> > > > > reginfo.c,
> > > > > I
> > > > > wonder if I should run the whole testsuite.
> > > 
> > > We're in stage 4 for gcc 12, so we should be especially careful
> > > about
> > > changes right now, and we're not meant to be adding new GCC 12
> > > features.
> > > 
> > > How close is gcc 12's libgccjit to being usable with your rustc
> > > backend?  If we're almost there, I'm willing to make our case for
> > > late-
> > > breaking libgccjit changes to the release managers, but if you
> > > think
> > > rustc users are going to need to build a patched libgccjit, then
> > > let's
> > > queue this up for gcc 13.
> > 
> > As I mentioned in my other email, if the 4 patches currently being
> > reviewed (and listed here:
> > https://github.com/antoyo/libgccjit-patches) were included in gcc
> > 12,
> > I'd be able to build rustc_codegen_gcc with an unpatched gcc.
> 
> Thanks.  Once the relevant patches look good to me, I'll approach the
> release managers with the proposal.
> 
> > 
> > It is to be noted however, that I'll need more patches for future
> > work.
> > Off the top of my head, I'll at least need a patch for the inline
> > attribute, try/catch and target-specific builtins.
> > The last 2 features will probably take some time to implement, so
> > I'll
> > let you judge if you think it's worth merging the 4 patches
> > currently
> > being reviewed for gcc 12.
> 
> Thanks, though I don't know enough about your project's features to
> make the judgement call.  Does rustc_codegen_gcc have releases yet,
> or
> are you just pointing people at the trunk of your repo?  I guess the
> question is - are you hoping to be able to point people at distro
> installs of gcc 12's libgccjit and have some version of
> rustc_codegen_gcc "just work" with that, or are they going to have to
> rebuild their own libgccjit to meaningfully use it?

rustc_codegen_gcc does not have releases. It is merged from time to
time into rustc, so we can as well say that I point them to trunk.

I can feature gate the future features/patches I will need so that
people can still use the project with an unpatched gcc.

There's some interest in having it work with an unpatched gcc because
that would allow people to start working on the infra to get the
project distributed via rustup so that it could be distributed as a
preview component.

> 
> [...snip various corrections...]
> 
> > 
> > > > diff --git a/gcc/testsuite/jit.dg/test-register-variable.c
> > > > b/gcc/testsuite/jit.dg/test-register-variable.c
> > > > new file mode 100644
> > > > index 000..3cea3f1668f
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/jit.dg/test-register-variable.c
> > > > +
> 
> [...snip...]
> 
> > > > +/* { dg-final { jit-verify-output-file-was-created "" } } */
> > > > +/* { dg-final { jit-verify-assembler-output "movl  \\\$1,
> > > > %r12d" } } */
> > > > +/* { dg-final { jit-verify-assembler-output "movl  \\\$2,
> > > > %r13d" } } */
> > > 
> > > How target-specific is this test?
> > 
> > It will only work on x86-64. Should I feature-gate the test
> > somehow?
> 
> 
> Yes; I think you can do this by adding this to the top of the test:
> 
>    /* { dg-do compile { target x86_64-*-* } } */
> 
> like test-asm.c does.

Thanks. I'll update the patch to do this.

> 
> > > 
> > > We should have test coverage for at least these two errors:
> > > 
> > > - gcc_jit_lvalue_set_register_name(global_variable,
> > > "this_is_not_a_register");
> > > - attempting to set the name for a var that doesn't fit in the
> > > given
> > > register (e.g. trying to use a register for an array that's way
> > > too
> > > big)
> > 
> > Done.
> 
> Thanks.
> 
> Is the updated patch available for review? It looks like you didn't
> attach it.
> 
> Dave
> 



Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-24 Thread Antoni Boucher via Gcc-patches
Indeed, I forgot to attach the patch.

Le lundi 24 janvier 2022 à 18:20 -0500, David Malcolm a écrit :
> On Sat, 2022-01-22 at 19:29 -0500, Antoni Boucher wrote:
> > Hi.
> > 
> > Le mardi 18 janvier 2022 à 18:49 -0500, David Malcolm a écrit :
> > > On Mon, 2022-01-17 at 19:46 -0500, Antoni Boucher via Gcc-patches
> > > wrote:
> > > > I missed the comment about the new define, so here's the
> > > > updated
> > > > patch.
> > > 
> > > Thanks for the patch.
> > > > 
> > > > Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit
> > > > a
> > > > écrit :
> > > > > Hi.
> > > > > This patch add supports for register variables in libgccjit.
> > > > > 
> > > > > It passes the JIT tests, but since I added a function in
> > > > > reginfo.c,
> > > > > I
> > > > > wonder if I should run the whole testsuite.
> > > 
> > > We're in stage 4 for gcc 12, so we should be especially careful
> > > about
> > > changes right now, and we're not meant to be adding new GCC 12
> > > features.
> > > 
> > > How close is gcc 12's libgccjit to being usable with your rustc
> > > backend?  If we're almost there, I'm willing to make our case for
> > > late-
> > > breaking libgccjit changes to the release managers, but if you
> > > think
> > > rustc users are going to need to build a patched libgccjit, then
> > > let's
> > > queue this up for gcc 13.
> > 
> > As I mentioned in my other email, if the 4 patches currently being
> > reviewed (and listed here:
> > https://github.com/antoyo/libgccjit-patches) were included in gcc
> > 12,
> > I'd be able to build rustc_codegen_gcc with an unpatched gcc.
> 
> Thanks.  Once the relevant patches look good to me, I'll approach the
> release managers with the proposal.
> 
> > 
> > It is to be noted however, that I'll need more patches for future
> > work.
> > Off the top of my head, I'll at least need a patch for the inline
> > attribute, try/catch and target-specific builtins.
> > The last 2 features will probably take some time to implement, so
> > I'll
> > let you judge if you think it's worth merging the 4 patches
> > currently
> > being reviewed for gcc 12.
> 
> Thanks, though I don't know enough about your project's features to
> make the judgement call.  Does rustc_codegen_gcc have releases yet,
> or
> are you just pointing people at the trunk of your repo?  I guess the
> question is - are you hoping to be able to point people at distro
> installs of gcc 12's libgccjit and have some version of
> rustc_codegen_gcc "just work" with that, or are they going to have to
> rebuild their own libgccjit to meaningfully use it?
> 
> [...snip various corrections...]
> 
> > 
> > > > diff --git a/gcc/testsuite/jit.dg/test-register-variable.c
> > > > b/gcc/testsuite/jit.dg/test-register-variable.c
> > > > new file mode 100644
> > > > index 000..3cea3f1668f
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/jit.dg/test-register-variable.c
> > > > +
> 
> [...snip...]
> 
> > > > +/* { dg-final { jit-verify-output-file-was-created "" } } */
> > > > +/* { dg-final { jit-verify-assembler-output "movl  \\\$1,
> > > > %r12d" } } */
> > > > +/* { dg-final { jit-verify-assembler-output "movl  \\\$2,
> > > > %r13d" } } */
> > > 
> > > How target-specific is this test?
> > 
> > It will only work on x86-64. Should I feature-gate the test
> > somehow?
> 
> 
> Yes; I think you can do this by adding this to the top of the test:
> 
>    /* { dg-do compile { target x86_64-*-* } } */
> 
> like test-asm.c does.
> 
> > > 
> > > We should have test coverage for at least these two errors:
> > > 
> > > - gcc_jit_lvalue_set_register_name(global_variable,
> > > "this_is_not_a_register");
> > > - attempting to set the name for a var that doesn't fit in the
> > > given
> > > register (e.g. trying to use a register for an array that's way
> > > too
> > > big)
> > 
> > Done.
> 
> Thanks.
> 
> Is the updated patch available for review? It looks like you didn't
> attach it.
> 
> Dave
> 

From cd76593905a43ceb53cb2325f2b742ba331da2f8 Mon Sep 17 00:00:00 2001
From: Antoni Boucher 
Date: Sun, 29 Aug 2021 10:54:55 -0400
Sub

Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-24 Thread David Malcolm via Gcc-patches
On Sat, 2022-01-22 at 19:29 -0500, Antoni Boucher wrote:
> Hi.
> 
> Le mardi 18 janvier 2022 à 18:49 -0500, David Malcolm a écrit :
> > On Mon, 2022-01-17 at 19:46 -0500, Antoni Boucher via Gcc-patches
> > wrote:
> > > I missed the comment about the new define, so here's the updated
> > > patch.
> > 
> > Thanks for the patch.
> > > 
> > > Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit a
> > > écrit :
> > > > Hi.
> > > > This patch add supports for register variables in libgccjit.
> > > > 
> > > > It passes the JIT tests, but since I added a function in
> > > > reginfo.c,
> > > > I
> > > > wonder if I should run the whole testsuite.
> > 
> > We're in stage 4 for gcc 12, so we should be especially careful
> > about
> > changes right now, and we're not meant to be adding new GCC 12
> > features.
> > 
> > How close is gcc 12's libgccjit to being usable with your rustc
> > backend?  If we're almost there, I'm willing to make our case for
> > late-
> > breaking libgccjit changes to the release managers, but if you
> > think
> > rustc users are going to need to build a patched libgccjit, then
> > let's
> > queue this up for gcc 13.
> 
> As I mentioned in my other email, if the 4 patches currently being
> reviewed (and listed here:
> https://github.com/antoyo/libgccjit-patches) were included in gcc 12,
> I'd be able to build rustc_codegen_gcc with an unpatched gcc.

Thanks.  Once the relevant patches look good to me, I'll approach the
release managers with the proposal.

> 
> It is to be noted however, that I'll need more patches for future
> work.
> Off the top of my head, I'll at least need a patch for the inline
> attribute, try/catch and target-specific builtins.
> The last 2 features will probably take some time to implement, so
> I'll
> let you judge if you think it's worth merging the 4 patches currently
> being reviewed for gcc 12.

Thanks, though I don't know enough about your project's features to
make the judgement call.  Does rustc_codegen_gcc have releases yet, or
are you just pointing people at the trunk of your repo?  I guess the
question is - are you hoping to be able to point people at distro
installs of gcc 12's libgccjit and have some version of
rustc_codegen_gcc "just work" with that, or are they going to have to
rebuild their own libgccjit to meaningfully use it?

[...snip various corrections...]

> 
> > > diff --git a/gcc/testsuite/jit.dg/test-register-variable.c
> > > b/gcc/testsuite/jit.dg/test-register-variable.c
> > > new file mode 100644
> > > index 000..3cea3f1668f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/jit.dg/test-register-variable.c
> > > +

[...snip...]

> > > +/* { dg-final { jit-verify-output-file-was-created "" } } */
> > > +/* { dg-final { jit-verify-assembler-output "movl  \\\$1,
> > > %r12d" } } */
> > > +/* { dg-final { jit-verify-assembler-output "movl  \\\$2,
> > > %r13d" } } */
> > 
> > How target-specific is this test?
> 
> It will only work on x86-64. Should I feature-gate the test somehow?


Yes; I think you can do this by adding this to the top of the test:

   /* { dg-do compile { target x86_64-*-* } } */

like test-asm.c does.

> > 
> > We should have test coverage for at least these two errors:
> > 
> > - gcc_jit_lvalue_set_register_name(global_variable,
> > "this_is_not_a_register");
> > - attempting to set the name for a var that doesn't fit in the
> > given
> > register (e.g. trying to use a register for an array that's way too
> > big)
> 
> Done.

Thanks.

Is the updated patch available for review? It looks like you didn't
attach it.

Dave



Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-22 Thread Antoni Boucher via Gcc-patches
Hi.

Le mardi 18 janvier 2022 à 18:49 -0500, David Malcolm a écrit :
> On Mon, 2022-01-17 at 19:46 -0500, Antoni Boucher via Gcc-patches
> wrote:
> > I missed the comment about the new define, so here's the updated
> > patch.
> 
> Thanks for the patch.
> > 
> > Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit a
> > écrit :
> > > Hi.
> > > This patch add supports for register variables in libgccjit.
> > > 
> > > It passes the JIT tests, but since I added a function in
> > > reginfo.c,
> > > I
> > > wonder if I should run the whole testsuite.
> 
> We're in stage 4 for gcc 12, so we should be especially careful about
> changes right now, and we're not meant to be adding new GCC 12
> features.
> 
> How close is gcc 12's libgccjit to being usable with your rustc
> backend?  If we're almost there, I'm willing to make our case for
> late-
> breaking libgccjit changes to the release managers, but if you think
> rustc users are going to need to build a patched libgccjit, then
> let's
> queue this up for gcc 13.

As I mentioned in my other email, if the 4 patches currently being
reviewed (and listed here:
https://github.com/antoyo/libgccjit-patches) were included in gcc 12,
I'd be able to build rustc_codegen_gcc with an unpatched gcc.

It is to be noted however, that I'll need more patches for future work.
Off the top of my head, I'll at least need a patch for the inline
attribute, try/catch and target-specific builtins.
The last 2 features will probably take some time to implement, so I'll
let you judge if you think it's worth merging the 4 patches currently
being reviewed for gcc 12.

> 
> > 2022-01-17  Antoni Boucher 
> > 
> > gcc/jit/
> > PR target/104072
> 
> This should be "jit", rather than "target".
> 
> This will need updaing for the various .c to .cc renamings on trunk
> yesterday.

Done.

> 
> [...snip...]
> 
> > diff --git a/gcc/jit/dummy-frontend.c b/gcc/jit/dummy-frontend.c
> > index 84ff359bfe3..04d8fc6ab48 100644
> > --- a/gcc/jit/dummy-frontend.c
> > +++ b/gcc/jit/dummy-frontend.c
> > @@ -599,6 +599,8 @@ jit_langhook_init (void)
> >  
> >    build_common_builtin_nodes ();
> >  
> > +  clear_global_regs_cache ();
> > +
> 
> Similarly to my comments on the bitcasts patch, call this from a
> reginfo_cc_finalize function called from toplev::finalize instead.

Done.

> 
> > diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> > index 03704ef10b8..1757ad583fe 100644
> > --- a/gcc/jit/libgccjit.c
> > +++ b/gcc/jit/libgccjit.c
> > @@ -2649,6 +2649,18 @@ gcc_jit_lvalue_set_link_section
> > (gcc_jit_lvalue *lvalue,
> >    lvalue->set_link_section (section_name);
> >  }
> >  
> > +/* Public entrypoint.  See description in libgccjit.h.
> > +
> > +   After error-checking, the real work is done by the
> > +   gcc::jit::recording::lvalue::set_register_name method in jit-
> > recording.c.  */
> > +
> > +void
> > +gcc_jit_lvalue_set_register_name (gcc_jit_lvalue *lvalue,
> > + const char *reg_name)
> > +{
> 
> Need error checking here, to gracefully reject NULL value, and NULL
> reg_name.

Done.

> 
> > +  lvalue->set_register_name (reg_name);
> > +}
> > +
> 
> > diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
> > index c93d7055d43..af4427c4503 100644
> > --- a/gcc/jit/jit-playback.h
> > +++ b/gcc/jit/jit-playback.h
> > @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not
> > see
> >  #include  // for std::pair
> >  
> >  #include "timevar.h"
> > +#include "varasm.h"
> >  
> >  #include "jit-recording.h"
> >  
> > @@ -701,6 +702,14 @@ public:
> >  set_decl_section_name (as_tree (), name);
> >    }
> >  
> > +  void
> > +  set_register_name (const char* reg_name)
> > +  {
> > +    set_user_assembler_name (as_tree (), reg_name);
> > +    DECL_REGISTER (as_tree ()) = 1;
> > +    DECL_HARD_REGISTER (as_tree ()) = 1;
> > +  }
> 
> I'm not familiar enough with the backend to know if this is correct,
> sorry.
> 
> Is there an analogous thing in the C frontend that this corresponds
> to?

Not really as this is doing multiple things in one shot, while in the C
frontend, the register keyword will do one thing, specifying the
register name is another, …

> 
> [...snip...]
> 
> > diff --git a/gcc/reginfo.c b/gcc/reginfo.c
> > index 234f72eceeb..4fe375c4463 100644
> > --- a/gcc/reginfo.c
> > +++ b/gcc/reginfo.c
> > @@ -91,6 +91,14 @@ static const char initial_call_used_regs[] =
> > CALL_USED_REGISTERS;
> >     and are also considered fixed.  */
> >  char global_regs[FIRST_PSEUDO_REGISTER];
> >  
> > +void clear_global_regs_cache (void)
> > +{
> 
> This should be made static and called from a reginfo_cc_finalize,
> called from toplev::finalize.
> 
> > +  for (size_t i = 0 ; i < FIRST_PSEUDO_REGISTER ; i++)
> > +  {
> > +    global_regs[i] = 0;
> 
> Probably should also clear global_regs_decl[i].
> 
> > +  }
> 
> and unset all of global_reg_set, I believe.  I'm not particularly
> familiar with this code, so a backend expert should look at this.

Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-18 Thread David Malcolm via Gcc-patches
On Mon, 2022-01-17 at 19:46 -0500, Antoni Boucher via Gcc-patches
wrote:
> I missed the comment about the new define, so here's the updated
> patch.

Thanks for the patch.
> 
> Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit a
> écrit :
> > Hi.
> > This patch add supports for register variables in libgccjit.
> > 
> > It passes the JIT tests, but since I added a function in reginfo.c,
> > I
> > wonder if I should run the whole testsuite.

We're in stage 4 for gcc 12, so we should be especially careful about
changes right now, and we're not meant to be adding new GCC 12
features.

How close is gcc 12's libgccjit to being usable with your rustc
backend?  If we're almost there, I'm willing to make our case for late-
breaking libgccjit changes to the release managers, but if you think
rustc users are going to need to build a patched libgccjit, then let's
queue this up for gcc 13.

> 2022-01-17  Antoni Boucher 
> 
> gcc/jit/
>   PR target/104072

This should be "jit", rather than "target".

This will need updaing for the various .c to .cc renamings on trunk
yesterday.

[...snip...]

> diff --git a/gcc/jit/dummy-frontend.c b/gcc/jit/dummy-frontend.c
> index 84ff359bfe3..04d8fc6ab48 100644
> --- a/gcc/jit/dummy-frontend.c
> +++ b/gcc/jit/dummy-frontend.c
> @@ -599,6 +599,8 @@ jit_langhook_init (void)
>  
>build_common_builtin_nodes ();
>  
> +  clear_global_regs_cache ();
> +

Similarly to my comments on the bitcasts patch, call this from a
reginfo_cc_finalize function called from toplev::finalize instead.

> diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> index 03704ef10b8..1757ad583fe 100644
> --- a/gcc/jit/libgccjit.c
> +++ b/gcc/jit/libgccjit.c
> @@ -2649,6 +2649,18 @@ gcc_jit_lvalue_set_link_section (gcc_jit_lvalue 
> *lvalue,
>lvalue->set_link_section (section_name);
>  }
>  
> +/* Public entrypoint.  See description in libgccjit.h.
> +
> +   After error-checking, the real work is done by the
> +   gcc::jit::recording::lvalue::set_register_name method in jit-recording.c. 
>  */
> +
> +void
> +gcc_jit_lvalue_set_register_name (gcc_jit_lvalue *lvalue,
> +   const char *reg_name)
> +{

Need error checking here, to gracefully reject NULL value, and NULL
reg_name.

> +  lvalue->set_register_name (reg_name);
> +}
> +

> diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
> index c93d7055d43..af4427c4503 100644
> --- a/gcc/jit/jit-playback.h
> +++ b/gcc/jit/jit-playback.h
> @@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include  // for std::pair
>  
>  #include "timevar.h"
> +#include "varasm.h"
>  
>  #include "jit-recording.h"
>  
> @@ -701,6 +702,14 @@ public:
>  set_decl_section_name (as_tree (), name);
>}
>  
> +  void
> +  set_register_name (const char* reg_name)
> +  {
> +set_user_assembler_name (as_tree (), reg_name);
> +DECL_REGISTER (as_tree ()) = 1;
> +DECL_HARD_REGISTER (as_tree ()) = 1;
> +  }

I'm not familiar enough with the backend to know if this is correct,
sorry.

Is there an analogous thing in the C frontend that this corresponds to?

[...snip...]

> diff --git a/gcc/reginfo.c b/gcc/reginfo.c
> index 234f72eceeb..4fe375c4463 100644
> --- a/gcc/reginfo.c
> +++ b/gcc/reginfo.c
> @@ -91,6 +91,14 @@ static const char initial_call_used_regs[] = 
> CALL_USED_REGISTERS;
> and are also considered fixed.  */
>  char global_regs[FIRST_PSEUDO_REGISTER];
>  
> +void clear_global_regs_cache (void)
> +{

This should be made static and called from a reginfo_cc_finalize,
called from toplev::finalize.

> +  for (size_t i = 0 ; i < FIRST_PSEUDO_REGISTER ; i++)
> +  {
> +global_regs[i] = 0;

Probably should also clear global_regs_decl[i].

> +  }

and unset all of global_reg_set, I believe.  I'm not particularly
familiar with this code, so a backend expert should look at this.

> +}
> +


> diff --git a/gcc/system.h b/gcc/system.h
> index c25cd64366f..950969367b3 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -1316,4 +1316,6 @@ endswith (const char *str, const char *suffix)
>return memcmp (str + str_len - suffix_len, suffix, suffix_len) == 0;
>  }
>  
> +extern void clear_global_regs_cache (void);

Declare reginfo_cc_finalize as "extern", and do it from rtl.h, rather
than system.h


>  #endif /* ! GCC_SYSTEM_H */

[...snip...]

> diff --git a/gcc/testsuite/jit.dg/test-register-variable.c 
> b/gcc/testsuite/jit.dg/test-register-variable.c
> new file mode 100644
> index 000..3cea3f1668f
> --- /dev/null
> +++ b/gcc/testsuite/jit.dg/test-register-variable.c
> @@ -0,0 +1,54 @@
> +#include 
> +#include 
> +
> +#include "libgccjit.h"
> +
> +/* We don't want set_options() in harness.h to set -O3 so our little local
> +   is optimized away. */
> +#define TEST_ESCHEWS_SET_OPTIONS
> +static void set_options (gcc_jit_context *ctxt, const char *argv0)
> +{
> +}
> +
> +#define TEST_COMPILING_TO_FILE
> +#define OUTPUT_KIND  GCC_JIT_OUTPUT_KIND_ASSEMBLER
> +#define OUTPUT_FILENAME  

Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-18 Thread Antoni Boucher via Gcc-patches
Also, I put the extern in gcc/system.h, but I'm not sure it's the right
place.
Where should I put it?

Le lundi 17 janvier 2022 à 19:46 -0500, Antoni Boucher via Jit a
écrit :
> I missed the comment about the new define, so here's the updated
> patch.
> 
> Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit a
> écrit :
> > Hi.
> > This patch add supports for register variables in libgccjit.
> > 
> > It passes the JIT tests, but since I added a function in reginfo.c,
> > I
> > wonder if I should run the whole testsuite.
> > 
> > Thanks for the review.
> 



Re: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-17 Thread Antoni Boucher via Gcc-patches
I missed the comment about the new define, so here's the updated patch.

Le lundi 17 janvier 2022 à 19:24 -0500, Antoni Boucher via Jit a
écrit :
> Hi.
> This patch add supports for register variables in libgccjit.
> 
> It passes the JIT tests, but since I added a function in reginfo.c, I
> wonder if I should run the whole testsuite.
> 
> Thanks for the review.

From 2d372a741e9ef29eb377f9edfc6c539d1b2722e5 Mon Sep 17 00:00:00 2001
From: Antoni Boucher 
Date: Sun, 29 Aug 2021 10:54:55 -0400
Subject: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-17  Antoni Boucher 

gcc/jit/
	PR target/104072
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_21): New ABI tag.
	* docs/topics/expressions.rst: Add documentation for the
	function gcc_jit_lvalue_set_register_name.
	* dummy-frontend.c: Clear the global_regs cache to avoid an
	issue where compiling multiple times the same code gives an
	error about assigning the same register to 2 global variables.
	* jit-playback.h: New function (set_register_name).
	* jit-recording.c: New function (set_register_name) and add
	support for register variables.
	* jit-recording.h: New field (m_reg_name) and new function
	(set_register_name).
	* libgccjit.c: New function (gcc_jit_lvalue_set_register_name).
	* libgccjit.h: New function (gcc_jit_lvalue_set_register_name).
	* libgccjit.map (LIBGCCJIT_ABI_21): New ABI tag.

gcc/
	* reginfo.c: New function (clear_global_regs_cache).
	* system.h: New function (clear_global_regs_cache).

gcc/testsuite/
	* jit.dg/all-non-failing-tests.h: Add new
	test-register-variable.
	* jit.dg/test-register-variable.c: New test.
---
 gcc/jit/docs/topics/compatibility.rst |  9 
 gcc/jit/docs/topics/expressions.rst   | 20 +++
 gcc/jit/dummy-frontend.c  |  2 +
 gcc/jit/jit-playback.h|  9 
 gcc/jit/jit-recording.c   | 18 +--
 gcc/jit/jit-recording.h   |  9 ++--
 gcc/jit/libgccjit.c   | 12 +
 gcc/jit/libgccjit.h   | 12 +
 gcc/jit/libgccjit.map |  9 
 gcc/reginfo.c |  8 +++
 gcc/system.h  |  2 +
 gcc/testsuite/jit.dg/all-non-failing-tests.h  |  3 ++
 gcc/testsuite/jit.dg/test-register-variable.c | 54 +++
 13 files changed, 161 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-register-variable.c

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 16cebe31a10..cd0ae4838a2 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -302,3 +302,12 @@ thread-local storage model of a variable:
 section of a variable:
 
   * :func:`gcc_jit_lvalue_set_link_section`
+
+.. _LIBGCCJIT_ABI_21:
+
+``LIBGCCJIT_ABI_21``
+---
+``LIBGCCJIT_ABI_21`` covers the addition of an API entrypoint to set the
+register name of a variable:
+
+  * :func:`gcc_jit_lvalue_set_register_name`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 791a20398ca..afe333a520f 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -738,6 +738,26 @@ where the rvalue is computed by reading from the storage area.
 
   #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
 
+.. function:: void
+  gcc_jit_lvalue_set_register_name (gcc_jit_lvalue *lvalue,
+const char *reg_name);
+
+   Set the register name of a variable.
+   The parameter ``reg_name`` must be non-NULL. Analogous to:
+
+   .. code-block:: c
+
+ register int variable asm ("r12");
+
+   in C.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_21`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+  #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_register_name
+
 Global variables
 
 
diff --git a/gcc/jit/dummy-frontend.c b/gcc/jit/dummy-frontend.c
index 84ff359bfe3..04d8fc6ab48 100644
--- a/gcc/jit/dummy-frontend.c
+++ b/gcc/jit/dummy-frontend.c
@@ -599,6 +599,8 @@ jit_langhook_init (void)
 
   build_common_builtin_nodes ();
 
+  clear_global_regs_cache ();
+
   /* The default precision for floating point numbers.  This is used
  for floating point constants with abstract type.  This may
  eventually be controllable by a command line option.  */
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index c93d7055d43..af4427c4503 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 #include  // for std::pair
 
 #include "timevar.h"
+#include "varasm.h"
 
 #include "jit-recording.h"
 
@@ -701,6 +702,14 @@ public:
 set_decl_section_name (as_tree (), nam

[PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-17 Thread Antoni Boucher via Gcc-patches
Hi.
This patch add supports for register variables in libgccjit.

It passes the JIT tests, but since I added a function in reginfo.c, I
wonder if I should run the whole testsuite.

Thanks for the review.
From 328eca2be438c4a05c21942b4b2c3650ff7de0eb Mon Sep 17 00:00:00 2001
From: Antoni Boucher 
Date: Sun, 29 Aug 2021 10:54:55 -0400
Subject: [PATCH] libgccjit: Add support for register variables [PR104072]

2022-01-17  Antoni Boucher 

gcc/jit/
	PR target/104072
	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_21): New ABI tag.
	* docs/topics/expressions.rst: Add documentation for the
	function gcc_jit_lvalue_set_register_name.
	* dummy-frontend.c: Clear the global_regs cache to avoid an
	issue where compiling multiple times the same code gives an
	error about assigning the same register to 2 global variables.
	* jit-playback.h: New function (set_register_name).
	* jit-recording.c: New function (set_register_name) and add
	support for register variables.
	* jit-recording.h: New field (m_reg_name) and new function
	(set_register_name).
	* libgccjit.c: New function (gcc_jit_lvalue_set_register_name).
	* libgccjit.h: New function (gcc_jit_lvalue_set_register_name).
	* libgccjit.map (LIBGCCJIT_ABI_21): New ABI tag.

gcc/
	* reginfo.c: New function (clear_global_regs_cache).
	* system.h: New function (clear_global_regs_cache).

gcc/testsuite/
	* jit.dg/all-non-failing-tests.h: Add new
	test-register-variable.
	* jit.dg/test-register-variable.c: New test.
---
 gcc/jit/docs/topics/compatibility.rst |  9 
 gcc/jit/docs/topics/expressions.rst   | 20 +++
 gcc/jit/dummy-frontend.c  |  2 +
 gcc/jit/jit-playback.h|  9 
 gcc/jit/jit-recording.c   | 18 +--
 gcc/jit/jit-recording.h   |  9 ++--
 gcc/jit/libgccjit.c   | 12 +
 gcc/jit/libgccjit.h   |  7 +++
 gcc/jit/libgccjit.map |  9 
 gcc/reginfo.c |  8 +++
 gcc/system.h  |  2 +
 gcc/testsuite/jit.dg/all-non-failing-tests.h  |  3 ++
 gcc/testsuite/jit.dg/test-register-variable.c | 54 +++
 13 files changed, 156 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-register-variable.c

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 16cebe31a10..cd0ae4838a2 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -302,3 +302,12 @@ thread-local storage model of a variable:
 section of a variable:
 
   * :func:`gcc_jit_lvalue_set_link_section`
+
+.. _LIBGCCJIT_ABI_21:
+
+``LIBGCCJIT_ABI_21``
+---
+``LIBGCCJIT_ABI_21`` covers the addition of an API entrypoint to set the
+register name of a variable:
+
+  * :func:`gcc_jit_lvalue_set_register_name`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index 791a20398ca..afe333a520f 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -738,6 +738,26 @@ where the rvalue is computed by reading from the storage area.
 
   #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_link_section
 
+.. function:: void
+  gcc_jit_lvalue_set_register_name (gcc_jit_lvalue *lvalue,
+const char *reg_name);
+
+   Set the register name of a variable.
+   The parameter ``reg_name`` must be non-NULL. Analogous to:
+
+   .. code-block:: c
+
+ register int variable asm ("r12");
+
+   in C.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_21`; you can test for
+   its presence using
+
+   .. code-block:: c
+
+  #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_register_name
+
 Global variables
 
 
diff --git a/gcc/jit/dummy-frontend.c b/gcc/jit/dummy-frontend.c
index 84ff359bfe3..04d8fc6ab48 100644
--- a/gcc/jit/dummy-frontend.c
+++ b/gcc/jit/dummy-frontend.c
@@ -599,6 +599,8 @@ jit_langhook_init (void)
 
   build_common_builtin_nodes ();
 
+  clear_global_regs_cache ();
+
   /* The default precision for floating point numbers.  This is used
  for floating point constants with abstract type.  This may
  eventually be controllable by a command line option.  */
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index c93d7055d43..af4427c4503 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -24,6 +24,7 @@ along with GCC; see the file COPYING3.  If not see
 #include  // for std::pair
 
 #include "timevar.h"
+#include "varasm.h"
 
 #include "jit-recording.h"
 
@@ -701,6 +702,14 @@ public:
 set_decl_section_name (as_tree (), name);
   }
 
+  void
+  set_register_name (const char* reg_name)
+  {
+set_user_assembler_name (as_tree (), reg_name);
+DECL_REGISTER (as_tree ()) = 1;
+DECL_HARD_REGISTER (as_tree ()) = 1;
+  }
+
 pri