Fix ICE in lsplit when built with -O3 -fno-guess-branch-probability [PR103793]
Commit Message
no-guess-branch-probability option requires profile_count with initialized_p
guard. Also merge the missed part of r12-6086 of factor out function to
avoid duplicate code.
gcc/ChangeLog:
PR 103793
* tree-ssa-loop-split.c (fix_loop_bb_probability): New function.
(split_loop): Only update loop1's exit probability if
initialized.
(do_split_loop_on_cond): Call fix_loop_bb_probability.
gcc/testsuite/ChangeLog:
PR 103793
* gcc.dg/pr103793.c: New test.
---
gcc/tree-ssa-loop-split.c | 98 +++++++++++++++------------------
gcc/testsuite/gcc.dg/pr103793.c | 12 ++++
2 files changed, 57 insertions(+), 53 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/pr103793.c
Comments
On 12/21/2021 7:19 PM, Xionghu Luo wrote:
> no-guess-branch-probability option requires profile_count with initialized_p
> guard. Also merge the missed part of r12-6086 of factor out function to
> avoid duplicate code.
>
> gcc/ChangeLog:
>
> PR 103793
> * tree-ssa-loop-split.c (fix_loop_bb_probability): New function.
> (split_loop): Only update loop1's exit probability if
> initialized.
> (do_split_loop_on_cond): Call fix_loop_bb_probability.
>
> gcc/testsuite/ChangeLog:
>
> PR 103793
> * gcc.dg/pr103793.c: New test.
OK
jeff
> - /* Proportion second loop's bb counts except those dominated by false
> - branch to avoid drop 1s down. */
> - basic_block bbi_copy = get_bb_copy (false_edge->dest);
> - bbs2 = get_loop_body (loop2);
> - for (j = 0; j < loop2->num_nodes; j++)
> - if (bbs2[j] == loop2->latch
> - || !dominated_by_p (CDI_DOMINATORS, bbs2[j], bbi_copy))
> - bbs2[j]->count = bbs2[j]->count.apply_probability (
> - true_edge->probability.invert ());
> - free (bbs2);
> + if (true_edge->probability.initialized_p ())
> + {
> + edge exit_to_latch1 = single_pred_edge (loop1->latch);
> + exit_to_latch1->probability
> + = exit_to_latch1->probability.apply_scale (
> + true_edge->probability.to_reg_br_prob_base (),
> + REG_BR_PROB_BASE);
This should be
exit_to_latch1->probability *= true_edge->probability;
whici will do the right thing to undefined probabilities and will not
cause unnecesary roundoff errors and precision info loss.
Can you please update that in the patch (and drop the initialized_p
check)?
Honza
On 2021/12/29 03:33, Jan Hubicka wrote:
>> - /* Proportion second loop's bb counts except those dominated by false
>> - branch to avoid drop 1s down. */
>> - basic_block bbi_copy = get_bb_copy (false_edge->dest);
>> - bbs2 = get_loop_body (loop2);
>> - for (j = 0; j < loop2->num_nodes; j++)
>> - if (bbs2[j] == loop2->latch
>> - || !dominated_by_p (CDI_DOMINATORS, bbs2[j], bbi_copy))
>> - bbs2[j]->count = bbs2[j]->count.apply_probability (
>> - true_edge->probability.invert ());
>> - free (bbs2);
>> + if (true_edge->probability.initialized_p ())
>> + {
>> + edge exit_to_latch1 = single_pred_edge (loop1->latch);
>> + exit_to_latch1->probability
>> + = exit_to_latch1->probability.apply_scale (
>> + true_edge->probability.to_reg_br_prob_base (),
>> + REG_BR_PROB_BASE);
> This should be
> exit_to_latch1->probability *= true_edge->probability;
> whici will do the right thing to undefined probabilities and will not
> cause unnecesary roundoff errors and precision info loss.
>
> Can you please update that in the patch (and drop the initialized_p
> check)?
Thanks, updated and committed with r12-6140.
@@ -484,6 +484,39 @@ compute_new_first_bound (gimple_seq *stmts, class tree_niter_desc *niter,
return newend;
}
+/* Fix the two loop's bb count after split based on the split edge probability,
+ don't adjust the bbs dominated by true branches of that loop to avoid
+ dropping 1s down. */
+static void
+fix_loop_bb_probability (class loop *loop1, class loop *loop2, edge true_edge,
+ edge false_edge)
+{
+ update_ssa (TODO_update_ssa);
+
+ /* Proportion first loop's bb counts except those dominated by true
+ branch to avoid drop 1s down. */
+ basic_block *bbs1, *bbs2;
+ bbs1 = get_loop_body (loop1);
+ unsigned j;
+ for (j = 0; j < loop1->num_nodes; j++)
+ if (bbs1[j] == loop1->latch
+ || !dominated_by_p (CDI_DOMINATORS, bbs1[j], true_edge->dest))
+ bbs1[j]->count
+ = bbs1[j]->count.apply_probability (true_edge->probability);
+ free (bbs1);
+
+ /* Proportion second loop's bb counts except those dominated by false
+ branch to avoid drop 1s down. */
+ basic_block bbi_copy = get_bb_copy (false_edge->dest);
+ bbs2 = get_loop_body (loop2);
+ for (j = 0; j < loop2->num_nodes; j++)
+ if (bbs2[j] == loop2->latch
+ || !dominated_by_p (CDI_DOMINATORS, bbs2[j], bbi_copy))
+ bbs2[j]->count
+ = bbs2[j]->count.apply_probability (true_edge->probability.invert ());
+ free (bbs2);
+}
+
/* Checks if LOOP contains an conditional block whose condition
depends on which side in the iteration space it is, and if so
splits the iteration space into two loops. Returns true if the
@@ -610,37 +643,19 @@ split_loop (class loop *loop1)
tree guard_next = PHI_ARG_DEF_FROM_EDGE (phi, loop_latch_edge (loop1));
patch_loop_exit (loop1, guard_stmt, guard_next, newend, initial_true);
- update_ssa (TODO_update_ssa);
-
- /* Proportion first loop's bb counts except those dominated by true
- branch to avoid drop 1s down. */
- basic_block *bbs1, *bbs2;
- bbs1 = get_loop_body (loop1);
- unsigned j;
- for (j = 0; j < loop1->num_nodes; j++)
- if (bbs1[j] == loop1->latch
- || !dominated_by_p (CDI_DOMINATORS, bbs1[j], true_edge->dest))
- bbs1[j]->count
- = bbs1[j]->count.apply_probability (true_edge->probability);
- free (bbs1);
+ fix_loop_bb_probability (loop1, loop2, true_edge, false_edge);
/* Fix first loop's exit probability after scaling. */
- edge exit_to_latch1 = single_pred_edge (loop1->latch);
- exit_to_latch1->probability = exit_to_latch1->probability.apply_scale (
- true_edge->probability.to_reg_br_prob_base (), REG_BR_PROB_BASE);
- single_exit (loop1)->probability
- = exit_to_latch1->probability.invert ();
-
- /* Proportion second loop's bb counts except those dominated by false
- branch to avoid drop 1s down. */
- basic_block bbi_copy = get_bb_copy (false_edge->dest);
- bbs2 = get_loop_body (loop2);
- for (j = 0; j < loop2->num_nodes; j++)
- if (bbs2[j] == loop2->latch
- || !dominated_by_p (CDI_DOMINATORS, bbs2[j], bbi_copy))
- bbs2[j]->count = bbs2[j]->count.apply_probability (
- true_edge->probability.invert ());
- free (bbs2);
+ if (true_edge->probability.initialized_p ())
+ {
+ edge exit_to_latch1 = single_pred_edge (loop1->latch);
+ exit_to_latch1->probability
+ = exit_to_latch1->probability.apply_scale (
+ true_edge->probability.to_reg_br_prob_base (),
+ REG_BR_PROB_BASE);
+ single_exit (loop1)->probability
+ = exit_to_latch1->probability.invert ();
+ }
/* Finally patch out the two copies of the condition to be always
true/false (or opposite). */
@@ -1570,40 +1585,17 @@ do_split_loop_on_cond (struct loop *loop1, edge invar_branch)
between loop1 and loop2. */
connect_loop_phis (loop1, loop2, to_loop2);
- update_ssa (TODO_update_ssa);
-
edge true_edge, false_edge, skip_edge1, skip_edge2;
extract_true_false_edges_from_block (cond_bb, &true_edge, &false_edge);
- /* Proportion first loop's bb counts except those dominated by true
- branch to avoid drop 1s down. */
skip_edge1 = true_invar ? false_edge : true_edge;
skip_edge2 = true_invar ? true_edge : false_edge;
- basic_block *bbs1, *bbs2;
- bbs1 = get_loop_body (loop1);
- unsigned j;
- for (j = 0; j < loop1->num_nodes; j++)
- if (bbs1[j] == loop1->latch
- || !dominated_by_p (CDI_DOMINATORS, bbs1[j], skip_edge1->dest))
- bbs1[j]->count
- = bbs1[j]->count.apply_probability (skip_edge1->probability);
- free (bbs1);
+ fix_loop_bb_probability (loop1, loop2, skip_edge1, skip_edge2);
/* Fix first loop's exit probability after scaling. */
to_loop1->probability = invar_branch->probability.invert ();
to_loop2->probability = invar_branch->probability;
- /* Proportion second loop's bb counts except those dominated by false
- branch to avoid drop 1s down. */
- basic_block bbi_copy = get_bb_copy (skip_edge2->dest);
- bbs2 = get_loop_body (loop2);
- for (j = 0; j < loop2->num_nodes; j++)
- if (bbs2[j] == loop2->latch
- || !dominated_by_p (CDI_DOMINATORS, bbs2[j], bbi_copy))
- bbs2[j]->count
- = bbs2[j]->count.apply_probability (skip_edge2->probability);
- free (bbs2);
-
free_original_copy_tables ();
return true;
new file mode 100644
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fno-guess-branch-probability } */
+
+extern void bar (void);
+
+void
+foo (int x, int w)
+{
+ for (int y; y < w; y++)
+ if (y < x)
+ bar ();
+}