[PATCH] Fix for PR63569

[PATCH] Fix for PR63569

[Martin Liška]

[-- Attachment #1: Type: text/plain, Size: 153 bytes --]

Hello.

Following patch fixes PR63569.

Bootstrap executed on ppc64-linux and no regression seen on x86_64-pc-linux.
Ready for trunk?

Thank you,
Martin

[-- Attachment #2: ipa-icf-volatility.changelog --]
[-- Type: text/plain, Size: 387 bytes --]

gcc/testsuite/ChangeLog:

2014-10-17  Martin Liska  <mliska@suse.cz>

	* gcc.dg/ipa/ipa-icf-31.c: New test.


gcc/ChangeLog:

2014-10-17  Martin Liska  <mliska@suse.cz>

	* ipa-icf-gimple.c (func_checker::compare_volatility): New function.
	(func_checker::compare_gimple_call): Volatility check added.
	(func_checker::compare_gimple_assign): Likewise.
	* ipa-icf-gimple.h: New function.

[-- Attachment #3: ipa-icf-volatility.patch --]
[-- Type: text/x-patch, Size: 2715 bytes --]

diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 792a3e4..1b9ee85 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -452,6 +452,17 @@ func_checker::compare_tree_list_operand (tree t1, tree t2)
   return true;
 }
 
+/* Compares if both trees T1 and T2 have equal volatility.  */
+
+bool
+func_checker::compare_volatility (tree t1, tree t2)
+{
+  if (t1 && t2)
+    return TREE_THIS_VOLATILE (t1) == TREE_THIS_VOLATILE (t2);
+
+  return !(t1 || t2);
+}
+
 /* Verifies that trees T1 and T2, representing function declarations
    are equivalent from perspective of ICF.  */
 
@@ -663,6 +674,9 @@ func_checker::compare_gimple_call (gimple s1, gimple s2)
   t1 = gimple_get_lhs (s1);
   t2 = gimple_get_lhs (s2);
 
+  if (!compare_volatility (t1, t2))
+    return return_false_with_msg ("different volatility for call statement");
+
   return compare_operand (t1, t2);
 }
 
@@ -696,8 +710,11 @@ func_checker::compare_gimple_assign (gimple s1, gimple s2)
 
       if (!compare_operand (arg1, arg2))
 	return false;
-    }
 
+      if (!compare_volatility (arg1, arg2))
+	return return_false_with_msg ("different volatility for assignment "
+	  "statement");
+    }
 
   return true;
 }
diff --git a/gcc/ipa-icf-gimple.h b/gcc/ipa-icf-gimple.h
index 8487a2a..b791c21 100644
--- a/gcc/ipa-icf-gimple.h
+++ b/gcc/ipa-icf-gimple.h
@@ -209,6 +209,10 @@ public:
      two trees are semantically equivalent.  */
   bool compare_tree_list_operand (tree t1, tree t2);
 
+  /* Compares two tree list operands T1 and T2 and returns true if these
+     two trees are semantically equivalent.  */
+  bool compare_volatility (tree t1, tree t2);
+
   /* Verifies that trees T1 and T2, representing function declarations
      are equivalent from perspective of ICF.  */
   bool compare_function_decl (tree t1, tree t2);
diff --git a/gcc/testsuite/gcc.dg/ipa/ipa-icf-31.c b/gcc/testsuite/gcc.dg/ipa/ipa-icf-31.c
new file mode 100644
index 0000000..e70d72d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/ipa-icf-31.c
@@ -0,0 +1,33 @@
+/* { dg-do compile } */
+/* { dg-options "-fipa-icf -fdump-ipa-icf-details"  } */
+
+
+static int f(int t, int *a) __attribute__((noinline));
+
+static int g(int t, volatile int *a) __attribute__((noinline));
+static int g(int t, volatile int *a)
+{
+  int i;
+  int tt = 0;
+  for(i=0;i<t;i++)
+    tt += *a;
+  return tt;
+}
+static int f(int t, int *a)
+{
+  int i;
+  int tt = 0;
+  for(i=0;i<t;i++)
+    tt += *a;
+  return tt;
+}
+
+
+int main()
+{
+  return 0;
+}
+
+/* { dg-final { scan-ipa-dump "Equal symbols: 0" "icf"  } } */
+/* { dg-final { scan-ipa-dump "different volatility for assignment statement" "icf"  } } */
+/* { dg-final { cleanup-ipa-dump "icf" } } */

Re: [PATCH] Fix for PR63569

[Richard Biener]

On Fri, Oct 17, 2014 at 3:36 PM, Martin Liška <mliska@suse.cz> wrote:
> Hello.
>
> Following patch fixes PR63569.
>
> Bootstrap executed on ppc64-linux and no regression seen on x86_64-pc-linux.
> Ready for trunk?

Um.  As suggested in the bugreport I replied to please work on splitting
out general operand vs. memory operand compare.

+bool
+func_checker::compare_volatility (tree t1, tree t2)
+{
+  if (t1 && t2)
+    return TREE_THIS_VOLATILE (t1) == TREE_THIS_VOLATILE (t2);
+
+  return !(t1 || t2);

The last check looks unrelated to me.  Either you want to do this
quick check inline for all operand compares or defer to compare_operand?

Btw, I think the volatility check would be better placed in the memory
operand compare function when comparing handled-components and
decls.

Richard.

> Thank you,
> Martin