On Fri, 31 Jan 2025, Jakub Jelinek wrote:
> On Fri, Jan 31, 2025 at 02:29:57PM +0100, Jakub Jelinek wrote:
> > > }
> > > 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.
>
> So like this then (if it passes bootstrap/regtest)?
LGTM.
Richard.
> 2025-01-31 Jakub Jelinek <[email protected]>
>
> PR ipa/117432
> * ipa-icf-gimple.cc (func_checker::compare_asm_inputs_outputs):
> Also return_false if operands have incompatible types.
> (func_checker::compare_gimple_call): Check fntype1 vs. fntype2
> compatibility for all non-internal calls and assume fntype1 and
> fntype2 are non-NULL for those. For calls to non-prototyped
> calls or for stdarg_p functions after the last named argument (if any)
> check type compatibility of call arguments.
>
> * gcc.c-torture/execute/pr117432.c: New test.
> * gcc.target/i386/pr117432.c: New test.
>
> --- gcc/ipa-icf-gimple.cc.jj 2025-01-30 18:29:22.237190471 +0100
> +++ gcc/ipa-icf-gimple.cc 2025-01-31 15:25:57.168535197 +0100
> @@ -459,7 +459,9 @@ func_checker::compare_asm_inputs_outputs
> return false;
>
> if (!compare_operand (TREE_VALUE (t1), TREE_VALUE (t2),
> - get_operand_access_type (map, t1)))
> + get_operand_access_type (map, t1))
> + || !types_compatible_p (TREE_TYPE (TREE_VALUE (t1)),
> + TREE_TYPE (TREE_VALUE (t2))))
> return return_false ();
>
> tree p1 = TREE_PURPOSE (t1);
> @@ -709,26 +711,37 @@ func_checker::compare_gimple_call (gcall
> || gimple_call_alloca_for_var_p (s1) != gimple_call_alloca_for_var_p
> (s2))
> return false;
>
> - if (gimple_call_internal_p (s1)
> - && gimple_call_internal_fn (s1) != gimple_call_internal_fn (s2))
> - return false;
> -
> - tree fntype1 = gimple_call_fntype (s1);
> - tree fntype2 = gimple_call_fntype (s2);
> -
> - /* 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))
> + unsigned check_arg_types_from = 0;
> + if (gimple_call_internal_p (s1))
> {
> - if ((fntype1 && !fntype2)
> - || (!fntype1 && fntype2)
> - || (fntype1 && !types_compatible_p (fntype1, fntype2)))
> - return return_false_with_msg ("call function types are not compatible");
> + 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 (!types_compatible_p (fntype1, fntype2))
> + return return_false_with_msg ("call function types are not compatible");
> +
> + if (comp_type_attributes (fntype1, fntype2) != 1)
> + return return_false_with_msg ("different fntype attributes");
>
> - if (fntype1 && fntype2 && comp_type_attributes (fntype1, fntype2) != 1)
> - return return_false_with_msg ("different fntype attributes");
> + check_arg_types_from = gimple_call_num_args (s1);
> + if (!prototype_p (fntype1) || !prototype_p (fntype2))
> + check_arg_types_from = 0;
> + else if (stdarg_p (fntype1))
> + {
> + check_arg_types_from = list_length (TYPE_ARG_TYPES (fntype1));
> + if (stdarg_p (fntype2))
> + {
> + unsigned n = list_length (TYPE_ARG_TYPES (fntype2));
> + check_arg_types_from = MIN (check_arg_types_from, n);
> + }
> + }
> + else if (stdarg_p (fntype2))
> + check_arg_types_from = list_length (TYPE_ARG_TYPES (fntype2));
> + }
>
> tree chain1 = gimple_call_chain (s1);
> tree chain2 = gimple_call_chain (s2);
> @@ -746,6 +759,10 @@ func_checker::compare_gimple_call (gcall
>
> if (!compare_operand (t1, t2, get_operand_access_type (&map, t1)))
> return return_false_with_msg ("GIMPLE call operands are different");
> + if (i >= check_arg_types_from
> + && !types_compatible_p (TREE_TYPE (t1), TREE_TYPE (t2)))
> + return return_false_with_msg ("GIMPLE call operand types are "
> + "different");
> }
>
> /* Return value checking. */
> --- gcc/testsuite/gcc.c-torture/execute/pr117432.c.jj 2025-01-31
> 15:14:54.358852495 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr117432.c 2025-01-31
> 15:14:54.358852495 +0100
> @@ -0,0 +1,71 @@
> +/* PR ipa/117432 */
> +
> +#include <stdarg.h>
> +
> +long long r;
> +
> +__attribute__((noipa)) void
> +baz (int tag, ...)
> +{
> + va_list ap;
> + va_start (ap, tag);
> + if (!r)
> + r = va_arg (ap, long long);
> + else
> + r = va_arg (ap, int);
> + va_end (ap);
> +}
> +
> +void
> +foo (void)
> +{
> + baz (1, -1, 0);
> +}
> +
> +void
> +bar (void)
> +{
> + baz (1, -1LL, 0);
> +}
> +
> +__attribute__((noipa)) void
> +qux (...)
> +{
> + va_list ap;
> + va_start (ap);
> + if (!r)
> + r = va_arg (ap, long long);
> + else
> + r = va_arg (ap, int);
> + va_end (ap);
> +}
> +
> +void
> +corge (void)
> +{
> + qux (-2, 0);
> +}
> +
> +void
> +fred (void)
> +{
> + qux (-2LL, 0);
> +}
> +
> +int
> +main ()
> +{
> + bar ();
> + if (r != -1LL)
> + __builtin_abort ();
> + foo ();
> + if (r != -1)
> + __builtin_abort ();
> + r = 0;
> + fred ();
> + if (r != -2LL)
> + __builtin_abort ();
> + corge ();
> + if (r != -2)
> + __builtin_abort ();
> +}
> --- gcc/testsuite/gcc.target/i386/pr117432.c.jj 2025-01-31
> 15:14:54.358852495 +0100
> +++ gcc/testsuite/gcc.target/i386/pr117432.c 2025-01-31 15:14:54.358852495
> +0100
> @@ -0,0 +1,17 @@
> +/* PR ipa/117432 */
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler "# myinsn %rax" } } */
> +/* { dg-final { scan-assembler "# myinsn %eax" } } */
> +
> +void
> +foo (void)
> +{
> + asm volatile ("# myinsn %0" : : "r" (-42L));
> +}
> +
> +void
> +bar (void)
> +{
> + asm volatile ("# myinsn %0" : : "r" (-42));
> +}
>
>
> 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)
