public inbox for jit@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libgccjit: Add support for creating temporary variables
@ 2024-01-19 21:54 Antoni Boucher
  2024-01-24 14:54 ` David Malcolm
  0 siblings, 1 reply; 3+ messages in thread
From: Antoni Boucher @ 2024-01-19 21:54 UTC (permalink / raw)
  To: gcc-patches, jit

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

Hi.
This patch adds a new way to create local variable that won't generate
debug info: it is to be used for compiler-generated variables.
Thanks for the review.

[-- Attachment #2: 0001-libgccjit-Add-support-for-creating-temporary-variabl.patch --]
[-- Type: text/x-patch, Size: 14071 bytes --]

From 6f69e9db77f3c7e019fae74414ba5eed15298514 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Thu, 18 Jan 2024 16:54:59 -0500
Subject: [PATCH] libgccjit: Add support for creating temporary variables

gcc/jit/ChangeLog:

	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_26): New ABI tag.
	* docs/topics/functions.rst: Document gcc_jit_function_new_temp.
	* jit-playback.cc (new_local): Add new is_temp parameter.
	* jit-playback.h: Add new is_temp parameter.
	* jit-recording.cc (recording::function::new_temp): New method.
	(recording::local::replay_into): Support temporary variables.
	(recording::local::write_reproducer): Support temporary
	variables.
	* jit-recording.h (new_temp): New method.
	(m_is_temp): New field.
	* libgccjit.cc (gcc_jit_function_new_temp): New function.
	* libgccjit.h (gcc_jit_function_new_temp): New function.
	* libgccjit.map: New function.

gcc/testsuite/ChangeLog:

	* jit.dg/all-non-failing-tests.h: Mention test-temp.c.
	* jit.dg/test-temp.c: New test.
---
 gcc/jit/docs/topics/compatibility.rst        |  9 ++++
 gcc/jit/docs/topics/functions.rst            | 20 +++++++
 gcc/jit/jit-playback.cc                      | 21 ++++++--
 gcc/jit/jit-playback.h                       |  3 +-
 gcc/jit/jit-recording.cc                     | 52 +++++++++++++-----
 gcc/jit/jit-recording.h                      | 17 ++++--
 gcc/jit/libgccjit.cc                         | 31 +++++++++++
 gcc/jit/libgccjit.h                          |  7 +++
 gcc/jit/libgccjit.map                        |  5 ++
 gcc/testsuite/jit.dg/all-non-failing-tests.h |  3 ++
 gcc/testsuite/jit.dg/test-temp.c             | 56 ++++++++++++++++++++
 11 files changed, 205 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-temp.c

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index cbf5b414d8c..5d62e264a00 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -390,3 +390,12 @@ on functions and variables:
   * :func:`gcc_jit_function_add_string_attribute`
   * :func:`gcc_jit_function_add_integer_array_attribute`
   * :func:`gcc_jit_lvalue_add_string_attribute`
+
+.. _LIBGCCJIT_ABI_27:
+
+``LIBGCCJIT_ABI_27``
+--------------------
+``LIBGCCJIT_ABI_27`` covers the addition of a functions to create a new
+temporary variable:
+
+  * :func:`gcc_jit_function_new_temp`
diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/functions.rst
index 804605ea939..230caf42466 100644
--- a/gcc/jit/docs/topics/functions.rst
+++ b/gcc/jit/docs/topics/functions.rst
@@ -171,6 +171,26 @@ Functions
    underlying string, so it is valid to pass in a pointer to an on-stack
    buffer.
 
+.. function:: gcc_jit_lvalue *\
+              gcc_jit_function_new_temp (gcc_jit_function *func,\
+                                         gcc_jit_location *loc,\
+                                         gcc_jit_type *type)
+
+   Create a new local variable within the function, of the given type.
+   This function is similar to :func:`gcc_jit_function_new_local`, but
+   it is to be used for compiler-generated variables (as opposed to
+   user-defined variables in the language to be compiled) and these
+   variables won't show up in the debug info.
+
+   The parameter ``type`` must be non-`void`.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; you can test
+   for its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_gcc_jit_function_new_temp
+
 .. function::  size_t \
                gcc_jit_function_get_param_count (gcc_jit_function *func)
 
diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
index 84df6c100e6..cb6b2f66276 100644
--- a/gcc/jit/jit-playback.cc
+++ b/gcc/jit/jit-playback.cc
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "toplev.h"
 #include "tree-cfg.h"
 #include "convert.h"
+#include "gimple-expr.h"
 #include "stor-layout.h"
 #include "print-tree.h"
 #include "gimplify.h"
@@ -1950,13 +1951,27 @@ new_local (location *loc,
 	   type *type,
 	   const char *name,
 	   const std::vector<std::pair<gcc_jit_variable_attribute,
-				       std::string>> &attributes)
+				       std::string>> &attributes,
+	   bool is_temp)
 {
   gcc_assert (type);
-  gcc_assert (name);
-  tree inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
+  tree inner;
+  if (is_temp)
+  {
+    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
+			create_tmp_var_name ("JITTMP"),
+			type->as_tree ());
+    DECL_ARTIFICIAL (inner) = 1;
+    DECL_IGNORED_P (inner) = 1;
+    DECL_NAMELESS (inner) = 1;
+  }
+  else
+  {
+    gcc_assert (name);
+    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
 			   get_identifier (name),
 			   type->as_tree ());
+  }
   DECL_CONTEXT (inner) = this->m_inner_fndecl;
 
   /* Prepend to BIND_EXPR_VARS: */
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index 05bafcd21c4..d285c5a99af 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -527,7 +527,8 @@ public:
 	     type *type,
 	     const char *name,
 	     const std::vector<std::pair<gcc_jit_variable_attribute,
-					 std::string>> &attributes);
+					 std::string>> &attributes,
+	     bool is_temp);
 
   block*
   new_block (const char *name);
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 6ffadbea127..9d7a445e18c 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -4190,7 +4190,24 @@ recording::function::new_local (recording::location *loc,
 				type *type,
 				const char *name)
 {
-  local *result = new local (this, loc, type, new_string (name));
+  local *result = new local (this, loc, type, new_string (name), false);
+  m_ctxt->record (result);
+  m_locals.safe_push (result);
+  return result;
+}
+
+/* Create a recording::local instance and add it to
+   the functions's context's list of mementos, and to the function's
+   list of locals.
+
+   Implements the post-error-checking part of
+   gcc_jit_function_new_temp.  */
+
+recording::lvalue *
+recording::function::new_temp (recording::location *loc,
+			       type *type)
+{
+  local *result = new local (this, loc, type, NULL, true);
   m_ctxt->record (result);
   m_locals.safe_push (result);
   return result;
@@ -6770,7 +6787,8 @@ recording::local::replay_into (replayer *r)
       ->new_local (playback_location (r, m_loc),
 		   m_type->playback_type (),
 		   playback_string (m_name),
-		   m_string_attributes);
+		   m_string_attributes,
+		   m_is_temp);
 
   if (m_reg_name != NULL)
     obj->set_register_name (m_reg_name->c_str ());
@@ -6801,16 +6819,26 @@ void
 recording::local::write_reproducer (reproducer &r)
 {
   const char *id = r.make_identifier (this, "local");
-  r.write ("  gcc_jit_lvalue *%s =\n"
-	   "    gcc_jit_function_new_local (%s, /* gcc_jit_function *func */\n"
-	   "                                %s, /* gcc_jit_location *loc */\n"
-	   "                                %s, /* gcc_jit_type *type */\n"
-	   "                                %s); /* const char *name */\n",
-	   id,
-	   r.get_identifier (m_func),
-	   r.get_identifier (m_loc),
-	   r.get_identifier_as_type (m_type),
-	   m_name->get_debug_string ());
+  if (m_is_temp)
+    r.write ("  gcc_jit_lvalue *%s =\n"
+	     "    gcc_jit_function_new_temp (%s, /* gcc_jit_function *func */\n"
+	     "                               %s, /* gcc_jit_location *loc */\n"
+	     "                               %s); /* gcc_jit_type *type */\n",
+	     id,
+	     r.get_identifier (m_func),
+	     r.get_identifier (m_loc),
+	     r.get_identifier_as_type (m_type));
+  else
+    r.write ("  gcc_jit_lvalue *%s =\n"
+	     "    gcc_jit_function_new_local (%s, /* gcc_jit_function *func */\n"
+	     "                                %s, /* gcc_jit_location *loc */\n"
+	     "                                %s, /* gcc_jit_type *type */\n"
+	     "                                %s); /* const char *name */\n",
+	     id,
+	     r.get_identifier (m_func),
+	     r.get_identifier (m_loc),
+	     r.get_identifier_as_type (m_type),
+	     m_name->get_debug_string ());
 }
 
 /* The implementation of class gcc::jit::recording::statement.  */
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index cd2e0adbe30..559f44f1989 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1330,6 +1330,10 @@ public:
 	     type *type,
 	     const char *name);
 
+  lvalue *
+  new_temp (location *loc,
+	    type *type);
+
   block*
   new_block (const char *name);
 
@@ -2092,10 +2096,11 @@ private:
 class local : public lvalue
 {
 public:
-  local (function *func, location *loc, type *type_, string *name)
+  local (function *func, location *loc, type *type_, string *name, bool is_temp)
     : lvalue (func->m_ctxt, loc, type_),
     m_func (func),
-    m_name (name)
+    m_name (name),
+    m_is_temp (is_temp)
   {
     set_scope (func);
   }
@@ -2109,7 +2114,12 @@ public:
   void write_to_dump (dump &d) final override;
 
 private:
-  string * make_debug_string () final override { return m_name; }
+  string * make_debug_string () final override {
+    if (m_is_temp)
+      return m_ctxt->new_string ("temp");
+    else
+      return m_name;
+  }
   void write_reproducer (reproducer &r) final override;
   enum precedence get_precedence () const final override
   {
@@ -2119,6 +2129,7 @@ private:
 private:
   function *m_func;
   string *m_name;
+  bool m_is_temp;
 };
 
 class statement : public memento
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 9616f3802b8..1687a347e18 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -2790,6 +2790,37 @@ gcc_jit_function_new_local (gcc_jit_function *func,
   return (gcc_jit_lvalue *)func->new_local (loc, type, name);
 }
 
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is done by the
+   gcc::jit::recording::function::new_temp method in jit-recording.cc.  */
+
+gcc_jit_lvalue *
+gcc_jit_function_new_temp (gcc_jit_function *func,
+			   gcc_jit_location *loc,
+			   gcc_jit_type *type)
+{
+  RETURN_NULL_IF_FAIL (func, NULL, loc, "NULL function");
+  gcc::jit::recording::context *ctxt = func->m_ctxt;
+  JIT_LOG_FUNC (ctxt->get_logger ());
+  /* LOC can be NULL.  */
+  RETURN_NULL_IF_FAIL (func->get_kind () != GCC_JIT_FUNCTION_IMPORTED,
+		       ctxt, loc,
+		       "Cannot add temps to an imported function");
+  RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
+  RETURN_NULL_IF_FAIL_PRINTF1 (
+    type->has_known_size (),
+    ctxt, loc,
+    "unknown size for temp (type: %s)",
+    type->get_debug_string ());
+  RETURN_NULL_IF_FAIL (
+    !type->is_void (),
+    ctxt, loc,
+    "void type for temp");
+
+  return (gcc_jit_lvalue *)func->new_temp (loc, type);
+}
+
 /* Public entrypoint.  See description in libgccjit.h.
 
    After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 235cab053e0..05349d589d6 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1384,6 +1384,13 @@ gcc_jit_function_new_local (gcc_jit_function *func,
 			    gcc_jit_type *type,
 			    const char *name);
 
+extern gcc_jit_lvalue *
+gcc_jit_function_new_temp (gcc_jit_function *func,
+			   gcc_jit_location *loc,
+			   gcc_jit_type *type);
+
+#define LIBGCCJIT_HAVE_gcc_jit_function_new_temp
+
 /**********************************************************************
  Statement-creation.
  **********************************************************************/
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index dfb8a9d51fb..598513e2584 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -284,3 +284,8 @@ LIBGCCJIT_ABI_26 {
     gcc_jit_lvalue_add_string_attribute;
     gcc_jit_function_add_integer_array_attribute;
 } LIBGCCJIT_ABI_25;
+
+LIBGCCJIT_ABI_27 {
+  global:
+    gcc_jit_function_new_temp;
+} LIBGCCJIT_ABI_26;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index a3bd0358c02..a141198f1b5 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -346,6 +346,9 @@
 /* test-setting-alignment.c: This can't be in the testcases array as it
    is target-specific.  */
 
+/* test-temp.c: This can't be in the testcases array as it
+   is target-specific.  */
+
 /* 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-temp.c b/gcc/testsuite/jit.dg/test-temp.c
new file mode 100644
index 00000000000..9b1ba9b0100
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-temp.c
@@ -0,0 +1,56 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include "libgccjit.h"
+
+#define TEST_COMPILING_TO_FILE
+#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_ASSEMBLER
+#define OUTPUT_FILENAME  "output-of-test-test-temp.c.s"
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Let's try to inject the equivalent of:
+int
+func ()
+{
+   int temp = 10;
+   return temp;
+}
+   */
+  gcc_jit_type *int_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+
+  gcc_jit_function *func =
+    gcc_jit_context_new_function (ctxt,
+				  NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  int_type,
+				  "func",
+				  0, NULL, 0);
+
+  gcc_jit_block *initial =
+    gcc_jit_function_new_block (func, "initial");
+
+  gcc_jit_lvalue *temp =
+    gcc_jit_function_new_temp (func, NULL, int_type);
+
+  gcc_jit_rvalue *ten =
+    gcc_jit_context_new_rvalue_from_int (ctxt, int_type, 10);
+  gcc_jit_block_add_assignment (initial, NULL, temp, ten);
+
+  gcc_jit_block_end_with_return(initial, NULL,
+    gcc_jit_lvalue_as_rvalue (temp));
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  CHECK_NON_NULL (result);
+}
+
+/* { dg-final { jit-verify-output-file-was-created "" } } */
+/* { dg-final { jit-verify-assembler-output-not "JITTMP" } } */
-- 
2.43.0


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

* Re: [PATCH] libgccjit: Add support for creating temporary variables
  2024-01-19 21:54 [PATCH] libgccjit: Add support for creating temporary variables Antoni Boucher
@ 2024-01-24 14:54 ` David Malcolm
  2024-02-29 21:11   ` Antoni Boucher
  0 siblings, 1 reply; 3+ messages in thread
From: David Malcolm @ 2024-01-24 14:54 UTC (permalink / raw)
  To: Antoni Boucher, gcc-patches, jit

On Fri, 2024-01-19 at 16:54 -0500, Antoni Boucher wrote:
> Hi.
> This patch adds a new way to create local variable that won't
> generate
> debug info: it is to be used for compiler-generated variables.
> Thanks for the review.

Thanks for the patch.

> diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
> index cbf5b414d8c..5d62e264a00 100644
> --- a/gcc/jit/docs/topics/compatibility.rst
> +++ b/gcc/jit/docs/topics/compatibility.rst
> @@ -390,3 +390,12 @@ on functions and variables:
>    * :func:`gcc_jit_function_add_string_attribute`
>    * :func:`gcc_jit_function_add_integer_array_attribute`
>    * :func:`gcc_jit_lvalue_add_string_attribute`
> +
> +.. _LIBGCCJIT_ABI_27:
> +
> +``LIBGCCJIT_ABI_27``
> +--------------------
> +``LIBGCCJIT_ABI_27`` covers the addition of a functions to create a new

"functions" -> "function"

> +temporary variable:
> +
> +  * :func:`gcc_jit_function_new_temp`
> diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/functions.rst
> index 804605ea939..230caf42466 100644
> --- a/gcc/jit/docs/topics/functions.rst
> +++ b/gcc/jit/docs/topics/functions.rst
> @@ -171,6 +171,26 @@ Functions
>     underlying string, so it is valid to pass in a pointer to an on-stack
>     buffer.
>  
> +.. function:: gcc_jit_lvalue *\
> +              gcc_jit_function_new_temp (gcc_jit_function *func,\
> +                                         gcc_jit_location *loc,\
> +                                         gcc_jit_type *type)
> +
> +   Create a new local variable within the function, of the given type.
> +   This function is similar to :func:`gcc_jit_function_new_local`, but
> +   it is to be used for compiler-generated variables (as opposed to
> +   user-defined variables in the language to be compiled) and these
> +   variables won't show up in the debug info.
> +
> +   The parameter ``type`` must be non-`void`.
> +
> +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; you can test
> +   for its presence using

The ABI number is inconsistent here (it's 27 above and in the .map
file), but obviously you can fix this when you eventually commit this
based on what the ABI number actually is.

[...snip...]

> diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
> index 84df6c100e6..cb6b2f66276 100644
> --- a/gcc/jit/jit-playback.cc
> +++ b/gcc/jit/jit-playback.cc
> @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "toplev.h"
>  #include "tree-cfg.h"
>  #include "convert.h"
> +#include "gimple-expr.h"
>  #include "stor-layout.h"
>  #include "print-tree.h"
>  #include "gimplify.h"
> @@ -1950,13 +1951,27 @@ new_local (location *loc,
>  	   type *type,
>  	   const char *name,
>  	   const std::vector<std::pair<gcc_jit_variable_attribute,
> -				       std::string>> &attributes)
> +				       std::string>> &attributes,
> +	   bool is_temp)
>  {
>    gcc_assert (type);
> -  gcc_assert (name);
> -  tree inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> +  tree inner;
> +  if (is_temp)
> +  {
> +    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
> +			create_tmp_var_name ("JITTMP"),
> +			type->as_tree ());
> +    DECL_ARTIFICIAL (inner) = 1;
> +    DECL_IGNORED_P (inner) = 1;
> +    DECL_NAMELESS (inner) = 1;

We could assert that "name" is null in the is_temp branch.

An alternative approach might be to drop "is_temp", and instead make
"name" being null signify that it's a temporary, if you prefer that
approach.  Would client code ever want to specify a name prefix for a
temporary?


> +  }
> +  else
> +  {
> +    gcc_assert (name);
> +    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
>  			   get_identifier (name),
>  			   type->as_tree ());
> +  }
>    DECL_CONTEXT (inner) = this->m_inner_fndecl;
>  
>    /* Prepend to BIND_EXPR_VARS: */

[...snip...]

Thanks again for the patch.  Looks good to me as-is (apart from the
grammar and ABI number nits), but what do you think of eliminating
"is_temp" in favor of the "name" ptr being null?  I think it's your
call.

Dave


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

* Re: [PATCH] libgccjit: Add support for creating temporary variables
  2024-01-24 14:54 ` David Malcolm
@ 2024-02-29 21:11   ` Antoni Boucher
  0 siblings, 0 replies; 3+ messages in thread
From: Antoni Boucher @ 2024-02-29 21:11 UTC (permalink / raw)
  To: David Malcolm, gcc-patches, jit

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

Hi and thanks for the review!
Here's the updated patch.

Le 2024-01-24 à 09 h 54, David Malcolm a écrit :
> On Fri, 2024-01-19 at 16:54 -0500, Antoni Boucher wrote:
>> Hi.
>> This patch adds a new way to create local variable that won't
>> generate
>> debug info: it is to be used for compiler-generated variables.
>> Thanks for the review.
> 
> Thanks for the patch.
> 
>> diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
>> index cbf5b414d8c..5d62e264a00 100644
>> --- a/gcc/jit/docs/topics/compatibility.rst
>> +++ b/gcc/jit/docs/topics/compatibility.rst
>> @@ -390,3 +390,12 @@ on functions and variables:
>>     * :func:`gcc_jit_function_add_string_attribute`
>>     * :func:`gcc_jit_function_add_integer_array_attribute`
>>     * :func:`gcc_jit_lvalue_add_string_attribute`
>> +
>> +.. _LIBGCCJIT_ABI_27:
>> +
>> +``LIBGCCJIT_ABI_27``
>> +--------------------
>> +``LIBGCCJIT_ABI_27`` covers the addition of a functions to create a new
> 
> "functions" -> "function"
> 
>> +temporary variable:
>> +
>> +  * :func:`gcc_jit_function_new_temp`
>> diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/functions.rst
>> index 804605ea939..230caf42466 100644
>> --- a/gcc/jit/docs/topics/functions.rst
>> +++ b/gcc/jit/docs/topics/functions.rst
>> @@ -171,6 +171,26 @@ Functions
>>      underlying string, so it is valid to pass in a pointer to an on-stack
>>      buffer.
>>   
>> +.. function:: gcc_jit_lvalue *\
>> +              gcc_jit_function_new_temp (gcc_jit_function *func,\
>> +                                         gcc_jit_location *loc,\
>> +                                         gcc_jit_type *type)
>> +
>> +   Create a new local variable within the function, of the given type.
>> +   This function is similar to :func:`gcc_jit_function_new_local`, but
>> +   it is to be used for compiler-generated variables (as opposed to
>> +   user-defined variables in the language to be compiled) and these
>> +   variables won't show up in the debug info.
>> +
>> +   The parameter ``type`` must be non-`void`.
>> +
>> +   This entrypoint was added in :ref:`LIBGCCJIT_ABI_26`; you can test
>> +   for its presence using
> 
> The ABI number is inconsistent here (it's 27 above and in the .map
> file), but obviously you can fix this when you eventually commit this
> based on what the ABI number actually is.
> 
> [...snip...]
> 
>> diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
>> index 84df6c100e6..cb6b2f66276 100644
>> --- a/gcc/jit/jit-playback.cc
>> +++ b/gcc/jit/jit-playback.cc
>> @@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
>>   #include "toplev.h"
>>   #include "tree-cfg.h"
>>   #include "convert.h"
>> +#include "gimple-expr.h"
>>   #include "stor-layout.h"
>>   #include "print-tree.h"
>>   #include "gimplify.h"
>> @@ -1950,13 +1951,27 @@ new_local (location *loc,
>>   	   type *type,
>>   	   const char *name,
>>   	   const std::vector<std::pair<gcc_jit_variable_attribute,
>> -				       std::string>> &attributes)
>> +				       std::string>> &attributes,
>> +	   bool is_temp)
>>   {
>>     gcc_assert (type);
>> -  gcc_assert (name);
>> -  tree inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
>> +  tree inner;
>> +  if (is_temp)
>> +  {
>> +    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
>> +			create_tmp_var_name ("JITTMP"),
>> +			type->as_tree ());
>> +    DECL_ARTIFICIAL (inner) = 1;
>> +    DECL_IGNORED_P (inner) = 1;
>> +    DECL_NAMELESS (inner) = 1;
> 
> We could assert that "name" is null in the is_temp branch.
> 
> An alternative approach might be to drop "is_temp", and instead make
> "name" being null signify that it's a temporary, if you prefer that
> approach.  Would client code ever want to specify a name prefix for a
> temporary?

No, I don't think anyone would want a different prefix.

> 
> 
>> +  }
>> +  else
>> +  {
>> +    gcc_assert (name);
>> +    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
>>   			   get_identifier (name),
>>   			   type->as_tree ());
>> +  }
>>     DECL_CONTEXT (inner) = this->m_inner_fndecl;
>>   
>>     /* Prepend to BIND_EXPR_VARS: */
> 
> [...snip...]
> 
> Thanks again for the patch.  Looks good to me as-is (apart from the
> grammar and ABI number nits), but what do you think of eliminating
> "is_temp" in favor of the "name" ptr being null?  I think it's your
> call.
> 
> Dave
> 

[-- Attachment #2: 0001-libgccjit-Add-support-for-creating-temporary-variabl.patch --]
[-- Type: text/x-patch, Size: 12133 bytes --]

From 80ea12ce227b2ac5d5dcd99374532e30a775ecbd Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Thu, 18 Jan 2024 16:54:59 -0500
Subject: [PATCH] libgccjit: Add support for creating temporary variables

gcc/jit/ChangeLog:

	* docs/topics/compatibility.rst (LIBGCCJIT_ABI_28): New ABI tag.
	* docs/topics/functions.rst: Document gcc_jit_function_new_temp.
	* jit-playback.cc (new_local): Add support for temporary
	variables.
	* jit-recording.cc (recording::function::new_temp): New method.
	(recording::local::write_reproducer): Support temporary
	variables.
	* jit-recording.h (new_temp): New method.
	* libgccjit.cc (gcc_jit_function_new_temp): New function.
	* libgccjit.h (gcc_jit_function_new_temp): New function.
	* libgccjit.map: New function.

gcc/testsuite/ChangeLog:

	* jit.dg/all-non-failing-tests.h: Mention test-temp.c.
	* jit.dg/test-temp.c: New test.
---
 gcc/jit/docs/topics/compatibility.rst        |  9 ++++
 gcc/jit/docs/topics/functions.rst            | 20 +++++++
 gcc/jit/jit-playback.cc                      | 15 +++++-
 gcc/jit/jit-recording.cc                     | 47 ++++++++++++----
 gcc/jit/jit-recording.h                      | 11 +++-
 gcc/jit/libgccjit.cc                         | 31 +++++++++++
 gcc/jit/libgccjit.h                          |  7 +++
 gcc/jit/libgccjit.map                        |  5 ++
 gcc/testsuite/jit.dg/all-non-failing-tests.h |  3 ++
 gcc/testsuite/jit.dg/test-temp.c             | 56 ++++++++++++++++++++
 10 files changed, 191 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-temp.c

diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 9cfb054f653..ecd65010b5e 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -397,3 +397,12 @@ on functions and variables:
 --------------------
 ``LIBGCCJIT_ABI_27`` covers the addition of
 :func:`gcc_jit_context_new_sizeof`
+
+.. _LIBGCCJIT_ABI_28:
+
+``LIBGCCJIT_ABI_28``
+--------------------
+``LIBGCCJIT_ABI_28`` covers the addition of a function to create a new
+temporary variable:
+
+  * :func:`gcc_jit_function_new_temp`
diff --git a/gcc/jit/docs/topics/functions.rst b/gcc/jit/docs/topics/functions.rst
index 804605ea939..17eab55ee3f 100644
--- a/gcc/jit/docs/topics/functions.rst
+++ b/gcc/jit/docs/topics/functions.rst
@@ -171,6 +171,26 @@ Functions
    underlying string, so it is valid to pass in a pointer to an on-stack
    buffer.
 
+.. function:: gcc_jit_lvalue *\
+              gcc_jit_function_new_temp (gcc_jit_function *func,\
+                                         gcc_jit_location *loc,\
+                                         gcc_jit_type *type)
+
+   Create a new local variable within the function, of the given type.
+   This function is similar to :func:`gcc_jit_function_new_local`, but
+   it is to be used for compiler-generated variables (as opposed to
+   user-defined variables in the language to be compiled) and these
+   variables won't show up in the debug info.
+
+   The parameter ``type`` must be non-`void`.
+
+   This entrypoint was added in :ref:`LIBGCCJIT_ABI_28`; you can test
+   for its presence using
+
+   .. code-block:: c
+
+      #ifdef LIBGCCJIT_HAVE_gcc_jit_function_new_temp
+
 .. function::  size_t \
                gcc_jit_function_get_param_count (gcc_jit_function *func)
 
diff --git a/gcc/jit/jit-playback.cc b/gcc/jit/jit-playback.cc
index 6baa838af10..e454a64eb11 100644
--- a/gcc/jit/jit-playback.cc
+++ b/gcc/jit/jit-playback.cc
@@ -31,6 +31,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "toplev.h"
 #include "tree-cfg.h"
 #include "convert.h"
+#include "gimple-expr.h"
 #include "stor-layout.h"
 #include "print-tree.h"
 #include "gimplify.h"
@@ -1963,10 +1964,20 @@ new_local (location *loc,
 				       std::string>> &attributes)
 {
   gcc_assert (type);
-  gcc_assert (name);
-  tree inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
+  tree inner;
+  if (name)
+    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
 			   get_identifier (name),
 			   type->as_tree ());
+  else
+  {
+    inner = build_decl (UNKNOWN_LOCATION, VAR_DECL,
+			create_tmp_var_name ("JITTMP"),
+			type->as_tree ());
+    DECL_ARTIFICIAL (inner) = 1;
+    DECL_IGNORED_P (inner) = 1;
+    DECL_NAMELESS (inner) = 1;
+  }
   DECL_CONTEXT (inner) = this->m_inner_fndecl;
 
   /* Prepend to BIND_EXPR_VARS: */
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 68a2e860c1f..bd741a388db 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -4211,6 +4211,23 @@ recording::function::new_local (recording::location *loc,
   return result;
 }
 
+/* Create a recording::local instance and add it to
+   the functions's context's list of mementos, and to the function's
+   list of locals.
+
+   Implements the post-error-checking part of
+   gcc_jit_function_new_temp.  */
+
+recording::lvalue *
+recording::function::new_temp (recording::location *loc,
+			       type *type)
+{
+  local *result = new local (this, loc, type, NULL);
+  m_ctxt->record (result);
+  m_locals.safe_push (result);
+  return result;
+}
+
 /* Create a recording::block instance and add it to
    the functions's context's list of mementos, and to the function's
    list of blocks.
@@ -6853,16 +6870,26 @@ void
 recording::local::write_reproducer (reproducer &r)
 {
   const char *id = r.make_identifier (this, "local");
-  r.write ("  gcc_jit_lvalue *%s =\n"
-	   "    gcc_jit_function_new_local (%s, /* gcc_jit_function *func */\n"
-	   "                                %s, /* gcc_jit_location *loc */\n"
-	   "                                %s, /* gcc_jit_type *type */\n"
-	   "                                %s); /* const char *name */\n",
-	   id,
-	   r.get_identifier (m_func),
-	   r.get_identifier (m_loc),
-	   r.get_identifier_as_type (m_type),
-	   m_name->get_debug_string ());
+  if (m_name)
+    r.write ("  gcc_jit_lvalue *%s =\n"
+	     "    gcc_jit_function_new_local (%s, /* gcc_jit_function *func */\n"
+	     "                                %s, /* gcc_jit_location *loc */\n"
+	     "                                %s, /* gcc_jit_type *type */\n"
+	     "                                %s); /* const char *name */\n",
+	     id,
+	     r.get_identifier (m_func),
+	     r.get_identifier (m_loc),
+	     r.get_identifier_as_type (m_type),
+	     m_name->get_debug_string ());
+  else
+    r.write ("  gcc_jit_lvalue *%s =\n"
+	     "    gcc_jit_function_new_temp (%s, /* gcc_jit_function *func */\n"
+	     "                               %s, /* gcc_jit_location *loc */\n"
+	     "                               %s); /* gcc_jit_type *type */\n",
+	     id,
+	     r.get_identifier (m_func),
+	     r.get_identifier (m_loc),
+	     r.get_identifier_as_type (m_type));
 }
 
 /* The implementation of class gcc::jit::recording::statement.  */
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index d8d16f4fe29..e49d54c5dd3 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -1345,6 +1345,10 @@ public:
 	     type *type,
 	     const char *name);
 
+  lvalue *
+  new_temp (location *loc,
+	    type *type);
+
   block*
   new_block (const char *name);
 
@@ -2149,7 +2153,12 @@ public:
   void write_to_dump (dump &d) final override;
 
 private:
-  string * make_debug_string () final override { return m_name; }
+  string * make_debug_string () final override {
+    if (m_name)
+      return m_name;
+    else
+      return m_ctxt->new_string ("temp");
+  }
   void write_reproducer (reproducer &r) final override;
   enum precedence get_precedence () const final override
   {
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index f40a9781405..9d902115b5b 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -2808,6 +2808,37 @@ gcc_jit_function_new_local (gcc_jit_function *func,
   return (gcc_jit_lvalue *)func->new_local (loc, type, name);
 }
 
+/* Public entrypoint.  See description in libgccjit.h.
+
+   After error-checking, the real work is done by the
+   gcc::jit::recording::function::new_temp method in jit-recording.cc.  */
+
+gcc_jit_lvalue *
+gcc_jit_function_new_temp (gcc_jit_function *func,
+			   gcc_jit_location *loc,
+			   gcc_jit_type *type)
+{
+  RETURN_NULL_IF_FAIL (func, NULL, loc, "NULL function");
+  gcc::jit::recording::context *ctxt = func->m_ctxt;
+  JIT_LOG_FUNC (ctxt->get_logger ());
+  /* LOC can be NULL.  */
+  RETURN_NULL_IF_FAIL (func->get_kind () != GCC_JIT_FUNCTION_IMPORTED,
+		       ctxt, loc,
+		       "Cannot add temps to an imported function");
+  RETURN_NULL_IF_FAIL (type, ctxt, loc, "NULL type");
+  RETURN_NULL_IF_FAIL_PRINTF1 (
+    type->has_known_size (),
+    ctxt, loc,
+    "unknown size for temp (type: %s)",
+    type->get_debug_string ());
+  RETURN_NULL_IF_FAIL (
+    !type->is_void (),
+    ctxt, loc,
+    "void type for temp");
+
+  return (gcc_jit_lvalue *)func->new_temp (loc, type);
+}
+
 /* Public entrypoint.  See description in libgccjit.h.
 
    After error-checking, the real work is done by the
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 74e847b2dec..07f2e50112f 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -1396,6 +1396,13 @@ gcc_jit_function_new_local (gcc_jit_function *func,
 			    gcc_jit_type *type,
 			    const char *name);
 
+extern gcc_jit_lvalue *
+gcc_jit_function_new_temp (gcc_jit_function *func,
+			   gcc_jit_location *loc,
+			   gcc_jit_type *type);
+
+#define LIBGCCJIT_HAVE_gcc_jit_function_new_temp
+
 /**********************************************************************
  Statement-creation.
  **********************************************************************/
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 99aa5970be1..45a7f45bf43 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -289,3 +289,8 @@ LIBGCCJIT_ABI_27 {
   global:
     gcc_jit_context_new_sizeof;
 } LIBGCCJIT_ABI_26;
+
+LIBGCCJIT_ABI_28 {
+  global:
+    gcc_jit_function_new_temp;
+} LIBGCCJIT_ABI_27;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 14a0a321550..fa6b26a66b2 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -360,6 +360,9 @@
 #undef create_code
 #undef verify_code
 
+/* test-temp.c: This can't be in the testcases array as it
+   is target-specific.  */
+
 /* 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-temp.c b/gcc/testsuite/jit.dg/test-temp.c
new file mode 100644
index 00000000000..9b1ba9b0100
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-temp.c
@@ -0,0 +1,56 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+
+#include "libgccjit.h"
+
+#define TEST_COMPILING_TO_FILE
+#define OUTPUT_KIND      GCC_JIT_OUTPUT_KIND_ASSEMBLER
+#define OUTPUT_FILENAME  "output-of-test-test-temp.c.s"
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Let's try to inject the equivalent of:
+int
+func ()
+{
+   int temp = 10;
+   return temp;
+}
+   */
+  gcc_jit_type *int_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT);
+
+  gcc_jit_function *func =
+    gcc_jit_context_new_function (ctxt,
+				  NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  int_type,
+				  "func",
+				  0, NULL, 0);
+
+  gcc_jit_block *initial =
+    gcc_jit_function_new_block (func, "initial");
+
+  gcc_jit_lvalue *temp =
+    gcc_jit_function_new_temp (func, NULL, int_type);
+
+  gcc_jit_rvalue *ten =
+    gcc_jit_context_new_rvalue_from_int (ctxt, int_type, 10);
+  gcc_jit_block_add_assignment (initial, NULL, temp, ten);
+
+  gcc_jit_block_end_with_return(initial, NULL,
+    gcc_jit_lvalue_as_rvalue (temp));
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  CHECK_NON_NULL (result);
+}
+
+/* { dg-final { jit-verify-output-file-was-created "" } } */
+/* { dg-final { jit-verify-assembler-output-not "JITTMP" } } */
-- 
2.44.0


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

end of thread, other threads:[~2024-02-29 21:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-19 21:54 [PATCH] libgccjit: Add support for creating temporary variables Antoni Boucher
2024-01-24 14:54 ` David Malcolm
2024-02-29 21:11   ` Antoni Boucher

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