On 11/11/2014 12:11 AM, Jakub Jelinek wrote:
On Mon, Nov 10, 2014 at 10:08:54PM +0100, Richard Biener wrote:
@@ -662,9 +662,49 @@ func_checker::compare_gimple_call (gimpl
t1 = gimple_call_fndecl (s1);
t2 = gimple_call_fndecl (s2);
Just drop these and compare gimple_call_fn only.
+ tree chain1 = gimple_call_chain (s1);
+ tree chain2 = gimple_call_chain (s2);
+
+ if ((chain1 && !chain2) || (!chain1 && chain2))
+ return return_false_with_msg ("Tree call chains are different");
I miss a compare_operands for the call chain.
Otherwise OK.
Here is what I've committed after another bootstrap/regtest.
Note, I've tried:
__attribute__ ((noinline, noclone))
int f1 (int x)
{
int y = 3, z = 4;
__attribute__ ((noinline, noclone)) int
f2 (int a) { return a + x + y + z; }
return f2 (5);
}
__attribute__ ((noinline, noclone))
int f3 (int x)
{
int y = 3, z = 4;
__attribute__ ((noinline, noclone)) int
f4 (int a) { return a + x + y + z; }
return f4 (5);
}
int
main ()
{
if (f1 (9) != 21 || f3 (9) != 21)
__builtin_abort ();
return 0;
}
but ICF doesn't optimize this with or without the patch,
as the structs aren't the same type (supposedly different alias set?),
even when they have the same members laid out the same.
Hello Jakub.
You are right, more precisely types_compatible_p return false for these
two structures. I'll write this situation to my TODO list.
Thank you for sending the patch.
Martin
2014-11-11 Jakub Jelinek <ja...@redhat.com>
Martin Liska <mli...@suse.cz>
* ipa-icf-gimple.c (func_checker::compare_bb): Fix comment typo.
(func_checker::compare_gimple_call): Compare gimple_call_fn,
gimple_call_chain, gimple_call_fntype and call flags.
testsuite/
* gcc.dg/ubsan/ipa-icf-1.c: New test.
* gcc.dg/ipa/ipa-icf-31.c: New test.
--- gcc/ipa-icf-gimple.c.jj 2014-10-30 14:42:20.000000000 +0100
+++ gcc/ipa-icf-gimple.c 2014-11-10 19:08:38.339986360 +0100
@@ -554,7 +554,7 @@ func_checker::parse_labels (sem_bb *bb)
In general, a collection of equivalence dictionaries is built for types
like SSA names, declarations (VAR_DECL, PARM_DECL, ..). This infrastructure
- is utilized by every statement-by-stament comparison function. */
+ is utilized by every statement-by-statement comparison function. */
bool
func_checker::compare_bb (sem_bb *bb1, sem_bb *bb2)
@@ -659,12 +659,39 @@ func_checker::compare_gimple_call (gimpl
if (gimple_call_num_args (s1) != gimple_call_num_args (s2))
return false;
- t1 = gimple_call_fndecl (s1);
- t2 = gimple_call_fndecl (s2);
-
- /* Function pointer variables are not supported yet. */
+ t1 = gimple_call_fn (s1);
+ t2 = gimple_call_fn (s2);
if (!compare_operand (t1, t2))
- return return_false();
+ return return_false ();
+
+ /* Compare flags. */
+ if (gimple_call_internal_p (s1) != gimple_call_internal_p (s2)
+ || gimple_call_ctrl_altering_p (s1) != gimple_call_ctrl_altering_p (s2)
+ || gimple_call_tail_p (s1) != gimple_call_tail_p (s2)
+ || gimple_call_return_slot_opt_p (s1) != gimple_call_return_slot_opt_p
(s2)
+ || gimple_call_from_thunk_p (s1) != gimple_call_from_thunk_p (s2)
+ || gimple_call_va_arg_pack_p (s1) != gimple_call_va_arg_pack_p (s2)
+ || gimple_call_alloca_for_var_p (s1) != gimple_call_alloca_for_var_p (s2)
+ || gimple_call_with_bounds_p (s1) != gimple_call_with_bounds_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);
+ if ((fntype1 && !fntype2)
+ || (!fntype1 && fntype2)
+ || (fntype1 && !types_compatible_p (fntype1, fntype2)))
+ return return_false_with_msg ("call function types are not compatible");
+
+ tree chain1 = gimple_call_chain (s1);
+ tree chain2 = gimple_call_chain (s2);
+ if ((chain1 && !chain2)
+ || (!chain1 && chain2)
+ || !compare_operand (chain1, chain2))
+ return return_false_with_msg ("static call chains are different");
/* Checking of argument. */
for (i = 0; i < gimple_call_num_args (s1); ++i)
--- gcc/testsuite/gcc.dg/ubsan/ipa-icf-1.c.jj 2014-11-10 19:00:53.509525071
+0100
+++ gcc/testsuite/gcc.dg/ubsan/ipa-icf-1.c 2014-11-10 19:02:21.836925806
+0100
@@ -0,0 +1,23 @@
+/* { dg-do run } */
+/* { dg-skip-if "" { *-*-* } { "*" } { "-O2" } } */
+/* { dg-options "-fsanitize=undefined -fipa-icf" } */
+
+__attribute__ ((noinline, noclone))
+int f1 (int x, int y)
+{
+ return x + y;
+}
+
+__attribute__ ((noinline, noclone))
+int f2 (int x, int y)
+{
+ return x - y;
+}
+
+int
+main ()
+{
+ if (f1 (5, 6) != 11 || f2 (5, 6) != -1)
+ __builtin_abort ();
+ return 0;
+}
--- gcc/testsuite/gcc.dg/ipa/ipa-icf-31.c.jj 2014-11-10 18:59:16.604294652
+0100
+++ gcc/testsuite/gcc.dg/ipa/ipa-icf-31.c 2014-11-10 18:59:59.690519616
+0100
@@ -0,0 +1,41 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fipa-icf" } */
+
+__attribute__ ((noinline, noclone))
+int f1 (int x, int (*p1) (void), int (*p2) (void))
+{
+ if (x)
+ return p1 ();
+ else
+ return p2 ();
+}
+
+__attribute__ ((noinline, noclone))
+int f2 (int x, int (*p1) (void), int (*p2) (void))
+{
+ if (x)
+ return p2 ();
+ else
+ return p1 ();
+}
+
+__attribute__ ((noinline, noclone))
+int f3 (void)
+{
+ return 1;
+}
+
+__attribute__ ((noinline, noclone))
+int f4 (void)
+{
+ return 2;
+}
+
+int
+main ()
+{
+ if (f1 (0, f3, f4) != 2 || f1 (1, f3, f4) != 1 || f2 (0, f3, f4) != 1
+ || f2 (1, f3, f4) != 2)
+ __builtin_abort ();
+ return 0;
+}
Jakub
