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)?
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
