Re: [PATCH] Fix some ICF gimple_call handling issues

[Jakub Jelinek <jakub at redhat dot com>]

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.

2014-11-11  Jakub Jelinek  <jakub@redhat.com>
	    Martin Liska  <mliska@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