On December 21, 2017 7:05:10 PM GMT+01:00, Jakub Jelinek <ja...@redhat.com>
wrote:
>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?
OK.
Richard.
>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
