Hi!
When ICF encounteres a GIMPLE_PREDICT (as in the testcase) or GIMPLE_NOP,
it stops looking at further statements in the bb (assumes they must be the
same), which is of course wrong.
I'm attaching 3 versions of the patch, the first one I've
bootstrapped/regtested on x86_64-linux and i686-linux, the second one
will handle GIMPLE_PREDICT just as (fixed) GIMPLE_NOP handling, as Honza is
saying that at this point GIMPLE_PREDICT can be ignored (but we e.g. risk
when using the same function for something different that we'll not handle
GIMPLE_PREDICT right), the third one is the most minimal one (but I'm afraid
the lack of GIMPLE_EH_DISPATCH region checking might cause wrong-code).
Ok for trunk (which version) and 5.1 (which version)?
Jakub
2015-04-15 Jakub Jelinek <ja...@redhat.com>
PR ipa/65765
* ipa-icf-gimple.c (func_checker::compare_bb): For GIMPLE_NOP,
use break instead of return true. For GIMPLE_PREDICT, compare
predictor and outcome, and break instead of return true if it is
the same. For GIMPLE_EH_DISPATCH, compare dispatch region.
* g++.dg/ipa/pr65765.C: New test.
--- gcc/ipa-icf-gimple.c.jj 2015-04-12 21:29:24.000000000 +0200
+++ gcc/ipa-icf-gimple.c 2015-04-15 09:08:19.637659016 +0200
@@ -706,7 +706,11 @@ func_checker::compare_bb (sem_bb *bb1, s
return return_different_stmts (s1, s2, "GIMPLE_SWITCH");
break;
case GIMPLE_DEBUG:
+ break;
case GIMPLE_EH_DISPATCH:
+ if (gimple_eh_dispatch_region (as_a <geh_dispatch *> (s1))
+ != gimple_eh_dispatch_region (as_a <geh_dispatch *> (s2)))
+ return return_different_stmts (s1, s2, "GIMPLE_EH_DISPATCH");
break;
case GIMPLE_RESX:
if (!compare_gimple_resx (as_a <gresx *> (s1),
@@ -733,8 +737,12 @@ func_checker::compare_bb (sem_bb *bb1, s
return return_different_stmts (s1, s2, "GIMPLE_ASM");
break;
case GIMPLE_PREDICT:
+ if (gimple_predict_predictor (s1) != gimple_predict_predictor (s2)
+ || gimple_predict_outcome (s1) != gimple_predict_outcome (s2))
+ return return_different_stmts (s1, s2, "GIMPLE_PREDICT");
+ break;
case GIMPLE_NOP:
- return true;
+ break;
default:
return return_false_with_msg ("Unknown GIMPLE code reached");
}
--- gcc/testsuite/g++.dg/ipa/pr65765.C.jj 2015-04-15 09:14:03.329930193
+0200
+++ gcc/testsuite/g++.dg/ipa/pr65765.C 2015-04-15 09:13:41.000000000 +0200
@@ -0,0 +1,45 @@
+// PR ipa/65765
+// { dg-do run }
+// { dg-options "-O2" }
+
+int a, b, c, d, e;
+unsigned char h[] = { 1, 1 };
+
+__attribute__ ((cold)) int ModRM_Mode () { return a; }
+
+int
+ModRM_RM (int p1)
+{
+ return p1;
+}
+
+__attribute__ ((cold)) static bool ModRM_hasSIB (unsigned char p1)
+{
+ return ModRM_Mode () != 1 && ModRM_RM (p1);
+}
+
+__attribute__ ((cold)) static bool ModRM_hasRIP (unsigned char p1)
+{
+ return ModRM_Mode () && ModRM_RM (p1);
+}
+
+unsigned char *
+DisassembleHeapAccess (unsigned char *p1)
+{
+ b = *p1++;
+ if (ModRM_hasSIB (b))
+ c = *p1++;
+ int f = c, g = 0;
+ d = ModRM_hasRIP (g);
+ e = f == 0;
+ if (e)
+ p1 += sizeof 0;
+ return p1;
+}
+
+int
+main ()
+{
+ if (DisassembleHeapAccess (h) != h + 2)
+ __builtin_abort ();
+}
2015-04-15 Jakub Jelinek <ja...@redhat.com>
PR ipa/65765
* ipa-icf-gimple.c (func_checker::compare_bb): For GIMPLE_NOP
and GIMPLE_PREDICT use break instead of return true. For
GIMPLE_EH_DISPATCH, compare dispatch region.
* g++.dg/ipa/pr65765.C: New test.
--- gcc/ipa-icf-gimple.c.jj 2015-04-12 21:29:24.000000000 +0200
+++ gcc/ipa-icf-gimple.c 2015-04-15 09:08:19.637659016 +0200
@@ -706,7 +706,11 @@ func_checker::compare_bb (sem_bb *bb1, s
return return_different_stmts (s1, s2, "GIMPLE_SWITCH");
break;
case GIMPLE_DEBUG:
+ break;
case GIMPLE_EH_DISPATCH:
+ if (gimple_eh_dispatch_region (as_a <geh_dispatch *> (s1))
+ != gimple_eh_dispatch_region (as_a <geh_dispatch *> (s2)))
+ return return_different_stmts (s1, s2, "GIMPLE_EH_DISPATCH");
break;
case GIMPLE_RESX:
if (!compare_gimple_resx (as_a <gresx *> (s1),
@@ -734,7 +738,7 @@ func_checker::compare_bb (sem_bb *bb1, s
break;
case GIMPLE_PREDICT:
case GIMPLE_NOP:
- return true;
+ break;
default:
return return_false_with_msg ("Unknown GIMPLE code reached");
}
--- gcc/testsuite/g++.dg/ipa/pr65765.C.jj 2015-04-15 09:14:03.329930193
+0200
+++ gcc/testsuite/g++.dg/ipa/pr65765.C 2015-04-15 09:13:41.000000000 +0200
@@ -0,0 +1,45 @@
+// PR ipa/65765
+// { dg-do run }
+// { dg-options "-O2" }
+
+int a, b, c, d, e;
+unsigned char h[] = { 1, 1 };
+
+__attribute__ ((cold)) int ModRM_Mode () { return a; }
+
+int
+ModRM_RM (int p1)
+{
+ return p1;
+}
+
+__attribute__ ((cold)) static bool ModRM_hasSIB (unsigned char p1)
+{
+ return ModRM_Mode () != 1 && ModRM_RM (p1);
+}
+
+__attribute__ ((cold)) static bool ModRM_hasRIP (unsigned char p1)
+{
+ return ModRM_Mode () && ModRM_RM (p1);
+}
+
+unsigned char *
+DisassembleHeapAccess (unsigned char *p1)
+{
+ b = *p1++;
+ if (ModRM_hasSIB (b))
+ c = *p1++;
+ int f = c, g = 0;
+ d = ModRM_hasRIP (g);
+ e = f == 0;
+ if (e)
+ p1 += sizeof 0;
+ return p1;
+}
+
+int
+main ()
+{
+ if (DisassembleHeapAccess (h) != h + 2)
+ __builtin_abort ();
+}
2015-04-15 Jakub Jelinek <ja...@redhat.com>
PR ipa/65765
* ipa-icf-gimple.c (func_checker::compare_bb): For GIMPLE_NOP
and GIMPLE_PREDICT use break instead of return true.
* g++.dg/ipa/pr65765.C: New test.
--- gcc/ipa-icf-gimple.c.jj 2015-04-12 21:29:24.000000000 +0200
+++ gcc/ipa-icf-gimple.c 2015-04-15 09:08:19.637659016 +0200
@@ -734,7 +734,7 @@ func_checker::compare_bb (sem_bb *bb1, s
break;
case GIMPLE_PREDICT:
case GIMPLE_NOP:
- return true;
+ break;
default:
return return_false_with_msg ("Unknown GIMPLE code reached");
}
--- gcc/testsuite/g++.dg/ipa/pr65765.C.jj 2015-04-15 09:14:03.329930193
+0200
+++ gcc/testsuite/g++.dg/ipa/pr65765.C 2015-04-15 09:13:41.000000000 +0200
@@ -0,0 +1,45 @@
+// PR ipa/65765
+// { dg-do run }
+// { dg-options "-O2" }
+
+int a, b, c, d, e;
+unsigned char h[] = { 1, 1 };
+
+__attribute__ ((cold)) int ModRM_Mode () { return a; }
+
+int
+ModRM_RM (int p1)
+{
+ return p1;
+}
+
+__attribute__ ((cold)) static bool ModRM_hasSIB (unsigned char p1)
+{
+ return ModRM_Mode () != 1 && ModRM_RM (p1);
+}
+
+__attribute__ ((cold)) static bool ModRM_hasRIP (unsigned char p1)
+{
+ return ModRM_Mode () && ModRM_RM (p1);
+}
+
+unsigned char *
+DisassembleHeapAccess (unsigned char *p1)
+{
+ b = *p1++;
+ if (ModRM_hasSIB (b))
+ c = *p1++;
+ int f = c, g = 0;
+ d = ModRM_hasRIP (g);
+ e = f == 0;
+ if (e)
+ p1 += sizeof 0;
+ return p1;
+}
+
+int
+main ()
+{
+ if (DisassembleHeapAccess (h) != h + 2)
+ __builtin_abort ();
+}
