On Thu, 2023-12-21 at 08:36 -0500, Antoni Boucher wrote: > Hi. > Here's the updated patch. > Thanks.
Sorry for the delay in responding. The updated patch is good for trunk - thanks! Dave > > On Thu, 2023-12-07 at 20:15 -0500, David Malcolm wrote: > > On Thu, 2023-12-07 at 17:34 -0500, Antoni Boucher wrote: > > > Hi. > > > This patch adds checks gcc_jit_block_add_assignment_op to make > > > sure > > > it > > > is only ever called on numeric types. > > > > > > With the previous patch, this might require a change to also > > > allow > > > vector types here. > > > > > > Thanks for the review. > > > > Thanks for the patch. > > > > [...snip...] > > > > > @@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op > > > (gcc_jit_block *block, > > > lvalue->get_type ()->get_debug_string (), > > > rvalue->get_debug_string (), > > > rvalue->get_type ()->get_debug_string ()); > > > + // TODO: check if it is a numeric vector? > > > + RETURN_IF_FAIL_PRINTF3 ( > > > + lvalue->get_type ()->is_numeric () && rvalue->get_type ()- > > > > is_numeric (), ctxt, loc, > > > + "gcc_jit_block_add_assignment_op %s has non-numeric lvalue > > > %s > > > (type: %s)", > > > + gcc::jit::binary_op_reproducer_strings[op], > > > + lvalue->get_debug_string (), lvalue->get_type ()- > > > > get_debug_string ()); > > > > The condition being tested here should probably just be: > > > > lvalue->get_type ()->is_numeric () > > > > since otherwise if the lvalue's type is numeric and the rvalue's > > type > > fails to be, then the user would incorrectly get a message about > > the > > lvalue. > > > > > + RETURN_IF_FAIL_PRINTF3 ( > > > + rvalue->get_type ()->is_numeric () && rvalue->get_type ()- > > > > is_numeric (), ctxt, loc, > > > + "gcc_jit_block_add_assignment_op %s has non-numeric rvalue > > > %s > > > (type: %s)", > > > + gcc::jit::binary_op_reproducer_strings[op], > > > + rvalue->get_debug_string (), rvalue->get_type ()- > > > > get_debug_string ()); > > > > The condition being tested here seems to have a redundant repeated: > > && rvalue->get_type ()->is_numeric () > > > > Am I missing something, or is that a typo? > > > > [...snip...] > > > > The patch is OK otherwise. > > > > Thanks > > Dave > > > > > > >
