Hi!
compare_operand uses operand_equal_p under the hood, which e.g. for
INTEGER_CSTs will just match the values rather regardless of their types.
Now, in many comparing the type is redundant, if we have
x_2 = y_3 + 1;
we've already compared the type for the lhs and also for rhs1, there won't
be any surprises on rhs2.
As noted in the PR, there are cases where the type of the operand is the
sole place of information and we don't want to ICF merge functions if the
types differ.
One case is stdarg functions, arguments passed to ..., it is different
if we pass 1, 1L, 1LL.
Another case are the K&R unprototyped functions (sure, gone in C23).
And yet another case are inline asm operands, "r" (1) is different from "r"
(1L) from "r" (1LL).
So, the following patch determines based on lack of fntype (e.g. for
internal functions), or on !prototype_p, or on stdarg_p (in that case
using number of named arguments) which arguments need to have type checked
and does that, plus compares types on inline asm operands (maybe it would be
enough to do that just for input operands but we have just a routine to
handle both and I didn't feel we need to differentiate).
Fuirthermore, I've noticed fntype{1,2} isn't actually compared if it is a
direct call (gimple_call_fndecl is non-NULL). That is wrong too, we could
have
void (*fn) (int, long long) = (void (*) (int, long long)) foo;
fn (1, 1LL);
in one case and
void (*fn) (long long, int) = (void (*) (long long, int)) foo;
fn (1LL, 1);
in another, both folded into a direct call of foo with different
gimple_call_fntype. Sure, one of them would be UB at runtime (or both), but
what if we ICF merge it into something that into the one UB at runtime
and the program actually calls the correct one only?
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2025-01-31 Jakub Jelinek <ja...@redhat.com>
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): Also check fntype1 vs. fntype2
compatibility if at least one of the calls has different
gimple_call_fntype from the FUNCTION_TYPE of the called decl. 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-02 11:23:16.334519404 +0100
+++ gcc/ipa-icf-gimple.cc 2025-01-30 16:21:05.782127011 +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);
@@ -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)
{
if ((fntype1 && !fntype2)
|| (!fntype1 && fntype2)
@@ -738,6 +743,24 @@ func_checker::compare_gimple_call (gcall
get_operand_access_type (&map, chain1)))
return return_false_with_msg ("static call chains are different");
+ unsigned check_arg_types_from = gimple_call_num_args (s1);
+ if (!fntype1
+ || !fntype2
+ || !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));
+
/* Checking of argument. */
for (i = 0; i < gimple_call_num_args (s1); ++i)
{
@@ -746,6 +769,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-30
17:34:51.745191701 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr117432.c 2025-01-30
16:48:45.050300148 +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-30 17:34:32.427457117
+0100
+++ gcc/testsuite/gcc.target/i386/pr117432.c 2025-01-30 17:33:38.937192052
+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
