Re: [PATCH] Fix ICE in lsplit when built with -O3 -fno-guess-branch-probability [PR103793]
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
Re: [PATCH] Fix ICE in lsplit when built with -O3 -fno-guess-branch-probability [PR103793]
> - /* 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
Re: [PATCH] Fix ICE in lsplit when built with -O3 -fno-guess-branch-probability [PR103793]
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. -- Thanks, Xionghu