Re: [PATCH] tree-optimization/105679 - disable backward threading of unlikely entry

[Jeff Law <jeffreyalaw@gmail.com>]

On 7/29/2022 2:54 AM, Richard Biener via Gcc-patches wrote:
> The following makes the backward threader reject threads whose entry
> edge is probably never executed according to the profile.  That in
> particular, for the testcase, avoids threading the irq == 1 check
> on the path where irq > 31, thereby avoiding spurious -Warray-bounds
> diagnostics
>
>    if (irq_1(D) > 31)
>      goto <bb 3>; [0.00%]
>    else
>      goto <bb 4>; [100.00%]
>
> ;;   basic block 3, loop depth 0, count 0 (precise), probably never executed
>    _2 = (unsigned long) irq_1(D);
>    __builtin___ubsan_handle_shift_out_of_bounds (&*.Lubsan_data0, 1, _2);
>
>    _3 = 1 << irq_1(D);
>    mask_4 = (u32) _3;
>    entry = instance_5(D)->array[irq_1(D)];
>    capture (mask_4);
>    if (level_6(D) != 0)
>      goto <bb 7>; [34.00%]
>    else
>      goto <bb 5>; [66.00%]
>
> ;;   basic block 5, loop depth 0, count 708669600 (estimated locally), maybe hot  if (irq_1(D) == 1)
>      goto <bb 7>; [20.97%]
>    else
>      goto <bb 6>; [79.03%]
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>
> The testcase in the PR requries both ubsan and sancov so I'm not sure
> where to put it but IIRC there were quite some duplicate PRs wrt
> threading unlikely paths exposing diagnostics, eventually some
> testcase will come out of those (when we identify them).  Note
> the patch is quite conservative in only disabling likely never
> executed paths rather than requiring maybe_hot_edge_p (OTOH those
> are somewhat similar in the end).
>
> I'm going to push it when testing finishes but maybe there are some
> testcases to adjust.
>
> 	PR tree-optimization/105679
> 	* tree-ssa-threadbackwards.cc
> 	(back_threader_profitability::profitable_path_p): Avoid threading
> 	when the entry edge is probably never executed.
OK.   And more generally I'm absolutely OK with improving the costing 
heuristics.

The one thing we need to keep in mind when adding this kind of check is 
that we may start seeing new diagnostics from the middle end -- while 
the path may not be profitable to thread from a performance standpoint, 
it may be useful to thread from a diagnostic standpoint.

But I'd argue that for such cases we really want to move to a model 
where the predicate analysis bits are used consistently to prune away 
"can't happen" cases for middle end diagnostics rather than relying on 
threading to optimize away can't happen cold paths.

Jeff