https://gcc.gnu.org/g:916daed4ecb6ed8d329b66e97b9468c8f6e549bd
commit r14-11293-g916daed4ecb6ed8d329b66e97b9468c8f6e549bd
Author: Jakub Jelinek <ja...@redhat.com>
Date: Sat Feb 1 00:50:24 2025 +0100
icf: Compare call argument types in certain cases and asm operands
[PR117432]
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).
Furthermore, 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?
2025-02-01 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): 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.
(cherry picked from commit ebd111a2896816e4f5ddf5108f361b3d9d287fa0)
Diff:
---
gcc/ipa-icf-gimple.cc | 53 ++++++++++++-------
gcc/testsuite/gcc.c-torture/execute/pr117432.c | 72 ++++++++++++++++++++++++++
gcc/testsuite/gcc.target/i386/pr117432.c | 17 ++++++
3 files changed, 124 insertions(+), 18 deletions(-)
diff --git a/gcc/ipa-icf-gimple.cc b/gcc/ipa-icf-gimple.cc
index 4c3174b68b67..5b31f56a913d 100644
--- a/gcc/ipa-icf-gimple.cc
+++ b/gcc/ipa-icf-gimple.cc
@@ -459,7 +459,9 @@ func_checker::compare_asm_inputs_outputs (tree t1, tree t2,
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 *s1, gcall *s2)
|| 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 *s1, gcall *s2)
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. */
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr117432.c
b/gcc/testsuite/gcc.c-torture/execute/pr117432.c
new file mode 100644
index 000000000000..23049fa1142e
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr117432.c
@@ -0,0 +1,72 @@
+/* PR ipa/117432 */
+/* { dg-additional-options "-std=gnu2x" } */
+
+#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 ();
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr117432.c
b/gcc/testsuite/gcc.target/i386/pr117432.c
new file mode 100644
index 000000000000..adc54b883444
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr117432.c
@@ -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));
+}
