Re: [PATCH] libgccjit: Handle truncation and extension for casts [PR 95498]

[Antoni Boucher <bouanto@zoho.com>]

Hi.
Thanks for your feedback!

See answers below:

On Sat, Feb 20, 2021 at 11:20:35AM -0700, Tom Tromey wrote:
>>>>>> "Antoni" == Antoni Boucher via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>
>Antoni> gcc/jit/
>Antoni> 	PR target/95498
>Antoni> 	* jit-playback.c: Add support to handle truncation and extension
>Antoni> 	in the convert function.
>
>Antoni> +  switch (dst_code)
>Antoni> +    {
>Antoni> +    case INTEGER_TYPE:
>Antoni> +    case ENUMERAL_TYPE:
>Antoni> +      t_ret = convert_to_integer (dst_type, expr);
>Antoni> +      goto maybe_fold;
>Antoni> +
>Antoni> +    default:
>Antoni> +      gcc_assert (gcc::jit::active_playback_ctxt);
>Antoni> +      gcc::jit::active_playback_ctxt->add_error (NULL, "unhandled conversion");
>Antoni> +      fprintf (stderr, "input expression:\n");
>Antoni> +      debug_tree (expr);
>Antoni> +      fprintf (stderr, "requested type:\n");
>Antoni> +      debug_tree (dst_type);
>Antoni> +      return error_mark_node;
>Antoni> +
>Antoni> +    maybe_fold:
>Antoni> +      if (TREE_CODE (t_ret) != C_MAYBE_CONST_EXPR)
>Antoni> +	t_ret = fold (t_ret);
>Antoni> +      return t_ret;
>
>It seems weird to have a single 'goto' to maybe_fold, especially inside
>a switch like this.
>
>If you think the maybe_fold code won't be reused, then it should just be
>hoisted up and the 'goto' removed.

This actually depends on how the support for cast between integers and 
pointers will be implemented (see below).
If we will support truncating pointers (does that even make sense? and I 
guess we cannot extend a pointer unless we add the support for 
uint128_t), that label will be reused for that case.
Otherwise, it might not be reused.

So, please tell me which option to choose and I'll update my patch.

>On the other hand, if the maybe_fold code might be reused for some other
>case, then I suppose I would have the case end with 'break' and then
>have this code outside the switch.
>
>
>In another message, you wrote:
>
>Antoni> For your question, the current code already works with boolean and
>Antoni> reals and casts between integers and pointers is currently not
>Antoni> supported.
>
>I am curious why this wasn't supported.  It seems like something that
>one might want to do.

I have no idea as this is my first contribution to gcc.
But this would be indeed very useful and I opened an issue about this: 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438

>thanks,
>Tom

Thanks!