[Bug ipa/99517] __builtin_convertvector with casts

["rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99517

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #4)
> --- gcc/ipa-icf-gimple.c.jj	2021-01-04 10:25:38.752234741 +0100
> +++ gcc/ipa-icf-gimple.c	2021-03-10 15:02:06.287502784 +0100
> @@ -667,7 +667,7 @@ func_checker::compare_gimple_call (gcall
>    tree fntype1 = gimple_call_fntype (s1);
>    tree fntype2 = gimple_call_fntype (s2);
>  
> -  /* For direct calls we verify that types are comopatible so if we matced
> +  /* 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))
> @@ -703,6 +703,14 @@ func_checker::compare_gimple_call (gcall
>    t1 = gimple_get_lhs (s1);
>    t2 = gimple_get_lhs (s2);
>  
> +  /* For internal calls, lhs types need to be verified, as neither fntype
> nor
> +     callee comparisions can catch that.  */
> +  if (gimple_call_internal_p (s1)
> +      && t1
> +      && t2
> +      && !compatible_types_p (TREE_TYPE (t1), TREE_TYPE (t2)))
> +    return return_false_with_msg ("GIMPLE internal call LHS type mismatch");
> +
>    return compare_operand (t1, t2, get_operand_access_type (&map, t1));
>  }
>  
> 
> fixes this for me, working on the testcase for testsuite now.
> 
> Anyway, I'm a little bit worried that gimple_call_fntype isn't compared for
> direct calls, gimple_call_fntype doesn't have to match the function type of
> gimple_call_fndecl.

The comment says that we match that the fndecls definitions are equal,
one could argue that it's undefined behavior if the same definition is
invoked via two different gimple_call_fntype signatures and thus it's
OK to just go ahead (I think actual arguments are also compared so it
would collide there, too).

> And also surprised that compare_ssa_name e.g. doesn't compare
> SSA_NAME_OCCURS_IN_ABNORMAL_PHI or SSA_NAME_PTR_INFO or SSA_NAME_RANGE_INFO.
> Maybe for the latter two we are fine if we only optimize away
> __builtin_unreachable calls after IPA, but if we do it before, e.g. some
> function could have if (cond) __builtin_unreachable (); assertions that EVRP
> would stick into SSA_NAME_RANGE_INFO and another function be the same except
> without those assertions.  If ICF merges the latter into the former and
> calls the latter with arguments violating the assertions of the former, it
> might be miscompiled.

Hmm, yeah.  I guess one could try to carefully craft such a testcase.
For example non-null ranges can also be created from dead loads like

foo (int *p)
{
  *p;
  if (p != 0)
...

when you make sure to not run DCE before EVRP.  Not sure if that's enough
of a building-block to create a testcase.