Re: [PATCH] Fix ICE in lsplit when built with -O3 -fno-guess-branch-probability [PR103793]

[Jeff Law via Gcc-patches]

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]

[Jan Hubicka via Gcc-patches]

> - /* 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]

[Xionghu Luo via Gcc-patches]

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