On Fri, 29 Jul 2022, Aldy Hernandez wrote:
> On Fri, Jul 29, 2022 at 11:02 AM Richard Biener <rguent...@suse.de> 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).
>
> Sounds reasonable, if for no other reason than to quiet down the
> annoyingly large amount of false positives we have because of
> aggressive threading, or better ranges as a whole.
>
> How does this fit in with Jeff's original idea of isolating known
> problematic paths (null dereferences?), which in theory are also
> unlikely?
If profile estimation gets to see those the paths should turn unlikely.
Path isolation should also make sure to adjust the profile accordingly,
though local profile adjustments are somewhat difficult.
Richard.
> Thanks for doing this.
> Aldy
>
> >
> > 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.
> > ---
> > gcc/tree-ssa-threadbackward.cc | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/gcc/tree-ssa-threadbackward.cc b/gcc/tree-ssa-threadbackward.cc
> > index 3519aca84cd..90f5331c265 100644
> > --- a/gcc/tree-ssa-threadbackward.cc
> > +++ b/gcc/tree-ssa-threadbackward.cc
> > @@ -777,6 +777,15 @@ back_threader_profitability::profitable_path_p (const
> > vec<basic_block> &m_path,
> > "exceeds PARAM_MAX_FSM_THREAD_PATH_INSNS.\n");
> > return false;
> > }
> > + edge entry = find_edge (m_path[m_path.length () - 1],
> > + m_path[m_path.length () - 2]);
> > + if (probably_never_executed_edge_p (cfun, entry))
> > + {
> > + if (dump_file && (dump_flags & TDF_DETAILS))
> > + fprintf (dump_file, " FAIL: Jump-thread path not considered: "
> > + "path entry is probably never executed.\n");
> > + return false;
> > + }
> > }
> > else if (!m_speed_p && n_insns > 1)
> > {
> > --
> > 2.35.3
> >
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
