* bug#41615: [feature/native-comp] Dump prettier C code.
@ 2020-05-30 15:08 Nicolas Bértolo
2020-05-30 22:20 ` Nicolas Bértolo
0 siblings, 1 reply; 18+ messages in thread
From: Nicolas Bértolo @ 2020-05-30 15:08 UTC (permalink / raw)
To: 41615
[-- Attachment #1.1: Type: text/plain, Size: 307 bytes --]
I found two ways to improve the looks of the dumped code.
- Define static data using string literals of at most 200 bytes. This
reduces
the line count and lets us understand what the static object consists of.
- Define casts using functions. These functions should get always get
inlined.
Thanks, Nico
[-- Attachment #1.2: Type: text/html, Size: 402 bytes --]
[-- Attachment #2: 0002-Define-casts-using-functions.patch --]
[-- Type: application/octet-stream, Size: 11727 bytes --]
From 8ed42509e55e78e67d9989e4c119c1b8f6bea463 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Wed, 20 May 2020 00:46:30 -0300
Subject: [PATCH 2/2] Define casts using functions.
This is to dump prettier C files.
This does not affect compilation times in my tests.
* src/comp.c: Define a 15x15 cast matrix. Use it in emit_coerce().
---
src/comp.c | 278 ++++++++++++++++++++---------------------------------
1 file changed, 104 insertions(+), 174 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index 3f4b3e03b7..f5f6b1baca 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -454,6 +454,8 @@ #define F_RELOC_MAX_SIZE 1500
static f_reloc_t freloc;
+#define NUM_CAST_TYPES 15
+
/* C side of the compiler context. */
typedef struct {
@@ -512,21 +514,9 @@ #define F_RELOC_MAX_SIZE 1500
/* libgccjit has really limited support for casting therefore this union will
be used for the scope. */
gcc_jit_type *cast_union_type;
- gcc_jit_field *cast_union_as_ll;
- gcc_jit_field *cast_union_as_ull;
- gcc_jit_field *cast_union_as_l;
- gcc_jit_field *cast_union_as_ul;
- gcc_jit_field *cast_union_as_u;
- gcc_jit_field *cast_union_as_i;
- gcc_jit_field *cast_union_as_b;
- gcc_jit_field *cast_union_as_uintptr;
- gcc_jit_field *cast_union_as_ptrdiff;
- gcc_jit_field *cast_union_as_c_p;
- gcc_jit_field *cast_union_as_v_p;
- gcc_jit_field *cast_union_as_lisp_cons_ptr;
- gcc_jit_field *cast_union_as_lisp_word;
- gcc_jit_field *cast_union_as_lisp_word_tag;
- gcc_jit_field *cast_union_as_lisp_obj_ptr;
+ gcc_jit_function *cast_functions_from_to[NUM_CAST_TYPES][NUM_CAST_TYPES];
+ gcc_jit_type *cast_types[NUM_CAST_TYPES];
+ gcc_jit_field *cast_union_fields[NUM_CAST_TYPES];
gcc_jit_function *func; /* Current function being compiled. */
bool func_has_non_local; /* From comp-func has-non-local slot. */
gcc_jit_lvalue **f_frame; /* "Floating" frame for the current function. */
@@ -684,47 +674,6 @@ bcall0 (Lisp_Object f)
Ffuncall (1, &f);
}
-static gcc_jit_field *
-type_to_cast_field (gcc_jit_type *type)
-{
- gcc_jit_field *field;
-
- if (type == comp.long_long_type)
- field = comp.cast_union_as_ll;
- else if (type == comp.unsigned_long_long_type)
- field = comp.cast_union_as_ull;
- else if (type == comp.long_type)
- field = comp.cast_union_as_l;
- else if (type == comp.unsigned_long_type)
- field = comp.cast_union_as_ul;
- else if (type == comp.unsigned_type)
- field = comp.cast_union_as_u;
- else if (type == comp.int_type)
- field = comp.cast_union_as_i;
- else if (type == comp.bool_type)
- field = comp.cast_union_as_b;
- else if (type == comp.void_ptr_type)
- field = comp.cast_union_as_v_p;
- else if (type == comp.uintptr_type)
- field = comp.cast_union_as_uintptr;
- else if (type == comp.ptrdiff_type)
- field = comp.cast_union_as_ptrdiff;
- else if (type == comp.char_ptr_type)
- field = comp.cast_union_as_c_p;
- else if (type == comp.lisp_cons_ptr_type)
- field = comp.cast_union_as_lisp_cons_ptr;
- else if (type == comp.lisp_word_type)
- field = comp.cast_union_as_lisp_word;
- else if (type == comp.lisp_word_tag_type)
- field = comp.cast_union_as_lisp_word_tag;
- else if (type == comp.lisp_obj_ptr_type)
- field = comp.cast_union_as_lisp_obj_ptr;
- else
- xsignal1 (Qnative_ice, build_string ("unsupported cast"));
-
- return field;
-}
-
static gcc_jit_block *
retrive_block (Lisp_Object block_name)
{
@@ -985,11 +934,19 @@ emit_cond_jump (gcc_jit_rvalue *test,
}
+static int
+type_to_cast_index(gcc_jit_type * type)
+{
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ if (type == comp.cast_types[i])
+ return i;
+
+ xsignal1 (Qnative_ice, build_string ("unsupported cast"));
+}
+
static gcc_jit_rvalue *
emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj)
{
- static ptrdiff_t i;
-
gcc_jit_type *old_type = gcc_jit_rvalue_get_type (obj);
if (new_type == old_type)
@@ -1021,25 +978,14 @@ emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj)
}
#endif
- gcc_jit_field *orig_field =
- type_to_cast_field (old_type);
- gcc_jit_field *dest_field = type_to_cast_field (new_type);
-
- gcc_jit_lvalue *tmp_u =
- gcc_jit_function_new_local (comp.func,
- NULL,
- comp.cast_union_type,
- format_string ("union_cast_%td", i++));
- gcc_jit_block_add_assignment (comp.block,
- NULL,
- gcc_jit_lvalue_access_field (tmp_u,
- NULL,
- orig_field),
- obj);
+ int old_index = type_to_cast_index (old_type);
+ int new_index = type_to_cast_index (new_type);
- return gcc_jit_rvalue_access_field ( gcc_jit_lvalue_as_rvalue (tmp_u),
- NULL,
- dest_field);
+ /* Lookup the appropriate cast function in the cast matrix. */
+ return gcc_jit_context_new_call (comp.ctxt,
+ NULL,
+ comp.cast_functions_from_to[old_index][new_index],
+ 1, &obj);
}
static gcc_jit_rvalue *
@@ -3128,109 +3074,93 @@ define_thread_state_struct (void)
gcc_jit_type_get_pointer (gcc_jit_struct_as_type (comp.thread_state_s));
}
-static void
-define_cast_union (void)
+struct cast_type
{
+ gcc_jit_type * type;
+ const char *name;
+};
+
+static gcc_jit_function *
+define_cast_from_to(struct cast_type from, int from_index, struct cast_type to, int to_index)
+{
+ Lisp_Object name = CALLN(Fformat,
+ build_string("cast_from_%s_to_%s"),
+ build_string(from.name),
+ build_string(to.name));
+ gcc_jit_param * param = gcc_jit_context_new_param(comp.ctxt, NULL,
+ from.type, "arg");
+ gcc_jit_function * result =
+ gcc_jit_context_new_function (comp.ctxt,
+ NULL,
+ GCC_JIT_FUNCTION_ALWAYS_INLINE,
+ to.type,
+ SSDATA(name),
+ 1,
+ ¶m,
+ 0);
+
+ DECL_BLOCK (entry_block, result);
+
+ gcc_jit_lvalue *tmp_union
+ = gcc_jit_function_new_local (result, NULL, comp.cast_union_type, "union_cast");
+
+ gcc_jit_block_add_assignment (entry_block, NULL,
+ gcc_jit_lvalue_access_field (tmp_union, NULL,
+ comp.cast_union_fields[from_index]),
+ gcc_jit_param_as_rvalue(param));
+
+ gcc_jit_block_end_with_return(entry_block,
+ NULL,
+ gcc_jit_rvalue_access_field (
+ gcc_jit_lvalue_as_rvalue (tmp_union),
+ NULL,
+ comp.cast_union_fields[to_index]));
+
+ return result;
+}
+
+static void
+define_cast_functions (void)
+{
+ struct cast_type cast_types[NUM_CAST_TYPES] =
+ {{comp.bool_type, "bool"},
+ {comp.char_ptr_type, "char_ptr"},
+ {comp.int_type, "int"},
+ {comp.lisp_cons_ptr_type, "cons_ptr"},
+ {comp.lisp_obj_ptr_type, "lisp_obj_ptr"},
+ {comp.lisp_word_tag_type, "lisp_word_tag"},
+ {comp.lisp_word_type, "lisp_word"},
+ {comp.long_long_type, "long_long"},
+ {comp.long_type, "long"},
+ {comp.ptrdiff_type, "ptrdiff"},
+ {comp.uintptr_type, "uintptr"},
+ {comp.unsigned_long_long_type, "unsigned_long_long"},
+ {comp.unsigned_long_type, "unsigned_long"},
+ {comp.unsigned_type, "unsigned"},
+ {comp.void_ptr_type, "void_ptr"}};
+
+ /* Define the union used for casting. */
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ {
+ comp.cast_types[i] = cast_types[i].type;
+ comp.cast_union_fields[i] = gcc_jit_context_new_field (comp.ctxt,
+ NULL,
+ cast_types[i].type,
+ cast_types[i].name);
+ }
- comp.cast_union_as_ll =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.long_long_type,
- "ll");
- comp.cast_union_as_ull =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_long_long_type,
- "ull");
- comp.cast_union_as_l =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.long_type,
- "l");
- comp.cast_union_as_ul =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_long_type,
- "ul");
- comp.cast_union_as_u =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_type,
- "u");
- comp.cast_union_as_i =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.int_type,
- "i");
- comp.cast_union_as_b =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.bool_type,
- "b");
- comp.cast_union_as_uintptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.uintptr_type,
- "uintptr");
- comp.cast_union_as_ptrdiff =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.ptrdiff_type,
- "ptrdiff");
- comp.cast_union_as_c_p =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.char_ptr_type,
- "c_p");
- comp.cast_union_as_v_p =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.void_ptr_type,
- "v_p");
- comp.cast_union_as_lisp_cons_ptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_cons_ptr_type,
- "cons_ptr");
- comp.cast_union_as_lisp_word =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_word_type,
- "lisp_word");
- comp.cast_union_as_lisp_word_tag =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_word_tag_type,
- "lisp_word_tag");
- comp.cast_union_as_lisp_obj_ptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_obj_ptr_type,
- "lisp_obj_ptr");
-
-
- gcc_jit_field *cast_union_fields[] =
- { comp.cast_union_as_ll,
- comp.cast_union_as_ull,
- comp.cast_union_as_l,
- comp.cast_union_as_ul,
- comp.cast_union_as_u,
- comp.cast_union_as_i,
- comp.cast_union_as_b,
- comp.cast_union_as_uintptr,
- comp.cast_union_as_ptrdiff,
- comp.cast_union_as_c_p,
- comp.cast_union_as_v_p,
- comp.cast_union_as_lisp_cons_ptr,
- comp.cast_union_as_lisp_word,
- comp.cast_union_as_lisp_word_tag,
- comp.cast_union_as_lisp_obj_ptr };
comp.cast_union_type =
gcc_jit_context_new_union_type (comp.ctxt,
NULL,
"cast_union",
- ARRAYELTS (cast_union_fields),
- cast_union_fields);
+ NUM_CAST_TYPES,
+ comp.cast_union_fields);
+
+ /* Define the cast functions using a matrix. */
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ for (int j = 0; j < NUM_CAST_TYPES; ++j)
+ comp.cast_functions_from_to[i][j] =
+ define_cast_from_to (cast_types[i], i, cast_types[j], j);
}
static void
@@ -4041,7 +3971,7 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt,
define_jmp_buf ();
define_handler_struct ();
define_thread_state_struct ();
- define_cast_union ();
+ define_cast_functions ();
define_naive_bzero ();
define_naive_memcpy ();
--
2.25.1.windows.1
[-- Attachment #3: 0001-Define-static-data-using-string-literals.patch --]
[-- Type: application/octet-stream, Size: 16303 bytes --]
From 9a090c02aedd5ee23951bacb5f0c5275a373875d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Wed, 20 May 2020 00:34:32 -0300
Subject: [PATCH 1/2] Define static data using string literals.
The purpose of this change is to dump prettier C files.
This does not affect compilation times in my tests.
* src/comp.c (emit_static_object): Define static objects using string
literals, memcpy and bzero.
---
src/comp.c | 300 +++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 282 insertions(+), 18 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index 996168bbb5..3f4b3e03b7 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -46,6 +46,7 @@
# include "w32common.h"
#undef gcc_jit_block_add_assignment
+#undef gcc_jit_block_add_assignment_op
#undef gcc_jit_block_add_comment
#undef gcc_jit_block_add_eval
#undef gcc_jit_block_end_with_conditional
@@ -75,6 +76,7 @@
#undef gcc_jit_context_new_rvalue_from_int
#undef gcc_jit_context_new_rvalue_from_long
#undef gcc_jit_context_new_rvalue_from_ptr
+#undef gcc_jit_context_new_string_literal
#undef gcc_jit_context_new_struct_type
#undef gcc_jit_context_new_unary_op
#undef gcc_jit_context_new_union_type
@@ -164,6 +166,8 @@ DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_rvalue_from_long,
(gcc_jit_context *ctxt, gcc_jit_type *numeric_type, long value));
DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_rvalue_from_ptr,
(gcc_jit_context *ctxt, gcc_jit_type *pointer_type, void *value));
+DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_string_literal,
+ (gcc_jit_context *ctxt, const char *value));
DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_unary_op,
(gcc_jit_context *ctxt, gcc_jit_location *loc,
enum gcc_jit_unary_op op, gcc_jit_type *result_type,
@@ -197,6 +201,9 @@ DEF_DLL_FN (gcc_jit_type *, gcc_jit_type_get_pointer, (gcc_jit_type *type));
DEF_DLL_FN (void, gcc_jit_block_add_assignment,
(gcc_jit_block *block, gcc_jit_location *loc, gcc_jit_lvalue *lvalue,
gcc_jit_rvalue *rvalue));
+DEF_DLL_FN (void, gcc_jit_block_add_assignment_op,
+ (gcc_jit_block *block, gcc_jit_location *loc, gcc_jit_lvalue *lvalue,
+ enum gcc_jit_binary_op op, gcc_jit_rvalue *rvalue));
DEF_DLL_FN (void, gcc_jit_block_add_eval,
(gcc_jit_block *block, gcc_jit_location *loc,
gcc_jit_rvalue *rvalue));
@@ -239,6 +246,7 @@ init_gccjit_functions (void)
/* In alphabetical order */
LOAD_DLL_FN (library, gcc_jit_block_add_assignment);
+ LOAD_DLL_FN (library, gcc_jit_block_add_assignment_op);
LOAD_DLL_FN (library, gcc_jit_block_add_comment);
LOAD_DLL_FN (library, gcc_jit_block_add_eval);
LOAD_DLL_FN (library, gcc_jit_block_end_with_conditional);
@@ -268,6 +276,7 @@ init_gccjit_functions (void)
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_int);
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_long);
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_ptr);
+ LOAD_DLL_FN (library, gcc_jit_context_new_string_literal);
LOAD_DLL_FN (library, gcc_jit_context_new_struct_type);
LOAD_DLL_FN (library, gcc_jit_context_new_unary_op);
LOAD_DLL_FN (library, gcc_jit_context_new_union_type);
@@ -296,6 +305,7 @@ init_gccjit_functions (void)
/* In alphabetical order */
#define gcc_jit_block_add_assignment fn_gcc_jit_block_add_assignment
+#define gcc_jit_block_add_assignment_op fn_gcc_jit_block_add_assignment_op
#define gcc_jit_block_add_comment fn_gcc_jit_block_add_comment
#define gcc_jit_block_add_eval fn_gcc_jit_block_add_eval
#define gcc_jit_block_end_with_conditional fn_gcc_jit_block_end_with_conditional
@@ -325,6 +335,7 @@ #define gcc_jit_context_new_param fn_gcc_jit_context_new_param
#define gcc_jit_context_new_rvalue_from_int fn_gcc_jit_context_new_rvalue_from_int
#define gcc_jit_context_new_rvalue_from_long fn_gcc_jit_context_new_rvalue_from_long
#define gcc_jit_context_new_rvalue_from_ptr fn_gcc_jit_context_new_rvalue_from_ptr
+#define gcc_jit_context_new_string_literal fn_gcc_jit_context_new_string_literal
#define gcc_jit_context_new_struct_type fn_gcc_jit_context_new_struct_type
#define gcc_jit_context_new_unary_op fn_gcc_jit_context_new_unary_op
#define gcc_jit_context_new_union_type fn_gcc_jit_context_new_union_type
@@ -548,6 +559,8 @@ #define F_RELOC_MAX_SIZE 1500
gcc_jit_rvalue *data_relocs_ephemeral;
/* Synthesized struct holding func relocs. */
gcc_jit_lvalue *func_relocs;
+ gcc_jit_function *naive_bzero;
+ gcc_jit_function *naive_memcpy;
Lisp_Object d_default_idx;
Lisp_Object d_impure_idx;
Lisp_Object d_ephemeral_idx;
@@ -2346,10 +2359,7 @@ emit_static_object (const char *name, Lisp_Object obj)
{
/* libgccjit has no support for initialized static data.
The mechanism below is certainly not aesthetic but I assume the bottle neck
- in terms of performance at load time will still be the reader.
- NOTE: we can not relay on libgccjit even for valid NULL terminated C
- strings cause of this funny bug that will affect all pre gcc10 era gccs:
- https://gcc.gnu.org/ml/jit/2019-q3/msg00013.html */
+ in terms of performance at load time will still be the reader. */
Lisp_Object str = Fprin1_to_string (obj, Qnil);
ptrdiff_t len = SBYTES (str);
@@ -2398,22 +2408,91 @@ emit_static_object (const char *name, Lisp_Object obj)
gcc_jit_lvalue *arr =
gcc_jit_lvalue_access_field (data_struct, NULL, fields[1]);
- for (ptrdiff_t i = 0; i < len; i++, p++)
+ gcc_jit_lvalue *ptrvar = gcc_jit_function_new_local (f, NULL,
+ comp.char_ptr_type,
+ "ptr");
+
+ gcc_jit_block_add_assignment (
+ block,
+ NULL,
+ ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (
+ comp.ctxt,
+ NULL,
+ gcc_jit_lvalue_as_rvalue (arr),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt, comp.int_type, 0)),
+ NULL));
+
+ /* Emit a call to naive_bzero() to clear all the static object. It
+ is not strictly necessary since the dynamic linker will take care
+ of zeroing the memory too. */
+ gcc_jit_rvalue *bzero_args[2]
+ = {gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_long (comp.ctxt,
+ comp.unsigned_long_long_type,
+ len + 1)};
+
+ gcc_jit_block_add_eval (block, NULL,
+ gcc_jit_context_new_call (comp.ctxt, NULL,
+ comp.naive_bzero, 2,
+ bzero_args));
+
+ for (ptrdiff_t i = 0; i < len;)
{
- gcc_jit_block_add_assignment (
- block,
- NULL,
- gcc_jit_context_new_array_access (
- comp.ctxt,
- NULL,
- gcc_jit_lvalue_as_rvalue (arr),
- gcc_jit_context_new_rvalue_from_int (comp.ctxt,
- comp.ptrdiff_type,
- i)),
- gcc_jit_context_new_rvalue_from_int (comp.ctxt,
- comp.char_type,
- *p));
+ /* We can't use string literals longer that 200 bytes because
+ they cause a crash in older versions of gccjit.
+ https://gcc.gnu.org/ml/jit/2019-q3/msg00013.html */
+ char str[200];
+ strncpy (str, p, 200);
+ str[199] = 0;
+ uintptr_t l = strlen (str);
+
+ if (l != 0)
+ {
+ p += l;
+ i += l;
+
+ gcc_jit_rvalue *args[3]
+ = {gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_string_literal (comp.ctxt, str),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.unsigned_long_long_type,
+ l)};
+
+ gcc_jit_block_add_eval (block, NULL,
+ gcc_jit_context_new_call (comp.ctxt, NULL,
+ comp.naive_memcpy,
+ 3, args));
+ gcc_jit_block_add_assignment (block, NULL, ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type,
+ l)),
+ NULL));
+ }
+ else
+ {
+ /* If strlen returned 0 that means that the static object
+ contains a NULL byte. In that case just move over to the
+ next block. We can rely on the byte being zero because of
+ the previous call to bzero and because the dynamic linker
+ cleared it. */
+ p++;
+ i++;
+ gcc_jit_block_add_assignment (
+ block, NULL, ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (
+ comp.ctxt, NULL, gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type, 1)),
+ NULL));
+ }
}
+
gcc_jit_block_add_assignment (
block,
NULL,
@@ -2759,6 +2838,189 @@ define_jmp_buf (void)
1, &field);
}
+/*
+ * naive implementation of bzero. Hopefully it will be turned into a
+ * smarter implementation by the optimizer.
+ */
+
+static void
+define_naive_bzero (void)
+{
+ /*
+static void
+naive_bzero (char * ptr, unsigned long long len)
+{
+ unsigned long long i;
+
+entry_block:
+ i = (unsigned long long)0;
+ goto test_block;
+
+test_block:
+ if (len > i) goto loop_block; else goto return_block;
+
+loop_block:
+ ptr[i] = (char)0;
+ i += (unsigned long long)1;
+ goto test_block;
+
+return_block:
+ return;
+}
+ */
+ gcc_jit_param *params[2] = {
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.char_ptr_type, "ptr"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.unsigned_long_long_type,
+ "len"),
+ };
+
+ comp.naive_bzero
+ = gcc_jit_context_new_function (comp.ctxt, NULL, GCC_JIT_FUNCTION_INTERNAL,
+ comp.void_type, "naive_bzero", 2,
+ params, false);
+
+ DECL_BLOCK (entry_block, comp.naive_bzero);
+ DECL_BLOCK (test_block, comp.naive_bzero);
+ DECL_BLOCK (loop_block, comp.naive_bzero);
+ DECL_BLOCK (return_block, comp.naive_bzero);
+
+ gcc_jit_lvalue *ptr = gcc_jit_param_as_lvalue (params[0]);
+ gcc_jit_lvalue *i = gcc_jit_function_new_local (comp.naive_bzero, NULL,
+ comp.uintptr_type, "i");
+
+ gcc_jit_block_add_assignment (entry_block, NULL, i,
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type,
+ 0));
+ gcc_jit_block_end_with_jump (entry_block, NULL, test_block);
+
+ /* if (len > i) */
+
+ gcc_jit_rvalue *comparison_result
+ = gcc_jit_context_new_comparison (comp.ctxt, NULL, GCC_JIT_COMPARISON_GT,
+ gcc_jit_param_as_rvalue (params[1]),
+ gcc_jit_lvalue_as_rvalue (i));
+
+ gcc_jit_block_end_with_conditional (test_block, NULL, comparison_result,
+ loop_block, return_block);
+
+ /* ptr[i] = 0; */
+ /* i += 1; */
+
+ gcc_jit_block_add_assignment (loop_block, NULL,
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (ptr),
+ gcc_jit_lvalue_as_rvalue (i)),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt, comp.char_type, 0));
+
+ gcc_jit_block_add_assignment_op (
+ loop_block, NULL, i, GCC_JIT_BINARY_OP_PLUS,
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt, comp.uintptr_type, 1));
+
+ gcc_jit_block_end_with_jump (loop_block, NULL, test_block);
+
+ /* return; */
+
+ gcc_jit_block_end_with_void_return (return_block, NULL);
+}
+
+/*
+ * naive implementation of memcpy. Hopefully it will be turned into a
+ * smarter implementation by the optimizer.
+ */
+
+static void
+define_naive_memcpy (void)
+{
+ /*
+static void *
+naive_memcpy (char * dest, char * src, unsigned long long len)
+{
+ unsigned long long i;
+
+entry_block:
+ i = (unsigned long long)0;
+ goto test_block;
+
+test_block:
+ if (len > i) goto loop_block; else goto return_block;
+
+loop_block:
+ dest[i] = src[i];
+ i += (unsigned long long)1;
+ goto test_block;
+
+return_block:
+ return dest;
+}
+ */
+ gcc_jit_param *params[3] = {
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.char_ptr_type, "dest"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.char_ptr_type, "src"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.unsigned_long_long_type,
+ "len"),
+ };
+
+ comp.naive_memcpy
+ = gcc_jit_context_new_function (comp.ctxt, NULL, GCC_JIT_FUNCTION_INTERNAL,
+ comp.void_ptr_type, "naive_memcpy", 3,
+ params, false);
+
+ DECL_BLOCK (entry_block, comp.naive_memcpy);
+ DECL_BLOCK (test_block, comp.naive_memcpy);
+ DECL_BLOCK (loop_block, comp.naive_memcpy);
+ DECL_BLOCK (return_block, comp.naive_memcpy);
+
+ gcc_jit_lvalue *dest = gcc_jit_param_as_lvalue (params[0]);
+ gcc_jit_lvalue *src = gcc_jit_param_as_lvalue (params[1]);
+ gcc_jit_lvalue *i = gcc_jit_function_new_local (comp.naive_memcpy, NULL,
+ comp.uintptr_type, "i");
+
+ gcc_jit_block_add_assignment (entry_block, NULL, i,
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type,
+ 0));
+ gcc_jit_block_end_with_jump (entry_block, NULL, test_block);
+
+ /* if (len > i) */
+
+ gcc_jit_rvalue *comparison_result
+ = gcc_jit_context_new_comparison (comp.ctxt, NULL, GCC_JIT_COMPARISON_GT,
+ gcc_jit_param_as_rvalue (params[2]),
+ gcc_jit_lvalue_as_rvalue (i));
+
+ gcc_jit_block_end_with_conditional (test_block, NULL, comparison_result,
+ loop_block, return_block);
+
+ /* dest[i] = src[i]; */
+ /* i += 1; */
+
+ gcc_jit_block_add_assignment (loop_block, NULL,
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (dest),
+ gcc_jit_lvalue_as_rvalue (i)),
+ gcc_jit_lvalue_as_rvalue (
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (src),
+ gcc_jit_lvalue_as_rvalue (i))));
+
+ gcc_jit_block_add_assignment_op (loop_block,
+ NULL,
+ i,
+ GCC_JIT_BINARY_OP_PLUS,
+ gcc_jit_context_new_rvalue_from_int (
+ comp.ctxt,
+ comp.uintptr_type,
+ 1));
+
+ gcc_jit_block_end_with_jump (loop_block, NULL, test_block);
+
+ /* return dest; */
+
+ gcc_jit_block_end_with_return (return_block, NULL,
+ gcc_jit_param_as_rvalue (params[0]));
+}
+
/* struct handler definition */
static void
@@ -3780,6 +4042,8 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt,
define_handler_struct ();
define_thread_state_struct ();
define_cast_union ();
+ define_naive_bzero ();
+ define_naive_memcpy ();
return Qt;
}
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-30 15:08 bug#41615: [feature/native-comp] Dump prettier C code Nicolas Bértolo
@ 2020-05-30 22:20 ` Nicolas Bértolo
2020-05-31 10:45 ` Andrea Corallo
2020-05-31 11:37 ` Andrea Corallo
0 siblings, 2 replies; 18+ messages in thread
From: Nicolas Bértolo @ 2020-05-30 22:20 UTC (permalink / raw)
To: 41615
[-- Attachment #1: Type: text/plain, Size: 63 bytes --]
I have reformatted the patches.
Sorry for the inconveniences.
[-- Attachment #2: 0002-Define-casts-using-functions.patch --]
[-- Type: application/octet-stream, Size: 11688 bytes --]
From ef7b9b95cff6824af041a7536588334aeed1ee12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Sat, 30 May 2020 18:33:58 -0300
Subject: [PATCH 2/2] Define casts using functions.
This is to dump prettier C files.
This does not affect compilation times in my tests.
* src/comp.c: Define a 15x15 cast matrix. Use it in emit_coerce().
---
src/comp.c | 279 ++++++++++++++++++++---------------------------------
1 file changed, 106 insertions(+), 173 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index 2f78760764..9d8445de9a 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -455,6 +455,8 @@ #define F_RELOC_MAX_SIZE 1500
static f_reloc_t freloc;
+#define NUM_CAST_TYPES 15
+
/* C side of the compiler context. */
typedef struct {
@@ -513,21 +515,9 @@ #define F_RELOC_MAX_SIZE 1500
/* libgccjit has really limited support for casting therefore this union will
be used for the scope. */
gcc_jit_type *cast_union_type;
- gcc_jit_field *cast_union_as_ll;
- gcc_jit_field *cast_union_as_ull;
- gcc_jit_field *cast_union_as_l;
- gcc_jit_field *cast_union_as_ul;
- gcc_jit_field *cast_union_as_u;
- gcc_jit_field *cast_union_as_i;
- gcc_jit_field *cast_union_as_b;
- gcc_jit_field *cast_union_as_uintptr;
- gcc_jit_field *cast_union_as_ptrdiff;
- gcc_jit_field *cast_union_as_c_p;
- gcc_jit_field *cast_union_as_v_p;
- gcc_jit_field *cast_union_as_lisp_cons_ptr;
- gcc_jit_field *cast_union_as_lisp_word;
- gcc_jit_field *cast_union_as_lisp_word_tag;
- gcc_jit_field *cast_union_as_lisp_obj_ptr;
+ gcc_jit_function *cast_functions_from_to[NUM_CAST_TYPES][NUM_CAST_TYPES];
+ gcc_jit_type *cast_types[NUM_CAST_TYPES];
+ gcc_jit_field *cast_union_fields[NUM_CAST_TYPES];
gcc_jit_function *func; /* Current function being compiled. */
bool func_has_non_local; /* From comp-func has-non-local slot. */
gcc_jit_lvalue **f_frame; /* "Floating" frame for the current function. */
@@ -685,47 +675,6 @@ bcall0 (Lisp_Object f)
Ffuncall (1, &f);
}
-static gcc_jit_field *
-type_to_cast_field (gcc_jit_type *type)
-{
- gcc_jit_field *field;
-
- if (type == comp.long_long_type)
- field = comp.cast_union_as_ll;
- else if (type == comp.unsigned_long_long_type)
- field = comp.cast_union_as_ull;
- else if (type == comp.long_type)
- field = comp.cast_union_as_l;
- else if (type == comp.unsigned_long_type)
- field = comp.cast_union_as_ul;
- else if (type == comp.unsigned_type)
- field = comp.cast_union_as_u;
- else if (type == comp.int_type)
- field = comp.cast_union_as_i;
- else if (type == comp.bool_type)
- field = comp.cast_union_as_b;
- else if (type == comp.void_ptr_type)
- field = comp.cast_union_as_v_p;
- else if (type == comp.uintptr_type)
- field = comp.cast_union_as_uintptr;
- else if (type == comp.ptrdiff_type)
- field = comp.cast_union_as_ptrdiff;
- else if (type == comp.char_ptr_type)
- field = comp.cast_union_as_c_p;
- else if (type == comp.lisp_cons_ptr_type)
- field = comp.cast_union_as_lisp_cons_ptr;
- else if (type == comp.lisp_word_type)
- field = comp.cast_union_as_lisp_word;
- else if (type == comp.lisp_word_tag_type)
- field = comp.cast_union_as_lisp_word_tag;
- else if (type == comp.lisp_obj_ptr_type)
- field = comp.cast_union_as_lisp_obj_ptr;
- else
- xsignal1 (Qnative_ice, build_string ("unsupported cast"));
-
- return field;
-}
-
static gcc_jit_block *
retrive_block (Lisp_Object block_name)
{
@@ -986,11 +935,19 @@ emit_cond_jump (gcc_jit_rvalue *test,
}
+static int
+type_to_cast_index (gcc_jit_type * type)
+{
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ if (type == comp.cast_types[i])
+ return i;
+
+ xsignal1 (Qnative_ice, build_string ("unsupported cast"));
+}
+
static gcc_jit_rvalue *
emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj)
{
- static ptrdiff_t i;
-
gcc_jit_type *old_type = gcc_jit_rvalue_get_type (obj);
if (new_type == old_type)
@@ -1022,25 +979,14 @@ emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj)
}
#endif
- gcc_jit_field *orig_field =
- type_to_cast_field (old_type);
- gcc_jit_field *dest_field = type_to_cast_field (new_type);
-
- gcc_jit_lvalue *tmp_u =
- gcc_jit_function_new_local (comp.func,
- NULL,
- comp.cast_union_type,
- format_string ("union_cast_%td", i++));
- gcc_jit_block_add_assignment (comp.block,
- NULL,
- gcc_jit_lvalue_access_field (tmp_u,
- NULL,
- orig_field),
- obj);
+ int old_index = type_to_cast_index (old_type);
+ int new_index = type_to_cast_index (new_type);
- return gcc_jit_rvalue_access_field ( gcc_jit_lvalue_as_rvalue (tmp_u),
- NULL,
- dest_field);
+ /* Lookup the appropriate cast function in the cast matrix. */
+ return gcc_jit_context_new_call (comp.ctxt,
+ NULL,
+ comp.cast_functions_from_to[old_index][new_index],
+ 1, &obj);
}
static gcc_jit_rvalue *
@@ -3123,109 +3069,96 @@ define_thread_state_struct (void)
gcc_jit_type_get_pointer (gcc_jit_struct_as_type (comp.thread_state_s));
}
-static void
-define_cast_union (void)
+struct cast_type
{
+ gcc_jit_type *type;
+ const char *name;
+};
- comp.cast_union_as_ll =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.long_long_type,
- "ll");
- comp.cast_union_as_ull =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_long_long_type,
- "ull");
- comp.cast_union_as_l =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.long_type,
- "l");
- comp.cast_union_as_ul =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_long_type,
- "ul");
- comp.cast_union_as_u =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_type,
- "u");
- comp.cast_union_as_i =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.int_type,
- "i");
- comp.cast_union_as_b =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.bool_type,
- "b");
- comp.cast_union_as_uintptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.uintptr_type,
- "uintptr");
- comp.cast_union_as_ptrdiff =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.ptrdiff_type,
- "ptrdiff");
- comp.cast_union_as_c_p =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.char_ptr_type,
- "c_p");
- comp.cast_union_as_v_p =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.void_ptr_type,
- "v_p");
- comp.cast_union_as_lisp_cons_ptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_cons_ptr_type,
- "cons_ptr");
- comp.cast_union_as_lisp_word =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_word_type,
- "lisp_word");
- comp.cast_union_as_lisp_word_tag =
- gcc_jit_context_new_field (comp.ctxt,
+static gcc_jit_function *
+define_cast_from_to (struct cast_type from, int from_index, struct cast_type to,
+ int to_index)
+{
+ Lisp_Object name = CALLN (Fformat,
+ build_string ("cast_from_%s_to_%s"),
+ build_string (from.name),
+ build_string (to.name));
+ gcc_jit_param *param = gcc_jit_context_new_param (comp.ctxt, NULL,
+ from.type, "arg");
+ gcc_jit_function *result = gcc_jit_context_new_function (comp.ctxt,
NULL,
- comp.lisp_word_tag_type,
- "lisp_word_tag");
- comp.cast_union_as_lisp_obj_ptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_obj_ptr_type,
- "lisp_obj_ptr");
-
-
- gcc_jit_field *cast_union_fields[] =
- { comp.cast_union_as_ll,
- comp.cast_union_as_ull,
- comp.cast_union_as_l,
- comp.cast_union_as_ul,
- comp.cast_union_as_u,
- comp.cast_union_as_i,
- comp.cast_union_as_b,
- comp.cast_union_as_uintptr,
- comp.cast_union_as_ptrdiff,
- comp.cast_union_as_c_p,
- comp.cast_union_as_v_p,
- comp.cast_union_as_lisp_cons_ptr,
- comp.cast_union_as_lisp_word,
- comp.cast_union_as_lisp_word_tag,
- comp.cast_union_as_lisp_obj_ptr };
+ GCC_JIT_FUNCTION_ALWAYS_INLINE,
+ to.type,
+ SSDATA (name),
+ 1,
+ ¶m,
+ 0);
+
+ DECL_BLOCK (entry_block, result);
+
+ gcc_jit_lvalue *tmp_union
+ = gcc_jit_function_new_local (result,
+ NULL,
+ comp.cast_union_type,
+ "union_cast");
+
+ gcc_jit_block_add_assignment (entry_block, NULL,
+ gcc_jit_lvalue_access_field (tmp_union, NULL,
+ comp.cast_union_fields[from_index]),
+ gcc_jit_param_as_rvalue (param));
+
+ gcc_jit_block_end_with_return (entry_block,
+ NULL,
+ gcc_jit_rvalue_access_field (
+ gcc_jit_lvalue_as_rvalue (tmp_union),
+ NULL,
+ comp.cast_union_fields[to_index]));
+
+ return result;
+}
+
+static void
+define_cast_functions (void)
+{
+ struct cast_type cast_types[NUM_CAST_TYPES]
+ = {{comp.bool_type, "bool"},
+ {comp.char_ptr_type, "char_ptr"},
+ {comp.int_type, "int"},
+ {comp.lisp_cons_ptr_type, "cons_ptr"},
+ {comp.lisp_obj_ptr_type, "lisp_obj_ptr"},
+ {comp.lisp_word_tag_type, "lisp_word_tag"},
+ {comp.lisp_word_type, "lisp_word"},
+ {comp.long_long_type, "long_long"},
+ {comp.long_type, "long"},
+ {comp.ptrdiff_type, "ptrdiff"},
+ {comp.uintptr_type, "uintptr"},
+ {comp.unsigned_long_long_type, "unsigned_long_long"},
+ {comp.unsigned_long_type, "unsigned_long"},
+ {comp.unsigned_type, "unsigned"},
+ {comp.void_ptr_type, "void_ptr"}};
+
+ /* Define the union used for casting. */
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ {
+ comp.cast_types[i] = cast_types[i].type;
+ comp.cast_union_fields[i] = gcc_jit_context_new_field (comp.ctxt,
+ NULL,
+ cast_types[i].type,
+ cast_types[i].name);
+ }
+
comp.cast_union_type =
gcc_jit_context_new_union_type (comp.ctxt,
NULL,
"cast_union",
- ARRAYELTS (cast_union_fields),
- cast_union_fields);
+ NUM_CAST_TYPES,
+ comp.cast_union_fields);
+
+ /* Define the cast functions using a matrix. */
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ for (int j = 0; j < NUM_CAST_TYPES; ++j)
+ comp.cast_functions_from_to[i][j]
+ = define_cast_from_to (cast_types[i], i, cast_types[j], j);
}
static void
@@ -4036,7 +3969,7 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt,
define_jmp_buf ();
define_handler_struct ();
define_thread_state_struct ();
- define_cast_union ();
+ define_cast_functions ();
define_naive_bzero ();
define_naive_memcpy ();
--
2.25.1.windows.1
[-- Attachment #3: 0001-Define-static-data-using-string-literals.patch --]
[-- Type: application/octet-stream, Size: 16124 bytes --]
From 0720ec7eb3dc552b018273cd68a5f7d6bb2fdb72 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Wed, 20 May 2020 00:34:32 -0300
Subject: [PATCH 1/2] Define static data using string literals.
The purpose of this change is to dump prettier C files.
This does not affect compilation times in my tests.
* src/comp.c (emit_static_object): Define static objects using string
literals, memcpy and bzero.
---
src/comp.c | 295 +++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 277 insertions(+), 18 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index d3bff1e4cf..2f78760764 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -46,6 +46,7 @@
# include "w32common.h"
#undef gcc_jit_block_add_assignment
+#undef gcc_jit_block_add_assignment_op
#undef gcc_jit_block_add_comment
#undef gcc_jit_block_add_eval
#undef gcc_jit_block_end_with_conditional
@@ -75,6 +76,7 @@
#undef gcc_jit_context_new_rvalue_from_int
#undef gcc_jit_context_new_rvalue_from_long
#undef gcc_jit_context_new_rvalue_from_ptr
+#undef gcc_jit_context_new_string_literal
#undef gcc_jit_context_new_struct_type
#undef gcc_jit_context_new_unary_op
#undef gcc_jit_context_new_union_type
@@ -164,6 +166,8 @@ DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_rvalue_from_long,
(gcc_jit_context *ctxt, gcc_jit_type *numeric_type, long value));
DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_rvalue_from_ptr,
(gcc_jit_context *ctxt, gcc_jit_type *pointer_type, void *value));
+DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_string_literal,
+ (gcc_jit_context *ctxt, const char *value));
DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_unary_op,
(gcc_jit_context *ctxt, gcc_jit_location *loc,
enum gcc_jit_unary_op op, gcc_jit_type *result_type,
@@ -197,6 +201,10 @@ DEF_DLL_FN (gcc_jit_type *, gcc_jit_type_get_pointer, (gcc_jit_type *type));
DEF_DLL_FN (void, gcc_jit_block_add_assignment,
(gcc_jit_block *block, gcc_jit_location *loc, gcc_jit_lvalue *lvalue,
gcc_jit_rvalue *rvalue));
+DEF_DLL_FN (void, gcc_jit_block_add_assignment_op,
+ (gcc_jit_block *block, gcc_jit_location *loc,
+ gcc_jit_lvalue *lvalue, enum gcc_jit_binary_op op,
+ gcc_jit_rvalue *rvalue));
DEF_DLL_FN (void, gcc_jit_block_add_eval,
(gcc_jit_block *block, gcc_jit_location *loc,
gcc_jit_rvalue *rvalue));
@@ -239,6 +247,7 @@ init_gccjit_functions (void)
/* In alphabetical order */
LOAD_DLL_FN (library, gcc_jit_block_add_assignment);
+ LOAD_DLL_FN (library, gcc_jit_block_add_assignment_op);
LOAD_DLL_FN (library, gcc_jit_block_add_comment);
LOAD_DLL_FN (library, gcc_jit_block_add_eval);
LOAD_DLL_FN (library, gcc_jit_block_end_with_conditional);
@@ -268,6 +277,7 @@ init_gccjit_functions (void)
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_int);
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_long);
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_ptr);
+ LOAD_DLL_FN (library, gcc_jit_context_new_string_literal);
LOAD_DLL_FN (library, gcc_jit_context_new_struct_type);
LOAD_DLL_FN (library, gcc_jit_context_new_unary_op);
LOAD_DLL_FN (library, gcc_jit_context_new_union_type);
@@ -296,6 +306,7 @@ init_gccjit_functions (void)
/* In alphabetical order */
#define gcc_jit_block_add_assignment fn_gcc_jit_block_add_assignment
+#define gcc_jit_block_add_assignment_op fn_gcc_jit_block_add_assignment_op
#define gcc_jit_block_add_comment fn_gcc_jit_block_add_comment
#define gcc_jit_block_add_eval fn_gcc_jit_block_add_eval
#define gcc_jit_block_end_with_conditional fn_gcc_jit_block_end_with_conditional
@@ -325,6 +336,7 @@ #define gcc_jit_context_new_param fn_gcc_jit_context_new_param
#define gcc_jit_context_new_rvalue_from_int fn_gcc_jit_context_new_rvalue_from_int
#define gcc_jit_context_new_rvalue_from_long fn_gcc_jit_context_new_rvalue_from_long
#define gcc_jit_context_new_rvalue_from_ptr fn_gcc_jit_context_new_rvalue_from_ptr
+#define gcc_jit_context_new_string_literal fn_gcc_jit_context_new_string_literal
#define gcc_jit_context_new_struct_type fn_gcc_jit_context_new_struct_type
#define gcc_jit_context_new_unary_op fn_gcc_jit_context_new_unary_op
#define gcc_jit_context_new_union_type fn_gcc_jit_context_new_union_type
@@ -548,6 +560,8 @@ #define F_RELOC_MAX_SIZE 1500
gcc_jit_rvalue *data_relocs_ephemeral;
/* Synthesized struct holding func relocs. */
gcc_jit_lvalue *func_relocs;
+ gcc_jit_function *naive_bzero;
+ gcc_jit_function *naive_memcpy;
Lisp_Object d_default_idx;
Lisp_Object d_impure_idx;
Lisp_Object d_ephemeral_idx;
@@ -2346,10 +2360,7 @@ emit_static_object (const char *name, Lisp_Object obj)
{
/* libgccjit has no support for initialized static data.
The mechanism below is certainly not aesthetic but I assume the bottle neck
- in terms of performance at load time will still be the reader.
- NOTE: we can not relay on libgccjit even for valid NULL terminated C
- strings cause of this funny bug that will affect all pre gcc10 era gccs:
- https://gcc.gnu.org/ml/jit/2019-q3/msg00013.html */
+ in terms of performance at load time will still be the reader. */
Lisp_Object str = Fprin1_to_string (obj, Qnil);
ptrdiff_t len = SBYTES (str);
@@ -2398,22 +2409,91 @@ emit_static_object (const char *name, Lisp_Object obj)
gcc_jit_lvalue *arr =
gcc_jit_lvalue_access_field (data_struct, NULL, fields[1]);
- for (ptrdiff_t i = 0; i < len; i++, p++)
+ gcc_jit_lvalue *ptrvar = gcc_jit_function_new_local (f, NULL,
+ comp.char_ptr_type,
+ "ptr");
+
+ gcc_jit_block_add_assignment (
+ block,
+ NULL,
+ ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (
+ comp.ctxt,
+ NULL,
+ gcc_jit_lvalue_as_rvalue (arr),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt, comp.int_type, 0)),
+ NULL));
+
+ /* Emit a call to naive_bzero () to clear all the static object. It
+ is not strictly necessary since the dynamic linker will take care
+ of zeroing the memory too. */
+ gcc_jit_rvalue *bzero_args[2]
+ = {gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_long (comp.ctxt,
+ comp.unsigned_long_long_type,
+ len + 1)};
+
+ gcc_jit_block_add_eval (block, NULL,
+ gcc_jit_context_new_call (comp.ctxt, NULL,
+ comp.naive_bzero, 2,
+ bzero_args));
+
+ for (ptrdiff_t i = 0; i < len;)
{
- gcc_jit_block_add_assignment (
- block,
- NULL,
- gcc_jit_context_new_array_access (
- comp.ctxt,
- NULL,
- gcc_jit_lvalue_as_rvalue (arr),
- gcc_jit_context_new_rvalue_from_int (comp.ctxt,
- comp.ptrdiff_type,
- i)),
- gcc_jit_context_new_rvalue_from_int (comp.ctxt,
- comp.char_type,
- *p));
+ /* We can't use string literals longer that 200 bytes because
+ they cause a crash in older versions of gccjit.
+ https://gcc.gnu.org/ml/jit/2019-q3/msg00013.html. */
+ char str[200];
+ strncpy (str, p, 200);
+ str[199] = 0;
+ uintptr_t l = strlen (str);
+
+ if (l != 0)
+ {
+ p += l;
+ i += l;
+
+ gcc_jit_rvalue *args[3]
+ = {gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_string_literal (comp.ctxt, str),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.unsigned_long_long_type,
+ l)};
+
+ gcc_jit_block_add_eval (block, NULL,
+ gcc_jit_context_new_call (comp.ctxt, NULL,
+ comp.naive_memcpy,
+ 3, args));
+ gcc_jit_block_add_assignment (block, NULL, ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type,
+ l)),
+ NULL));
+ }
+ else
+ {
+ /* If strlen returned 0 that means that the static object
+ contains a NULL byte. In that case just move over to the
+ next block. We can rely on the byte being zero because
+ of the previous call to bzero and because the dynamic
+ linker cleared it. */
+ p++;
+ i++;
+ gcc_jit_block_add_assignment (
+ block, NULL, ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (
+ comp.ctxt, NULL, gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type, 1)),
+ NULL));
+ }
}
+
gcc_jit_block_add_assignment (
block,
NULL,
@@ -2759,6 +2839,183 @@ define_jmp_buf (void)
1, &field);
}
+/* Naive implementation of bzero. Hopefully it will be turned into a
+ smarter implementation by the optimizer. */
+static void
+define_naive_bzero (void)
+{
+/* Emitted code:
+static void
+naive_bzero (char * ptr, unsigned long long len)
+{
+ unsigned long long i;
+
+entry_block:
+ i = (unsigned long long)0;
+ goto test_block;
+
+test_block:
+ if (len > i) goto loop_block; else goto return_block;
+
+loop_block:
+ ptr[i] = (char)0;
+ i += (unsigned long long)1;
+ goto test_block;
+
+return_block:
+ return;
+}
+*/
+ gcc_jit_param *params[2] = {
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.char_ptr_type, "ptr"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.unsigned_long_long_type,
+ "len"),
+ };
+
+ comp.naive_bzero
+ = gcc_jit_context_new_function (comp.ctxt, NULL, GCC_JIT_FUNCTION_INTERNAL,
+ comp.void_type, "naive_bzero", 2,
+ params, false);
+
+ DECL_BLOCK (entry_block, comp.naive_bzero);
+ DECL_BLOCK (test_block, comp.naive_bzero);
+ DECL_BLOCK (loop_block, comp.naive_bzero);
+ DECL_BLOCK (return_block, comp.naive_bzero);
+
+ gcc_jit_lvalue *ptr = gcc_jit_param_as_lvalue (params[0]);
+ gcc_jit_lvalue *i = gcc_jit_function_new_local (comp.naive_bzero, NULL,
+ comp.uintptr_type, "i");
+
+ gcc_jit_block_add_assignment (entry_block, NULL, i,
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type,
+ 0));
+ gcc_jit_block_end_with_jump (entry_block, NULL, test_block);
+
+ /* if (len > i) */
+
+ gcc_jit_rvalue *comparison_result
+ = gcc_jit_context_new_comparison (comp.ctxt, NULL, GCC_JIT_COMPARISON_GT,
+ gcc_jit_param_as_rvalue (params[1]),
+ gcc_jit_lvalue_as_rvalue (i));
+
+ gcc_jit_block_end_with_conditional (test_block, NULL, comparison_result,
+ loop_block, return_block);
+
+ /* ptr[i] = 0; */
+ /* i += 1; */
+
+ gcc_jit_block_add_assignment (loop_block, NULL,
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (ptr),
+ gcc_jit_lvalue_as_rvalue (i)),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt, comp.char_type, 0));
+
+ gcc_jit_block_add_assignment_op (
+ loop_block, NULL, i, GCC_JIT_BINARY_OP_PLUS,
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt, comp.uintptr_type, 1));
+
+ gcc_jit_block_end_with_jump (loop_block, NULL, test_block);
+
+ /* return; */
+
+ gcc_jit_block_end_with_void_return (return_block, NULL);
+}
+
+/* Naive implementation of memcpy. Hopefully it will be turned into a
+ smarter implementation by the optimizer. */
+static void
+define_naive_memcpy (void)
+{
+/* Emitted code:
+static void *
+naive_memcpy (char * dest, char * src, unsigned long long len)
+{
+ unsigned long long i;
+
+entry_block:
+ i = (unsigned long long)0;
+ goto test_block;
+
+test_block:
+ if (len > i) goto loop_block; else goto return_block;
+
+loop_block:
+ dest[i] = src[i];
+ i += (unsigned long long)1;
+ goto test_block;
+
+return_block:
+ return dest;
+}
+*/
+ gcc_jit_param *params[3] = {
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.char_ptr_type, "dest"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.char_ptr_type, "src"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.unsigned_long_long_type,
+ "len"),
+ };
+
+ comp.naive_memcpy
+ = gcc_jit_context_new_function (comp.ctxt, NULL, GCC_JIT_FUNCTION_INTERNAL,
+ comp.void_ptr_type, "naive_memcpy", 3,
+ params, false);
+
+ DECL_BLOCK (entry_block, comp.naive_memcpy);
+ DECL_BLOCK (test_block, comp.naive_memcpy);
+ DECL_BLOCK (loop_block, comp.naive_memcpy);
+ DECL_BLOCK (return_block, comp.naive_memcpy);
+
+ gcc_jit_lvalue *dest = gcc_jit_param_as_lvalue (params[0]);
+ gcc_jit_lvalue *src = gcc_jit_param_as_lvalue (params[1]);
+ gcc_jit_lvalue *i = gcc_jit_function_new_local (comp.naive_memcpy, NULL,
+ comp.uintptr_type, "i");
+
+ gcc_jit_block_add_assignment (entry_block, NULL, i,
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type,
+ 0));
+ gcc_jit_block_end_with_jump (entry_block, NULL, test_block);
+
+ /* if (len > i) */
+
+ gcc_jit_rvalue *comparison_result
+ = gcc_jit_context_new_comparison (comp.ctxt, NULL, GCC_JIT_COMPARISON_GT,
+ gcc_jit_param_as_rvalue (params[2]),
+ gcc_jit_lvalue_as_rvalue (i));
+
+ gcc_jit_block_end_with_conditional (test_block, NULL, comparison_result,
+ loop_block, return_block);
+
+ /* dest[i] = src[i]; */
+ /* i += 1; */
+
+ gcc_jit_block_add_assignment (loop_block, NULL,
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (dest),
+ gcc_jit_lvalue_as_rvalue (i)),
+ gcc_jit_lvalue_as_rvalue (
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (src),
+ gcc_jit_lvalue_as_rvalue (i))));
+
+ gcc_jit_block_add_assignment_op (loop_block,
+ NULL,
+ i,
+ GCC_JIT_BINARY_OP_PLUS,
+ gcc_jit_context_new_rvalue_from_int (
+ comp.ctxt,
+ comp.uintptr_type,
+ 1));
+
+ gcc_jit_block_end_with_jump (loop_block, NULL, test_block);
+
+ /* return dest; */
+
+ gcc_jit_block_end_with_return (return_block, NULL,
+ gcc_jit_param_as_rvalue (params[0]));
+}
+
/* struct handler definition */
static void
@@ -3780,6 +4037,8 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt,
define_handler_struct ();
define_thread_state_struct ();
define_cast_union ();
+ define_naive_bzero ();
+ define_naive_memcpy ();
return Qt;
}
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-30 22:20 ` Nicolas Bértolo
@ 2020-05-31 10:45 ` Andrea Corallo
2020-05-31 15:42 ` Nicolas Bértolo
2020-05-31 11:37 ` Andrea Corallo
1 sibling, 1 reply; 18+ messages in thread
From: Andrea Corallo @ 2020-05-31 10:45 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
> I have reformatted the patches.
>
> Sorry for the inconveniences.
>
> From ef7b9b95cff6824af041a7536588334aeed1ee12 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
> Date: Sat, 30 May 2020 18:33:58 -0300
> Subject: [PATCH 2/2] Define casts using functions.
>
> This is to dump prettier C files.
> This does not affect compilation times in my tests.
>
> * src/comp.c: Define a 15x15 cast matrix. Use it in emit_coerce().
I like the idea of the patch. I've tested it too and I do not see
compile time overhead.
> src/comp.c | 279 ++++++++++++++++++++---------------------------------
> 1 file changed, 106 insertions(+), 173 deletions(-)
>
> diff --git a/src/comp.c b/src/comp.c
> index 2f78760764..9d8445de9a 100644
> --- a/src/comp.c
> +++ b/src/comp.c
> @@ -455,6 +455,8 @@ #define F_RELOC_MAX_SIZE 1500
>
> static f_reloc_t freloc;
>
> +#define NUM_CAST_TYPES 15
> +
> /* C side of the compiler context. */
>
[...]
> +static gcc_jit_function *
> +define_cast_from_to (struct cast_type from, int from_index, struct cast_type to,
> + int to_index)
> +{
> + Lisp_Object name = CALLN (Fformat,
> + build_string ("cast_from_%s_to_%s"),
> + build_string (from.name),
> + build_string (to.name));
Minor, for other code if we know strings are not very long we use
format_string not to allocate. Libgccjit copy the string by itself.
> + gcc_jit_param *param = gcc_jit_context_new_param (comp.ctxt, NULL,
> + from.type, "arg");
> + gcc_jit_function *result = gcc_jit_context_new_function (comp.ctxt,
> NULL,
> - comp.lisp_word_tag_type,
> - "lisp_word_tag");
> - comp.cast_union_as_lisp_obj_ptr =
> - gcc_jit_context_new_field (comp.ctxt,
> - NULL,
> - comp.lisp_obj_ptr_type,
> - "lisp_obj_ptr");
> -
> -
> - gcc_jit_field *cast_union_fields[] =
> - { comp.cast_union_as_ll,
> - comp.cast_union_as_ull,
> - comp.cast_union_as_l,
> - comp.cast_union_as_ul,
> - comp.cast_union_as_u,
> - comp.cast_union_as_i,
> - comp.cast_union_as_b,
> - comp.cast_union_as_uintptr,
> - comp.cast_union_as_ptrdiff,
> - comp.cast_union_as_c_p,
> - comp.cast_union_as_v_p,
> - comp.cast_union_as_lisp_cons_ptr,
> - comp.cast_union_as_lisp_word,
> - comp.cast_union_as_lisp_word_tag,
> - comp.cast_union_as_lisp_obj_ptr };
> + GCC_JIT_FUNCTION_ALWAYS_INLINE,
We'll have to keep an eye on this always inline. Depending on the GCC
version if the inline fail I've seen these ending-up in a unrecoverable
compilation error within libgccjit.
> + to.type,
> + SSDATA (name),
> + 1,
> + ¶m,
> + 0);
> +
> + DECL_BLOCK (entry_block, result);
> +
> + gcc_jit_lvalue *tmp_union
> + = gcc_jit_function_new_local (result,
> + NULL,
> + comp.cast_union_type,
> + "union_cast");
> +
> + gcc_jit_block_add_assignment (entry_block, NULL,
> + gcc_jit_lvalue_access_field (tmp_union, NULL,
> + comp.cast_union_fields[from_index]),
> + gcc_jit_param_as_rvalue (param));
> +
> + gcc_jit_block_end_with_return (entry_block,
> + NULL,
> + gcc_jit_rvalue_access_field (
> + gcc_jit_lvalue_as_rvalue (tmp_union),
> + NULL,
> + comp.cast_union_fields[to_index]));
> +
> + return result;
> +}
> +
> +static void
> +define_cast_functions (void)
> +{
> + struct cast_type cast_types[NUM_CAST_TYPES]
> + = {{comp.bool_type, "bool"},
Just a nit, in the rest of the file I used spaces after { for array
initializers, I think in this patch previously you did it too.
> + {comp.char_ptr_type, "char_ptr"},
> + {comp.int_type, "int"},
> + {comp.lisp_cons_ptr_type, "cons_ptr"},
> + {comp.lisp_obj_ptr_type, "lisp_obj_ptr"},
> + {comp.lisp_word_tag_type, "lisp_word_tag"},
> + {comp.lisp_word_type, "lisp_word"},
> + {comp.long_long_type, "long_long"},
> + {comp.long_type, "long"},
> + {comp.ptrdiff_type, "ptrdiff"},
> + {comp.uintptr_type, "uintptr"},
> + {comp.unsigned_long_long_type, "unsigned_long_long"},
> + {comp.unsigned_long_type, "unsigned_long"},
> + {comp.unsigned_type, "unsigned"},
> + {comp.void_ptr_type, "void_ptr"}};
> +
On the subject of 'emit_coerce' in case you like to do more work in the
area I think would be good to have an additional boolean parameter
called like 'safe'.
If this is true and we cast from a smaller size to a bigger one we
should zero the cast union first. In case false we should raise an ICE.
Thanks
Andrea
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-30 22:20 ` Nicolas Bértolo
2020-05-31 10:45 ` Andrea Corallo
@ 2020-05-31 11:37 ` Andrea Corallo
2020-05-31 15:48 ` Andrea Corallo
2020-05-31 15:54 ` Nicolas Bértolo
1 sibling, 2 replies; 18+ messages in thread
From: Andrea Corallo @ 2020-05-31 11:37 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]
Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
> I have reformatted the patches.
>
> Sorry for the inconveniences.
>
>
> From 0720ec7eb3dc552b018273cd68a5f7d6bb2fdb72 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
> Date: Wed, 20 May 2020 00:34:32 -0300
> Subject: [PATCH 1/2] Define static data using string literals.
>
> The purpose of this change is to dump prettier C files.
> This does not affect compilation times in my tests.
>
> * src/comp.c (emit_static_object): Define static objects using string
> literals, memcpy and bzero.
> ---
> src/comp.c | 295 +++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 277 insertions(+), 18 deletions(-)
I like this considerably less :)
It introduces quite some complexity and the same advantage in
debuggability can be achieved with something like the attached 8 line
patch (untested).
Generally speaking I want to try to keep our back-end as simple as we
manage to.
On the subject of 'emit_static_object' the current situation is not
ideal. But rather that working around the workaround I believe the right
thing to do is to improve GCC with a new entry point and keep the
current arrangement as a simple fallback.
I've already an half cooked GCC patch to allow for directly injecting
blobs, this should have more then one advantage. Hopefully I manage to
start testing it today, I'm rather curious.
Andrea
--
akrl@sdf.org
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Emit-better-debug-comments-in-emit_static_object.patch --]
[-- Type: text/x-diff, Size: 1072 bytes --]
From 2154060e21c6d74d46cf274abaa716bec8fd2ac5 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <akrl@sdf.org>
Date: Sun, 31 May 2020 12:22:46 +0100
Subject: [PATCH] * Emit better debug comments in emit_static_object
* src/comp.c (emit_static_object): Do not truncate debug
comments at the first NULL character.
---
src/comp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index d3bff1e4cf..179e97a551 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -2392,8 +2392,12 @@ emit_static_object (const char *name, Lisp_Object obj)
0, NULL, 0);
DECL_BLOCK (block, f);
- /* NOTE this truncates if the data has some zero byte before termination. */
- gcc_jit_block_add_comment (block, NULL, p);
+ char *comment = memcpy (xmalloc (len), p, len);
+ for (ptrdiff_t i = 0; i < len - 1; i++)
+ if (!comment[i])
+ comment[i] = '\n';
+ gcc_jit_block_add_comment (block, NULL, comment);
+ xfree (comment);
gcc_jit_lvalue *arr =
gcc_jit_lvalue_access_field (data_struct, NULL, fields[1]);
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 10:45 ` Andrea Corallo
@ 2020-05-31 15:42 ` Nicolas Bértolo
0 siblings, 0 replies; 18+ messages in thread
From: Nicolas Bértolo @ 2020-05-31 15:42 UTC (permalink / raw)
To: Andrea Corallo; +Cc: 41615
> I like the idea of the patch. I've tested it too and I do not see
> compile time overhead.
Good :).
> On the subject of 'emit_coerce' in case you like to do more work in the
> area I think would be good to have an additional boolean parameter
> called like 'safe'.
> If this is true and we cast from a smaller size to a bigger one we
> should zero the cast union first. In case false we should raise an ICE.
Ok, will try that. I will reuse the definition of naive_bzero() from the other
patch.
I will post an updated patch soon.
Nico.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 11:37 ` Andrea Corallo
@ 2020-05-31 15:48 ` Andrea Corallo
2020-05-31 15:54 ` Nicolas Bértolo
1 sibling, 0 replies; 18+ messages in thread
From: Andrea Corallo @ 2020-05-31 15:48 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
Andrea Corallo <akrl@sdf.org> writes:
> Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
>
>> I have reformatted the patches.
>>
>> Sorry for the inconveniences.
>>
>>
>> From 0720ec7eb3dc552b018273cd68a5f7d6bb2fdb72 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
>> Date: Wed, 20 May 2020 00:34:32 -0300
>> Subject: [PATCH 1/2] Define static data using string literals.
>>
>> The purpose of this change is to dump prettier C files.
>> This does not affect compilation times in my tests.
>>
>> * src/comp.c (emit_static_object): Define static objects using string
>> literals, memcpy and bzero.
>> ---
>> src/comp.c | 295 +++++++++++++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 277 insertions(+), 18 deletions(-)
>
> I like this considerably less :)
>
> It introduces quite some complexity and the same advantage in
> debuggability can be achieved with something like the attached 8 line
> patch (untested).
>
> Generally speaking I want to try to keep our back-end as simple as we
> manage to.
>
> On the subject of 'emit_static_object' the current situation is not
> ideal. But rather that working around the workaround I believe the right
> thing to do is to improve GCC with a new entry point and keep the
> current arrangement as a simple fallback.
>
> I've already an half cooked GCC patch to allow for directly injecting
> blobs, this should have more then one advantage. Hopefully I manage to
> start testing it today, I'm rather curious.
>
> Andrea
Interesting news
I've boostraped the Emacs with my GCC with blob support and measured an
embarrassing low overall compile time.
Then I questioned my-self "why is Nico not seeing any compile time
benefit from his patch?"
So I boostraped "Define static data using string literals" measuring a
considerable improvement too. Is not as good as the one with specific
GCC blob support but still very beneficial.
Are you sure you do not see improvements in compile-time when this is
applied? If is not the case maybe your memcpy is inlined and unrolled,
but is quite strange that I do not see it here.
Andrea
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 11:37 ` Andrea Corallo
2020-05-31 15:48 ` Andrea Corallo
@ 2020-05-31 15:54 ` Nicolas Bértolo
2020-05-31 16:57 ` Andrea Corallo
1 sibling, 1 reply; 18+ messages in thread
From: Nicolas Bértolo @ 2020-05-31 15:54 UTC (permalink / raw)
To: Andrea Corallo; +Cc: 41615
> I like this considerably less :)
Ok, let's say goodbye to this patch.
> It introduces quite some complexity and the same advantage in
> debuggability can be achieved with something like the attached 8 line
> patch (untested).
Sounds good, I haven't tested it either.
> Generally speaking I want to try to keep our back-end as simple as we
> manage to.
I initially wrote this patch chasing the reason for slow compile times. I think
that a 10k line C file should be compiled much faster than what gccjit achieves.
I thought that "uncommon" (for C) ways of doing thing were causing gccjit to get
stuck trying to optimize them hard, until it gave up. I thought that filling the
static data using memcpy() and constant strings would help GCC recognize this as
a constant initialization and hopefully just store a completely initialized copy
in memory.
I found that GCC would inline memcpy() and the static initialization would turn
into a very long unrolled loop with SSE instructions. I tested this with -O3
only in gccjit to force maximum optimization. I found this super strange
considering that -ftree-loop-distribute-patterns is enabled at -O3 and it should
recognize the naive_memcpy() function as an implementation of memcpy() and issue
calls to libc's implementation. Instead, it was inlining and unrolling it.
> On the subject of 'emit_static_object' the current situation is not
> ideal. But rather that working around the workaround I believe the right
> thing to do is to improve GCC with a new entry point and keep the
> current arrangement as a simple fallback.
I agree.
> I've already an half cooked GCC patch to allow for directly injecting
> blobs, this should have more then one advantage. Hopefully I manage to
> start testing it today, I'm rather curious.
Great to hear.
Nico.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 15:54 ` Nicolas Bértolo
@ 2020-05-31 16:57 ` Andrea Corallo
2020-05-31 17:26 ` Nicolas Bértolo
0 siblings, 1 reply; 18+ messages in thread
From: Andrea Corallo @ 2020-05-31 16:57 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
[-- Attachment #1: Type: text/plain, Size: 2168 bytes --]
Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
>> I like this considerably less :)
>
> Ok, let's say goodbye to this patch.
>
>> It introduces quite some complexity and the same advantage in
>> debuggability can be achieved with something like the attached 8 line
>> patch (untested).
>
> Sounds good, I haven't tested it either.
>
>> Generally speaking I want to try to keep our back-end as simple as we
>> manage to.
>
> I initially wrote this patch chasing the reason for slow compile times. I think
> that a 10k line C file should be compiled much faster than what gccjit achieves.
> I thought that "uncommon" (for C) ways of doing thing were causing gccjit to get
> stuck trying to optimize them hard, until it gave up. I thought that filling the
> static data using memcpy() and constant strings would help GCC recognize this as
> a constant initialization and hopefully just store a completely initialized copy
> in memory.
>
> I found that GCC would inline memcpy() and the static initialization would turn
> into a very long unrolled loop with SSE instructions. I tested this with -O3
> only in gccjit to force maximum optimization. I found this super strange
> considering that -ftree-loop-distribute-patterns is enabled at -O3 and it should
> recognize the naive_memcpy() function as an implementation of memcpy() and issue
> calls to libc's implementation. Instead, it was inlining and unrolling it.
Ok you confirm the suspects I wrote in the other mail!
I've used your patch as a base, apart for minors here and there I've
stripped out the definitions of bzero and memcpy.
I believe bzero is unnecessary given these are static allocated.
For memcpy we can just use the standard library implementation given
elns are linked to it. The other advantage is that doing this way (here
at least) memcpy is not inlined also at speed 3, so we don't trap in the
optimizer issue!
All summed is even a little faster than the stock patch and closer to
the one with the specific GCC blob support.
Let me know if you like the attached and if does the job for you too.
Thanks
Andrea
--
akrl@sdf.org
[-- Attachment #2: 0001-Cut-down-compile-time-emitting-static-data-as-string.patch --]
[-- Type: text/x-diff, Size: 10816 bytes --]
From 3efb2808d415f723ade4a0f9f61738e1a707156c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Wed, 20 May 2020 00:34:32 -0300
Subject: [PATCH] * Cut down compile-time emitting static data as string
literals
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This change drastically reduce compile time. Apparently GCC optimizer
does not scale up well at all for long sequences of assignments into a
single array.
Nicolás Bértolo <nicolasbertolo@gmail.com>
Andrea Corallo <akrl@sdf.org>
* src/comp.c (gcc_jit_context_new_string_literal)
(gcc_jit_block_add_assignment_op): New imports.
(comp_t): New 'size_t_type' 'memcpy' fields.
(emit_static_object): Define static objects using string literals
and memcpy.
(define_memcpy): New function.
(Fcomp__init_ctxt): Define 'size_t_type' and 'memcpy'.
---
src/comp.c | 120 ++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 105 insertions(+), 15 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index f288fc2551..81c4d2fe32 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -46,6 +46,7 @@
# include "w32common.h"
#undef gcc_jit_block_add_assignment
+#undef gcc_jit_block_add_assignment_op
#undef gcc_jit_block_add_comment
#undef gcc_jit_block_add_eval
#undef gcc_jit_block_end_with_conditional
@@ -75,6 +76,7 @@
#undef gcc_jit_context_new_rvalue_from_int
#undef gcc_jit_context_new_rvalue_from_long
#undef gcc_jit_context_new_rvalue_from_ptr
+#undef gcc_jit_context_new_string_literal
#undef gcc_jit_context_new_struct_type
#undef gcc_jit_context_new_unary_op
#undef gcc_jit_context_new_union_type
@@ -164,6 +166,8 @@ DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_rvalue_from_long,
(gcc_jit_context *ctxt, gcc_jit_type *numeric_type, long value));
DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_rvalue_from_ptr,
(gcc_jit_context *ctxt, gcc_jit_type *pointer_type, void *value));
+DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_string_literal,
+ (gcc_jit_context *ctxt, const char *value));
DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_unary_op,
(gcc_jit_context *ctxt, gcc_jit_location *loc,
enum gcc_jit_unary_op op, gcc_jit_type *result_type,
@@ -197,6 +201,10 @@ DEF_DLL_FN (gcc_jit_type *, gcc_jit_type_get_pointer, (gcc_jit_type *type));
DEF_DLL_FN (void, gcc_jit_block_add_assignment,
(gcc_jit_block *block, gcc_jit_location *loc, gcc_jit_lvalue *lvalue,
gcc_jit_rvalue *rvalue));
+DEF_DLL_FN (void, gcc_jit_block_add_assignment_op,
+ (gcc_jit_block *block, gcc_jit_location *loc,
+ gcc_jit_lvalue *lvalue, enum gcc_jit_binary_op op,
+ gcc_jit_rvalue *rvalue));
DEF_DLL_FN (void, gcc_jit_block_add_eval,
(gcc_jit_block *block, gcc_jit_location *loc,
gcc_jit_rvalue *rvalue));
@@ -239,6 +247,7 @@ init_gccjit_functions (void)
/* In alphabetical order */
LOAD_DLL_FN (library, gcc_jit_block_add_assignment);
+ LOAD_DLL_FN (library, gcc_jit_block_add_assignment_op);
LOAD_DLL_FN (library, gcc_jit_block_add_comment);
LOAD_DLL_FN (library, gcc_jit_block_add_eval);
LOAD_DLL_FN (library, gcc_jit_block_end_with_conditional);
@@ -268,6 +277,7 @@ init_gccjit_functions (void)
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_int);
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_long);
LOAD_DLL_FN (library, gcc_jit_context_new_rvalue_from_ptr);
+ LOAD_DLL_FN (library, gcc_jit_context_new_string_literal);
LOAD_DLL_FN (library, gcc_jit_context_new_struct_type);
LOAD_DLL_FN (library, gcc_jit_context_new_unary_op);
LOAD_DLL_FN (library, gcc_jit_context_new_union_type);
@@ -296,6 +306,7 @@ init_gccjit_functions (void)
/* In alphabetical order */
#define gcc_jit_block_add_assignment fn_gcc_jit_block_add_assignment
+#define gcc_jit_block_add_assignment_op fn_gcc_jit_block_add_assignment_op
#define gcc_jit_block_add_comment fn_gcc_jit_block_add_comment
#define gcc_jit_block_add_eval fn_gcc_jit_block_add_eval
#define gcc_jit_block_end_with_conditional fn_gcc_jit_block_end_with_conditional
@@ -325,6 +336,7 @@ #define gcc_jit_context_new_param fn_gcc_jit_context_new_param
#define gcc_jit_context_new_rvalue_from_int fn_gcc_jit_context_new_rvalue_from_int
#define gcc_jit_context_new_rvalue_from_long fn_gcc_jit_context_new_rvalue_from_long
#define gcc_jit_context_new_rvalue_from_ptr fn_gcc_jit_context_new_rvalue_from_ptr
+#define gcc_jit_context_new_string_literal fn_gcc_jit_context_new_string_literal
#define gcc_jit_context_new_struct_type fn_gcc_jit_context_new_struct_type
#define gcc_jit_context_new_unary_op fn_gcc_jit_context_new_unary_op
#define gcc_jit_context_new_union_type fn_gcc_jit_context_new_union_type
@@ -462,6 +474,7 @@ #define F_RELOC_MAX_SIZE 1500
gcc_jit_type *char_ptr_type;
gcc_jit_type *ptrdiff_type;
gcc_jit_type *uintptr_type;
+ gcc_jit_type *size_t_type;
#if LISP_WORDS_ARE_POINTERS
gcc_jit_type *lisp_X;
#endif
@@ -548,6 +561,7 @@ #define F_RELOC_MAX_SIZE 1500
gcc_jit_rvalue *data_relocs_ephemeral;
/* Synthesized struct holding func relocs. */
gcc_jit_lvalue *func_relocs;
+ gcc_jit_function *memcpy;
Lisp_Object d_default_idx;
Lisp_Object d_impure_idx;
Lisp_Object d_ephemeral_idx;
@@ -2347,7 +2361,7 @@ emit_static_object (const char *name, Lisp_Object obj)
/* libgccjit has no support for initialized static data.
The mechanism below is certainly not aesthetic but I assume the bottle neck
in terms of performance at load time will still be the reader.
- NOTE: we can not relay on libgccjit even for valid NULL terminated C
+ NOTE: we can not rely on libgccjit even for valid NULL terminated C
strings cause of this funny bug that will affect all pre gcc10 era gccs:
https://gcc.gnu.org/ml/jit/2019-q3/msg00013.html */
@@ -2405,22 +2419,78 @@ emit_static_object (const char *name, Lisp_Object obj)
gcc_jit_lvalue *arr =
gcc_jit_lvalue_access_field (data_struct, NULL, fields[1]);
- for (ptrdiff_t i = 0; i < len; i++, p++)
+ gcc_jit_lvalue *ptrvar = gcc_jit_function_new_local (f, NULL,
+ comp.char_ptr_type,
+ "ptr");
+
+ gcc_jit_block_add_assignment (
+ block,
+ NULL,
+ ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (
+ comp.ctxt,
+ NULL,
+ gcc_jit_lvalue_as_rvalue (arr),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt, comp.int_type, 0)),
+ NULL));
+
+ for (ptrdiff_t i = 0; i < len;)
{
- gcc_jit_block_add_assignment (
- block,
- NULL,
- gcc_jit_context_new_array_access (
- comp.ctxt,
- NULL,
- gcc_jit_lvalue_as_rvalue (arr),
- gcc_jit_context_new_rvalue_from_int (comp.ctxt,
- comp.ptrdiff_type,
- i)),
- gcc_jit_context_new_rvalue_from_int (comp.ctxt,
- comp.char_type,
- *p));
+ /* We can't use string literals longer that 200 bytes because
+ they cause a crash in older versions of gccjit.
+ https://gcc.gnu.org/ml/jit/2019-q3/msg00013.html. */
+ char str[200];
+ strncpy (str, p, 200);
+ str[199] = 0;
+ uintptr_t l = strlen (str);
+
+ if (l != 0)
+ {
+ p += l;
+ i += l;
+
+ gcc_jit_rvalue *args[3]
+ = {gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_string_literal (comp.ctxt, str),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.size_t_type,
+ l)};
+
+ gcc_jit_block_add_eval (block, NULL,
+ gcc_jit_context_new_call (comp.ctxt, NULL,
+ comp.memcpy,
+ ARRAYELTS (args),
+ args));
+ gcc_jit_block_add_assignment (block, NULL, ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (comp.ctxt, NULL,
+ gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type,
+ l)),
+ NULL));
+ }
+ else
+ {
+ /* If strlen returned 0 that means that the static object
+ contains a NULL byte. In that case just move over to the
+ next block. We can rely on the byte being zero because
+ of the previous call to bzero and because the dynamic
+ linker cleared it. */
+ p++;
+ i++;
+ gcc_jit_block_add_assignment (
+ block, NULL, ptrvar,
+ gcc_jit_lvalue_get_address (
+ gcc_jit_context_new_array_access (
+ comp.ctxt, NULL, gcc_jit_lvalue_as_rvalue (ptrvar),
+ gcc_jit_context_new_rvalue_from_int (comp.ctxt,
+ comp.uintptr_type, 1)),
+ NULL));
+ }
}
+
gcc_jit_block_add_assignment (
block,
NULL,
@@ -2766,6 +2836,21 @@ define_jmp_buf (void)
1, &field);
}
+static void
+define_memcpy (void)
+{
+
+ gcc_jit_param *params[] =
+ { gcc_jit_context_new_param (comp.ctxt, NULL, comp.void_ptr_type, "dest"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.void_ptr_type, "src"),
+ gcc_jit_context_new_param (comp.ctxt, NULL, comp.size_t_type, "n") };
+
+ comp.memcpy =
+ gcc_jit_context_new_function (comp.ctxt, NULL, GCC_JIT_FUNCTION_IMPORTED,
+ comp.void_ptr_type, "memcpy",
+ ARRAYELTS (params), params, false);
+}
+
/* struct handler definition */
static void
@@ -3772,6 +3857,9 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt,
comp.uintptr_type = gcc_jit_context_get_int_type (comp.ctxt,
sizeof (void *),
false);
+ comp.size_t_type = gcc_jit_context_get_int_type (comp.ctxt,
+ sizeof (size_t),
+ false);
comp.exported_funcs_h = CALLN (Fmake_hash_table, QCtest, Qequal);
/*
@@ -3780,6 +3868,8 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt,
*/
comp.imported_funcs_h = CALLN (Fmake_hash_table);
+ define_memcpy ();
+
/* Define data structures. */
define_lisp_cons ();
--
2.17.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 16:57 ` Andrea Corallo
@ 2020-05-31 17:26 ` Nicolas Bértolo
2020-05-31 18:11 ` Andrea Corallo
0 siblings, 1 reply; 18+ messages in thread
From: Nicolas Bértolo @ 2020-05-31 17:26 UTC (permalink / raw)
To: Andrea Corallo; +Cc: 41615
> I believe bzero is unnecessary given these are static allocated.
Ok with me.
> For memcpy we can just use the standard library implementation given
> elns are linked to it. The other advantage is that doing this way (here
> at least) memcpy is not inlined also at speed 3, so we don't trap in the
> optimizer issue!
This is good!
> All summed is even a little faster than the stock patch and closer to
> the one with the specific GCC blob support.
Good.
> Let me know if you like the attached and if does the job for you too.
I like it. I see calls to memcpy even with -O3, which is great.
Nico
El dom., 31 may. 2020 a las 13:57, Andrea Corallo (<akrl@sdf.org>) escribió:
>
> Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
>
> >> I like this considerably less :)
> >
> > Ok, let's say goodbye to this patch.
> >
> >> It introduces quite some complexity and the same advantage in
> >> debuggability can be achieved with something like the attached 8 line
> >> patch (untested).
> >
> > Sounds good, I haven't tested it either.
> >
> >> Generally speaking I want to try to keep our back-end as simple as we
> >> manage to.
> >
> > I initially wrote this patch chasing the reason for slow compile times. I think
> > that a 10k line C file should be compiled much faster than what gccjit achieves.
> > I thought that "uncommon" (for C) ways of doing thing were causing gccjit to get
> > stuck trying to optimize them hard, until it gave up. I thought that filling the
> > static data using memcpy() and constant strings would help GCC recognize this as
> > a constant initialization and hopefully just store a completely initialized copy
> > in memory.
> >
> > I found that GCC would inline memcpy() and the static initialization would turn
> > into a very long unrolled loop with SSE instructions. I tested this with -O3
> > only in gccjit to force maximum optimization. I found this super strange
> > considering that -ftree-loop-distribute-patterns is enabled at -O3 and it should
> > recognize the naive_memcpy() function as an implementation of memcpy() and issue
> > calls to libc's implementation. Instead, it was inlining and unrolling it.
>
> Ok you confirm the suspects I wrote in the other mail!
>
> I've used your patch as a base, apart for minors here and there I've
> stripped out the definitions of bzero and memcpy.
>
> I believe bzero is unnecessary given these are static allocated.
>
> For memcpy we can just use the standard library implementation given
> elns are linked to it. The other advantage is that doing this way (here
> at least) memcpy is not inlined also at speed 3, so we don't trap in the
> optimizer issue!
>
> All summed is even a little faster than the stock patch and closer to
> the one with the specific GCC blob support.
>
> Let me know if you like the attached and if does the job for you too.
>
> Thanks
>
> Andrea
>
> --
> akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 17:26 ` Nicolas Bértolo
@ 2020-05-31 18:11 ` Andrea Corallo
2020-05-31 19:38 ` Nicolas Bértolo
0 siblings, 1 reply; 18+ messages in thread
From: Andrea Corallo @ 2020-05-31 18:11 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
>> I believe bzero is unnecessary given these are static allocated.
>
> Ok with me.
>
>> For memcpy we can just use the standard library implementation given
>> elns are linked to it. The other advantage is that doing this way (here
>> at least) memcpy is not inlined also at speed 3, so we don't trap in the
>> optimizer issue!
>
> This is good!
>
>> All summed is even a little faster than the stock patch and closer to
>> the one with the specific GCC blob support.
>
> Good.
>
>> Let me know if you like the attached and if does the job for you too.
>
> I like it. I see calls to memcpy even with -O3, which is great.
>
> Nico
Great, I've measured the startup time here on GNU/Linux 64 bit and is
slower by few milliseconds in average therefore totally acceptable.
The whole compilation is something like 5x faster here.
I've pushed it as 3efb2808d4
Thanks
Andrea
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 18:11 ` Andrea Corallo
@ 2020-05-31 19:38 ` Nicolas Bértolo
2020-05-31 20:00 ` Andrea Corallo
2020-06-01 7:19 ` Andrea Corallo
0 siblings, 2 replies; 18+ messages in thread
From: Nicolas Bértolo @ 2020-05-31 19:38 UTC (permalink / raw)
To: Andrea Corallo; +Cc: 41615
> The whole compilation is something like 5x faster here.
Amazing.I took a closer look at the code that uses casts to bools and
I think I found a
bug.
Casting to bool using an enum is equivalent to taking the lowest byte using
a byte mask. This wrongly casts to "false" integers whose lowest byte is nil.
bool cast_from_unsigned_long_long_to_bool (unsigned long long x)
{
return (x & 0xFF);
}
The correct way to cast to bool is to mimic C semantics:
bool cast_from_unsigned_long_long_to_bool (unsigned long long x)
{
if (x != 0)
return true;
else
return false;
}
Am I right?
Nico.
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 19:38 ` Nicolas Bértolo
@ 2020-05-31 20:00 ` Andrea Corallo
2020-05-31 20:18 ` Andrea Corallo
2020-06-01 7:19 ` Andrea Corallo
1 sibling, 1 reply; 18+ messages in thread
From: Andrea Corallo @ 2020-05-31 20:00 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
>> The whole compilation is something like 5x faster here.
> Amazing.I took a closer look at the code that uses casts to bools and
> I think I found a
> bug.
>
> Casting to bool using an enum is equivalent to taking the lowest byte using
> a byte mask. This wrongly casts to "false" integers whose lowest byte is nil.
>
> bool cast_from_unsigned_long_long_to_bool (unsigned long long x)
> {
> return (x & 0xFF);
> }
>
> The correct way to cast to bool is to mimic C semantics:
>
> bool cast_from_unsigned_long_long_to_bool (unsigned long long x)
> {
> if (x != 0)
> return true;
> else
> return false;
> }
>
> Am I right?
You are.
I never fixed it (is present also in the pure union cast mechanism)
because I had not time so far and practically I suspect is not a real
case of problematic code we generate. But is *certanly* good to fix.
BTW If you are into the mood to dig into these... also sign extentions
probably should be handled ;)
Andrea
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 20:00 ` Andrea Corallo
@ 2020-05-31 20:18 ` Andrea Corallo
0 siblings, 0 replies; 18+ messages in thread
From: Andrea Corallo @ 2020-05-31 20:18 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
Andrea Corallo <akrl@sdf.org> writes:
> BTW If you are into the mood to dig into these... also sign extentions
> probably should be handled ;)
BTW2 before jumping into sign extentions we should prove we have some
case of this we care about, I'd probably start just placing an assertion
in emit_coerce.
Andrea
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-05-31 19:38 ` Nicolas Bértolo
2020-05-31 20:00 ` Andrea Corallo
@ 2020-06-01 7:19 ` Andrea Corallo
2020-06-01 12:25 ` Nicolas Bértolo
1 sibling, 1 reply; 18+ messages in thread
From: Andrea Corallo @ 2020-06-01 7:19 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
>> The whole compilation is something like 5x faster here.
> Amazing.I took a closer look at the code that uses casts to bools and
> I think I found a
> bug.
>
> Casting to bool using an enum is equivalent to taking the lowest byte using
> a byte mask. This wrongly casts to "false" integers whose lowest byte is nil.
>
> bool cast_from_unsigned_long_long_to_bool (unsigned long long x)
> {
> return (x & 0xFF);
> }
>
> The correct way to cast to bool is to mimic C semantics:
>
> bool cast_from_unsigned_long_long_to_bool (unsigned long long x)
> {
> if (x != 0)
> return true;
> else
> return false;
> }
>
> Am I right?
>
> Nico.
Okay, now I recall better the whole story.
I believe is okay that emit_coerce can truncate numbers (as regular cast
can do).
Where we have to be careful is into coercing before calling
'emit_cond_jump', again the same attention we use in C when casting
values.
BTW IIRC I've experienced libgccjit crashed on brances with non boolean
tests. At the time I cured that directly in 'emit_cond_jump' performing
a negation on the number to extract the boolean to be used as a test,
this works well.
Andrea
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-06-01 7:19 ` Andrea Corallo
@ 2020-06-01 12:25 ` Nicolas Bértolo
2020-06-01 20:28 ` Andrea Corallo
0 siblings, 1 reply; 18+ messages in thread
From: Nicolas Bértolo @ 2020-06-01 12:25 UTC (permalink / raw)
To: Andrea Corallo; +Cc: 41615
[-- Attachment #1: Type: text/plain, Size: 278 bytes --]
I rewrote the "cast with functions" patch and added a few more patches.
- Implement cast to bool as !!x instead of (x & 0xFF).
- Throw an ICE when asked to perform sign extension. I didn't see any
issues with this one. So for now it is not necessary to implement them.
Nico.
[-- Attachment #2: 0003-Implement-casts-to-bool-using-double-negation-like-i.patch --]
[-- Type: application/octet-stream, Size: 3761 bytes --]
From 39b8d7b0bbcda4f4f34759b02cc2cf30523536ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Sun, 31 May 2020 17:24:03 -0300
Subject: [PATCH 3/4] Implement casts to bool using double negation like in C.
* src/comp.c (define_cast_to_bool): New function that uses double
negation to cast to bool.
---
src/comp.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/src/comp.c b/src/comp.c
index 6f92801b13f..ed2e5f37338 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -2909,10 +2909,76 @@ define_thread_state_struct (void)
size_t bytes_size;
};
+static gcc_jit_function *
+define_cast_to_bool (struct cast_type from, int from_index)
+{
+ char *name = format_string ("cast_from_%s_to_%s", from.name, "bool");
+ gcc_jit_param *param = gcc_jit_context_new_param (comp.ctxt, NULL,
+ from.type, "arg");
+ gcc_jit_function *result = gcc_jit_context_new_function (comp.ctxt,
+ NULL,
+ GCC_JIT_FUNCTION_INTERNAL,
+ comp.bool_type,
+ name,
+ 1,
+ ¶m,
+ 0);
+
+ DECL_BLOCK (entry_block, result);
+
+ gcc_jit_lvalue *tmp_union
+ = gcc_jit_function_new_local (result,
+ NULL,
+ comp.cast_union_type,
+ "union_cast");
+
+ /* Zero the union first. */
+ gcc_jit_block_add_assignment (entry_block, NULL,
+ gcc_jit_lvalue_access_field (tmp_union, NULL,
+ comp.cast_union_fields[NUM_CAST_TYPES]),
+ gcc_jit_context_new_rvalue_from_int (
+ comp.ctxt,
+ comp.cast_types[NUM_CAST_TYPES],
+ 0));
+
+ gcc_jit_block_add_assignment (entry_block, NULL,
+ gcc_jit_lvalue_access_field (tmp_union, NULL,
+ comp.cast_union_fields[from_index]),
+ gcc_jit_param_as_rvalue (param));
+
+ gcc_jit_rvalue *cast_to_biggest
+ = gcc_jit_rvalue_access_field (gcc_jit_lvalue_as_rvalue (tmp_union),
+ NULL,
+ comp.cast_union_fields[NUM_CAST_TYPES]);
+
+ gcc_jit_rvalue *first_negation
+ = gcc_jit_context_new_unary_op (comp.ctxt,
+ NULL,
+ GCC_JIT_UNARY_OP_LOGICAL_NEGATE,
+ comp.bool_type,
+ cast_to_biggest);
+
+ gcc_jit_rvalue *second_negation
+ = gcc_jit_context_new_unary_op (comp.ctxt,
+ NULL,
+ GCC_JIT_UNARY_OP_LOGICAL_NEGATE,
+ comp.bool_type,
+ first_negation);
+
+ gcc_jit_block_end_with_return (entry_block,
+ NULL,
+ second_negation);
+
+ return result;
+}
+
static gcc_jit_function *
define_cast_from_to (struct cast_type from, int from_index, struct cast_type to,
int to_index)
{
+ if (to.type == comp.bool_type)
+ return define_cast_to_bool (from, from_index);
+
char *name = format_string ("cast_from_%s_to_%s", from.name, to.name);
gcc_jit_param *param = gcc_jit_context_new_param (comp.ctxt, NULL,
from.type, "arg");
--
2.25.1.windows.1
[-- Attachment #3: 0004-Throw-an-ICE-when-asked-to-emit-a-cast-with-sign-ext.patch --]
[-- Type: application/octet-stream, Size: 6248 bytes --]
From 435ed84c6df4911b238f67c79492533e0c71ca46 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Sun, 31 May 2020 18:09:12 -0300
Subject: [PATCH 4/4] Throw an ICE when asked to emit a cast with sign
extension.
* src/comp.c (cast_kind_of_type): Enum that specifies the kind of type
in the cast enum (unsigned, signed, pointer).
(emit_coerce): Throw an ICE when asked to emit a cast with sign
extension.
(define_cast_from_to): Return NULL for casts involving sign extension.
(define_cast_functions): Specify the kind of each type in the cast
union.
---
src/comp.c | 58 ++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 43 insertions(+), 15 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index ed2e5f37338..8dc936f451a 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -450,6 +450,13 @@ #define F_RELOC_MAX_SIZE 1500
#define NUM_CAST_TYPES 15
+enum cast_kind_of_type
+ {
+ kind_unsigned,
+ kind_signed,
+ kind_pointer
+ };
+
/* C side of the compiler context. */
typedef struct {
@@ -514,6 +521,7 @@ #define NUM_CAST_TYPES 15
member. */
gcc_jit_type *cast_types[NUM_CAST_TYPES+1];
size_t cast_type_sizes[NUM_CAST_TYPES+1];
+ enum cast_kind_of_type cast_type_kind[NUM_CAST_TYPES+1];
const char *cast_type_names[NUM_CAST_TYPES+1];
gcc_jit_field *cast_union_fields[NUM_CAST_TYPES+1];
size_t cast_union_field_biggest_type;
@@ -980,6 +988,13 @@ emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj)
int old_index = type_to_cast_index (old_type);
int new_index = type_to_cast_index (new_type);
+ if (comp.cast_type_sizes[old_index] < comp.cast_type_sizes[new_index]
+ && comp.cast_type_kind[new_index] == kind_signed)
+ xsignal3 (Qnative_ice,
+ build_string ("FIXME: sign extension not implemented"),
+ build_string (comp.cast_type_names[old_index]),
+ build_string (comp.cast_type_names[new_index]));
+
/* Lookup the appropriate cast function in the cast matrix. */
return gcc_jit_context_new_call (comp.ctxt,
NULL,
@@ -2907,6 +2922,7 @@ define_thread_state_struct (void)
gcc_jit_type *type;
const char *name;
size_t bytes_size;
+ enum cast_kind_of_type kind;
};
static gcc_jit_function *
@@ -2979,6 +2995,11 @@ define_cast_from_to (struct cast_type from, int from_index, struct cast_type to,
if (to.type == comp.bool_type)
return define_cast_to_bool (from, from_index);
+ /* FIXME: sign extension not implemented. */
+ if (comp.cast_type_sizes[from_index] < comp.cast_type_sizes[to_index]
+ && comp.cast_type_kind[to_index] == kind_signed)
+ return NULL;
+
char *name = format_string ("cast_from_%s_to_%s", from.name, to.name);
gcc_jit_param *param = gcc_jit_context_new_param (comp.ctxt, NULL,
from.type, "arg");
@@ -3027,22 +3048,27 @@ define_cast_from_to (struct cast_type from, int from_index, struct cast_type to,
define_cast_functions (void)
{
struct cast_type cast_types[NUM_CAST_TYPES]
- = { { comp.bool_type, "bool", sizeof (bool) },
- { comp.char_ptr_type, "char_ptr", sizeof (char *) },
- { comp.int_type, "int", sizeof (int) },
- { comp.lisp_cons_ptr_type, "cons_ptr", sizeof (struct Lisp_Cons *) },
- { comp.lisp_obj_ptr_type, "lisp_obj_ptr", sizeof (Lisp_Object *) },
- { comp.lisp_word_tag_type, "lisp_word_tag", sizeof (Lisp_Word_tag) },
- { comp.lisp_word_type, "lisp_word", sizeof (Lisp_Word) },
- { comp.long_long_type, "long_long", sizeof (long long) },
- { comp.long_type, "long", sizeof (long) },
- { comp.ptrdiff_type, "ptrdiff", sizeof (ptrdiff_t) },
- { comp.uintptr_type, "uintptr", sizeof (uintptr_t) },
+ = { { comp.bool_type, "bool", sizeof (bool), kind_unsigned },
+ { comp.char_ptr_type, "char_ptr", sizeof (char *), kind_pointer },
+ { comp.int_type, "int", sizeof (int), kind_signed },
+ { comp.lisp_cons_ptr_type, "cons_ptr", sizeof (struct Lisp_Cons *),
+ kind_pointer },
+ { comp.lisp_obj_ptr_type, "lisp_obj_ptr", sizeof (Lisp_Object *),
+ kind_pointer },
+ { comp.lisp_word_tag_type, "lisp_word_tag", sizeof (Lisp_Word_tag),
+ kind_unsigned },
+ { comp.lisp_word_type, "lisp_word", sizeof (Lisp_Word),
+ LISP_WORDS_ARE_POINTERS ? kind_pointer : kind_signed },
+ { comp.long_long_type, "long_long", sizeof (long long), kind_signed },
+ { comp.long_type, "long", sizeof (long), kind_signed },
+ { comp.ptrdiff_type, "ptrdiff", sizeof (ptrdiff_t), kind_signed },
+ { comp.uintptr_type, "uintptr", sizeof (uintptr_t), kind_unsigned },
{ comp.unsigned_long_long_type, "unsigned_long_long",
- sizeof (unsigned long long) },
- { comp.unsigned_long_type, "unsigned_long", sizeof (unsigned long) },
- { comp.unsigned_type, "unsigned", sizeof (unsigned) },
- { comp.void_ptr_type, "void_ptr", sizeof (void*) } };
+ sizeof (unsigned long long), kind_unsigned },
+ { comp.unsigned_long_type, "unsigned_long", sizeof (unsigned long),
+ kind_unsigned },
+ { comp.unsigned_type, "unsigned", sizeof (unsigned), kind_unsigned },
+ { comp.void_ptr_type, "void_ptr", sizeof (void*), kind_pointer } };
/* Find the biggest size. It should be unsigned long long, but to be
sure we find it programmatically. */
@@ -3060,6 +3086,7 @@ define_cast_functions (void)
cast_types[i].name);
comp.cast_type_names[i] = cast_types[i].name;
comp.cast_type_sizes[i] = cast_types[i].bytes_size;
+ comp.cast_type_kind[i] = cast_types[i].kind;
}
gcc_jit_type *biggest_type = gcc_jit_context_get_int_type (comp.ctxt,
@@ -3070,6 +3097,7 @@ define_cast_functions (void)
= gcc_jit_context_new_field (comp.ctxt, NULL, biggest_type, "biggest_type");
comp.cast_type_names[NUM_CAST_TYPES] = "biggest_type";
comp.cast_type_sizes[NUM_CAST_TYPES] = biggest_size;
+ comp.cast_type_kind[NUM_CAST_TYPES] = kind_unsigned;
comp.cast_union_type =
gcc_jit_context_new_union_type (comp.ctxt,
--
2.25.1.windows.1
[-- Attachment #4: 0001-Remove-unnecessary-DLL-load-of-gcc_jit_block_add_ass.patch --]
[-- Type: application/octet-stream, Size: 2111 bytes --]
From ec7af221c7dbf9b9fc551ef38d6e70a1bf09d4e8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Sun, 31 May 2020 15:55:18 -0300
Subject: [PATCH 1/4] Remove unnecessary DLL load of
gcc_jit_block_add_assignment_op.
---
src/comp.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index 81c4d2fe32a..8d8094e9869 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -46,7 +46,6 @@
# include "w32common.h"
#undef gcc_jit_block_add_assignment
-#undef gcc_jit_block_add_assignment_op
#undef gcc_jit_block_add_comment
#undef gcc_jit_block_add_eval
#undef gcc_jit_block_end_with_conditional
@@ -201,10 +200,6 @@ DEF_DLL_FN (gcc_jit_type *, gcc_jit_type_get_pointer, (gcc_jit_type *type));
DEF_DLL_FN (void, gcc_jit_block_add_assignment,
(gcc_jit_block *block, gcc_jit_location *loc, gcc_jit_lvalue *lvalue,
gcc_jit_rvalue *rvalue));
-DEF_DLL_FN (void, gcc_jit_block_add_assignment_op,
- (gcc_jit_block *block, gcc_jit_location *loc,
- gcc_jit_lvalue *lvalue, enum gcc_jit_binary_op op,
- gcc_jit_rvalue *rvalue));
DEF_DLL_FN (void, gcc_jit_block_add_eval,
(gcc_jit_block *block, gcc_jit_location *loc,
gcc_jit_rvalue *rvalue));
@@ -247,7 +242,6 @@ init_gccjit_functions (void)
/* In alphabetical order */
LOAD_DLL_FN (library, gcc_jit_block_add_assignment);
- LOAD_DLL_FN (library, gcc_jit_block_add_assignment_op);
LOAD_DLL_FN (library, gcc_jit_block_add_comment);
LOAD_DLL_FN (library, gcc_jit_block_add_eval);
LOAD_DLL_FN (library, gcc_jit_block_end_with_conditional);
@@ -306,7 +300,6 @@ init_gccjit_functions (void)
/* In alphabetical order */
#define gcc_jit_block_add_assignment fn_gcc_jit_block_add_assignment
-#define gcc_jit_block_add_assignment_op fn_gcc_jit_block_add_assignment_op
#define gcc_jit_block_add_comment fn_gcc_jit_block_add_comment
#define gcc_jit_block_add_eval fn_gcc_jit_block_add_eval
#define gcc_jit_block_end_with_conditional fn_gcc_jit_block_end_with_conditional
--
2.25.1.windows.1
[-- Attachment #5: 0002-Define-casts-using-functions.patch --]
[-- Type: application/octet-stream, Size: 14051 bytes --]
From 91189343ccd6943eafc2f3dc8b3b19b8ea879903 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
Date: Sat, 30 May 2020 18:33:58 -0300
Subject: [PATCH 2/4] Define casts using functions.
This is to dump prettier C files.
This does not affect compilation times in my tests.
* src/comp.c: Define a 15x15 cast matrix. Use it in emit_coerce().
---
src/comp.c | 313 +++++++++++++++++++++++------------------------------
1 file changed, 138 insertions(+), 175 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index 8d8094e9869..6f92801b13f 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -448,6 +448,8 @@ #define F_RELOC_MAX_SIZE 1500
static f_reloc_t freloc;
+#define NUM_CAST_TYPES 15
+
/* C side of the compiler context. */
typedef struct {
@@ -507,21 +509,14 @@ #define F_RELOC_MAX_SIZE 1500
/* libgccjit has really limited support for casting therefore this union will
be used for the scope. */
gcc_jit_type *cast_union_type;
- gcc_jit_field *cast_union_as_ll;
- gcc_jit_field *cast_union_as_ull;
- gcc_jit_field *cast_union_as_l;
- gcc_jit_field *cast_union_as_ul;
- gcc_jit_field *cast_union_as_u;
- gcc_jit_field *cast_union_as_i;
- gcc_jit_field *cast_union_as_b;
- gcc_jit_field *cast_union_as_uintptr;
- gcc_jit_field *cast_union_as_ptrdiff;
- gcc_jit_field *cast_union_as_c_p;
- gcc_jit_field *cast_union_as_v_p;
- gcc_jit_field *cast_union_as_lisp_cons_ptr;
- gcc_jit_field *cast_union_as_lisp_word;
- gcc_jit_field *cast_union_as_lisp_word_tag;
- gcc_jit_field *cast_union_as_lisp_obj_ptr;
+ gcc_jit_function *cast_functions_from_to[NUM_CAST_TYPES][NUM_CAST_TYPES];
+ /* We add one to make space for the last member which is the "biggest_type"
+ member. */
+ gcc_jit_type *cast_types[NUM_CAST_TYPES+1];
+ size_t cast_type_sizes[NUM_CAST_TYPES+1];
+ const char *cast_type_names[NUM_CAST_TYPES+1];
+ gcc_jit_field *cast_union_fields[NUM_CAST_TYPES+1];
+ size_t cast_union_field_biggest_type;
gcc_jit_function *func; /* Current function being compiled. */
bool func_has_non_local; /* From comp-func has-non-local slot. */
gcc_jit_lvalue **f_frame; /* "Floating" frame for the current function. */
@@ -678,47 +673,6 @@ bcall0 (Lisp_Object f)
Ffuncall (1, &f);
}
-static gcc_jit_field *
-type_to_cast_field (gcc_jit_type *type)
-{
- gcc_jit_field *field;
-
- if (type == comp.long_long_type)
- field = comp.cast_union_as_ll;
- else if (type == comp.unsigned_long_long_type)
- field = comp.cast_union_as_ull;
- else if (type == comp.long_type)
- field = comp.cast_union_as_l;
- else if (type == comp.unsigned_long_type)
- field = comp.cast_union_as_ul;
- else if (type == comp.unsigned_type)
- field = comp.cast_union_as_u;
- else if (type == comp.int_type)
- field = comp.cast_union_as_i;
- else if (type == comp.bool_type)
- field = comp.cast_union_as_b;
- else if (type == comp.void_ptr_type)
- field = comp.cast_union_as_v_p;
- else if (type == comp.uintptr_type)
- field = comp.cast_union_as_uintptr;
- else if (type == comp.ptrdiff_type)
- field = comp.cast_union_as_ptrdiff;
- else if (type == comp.char_ptr_type)
- field = comp.cast_union_as_c_p;
- else if (type == comp.lisp_cons_ptr_type)
- field = comp.cast_union_as_lisp_cons_ptr;
- else if (type == comp.lisp_word_type)
- field = comp.cast_union_as_lisp_word;
- else if (type == comp.lisp_word_tag_type)
- field = comp.cast_union_as_lisp_word_tag;
- else if (type == comp.lisp_obj_ptr_type)
- field = comp.cast_union_as_lisp_obj_ptr;
- else
- xsignal1 (Qnative_ice, build_string ("unsupported cast"));
-
- return field;
-}
-
static gcc_jit_block *
retrive_block (Lisp_Object block_name)
{
@@ -979,11 +933,19 @@ emit_cond_jump (gcc_jit_rvalue *test,
}
+static int
+type_to_cast_index (gcc_jit_type * type)
+{
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ if (type == comp.cast_types[i])
+ return i;
+
+ xsignal1 (Qnative_ice, build_string ("unsupported cast"));
+}
+
static gcc_jit_rvalue *
emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj)
{
- static ptrdiff_t i;
-
gcc_jit_type *old_type = gcc_jit_rvalue_get_type (obj);
if (new_type == old_type)
@@ -1015,25 +977,14 @@ emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj)
}
#endif
- gcc_jit_field *orig_field =
- type_to_cast_field (old_type);
- gcc_jit_field *dest_field = type_to_cast_field (new_type);
-
- gcc_jit_lvalue *tmp_u =
- gcc_jit_function_new_local (comp.func,
- NULL,
- comp.cast_union_type,
- format_string ("union_cast_%td", i++));
- gcc_jit_block_add_assignment (comp.block,
- NULL,
- gcc_jit_lvalue_access_field (tmp_u,
- NULL,
- orig_field),
- obj);
+ int old_index = type_to_cast_index (old_type);
+ int new_index = type_to_cast_index (new_type);
- return gcc_jit_rvalue_access_field ( gcc_jit_lvalue_as_rvalue (tmp_u),
- NULL,
- dest_field);
+ /* Lookup the appropriate cast function in the cast matrix. */
+ return gcc_jit_context_new_call (comp.ctxt,
+ NULL,
+ comp.cast_functions_from_to[old_index][new_index],
+ 1, &obj);
}
static gcc_jit_rvalue *
@@ -1090,7 +1041,7 @@ emit_rvalue_from_long_long (gcc_jit_type *type, long long n)
gcc_jit_context_new_rvalue_from_int (comp.ctxt,
comp.unsigned_long_long_type,
32)),
- low));
+ low));
}
static gcc_jit_rvalue *
@@ -1132,7 +1083,7 @@ emit_rvalue_from_unsigned_long_long (gcc_jit_type *type, unsigned long long n)
gcc_jit_context_new_rvalue_from_int (comp.ctxt,
comp.unsigned_long_long_type,
32)),
- low));
+ low));
}
static gcc_jit_rvalue *
@@ -2951,109 +2902,121 @@ define_thread_state_struct (void)
gcc_jit_type_get_pointer (gcc_jit_struct_as_type (comp.thread_state_s));
}
-static void
-define_cast_union (void)
+struct cast_type
{
+ gcc_jit_type *type;
+ const char *name;
+ size_t bytes_size;
+};
- comp.cast_union_as_ll =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.long_long_type,
- "ll");
- comp.cast_union_as_ull =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_long_long_type,
- "ull");
- comp.cast_union_as_l =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.long_type,
- "l");
- comp.cast_union_as_ul =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_long_type,
- "ul");
- comp.cast_union_as_u =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.unsigned_type,
- "u");
- comp.cast_union_as_i =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.int_type,
- "i");
- comp.cast_union_as_b =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.bool_type,
- "b");
- comp.cast_union_as_uintptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.uintptr_type,
- "uintptr");
- comp.cast_union_as_ptrdiff =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.ptrdiff_type,
- "ptrdiff");
- comp.cast_union_as_c_p =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.char_ptr_type,
- "c_p");
- comp.cast_union_as_v_p =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.void_ptr_type,
- "v_p");
- comp.cast_union_as_lisp_cons_ptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_cons_ptr_type,
- "cons_ptr");
- comp.cast_union_as_lisp_word =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_word_type,
- "lisp_word");
- comp.cast_union_as_lisp_word_tag =
- gcc_jit_context_new_field (comp.ctxt,
+static gcc_jit_function *
+define_cast_from_to (struct cast_type from, int from_index, struct cast_type to,
+ int to_index)
+{
+ char *name = format_string ("cast_from_%s_to_%s", from.name, to.name);
+ gcc_jit_param *param = gcc_jit_context_new_param (comp.ctxt, NULL,
+ from.type, "arg");
+ gcc_jit_function *result = gcc_jit_context_new_function (comp.ctxt,
NULL,
- comp.lisp_word_tag_type,
- "lisp_word_tag");
- comp.cast_union_as_lisp_obj_ptr =
- gcc_jit_context_new_field (comp.ctxt,
- NULL,
- comp.lisp_obj_ptr_type,
- "lisp_obj_ptr");
-
-
- gcc_jit_field *cast_union_fields[] =
- { comp.cast_union_as_ll,
- comp.cast_union_as_ull,
- comp.cast_union_as_l,
- comp.cast_union_as_ul,
- comp.cast_union_as_u,
- comp.cast_union_as_i,
- comp.cast_union_as_b,
- comp.cast_union_as_uintptr,
- comp.cast_union_as_ptrdiff,
- comp.cast_union_as_c_p,
- comp.cast_union_as_v_p,
- comp.cast_union_as_lisp_cons_ptr,
- comp.cast_union_as_lisp_word,
- comp.cast_union_as_lisp_word_tag,
- comp.cast_union_as_lisp_obj_ptr };
+ GCC_JIT_FUNCTION_INTERNAL,
+ to.type,
+ name,
+ 1,
+ ¶m,
+ 0);
+
+ DECL_BLOCK (entry_block, result);
+
+ gcc_jit_lvalue *tmp_union
+ = gcc_jit_function_new_local (result,
+ NULL,
+ comp.cast_union_type,
+ "union_cast");
+
+ /* Zero the union first. */
+ gcc_jit_block_add_assignment (entry_block, NULL,
+ gcc_jit_lvalue_access_field (tmp_union, NULL,
+ comp.cast_union_fields[NUM_CAST_TYPES]),
+ gcc_jit_context_new_rvalue_from_int (
+ comp.ctxt,
+ comp.cast_types[NUM_CAST_TYPES],
+ 0));
+
+ gcc_jit_block_add_assignment (entry_block, NULL,
+ gcc_jit_lvalue_access_field (tmp_union, NULL,
+ comp.cast_union_fields[from_index]),
+ gcc_jit_param_as_rvalue (param));
+
+ gcc_jit_block_end_with_return (entry_block,
+ NULL,
+ gcc_jit_rvalue_access_field (
+ gcc_jit_lvalue_as_rvalue (tmp_union),
+ NULL,
+ comp.cast_union_fields[to_index]));
+
+ return result;
+}
+
+static void
+define_cast_functions (void)
+{
+ struct cast_type cast_types[NUM_CAST_TYPES]
+ = { { comp.bool_type, "bool", sizeof (bool) },
+ { comp.char_ptr_type, "char_ptr", sizeof (char *) },
+ { comp.int_type, "int", sizeof (int) },
+ { comp.lisp_cons_ptr_type, "cons_ptr", sizeof (struct Lisp_Cons *) },
+ { comp.lisp_obj_ptr_type, "lisp_obj_ptr", sizeof (Lisp_Object *) },
+ { comp.lisp_word_tag_type, "lisp_word_tag", sizeof (Lisp_Word_tag) },
+ { comp.lisp_word_type, "lisp_word", sizeof (Lisp_Word) },
+ { comp.long_long_type, "long_long", sizeof (long long) },
+ { comp.long_type, "long", sizeof (long) },
+ { comp.ptrdiff_type, "ptrdiff", sizeof (ptrdiff_t) },
+ { comp.uintptr_type, "uintptr", sizeof (uintptr_t) },
+ { comp.unsigned_long_long_type, "unsigned_long_long",
+ sizeof (unsigned long long) },
+ { comp.unsigned_long_type, "unsigned_long", sizeof (unsigned long) },
+ { comp.unsigned_type, "unsigned", sizeof (unsigned) },
+ { comp.void_ptr_type, "void_ptr", sizeof (void*) } };
+
+ /* Find the biggest size. It should be unsigned long long, but to be
+ sure we find it programmatically. */
+ size_t biggest_size = 0;
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ biggest_size = max (biggest_size, cast_types[i].bytes_size);
+
+ /* Define the union used for casting. */
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ {
+ comp.cast_types[i] = cast_types[i].type;
+ comp.cast_union_fields[i] = gcc_jit_context_new_field (comp.ctxt,
+ NULL,
+ cast_types[i].type,
+ cast_types[i].name);
+ comp.cast_type_names[i] = cast_types[i].name;
+ comp.cast_type_sizes[i] = cast_types[i].bytes_size;
+ }
+
+ gcc_jit_type *biggest_type = gcc_jit_context_get_int_type (comp.ctxt,
+ biggest_size,
+ false);
+ comp.cast_types[NUM_CAST_TYPES] = biggest_type;
+ comp.cast_union_fields[NUM_CAST_TYPES]
+ = gcc_jit_context_new_field (comp.ctxt, NULL, biggest_type, "biggest_type");
+ comp.cast_type_names[NUM_CAST_TYPES] = "biggest_type";
+ comp.cast_type_sizes[NUM_CAST_TYPES] = biggest_size;
+
comp.cast_union_type =
gcc_jit_context_new_union_type (comp.ctxt,
NULL,
"cast_union",
- ARRAYELTS (cast_union_fields),
- cast_union_fields);
+ NUM_CAST_TYPES+1,
+ comp.cast_union_fields);
+
+ /* Define the cast functions using a matrix. */
+ for (int i = 0; i < NUM_CAST_TYPES; ++i)
+ for (int j = 0; j < NUM_CAST_TYPES; ++j)
+ comp.cast_functions_from_to[i][j]
+ = define_cast_from_to (cast_types[i], i, cast_types[j], j);
}
static void
@@ -3869,7 +3832,7 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt,
define_jmp_buf ();
define_handler_struct ();
define_thread_state_struct ();
- define_cast_union ();
+ define_cast_functions ();
return Qt;
}
--
2.25.1.windows.1
^ permalink raw reply related [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-06-01 12:25 ` Nicolas Bértolo
@ 2020-06-01 20:28 ` Andrea Corallo
2020-06-01 23:11 ` Nicolas Bértolo
0 siblings, 1 reply; 18+ messages in thread
From: Andrea Corallo @ 2020-06-01 20:28 UTC (permalink / raw)
To: Nicolas Bértolo; +Cc: 41615
Nicolas Bértolo <nicolasbertolo@gmail.com> writes:
> I rewrote the "cast with functions" patch and added a few more patches.
> - Implement cast to bool as !!x instead of (x & 0xFF).
> - Throw an ICE when asked to perform sign extension. I didn't see any
> issues with this one. So for now it is not necessary to implement them.
>
> Nico.
>
> From 39b8d7b0bbcda4f4f34759b02cc2cf30523536ff Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
> Date: Sun, 31 May 2020 17:24:03 -0300
> Subject: [PATCH 3/4] Implement casts to bool using double negation like in C.
As I commented early I think this would be not ideal. The trick of the
negation is done already in emit_cond_jump and regarding the cast
operation I think is important to keep the C semantic (that is the one
we have).
I pushed the others with some very minor change, have a look.
> From 435ed84c6df4911b238f67c79492533e0c71ca46 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
> Date: Sun, 31 May 2020 18:09:12 -0300
> Subject: [PATCH 4/4] Throw an ICE when asked to emit a cast with sign
> extension.
>
> * src/comp.c (cast_kind_of_type): Enum that specifies the kind of type
> in the cast enum (unsigned, signed, pointer).
> (emit_coerce): Throw an ICE when asked to emit a cast with sign
> extension.
> (define_cast_from_to): Return NULL for casts involving sign extension.
> (define_cast_functions): Specify the kind of each type in the cast
> union.
> ---
> src/comp.c | 58 ++++++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 43 insertions(+), 15 deletions(-)
>
[...]
> typedef struct {
> @@ -514,6 +521,7 @@ #define NUM_CAST_TYPES 15
> member. */
> gcc_jit_type *cast_types[NUM_CAST_TYPES+1];
> size_t cast_type_sizes[NUM_CAST_TYPES+1];
> + enum cast_kind_of_type cast_type_kind[NUM_CAST_TYPES+1];
I've just added the spaces around + (here and for the other).
> From ec7af221c7dbf9b9fc551ef38d6e70a1bf09d4e8 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
> Date: Sun, 31 May 2020 15:55:18 -0300
> Subject: [PATCH 1/4] Remove unnecessary DLL load of
> gcc_jit_block_add_assignment_op.
I've jsut added a bare ChangeLog entry, pushed.
> From 91189343ccd6943eafc2f3dc8b3b19b8ea879903 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Nicol=C3=A1s=20B=C3=A9rtolo?= <nicolasbertolo@gmail.com>
> Date: Sat, 30 May 2020 18:33:58 -0300
> Subject: [PATCH 2/4] Define casts using functions.
>
> This is to dump prettier C files.
> This does not affect compilation times in my tests.
>
> * src/comp.c: Define a 15x15 cast matrix. Use it in emit_coerce().
[...]
> static gcc_jit_rvalue *
> @@ -1090,7 +1041,7 @@ emit_rvalue_from_long_long (gcc_jit_type *type, long long n)
> gcc_jit_context_new_rvalue_from_int (comp.ctxt,
> comp.unsigned_long_long_type,
> 32)),
> - low));
> + low));
I guess this was involuntary, I removed this change.
> }
>
> static gcc_jit_rvalue *
> @@ -1132,7 +1083,7 @@ emit_rvalue_from_unsigned_long_long (gcc_jit_type *type, unsigned long long n)
> gcc_jit_context_new_rvalue_from_int (comp.ctxt,
> comp.unsigned_long_long_type,
> 32)),
> - low));
> + low));
Likewise
> +static void
> +define_cast_functions (void)
> +{
> + struct cast_type cast_types[NUM_CAST_TYPES]
> + = { { comp.bool_type, "bool", sizeof (bool) },
> + { comp.char_ptr_type, "char_ptr", sizeof (char *) },
> + { comp.int_type, "int", sizeof (int) },
> + { comp.lisp_cons_ptr_type, "cons_ptr", sizeof (struct Lisp_Cons *) },
> + { comp.lisp_obj_ptr_type, "lisp_obj_ptr", sizeof (Lisp_Object *) },
> + { comp.lisp_word_tag_type, "lisp_word_tag", sizeof (Lisp_Word_tag) },
> + { comp.lisp_word_type, "lisp_word", sizeof (Lisp_Word) },
> + { comp.long_long_type, "long_long", sizeof (long long) },
> + { comp.long_type, "long", sizeof (long) },
> + { comp.ptrdiff_type, "ptrdiff", sizeof (ptrdiff_t) },
> + { comp.uintptr_type, "uintptr", sizeof (uintptr_t) },
> + { comp.unsigned_long_long_type, "unsigned_long_long",
> + sizeof (unsigned long long) },
> + { comp.unsigned_long_type, "unsigned_long", sizeof (unsigned long) },
> + { comp.unsigned_type, "unsigned", sizeof (unsigned) },
> + { comp.void_ptr_type, "void_ptr", sizeof (void*) } };
Fine for now, but I don't like to have all this information sparse.
We should take what is now cast_type (add the sign information) call it
something like comp_type and use it allover. So that when we initialize
a type all the information is in one place and is not duplicated.
If you like to pick this task would be very welcome.
[...]
> comp.cast_union_type =
> gcc_jit_context_new_union_type (comp.ctxt,
> NULL,
> "cast_union",
> - ARRAYELTS (cast_union_fields),
> - cast_union_fields);
> + NUM_CAST_TYPES+1,
Spaces around +
> + comp.cast_union_fields);
> +
> + /* Define the cast functions using a matrix. */
> + for (int i = 0; i < NUM_CAST_TYPES; ++i)
> + for (int j = 0; j < NUM_CAST_TYPES; ++j)
> + comp.cast_functions_from_to[i][j]
Okay so the three pushed.
Thanks
Andrea
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-06-01 20:28 ` Andrea Corallo
@ 2020-06-01 23:11 ` Nicolas Bértolo
2020-06-04 9:52 ` Andrea Corallo
0 siblings, 1 reply; 18+ messages in thread
From: Nicolas Bértolo @ 2020-06-01 23:11 UTC (permalink / raw)
To: Andrea Corallo; +Cc: 41615
> As I commented early I think this would be not ideal. The trick of the
> negation is done already in emit_cond_jump and regarding the cast
> operation I think is important to keep the C semantic (that is the one
> we have).
Sorry, my bad. I reread your emails and I understood them backwards. I thought
you didn't mind conversions that discarded data for integer->integer casts.
> We should take what is now cast_type (add the sign information) call it
> something like comp_type and use it allover. So that when we initialize
> a type all the information is in one place and is not duplicated.
> If you like to pick this task would be very welcome.
I'm still chasing the bug that causes Emacs to crash when accessing the
all_loaded_comp_units_h weak hash table. When I finish with it I will take a
look at this.
Thanks
Nico
^ permalink raw reply [flat|nested] 18+ messages in thread
* bug#41615: [feature/native-comp] Dump prettier C code.
2020-06-01 23:11 ` Nicolas Bértolo
@ 2020-06-04 9:52 ` Andrea Corallo
0 siblings, 0 replies; 18+ messages in thread
From: Andrea Corallo @ 2020-06-04 9:52 UTC (permalink / raw)
To: 41615-done
closing
--
akrl@sdf.org
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-06-04 9:52 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-30 15:08 bug#41615: [feature/native-comp] Dump prettier C code Nicolas Bértolo
2020-05-30 22:20 ` Nicolas Bértolo
2020-05-31 10:45 ` Andrea Corallo
2020-05-31 15:42 ` Nicolas Bértolo
2020-05-31 11:37 ` Andrea Corallo
2020-05-31 15:48 ` Andrea Corallo
2020-05-31 15:54 ` Nicolas Bértolo
2020-05-31 16:57 ` Andrea Corallo
2020-05-31 17:26 ` Nicolas Bértolo
2020-05-31 18:11 ` Andrea Corallo
2020-05-31 19:38 ` Nicolas Bértolo
2020-05-31 20:00 ` Andrea Corallo
2020-05-31 20:18 ` Andrea Corallo
2020-06-01 7:19 ` Andrea Corallo
2020-06-01 12:25 ` Nicolas Bértolo
2020-06-01 20:28 ` Andrea Corallo
2020-06-01 23:11 ` Nicolas Bértolo
2020-06-04 9:52 ` Andrea Corallo
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
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).