Thanks for your answer. See my answers below:
Le jeudi 13 mai 2021 à 18:13 -0400, David Malcolm a écrit : > On Sat, 2021-02-20 at 17:17 -0500, Antoni Boucher via Gcc-patches > wrote: > > Hi. > > Thanks for your feedback! > > > > Sorry about the delay in responding. > > In the past I was hesitant about adding more cast support to libgccjit > since I felt that the user could always just create a union to do the > cast. Then I tried actually using the libgccjit API to do this, and > realized how much work it adds, so I now think we do want to support > casting more types. > > > > 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) > > Do we even get C_MAYBE_CONST_EXPR in libgccjit? That tree code is > defined in c-family/c-common.def; how can nodes of that kind be created > outside of the c-family? I am not sure, but that seems like it's only created in c-family indeed. However, we do use it in libgccjit here: https://github.com/gcc-mirror/gcc/blob/master/gcc/jit/jit-playback.c#L1180 > > > > 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. > > FWIW I don't think we'll want to support truncating or extending > pointers. Ok, but do you think we'll want to support casts between integers and pointers? I opened an issue about this (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95438) and would be willing to do a patch for it eventually. > > > > > 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! > > > >