Hi! As the following two testcases shows, there are lots of issues in ICF compare_gimple_call, in particular, it doesn't handle indirect calls properly (see the ipa-icf-31.c testcase), doesn't handle internal calls properly (see ubsan/ipa-icf-1.c), didn't check gimple_call flags at all.
As discussed with Honza, the call chain test (from Martin) is probably insufficient, I'm open with leaving it out from the patch, but perhaps what the patch has is better than nothing at all for now. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2014-11-10 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) @@ -662,9 +662,49 @@ func_checker::compare_gimple_call (gimpl t1 = gimple_call_fndecl (s1); t2 = gimple_call_fndecl (s2); - /* Function pointer variables are not supported yet. */ if (!compare_operand (t1, t2)) - return return_false(); + return return_false (); + + if (t1 == NULL_TREE) + { + t1 = gimple_call_fn (s1); + t2 = gimple_call_fn (s2); + if (!compare_operand (t1, t2)) + 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 == NULL_TREE) + { + if (fntype2) + return false; + } + else if (fntype2 == NULL_TREE) + return false; + else if (!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)) + return return_false_with_msg ("Tree 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