@David: PING
On 2/20/21 11:17 PM, Antoni Boucher via Gcc-patches wrote:
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!
