* [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
@ 2020-06-03 10:11 Andrea Corallo
2020-06-03 15:03 ` David Malcolm
0 siblings, 1 reply; 14+ messages in thread
From: Andrea Corallo @ 2020-06-03 10:11 UTC (permalink / raw)
To: gcc-patches, jit; +Cc: nd, David Malcolm
[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]
Hi all,
I'd like to submit this patch to extend libgccjit to allow for storing
binary blobs (equivalent to initialized char arrays).
The use-case is when libgccjit is used as a back-end for dynamic
programming languages. In this case is often necessary to store
serialized objects into the compilation unit as support to the
generated code.
The proposed entry point is the following:
gcc_jit_lvalue *
gcc_jit_context_new_blob (gcc_jit_context *ctxt,
gcc_jit_location *loc,
enum gcc_jit_global_kind kind,
const void *ptr,
size_t size,
const char *name);
This create a special kind of libgccjit global that will be initialized
with the memory content pointed by 'ptr'.
I've added a testcase and the regression is clean. I've also tested
it with the libgccjit based Emacs disabling the current workaround we have
for this.
Feedback is very welcome.
Andrea
gcc/jit/ChangeLog
2020-06-02 Andrea Corallo <andrea.corallo@arm.com>
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.
* docs/topics/expressions.rst (Binary Blobs): New section
documenting gcc_jit_context_new_blob.
* jit-common.h: Document class blob.
* jit-playback.c (playback::context::global_new_decl)
(playback::context::global_finalize_lvalue): New methods.
(playback::context::new_global): Make use of global_new_decl,
global_finalize_lvalue.
(playback::context::new_blob): New method.
* jit-playback.h (new_blob, global_new_decl): New method
declarations.
* jit-recording.c (recording::context::new_blob)
(recording::blob::replay_into)
(recording::global::write_qualifier_to_dump): New methods.
(recording::global::write_to_dump): Use write_qualifier_to_dump.
(recording::blob::write_to_dump): New method.
* jit-recording.h (class blob): New class.
(class global): Have m_kind and m_name as protected.
(class global): Remove FINAL qualifier to replay_into and
write_to_dump.
(class global): New method write_qualifier_to_dump decl.
(class context): New method new_blob decl.
* libgccjit++.h (context::new_blob): New method.
* libgccjit.c (gcc_jit_lvalue_as_rvalue): New function.
* libgccjit.h (gcc_jit_context_new_blob): New function
declaration.
(LIBGCCJIT_HAVE_gcc_jit_context_new_blob): New macro.
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
gcc/testsuite/ChangeLog
2020-06-02 Andrea Corallo <andrea.corallo@arm.com>
* jit.dg/all-non-failing-tests.h: Add test-blob.c.
* jit.dg/test-blob.c: New test.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-new-gcc_jit_context_new_blob-entry-point.patch --]
[-- Type: text/x-diff, Size: 20317 bytes --]
From b861fbbfbcbf18cc1b69dc4b7f91c596af40c8e4 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Sat, 30 May 2020 10:33:08 +0100
Subject: [PATCH] Add new gcc_jit_context_new_blob entry point
gcc/jit/ChangeLog
2020-06-02 Andrea Corallo <andrea.corallo@arm.com>
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.
* docs/topics/expressions.rst (Binary Blobs): New section
documenting gcc_jit_context_new_blob.
* jit-common.h: Document class blob.
* jit-playback.c (playback::context::global_new_decl)
(playback::context::global_finalize_lvalue): New methods.
(playback::context::new_global): Make use of global_new_decl,
global_finalize_lvalue.
(playback::context::new_blob): New method.
* jit-playback.h (new_blob, global_new_decl): New method
declarations.
* jit-recording.c (recording::context::new_blob)
(recording::blob::replay_into)
(recording::global::write_qualifier_to_dump): New methods.
(recording::global::write_to_dump): Use write_qualifier_to_dump.
(recording::blob::write_to_dump): New method.
* jit-recording.h (class blob): New class.
(class global): Have m_kind and m_name as protected.
(class global): Remove FINAL qualifier to replay_into and
write_to_dump.
(class global): New method write_qualifier_to_dump decl.
(class context): New method new_blob decl.
* libgccjit++.h (context::new_blob): New method.
* libgccjit.c (gcc_jit_lvalue_as_rvalue): New function.
* libgccjit.h (gcc_jit_context_new_blob): New function
declaration.
(LIBGCCJIT_HAVE_gcc_jit_context_new_blob): New macro.
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
gcc/testsuite/ChangeLog
2020-06-02 Andrea Corallo <andrea.corallo@arm.com>
* jit.dg/all-non-failing-tests.h: Add test-blob.c.
* jit.dg/test-blob.c: New test.
---
gcc/jit/docs/topics/compatibility.rst | 5 ++
gcc/jit/docs/topics/expressions.rst | 49 ++++++++++
gcc/jit/jit-common.h | 1 +
gcc/jit/jit-playback.c | 67 ++++++++++++--
gcc/jit/jit-playback.h | 16 ++++
gcc/jit/jit-recording.c | 95 +++++++++++++++++---
gcc/jit/jit-recording.h | 48 +++++++++-
gcc/jit/libgccjit++.h | 21 +++++
gcc/jit/libgccjit.c | 30 +++++++
gcc/jit/libgccjit.h | 17 ++++
gcc/jit/libgccjit.map | 7 +-
gcc/testsuite/jit.dg/all-non-failing-tests.h | 7 ++
gcc/testsuite/jit.dg/test-blob.c | 54 +++++++++++
13 files changed, 393 insertions(+), 24 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-blob.c
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index 0c0ce070d722..3d9684c73b4b 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -215,3 +215,8 @@ entrypoints:
* :func:`gcc_jit_version_minor`
* :func:`gcc_jit_version_patchlevel`
+
+``LIBGCCJIT_ABI_14``
+--------------------
+``LIBGCCJIT_ABI_14`` covers the addition of
+:func:`gcc_jit_context_new_blob`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index db2f2ca2e9c6..deb46594d80a 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -576,6 +576,55 @@ Global variables
referring to it. Analogous to using an "extern" global from a
header file.
+Binary Blobs
+****************
+
+.. function:: gcc_jit_context_new_blob (gcc_jit_context *ctxt,\
+ gcc_jit_location *loc,\
+ enum gcc_jit_global_kind kind,\
+ const void *ptr,\
+ size_t size,\
+ const char *name);
+
+ Add a new global variable equivalent to an initialized array of
+ char to the context.
+
+ The parameter ``name`` must be non-NULL. The call takes a copy of the
+ underlying string, so it is valid to pass in a pointer to an on-stack
+ buffer.
+
+ Similarly the parameter ``ptr`` must be non-NULL. The call copies
+ the memory content pointed by ``ptr`` for ``size`` bytes. This
+ content will be stores in the compilation unit and used as
+ initialization value of the array.
+
+ .. c:macro:: GCC_JIT_GLOBAL_EXPORTED
+
+ Global is defined by the client code and is visible
+ by name outside of this JIT context via
+ :c:func:`gcc_jit_result_get_global` (and this value is required for
+ the global to be accessible via that entrypoint).
+
+ .. c:macro:: GCC_JIT_GLOBAL_INTERNAL
+
+ Global is defined by the client code, but is invisible
+ outside of it. Analogous to a "static" global within a .c file.
+ Specifically, the variable will only be visible within this
+ context and within child contexts.
+
+ .. c:macro:: GCC_JIT_GLOBAL_IMPORTED
+
+ Global is not defined by the client code; we're merely
+ referring to it. Analogous to using an "extern" global from a
+ header file.
+
+ This entrypoint was added in :ref:`LIBGCCJIT_ABI_14`; you can test for
+ its presence using
+
+ .. code-block:: c
+
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_blob
+
Working with pointers, structs and unions
-----------------------------------------
diff --git a/gcc/jit/jit-common.h b/gcc/jit/jit-common.h
index 4570bd2d717f..7a1578d10e89 100644
--- a/gcc/jit/jit-common.h
+++ b/gcc/jit/jit-common.h
@@ -127,6 +127,7 @@ namespace recording {
class lvalue;
class local;
class global;
+ class blob;
class param;
class base_call;
class function_pointer;
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index 074434a9f6b2..1cc674a8ddd9 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -111,6 +111,14 @@ public:
type *type,
const char *name);
+ lvalue *
+ new_blob (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const void *ptr,
+ size_t size,
+ const char *name);
+
template <typename HOST_TYPE>
rvalue *
new_rvalue_from_const (type *type,
@@ -266,6 +274,14 @@ private:
const char * get_path_s_file () const;
const char * get_path_so_file () const;
+ tree
+ global_new_decl (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name);
+ lvalue *
+ global_finalize_lvalue (tree inner);
+
private:
/* Functions for implementing "compile". */
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index d2c8bb4c154d..8cd986942bc7 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -506,14 +506,14 @@ new_function (location *loc,
return func;
}
-/* Construct a playback::lvalue instance (wrapping a tree). */
+/* In use by new_global and new_blob. */
-playback::lvalue *
+tree
playback::context::
-new_global (location *loc,
- enum gcc_jit_global_kind kind,
- type *type,
- const char *name)
+global_new_decl (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name)
{
gcc_assert (type);
gcc_assert (name);
@@ -543,6 +543,15 @@ new_global (location *loc,
if (loc)
set_tree_location (inner, loc);
+ return inner;
+}
+
+/* In use by new_global and new_blob. */
+
+playback::lvalue *
+playback::context::
+global_finalize_lvalue (tree inner)
+{
varpool_node::get_create (inner);
varpool_node::finalize_decl (inner);
@@ -552,6 +561,52 @@ new_global (location *loc,
return new lvalue (this, inner);
}
+/* Construct a playback::lvalue instance (wrapping a tree). */
+
+playback::lvalue *
+playback::context::
+new_global (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name)
+{
+ tree inner = global_new_decl (loc, kind, type, name);
+
+ return global_finalize_lvalue (inner);
+}
+
+playback::lvalue *
+playback::context::
+new_blob (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const void *ptr,
+ size_t size,
+ const char *name)
+{
+ tree inner = global_new_decl (loc, kind, type, name);
+
+ static vec<constructor_elt, va_gc> *constructor_elements;
+ const char *p = (const char *)ptr;
+ for (size_t i = 0; i < size; i++)
+ {
+ /* Compare with 'output_init_element' c-typeck.c:9691. */
+ constructor_elt celt =
+ { build_int_cst (long_unsigned_type_node, i),
+ build_int_cst (char_type_node, p[i]) };
+ vec_safe_push (constructor_elements, celt);
+ }
+ /* Compare with 'pop_init_level' c-typeck.c:8780. */
+ tree ctor = build_constructor (type->as_tree (), constructor_elements);
+ constructor_elements = NULL;
+
+ /* Compare with 'store_init_value' c-typeck.c:7555. */
+ DECL_INITIAL (inner) = ctor;
+
+ return global_finalize_lvalue (inner);
+}
+
+
/* Implementation of the various
gcc::jit::playback::context::new_rvalue_from_const <HOST_TYPE>
methods.
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 726b9c4b8371..5e2762ff8785 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -204,6 +204,13 @@ public:
rvalue *max_value,
block *block);
+ lvalue *
+ new_blob (location *loc,
+ enum gcc_jit_global_kind kind,
+ const void *ptr,
+ size_t size,
+ const char *name);
+
void
set_str_option (enum gcc_jit_str_option opt,
const char *value);
@@ -1329,11 +1336,14 @@ public:
m_name (name)
{}
- void replay_into (replayer *) FINAL OVERRIDE;
+ void replay_into (replayer *) OVERRIDE;
void visit_children (rvalue_visitor *) FINAL OVERRIDE {}
- void write_to_dump (dump &d) FINAL OVERRIDE;
+ void write_to_dump (dump &d) OVERRIDE;
+
+protected:
+ void write_qualifier_to_dump (dump &d);
private:
string * make_debug_string () FINAL OVERRIDE { return m_name; }
@@ -1343,11 +1353,43 @@ private:
return PRECEDENCE_PRIMARY;
}
-private:
+protected:
enum gcc_jit_global_kind m_kind;
string *m_name;
};
+class blob : public global
+{
+public:
+ blob (context *ctxt,
+ location *loc,
+ enum gcc_jit_global_kind kind,
+ void *ptr,
+ size_t size,
+ string *name)
+ : global (ctxt, loc, kind,
+ ctxt->new_array_type (loc,
+ ctxt->get_type (GCC_JIT_TYPE_CHAR),
+ size),
+ name),
+ m_ptr (ptr),
+ m_size (size)
+ {}
+
+ ~blob ()
+ {
+ free (m_ptr);
+ }
+
+ void replay_into (replayer *) FINAL OVERRIDE;
+
+ void write_to_dump (dump &d) FINAL OVERRIDE;
+
+private:
+ void *m_ptr;
+ size_t m_size;
+};
+
template <typename HOST_TYPE>
class memento_of_new_rvalue_from_const : public rvalue
{
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index b73cd76a0a02..b23ebdc57d2c 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -1229,6 +1229,29 @@ recording::context::new_case (recording::rvalue *min_value,
return result;
}
+/* Create a recording::blob instance and add it to this context's list
+ of mementos.
+
+ Implements the post-error-checking part of
+ gcc_jit_context_new_blob. */
+
+recording::lvalue *
+recording::context::new_blob (recording::location *loc,
+ enum gcc_jit_global_kind kind,
+ const void *ptr,
+ size_t size,
+ const char *name)
+{
+ void *data = xmalloc (size);
+ recording::blob *result =
+ new recording::blob (this, loc, kind, memcpy (data, ptr, size), size,
+ new_string (name));
+ record (result);
+ m_globals.safe_push (result);
+
+ return result;
+}
+
/* Set the given string option for this context, or add an error if
it's not recognized.
@@ -4399,6 +4422,38 @@ recording::global::replay_into (replayer *r)
playback_string (m_name)));
}
+void
+recording::blob::replay_into (replayer *r)
+{
+ set_playback_obj (r->new_blob (playback_location (r, m_loc),
+ m_kind,
+ m_type->playback_type (),
+ m_ptr,
+ m_size,
+ playback_string (m_name)));
+}
+
+void
+recording::global::write_qualifier_to_dump (dump &d)
+{
+ switch (m_kind)
+ {
+ default:
+ gcc_unreachable ();
+
+ case GCC_JIT_GLOBAL_EXPORTED:
+ break;
+
+ case GCC_JIT_GLOBAL_INTERNAL:
+ d.write ("static ");
+ break;
+
+ case GCC_JIT_GLOBAL_IMPORTED:
+ d.write ("extern ");
+ break;
+ }
+}
+
/* Override the default implementation of
recording::memento::write_to_dump for globals.
This will be of the form:
@@ -4424,25 +4479,37 @@ recording::global::write_to_dump (dump &d)
if (d.update_locations ())
m_loc = d.make_location ();
- switch (m_kind)
- {
- default:
- gcc_unreachable ();
+ write_qualifier_to_dump (d);
- case GCC_JIT_GLOBAL_EXPORTED:
- break;
+ d.write ("%s %s;\n",
+ m_type->get_debug_string (),
+ get_debug_string ());
+}
- case GCC_JIT_GLOBAL_INTERNAL:
- d.write ("static ");
- break;
- case GCC_JIT_GLOBAL_IMPORTED:
- d.write ("extern ");
- break;
- }
- d.write ("%s %s;\n",
+/* Override the default implementation of
+ recording::memento::write_to_dump for blobs. */
+
+void
+recording::blob::write_to_dump (dump &d)
+{
+ if (d.update_locations ())
+ m_loc = d.make_location ();
+
+ write_qualifier_to_dump (d);
+
+ d.write ("%s %s =\n { ",
m_type->get_debug_string (),
get_debug_string ());
+
+ const char *p = (const char *)m_ptr;
+ for (size_t i = 0; i < m_size; i++)
+ {
+ d.write ("0x%x, ", p[i]);
+ if (i && !(i % 64))
+ d.write ("\n ");
+ }
+ d.write ("};\n");
}
/* A table of enum gcc_jit_global_kind values expressed in string
diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
index 69e67766640c..bb663e9aca1f 100644
--- a/gcc/jit/libgccjit++.h
+++ b/gcc/jit/libgccjit++.h
@@ -182,6 +182,12 @@ namespace gccjit
const std::string &name,
location loc = location ());
+ lvalue new_blob (enum gcc_jit_global_kind kind,
+ const void *ptr,
+ size_t size,
+ const std::string &name,
+ location loc = location ());
+
rvalue new_rvalue (type numeric_type,
int value) const;
rvalue new_rvalue (type numeric_type,
@@ -860,6 +866,21 @@ context::new_global (enum gcc_jit_global_kind kind,
name.c_str ()));
}
+inline lvalue
+context::new_blob (enum gcc_jit_global_kind kind,
+ const void *ptr,
+ size_t size,
+ const std::string &name,
+ location loc)
+{
+ return lvalue (gcc_jit_context_new_blob (m_inner_ctxt,
+ loc.get_inner_location (),
+ kind,
+ ptr,
+ size,
+ name.c_str ()));
+}
+
inline rvalue
context::new_rvalue (type numeric_type,
int value) const
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 1c5a12e9c015..a66be3568fa0 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -788,6 +788,23 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
gcc_jit_type *type,
const char *name);
+#define LIBGCCJIT_HAVE_gcc_jit_context_new_blob
+
+/* Create a binary blob.
+
+ This API entrypoint was added in LIBGCCJIT_ABI_14; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_blob
+*/
+
+extern gcc_jit_lvalue *
+gcc_jit_context_new_blob (gcc_jit_context *ctxt,
+ gcc_jit_location *loc,
+ enum gcc_jit_global_kind kind,
+ const void *ptr,
+ size_t size,
+ const char *name);
+
/* Upcasting. */
extern gcc_jit_object *
gcc_jit_lvalue_as_object (gcc_jit_lvalue *lvalue);
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index a29e9885e59b..a504d568b388 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -1097,6 +1097,36 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
return (gcc_jit_lvalue *)ctxt->new_global (loc, kind, type, name);
}
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::context::new_blob method, in
+ jit-recording.c. */
+
+gcc_jit_lvalue *
+gcc_jit_context_new_blob (gcc_jit_context *ctxt,
+ gcc_jit_location *loc,
+ enum gcc_jit_global_kind kind,
+ const void *ptr,
+ size_t size,
+ const char *name)
+{
+ RETURN_NULL_IF_FAIL (ctxt, NULL, loc, "NULL context");
+ JIT_LOG_FUNC (ctxt->get_logger ());
+ /* LOC can be NULL. */
+ RETURN_NULL_IF_FAIL_PRINTF1 (
+ ((kind >= GCC_JIT_GLOBAL_EXPORTED)
+ && (kind <= GCC_JIT_GLOBAL_IMPORTED)),
+ ctxt, loc,
+ "unrecognized value for enum gcc_jit_global_kind: %i",
+ kind);
+ RETURN_NULL_IF_FAIL (name, ctxt, loc, "NULL ptr");
+ /* SIZE can be zero. */
+ RETURN_NULL_IF_FAIL (name, ctxt, loc, "NULL name");
+
+ return (gcc_jit_lvalue *)ctxt->new_blob (loc, kind, ptr, size, name);
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, this calls the trivial
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 6137dd4b4b03..40c3605310d8 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -186,4 +186,9 @@ LIBGCCJIT_ABI_13 {
gcc_jit_version_major;
gcc_jit_version_minor;
gcc_jit_version_patchlevel;
-} LIBGCCJIT_ABI_12;
\ No newline at end of file
+} LIBGCCJIT_ABI_12;
+
+LIBGCCJIT_ABI_14 {
+ global:
+ gcc_jit_context_new_blob;
+} LIBGCCJIT_ABI_13;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index ad469dad6994..7eaf373e6ed8 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -67,6 +67,13 @@
#undef create_code
#undef verify_code
+/* test-blob.c */
+#define create_code create_code_blob
+#define verify_code verify_code_blob
+#include "test-blob.c"
+#undef create_code
+#undef verify_code
+
/* test-calling-external-function.c */
#define create_code create_code_calling_external_function
#define verify_code verify_code_calling_external_function
diff --git a/gcc/testsuite/jit.dg/test-blob.c b/gcc/testsuite/jit.dg/test-blob.c
new file mode 100644
index 000000000000..d344975dd5a7
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-blob.c
@@ -0,0 +1,54 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+#define BIG_BLOB_SIZE (1 << 12) /* 4KB. */
+
+static char test_blob1[] = { 0xc, 0xa, 0xf, 0xf, 0xe };
+static char test_blob2[] = { 0x3, 0x2, 0x1, 0x0, 0x1, 0x2, 0x3 };
+static char test_blob3[BIG_BLOB_SIZE];
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+
+ char bin_blob1[] = { 0xc, 0xa, 0xf, 0xf, 0xe };
+ char bin_blob2[] = { 0x3, 0x2, 0x1, 0x0, 0x1, 0x2, 0x3 };
+ char bin_blob3[4096]...
+ */
+
+ gcc_jit_context_new_blob (ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, test_blob1,
+ sizeof (test_blob1), "bin_blob1");
+
+ gcc_jit_context_new_blob (ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, test_blob2,
+ sizeof (test_blob2), "bin_blob2");
+
+ for (size_t i = 0; i < BIG_BLOB_SIZE; i++)
+ test_blob3[i] = i * i + i;
+ gcc_jit_context_new_blob (ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED, test_blob3,
+ sizeof (test_blob3), "bin_blob3");
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ CHECK_NON_NULL (result);
+ void *blob = gcc_jit_result_get_global (result, "bin_blob1");
+ CHECK_NON_NULL (blob);
+ CHECK_VALUE (memcmp (test_blob1, blob, sizeof (test_blob1)), 0);
+
+ blob = gcc_jit_result_get_global (result, "bin_blob2");
+ CHECK_NON_NULL (blob);
+ CHECK_VALUE (memcmp (test_blob2, blob, sizeof (test_blob2)), 0);
+
+ blob = gcc_jit_result_get_global (result, "bin_blob3");
+ CHECK_NON_NULL (blob);
+ CHECK_VALUE (memcmp (test_blob3, blob, sizeof (test_blob3)), 0);
+
+}
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-06-03 10:11 [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point Andrea Corallo
@ 2020-06-03 15:03 ` David Malcolm
2020-06-03 16:36 ` Andrea Corallo
0 siblings, 1 reply; 14+ messages in thread
From: David Malcolm @ 2020-06-03 15:03 UTC (permalink / raw)
To: Andrea Corallo, gcc-patches, jit; +Cc: nd
On Wed, 2020-06-03 at 12:11 +0200, Andrea Corallo wrote:
> Hi all,
>
> I'd like to submit this patch to extend libgccjit to allow for
> storing
> binary blobs (equivalent to initialized char arrays).
>
> The use-case is when libgccjit is used as a back-end for dynamic
> programming languages. In this case is often necessary to store
> serialized objects into the compilation unit as support to the
> generated code.
>
> The proposed entry point is the following:
>
> gcc_jit_lvalue *
> gcc_jit_context_new_blob (gcc_jit_context *ctxt,
> gcc_jit_location *loc,
> enum gcc_jit_global_kind kind,
> const void *ptr,
> size_t size,
> const char *name);
>
> This create a special kind of libgccjit global that will be
> initialized
> with the memory content pointed by 'ptr'.
>
> I've added a testcase and the regression is clean. I've also tested
> it with the libgccjit based Emacs disabling the current workaround we
> have
> for this.
>
> Feedback is very welcome.
Thanks for the patch.
Is this entrypoint only needed for the ahead-of-time use case? If the
client code is instead going to do an in-memory compilation, then I
believe it can simply build the buffer of data in memory and expose it
to the jitted code via gcc_jit_context_new_rvalue_from_ptr.
It occurred to me that the entrypoint is combining two things:
- creating a global char[]
- creating an initializer for that global
which got me wondering if we should instead have a way to add
initializers for globals.
My first thought was something like:
gcc_jit_context_new_global_with_initializer
which would be like gcc_jit_context_new_global but would have an
additional gcc_jit_rvalue *init_value param?
The global would have to be of kind GCC_JIT_GLOBAL_EXPORTED or
GCC_JIT_GLOBAL_INTERNAL, not GCC_JIT_GLOBAL_IMPORTED.
Alternatively, maybe it would be better to have
gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
gcc_jit_rvalue *init_val);
to make the API more flexible.
But even if we had this, we'd still need some way to create the rvalue
for that initial value. Also, maybe there ought to be a distinction
between rvalues that can vary at runtime vs those that can be computed
at compile-time (and are thus suitable for use in static
initialization).
I suspect you may have gone through the same thought process and come
up with a simpler approach. (I'm mostly just "thinking out loud"
here, sorry if it isn't very coherent).
Should it be a "const char[]" rather than just a "char[]"?
One specific nit about the patch: in compatibility.rst, there should be
a:
.. _LIBGCCJIT_ABI_14:
label before the heading.
Hope this is constructive
Dave
> Andrea
>
> gcc/jit/ChangeLog
>
> 2020-06-02 Andrea Corallo <andrea.corallo@arm.com>
>
> * docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI
> tag.
>
> * docs/topics/expressions.rst (Binary Blobs): New section
> documenting gcc_jit_context_new_blob.
>
> * jit-common.h: Document class blob.
>
> * jit-playback.c (playback::context::global_new_decl)
> (playback::context::global_finalize_lvalue): New methods.
> (playback::context::new_global): Make use of global_new_decl,
> global_finalize_lvalue.
> (playback::context::new_blob): New method.
>
> * jit-playback.h (new_blob, global_new_decl): New method
> declarations.
>
> * jit-recording.c (recording::context::new_blob)
> (recording::blob::replay_into)
> (recording::global::write_qualifier_to_dump): New methods.
> (recording::global::write_to_dump): Use
> write_qualifier_to_dump.
> (recording::blob::write_to_dump): New method.
>
> * jit-recording.h (class blob): New class.
> (class global): Have m_kind and m_name as protected.
> (class global): Remove FINAL qualifier to replay_into and
> write_to_dump.
> (class global): New method write_qualifier_to_dump decl.
> (class context): New method new_blob decl.
>
> * libgccjit++.h (context::new_blob): New method.
>
> * libgccjit.c (gcc_jit_lvalue_as_rvalue): New function.
>
> * libgccjit.h (gcc_jit_context_new_blob): New function
> declaration.
> (LIBGCCJIT_HAVE_gcc_jit_context_new_blob): New macro.
>
> * libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
>
> gcc/testsuite/ChangeLog
>
> 2020-06-02 Andrea Corallo <andrea.corallo@arm.com>
>
> * jit.dg/all-non-failing-tests.h: Add test-blob.c.
>
> * jit.dg/test-blob.c: New test.
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-06-03 15:03 ` David Malcolm
@ 2020-06-03 16:36 ` Andrea Corallo
2020-07-01 16:14 ` Andrea Corallo
0 siblings, 1 reply; 14+ messages in thread
From: Andrea Corallo @ 2020-06-03 16:36 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches, jit, nd
David Malcolm <dmalcolm@redhat.com> writes:
> Thanks for the patch.
>
> Is this entrypoint only needed for the ahead-of-time use case? If the
> client code is instead going to do an in-memory compilation, then I
> believe it can simply build the buffer of data in memory and expose it
> to the jitted code via gcc_jit_context_new_rvalue_from_ptr.
Hi Dave,
correct, if you can leak pointers into the generated code this entry
point is likely to be unnecessary.
> It occurred to me that the entrypoint is combining two things:
> - creating a global char[]
> - creating an initializer for that global
>
> which got me wondering if we should instead have a way to add
> initializers for globals.
>
> My first thought was something like:
>
> gcc_jit_context_new_global_with_initializer
>
> which would be like gcc_jit_context_new_global but would have an
> additional gcc_jit_rvalue *init_value param?
> The global would have to be of kind GCC_JIT_GLOBAL_EXPORTED or
> GCC_JIT_GLOBAL_INTERNAL, not GCC_JIT_GLOBAL_IMPORTED.
>
> Alternatively, maybe it would be better to have
>
> gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> gcc_jit_rvalue *init_val);
>
> to make the API more flexible.
>
> But even if we had this, we'd still need some way to create the rvalue
> for that initial value. Also, maybe there ought to be a distinction
> between rvalues that can vary at runtime vs those that can be computed
> at compile-time (and are thus suitable for use in static
> initialization).
>
> I suspect you may have gone through the same thought process and come
> up with a simpler approach. (I'm mostly just "thinking out loud"
> here, sorry if it isn't very coherent).
Yes I had kind of similar thoughs.
Ideally would be good to have a generic solution, the complication is
that as you mentioned not every rvalue is suitable for initializing
every global, but rather the opposite. My fear was that the space to be
covered would be non trivial for a robust solution in this case.
Also I believe we currently have no way to express in libgccjit rvalues
an array with some content, so how to use this as initializer? Perhaps
also we should have a new type gcc_jit_initializer?
On the other hand I thought that for simple things like integers the
problem is tipically already solved with an assignment in some init code
(infact I think so far nobody complained) while the real standing
limitation is for blobs (perhaps I'm wrong). And in the end if you can
stuff some data in, you can use it later for any scope.
Another "hybrid" solution would be to have specific entry point for each
type of the subset we want to allow for static initialization. This way
we still control the creation of the initializer internally so it's
safe. In this view this blob entry point would be just one of these
(probably with a different name like
'gcc_jit_context_new_glob_init_char_array').
> Should it be a "const char[]" rather than just a "char[]"?
Good question, I believe depends on the usecase so I left out the const.
Perhaps should be a parameter of the entry point.
> One specific nit about the patch: in compatibility.rst, there should be
> a:
>
> .. _LIBGCCJIT_ABI_14:
>
> label before the heading.
Ops
> Hope this is constructive
> Dave
Sure it is.
Thanks
Andrea
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-06-03 16:36 ` Andrea Corallo
@ 2020-07-01 16:14 ` Andrea Corallo
2020-07-14 10:00 ` Andrea Corallo
2020-07-24 22:05 ` David Malcolm
0 siblings, 2 replies; 14+ messages in thread
From: Andrea Corallo @ 2020-07-01 16:14 UTC (permalink / raw)
To: David Malcolm; +Cc: jit, nd, gcc-patches
Andrea Corallo <andrea.corallo@arm.com> writes:
>> It occurred to me that the entrypoint is combining two things:
>> - creating a global char[]
>> - creating an initializer for that global
>>
>> which got me wondering if we should instead have a way to add
>> initializers for globals.
>>
>> My first thought was something like:
>>
>> gcc_jit_context_new_global_with_initializer
>>
>> which would be like gcc_jit_context_new_global but would have an
>> additional gcc_jit_rvalue *init_value param?
>> The global would have to be of kind GCC_JIT_GLOBAL_EXPORTED or
>> GCC_JIT_GLOBAL_INTERNAL, not GCC_JIT_GLOBAL_IMPORTED.
>>
>> Alternatively, maybe it would be better to have
>>
>> gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
>> gcc_jit_rvalue *init_val);
>>
>> to make the API more flexible.
>>
>> But even if we had this, we'd still need some way to create the rvalue
>> for that initial value. Also, maybe there ought to be a distinction
>> between rvalues that can vary at runtime vs those that can be computed
>> at compile-time (and are thus suitable for use in static
>> initialization).
>>
>> I suspect you may have gone through the same thought process and come
>> up with a simpler approach. (I'm mostly just "thinking out loud"
>> here, sorry if it isn't very coherent).
>
> Yes I had kind of similar thoughs.
>
> Ideally would be good to have a generic solution, the complication is
> that as you mentioned not every rvalue is suitable for initializing
> every global, but rather the opposite. My fear was that the space to be
> covered would be non trivial for a robust solution in this case.
>
> Also I believe we currently have no way to express in libgccjit rvalues
> an array with some content, so how to use this as initializer? Perhaps
> also we should have a new type gcc_jit_initializer?
>
> On the other hand I thought that for simple things like integers the
> problem is tipically already solved with an assignment in some init code
> (infact I think so far nobody complained) while the real standing
> limitation is for blobs (perhaps I'm wrong). And in the end if you can
> stuff some data in, you can use it later for any scope.
>
> Another "hybrid" solution would be to have specific entry point for each
> type of the subset we want to allow for static initialization. This way
> we still control the creation of the initializer internally so it's
> safe. In this view this blob entry point would be just one of these
> (probably with a different name like
> 'gcc_jit_context_new_glob_init_char_array').
>
Hi Dave,
wanted to ask if you formed an opinion about the patch and/or more in
general the problem of static initialize data.
Thanks
Andrea
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-07-01 16:14 ` Andrea Corallo
@ 2020-07-14 10:00 ` Andrea Corallo
2020-07-24 22:05 ` David Malcolm
1 sibling, 0 replies; 14+ messages in thread
From: Andrea Corallo @ 2020-07-14 10:00 UTC (permalink / raw)
To: David Malcolm; +Cc: jit, nd, gcc-patches
Andrea Corallo <andrea.corallo@arm.com> writes:
> Andrea Corallo <andrea.corallo@arm.com> writes:
>
>>> It occurred to me that the entrypoint is combining two things:
>>> - creating a global char[]
>>> - creating an initializer for that global
>>>
>>> which got me wondering if we should instead have a way to add
>>> initializers for globals.
>>>
>>> My first thought was something like:
>>>
>>> gcc_jit_context_new_global_with_initializer
>>>
>>> which would be like gcc_jit_context_new_global but would have an
>>> additional gcc_jit_rvalue *init_value param?
>>> The global would have to be of kind GCC_JIT_GLOBAL_EXPORTED or
>>> GCC_JIT_GLOBAL_INTERNAL, not GCC_JIT_GLOBAL_IMPORTED.
>>>
>>> Alternatively, maybe it would be better to have
>>>
>>> gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
>>> gcc_jit_rvalue *init_val);
>>>
>>> to make the API more flexible.
>>>
>>> But even if we had this, we'd still need some way to create the rvalue
>>> for that initial value. Also, maybe there ought to be a distinction
>>> between rvalues that can vary at runtime vs those that can be computed
>>> at compile-time (and are thus suitable for use in static
>>> initialization).
>>>
>>> I suspect you may have gone through the same thought process and come
>>> up with a simpler approach. (I'm mostly just "thinking out loud"
>>> here, sorry if it isn't very coherent).
>>
>> Yes I had kind of similar thoughs.
>>
>> Ideally would be good to have a generic solution, the complication is
>> that as you mentioned not every rvalue is suitable for initializing
>> every global, but rather the opposite. My fear was that the space to be
>> covered would be non trivial for a robust solution in this case.
>>
>> Also I believe we currently have no way to express in libgccjit rvalues
>> an array with some content, so how to use this as initializer? Perhaps
>> also we should have a new type gcc_jit_initializer?
>>
>> On the other hand I thought that for simple things like integers the
>> problem is tipically already solved with an assignment in some init code
>> (infact I think so far nobody complained) while the real standing
>> limitation is for blobs (perhaps I'm wrong). And in the end if you can
>> stuff some data in, you can use it later for any scope.
>>
>> Another "hybrid" solution would be to have specific entry point for each
>> type of the subset we want to allow for static initialization. This way
>> we still control the creation of the initializer internally so it's
>> safe. In this view this blob entry point would be just one of these
>> (probably with a different name like
>> 'gcc_jit_context_new_glob_init_char_array').
>>
>
> Hi Dave,
>
> wanted to ask if you formed an opinion about the patch and/or more in
> general the problem of static initialize data.
>
> Thanks
>
> Andrea
Ping
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-07-01 16:14 ` Andrea Corallo
2020-07-14 10:00 ` Andrea Corallo
@ 2020-07-24 22:05 ` David Malcolm
2020-07-24 22:12 ` David Malcolm
1 sibling, 1 reply; 14+ messages in thread
From: David Malcolm @ 2020-07-24 22:05 UTC (permalink / raw)
To: Andrea Corallo; +Cc: jit, nd, gcc-patches
On Wed, 2020-07-01 at 18:14 +0200, Andrea Corallo wrote:
> Andrea Corallo <andrea.corallo@arm.com> writes:
>
> > > It occurred to me that the entrypoint is combining two things:
> > > - creating a global char[]
> > > - creating an initializer for that global
> > >
> > > which got me wondering if we should instead have a way to add
> > > initializers for globals.
> > >
> > > My first thought was something like:
> > >
> > > gcc_jit_context_new_global_with_initializer
> > >
> > > which would be like gcc_jit_context_new_global but would have an
> > > additional gcc_jit_rvalue *init_value param?
> > > The global would have to be of kind GCC_JIT_GLOBAL_EXPORTED or
> > > GCC_JIT_GLOBAL_INTERNAL, not GCC_JIT_GLOBAL_IMPORTED.
> > >
> > > Alternatively, maybe it would be better to have
> > >
> > > gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> > > gcc_jit_rvalue *init_val);
> > >
> > > to make the API more flexible.
> > >
> > > But even if we had this, we'd still need some way to create the
> > > rvalue
> > > for that initial value. Also, maybe there ought to be a
> > > distinction
> > > between rvalues that can vary at runtime vs those that can be
> > > computed
> > > at compile-time (and are thus suitable for use in static
> > > initialization).
> > >
> > > I suspect you may have gone through the same thought process and
> > > come
> > > up with a simpler approach. (I'm mostly just "thinking out
> > > loud"
> > > here, sorry if it isn't very coherent).
> >
> > Yes I had kind of similar thoughs.
> >
> > Ideally would be good to have a generic solution, the complication
> > is
> > that as you mentioned not every rvalue is suitable for initializing
> > every global, but rather the opposite. My fear was that the space
> > to be
> > covered would be non trivial for a robust solution in this case.
> >
> > Also I believe we currently have no way to express in libgccjit
> > rvalues
> > an array with some content, so how to use this as
> > initializer? Perhaps
> > also we should have a new type gcc_jit_initializer?
> >
> > On the other hand I thought that for simple things like integers
> > the
> > problem is tipically already solved with an assignment in some init
> > code
> > (infact I think so far nobody complained) while the real standing
> > limitation is for blobs (perhaps I'm wrong). And in the end if you
> > can
> > stuff some data in, you can use it later for any scope.
> >
> > Another "hybrid" solution would be to have specific entry point for
> > each
> > type of the subset we want to allow for static
> > initialization. This way
> > we still control the creation of the initializer internally so it's
> > safe. In this view this blob entry point would be just one of
> > these
> > (probably with a different name like
> > 'gcc_jit_context_new_glob_init_char_array').
> >
>
> Hi Dave,
>
> wanted to ask if you formed an opinion about the patch and/or more in
> general the problem of static initialize data.
Sorry for the delay in responding.
Another idea that occurred to me is, as before, to generalize the
entrypoint to cover creating globals with arbitrary types and to
support initializers (either with a new
gcc_jit_context_new_global_with_initializer
or a new:
gcc_jit_global_set_initializer
as above, but instead of passing in a gcc_jit_rvalue *init_val, to pass
in a
const void *initializer
size_t num_bytes
pair pointing to the initializer buffer, with the behavior that these
are the bytes that will be used for the initial value of the global,
whatever that means for the given type.
Something like either:
extern gcc_jit_lvalue *
gcc_jit_context_new_global_with_initializer (gcc_jit_context *ctxt,
gcc_jit_location *loc,
enum gcc_jit_global_kind kind,
gcc_jit_type *type,
const char *name,
const void *initializer,
size_t num_bytes);
or:
extern void
gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
const void *initializer,
size_t num_bytes);
or somesuch. The former is similar to the proposed new entrypoint from
your patch, but gains a type argument (and is modeled more closely on
the existing entrypoint for creating globals).
I haven't thought this through in detail, and I'm not sure exactly how
it would work for arbitrary types, but I thought it worth sharing.
(For example I can think of nasty issues if we ever want to support
cross-compilation, e.g. where sizeof types or endianness differs
between host and target).
Maybe we can make it work initially for char[] types, rejecting others,
assuming that that's the most pressing need in your work, and we can
add support for other types internally as followups without needing to
add new entrypoints?
Thoughts?
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-07-24 22:05 ` David Malcolm
@ 2020-07-24 22:12 ` David Malcolm
2020-08-03 8:07 ` Andrea Corallo
0 siblings, 1 reply; 14+ messages in thread
From: David Malcolm @ 2020-07-24 22:12 UTC (permalink / raw)
To: Andrea Corallo; +Cc: jit, nd, gcc-patches
On Fri, 2020-07-24 at 18:05 -0400, David Malcolm via Gcc-patches wrote:
[...]
> I haven't thought this through in detail, and I'm not sure exactly
> how
> it would work for arbitrary types, but I thought it worth sharing.
> (For example I can think of nasty issues if we ever want to support
> cross-compilation, e.g. where sizeof types or endianness differs
> between host and target).
...which is an argument in favor of retaining the name "blob", perhaps
as the name of the argument in the header file e.g.:
extern void
gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
const void *blob,
size_t num_bytes);
as a subtle hint to the user that they need to be wary about binary
layouts ("here be dragons").
[...]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-07-24 22:12 ` David Malcolm
@ 2020-08-03 8:07 ` Andrea Corallo
2020-08-06 19:53 ` David Malcolm
0 siblings, 1 reply; 14+ messages in thread
From: Andrea Corallo @ 2020-08-03 8:07 UTC (permalink / raw)
To: David Malcolm; +Cc: jit, nd, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 3245 bytes --]
David Malcolm <dmalcolm@redhat.com> writes:
> On Fri, 2020-07-24 at 18:05 -0400, David Malcolm via Gcc-patches wrote:
>
> [...]
>
>> I haven't thought this through in detail, and I'm not sure exactly
>> how
>> it would work for arbitrary types, but I thought it worth sharing.
>> (For example I can think of nasty issues if we ever want to support
>> cross-compilation, e.g. where sizeof types or endianness differs
>> between host and target).
>
> ...which is an argument in favor of retaining the name "blob", perhaps
> as the name of the argument in the header file e.g.:
>
> extern void
> gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> const void *blob,
> size_t num_bytes);
>
>
> as a subtle hint to the user that they need to be wary about binary
> layouts ("here be dragons").
>
> [...]
Hi Dave & all,
following up this is my take on the implementation of:
gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
const void *blob,
size_t num_bytes);
'global' must be an array but in the seek of generality it now supports
all the various integral types and is not limited to char[].
As you anticipated the implementation I came up is currently not safe
for cross-compilation, not sure is requirement tho.
make check-jit is clean
Feedback very welcome
Thanks!
Andrea
gcc/jit/ChangeLog
2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.
* docs/topics/expressions.rst (gcc_jit_global_set_initializer):
Document new entry point in section 'Global variables'.
* jit-playback.c (global_new_decl, global_finalize_lvalue): New
method.
(playback::context::new_global): Make use of global_new_decl,
global_finalize_lvalue.
(load_blob_in_ctor): New template function in use by the
following.
(playback::context::new_global_initialized): New method.
* jit-playback.h (class context): Decl 'new_global_initialized',
'global_new_decl', 'global_finalize_lvalue'.
(lvalue::set_initializer): Add implementation.
* jit-recording.c (recording::memento_of_get_pointer::get_size)
(recording::memento_of_get_type::get_size): Add implementation.
(recording::global::write_initializer_reproducer): New function in
use by 'recording::global::write_reproducer'.
(recording::global::replay_into)
(recording::global::write_to_dump)
(recording::global::write_reproducer): Handle
initialized case.
* jit-recording.h (class type): Decl 'get_size' and
'num_elements'.
* libgccjit++.h (class lvalue): Declare new 'set_initializer'
method.
(class lvalue): Decl 'is_global' and 'set_initializer'.
(class class global) Decl 'write_initializer_reproducer'. Add
'm_initializer', 'm_initializer_num_bytes' fields. Implement
'set_initializer'.
* libgccjit.c (gcc_jit_global_set_initializer): New function.
* libgccjit.h (gcc_jit_global_set_initializer): New function
declaration.
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
gcc/testsuite/ChangeLog
2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
* jit.dg/all-non-failing-tests.h: Add test-blob.c.
* jit.dg/test-global-set-initializer.c: New testcase.
[-- Attachment #2: 0001-Add-new-gcc_jit_global_set_initializer-entry-point.patch --]
[-- Type: text/plain, Size: 24482 bytes --]
From 74bd96fde6d4baed978555279d17898b989c00ad Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Sat, 30 May 2020 10:33:08 +0100
Subject: [PATCH] Add new gcc_jit_global_set_initializer entry point
gcc/jit/ChangeLog
2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.
* docs/topics/expressions.rst (gcc_jit_global_set_initializer):
Document new entry point in section 'Global variables'.
* jit-playback.c (global_new_decl, global_finalize_lvalue): New
method.
(playback::context::new_global): Make use of global_new_decl,
global_finalize_lvalue.
(load_blob_in_ctor): New template function in use by the
following.
(playback::context::new_global_initialized): New method.
* jit-playback.h (class context): Decl 'new_global_initialized',
'global_new_decl', 'global_finalize_lvalue'.
(lvalue::set_initializer): Add implementation.
* jit-recording.c (recording::memento_of_get_pointer::get_size)
(recording::memento_of_get_type::get_size): Add implementation.
(recording::global::write_initializer_reproducer): New function in
use by 'recording::global::write_reproducer'.
(recording::global::replay_into)
(recording::global::write_to_dump)
(recording::global::write_reproducer): Handle
initialized case.
* jit-recording.h (class type): Decl 'get_size' and
'num_elements'.
* libgccjit++.h (class lvalue): Declare new 'set_initializer'
method.
(class lvalue): Decl 'is_global' and 'set_initializer'.
(class class global) Decl 'write_initializer_reproducer'. Add
'm_initializer', 'm_initializer_num_bytes' fields. Implement
'set_initializer'.
* libgccjit.c (gcc_jit_global_set_initializer): New function.
* libgccjit.h (gcc_jit_global_set_initializer): New function
declaration.
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
gcc/testsuite/ChangeLog
2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
* jit.dg/all-non-failing-tests.h: Add test-blob.c.
* jit.dg/test-global-set-initializer.c: New testcase.
---
gcc/jit/docs/topics/compatibility.rst | 7 +
gcc/jit/docs/topics/expressions.rst | 21 +++
gcc/jit/jit-playback.c | 105 +++++++++++++-
gcc/jit/jit-playback.h | 17 +++
gcc/jit/jit-recording.c | 137 +++++++++++++++++-
gcc/jit/jit-recording.h | 26 ++++
gcc/jit/libgccjit++.h | 10 ++
gcc/jit/libgccjit.c | 36 +++++
gcc/jit/libgccjit.h | 14 ++
gcc/jit/libgccjit.map | 7 +-
gcc/testsuite/jit.dg/all-non-failing-tests.h | 7 +
.../jit.dg/test-global-set-initializer.c | 78 ++++++++++
12 files changed, 453 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-global-set-initializer.c
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index bb3387fa583b..6bfa101ed718 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -219,3 +219,10 @@ entrypoints:
* :func:`gcc_jit_version_minor`
* :func:`gcc_jit_version_patchlevel`
+
+.. _LIBGCCJIT_ABI_14:
+
+``LIBGCCJIT_ABI_14``
+--------------------
+``LIBGCCJIT_ABI_14`` covers the addition of
+:func:`gcc_jit_global_set_initializer`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index d783ceea51a8..7699dcfd27be 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -582,6 +582,27 @@ Global variables
referring to it. Analogous to using an "extern" global from a
header file.
+.. function:: gcc_jit_lvalue *\
+ gcc_jit_global_set_initializer (gcc_jit_lvalue *global,\
+ const void *blob,\
+ size_t num_bytes)
+
+ Set an initializer for an object using the memory content pointed
+ by ``blob`` for ``num_bytes``. ``global`` must be an arrays of an
+ integral type.
+
+ The parameter ``blob`` must be non-NULL. The call copies the memory
+ pointed by ``blob`` for ``num_bytes`` bytes, so it is valid to pass
+ in a pointer to an on-stack buffer. The content will be stored in
+ the compilation unit and used as initialization value of the array.
+
+ This entrypoint was added in :ref:`LIBGCCJIT_ABI_14`; you can test for
+ its presence using
+
+ .. code-block:: c
+
+ #ifdef LIBGCCJIT_HAVE_global_set_initializer
+
Working with pointers, structs and unions
-----------------------------------------
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index f9b3e675368c..50b69753bb42 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -111,6 +111,15 @@ public:
type *type,
const char *name);
+ lvalue *
+ new_global_initialized (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ size_t element_size,
+ size_t initializer_num_elem,
+ const void *initializer,
+ const char *name);
+
template <typename HOST_TYPE>
rvalue *
new_rvalue_from_const (type *type,
@@ -266,6 +275,14 @@ private:
const char * get_path_s_file () const;
const char * get_path_so_file () const;
+ tree
+ global_new_decl (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name);
+ lvalue *
+ global_finalize_lvalue (tree inner);
+
private:
/* Functions for implementing "compile". */
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 0fddf04da873..52fc92f5928c 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -510,14 +510,14 @@ new_function (location *loc,
return func;
}
-/* Construct a playback::lvalue instance (wrapping a tree). */
+/* In use by new_global and new_global_initialized. */
-playback::lvalue *
+tree
playback::context::
-new_global (location *loc,
- enum gcc_jit_global_kind kind,
- type *type,
- const char *name)
+global_new_decl (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name)
{
gcc_assert (type);
gcc_assert (name);
@@ -547,6 +547,15 @@ new_global (location *loc,
if (loc)
set_tree_location (inner, loc);
+ return inner;
+}
+
+/* In use by new_global and new_global_initialized. */
+
+playback::lvalue *
+playback::context::
+global_finalize_lvalue (tree inner)
+{
varpool_node::get_create (inner);
varpool_node::finalize_decl (inner);
@@ -556,6 +565,90 @@ new_global (location *loc,
return new lvalue (this, inner);
}
+/* Construct a playback::lvalue instance (wrapping a tree). */
+
+playback::lvalue *
+playback::context::
+new_global (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name)
+{
+ tree inner = global_new_decl (loc, kind, type, name);
+
+ return global_finalize_lvalue (inner);
+}
+
+/* Fill 'constructor_elements' with the memory content of
+ 'initializer'. Each element of the initializer is of the size of
+ type T. In use by new_global_initialized.*/
+
+template<typename T>
+static void
+load_blob_in_ctor (vec<constructor_elt, va_gc> *&constructor_elements,
+ size_t num_elem,
+ const void *initializer)
+{
+ /* Loosely based on 'output_init_element' c-typeck.c:9691. */
+ const T *p = (const T *)initializer;
+ tree node = make_unsigned_type (BITS_PER_UNIT * sizeof (T));
+ for (size_t i = 0; i < num_elem; i++)
+ {
+ constructor_elt celt =
+ { build_int_cst (long_unsigned_type_node, i),
+ build_int_cst (node, p[i]) };
+ vec_safe_push (constructor_elements, celt);
+ }
+}
+
+/* Construct an initialized playback::lvalue instance (wrapping a
+ tree). */
+
+playback::lvalue *
+playback::context::
+new_global_initialized (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ size_t element_size,
+ size_t initializer_num_elem,
+ const void *initializer,
+ const char *name)
+{
+ tree inner = global_new_decl (loc, kind, type, name);
+
+ static vec<constructor_elt, va_gc> *constructor_elements;
+
+ switch (element_size)
+ {
+ case 1:
+ load_blob_in_ctor<uint8_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ case 2:
+ load_blob_in_ctor<uint16_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ case 4:
+ load_blob_in_ctor<uint32_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ case 8:
+ load_blob_in_ctor<uint64_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ default:
+ gcc_unreachable ();
+ }
+ /* Compare with 'pop_init_level' c-typeck.c:8780. */
+ tree ctor = build_constructor (type->as_tree (), constructor_elements);
+ constructor_elements = NULL;
+
+ /* Compare with 'store_init_value' c-typeck.c:7555. */
+ DECL_INITIAL (inner) = ctor;
+
+ return global_finalize_lvalue (inner);
+}
+
/* Implementation of the various
gcc::jit::playback::context::new_rvalue_from_const <HOST_TYPE>
methods.
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 726b9c4b8371..7a7a61a0e126 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -502,6 +502,7 @@ public:
This will return NULL if it's not valid to dereference this type.
The caller is responsible for setting an error. */
virtual type *dereference () = 0;
+ virtual size_t get_size () { gcc_unreachable (); }
/* Dynamic casts. */
virtual function_type *dyn_cast_function_type () { return NULL; }
@@ -534,6 +535,7 @@ public:
virtual type *is_array () = 0;
virtual bool is_void () const { return false; }
virtual bool has_known_size () const { return true; }
+ virtual int num_elements () { gcc_unreachable (); }
bool is_numeric () const
{
@@ -569,6 +571,8 @@ public:
type *dereference () FINAL OVERRIDE;
+ size_t get_size () FINAL OVERRIDE;
+
bool accepts_writes_from (type *rtype) FINAL OVERRIDE
{
if (m_kind == GCC_JIT_TYPE_VOID_PTR)
@@ -610,6 +614,8 @@ public:
type *dereference () FINAL OVERRIDE { return m_other_type; }
+ size_t get_size () FINAL OVERRIDE;
+
bool accepts_writes_from (type *rtype) FINAL OVERRIDE;
void replay_into (replayer *r) FINAL OVERRIDE;
@@ -755,6 +761,7 @@ class array_type : public type
bool is_bool () const FINAL OVERRIDE { return false; }
type *is_pointer () FINAL OVERRIDE { return NULL; }
type *is_array () FINAL OVERRIDE { return m_element_type; }
+ int num_elements () FINAL OVERRIDE { return m_num_elements; }
void replay_into (replayer *) FINAL OVERRIDE;
@@ -1107,6 +1114,8 @@ public:
const char *access_as_rvalue (reproducer &r) OVERRIDE;
virtual const char *access_as_lvalue (reproducer &r);
+ virtual bool is_global () const { return false; }
+ virtual void set_initializer (const void *, size_t) { gcc_unreachable (); }
};
class param : public lvalue
@@ -1335,8 +1344,23 @@ public:
void write_to_dump (dump &d) FINAL OVERRIDE;
+ virtual bool is_global () const FINAL OVERRIDE { return true; }
+
+ virtual void
+ set_initializer (const void *initializer,
+ size_t num_bytes) FINAL OVERRIDE
+ {
+ if (m_initializer)
+ free (m_initializer);
+ m_initializer = xmalloc (num_bytes);
+ memcpy (m_initializer, initializer, num_bytes);
+ m_initializer_num_bytes = num_bytes;
+ }
+
private:
string * make_debug_string () FINAL OVERRIDE { return m_name; }
+ template <typename T>
+ void write_initializer_reproducer (const char *id, reproducer &r);
void write_reproducer (reproducer &r) FINAL OVERRIDE;
enum precedence get_precedence () const FINAL OVERRIDE
{
@@ -1346,6 +1370,8 @@ private:
private:
enum gcc_jit_global_kind m_kind;
string *m_name;
+ void *m_initializer = NULL;
+ size_t m_initializer_num_bytes = 0;
};
template <typename HOST_TYPE>
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index b73cd76a0a02..f97de00f63c3 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -2175,6 +2175,57 @@ recording::type::access_as_type (reproducer &r)
return r.get_identifier (this);
}
+/* Override of default implementation of
+ recording::type::get_size.
+
+ Return the size in bytes. This is in use for global
+ initialization. */
+
+size_t
+recording::memento_of_get_type::get_size ()
+{
+ int size;
+ switch (m_kind)
+ {
+ case GCC_JIT_TYPE_VOID:
+ return 0;
+ case GCC_JIT_TYPE_BOOL:
+ case GCC_JIT_TYPE_CHAR:
+ case GCC_JIT_TYPE_SIGNED_CHAR:
+ case GCC_JIT_TYPE_UNSIGNED_CHAR:
+ return 1;
+ case GCC_JIT_TYPE_SHORT:
+ case GCC_JIT_TYPE_UNSIGNED_SHORT:
+ size = SHORT_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_INT:
+ case GCC_JIT_TYPE_UNSIGNED_INT:
+ size = INT_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_LONG:
+ case GCC_JIT_TYPE_UNSIGNED_LONG:
+ size = LONG_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_LONG_LONG:
+ case GCC_JIT_TYPE_UNSIGNED_LONG_LONG:
+ size = LONG_LONG_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_FLOAT:
+ size = FLOAT_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_DOUBLE:
+ size = DOUBLE_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_LONG_DOUBLE:
+ size = LONG_DOUBLE_TYPE_SIZE;
+ break;
+ default:
+ gcc_unreachable ();
+ }
+
+ return size / BITS_PER_UNIT;
+}
+
/* Implementation of pure virtual hook recording::type::dereference for
recording::memento_of_get_type. */
@@ -2482,6 +2533,15 @@ recording::memento_of_get_type::write_reproducer (reproducer &r)
/* The implementation of class gcc::jit::recording::memento_of_get_pointer. */
+/* Override of default implementation of
+ recording::type::get_size for get_pointer. */
+
+size_t
+recording::memento_of_get_pointer::get_size ()
+{
+ return POINTER_SIZE / BITS_PER_UNIT;
+}
+
/* Override of default implementation of
recording::type::accepts_writes_from for get_pointer.
@@ -4393,10 +4453,20 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
void
recording::global::replay_into (replayer *r)
{
- set_playback_obj (r->new_global (playback_location (r, m_loc),
- m_kind,
- m_type->playback_type (),
- playback_string (m_name)));
+ set_playback_obj (
+ m_initializer
+ ? r->new_global_initialized (playback_location (r, m_loc),
+ m_kind,
+ m_type->playback_type (),
+ m_type->dereference ()->get_size (),
+ m_initializer_num_bytes
+ / m_type->dereference ()->get_size (),
+ m_initializer,
+ playback_string (m_name))
+ : r->new_global (playback_location (r, m_loc),
+ m_kind,
+ m_type->playback_type (),
+ playback_string (m_name)));
}
/* Override the default implementation of
@@ -4440,9 +4510,26 @@ recording::global::write_to_dump (dump &d)
d.write ("extern ");
break;
}
- d.write ("%s %s;\n",
+
+ d.write ("%s %s",
m_type->get_debug_string (),
get_debug_string ());
+
+ if (!m_initializer)
+ {
+ d.write (";\n");
+ return;
+ }
+
+ d.write ("=\n { ");
+ const char *p = (const char *)m_initializer;
+ for (size_t i = 0; i < m_initializer_num_bytes; i++)
+ {
+ d.write ("0x%x, ", p[i]);
+ if (i && !(i % 64))
+ d.write ("\n ");
+ }
+ d.write ("};\n");
}
/* A table of enum gcc_jit_global_kind values expressed in string
@@ -4454,6 +4541,27 @@ static const char * const global_kind_reproducer_strings[] = {
"GCC_JIT_GLOBAL_IMPORTED"
};
+template <typename T>
+void
+recording::global::write_initializer_reproducer (const char *id, reproducer &r)
+{
+ const char *init_id = r.make_tmp_identifier ("init_for", this);
+ r.write (" %s %s[] =\n {",
+ m_type->dereference ()->get_debug_string (),
+ init_id);
+
+ const T *p = (const T *)m_initializer;
+ for (size_t i = 0; i < m_initializer_num_bytes / sizeof (T); i++)
+ {
+ r.write ("%lu, ", (uint64_t)p[i]);
+ if (i && !(i % 64))
+ r.write ("\n ");
+ }
+ r.write ("};\n");
+ r.write (" gcc_jit_global_set_initializer (%s, %s, sizeof (%s));\n",
+ id, init_id, init_id);
+}
+
/* Implementation of recording::memento::write_reproducer for globals. */
void
@@ -4472,6 +4580,25 @@ recording::global::write_reproducer (reproducer &r)
global_kind_reproducer_strings[m_kind],
r.get_identifier_as_type (get_type ()),
m_name->get_debug_string ());
+
+ if (m_initializer)
+ switch (m_type->dereference ()->get_size ())
+ {
+ case 1:
+ write_initializer_reproducer<uint8_t> (id, r);
+ break;
+ case 2:
+ write_initializer_reproducer<uint16_t> (id, r);
+ break;
+ case 4:
+ write_initializer_reproducer<uint32_t> (id, r);
+ break;
+ case 8:
+ write_initializer_reproducer<uint64_t> (id, r);
+ break;
+ default:
+ gcc_unreachable ();
+ }
}
/* The implementation of the various const-handling classes:
diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
index 69e67766640c..1b9ef1a5db98 100644
--- a/gcc/jit/libgccjit++.h
+++ b/gcc/jit/libgccjit++.h
@@ -488,6 +488,7 @@ namespace gccjit
location loc = location ());
rvalue get_address (location loc = location ());
+ lvalue set_initializer (const void *blob, size_t num_bytes);
};
class param : public lvalue
@@ -1737,6 +1738,15 @@ lvalue::get_address (location loc)
loc.get_inner_location ()));
}
+inline lvalue
+lvalue::set_initializer (const void *blob, size_t num_bytes)
+{
+ gcc_jit_global_set_initializer (get_inner_lvalue (),
+ blob,
+ num_bytes);
+ return *this;
+}
+
// class param : public lvalue
inline param::param () : lvalue () {}
inline param::param (gcc_jit_param *inner)
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 1c5a12e9c015..08b855230f6a 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -788,6 +788,20 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
gcc_jit_type *type,
const char *name);
+#define LIBGCCJIT_HAVE_global_set_initializer
+
+/* Set a static initializer for a global return the global itself.
+
+ This API entrypoint was added in LIBGCCJIT_ABI_14; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
+*/
+
+extern gcc_jit_lvalue *
+gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
+ const void *blob,
+ size_t num_bytes);
+
/* Upcasting. */
extern gcc_jit_object *
gcc_jit_lvalue_as_object (gcc_jit_lvalue *lvalue);
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index 3d04f6db3aff..80c5aa6ac115 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -1117,6 +1117,42 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
return (gcc_jit_lvalue *)ctxt->new_global (loc, kind, type, name);
}
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::global::set_initializer method, in
+ jit-recording.c. */
+
+extern gcc_jit_lvalue *
+gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
+ const void *blob,
+ size_t num_bytes)
+{
+ RETURN_NULL_IF_FAIL (global, NULL, NULL, "NULL global");
+ RETURN_NULL_IF_FAIL (blob, NULL, NULL, "NULL blob");
+ RETURN_NULL_IF_FAIL_PRINTF1 (global->is_global (), NULL, NULL,
+ "global \"%s\" not a global",
+ global->get_debug_string ());
+
+ gcc::jit::recording::type *lval_type = global->get_type ();
+ RETURN_NULL_IF_FAIL_PRINTF1 (lval_type->is_array (), NULL, NULL,
+ "global \"%s\" not an array",
+ global->get_debug_string ());
+ RETURN_NULL_IF_FAIL_PRINTF1 (lval_type->dereference ()->is_int (), NULL, NULL,
+ "global \"%s\" not an array of integral type",
+ global->get_debug_string ());
+ RETURN_NULL_IF_FAIL_PRINTF1 (
+ lval_type->dereference ()->get_size() * lval_type->num_elements ()
+ == num_bytes,
+ NULL, NULL,
+ "global \"%s\" and initializer don't match size",
+ global->get_debug_string ());
+
+ global->set_initializer (blob, num_bytes);
+
+ return global;
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, this calls the trivial
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 6137dd4b4b03..a6e67e781a4b 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -186,4 +186,9 @@ LIBGCCJIT_ABI_13 {
gcc_jit_version_major;
gcc_jit_version_minor;
gcc_jit_version_patchlevel;
-} LIBGCCJIT_ABI_12;
\ No newline at end of file
+} LIBGCCJIT_ABI_12;
+
+LIBGCCJIT_ABI_14 {
+ global:
+ gcc_jit_global_set_initializer;
+} LIBGCCJIT_ABI_13;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 632ab8cfb2e4..4202eb7798b7 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -174,6 +174,13 @@
#undef create_code
#undef verify_code
+/* test-global-set-initializer.c */
+#define create_code create_code_global_set_initializer
+#define verify_code verify_code_global_set_initializer
+#include "test-global-set-initializer.c"
+#undef create_code
+#undef verify_code
+
/* test-hello-world.c */
#define create_code create_code_hello_world
#define verify_code verify_code_hello_world
diff --git a/gcc/testsuite/jit.dg/test-global-set-initializer.c b/gcc/testsuite/jit.dg/test-global-set-initializer.c
new file mode 100644
index 000000000000..d38aba7d73f5
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-global-set-initializer.c
@@ -0,0 +1,78 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+#define BIG_BLOB_SIZE (1 << 12) /* 4KB. */
+
+static signed char test_blob1[] = { 0xc, 0xa, 0xf, 0xf, 0xe };
+static unsigned test_blob2[] = { 0x3, 0x2, 0x1, 0x0, 0x1, 0x2, 0x3 };
+static unsigned char test_blob3[BIG_BLOB_SIZE];
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+
+ signed char bin_blob1[] = { 0xc, 0xa, 0xf, 0xf, 0xe };
+ unsigned bin_blob2[] = { 0x3, 0x2, 0x1, 0x0, 0x1, 0x2, 0x3 };
+ unsigned char bin_blob3[4096]...
+ */
+ gcc_jit_type *unsigned_char_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_UNSIGNED_CHAR);
+ gcc_jit_type *signed_char_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_SIGNED_CHAR);
+ gcc_jit_type *unsigned_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_UNSIGNED_INT);
+
+ gcc_jit_lvalue *glob =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED,
+ gcc_jit_context_new_array_type (ctxt, NULL, signed_char_type,
+ sizeof (test_blob1)),
+ "bin_blob1");
+ gcc_jit_global_set_initializer (glob, test_blob1, sizeof (test_blob1));
+
+ glob =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED,
+ gcc_jit_context_new_array_type (
+ ctxt, NULL, unsigned_type,
+ sizeof (test_blob2) / sizeof (*test_blob2)),
+ "bin_blob2");
+ gcc_jit_global_set_initializer (glob, test_blob2,
+ sizeof (test_blob2));
+
+ for (size_t i = 0; i < BIG_BLOB_SIZE; i++)
+ test_blob3[i] = i * i + i;
+ glob =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED,
+ gcc_jit_context_new_array_type (ctxt, NULL, unsigned_char_type,
+ sizeof (test_blob3)),
+ "bin_blob3");
+ gcc_jit_global_set_initializer (glob, test_blob3, sizeof (test_blob3));
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ CHECK_NON_NULL (result);
+ void *glob = gcc_jit_result_get_global (result, "bin_blob1");
+ CHECK_NON_NULL (glob);
+ CHECK_VALUE (memcmp (test_blob1, glob, sizeof (test_blob1)), 0);
+
+ glob = gcc_jit_result_get_global (result, "bin_blob2");
+ CHECK_NON_NULL (glob);
+ CHECK_VALUE (memcmp (test_blob2, glob,
+ sizeof (test_blob2)), 0);
+
+ glob = gcc_jit_result_get_global (result, "bin_blob3");
+ CHECK_NON_NULL (glob);
+ CHECK_VALUE (memcmp (test_blob3, glob, sizeof (test_blob3)), 0);
+
+}
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-08-03 8:07 ` Andrea Corallo
@ 2020-08-06 19:53 ` David Malcolm
2020-08-19 7:17 ` [PATCH V2] " Andrea Corallo
0 siblings, 1 reply; 14+ messages in thread
From: David Malcolm @ 2020-08-06 19:53 UTC (permalink / raw)
To: Andrea Corallo; +Cc: jit, nd, gcc-patches
On Mon, 2020-08-03 at 10:07 +0200, Andrea Corallo wrote:
> David Malcolm <dmalcolm@redhat.com> writes:
>
> > On Fri, 2020-07-24 at 18:05 -0400, David Malcolm via Gcc-patches wrote:
> >
> > [...]
> >
> >> I haven't thought this through in detail, and I'm not sure exactly
> >> how
> >> it would work for arbitrary types, but I thought it worth sharing.
> >> (For example I can think of nasty issues if we ever want to support
> >> cross-compilation, e.g. where sizeof types or endianness differs
> >> between host and target).
> >
> > ...which is an argument in favor of retaining the name "blob", perhaps
> > as the name of the argument in the header file e.g.:
> >
> > extern void
> > gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> > const void *blob,
> > size_t num_bytes);
> >
> >
> > as a subtle hint to the user that they need to be wary about binary
> > layouts ("here be dragons").
> >
> > [...]
>
> Hi Dave & all,
>
> following up this is my take on the implementation of:
>
> gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> const void *blob,
> size_t num_bytes);
>
> 'global' must be an array but in the seek of generality it now supports
> all the various integral types and is not limited to char[].
>
> As you anticipated the implementation I came up is currently not safe
> for cross-compilation, not sure is requirement tho.
>
> make check-jit is clean
>
> Feedback very welcome
>
> Thanks!
Thanks for the updated patch. Comments inline below.
> Andrea
>
> gcc/jit/ChangeLog
>
> 2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
>
> * docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.
> * docs/topics/expressions.rst (gcc_jit_global_set_initializer):
> Document new entry point in section 'Global variables'.
> * jit-playback.c (global_new_decl, global_finalize_lvalue): New
> method.
> (playback::context::new_global): Make use of global_new_decl,
> global_finalize_lvalue.
> (load_blob_in_ctor): New template function in use by the
> following.
> (playback::context::new_global_initialized): New method.
> * jit-playback.h (class context): Decl 'new_global_initialized',
> 'global_new_decl', 'global_finalize_lvalue'.
> (lvalue::set_initializer): Add implementation.
> * jit-recording.c (recording::memento_of_get_pointer::get_size)
> (recording::memento_of_get_type::get_size): Add implementation.
> (recording::global::write_initializer_reproducer): New function in
> use by 'recording::global::write_reproducer'.
> (recording::global::replay_into)
> (recording::global::write_to_dump)
> (recording::global::write_reproducer): Handle
> initialized case.
> * jit-recording.h (class type): Decl 'get_size' and
> 'num_elements'.
> * libgccjit++.h (class lvalue): Declare new 'set_initializer'
> method.
> (class lvalue): Decl 'is_global' and 'set_initializer'.
> (class class global) Decl 'write_initializer_reproducer'. Add
> 'm_initializer', 'm_initializer_num_bytes' fields. Implement
> 'set_initializer'.
> * libgccjit.c (gcc_jit_global_set_initializer): New function.
> * libgccjit.h (gcc_jit_global_set_initializer): New function
> declaration.
> * libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
>
> gcc/testsuite/ChangeLog
>
> 2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
>
> * jit.dg/all-non-failing-tests.h: Add test-blob.c.
> * jit.dg/test-global-set-initializer.c: New testcase.
[...]
> diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
> index d783ceea51a8..7699dcfd27be 100644
> --- a/gcc/jit/docs/topics/expressions.rst
> +++ b/gcc/jit/docs/topics/expressions.rst
> @@ -582,6 +582,27 @@ Global variables
> referring to it. Analogous to using an "extern" global from a
> header file.
>
> +.. function:: gcc_jit_lvalue *\
> + gcc_jit_global_set_initializer (gcc_jit_lvalue *global,\
> + const void *blob,\
> + size_t num_bytes)
> +
> + Set an initializer for an object using the memory content pointed
> + by ``blob`` for ``num_bytes``. ``global`` must be an arrays of an
Typo: "arrays" -> "array"
> + integral type.
Why the non-void return type? Looking at libgccjit.c I see it returns
"global" if it succeeds, or NULL if it fails. Wouldn't it be better to
simply have void return type, and rely on the normaly error-handling
mechanisms?
Or is this inspired by the inline asm patch? (for PR 87291)
[...]
> --- a/gcc/jit/jit-playback.h
> +++ b/gcc/jit/jit-playback.h
> @@ -111,6 +111,15 @@ public:
> type *type,
> const char *name);
>
> + lvalue *
> + new_global_initialized (location *loc,
> + enum gcc_jit_global_kind kind,
> + type *type,
> + size_t element_size,
> + size_t initializer_num_elem,
> + const void *initializer,
> + const char *name);
> +
> template <typename HOST_TYPE>
> rvalue *
> new_rvalue_from_const (type *type,
> @@ -266,6 +275,14 @@ private:
> const char * get_path_s_file () const;
> const char * get_path_so_file () const;
>
> + tree
> + global_new_decl (location *loc,
> + enum gcc_jit_global_kind kind,
> + type *type,
> + const char *name);
> + lvalue *
> + global_finalize_lvalue (tree inner);
> +
> private:
>
> /* Functions for implementing "compile". */
> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
> index 0fddf04da873..52fc92f5928c 100644
> --- a/gcc/jit/jit-playback.c
> +++ b/gcc/jit/jit-playback.c
> @@ -510,14 +510,14 @@ new_function (location *loc,
> return func;
> }
>
> -/* Construct a playback::lvalue instance (wrapping a tree). */
> +/* In use by new_global and new_global_initialized. */
>
> -playback::lvalue *
> +tree
> playback::context::
> -new_global (location *loc,
> - enum gcc_jit_global_kind kind,
> - type *type,
> - const char *name)
> +global_new_decl (location *loc,
> + enum gcc_jit_global_kind kind,
> + type *type,
> + const char *name)
> {
> gcc_assert (type);
> gcc_assert (name);
> @@ -547,6 +547,15 @@ new_global (location *loc,
> if (loc)
> set_tree_location (inner, loc);
>
> + return inner;
> +}
> +
> +/* In use by new_global and new_global_initialized. */
> +
> +playback::lvalue *
> +playback::context::
> +global_finalize_lvalue (tree inner)
> +{
> varpool_node::get_create (inner);
>
> varpool_node::finalize_decl (inner);
> @@ -556,6 +565,90 @@ new_global (location *loc,
> return new lvalue (this, inner);
> }
>
> +/* Construct a playback::lvalue instance (wrapping a tree). */
> +
> +playback::lvalue *
> +playback::context::
> +new_global (location *loc,
> + enum gcc_jit_global_kind kind,
> + type *type,
> + const char *name)
> +{
> + tree inner = global_new_decl (loc, kind, type, name);
> +
> + return global_finalize_lvalue (inner);
> +}
> +
> +/* Fill 'constructor_elements' with the memory content of
> + 'initializer'. Each element of the initializer is of the size of
> + type T. In use by new_global_initialized.*/
> +
> +template<typename T>
> +static void
> +load_blob_in_ctor (vec<constructor_elt, va_gc> *&constructor_elements,
> + size_t num_elem,
> + const void *initializer)
> +{
> + /* Loosely based on 'output_init_element' c-typeck.c:9691. */
> + const T *p = (const T *)initializer;
> + tree node = make_unsigned_type (BITS_PER_UNIT * sizeof (T));
> + for (size_t i = 0; i < num_elem; i++)
> + {
> + constructor_elt celt =
> + { build_int_cst (long_unsigned_type_node, i),
> + build_int_cst (node, p[i]) };
> + vec_safe_push (constructor_elements, celt);
> + }
> +}
> +
> +/* Construct an initialized playback::lvalue instance (wrapping a
> + tree). */
> +
> +playback::lvalue *
> +playback::context::
> +new_global_initialized (location *loc,
> + enum gcc_jit_global_kind kind,
> + type *type,
> + size_t element_size,
> + size_t initializer_num_elem,
> + const void *initializer,
> + const char *name)
> +{
> + tree inner = global_new_decl (loc, kind, type, name);
> +
> + static vec<constructor_elt, va_gc> *constructor_elements;
Why the use of a function-level static variable here, and why va_gc?
Wouldn't an auto_vec<constructor_elt> be cleaner?
Ah: is it because of the call to build_constructor?
If so, can the "static" be removed and replaced with an "= NULL;"
initializer?
(I'm very wary of function-level static variables, as they're a place
when state can "hide" between multiple in-process invocations of the
compiler)
> +
> + switch (element_size)
> + {
> + case 1:
> + load_blob_in_ctor<uint8_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + case 2:
> + load_blob_in_ctor<uint16_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + case 4:
> + load_blob_in_ctor<uint32_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + case 8:
> + load_blob_in_ctor<uint64_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + default:
> + gcc_unreachable ();
> + }
> + /* Compare with 'pop_init_level' c-typeck.c:8780. */
> + tree ctor = build_constructor (type->as_tree (), constructor_elements);
> + constructor_elements = NULL;
> +
> + /* Compare with 'store_init_value' c-typeck.c:7555. */
> + DECL_INITIAL (inner) = ctor;
> +
> + return global_finalize_lvalue (inner);
> +}
> +
> /* Implementation of the various
> gcc::jit::playback::context::new_rvalue_from_const <HOST_TYPE>
> methods.
> diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
> index 726b9c4b8371..7a7a61a0e126 100644
> --- a/gcc/jit/jit-recording.h
> +++ b/gcc/jit/jit-recording.h
> @@ -502,6 +502,7 @@ public:
> This will return NULL if it's not valid to dereference this type.
> The caller is responsible for setting an error. */
> virtual type *dereference () = 0;
> + virtual size_t get_size () { gcc_unreachable (); }
It looks like get_size is only implemented for memento_of_get_pointer
and memento_of_get_type::get_size.
A gcc_unreachable in the base class seems like a "code smell", but it's
probably OK here: I believe the restriction here is that this should
only be called on initializers for globals, and we only set these on
globals of certain types, right? Please can you capture that
restriction in a comment next to the gcc_unreachable.
> /* Dynamic casts. */
> virtual function_type *dyn_cast_function_type () { return NULL; }
> @@ -534,6 +535,7 @@ public:
> virtual type *is_array () = 0;
> virtual bool is_void () const { return false; }
> virtual bool has_known_size () const { return true; }
> + virtual int num_elements () { gcc_unreachable (); }
Again, I'm wary of a virtual fn with a gcc_unreachable in the base
class. Here, I believe it's only implemented for array_type.
Wouldn't this be better as a non-virtual member function of array_type,
and simply cast to the subclass at any caller(s)?
> bool is_numeric () const
> {
> @@ -569,6 +571,8 @@ public:
>
> type *dereference () FINAL OVERRIDE;
>
> + size_t get_size () FINAL OVERRIDE;
> +
> bool accepts_writes_from (type *rtype) FINAL OVERRIDE
> {
> if (m_kind == GCC_JIT_TYPE_VOID_PTR)
> @@ -610,6 +614,8 @@ public:
>
> type *dereference () FINAL OVERRIDE { return m_other_type; }
>
> + size_t get_size () FINAL OVERRIDE;
> +
> bool accepts_writes_from (type *rtype) FINAL OVERRIDE;
>
> void replay_into (replayer *r) FINAL OVERRIDE;
> @@ -755,6 +761,7 @@ class array_type : public type
> bool is_bool () const FINAL OVERRIDE { return false; }
> type *is_pointer () FINAL OVERRIDE { return NULL; }
> type *is_array () FINAL OVERRIDE { return m_element_type; }
> + int num_elements () FINAL OVERRIDE { return m_num_elements; }
>
> void replay_into (replayer *) FINAL OVERRIDE;
>
> @@ -1107,6 +1114,8 @@ public:
>
> const char *access_as_rvalue (reproducer &r) OVERRIDE;
> virtual const char *access_as_lvalue (reproducer &r);
> + virtual bool is_global () const { return false; }
> + virtual void set_initializer (const void *, size_t) { gcc_unreachable (); }
Similar comments here.
> class param : public lvalue
> @@ -1335,8 +1344,23 @@ public:
>
> void write_to_dump (dump &d) FINAL OVERRIDE;
>
> + virtual bool is_global () const FINAL OVERRIDE { return true; }
No need to add "virtual" on a function that's labelled FINAL and/or
OVERRIDE.
> + virtual void
> + set_initializer (const void *initializer,
> + size_t num_bytes) FINAL OVERRIDE
> + {
> + if (m_initializer)
> + free (m_initializer);
> + m_initializer = xmalloc (num_bytes);
> + memcpy (m_initializer, initializer, num_bytes);
> + m_initializer_num_bytes = num_bytes;
> + }
As noted above, could this be non-virtual, and use casting instead?
I can't remember if this class has a dtor, but presumably we should
also be freeing m_initializer there.
> private:
> string * make_debug_string () FINAL OVERRIDE { return m_name; }
> + template <typename T>
> + void write_initializer_reproducer (const char *id, reproducer &r);
> void write_reproducer (reproducer &r) FINAL OVERRIDE;
> enum precedence get_precedence () const FINAL OVERRIDE
> {
> @@ -1346,6 +1370,8 @@ private:
> private:
> enum gcc_jit_global_kind m_kind;
> string *m_name;
> + void *m_initializer = NULL;
> + size_t m_initializer_num_bytes = 0;
> };
Are we still targetting C++98, and are these initializers compatible
with it?
> template <typename HOST_TYPE>
> diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
> index b73cd76a0a02..f97de00f63c3 100644
> --- a/gcc/jit/jit-recording.c
> +++ b/gcc/jit/jit-recording.c
[...]
> @@ -4440,9 +4510,26 @@ recording::global::write_to_dump (dump &d)
> d.write ("extern ");
> break;
> }
> - d.write ("%s %s;\n",
> +
> + d.write ("%s %s",
> m_type->get_debug_string (),
> get_debug_string ());
> +
> + if (!m_initializer)
> + {
> + d.write (";\n");
> + return;
> + }
> +
> + d.write ("=\n { ");
> + const char *p = (const char *)m_initializer;
Should be "const unsigned char"?
> + for (size_t i = 0; i < m_initializer_num_bytes; i++)
> + {
> + d.write ("0x%x, ", p[i]);
> + if (i && !(i % 64))
> + d.write ("\n ");
> + }
> + d.write ("};\n");
> }
>
> /* A table of enum gcc_jit_global_kind values expressed in string
> @@ -4454,6 +4541,27 @@ static const char * const global_kind_reproducer_strings[] = {
> "GCC_JIT_GLOBAL_IMPORTED"
> };
>
> +template <typename T>
> +void
> +recording::global::write_initializer_reproducer (const char *id, reproducer &r)
> +{
> + const char *init_id = r.make_tmp_identifier ("init_for", this);
> + r.write (" %s %s[] =\n {",
> + m_type->dereference ()->get_debug_string (),
> + init_id);
> +
> + const T *p = (const T *)m_initializer;
> + for (size_t i = 0; i < m_initializer_num_bytes / sizeof (T); i++)
> + {
> + r.write ("%lu, ", (uint64_t)p[i]);
Does this assume that "%lu" matches uint64_t?
> + if (i && !(i % 64))
> + r.write ("\n ");
> + }
> + r.write ("};\n");
> + r.write (" gcc_jit_global_set_initializer (%s, %s, sizeof (%s));\n",
> + id, init_id, init_id);
> +}
> +
> /* Implementation of recording::memento::write_reproducer for globals. */
>
> void
> @@ -4472,6 +4580,25 @@ recording::global::write_reproducer (reproducer &r)
> global_kind_reproducer_strings[m_kind],
> r.get_identifier_as_type (get_type ()),
> m_name->get_debug_string ());
> +
> + if (m_initializer)
> + switch (m_type->dereference ()->get_size ())
> + {
> + case 1:
> + write_initializer_reproducer<uint8_t> (id, r);
> + break;
> + case 2:
> + write_initializer_reproducer<uint16_t> (id, r);
> + break;
> + case 4:
> + write_initializer_reproducer<uint32_t> (id, r);
> + break;
> + case 8:
> + write_initializer_reproducer<uint64_t> (id, r);
> + break;
> + default:
> + gcc_unreachable ();
> + }
> }
>
> /* The implementation of the various const-handling classes:
> diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
> index 69e67766640c..1b9ef1a5db98 100644
> --- a/gcc/jit/libgccjit++.h
> +++ b/gcc/jit/libgccjit++.h
> @@ -488,6 +488,7 @@ namespace gccjit
> location loc = location ());
>
> rvalue get_address (location loc = location ());
> + lvalue set_initializer (const void *blob, size_t num_bytes);
> };
>
> class param : public lvalue
> @@ -1737,6 +1738,15 @@ lvalue::get_address (location loc)
> loc.get_inner_location ()));
> }
>
> +inline lvalue
> +lvalue::set_initializer (const void *blob, size_t num_bytes)
> +{
> + gcc_jit_global_set_initializer (get_inner_lvalue (),
> + blob,
> + num_bytes);
> + return *this;
> +}
(Again the return type)
> // class param : public lvalue
> inline param::param () : lvalue () {}
> inline param::param (gcc_jit_param *inner)
> diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
> index 1c5a12e9c015..08b855230f6a 100644
> --- a/gcc/jit/libgccjit.h
> +++ b/gcc/jit/libgccjit.h
> @@ -788,6 +788,20 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
> gcc_jit_type *type,
> const char *name);
>
> +#define LIBGCCJIT_HAVE_global_set_initializer
> +
> +/* Set a static initializer for a global return the global itself.
> +
> + This API entrypoint was added in LIBGCCJIT_ABI_14; you can test for its
> + presence using
> + #ifdef LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
> +*/
> +
> +extern gcc_jit_lvalue *
> +gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> + const void *blob,
> + size_t num_bytes);
> +
> /* Upcasting. */
> extern gcc_jit_object *
> gcc_jit_lvalue_as_object (gcc_jit_lvalue *lvalue);
> diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> index 3d04f6db3aff..80c5aa6ac115 100644
> --- a/gcc/jit/libgccjit.c
> +++ b/gcc/jit/libgccjit.c
> @@ -1117,6 +1117,42 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
> return (gcc_jit_lvalue *)ctxt->new_global (loc, kind, type, name);
> }
>
> +/* Public entrypoint. See description in libgccjit.h.
> +
> + After error-checking, the real work is done by the
> + gcc::jit::recording::global::set_initializer method, in
> + jit-recording.c. */
> +
> +extern gcc_jit_lvalue *
> +gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> + const void *blob,
> + size_t num_bytes)
> +{
> + RETURN_NULL_IF_FAIL (global, NULL, NULL, "NULL global");
> + RETURN_NULL_IF_FAIL (blob, NULL, NULL, "NULL blob");
> + RETURN_NULL_IF_FAIL_PRINTF1 (global->is_global (), NULL, NULL,
> + "global \"%s\" not a global",
^^^^^^
"lvalue \"%s\" is not a global",
surely, as it's not a global.
> + global->get_debug_string ());
> + gcc::jit::recording::type *lval_type = global->get_type ();
> + RETURN_NULL_IF_FAIL_PRINTF1 (lval_type->is_array (), NULL, NULL,
> + "global \"%s\" not an array",
"not" -> "is not".
> + global->get_debug_string ());
> + RETURN_NULL_IF_FAIL_PRINTF1 (lval_type->dereference ()->is_int (), NULL, NULL,
> + "global \"%s\" not an array of integral type",
"not" -> "is not".
> + global->get_debug_string ());
> + RETURN_NULL_IF_FAIL_PRINTF1 (
> + lval_type->dereference ()->get_size() * lval_type->num_elements ()
> + == num_bytes,
> + NULL, NULL,
> + "global \"%s\" and initializer don't match size",
Would be more user-friendly to specify the two values in the error
message, though off the top of my head I don't remember the format code
for size_t.
> + global->get_debug_string ());
> +
> + global->set_initializer (blob, num_bytes);
> +
> + return global;
> +}
> +
[...]
Thanks again for the patch; hope this is constructive
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-08-06 19:53 ` David Malcolm
@ 2020-08-19 7:17 ` Andrea Corallo
2020-09-09 7:51 ` Andrea Corallo
2020-09-10 22:22 ` David Malcolm
0 siblings, 2 replies; 14+ messages in thread
From: Andrea Corallo @ 2020-08-19 7:17 UTC (permalink / raw)
To: David Malcolm; +Cc: jit, nd, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 8075 bytes --]
David Malcolm <dmalcolm@redhat.com> writes:
> Thanks for the updated patch. Comments inline below.
Hi Dave,
sorry for the late reply.
> [...]
>
>> diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
>> index d783ceea51a8..7699dcfd27be 100644
>> --- a/gcc/jit/docs/topics/expressions.rst
>> +++ b/gcc/jit/docs/topics/expressions.rst
>> @@ -582,6 +582,27 @@ Global variables
>> referring to it. Analogous to using an "extern" global from a
>> header file.
>>
>> +.. function:: gcc_jit_lvalue *\
>> + gcc_jit_global_set_initializer (gcc_jit_lvalue *global,\
>> + const void *blob,\
>> + size_t num_bytes)
>> +
>> + Set an initializer for an object using the memory content pointed
>> + by ``blob`` for ``num_bytes``. ``global`` must be an arrays of an
>
> Typo: "arrays" -> "array"
Fixed
>> + integral type.
>
> Why the non-void return type? Looking at libgccjit.c I see it returns
> "global" if it succeeds, or NULL if it fails. Wouldn't it be better to
> simply have void return type, and rely on the normaly error-handling
> mechanisms?
> Or is this inspired by the inline asm patch? (for PR 87291)
The idea is that this way the user could also create the global,
initialize it and directly use it like:
foo (gcc_jit_global_set_initializer (gcc_jit_context_new_global (...), ...))
I left it that way, let me know if you prefer otherwise.
[...]
>
>> --- a/gcc/jit/jit-playback.h
>> +++ b/gcc/jit/jit-playback.h
>> @@ -111,6 +111,15 @@ public:
>> type *type,
>> const char *name);
>>
>> + lvalue *
>> + new_global_initialized (location *loc,
>> + enum gcc_jit_global_kind kind,
>> + type *type,
>> + size_t element_size,
>> + size_t initializer_num_elem,
>> + const void *initializer,
>> + const char *name);
>> +
>> template <typename HOST_TYPE>
>> rvalue *
>> new_rvalue_from_const (type *type,
>> @@ -266,6 +275,14 @@ private:
>> const char * get_path_s_file () const;
>> const char * get_path_so_file () const;
>>
>> + tree
>> + global_new_decl (location *loc,
>> + enum gcc_jit_global_kind kind,
>> + type *type,
>> + const char *name);
>> + lvalue *
>> + global_finalize_lvalue (tree inner);
>> +
>> private:
>>
>> /* Functions for implementing "compile". */
>> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
>> index 0fddf04da873..52fc92f5928c 100644
>> --- a/gcc/jit/jit-playback.c
>> +++ b/gcc/jit/jit-playback.c
>> @@ -510,14 +510,14 @@ new_function (location *loc,
>> return func;
>> }
>>
>> -/* Construct a playback::lvalue instance (wrapping a tree). */
>> +/* In use by new_global and new_global_initialized. */
>>
>> -playback::lvalue *
>> +tree
>> playback::context::
>> -new_global (location *loc,
>> - enum gcc_jit_global_kind kind,
>> - type *type,
>> - const char *name)
>> +global_new_decl (location *loc,
>> + enum gcc_jit_global_kind kind,
>> + type *type,
>> + const char *name)
>> {
>> gcc_assert (type);
>> gcc_assert (name);
>> @@ -547,6 +547,15 @@ new_global (location *loc,
>> if (loc)
>> set_tree_location (inner, loc);
>>
>> + return inner;
>> +}
>> +
>> +/* In use by new_global and new_global_initialized. */
>> +
>> +playback::lvalue *
>> +playback::context::
>> +global_finalize_lvalue (tree inner)
>> +{
>> varpool_node::get_create (inner);
>>
>> varpool_node::finalize_decl (inner);
>> @@ -556,6 +565,90 @@ new_global (location *loc,
>> return new lvalue (this, inner);
>> }
>>
>> +/* Construct a playback::lvalue instance (wrapping a tree). */
>> +
>> +playback::lvalue *
>> +playback::context::
>> +new_global (location *loc,
>> + enum gcc_jit_global_kind kind,
>> + type *type,
>> + const char *name)
>> +{
>> + tree inner = global_new_decl (loc, kind, type, name);
>> +
>> + return global_finalize_lvalue (inner);
>> +}
>> +
>> +/* Fill 'constructor_elements' with the memory content of
>> + 'initializer'. Each element of the initializer is of the size of
>> + type T. In use by new_global_initialized.*/
>> +
>> +template<typename T>
>> +static void
>> +load_blob_in_ctor (vec<constructor_elt, va_gc> *&constructor_elements,
>> + size_t num_elem,
>> + const void *initializer)
>> +{
>> + /* Loosely based on 'output_init_element' c-typeck.c:9691. */
>> + const T *p = (const T *)initializer;
>> + tree node = make_unsigned_type (BITS_PER_UNIT * sizeof (T));
>> + for (size_t i = 0; i < num_elem; i++)
>> + {
>> + constructor_elt celt =
>> + { build_int_cst (long_unsigned_type_node, i),
>> + build_int_cst (node, p[i]) };
>> + vec_safe_push (constructor_elements, celt);
>> + }
>> +}
>> +
>> +/* Construct an initialized playback::lvalue instance (wrapping a
>> + tree). */
>> +
>> +playback::lvalue *
>> +playback::context::
>> +new_global_initialized (location *loc,
>> + enum gcc_jit_global_kind kind,
>> + type *type,
>> + size_t element_size,
>> + size_t initializer_num_elem,
>> + const void *initializer,
>> + const char *name)
>> +{
>> + tree inner = global_new_decl (loc, kind, type, name);
>> +
>> + static vec<constructor_elt, va_gc> *constructor_elements;
>
> Why the use of a function-level static variable here, and why va_gc?
> Wouldn't an auto_vec<constructor_elt> be cleaner?
> Ah: is it because of the call to build_constructor?
> If so, can the "static" be removed and replaced with an "= NULL;"
> initializer?
>
> (I'm very wary of function-level static variables, as they're a place
> when state can "hide" between multiple in-process invocations of the
> compiler)
Sorry that's due to my ignorance on how GCC GC works, I thought GC roots
had to be static, fixed.
[...]
>> private:
>> string * make_debug_string () FINAL OVERRIDE { return m_name; }
>> + template <typename T>
>> + void write_initializer_reproducer (const char *id, reproducer &r);
>> void write_reproducer (reproducer &r) FINAL OVERRIDE;
>> enum precedence get_precedence () const FINAL OVERRIDE
>> {
>> @@ -1346,6 +1370,8 @@ private:
>> private:
>> enum gcc_jit_global_kind m_kind;
>> string *m_name;
>> + void *m_initializer = NULL;
>> + size_t m_initializer_num_bytes = 0;
>> };
>
> Are we still targetting C++98, and are these initializers compatible
> with it?
Apparently C++03 but AFAIU this should be compatible only with C++11 on,
moved the zeroing into the ctor.
>> template <typename HOST_TYPE>
>> diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
>> index b73cd76a0a02..f97de00f63c3 100644
>> --- a/gcc/jit/jit-recording.c
>> +++ b/gcc/jit/jit-recording.c
>
> [...]
>
>> @@ -4440,9 +4510,26 @@ recording::global::write_to_dump (dump &d)
>> d.write ("extern ");
>> break;
>> }
>> - d.write ("%s %s;\n",
>> +
>> + d.write ("%s %s",
>> m_type->get_debug_string (),
>> get_debug_string ());
>> +
>> + if (!m_initializer)
>> + {
>> + d.write (";\n");
>> + return;
>> + }
>> +
>> + d.write ("=\n { ");
>> + const char *p = (const char *)m_initializer;
>
> Should be "const unsigned char"?
Probably better, fixed.
>> + for (size_t i = 0; i < m_initializer_num_bytes; i++)
>> + {
>> + d.write ("0x%x, ", p[i]);
>> + if (i && !(i % 64))
>> + d.write ("\n ");
>> + }
>> + d.write ("};\n");
>> }
>>
>> /* A table of enum gcc_jit_global_kind values expressed in string
>> @@ -4454,6 +4541,27 @@ static const char * const global_kind_reproducer_strings[] = {
>> "GCC_JIT_GLOBAL_IMPORTED"
>> };
[...]
>
> Thanks again for the patch; hope this is constructive
> Dave
Sure it is, thanks for reviewing.
Attached the updated version of the patch.
make check-jit is clean plus I tested the new entry point with the
modified Emacs.
Thanks
Andrea
[-- Attachment #2: 0001-Add-new-gcc_jit_global_set_initializer-entry-point.patch --]
[-- Type: text/plain, Size: 24798 bytes --]
From 74970a5ac988e8a641967dee39ec8fa711228a91 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Sat, 30 May 2020 10:33:08 +0100
Subject: [PATCH] Add new gcc_jit_global_set_initializer entry point
gcc/jit/ChangeLog
2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
* docs/topics/compatibility.rst (LIBGCCJIT_ABI_14): New ABI tag.
* docs/topics/expressions.rst (gcc_jit_global_set_initializer):
Document new entry point in section 'Global variables'.
* jit-playback.c (global_new_decl, global_finalize_lvalue): New
method.
(playback::context::new_global): Make use of global_new_decl,
global_finalize_lvalue.
(load_blob_in_ctor): New template function in use by the
following.
(playback::context::new_global_initialized): New method.
* jit-playback.h (class context): Decl 'new_global_initialized',
'global_new_decl', 'global_finalize_lvalue'.
(lvalue::set_initializer): Add implementation.
* jit-recording.c (recording::memento_of_get_pointer::get_size)
(recording::memento_of_get_type::get_size): Add implementation.
(recording::global::write_initializer_reproducer): New function in
use by 'recording::global::write_reproducer'.
(recording::global::replay_into)
(recording::global::write_to_dump)
(recording::global::write_reproducer): Handle
initialized case.
* jit-recording.h (class type): Decl 'get_size' and
'num_elements'.
* libgccjit++.h (class lvalue): Declare new 'set_initializer'
method.
(class lvalue): Decl 'is_global' and 'set_initializer'.
(class global) Decl 'write_initializer_reproducer'. Add
'm_initializer', 'm_initializer_num_bytes' fields. Implement
'set_initializer'. Add a destructor to free 'm_initializer'.
* libgccjit.c (gcc_jit_global_set_initializer): New function.
* libgccjit.h (gcc_jit_global_set_initializer): New function
declaration.
* libgccjit.map (LIBGCCJIT_ABI_14): New ABI tag.
gcc/testsuite/ChangeLog
2020-08-01 Andrea Corallo <andrea.corallo@arm.com>
* jit.dg/all-non-failing-tests.h: Add test-blob.c.
* jit.dg/test-global-set-initializer.c: New testcase.
---
gcc/jit/docs/topics/compatibility.rst | 7 +
gcc/jit/docs/topics/expressions.rst | 21 +++
gcc/jit/jit-playback.c | 105 +++++++++++++-
gcc/jit/jit-playback.h | 17 +++
gcc/jit/jit-recording.c | 137 +++++++++++++++++-
gcc/jit/jit-recording.h | 38 ++++-
gcc/jit/libgccjit++.h | 10 ++
gcc/jit/libgccjit.c | 38 +++++
gcc/jit/libgccjit.h | 14 ++
gcc/jit/libgccjit.map | 7 +-
gcc/testsuite/jit.dg/all-non-failing-tests.h | 7 +
.../jit.dg/test-global-set-initializer.c | 78 ++++++++++
12 files changed, 466 insertions(+), 13 deletions(-)
create mode 100644 gcc/testsuite/jit.dg/test-global-set-initializer.c
diff --git a/gcc/jit/docs/topics/compatibility.rst b/gcc/jit/docs/topics/compatibility.rst
index bb3387fa583..6bfa101ed71 100644
--- a/gcc/jit/docs/topics/compatibility.rst
+++ b/gcc/jit/docs/topics/compatibility.rst
@@ -219,3 +219,10 @@ entrypoints:
* :func:`gcc_jit_version_minor`
* :func:`gcc_jit_version_patchlevel`
+
+.. _LIBGCCJIT_ABI_14:
+
+``LIBGCCJIT_ABI_14``
+--------------------
+``LIBGCCJIT_ABI_14`` covers the addition of
+:func:`gcc_jit_global_set_initializer`
diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
index d783ceea51a..28f81be0060 100644
--- a/gcc/jit/docs/topics/expressions.rst
+++ b/gcc/jit/docs/topics/expressions.rst
@@ -582,6 +582,27 @@ Global variables
referring to it. Analogous to using an "extern" global from a
header file.
+.. function:: gcc_jit_lvalue *\
+ gcc_jit_global_set_initializer (gcc_jit_lvalue *global,\
+ const void *blob,\
+ size_t num_bytes)
+
+ Set an initializer for an object using the memory content pointed
+ by ``blob`` for ``num_bytes``. ``global`` must be an array of an
+ integral type.
+
+ The parameter ``blob`` must be non-NULL. The call copies the memory
+ pointed by ``blob`` for ``num_bytes`` bytes, so it is valid to pass
+ in a pointer to an on-stack buffer. The content will be stored in
+ the compilation unit and used as initialization value of the array.
+
+ This entrypoint was added in :ref:`LIBGCCJIT_ABI_14`; you can test for
+ its presence using
+
+ .. code-block:: c
+
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
+
Working with pointers, structs and unions
-----------------------------------------
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 0fddf04da87..b68e18278e3 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -510,14 +510,14 @@ new_function (location *loc,
return func;
}
-/* Construct a playback::lvalue instance (wrapping a tree). */
+/* In use by new_global and new_global_initialized. */
-playback::lvalue *
+tree
playback::context::
-new_global (location *loc,
- enum gcc_jit_global_kind kind,
- type *type,
- const char *name)
+global_new_decl (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name)
{
gcc_assert (type);
gcc_assert (name);
@@ -547,6 +547,15 @@ new_global (location *loc,
if (loc)
set_tree_location (inner, loc);
+ return inner;
+}
+
+/* In use by new_global and new_global_initialized. */
+
+playback::lvalue *
+playback::context::
+global_finalize_lvalue (tree inner)
+{
varpool_node::get_create (inner);
varpool_node::finalize_decl (inner);
@@ -556,6 +565,90 @@ new_global (location *loc,
return new lvalue (this, inner);
}
+/* Construct a playback::lvalue instance (wrapping a tree). */
+
+playback::lvalue *
+playback::context::
+new_global (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name)
+{
+ tree inner = global_new_decl (loc, kind, type, name);
+
+ return global_finalize_lvalue (inner);
+}
+
+/* Fill 'constructor_elements' with the memory content of
+ 'initializer'. Each element of the initializer is of the size of
+ type T. In use by new_global_initialized.*/
+
+template<typename T>
+static void
+load_blob_in_ctor (vec<constructor_elt, va_gc> *&constructor_elements,
+ size_t num_elem,
+ const void *initializer)
+{
+ /* Loosely based on 'output_init_element' c-typeck.c:9691. */
+ const T *p = (const T *)initializer;
+ tree node = make_unsigned_type (BITS_PER_UNIT * sizeof (T));
+ for (size_t i = 0; i < num_elem; i++)
+ {
+ constructor_elt celt =
+ { build_int_cst (long_unsigned_type_node, i),
+ build_int_cst (node, p[i]) };
+ vec_safe_push (constructor_elements, celt);
+ }
+}
+
+/* Construct an initialized playback::lvalue instance (wrapping a
+ tree). */
+
+playback::lvalue *
+playback::context::
+new_global_initialized (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ size_t element_size,
+ size_t initializer_num_elem,
+ const void *initializer,
+ const char *name)
+{
+ tree inner = global_new_decl (loc, kind, type, name);
+
+ vec<constructor_elt, va_gc> *constructor_elements = NULL;
+
+ switch (element_size)
+ {
+ case 1:
+ load_blob_in_ctor<uint8_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ case 2:
+ load_blob_in_ctor<uint16_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ case 4:
+ load_blob_in_ctor<uint32_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ case 8:
+ load_blob_in_ctor<uint64_t> (constructor_elements, initializer_num_elem,
+ initializer);
+ break;
+ default:
+ gcc_unreachable ();
+ }
+ /* Compare with 'pop_init_level' c-typeck.c:8780. */
+ tree ctor = build_constructor (type->as_tree (), constructor_elements);
+ constructor_elements = NULL;
+
+ /* Compare with 'store_init_value' c-typeck.c:7555. */
+ DECL_INITIAL (inner) = ctor;
+
+ return global_finalize_lvalue (inner);
+}
+
/* Implementation of the various
gcc::jit::playback::context::new_rvalue_from_const <HOST_TYPE>
methods.
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index f9b3e675368..50b69753bb4 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -111,6 +111,15 @@ public:
type *type,
const char *name);
+ lvalue *
+ new_global_initialized (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ size_t element_size,
+ size_t initializer_num_elem,
+ const void *initializer,
+ const char *name);
+
template <typename HOST_TYPE>
rvalue *
new_rvalue_from_const (type *type,
@@ -266,6 +275,14 @@ private:
const char * get_path_s_file () const;
const char * get_path_so_file () const;
+ tree
+ global_new_decl (location *loc,
+ enum gcc_jit_global_kind kind,
+ type *type,
+ const char *name);
+ lvalue *
+ global_finalize_lvalue (tree inner);
+
private:
/* Functions for implementing "compile". */
diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
index b73cd76a0a0..39510097c6f 100644
--- a/gcc/jit/jit-recording.c
+++ b/gcc/jit/jit-recording.c
@@ -2175,6 +2175,57 @@ recording::type::access_as_type (reproducer &r)
return r.get_identifier (this);
}
+/* Override of default implementation of
+ recording::type::get_size.
+
+ Return the size in bytes. This is in use for global
+ initialization. */
+
+size_t
+recording::memento_of_get_type::get_size ()
+{
+ int size;
+ switch (m_kind)
+ {
+ case GCC_JIT_TYPE_VOID:
+ return 0;
+ case GCC_JIT_TYPE_BOOL:
+ case GCC_JIT_TYPE_CHAR:
+ case GCC_JIT_TYPE_SIGNED_CHAR:
+ case GCC_JIT_TYPE_UNSIGNED_CHAR:
+ return 1;
+ case GCC_JIT_TYPE_SHORT:
+ case GCC_JIT_TYPE_UNSIGNED_SHORT:
+ size = SHORT_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_INT:
+ case GCC_JIT_TYPE_UNSIGNED_INT:
+ size = INT_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_LONG:
+ case GCC_JIT_TYPE_UNSIGNED_LONG:
+ size = LONG_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_LONG_LONG:
+ case GCC_JIT_TYPE_UNSIGNED_LONG_LONG:
+ size = LONG_LONG_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_FLOAT:
+ size = FLOAT_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_DOUBLE:
+ size = DOUBLE_TYPE_SIZE;
+ break;
+ case GCC_JIT_TYPE_LONG_DOUBLE:
+ size = LONG_DOUBLE_TYPE_SIZE;
+ break;
+ default:
+ gcc_unreachable ();
+ }
+
+ return size / BITS_PER_UNIT;
+}
+
/* Implementation of pure virtual hook recording::type::dereference for
recording::memento_of_get_type. */
@@ -2482,6 +2533,15 @@ recording::memento_of_get_type::write_reproducer (reproducer &r)
/* The implementation of class gcc::jit::recording::memento_of_get_pointer. */
+/* Override of default implementation of
+ recording::type::get_size for get_pointer. */
+
+size_t
+recording::memento_of_get_pointer::get_size ()
+{
+ return POINTER_SIZE / BITS_PER_UNIT;
+}
+
/* Override of default implementation of
recording::type::accepts_writes_from for get_pointer.
@@ -4393,10 +4453,20 @@ recording::block::dump_edges_to_dot (pretty_printer *pp)
void
recording::global::replay_into (replayer *r)
{
- set_playback_obj (r->new_global (playback_location (r, m_loc),
- m_kind,
- m_type->playback_type (),
- playback_string (m_name)));
+ set_playback_obj (
+ m_initializer
+ ? r->new_global_initialized (playback_location (r, m_loc),
+ m_kind,
+ m_type->playback_type (),
+ m_type->dereference ()->get_size (),
+ m_initializer_num_bytes
+ / m_type->dereference ()->get_size (),
+ m_initializer,
+ playback_string (m_name))
+ : r->new_global (playback_location (r, m_loc),
+ m_kind,
+ m_type->playback_type (),
+ playback_string (m_name)));
}
/* Override the default implementation of
@@ -4440,9 +4510,26 @@ recording::global::write_to_dump (dump &d)
d.write ("extern ");
break;
}
- d.write ("%s %s;\n",
+
+ d.write ("%s %s",
m_type->get_debug_string (),
get_debug_string ());
+
+ if (!m_initializer)
+ {
+ d.write (";\n");
+ return;
+ }
+
+ d.write ("=\n { ");
+ const unsigned char *p = (const unsigned char *)m_initializer;
+ for (size_t i = 0; i < m_initializer_num_bytes; i++)
+ {
+ d.write ("0x%x, ", p[i]);
+ if (i && !(i % 64))
+ d.write ("\n ");
+ }
+ d.write ("};\n");
}
/* A table of enum gcc_jit_global_kind values expressed in string
@@ -4454,6 +4541,27 @@ static const char * const global_kind_reproducer_strings[] = {
"GCC_JIT_GLOBAL_IMPORTED"
};
+template <typename T>
+void
+recording::global::write_initializer_reproducer (const char *id, reproducer &r)
+{
+ const char *init_id = r.make_tmp_identifier ("init_for", this);
+ r.write (" %s %s[] =\n {",
+ m_type->dereference ()->get_debug_string (),
+ init_id);
+
+ const T *p = (const T *)m_initializer;
+ for (size_t i = 0; i < m_initializer_num_bytes / sizeof (T); i++)
+ {
+ r.write ("%" PRIu64 ", ", (uint64_t)p[i]);
+ if (i && !(i % 64))
+ r.write ("\n ");
+ }
+ r.write ("};\n");
+ r.write (" gcc_jit_global_set_initializer (%s, %s, sizeof (%s));\n",
+ id, init_id, init_id);
+}
+
/* Implementation of recording::memento::write_reproducer for globals. */
void
@@ -4472,6 +4580,25 @@ recording::global::write_reproducer (reproducer &r)
global_kind_reproducer_strings[m_kind],
r.get_identifier_as_type (get_type ()),
m_name->get_debug_string ());
+
+ if (m_initializer)
+ switch (m_type->dereference ()->get_size ())
+ {
+ case 1:
+ write_initializer_reproducer<uint8_t> (id, r);
+ break;
+ case 2:
+ write_initializer_reproducer<uint16_t> (id, r);
+ break;
+ case 4:
+ write_initializer_reproducer<uint32_t> (id, r);
+ break;
+ case 8:
+ write_initializer_reproducer<uint64_t> (id, r);
+ break;
+ default:
+ gcc_unreachable ();
+ }
}
/* The implementation of the various const-handling classes:
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 726b9c4b837..aa38e606a8d 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -502,6 +502,12 @@ public:
This will return NULL if it's not valid to dereference this type.
The caller is responsible for setting an error. */
virtual type *dereference () = 0;
+ /* Get the type size in bytes.
+
+ This is implemented only for memento_of_get_type and
+ memento_of_get_pointer as is use for initializing globals of
+ these types. */
+ virtual size_t get_size () { gcc_unreachable (); }
/* Dynamic casts. */
virtual function_type *dyn_cast_function_type () { return NULL; }
@@ -569,6 +575,8 @@ public:
type *dereference () FINAL OVERRIDE;
+ size_t get_size () FINAL OVERRIDE;
+
bool accepts_writes_from (type *rtype) FINAL OVERRIDE
{
if (m_kind == GCC_JIT_TYPE_VOID_PTR)
@@ -610,6 +618,8 @@ public:
type *dereference () FINAL OVERRIDE { return m_other_type; }
+ size_t get_size () FINAL OVERRIDE;
+
bool accepts_writes_from (type *rtype) FINAL OVERRIDE;
void replay_into (replayer *r) FINAL OVERRIDE;
@@ -755,6 +765,7 @@ class array_type : public type
bool is_bool () const FINAL OVERRIDE { return false; }
type *is_pointer () FINAL OVERRIDE { return NULL; }
type *is_array () FINAL OVERRIDE { return m_element_type; }
+ int num_elements () { return m_num_elements; }
void replay_into (replayer *) FINAL OVERRIDE;
@@ -1107,6 +1118,7 @@ public:
const char *access_as_rvalue (reproducer &r) OVERRIDE;
virtual const char *access_as_lvalue (reproducer &r);
+ virtual bool is_global () const { return false; }
};
class param : public lvalue
@@ -1327,7 +1339,14 @@ public:
: lvalue (ctxt, loc, type),
m_kind (kind),
m_name (name)
- {}
+ {
+ m_initializer = NULL;
+ m_initializer_num_bytes = 0;
+ }
+ ~global ()
+ {
+ free (m_initializer);
+ }
void replay_into (replayer *) FINAL OVERRIDE;
@@ -1335,8 +1354,23 @@ public:
void write_to_dump (dump &d) FINAL OVERRIDE;
+ bool is_global () const FINAL OVERRIDE { return true; }
+
+ void
+ set_initializer (const void *initializer,
+ size_t num_bytes)
+ {
+ if (m_initializer)
+ free (m_initializer);
+ m_initializer = xmalloc (num_bytes);
+ memcpy (m_initializer, initializer, num_bytes);
+ m_initializer_num_bytes = num_bytes;
+ }
+
private:
string * make_debug_string () FINAL OVERRIDE { return m_name; }
+ template <typename T>
+ void write_initializer_reproducer (const char *id, reproducer &r);
void write_reproducer (reproducer &r) FINAL OVERRIDE;
enum precedence get_precedence () const FINAL OVERRIDE
{
@@ -1346,6 +1380,8 @@ private:
private:
enum gcc_jit_global_kind m_kind;
string *m_name;
+ void *m_initializer;
+ size_t m_initializer_num_bytes;
};
template <typename HOST_TYPE>
diff --git a/gcc/jit/libgccjit++.h b/gcc/jit/libgccjit++.h
index 69e67766640..1b9ef1a5db9 100644
--- a/gcc/jit/libgccjit++.h
+++ b/gcc/jit/libgccjit++.h
@@ -488,6 +488,7 @@ namespace gccjit
location loc = location ());
rvalue get_address (location loc = location ());
+ lvalue set_initializer (const void *blob, size_t num_bytes);
};
class param : public lvalue
@@ -1737,6 +1738,15 @@ lvalue::get_address (location loc)
loc.get_inner_location ()));
}
+inline lvalue
+lvalue::set_initializer (const void *blob, size_t num_bytes)
+{
+ gcc_jit_global_set_initializer (get_inner_lvalue (),
+ blob,
+ num_bytes);
+ return *this;
+}
+
// class param : public lvalue
inline param::param () : lvalue () {}
inline param::param (gcc_jit_param *inner)
diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
index 3d04f6db3af..bc45bd80b2c 100644
--- a/gcc/jit/libgccjit.c
+++ b/gcc/jit/libgccjit.c
@@ -1117,6 +1117,44 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
return (gcc_jit_lvalue *)ctxt->new_global (loc, kind, type, name);
}
+/* Public entrypoint. See description in libgccjit.h.
+
+ After error-checking, the real work is done by the
+ gcc::jit::recording::global::set_initializer method, in
+ jit-recording.c. */
+
+extern gcc_jit_lvalue *
+gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
+ const void *blob,
+ size_t num_bytes)
+{
+ RETURN_NULL_IF_FAIL (global, NULL, NULL, "NULL global");
+ RETURN_NULL_IF_FAIL (blob, NULL, NULL, "NULL blob");
+ RETURN_NULL_IF_FAIL_PRINTF1 (global->is_global (), NULL, NULL,
+ "lvalue \"%s\" not a global",
+ global->get_debug_string ());
+
+ gcc::jit::recording::type *lval_type = global->get_type ();
+ RETURN_NULL_IF_FAIL_PRINTF1 (lval_type->is_array (), NULL, NULL,
+ "global \"%s\" is not an array",
+ global->get_debug_string ());
+ RETURN_NULL_IF_FAIL_PRINTF1 (lval_type->dereference ()->is_int (), NULL, NULL,
+ "global \"%s\" is not an array of integral type",
+ global->get_debug_string ());
+ size_t lvalue_size =
+ lval_type->dereference ()->get_size ()
+ * static_cast <gcc::jit::recording::array_type *> (lval_type)->num_elements ();
+ RETURN_NULL_IF_FAIL_PRINTF3 (
+ lvalue_size == num_bytes, NULL, NULL,
+ "global \"%s\" of size %zu don't match initializer of size %zu",
+ global->get_debug_string (), lvalue_size, num_bytes);
+
+ reinterpret_cast <gcc::jit::recording::global *> (global)
+ ->set_initializer (blob, num_bytes);
+
+ return global;
+}
+
/* Public entrypoint. See description in libgccjit.h.
After error-checking, this calls the trivial
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 1c5a12e9c01..3cbcbef2693 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -788,6 +788,20 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
gcc_jit_type *type,
const char *name);
+#define LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
+
+/* Set a static initializer for a global return the global itself.
+
+ This API entrypoint was added in LIBGCCJIT_ABI_14; you can test for its
+ presence using
+ #ifdef LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
+*/
+
+extern gcc_jit_lvalue *
+gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
+ const void *blob,
+ size_t num_bytes);
+
/* Upcasting. */
extern gcc_jit_object *
gcc_jit_lvalue_as_object (gcc_jit_lvalue *lvalue);
diff --git a/gcc/jit/libgccjit.map b/gcc/jit/libgccjit.map
index 6137dd4b4b0..a6e67e781a4 100644
--- a/gcc/jit/libgccjit.map
+++ b/gcc/jit/libgccjit.map
@@ -186,4 +186,9 @@ LIBGCCJIT_ABI_13 {
gcc_jit_version_major;
gcc_jit_version_minor;
gcc_jit_version_patchlevel;
-} LIBGCCJIT_ABI_12;
\ No newline at end of file
+} LIBGCCJIT_ABI_12;
+
+LIBGCCJIT_ABI_14 {
+ global:
+ gcc_jit_global_set_initializer;
+} LIBGCCJIT_ABI_13;
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 632ab8cfb2e..4202eb7798b 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -174,6 +174,13 @@
#undef create_code
#undef verify_code
+/* test-global-set-initializer.c */
+#define create_code create_code_global_set_initializer
+#define verify_code verify_code_global_set_initializer
+#include "test-global-set-initializer.c"
+#undef create_code
+#undef verify_code
+
/* test-hello-world.c */
#define create_code create_code_hello_world
#define verify_code verify_code_hello_world
diff --git a/gcc/testsuite/jit.dg/test-global-set-initializer.c b/gcc/testsuite/jit.dg/test-global-set-initializer.c
new file mode 100644
index 00000000000..d38aba7d73f
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-global-set-initializer.c
@@ -0,0 +1,78 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <time.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+#define BIG_BLOB_SIZE (1 << 12) /* 4KB. */
+
+static signed char test_blob1[] = { 0xc, 0xa, 0xf, 0xf, 0xe };
+static unsigned test_blob2[] = { 0x3, 0x2, 0x1, 0x0, 0x1, 0x2, 0x3 };
+static unsigned char test_blob3[BIG_BLOB_SIZE];
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+
+ signed char bin_blob1[] = { 0xc, 0xa, 0xf, 0xf, 0xe };
+ unsigned bin_blob2[] = { 0x3, 0x2, 0x1, 0x0, 0x1, 0x2, 0x3 };
+ unsigned char bin_blob3[4096]...
+ */
+ gcc_jit_type *unsigned_char_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_UNSIGNED_CHAR);
+ gcc_jit_type *signed_char_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_SIGNED_CHAR);
+ gcc_jit_type *unsigned_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_UNSIGNED_INT);
+
+ gcc_jit_lvalue *glob =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED,
+ gcc_jit_context_new_array_type (ctxt, NULL, signed_char_type,
+ sizeof (test_blob1)),
+ "bin_blob1");
+ gcc_jit_global_set_initializer (glob, test_blob1, sizeof (test_blob1));
+
+ glob =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED,
+ gcc_jit_context_new_array_type (
+ ctxt, NULL, unsigned_type,
+ sizeof (test_blob2) / sizeof (*test_blob2)),
+ "bin_blob2");
+ gcc_jit_global_set_initializer (glob, test_blob2,
+ sizeof (test_blob2));
+
+ for (size_t i = 0; i < BIG_BLOB_SIZE; i++)
+ test_blob3[i] = i * i + i;
+ glob =
+ gcc_jit_context_new_global (
+ ctxt, NULL, GCC_JIT_GLOBAL_EXPORTED,
+ gcc_jit_context_new_array_type (ctxt, NULL, unsigned_char_type,
+ sizeof (test_blob3)),
+ "bin_blob3");
+ gcc_jit_global_set_initializer (glob, test_blob3, sizeof (test_blob3));
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ CHECK_NON_NULL (result);
+ void *glob = gcc_jit_result_get_global (result, "bin_blob1");
+ CHECK_NON_NULL (glob);
+ CHECK_VALUE (memcmp (test_blob1, glob, sizeof (test_blob1)), 0);
+
+ glob = gcc_jit_result_get_global (result, "bin_blob2");
+ CHECK_NON_NULL (glob);
+ CHECK_VALUE (memcmp (test_blob2, glob,
+ sizeof (test_blob2)), 0);
+
+ glob = gcc_jit_result_get_global (result, "bin_blob3");
+ CHECK_NON_NULL (glob);
+ CHECK_VALUE (memcmp (test_blob3, glob, sizeof (test_blob3)), 0);
+
+}
--
2.20.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-08-19 7:17 ` [PATCH V2] " Andrea Corallo
@ 2020-09-09 7:51 ` Andrea Corallo
2020-09-10 22:22 ` David Malcolm
1 sibling, 0 replies; 14+ messages in thread
From: Andrea Corallo @ 2020-09-09 7:51 UTC (permalink / raw)
To: David Malcolm; +Cc: jit, nd, gcc-patches
Andrea Corallo <andrea.corallo@arm.com> writes:
[...]
> Sure it is, thanks for reviewing.
>
> Attached the updated version of the patch.
>
> make check-jit is clean plus I tested the new entry point with the
> modified Emacs.
>
> Thanks
>
> Andrea
Ping
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-08-19 7:17 ` [PATCH V2] " Andrea Corallo
2020-09-09 7:51 ` Andrea Corallo
@ 2020-09-10 22:22 ` David Malcolm
2020-09-11 10:31 ` Andrea Corallo
1 sibling, 1 reply; 14+ messages in thread
From: David Malcolm @ 2020-09-10 22:22 UTC (permalink / raw)
To: Andrea Corallo; +Cc: jit, nd, gcc-patches
On Wed, 2020-08-19 at 09:17 +0200, Andrea Corallo wrote:
> David Malcolm <dmalcolm@redhat.com> writes:
>
> > Thanks for the updated patch. Comments inline below.
>
> Hi Dave,
>
> sorry for the late reply.
Likewise, sorry.
[...]
> > Why the non-void return type? Looking at libgccjit.c I see it returns
> > "global" if it succeeds, or NULL if it fails. Wouldn't it be better to
> > simply have void return type, and rely on the normaly error-handling
> > mechanisms?
> > Or is this inspired by the inline asm patch? (for PR 87291)
>
> The idea is that this way the user could also create the global,
> initialize it and directly use it like:
>
> foo (gcc_jit_global_set_initializer (gcc_jit_context_new_global (...), ...))
>
> I left it that way, let me know if you prefer otherwise.
I don't have a strong preference here, so let's go with what you've
written.
[...]
> >> +/* Construct an initialized playback::lvalue instance (wrapping a
> >> + tree). */
> >> +
> >> +playback::lvalue *
> >> +playback::context::
> >> +new_global_initialized (location *loc,
> >> + enum gcc_jit_global_kind kind,
> >> + type *type,
> >> + size_t element_size,
> >> + size_t initializer_num_elem,
> >> + const void *initializer,
> >> + const char *name)
> >> +{
> >> + tree inner = global_new_decl (loc, kind, type, name);
> >> +
> >> + static vec<constructor_elt, va_gc> *constructor_elements;
> >
> > Why the use of a function-level static variable here, and why va_gc?
> > Wouldn't an auto_vec<constructor_elt> be cleaner?
> > Ah: is it because of the call to build_constructor?
> > If so, can the "static" be removed and replaced with an "= NULL;"
> > initializer?
> >
> > (I'm very wary of function-level static variables, as they're a place
> > when state can "hide" between multiple in-process invocations of the
> > compiler)
>
> Sorry that's due to my ignorance on how GCC GC works, I thought GC roots
> had to be static, fixed.
FWIW I don't think this is a GC root; the only GTY-marked roots in
gcc/jit are those in dummy-frontend.c
Looking at gcc/jit/notes.txt, this playback code is called in the "No
GC in here" region within jit_langhook_parse_file.
[...]
> diff --git a/gcc/jit/docs/topics/expressions.rst b/gcc/jit/docs/topics/expressions.rst
> index d783ceea51a..28f81be0060 100644
> --- a/gcc/jit/docs/topics/expressions.rst
> +++ b/gcc/jit/docs/topics/expressions.rst
> @@ -582,6 +582,27 @@ Global variables
> referring to it. Analogous to using an "extern" global from a
> header file.
>
> +.. function:: gcc_jit_lvalue *\
> + gcc_jit_global_set_initializer (gcc_jit_lvalue *global,\
> + const void *blob,\
> + size_t num_bytes)
> +
> + Set an initializer for an object using the memory content pointed
> + by ``blob`` for ``num_bytes``. ``global`` must be an array of an
> + integral type.
> +
> + The parameter ``blob`` must be non-NULL. The call copies the memory
> + pointed by ``blob`` for ``num_bytes`` bytes, so it is valid to pass
> + in a pointer to an on-stack buffer. The content will be stored in
> + the compilation unit and used as initialization value of the array.
Please document here that the return value is "global".
[...]
> +template<typename T>
> +static void
> +load_blob_in_ctor (vec<constructor_elt, va_gc> *&constructor_elements,
> + size_t num_elem,
> + const void *initializer)
> +{
> + /* Loosely based on 'output_init_element' c-typeck.c:9691. */
> + const T *p = (const T *)initializer;
> + tree node = make_unsigned_type (BITS_PER_UNIT * sizeof (T));
> + for (size_t i = 0; i < num_elem; i++)
> + {
> + constructor_elt celt =
> + { build_int_cst (long_unsigned_type_node, i),
> + build_int_cst (node, p[i]) };
> + vec_safe_push (constructor_elements, celt);
In theory it would be slightly quicker to grow the vector once, and use
vec_quick_push inside the loop, but given how much allocation we're
doing it may be lost in the noise.
> + }
> +}
> +
> +/* Construct an initialized playback::lvalue instance (wrapping a
> + tree). */
> +
> +playback::lvalue *
> +playback::context::
> +new_global_initialized (location *loc,
> + enum gcc_jit_global_kind kind,
> + type *type,
> + size_t element_size,
> + size_t initializer_num_elem,
> + const void *initializer,
> + const char *name)
> +{
> + tree inner = global_new_decl (loc, kind, type, name);
> +
> + vec<constructor_elt, va_gc> *constructor_elements = NULL;
> +
> + switch (element_size)
> + {
> + case 1:
> + load_blob_in_ctor<uint8_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + case 2:
> + load_blob_in_ctor<uint16_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + case 4:
> + load_blob_in_ctor<uint32_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + case 8:
> + load_blob_in_ctor<uint64_t> (constructor_elements, initializer_num_elem,
> + initializer);
> + break;
> + default:
> + gcc_unreachable ();
Is there a way to hit this gcc_unreachable?
If I'm reading it right, presumably this is only going to be called on
the result of get_size, and this can only return the sizes covered by
the cases, right? If so, please add a comment to this effect.
[...]
> diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c
> index b73cd76a0a0..39510097c6f 100644
> --- a/gcc/jit/jit-recording.c
> +++ b/gcc/jit/jit-recording.c
> @@ -2175,6 +2175,57 @@ recording::type::access_as_type (reproducer &r)
> return r.get_identifier (this);
> }
>
> +/* Override of default implementation of
> + recording::type::get_size.
> +
> + Return the size in bytes. This is in use for global
> + initialization. */
> +
> +size_t
> +recording::memento_of_get_type::get_size ()
> +{
> + int size;
> + switch (m_kind)
> + {
> + case GCC_JIT_TYPE_VOID:
> + return 0;
> + case GCC_JIT_TYPE_BOOL:
> + case GCC_JIT_TYPE_CHAR:
> + case GCC_JIT_TYPE_SIGNED_CHAR:
> + case GCC_JIT_TYPE_UNSIGNED_CHAR:
> + return 1;
> + case GCC_JIT_TYPE_SHORT:
> + case GCC_JIT_TYPE_UNSIGNED_SHORT:
> + size = SHORT_TYPE_SIZE;
> + break;
> + case GCC_JIT_TYPE_INT:
> + case GCC_JIT_TYPE_UNSIGNED_INT:
> + size = INT_TYPE_SIZE;
> + break;
> + case GCC_JIT_TYPE_LONG:
> + case GCC_JIT_TYPE_UNSIGNED_LONG:
> + size = LONG_TYPE_SIZE;
> + break;
> + case GCC_JIT_TYPE_LONG_LONG:
> + case GCC_JIT_TYPE_UNSIGNED_LONG_LONG:
> + size = LONG_LONG_TYPE_SIZE;
> + break;
> + case GCC_JIT_TYPE_FLOAT:
> + size = FLOAT_TYPE_SIZE;
> + break;
> + case GCC_JIT_TYPE_DOUBLE:
> + size = DOUBLE_TYPE_SIZE;
> + break;
> + case GCC_JIT_TYPE_LONG_DOUBLE:
> + size = LONG_DOUBLE_TYPE_SIZE;
> + break;
> + default:
> + gcc_unreachable ();
> + }
> +
> + return size / BITS_PER_UNIT;
> +}
> +
> /* Implementation of pure virtual hook recording::type::dereference for
> recording::memento_of_get_type. */
>
[...]
> @@ -4472,6 +4580,25 @@ recording::global::write_reproducer (reproducer &r)
> global_kind_reproducer_strings[m_kind],
> r.get_identifier_as_type (get_type ()),
> m_name->get_debug_string ());
> +
> + if (m_initializer)
> + switch (m_type->dereference ()->get_size ())
> + {
> + case 1:
> + write_initializer_reproducer<uint8_t> (id, r);
> + break;
> + case 2:
> + write_initializer_reproducer<uint16_t> (id, r);
> + break;
> + case 4:
> + write_initializer_reproducer<uint32_t> (id, r);
> + break;
> + case 8:
> + write_initializer_reproducer<uint64_t> (id, r);
> + break;
> + default:
> + gcc_unreachable ();
Similar comments as above.
> diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
> index 726b9c4b837..aa38e606a8d 100644
> --- a/gcc/jit/jit-recording.h
> +++ b/gcc/jit/jit-recording.h
> @@ -502,6 +502,12 @@ public:
> This will return NULL if it's not valid to dereference this type.
> The caller is responsible for setting an error. */
> virtual type *dereference () = 0;
> + /* Get the type size in bytes.
> +
> + This is implemented only for memento_of_get_type and
> + memento_of_get_pointer as is use for initializing globals of
"as is use" -> "as it is used"
[...]
> diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c
> index 3d04f6db3af..bc45bd80b2c 100644
> --- a/gcc/jit/libgccjit.c
> +++ b/gcc/jit/libgccjit.c
[...]
> +extern gcc_jit_lvalue *
> +gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> + const void *blob,
> + size_t num_bytes)
> +{
[...]
> + RETURN_NULL_IF_FAIL_PRINTF3 (
> + lvalue_size == num_bytes, NULL, NULL,
> + "global \"%s\" of size %zu don't match initializer of size %zu",
> + global->get_debug_string (), lvalue_size, num_bytes);
Please reword (following the pattern of the other "mismatching" errors
in libgccjit.c) to:
"mismatching sizes:"
" global \"%s\" has size %zu whereas initializer has size %zu",
global->get_debug_string (), lvalue_size, num_bytes);
[...]
> + reinterpret_cast <gcc::jit::recording::global *> (global)
> + ->set_initializer (blob, num_bytes);
Was there a reason for using reinterpret_cast here, rather than
static_cast? The file uses static_cast for this kind of thing
throughout, so I'd prefer that here.
[...]
> diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
> index 1c5a12e9c01..3cbcbef2693 100644
> --- a/gcc/jit/libgccjit.h
> +++ b/gcc/jit/libgccjit.h
> @@ -788,6 +788,20 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
> gcc_jit_type *type,
> const char *name);
>
> +#define LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
> +
> +/* Set a static initializer for a global return the global itself.
Please reword to:
/* Set an initial value for a global, which must be an array of
integral type. Return the global itself.
> +
> + This API entrypoint was added in LIBGCCJIT_ABI_14; you can test for its
> + presence using
> + #ifdef LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
> +*/
> +
> +extern gcc_jit_lvalue *
> +gcc_jit_global_set_initializer (gcc_jit_lvalue *global,
> + const void *blob,
> + size_t num_bytes);
> +
[...]
OK for master with these nits fixed, assuming your usual testing.
Thanks!
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-09-10 22:22 ` David Malcolm
@ 2020-09-11 10:31 ` Andrea Corallo
2020-09-11 11:27 ` David Malcolm
0 siblings, 1 reply; 14+ messages in thread
From: Andrea Corallo @ 2020-09-11 10:31 UTC (permalink / raw)
To: David Malcolm; +Cc: jit, nd, gcc-patches
Hi Dave,
thanks for the review!
David Malcolm <dmalcolm@redhat.com> writes:
[...]
> Was there a reason for using reinterpret_cast here, rather than
> static_cast?
Yes the reason is that apparently we can't use static_cast for that:
"error: invalid ‘static_cast’ from type ‘gcc_jit_lvalue*’ to type ‘gcc::jit::recording::global*’"
Why is that is another question I fear I'm not very qualified to answer.
If you feel is necessary and want to suggest some rework to improve this
I'll be happy to put some effort in if you like.
>> diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
>> index 1c5a12e9c01..3cbcbef2693 100644
>> --- a/gcc/jit/libgccjit.h
>> +++ b/gcc/jit/libgccjit.h
>> @@ -788,6 +788,20 @@ gcc_jit_context_new_global (gcc_jit_context *ctxt,
>> gcc_jit_type *type,
>> const char *name);
>>
>> +#define LIBGCCJIT_HAVE_gcc_jit_global_set_initializer
>> +
>> +/* Set a static initializer for a global return the global itself.
[...]
>
> OK for master with these nits fixed, assuming your usual testing.
>
> Thanks!
> Dave
After applying the suggestions and having tested it I've installed the
patch as 4ecc0061c40.
Thanks!
Andrea
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH V2] libgccjit: Add new gcc_jit_context_new_blob entry point
2020-09-11 10:31 ` Andrea Corallo
@ 2020-09-11 11:27 ` David Malcolm
0 siblings, 0 replies; 14+ messages in thread
From: David Malcolm @ 2020-09-11 11:27 UTC (permalink / raw)
To: Andrea Corallo; +Cc: jit, nd, gcc-patches
On Fri, 2020-09-11 at 12:31 +0200, Andrea Corallo wrote:
> Hi Dave,
>
> thanks for the review!
>
> David Malcolm <dmalcolm@redhat.com> writes:
>
> [...]
>
> > Was there a reason for using reinterpret_cast here, rather than
> > static_cast?
>
> Yes the reason is that apparently we can't use static_cast for that:
>
> "error: invalid ‘static_cast’ from type ‘gcc_jit_lvalue*’ to type
> ‘gcc::jit::recording::global*’"
>
> Why is that is another question I fear I'm not very qualified to
> answer.
> If you feel is necessary and want to suggest some rework to improve
> this
> I'll be happy to put some effort in if you like.
It's because we're effectively downcasting from lvalue to global. This
is safe because of the check for
global->is_global ().
A slightly cleaner way to do it would be to have a dyn_cast_global
vfunc here rather than is_global, and to use the result - but I don't
think it matters.
[...]
> > > After applying the suggestions and having tested it I've
> > > installed
> the
> patch as 4ecc0061c40.
>
> Thanks!
Thanks
Dave
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-09-11 11:27 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-03 10:11 [PATCH] libgccjit: Add new gcc_jit_context_new_blob entry point Andrea Corallo
2020-06-03 15:03 ` David Malcolm
2020-06-03 16:36 ` Andrea Corallo
2020-07-01 16:14 ` Andrea Corallo
2020-07-14 10:00 ` Andrea Corallo
2020-07-24 22:05 ` David Malcolm
2020-07-24 22:12 ` David Malcolm
2020-08-03 8:07 ` Andrea Corallo
2020-08-06 19:53 ` David Malcolm
2020-08-19 7:17 ` [PATCH V2] " Andrea Corallo
2020-09-09 7:51 ` Andrea Corallo
2020-09-10 22:22 ` David Malcolm
2020-09-11 10:31 ` Andrea Corallo
2020-09-11 11:27 ` 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).