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 00000000000..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 <boua...@zoho.com>
Date: Sun, 29 Aug 2021 10:54:55 -0400
Subject: [PATCH] libgccjit: Add support for register variables [PR104072]
2022-01-17 Antoni Boucher <boua...@zoho.com>
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 | 54 +++++++++++++++++++
gcc/toplev.cc | 1 +
15 files changed, 248 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 *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_22`; 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/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 <utility> // 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;
+ }
+
private:
bool mark_addressable (location *loc);
};
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 1e3fadfacd7..5703114f138 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -3807,6 +3807,11 @@ void recording::lvalue::set_link_section (const char *name)
m_link_section = new_string (name);
}
+void recording::lvalue::set_register_name (const char *reg_name)
+{
+ m_reg_name = new_string (reg_name);
+}
+
/* The implementation of class gcc::jit::recording::param. */
/* Implementation of pure virtual hook recording::memento::replay_into
@@ -4673,6 +4678,9 @@ recording::global::replay_into (replayer *r)
if (m_link_section != NULL)
global->set_link_section (m_link_section->c_str ());
+ if (m_reg_name != NULL)
+ global->set_register_name (m_reg_name->c_str ());
+
set_playback_obj (global);
}
@@ -6343,11 +6351,15 @@ recording::function_pointer::write_reproducer (reproducer &r)
void
recording::local::replay_into (replayer *r)
{
- set_playback_obj (
- m_func->playback_function ()
+ playback::lvalue *obj = m_func->playback_function ()
->new_local (playback_location (r, m_loc),
m_type->playback_type (),
- playback_string (m_name)));
+ playback_string (m_name));
+
+ if (m_reg_name != NULL)
+ obj->set_register_name (m_reg_name->c_str ());
+
+ set_playback_obj (obj);
}
/* Override the default implementation of
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 846d65cb202..60b363d590e 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1147,8 +1147,9 @@ public:
location *loc,
type *type_)
: rvalue (ctxt, loc, type_),
- m_tls_model (GCC_JIT_TLS_MODEL_NONE),
- m_link_section (NULL)
+ m_link_section (NULL),
+ m_reg_name (NULL),
+ m_tls_model (GCC_JIT_TLS_MODEL_NONE)
{}
playback::lvalue *
@@ -1172,10 +1173,12 @@ public:
virtual bool is_global () const { return false; }
void set_tls_model (enum gcc_jit_tls_model model);
void set_link_section (const char *name);
+ void set_register_name (const char *reg_name);
protected:
- enum gcc_jit_tls_model m_tls_model;
string *m_link_section;
+ string *m_reg_name;
+ enum gcc_jit_tls_model m_tls_model;
};
class param : public lvalue
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 4c352e8c93d..c2adaa384b6 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -2649,6 +2649,20 @@ 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.cc. */
+
+void
+gcc_jit_lvalue_set_register_name (gcc_jit_lvalue *lvalue,
+ const char *reg_name)
+{
+ RETURN_IF_FAIL (lvalue, NULL, NULL, "NULL lvalue");
+ RETURN_IF_FAIL (reg_name, NULL, NULL, "NULL reg_name");
+ lvalue->set_register_name (reg_name);
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 2a5ffacb1fe..c8a9ec4f6a4 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1277,6 +1277,18 @@ extern void
gcc_jit_lvalue_set_link_section (gcc_jit_lvalue *lvalue,
const char *section_name);
+#define LIBGCCJIT_HAVE_gcc_jit_lvalue_set_register_name
+
+/* Make this variable a register variable and set its register name.
+
+ This API entrypoint was added in LIBGCCJIT_ABI_22; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_lvalue_set_register_name
+*/
+void
+gcc_jit_lvalue_set_register_name (gcc_jit_lvalue *lvalue,
+ const char *reg_name);
+
extern gcc_jit_lvalue *
gcc_jit_function_new_local (gcc_jit_function *func,
gcc_jit_location *loc,
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index f373fd39ac7..8b7dc13c97d 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -243,3 +243,14 @@ LIBGCCJIT_ABI_19 {
gcc_jit_context_new_union_constructor;
gcc_jit_global_set_initializer_rvalue;
} LIBGCCJIT_ABI_18;
+
+LIBGCCJIT_ABI_20 {
+} LIBGCCJIT_ABI_19;
+
+LIBGCCJIT_ABI_21 {
+} LIBGCCJIT_ABI_20;
+
+LIBGCCJIT_ABI_22 {
+ global:
+ gcc_jit_lvalue_set_register_name;
+} LIBGCCJIT_ABI_21;
diff --git a/gcc/reginfo.cc b/gcc/reginfo.cc
index 234f72eceeb..07ee9596e80 100644
--- a/gcc/reginfo.cc
+++ b/gcc/reginfo.cc
@@ -122,6 +122,24 @@ const char * reg_class_names[] = REG_CLASS_NAMES;
reginfo has been initialized. */
static int no_global_reg_vars = 0;
+static void
+clear_global_regs_cache (void)
+{
+ for (size_t i = 0 ; i < FIRST_PSEUDO_REGISTER ; i++)
+ {
+ global_regs[i] = 0;
+ global_regs_decl[i] = NULL;
+ }
+}
+
+void
+reginfo_cc_finalize (void)
+{
+ clear_global_regs_cache ();
+ no_global_reg_vars = 0;
+ CLEAR_HARD_REG_SET (global_reg_set);
+}
+
/* Given a register bitmap, turn on the bits in a HARD_REG_SET that
correspond to the hard registers, if any, set in that map. This
could be done far more efficiently by having all sorts of special-cases
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 648f9b8a601..13163d94ec9 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -3774,6 +3774,7 @@ extern bool resize_reg_info (void);
extern void free_reg_info (void);
extern void init_subregs_of_mode (void);
extern void finish_subregs_of_mode (void);
+extern void reginfo_cc_finalize (void);
/* recog.cc */
extern rtx extract_asm_operands (rtx);
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 29afe064db6..0603ace255a 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -306,6 +306,9 @@
#undef create_code
#undef verify_code
+/* test-register-variable.c: This can't be in the testcases array as it
+ doesn't have a verify_code implementation. */
+
/* test-string-literal.c */
#define create_code create_code_string_literal
#define verify_code verify_code_string_literal
diff --git a/gcc/testsuite/jit.dg/test-error-register-variable-bad-name.c b/gcc/testsuite/jit.dg/test-error-register-variable-bad-name.c
new file mode 100644
index 00000000000..3f2699374af
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-error-register-variable-bad-name.c
@@ -0,0 +1,35 @@
+/*
+
+ Test that the proper error is triggered when we build a register variable
+ with a register name that doesn't exist.
+
+*/
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ gcc_jit_type *int_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+ gcc_jit_lvalue *global_variable =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "global_variable");
+ gcc_jit_lvalue_set_register_name(global_variable, "this_is_not_a_register");
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ /* Ensure that the bad API usage prevents the API giving a bogus
+ result back. */
+ CHECK_VALUE (result, NULL);
+
+ /* Verify that the correct error message was emitted. */
+ CHECK_STRING_VALUE (gcc_jit_context_get_last_error (ctxt),
+ "invalid register name for '\033[01m\033[Kglobal_variable\33[m\33[K'");
+}
diff --git a/gcc/testsuite/jit.dg/test-error-register-variable-size-mismatch.c b/gcc/testsuite/jit.dg/test-error-register-variable-size-mismatch.c
new file mode 100644
index 00000000000..d9a0ac505d1
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-error-register-variable-size-mismatch.c
@@ -0,0 +1,40 @@
+/*
+
+ Test that the proper error is triggered when we build a register variable
+ with a register name that doesn't exist.
+
+*/
+
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ gcc_jit_type *int_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+ gcc_jit_type *array_type =
+ gcc_jit_context_new_array_type (ctxt, NULL, int_type, 4096);
+ gcc_jit_lvalue *global_variable =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, array_type, "global_variable");
+ gcc_jit_lvalue_set_register_name(global_variable, "r12");
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ /* Ensure that the bad API usage prevents the API giving a bogus
+ result back. */
+ CHECK_VALUE (result, NULL);
+
+ /* Verify that the correct error message was emitted. */
+ // FIXME: this doesn't compare equal because it seems global_variable is formatted in bold.
+ // Maybe trigger the error myself with
+ // decode_reg_name (asmspec)?
+ CHECK_STRING_VALUE (gcc_jit_context_get_last_error (ctxt),
+ "data type of '\033[01m\033[Kglobal_variable\33[m\33[K' isn't suitable for a register");
+}
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 00000000000..3cea3f1668f
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-register-variable.c
@@ -0,0 +1,54 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#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 "output-of-test-link-section-assembler.c.s"
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+ register int global_variable asm ("r13");
+ int main() {
+ register int variable asm ("r12");
+ return 0;
+ }
+ */
+ gcc_jit_type *int_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+ gcc_jit_lvalue *global_variable =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, int_type, "global_variable");
+ gcc_jit_lvalue_set_register_name(global_variable, "r13");
+
+ gcc_jit_function *func_main =
+ gcc_jit_context_new_function (ctxt, NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ int_type,
+ "main",
+ 0, NULL,
+ 0);
+ gcc_jit_lvalue *variable = gcc_jit_function_new_local(func_main, NULL, int_type, "variable");
+ gcc_jit_lvalue_set_register_name(variable, "r12");
+ gcc_jit_rvalue *two = gcc_jit_context_new_rvalue_from_int (ctxt, int_type, 2);
+ gcc_jit_rvalue *one = gcc_jit_context_one (ctxt, int_type);
+ gcc_jit_block *block = gcc_jit_function_new_block (func_main, NULL);
+ gcc_jit_block_add_assignment(block, NULL, variable, one);
+ gcc_jit_block_add_assignment(block, NULL, global_variable, two);
+ gcc_jit_block_end_with_return (block, NULL, gcc_jit_lvalue_as_rvalue(variable));
+}
+
+/* { 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" } } */
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index 534da1462e8..edba96b002d 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -2368,6 +2368,7 @@ toplev::finalize (void)
gcse_c_finalize ();
ipa_cp_c_finalize ();
ira_costs_c_finalize ();
+ reginfo_cc_finalize ();
/* save_decoded_options uses opts_obstack, so these must
be cleaned up together. */
--
2.26.2.7.g19db9cfb68.dirty
