[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] comp.c: Use the newly added bitcast API for type coercion, w
From: |
Andrea Corallo |
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 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) |
Vibhav Pant <vibhavp@gmail.com> 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