On 11/20/2014 05:41 PM, Richard Biener wrote:
On Thu, Nov 20, 2014 at 5:30 PM, Martin Liška <mli...@suse.cz> wrote:
Hello.
Following patch fixes ICE in IPA ICF. Problem was that number of non-debug
statements in a BB can
change (for instance by IPA split), so that the number is recomputed.
Huh, so can it get different for both candidates? I think the stmt compare
loop should be terminated on gsi_end_p of either iterator and return
false for any remaining non-debug-stmts on the other.
Thus, not walk all stmts twice here.
Hello.
Sorry for the previous patch, you are right it can be fixed in purer
way. Please take a look at attached patch.
As IPA split is run early I don't see how it should affect a real IPA
pass though?
Sorry for non precise information, the problematic BB is changed here:
#0 gsi_split_seq_before (i=0x7fffffffd550, pnew_seq=0x7fffffffd528) at
../../gcc/gimple-iterator.c:429
#1 0x0000000000b95a2a in gimple_split_block (bb=0x7ffff6c41548,
stmt=0x0) at ../../gcc/tree-cfg.c:5707
#2 0x00000000007563cf in split_block (bb=0x7ffff6c41548, i=i@entry=0x0)
at ../../gcc/cfghooks.c:508
#3 0x0000000000756b44 in split_block_after_labels (bb=<optimized out>)
at ../../gcc/cfghooks.c:549
#4 make_forwarder_block (bb=<optimized out>,
redirect_edge_p=redirect_edge_p@entry=0x75d4e0
<mfb_keep_just(edge_def*)>, new_bb_cbk=new_bb_cbk@entry=0x0) at
../../gcc/cfghooks.c:842
#5 0x000000000076085a in create_preheader (loop=0x7ffff6d56948,
flags=<optimized out>) at ../../gcc/cfgloopmanip.c:1563
#6 0x0000000000760aea in create_preheaders (flags=1) at
../../gcc/cfgloopmanip.c:1613
#7 0x00000000009bc6b0 in apply_loop_flags (flags=15) at
../../gcc/loop-init.c:75
#8 0x00000000009bc7d3 in loop_optimizer_init (flags=15) at
../../gcc/loop-init.c:136
#9 0x0000000000957914 in estimate_function_body_sizes
(node=0x7ffff6c47620, early=false) at ../../gcc/ipa-inline-analysis.c:2480
#10 0x000000000095948b in compute_inline_parameters
(node=0x7ffff6c47620, early=false) at ../../gcc/ipa-inline-analysis.c:2907
#11 0x000000000095bd88 in inline_analyze_function (node=0x7ffff6c47620)
at ../../gcc/ipa-inline-analysis.c:3994
#12 0x000000000095bed3 in inline_generate_summary () at
../../gcc/ipa-inline-analysis.c:4045
#13 0x0000000000a70b71 in execute_ipa_summary_passes
(ipa_pass=0x1dcb9e0) at ../../gcc/passes.c:2137
#14 0x0000000000777a15 in ipa_passes () at ../../gcc/cgraphunit.c:2074
#15 symbol_table::compile (this=this@entry=0x7ffff6c3a000) at
../../gcc/cgraphunit.c:2187
#16 0x0000000000778bcd in symbol_table::finalize_compilation_unit
(this=0x7ffff6c3a000) at ../../gcc/cgraphunit.c:2340
#17 0x00000000006580ee in c_write_global_declarations () at
../../gcc/c/c-decl.c:10777
#18 0x0000000000b5bb8b in compile_file () at ../../gcc/toplev.c:584
#19 0x0000000000b5def1 in do_compile () at ../../gcc/toplev.c:2041
#20 0x0000000000b5e0fa in toplev::main (this=0x7fffffffdc9f, argc=20,
argv=0x7fffffffdd98) at ../../gcc/toplev.c:2138
#21 0x000000000063f1d9 in main (argc=20, argv=0x7fffffffdd98) at
../../gcc/main.c:38
Patch can bootstrap on x86_64-linux-pc and no regression has been seen.
Ready for trunk?
Thanks,
Martin
Thanks,
Richard.
Patch can bootstrap on x86_64-linux-pc and no regression has been seen.
Ready for trunk?
Thanks,
Martin
>From 09b90f6a5ec1e49464f57c333af43574ad8c1375 Mon Sep 17 00:00:00 2001
From: mliska <mli...@suse.cz>
Date: Thu, 20 Nov 2014 16:28:54 +0100
Subject: [PATCH] Fix and new test.
gcc/ChangeLog:
2014-11-21 Martin Liska <mli...@suse.cz>
* gimple-iterator.h (gsi_start_bb_nondebug): New function.
* ipa-icf-gimple.c (func_checker::compare_bb): Correct iteration
replaces loop based on precomputed number of non-debug statements.
gcc/testsuite/ChangeLog:
2014-11-21 Martin Liska <mli...@suse.cz>
* gcc.dg/ipa/pr63909.c: New test.
---
gcc/gimple-iterator.h | 13 +++++++++++++
gcc/ipa-icf-gimple.c | 25 ++++++++++---------------
gcc/testsuite/gcc.dg/ipa/pr63909.c | 27 +++++++++++++++++++++++++++
3 files changed, 50 insertions(+), 15 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr63909.c
diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h
index fb6cc07..e9602b3 100644
--- a/gcc/gimple-iterator.h
+++ b/gcc/gimple-iterator.h
@@ -211,6 +211,19 @@ gsi_stmt (gimple_stmt_iterator i)
return i.ptr;
}
+/* Return a new iterator pointing to the first non-debug statement
+ in basic block BB. */
+
+static inline gimple_stmt_iterator
+gsi_start_bb_nondebug (basic_block bb)
+{
+ gimple_stmt_iterator gsi = gsi_start_bb (bb);
+ while (!gsi_end_p (gsi) && is_gimple_debug (gsi_stmt (gsi)))
+ gsi_next (&gsi);
+
+ return gsi;
+}
+
/* Return a block statement iterator that points to the first non-label
statement in block BB. */
diff --git a/gcc/ipa-icf-gimple.c b/gcc/ipa-icf-gimple.c
index 8f2a438..ec0290a 100644
--- a/gcc/ipa-icf-gimple.c
+++ b/gcc/ipa-icf-gimple.c
@@ -559,24 +559,16 @@ func_checker::parse_labels (sem_bb *bb)
bool
func_checker::compare_bb (sem_bb *bb1, sem_bb *bb2)
{
- unsigned i;
gimple_stmt_iterator gsi1, gsi2;
gimple s1, s2;
- if (bb1->nondbg_stmt_count != bb2->nondbg_stmt_count
- || bb1->edge_count != bb2->edge_count)
- return return_false ();
-
- gsi1 = gsi_start_bb (bb1->bb);
- gsi2 = gsi_start_bb (bb2->bb);
+ gsi1 = gsi_start_bb_nondebug (bb1->bb);
+ gsi2 = gsi_start_bb_nondebug (bb2->bb);
- for (i = 0; i < bb1->nondbg_stmt_count; i++)
+ while (!gsi_end_p (gsi1))
{
- if (is_gimple_debug (gsi_stmt (gsi1)))
- gsi_next_nondebug (&gsi1);
-
- if (is_gimple_debug (gsi_stmt (gsi2)))
- gsi_next_nondebug (&gsi2);
+ if (gsi_end_p (gsi2))
+ return return_false ();
s1 = gsi_stmt (gsi1);
s2 = gsi_stmt (gsi2);
@@ -646,10 +638,13 @@ func_checker::compare_bb (sem_bb *bb1, sem_bb *bb2)
return return_false_with_msg ("Unknown GIMPLE code reached");
}
- gsi_next (&gsi1);
- gsi_next (&gsi2);
+ gsi_next_nondebug (&gsi1);
+ gsi_next_nondebug (&gsi2);
}
+ if (!gsi_end_p (gsi2))
+ return return_false ();
+
return true;
}
diff --git a/gcc/testsuite/gcc.dg/ipa/pr63909.c b/gcc/testsuite/gcc.dg/ipa/pr63909.c
new file mode 100644
index 0000000..8538e21
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr63909.c
@@ -0,0 +1,27 @@
+/* { dg-options "-O2 -fno-guess-branch-probability" } */
+
+int z;
+
+__attribute__((noinline))
+void g ()
+{
+ if (++z)
+ __builtin_exit (0);
+ g ();
+}
+
+__attribute__((noinline))
+void f ()
+{
+ if (++z)
+ __builtin_exit (0);
+ f ();
+}
+
+int main()
+{
+ f ();
+ g ();
+
+ return 0;
+}
--
2.1.2
