public inbox for jit@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libgccjit: Add support for register variables [PR104072]
@ 2022-01-18  0:24 Antoni Boucher
  2022-01-18  0:46 ` Antoni Boucher
  0 siblings, 1 reply; 10+ messages in thread
From: Antoni Boucher @ 2022-01-18  0:24 UTC (permalink / raw)
  To: gcc-patches, jit

[-- Attachment #1: Type: text/plain, Size: 204 bytes --]

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.

[-- Attachment #2: 0001-libgccjit-Add-support-for-register-variables-PR10407.patch --]
[-- Type: text/x-patch, Size: 12518 bytes --]

From 328eca2be438c4a05c21942b4b2c3650ff7de0eb Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@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 <bouanto@zoho.com>

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 <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.c b/gcc/jit/jit-recording.c
index ee8934131d1..393d72298a7 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -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 b663d0f21f5..d8575487de4 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.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)
+{
+  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..bf63c13903d 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1277,6 +1277,13 @@ 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.  */
+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..f45abcf5305 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -243,3 +243,12 @@ LIBGCCJIT_ABI_19 {
     gcc_jit_context_new_union_constructor;
     gcc_jit_global_set_initializer_rvalue;
 } LIBGCCJIT_ABI_18;
+
+LIBGCCJIT_ABI_20 {
+  global:
+} LIBGCCJIT_ABI_19;
+
+LIBGCCJIT_ABI_21 {
+  global:
+    gcc_jit_lvalue_set_register_name;
+} LIBGCCJIT_ABI_20;
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)
+{
+  for (size_t i = 0 ; i < FIRST_PSEUDO_REGISTER ; i++)
+  {
+    global_regs[i] = 0;
+  }
+}
+
 /* The set of global registers.  */
 HARD_REG_SET global_reg_set;
 
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);
+
 #endif /* ! GCC_SYSTEM_H */
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-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" } } */
-- 
2.26.2.7.g19db9cfb68.dirty


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-18  0:24 [PATCH] libgccjit: Add support for register variables [PR104072] Antoni Boucher
@ 2022-01-18  0:46 ` Antoni Boucher
  2022-01-18 15:54   ` Antoni Boucher
  2022-01-18 23:49   ` David Malcolm
  0 siblings, 2 replies; 10+ messages in thread
From: Antoni Boucher @ 2022-01-18  0:46 UTC (permalink / raw)
  To: gcc-patches, jit

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

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.


[-- Attachment #2: 0001-libgccjit-Add-support-for-register-variables-PR10407.patch --]
[-- Type: text/x-patch, Size: 12678 bytes --]

From 2d372a741e9ef29eb377f9edfc6c539d1b2722e5 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@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 <bouanto@zoho.com>

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 <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.c b/gcc/jit/jit-recording.c
index ee8934131d1..393d72298a7 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -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 b663d0f21f5..d8575487de4 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.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)
+{
+  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..fec6d516855 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_21; 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..f45abcf5305 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -243,3 +243,12 @@ LIBGCCJIT_ABI_19 {
     gcc_jit_context_new_union_constructor;
     gcc_jit_global_set_initializer_rvalue;
 } LIBGCCJIT_ABI_18;
+
+LIBGCCJIT_ABI_20 {
+  global:
+} LIBGCCJIT_ABI_19;
+
+LIBGCCJIT_ABI_21 {
+  global:
+    gcc_jit_lvalue_set_register_name;
+} LIBGCCJIT_ABI_20;
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)
+{
+  for (size_t i = 0 ; i < FIRST_PSEUDO_REGISTER ; i++)
+  {
+    global_regs[i] = 0;
+  }
+}
+
 /* The set of global registers.  */
 HARD_REG_SET global_reg_set;
 
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);
+
 #endif /* ! GCC_SYSTEM_H */
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-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" } } */
-- 
2.26.2.7.g19db9cfb68.dirty


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-18  0:46 ` Antoni Boucher
@ 2022-01-18 15:54   ` Antoni Boucher
  2022-01-18 23:49   ` David Malcolm
  1 sibling, 0 replies; 10+ messages in thread
From: Antoni Boucher @ 2022-01-18 15:54 UTC (permalink / raw)
  To: gcc-patches, jit

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-18  0:46 ` Antoni Boucher
  2022-01-18 15:54   ` Antoni Boucher
@ 2022-01-18 23:49   ` David Malcolm
  2022-01-23  0:29     ` Antoni Boucher
  1 sibling, 1 reply; 10+ messages in thread
From: David Malcolm @ 2022-01-18 23:49 UTC (permalink / raw)
  To: Antoni Boucher, gcc-patches, jit

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 <bouanto@zoho.com>
> 
> 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 <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;
> +  }

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 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" } } */

How target-specific is this test?

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)

Hope this is constructive; thanks again for the patch
Dave



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-18 23:49   ` David Malcolm
@ 2022-01-23  0:29     ` Antoni Boucher
  2022-01-24 23:20       ` David Malcolm
  0 siblings, 1 reply; 10+ messages in thread
From: Antoni Boucher @ 2022-01-23  0:29 UTC (permalink / raw)
  To: David Malcolm, gcc-patches, jit

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 <bouanto@zoho.com>
> > 
> > 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 <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;
> > +  }
> 
> 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.

Done.

> 
> > +}
> > +
> 
> 
> > 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 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" } } */
> 
> How target-specific is this test?

It will only work on x86-64. Should I feature-gate the test somehow?

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

> 
> Hope this is constructive; thanks again for the patch
> Dave
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-23  0:29     ` Antoni Boucher
@ 2022-01-24 23:20       ` David Malcolm
  2022-01-24 23:28         ` Antoni Boucher
  2022-01-25 17:13         ` Antoni Boucher
  0 siblings, 2 replies; 10+ messages in thread
From: David Malcolm @ 2022-01-24 23:20 UTC (permalink / raw)
  To: Antoni Boucher, gcc-patches, jit

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-24 23:20       ` David Malcolm
@ 2022-01-24 23:28         ` Antoni Boucher
  2022-01-25 17:13         ` Antoni Boucher
  1 sibling, 0 replies; 10+ messages in thread
From: Antoni Boucher @ 2022-01-24 23:28 UTC (permalink / raw)
  To: David Malcolm, gcc-patches, jit

[-- Attachment #1: Type: text/plain, Size: 4096 bytes --]

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
> 


[-- Attachment #2: 0001-libgccjit-Add-support-for-register-variables-PR10407.patch --]
[-- Type: text/x-patch, Size: 16410 bytes --]

From cd76593905a43ceb53cb2325f2b742ba331da2f8 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@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 <bouanto@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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-24 23:20       ` David Malcolm
  2022-01-24 23:28         ` Antoni Boucher
@ 2022-01-25 17:13         ` Antoni Boucher
  2022-02-10  1:33           ` Antoni Boucher
  1 sibling, 1 reply; 10+ messages in thread
From: Antoni Boucher @ 2022-01-25 17:13 UTC (permalink / raw)
  To: David Malcolm, gcc-patches, jit

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

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
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-01-25 17:13         ` Antoni Boucher
@ 2022-02-10  1:33           ` Antoni Boucher
  2022-04-12 21:43             ` David Malcolm
  0 siblings, 1 reply; 10+ messages in thread
From: Antoni Boucher @ 2022-02-10  1:33 UTC (permalink / raw)
  To: David Malcolm, gcc-patches, jit

[-- Attachment #1: Type: text/plain, Size: 5097 bytes --]

Here's the updated patch.

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


[-- Attachment #2: 0001-libgccjit-Add-support-for-register-variables-PR10407.patch --]
[-- Type: text/x-patch, Size: 16451 bytes --]

From 542b7af61292a04d61480388d425a1057ecd66d4 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@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 <bouanto@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 | 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 *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..ce6dcaa9213
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-register-variable.c
@@ -0,0 +1,56 @@
+/* { dg-do compile { target x86_64-*-* } } */
+
+#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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] libgccjit: Add support for register variables [PR104072]
  2022-02-10  1:33           ` Antoni Boucher
@ 2022-04-12 21:43             ` David Malcolm
  0 siblings, 0 replies; 10+ messages in thread
From: David Malcolm @ 2022-04-12 21:43 UTC (permalink / raw)
  To: Antoni Boucher, gcc-patches, jit

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



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-04-12 21:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-18  0:24 [PATCH] libgccjit: Add support for register variables [PR104072] Antoni Boucher
2022-01-18  0:46 ` Antoni Boucher
2022-01-18 15:54   ` Antoni Boucher
2022-01-18 23:49   ` David Malcolm
2022-01-23  0:29     ` Antoni Boucher
2022-01-24 23:20       ` David Malcolm
2022-01-24 23:28         ` Antoni Boucher
2022-01-25 17:13         ` Antoni Boucher
2022-02-10  1:33           ` Antoni Boucher
2022-04-12 21:43             ` David Malcolm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).