Hi!
The following two testcases show multiple issues in
reorder_basic_blocks_simple. Both issues only occur if the greedy algorithm
decides to put some basic block before the ENTRY successor. In that case
reorder_basic_blocks_simple has code to split the ENTRY successor edge
and put the new bb before all the others. If pass_partition_blocks
was done earlier, one issue is that in this case (when the ENTRY successor
edge was ignored by the greedy algorithm) ENTRY_BLOCK_PTR_FOR_FN (cfun)->aux
is the ENTRY block itself, and only the non-fixed basic blocks are cold or
hot, not ENTRY/EXIT, so starting with current_partition set to BB_PARTITION
of the ENTRY block doesn't work - it will be BB_UNPARITIONED and so neither
of the passes of the following loop will do anything. IMHO we want to
use the partition of the ENTRY successor bb, whether it is hot or cold
(typically hot). The other issue is that when we force_nonfallthru
on the ENTRY successor edge, e->src is BB_UNPARTITIONED, e->dest if
bbpart was done is either hot or cold (but for these edges we of course
never set EDGE_CROSSING). force_nonfallthru_and_redirect creates a new
bb which will be BB_UNPARTITIONED and so the new edge is EDGE_CROSSING
and the new jump is CROSSING_JUMP_P. Then reorder_basic_blocks_simple
fixes up the partition of the new bb through BB_COPY_PARTITION, but it is
too late, it would need to undo EDGE_CROSSING flag on the edge and
CROSSING_JUMP_P too. IMHO it is better to get the partition right from the
beginning, then we don't have to undo anything.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2017-12-21 Jakub Jelinek <ja...@redhat.com>
PR rtl-optimization/80747
PR rtl-optimization/83512
* cfgrtl.c (force_nonfallthru_and_redirect): When splitting
succ edge from ENTRY, copy partition from e->dest to the newly
created bb.
* bb-reorder.c (reorder_basic_blocks_simple): If last_tail is
ENTRY, use BB_PARTITION of its successor block as current_partition.
Don't copy partition when splitting succ edge from ENTRY.
* gcc.dg/pr80747.c: New test.
* gcc.dg/pr83512.c: New test.
--- gcc/cfgrtl.c.jj 2017-12-20 20:40:19.000000000 +0100
+++ gcc/cfgrtl.c 2017-12-21 16:25:24.172698221 +0100
@@ -1534,6 +1534,9 @@ force_nonfallthru_and_redirect (edge e,
ENTRY_BLOCK_PTR_FOR_FN (cfun));
bb->count = ENTRY_BLOCK_PTR_FOR_FN (cfun)->count;
+ /* Make sure new block ends up in correct hot/cold section. */
+ BB_COPY_PARTITION (bb, e->dest);
+
/* Change the existing edge's source to be the new block, and add
a new edge from the entry block to the new block. */
e->src = bb;
--- gcc/bb-reorder.c.jj 2017-11-22 23:34:45.000000000 +0100
+++ gcc/bb-reorder.c 2017-12-21 16:25:54.237319354 +0100
@@ -2405,7 +2405,10 @@ reorder_basic_blocks_simple (void)
basic_block last_tail = (basic_block) ENTRY_BLOCK_PTR_FOR_FN (cfun)->aux;
- int current_partition = BB_PARTITION (last_tail);
+ int current_partition
+ = BB_PARTITION (last_tail == ENTRY_BLOCK_PTR_FOR_FN (cfun)
+ ? EDGE_SUCC (ENTRY_BLOCK_PTR_FOR_FN (cfun), 0)->dest
+ : last_tail);
bool need_another_pass = true;
for (int pass = 0; pass < 2 && need_another_pass; pass++)
@@ -2446,7 +2449,6 @@ reorder_basic_blocks_simple (void)
{
force_nonfallthru (e);
e->src->aux = ENTRY_BLOCK_PTR_FOR_FN (cfun)->aux;
- BB_COPY_PARTITION (e->src, e->dest);
}
}
--- gcc/testsuite/gcc.dg/pr80747.c.jj 2017-12-21 16:27:18.234260846 +0100
+++ gcc/testsuite/gcc.dg/pr80747.c 2017-12-21 16:26:56.000000000 +0100
@@ -0,0 +1,18 @@
+/* PR rtl-optimization/80747 */
+/* { dg-do compile } */
+/* { dg-options "-fprofile-use -freorder-blocks-and-partition -O1
-foptimize-sibling-calls" } */
+
+int
+foo (int a)
+{
+ int r;
+ if (a & 1)
+ r = foo (a - 1);
+ else if (a)
+ r = foo (a - 2);
+ else
+ return 0;
+ if (r)
+ r = r;
+ return r;
+}
--- gcc/testsuite/gcc.dg/pr83512.c.jj 2017-12-21 16:41:11.487769515 +0100
+++ gcc/testsuite/gcc.dg/pr83512.c 2017-12-21 16:40:50.000000000 +0100
@@ -0,0 +1,16 @@
+/* PR rtl-optimization/83512 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -freorder-blocks-algorithm=simple" } */
+
+int a;
+
+void
+foo (int *x)
+{
+ for (;;)
+ {
+ for (*x = 0; *x < 1; *x++)
+ ;
+ ++a;
+ }
+}
Jakub
