On Fri, Jan 31, 2025 at 02:19:28PM +0100, Richard Biener wrote:
> > For internal calls gimple_call_fndecl (s1) will be NULL, so
> > !gimple_call_fndecl (s1) will be true and so the new checks aren't done.
>
> Yes, but also fntype1/2 will be NULL then.
>
> > > if (gimple_call_internal_p (s1) (with gimple_call_internal_fn compare
> > > in a conditiona if) would be a lot clearer?
> >
> > What the patch does is just trying to avoid the comparison in the common
> > case (direct calls from the beginning and there what the comment says
> > applies, if there would be a mismatch, we'd already knew that).
> >
> > If you want to compare unconditionally, it would be about just removing the
> > if (!gimple_call_fndecl (s1))
> > {
> > and
> > }
> > and reindenting + rewriting the comment above it. Shall I do that?
>
> That's what I suggested, or rather
>
> if (gimple_call_internal_p (s1))
> {
> if (gimple_call_internal_fn (s1) != gimple_call_internal_fn (s2))
> return false;
> }
> else
> {
> tree fntype1 = gimple_call_fntype (s1);
> tree fntype2 = gimple_call_fntype (s2);
>
> if ((fntype1 && !fntype2)
> || (!fntype1 && fntype2)
> || (fntype1 && !types_compatible_p (fntype1, fntype2)))
> return return_false_with_msg ("call function types are not
> compatible");
> }
>
> I think in the else { fntype1 and fntype2 should never be NULL and thus
> this should simplify even more.
This isn't possible because fntype{1,2} are used later on in the function;
sure, that
if (fntype1 && fntype2 && comp_type_attributes (fntype1, fntype2) != 1)
return return_false_with_msg ("different fntype attributes");
can be moved into the else, but the new checks to determine which args to
check still use that.
Jakub
