On Fri, 31 Jan 2025, Jakub Jelinek wrote:
> On Fri, Jan 31, 2025 at 01:38:36PM +0100, Richard Biener wrote:
> > > @@ -718,8 +720,11 @@ func_checker::compare_gimple_call (gcall
> > >
> > > /* For direct calls we verify that types are compatible so if we
> > > matched
> > > callees, callers must match, too. For indirect calls however verify
> > > - function type. */
> > > - if (!gimple_call_fndecl (s1))
> > > + function type. And also verify it for direct calls with some
> > > different
> > > + fntype. */
> > > + if (!gimple_call_fndecl (s1)
> > > + || TREE_TYPE (TREE_TYPE (t1)) != fntype1
> > > + || TREE_TYPE (TREE_TYPE (t2)) != fntype2)
> >
> > I think we want to always compare the ABI relevant fntypes. It seems
> > we can arrive here with internal function calls where t1/t2 are
> > "somthing" (NULL?). I guess doing this as else {} of the
>
> 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.
Richard.
> Jakub
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
