It seems we don't need to do the cleanup in i386-builtins.cc anymore, so
I removed it.
David: Is it possible that your recent fixes for the GC within libgccjit
also fixed the issue here?
Here's the updated patch and answers below.
(GitHub link if you find it easier for review:
https://github.com/antoyo/libgccjit/pull/5)
Thanks.
Le 2024-06-26 à 18 h 49, David Malcolm a écrit :
On Thu, 2023-11-23 at 17:17 -0500, Antoni Boucher wrote:
Hi.
I did split the patch and sent one for the bfloat16 support and
another
one for the vector support.
Here's the updated patch for the machine-dependent builtins.
Thanks for the patch; sorry about the long delay in reviewing it.
CCing Jan and Uros re the i386 part of that patch; for reference the
patch being discussed is here:
https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638027.html
From e025f95f4790ae861e709caf23cbc0723c1a3804 Mon Sep 17 00:00:00 2001
From: Antoni Boucher
Date: Mon, 23 Jan 2023 17:21:15 -0500
Subject: [PATCH] libgccjit: Add support for machine-dependent builtins
[...snip...]
diff --git a/gcc/config/i386/i386-builtins.cc b/gcc/config/i386/i386-builtins.cc
index 42fc3751676..5cc1d6f4d2e 100644
--- a/gcc/config/i386/i386-builtins.cc
+++ b/gcc/config/i386/i386-builtins.cc
@@ -225,6 +225,22 @@ static GTY(()) tree ix86_builtins[(int) IX86_BUILTIN_MAX];
struct builtin_isa ix86_builtins_isa[(int) IX86_BUILTIN_MAX];
+static void
+clear_builtin_types (void)
+{
+ for (int i = 0 ; i < IX86_BT_LAST_CPTR + 1 ; i++)
+ix86_builtin_type_tab[i] = NULL;
+
+ for (int i = 0 ; i < IX86_BUILTIN_MAX ; i++)
+ {
+ix86_builtins[i] = NULL;
+ix86_builtins_isa[i].set_and_not_built_p = true;
+ }
+
+ for (int i = 0 ; i < IX86_BT_LAST_ALIAS + 1 ; i++)
+ix86_builtin_func_type_tab[i] = NULL;
+}
+
tree get_ix86_builtin (enum ix86_builtins c)
{
return ix86_builtins[c];
@@ -1483,6 +1499,8 @@ ix86_init_builtins (void)
{
tree ftype, decl;
+ clear_builtin_types ();
+
ix86_init_builtin_types ();
/* Builtins to get CPU type and features. */
Please can one of the i386 maintainers check this?
(CCing Jan and Uros: this is for the case where the compiler code runs
multiple times in-process due to being linked into libgccjit.so. We
want to restore state within i386-builtins.cc to an initial state, and
ensure that no GC-managed objects persist from previous in-memory
compiles).
diff --git a/gcc/jit/docs/topics/compatibility.rst
b/gcc/jit/docs/topics/compatibility.rst
index ebede440ee4..764de23341e 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -378,3 +378,12 @@ alignment of a variable:
``LIBGCCJIT_ABI_25`` covers the addition of
:func:`gcc_jit_type_get_restrict`
+
+.. _LIBGCCJIT_ABI_26:
+
+``LIBGCCJIT_ABI_26``
+
+
+``LIBGCCJIT_ABI_26`` covers the addition of a function to get target builtins:
+
+ * :func:`gcc_jit_context_get_target_builtin_function`
diff --git a/gcc/jit/docs/topics/functions.rst
b/gcc/jit/docs/topics/functions.rst
index cf5cb716daf..e9b77fdb892 100644
--- a/gcc/jit/docs/topics/functions.rst
+++ b/gcc/jit/docs/topics/functions.rst
@@ -140,6 +140,25 @@ Functions
uses such a parameter will lead to an error being emitted within
the context.
+.. function:: gcc_jit_function *\
+ gcc_jit_context_get_target_builtin_function (gcc_jit_context
*ctxt,\
+const char *name)
+
+ Get the :type:`gcc_jit_function` for the built-in function with the
+ given name. For example:
Might be nice to add the "(sometimes called intrinsic functions)" text
you have in the header here.
Done
[...snip]
diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
index a729086bafb..3ca9702d429 100644
--- a/gcc/jit/dummy-frontend.cc
+++ b/gcc/jit/dummy-frontend.cc
[...]
@@ -29,8 +30,14 @@ along with GCC; see the file COPYING3. If not see
#include "options.h"
#include "stringpool.h"
#include "attribs.h"
+#include "jit-recording.h"
+#include "print-tree.h"
#include
+#include
+#include
+
+using namespace gcc::jit;
/* Attribute handling. */
@@ -86,6 +93,11 @@ static const struct attribute_spec::exclusions attr_const_pure_exclusions[] =
ATTR_EXCL (NULL, false, false, false)
};
+hash_map target_builtins{};
I was wondering if this needs a GTY marker, but I don't think it does:
presumably it's only used within jit_langhook_parse_file where no GC
can happen - unless jit_langhook_write_globals makes use of it?
It is also used in jit-playback.cc: does that mean it needs a GTY marker?
+std::unordered_map
target_function_types
+{};
+recording::context target_builtins_ctxt{NULL};
Please add a comment to target_builtins_ctxt saying what it's for. As
far as I can tell, it's for getting at recording::types from
tree_type_to_jit_type; we then use a new "copy" mechanism to c
