On Thu, Jul 15, 2021 at 3:23 PM Richard Biener <richard.guent...@gmail.com> wrote: > > On Thu, Jul 15, 2021 at 3:21 PM Richard Biener > <richard.guent...@gmail.com> wrote: > > > > On Thu, Jul 15, 2021 at 3:16 PM Aldy Hernandez <al...@redhat.com> wrote: > > > > > > > > > > > > On 7/15/21 3:06 PM, Richard Biener wrote: > > > > On Thu, Jul 15, 2021 at 1:06 PM Aldy Hernandez <al...@redhat.com> wrote: > > > >> > > > >> Well, if we don't adjust gimple_call_return_type() to handle built-ins > > > >> with no LHS, then we must adjust the callers. > > > >> > > > >> The attached patch fixes gimple_expr_type() per it's documentation: > > > >> > > > >> /* Return the type of the main expression computed by STMT. Return > > > >> void_type_node if the statement computes nothing. */ > > > >> > > > >> Currently gimple_expr_type is ICEing because it calls > > > >> gimple_call_return_type. > > > >> > > > >> I still think gimple_call_return_type should return void_type_node > > > >> instead of ICEing, but this will also fix my problem. > > > >> > > > >> Anyone have a problem with this? > > > > > > > > It's still somewhat inconsistent, no? Because for a call without a LHS > > > > it's now either void_type_node or the type of the return value. > > > > > > > > It's probably known I dislike gimple_expr_type itself (it was introduced > > > > to make the transition to tuples easier). I wonder why you can't simply > > > > fix range_of_call to do > > > > > > > > tree lhs = gimple_call_lhs (call); > > > > if (lhs) > > > > type = TREE_TYPE (lhs); > > > > > > That would still leave gimple_expr_type() broken. It's comment clearly > > > says it should return void_type_node. > > > > Does it? What does it say for > > > > int foo (); > > > > and the stmt > > > > 'foo ();' > > > > ? How's this different from > > > > 'bar ();' > > > > when bar is an internal function? Note how the comment > > speaks about 'type of the main EXPRESSION' and > > 'if the STATEMEMT computes nothing' (emphasis mine). > > I don't think it's all that clear. A gimple_cond stmt > > doesn't compute anything, does it? Does the 'foo ()' > > statement compute anything? The current implementation > > (and your patched one) says so. But why does > > > > .ADD_OVERFLOW (_1, _2); > > > > not (according to your patched implementation)? It computes > > something and that something has a type that depends on > > the types of _1 and _2 and on the actual internal function. > > But we don't have it readily available. If you need it then > > you are on your own - but returning void_type_node is wrong. > > That said, in 99% of all cases people should have used > TREE_TYPE (gimple_get_lhs (stmt)) insead of > gimple_expr_type since that makes clear that we're > talking of a result that materializes somewhere. It also > makes the required guard obvious - gimple_get_lhs (stmt) != NULL. > > Then there are the legacy callers that call it on a GIMPLE_COND > and the (IMHO broken) ones that expect it to do magic for > masked loads and stores.
Btw, void_type_node is also wrong for a GIMPLE_ASM with outputs. I think if you really want to fix the ICEing then return NULL for "we don't know" and adjust the current default as well. Richard. > Richard. > > > Richard. > > > > > I still think we should just fix gimple_call_return_type to return > > > void_type_node instead of ICEing. > > >
