bug#41615: [feature/native-comp] Dump prettier C code.

[Andrea Corallo <akrl@sdf.org>]

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