From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Andrea Corallo Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] comp.c: Use the newly added bitcast API for type coercion, when available. (feature/jit-improved-type-punning) Date: Wed, 28 Sep 2022 12:37:42 +0000 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="25260"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) Cc: emacs-devel@gnu.org To: Vibhav Pant Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Sep 28 14:58:23 2022 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1odWdr-0006R7-Q2 for ged-emacs-devel@m.gmane-mx.org; Wed, 28 Sep 2022 14:58:23 +0200 Original-Received: from localhost ([::1]:35930 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1odWdp-0004JF-Sk for ged-emacs-devel@m.gmane-mx.org; Wed, 28 Sep 2022 08:58:22 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53552) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1odWK2-0001UJ-Lv for emacs-devel@gnu.org; Wed, 28 Sep 2022 08:37:57 -0400 Original-Received: from mx.sdf.org ([205.166.94.24]:62827) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1odWJy-0007s2-1u for emacs-devel@gnu.org; Wed, 28 Sep 2022 08:37:53 -0400 Original-Received: from ma.sdf.org (ma.sdf.org [205.166.94.33]) by mx.sdf.org (8.15.2/8.14.5) with ESMTPS id 28SCbgT7015469 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256 bits) verified NO); Wed, 28 Sep 2022 12:37:42 GMT In-Reply-To: (Vibhav Pant's message of "Tue, 27 Sep 2022 23:45:05 +0530") Received-SPF: pass client-ip=205.166.94.24; envelope-from=akrl@sdf.org; helo=mx.sdf.org X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.io gmane.emacs.devel:296390 Archived-At: Vibhav Pant writes: > * src/comp.c: Add declarations for gcc_jit_type_is_pointer, > gcc_jit_context_new_bitcast, when provided. > > * (type_to_cast_index, define_type_punning, define_cast_from_to, > define_cast_functions): Define functions when > gcc_jit_context_new_bitcast is not available. > > * (emit_coerce): Use gcc_jit_context_new_bitcast to coerce types, when > available. > > > The code is also available on the branch feature/jit-improved-type- > punning. > > Thanks, > Vibhav Hi Vibhav, thanks for the patch, please find some comments below. > diff --git a/src/comp.c b/src/comp.c > index 4813ca04a9..ddfbe2623e 100644 > --- a/src/comp.c > +++ b/src/comp.c > @@ -68,6 +68,7 @@ > #undef gcc_jit_context_get_type > #undef gcc_jit_context_new_array_access > #undef gcc_jit_context_new_array_type > +#undef gcc_jit_context_new_bitcast > #undef gcc_jit_context_new_binary_op > #undef gcc_jit_context_new_call > #undef gcc_jit_context_new_call_through_ptr > @@ -108,6 +109,7 @@ > #undef gcc_jit_struct_set_fields > #undef gcc_jit_type_get_const > #undef gcc_jit_type_get_pointer > +#undef gcc_jit_type_is_pointer > #undef gcc_jit_version_major > #undef gcc_jit_version_minor > #undef gcc_jit_version_patchlevel > @@ -180,8 +182,13 @@ DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_call_through_ptr, > (gcc_jit_context *ctxt, gcc_jit_location *loc, > gcc_jit_rvalue *fn_ptr, int numargs, gcc_jit_rvalue **args)); > DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_cast, > + (gcc_jit_context * ctxt, gcc_jit_location *loc, > + gcc_jit_rvalue *rvalue, gcc_jit_type *type)); > +#ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > +DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_bitcast, > (gcc_jit_context *ctxt, gcc_jit_location *loc, > gcc_jit_rvalue *rvalue, gcc_jit_type *type)); > +#endif > DEF_DLL_FN (gcc_jit_rvalue *, gcc_jit_context_new_comparison, > (gcc_jit_context *ctxt, gcc_jit_location *loc, > enum gcc_jit_comparison op, gcc_jit_rvalue *a, gcc_jit_rvalue *b)); > @@ -224,6 +231,9 @@ DEF_DLL_FN (gcc_jit_type *, gcc_jit_struct_as_type, > (gcc_jit_struct *struct_type)); > DEF_DLL_FN (gcc_jit_type *, gcc_jit_type_get_const, (gcc_jit_type *type)); > DEF_DLL_FN (gcc_jit_type *, gcc_jit_type_get_pointer, (gcc_jit_type *type)); > +#ifdef LIBGCCJIT_HAVE_REFLECTION > +DEF_DLL_FN (gcc_jit_type *, gcc_jit_type_is_pointer, (gcc_jit_type *type)); > +#endif > 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)); > @@ -293,6 +303,9 @@ init_gccjit_functions (void) > LOAD_DLL_FN (library, gcc_jit_context_get_type); > LOAD_DLL_FN (library, gcc_jit_context_new_array_access); > LOAD_DLL_FN (library, gcc_jit_context_new_array_type); > +#ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > + LOAD_DLL_FN (library, gcc_jit_context_new_bitcast); > +#endif > LOAD_DLL_FN (library, gcc_jit_context_new_binary_op); > LOAD_DLL_FN (library, gcc_jit_context_new_call); > LOAD_DLL_FN (library, gcc_jit_context_new_call_through_ptr); > @@ -334,6 +347,9 @@ init_gccjit_functions (void) > LOAD_DLL_FN (library, gcc_jit_struct_set_fields); > LOAD_DLL_FN (library, gcc_jit_type_get_const); > LOAD_DLL_FN (library, gcc_jit_type_get_pointer); > +#ifdef LIBGCCJIT_HAVE_REFLECTION > + LOAD_DLL_FN (library, gcc_jit_type_is_pointer); > +#endif > LOAD_DLL_FN_OPT (library, gcc_jit_context_add_command_line_option); > LOAD_DLL_FN_OPT (library, gcc_jit_context_add_driver_option); > #if defined (LIBGCCJIT_HAVE_gcc_jit_global_set_initializer) > @@ -368,6 +384,9 @@ #define gcc_jit_context_get_int_type fn_gcc_jit_context_get_int_type > #define gcc_jit_context_get_type fn_gcc_jit_context_get_type > #define gcc_jit_context_new_array_access fn_gcc_jit_context_new_array_access > #define gcc_jit_context_new_array_type fn_gcc_jit_context_new_array_type > +#ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > + #define gcc_jit_context_new_bitcast fn_gcc_jit_context_new_bitcast > +#endif > #define gcc_jit_context_new_binary_op fn_gcc_jit_context_new_binary_op > #define gcc_jit_context_new_call fn_gcc_jit_context_new_call > #define gcc_jit_context_new_call_through_ptr fn_gcc_jit_context_new_call_through_ptr > @@ -410,6 +429,9 @@ #define gcc_jit_rvalue_dereference_field fn_gcc_jit_rvalue_dereference_field > #define gcc_jit_rvalue_get_type fn_gcc_jit_rvalue_get_type > #define gcc_jit_struct_as_type fn_gcc_jit_struct_as_type > #define gcc_jit_struct_set_fields fn_gcc_jit_struct_set_fields > +#ifdef LIBGCCJIT_HAVE_REFLECTION > +#define gcc_jit_type_is_pointer fn_gcc_jit_type_is_pointer > +#endif > #define gcc_jit_type_get_const fn_gcc_jit_type_get_const > #define gcc_jit_type_get_pointer fn_gcc_jit_type_get_pointer > #if defined (LIBGCCJIT_HAVE_gcc_jit_version) > @@ -518,7 +540,9 @@ #define F_RELOC_MAX_SIZE 1500 > > static f_reloc_t freloc; > > +#ifndef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > #define NUM_CAST_TYPES 15 > +#endif > > typedef struct { > EMACS_INT len; > @@ -593,13 +617,15 @@ #define NUM_CAST_TYPES 15 > gcc_jit_rvalue *current_thread_ref; > /* Other globals. */ > gcc_jit_rvalue *pure_ptr; > - /* libgccjit has really limited support for casting therefore this union will > - be used for the scope. */ > +#ifndef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > + /* This version of libgccjit has really limited support for casting > + therefore this union will be used for the scope. */ > gcc_jit_type *cast_union_type; > gcc_jit_function *cast_functions_from_to[NUM_CAST_TYPES][NUM_CAST_TYPES]; > gcc_jit_function *cast_ptr_to_int; > gcc_jit_function *cast_int_to_ptr; > gcc_jit_type *cast_types[NUM_CAST_TYPES]; > +#endif > gcc_jit_function *func; /* Current function being compiled. */ > bool func_has_non_local; /* From comp-func has-non-local slot. */ > EMACS_INT func_speed; /* From comp-func speed slot. */ > @@ -1100,6 +1126,7 @@ emit_cond_jump (gcc_jit_rvalue *test, > > } > > +#ifndef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > static int > type_to_cast_index (gcc_jit_type * type) > { > @@ -1109,6 +1136,7 @@ type_to_cast_index (gcc_jit_type * type) > > xsignal1 (Qnative_ice, build_string ("unsupported cast")); > } > +#endif > > static gcc_jit_rvalue * > emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj) > @@ -1145,14 +1173,41 @@ emit_coerce (gcc_jit_type *new_type, gcc_jit_rvalue *obj) > } > #endif > > +#ifdef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > + bool old_is_ptr = gcc_jit_type_is_pointer (old_type) != NULL; > + bool new_is_ptr = gcc_jit_type_is_pointer (new_type) != NULL; > + > + gcc_jit_rvalue *tmp = obj; > + > + if (old_is_ptr != new_is_ptr) > + { > + if (old_is_ptr) > + { > + tmp = gcc_jit_context_new_cast (comp.ctxt, NULL, tmp, > + comp.void_ptr_type); > + tmp = gcc_jit_context_new_bitcast (comp.ctxt, NULL, tmp, > + comp.uintptr_type); > + } > + else > + { > + tmp = gcc_jit_context_new_cast (comp.ctxt, NULL, tmp, > + comp.uintptr_type); > + tmp = gcc_jit_context_new_bitcast (comp.ctxt, NULL, tmp, > + comp.void_ptr_type); Could you clarify why we need this double cast in both cases here? > + } > + } > + return gcc_jit_context_new_cast (comp.ctxt, NULL, tmp, new_type); > +#else /* !defined(LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast) */ Nit: I'd prefer a new line before and after this #else > int old_index = type_to_cast_index (old_type); > int new_index = type_to_cast_index (new_type); > > /* 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); > + NULL, > + comp.cast_functions_from_to > + [old_index][new_index], > + 1, &obj); > +#endif > } > > static gcc_jit_rvalue * > @@ -3318,6 +3373,7 @@ define_thread_state_struct (void) > gcc_jit_type_get_pointer (gcc_jit_struct_as_type (comp.thread_state_s)); > } > > +#ifndef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > static gcc_jit_function * > define_type_punning (const char *name, > gcc_jit_type *from, gcc_jit_field *from_field, > @@ -3336,6 +3392,7 @@ define_type_punning (const char *name, > > DECL_BLOCK (entry_block, result); > > + Are this and the following new line added voluntarily? > gcc_jit_lvalue *tmp_union > = gcc_jit_function_new_local (result, > NULL, > @@ -3422,6 +3479,7 @@ define_cast_functions (void) > { comp.unsigned_long_type, "unsigned_long", false }, > { comp.unsigned_type, "unsigned", false }, > { comp.void_ptr_type, "void_ptr", true } }; > + > gcc_jit_field *cast_union_fields[2]; > > /* Define the union used for type punning. */ > @@ -3451,6 +3509,7 @@ define_cast_functions (void) > comp.void_ptr_type, > cast_union_fields[0]); > > + > for (int i = 0; i < NUM_CAST_TYPES; ++i) > comp.cast_types[i] = cast_types[i].type; > > @@ -3460,6 +3519,7 @@ define_cast_functions (void) > comp.cast_functions_from_to[i][j] = > define_cast_from_to (cast_types[i], cast_types[j]); > } > +#endif /* !defined(LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast) */ > > static void > define_CHECK_TYPE (void) > @@ -4660,7 +4720,9 @@ DEFUN ("comp--init-ctxt", Fcomp__init_ctxt, Scomp__init_ctxt, > define_jmp_buf (); > define_handler_struct (); > define_thread_state_struct (); > +#ifndef LIBGCCJIT_HAVE_gcc_jit_context_new_bitcast > define_cast_functions (); > +#endif > > return Qt; > } Which kind of tests did this patch went through? I assumed you tried a bootstrap could you please confirm? Also have comp tests been tried? Thanks Andrea