Package: emacs;
Reported by: Nicolas Bértolo <nicolasbertolo <at> gmail.com>
Date: Sat, 30 May 2020 15:09:01 UTC
Severity: normal
Done: Andrea Corallo <akrl <at> sdf.org>
Bug is archived. No further changes may be made.
Message #50 received at 41615 <at> debbugs.gnu.org (full text, mbox):
From: Andrea Corallo <akrl <at> sdf.org> To: Nicolas Bértolo <nicolasbertolo <at> gmail.com> Cc: 41615 <at> debbugs.gnu.org Subject: Re: bug#41615: [feature/native-comp] Dump prettier C code. Date: Mon, 01 Jun 2020 20:28:59 +0000
Nicolas Bértolo <nicolasbertolo <at> 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 <at> 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 <at> 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 <at> 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 <at> 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 <at> sdf.org
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.