Re: Do not account __builtin_unreachable guards in inliner

[Richard Biener <richard.guenther@gmail.com>]

On Fri, Jun 23, 2023 at 12:11 PM Jan Hubicka <hubicka@ucw.cz> wrote:
>
> > On Mon, Jun 19, 2023 at 12:15 PM Jan Hubicka <hubicka@ucw.cz> wrote:
> > >
> > > > On Mon, Jun 19, 2023 at 9:52 AM Jan Hubicka via Gcc-patches
> > > > <gcc-patches@gcc.gnu.org> wrote:
> > > > >
> > > > > Hi,
> > > > > this was suggested earlier somewhere, but I can not find the thread.
> > > > > C++ has assume attribute that expands int
> > > > >   if (conditional)
> > > > >     __builtin_unreachable ()
> > > > > We do not want to account the conditional in inline heuristics since
> > > > > we know that it is going to be optimized out.
> > > > >
> > > > > Bootstrapped/regtested x86_64-linux, will commit it later today if
> > > > > thre are no complains.
> > > >
> > > > I think we also had the request to not account the condition feeding
> > > > stmts (if they only feed it and have no side-effects).  libstdc++ has
> > > > complex range comparisons here.  Also ...
> > >
> > > I was thinking of this: it depends on how smart do we want to get.
> > > We also have dead conditionals guarding clobbers, predicts and other
> > > stuff.  In general we can use mark phase of cd-dce telling it to ignore
> > > those statements and then use its resut in the analysis.
> >
> > Hmm, possible but a bit heavy-handed.  There's simple_dce_from_worklist
> > which might be a way to do this (of course we cannot use that 1:1).  Also
> > then consider
> >
> >  a = a + 1;
> >  if (a > 10)
> >    __builtin_unreachable ();
> >  if (a < 5)
> >    __builtin_unreachable ();
> >
> > and a has more than one use but both are going away.  So indeed a
> > more global analysis would be needed to get the full benefit.
>
> I was looking into simple_dce_from_worklist and if I understand it
> right, it simply walks list of SSA names which probably lost some uses
> by the consuming pass. If they have zero non-debug uses and defining statement has
> no side effects, then they are removed.
>
> I think this is not really fitting the bill here since the example above
> is likely to be common and also if we want one assign filling
> conditional optimized out, we probably want to handle case with multiple
> assignments.  What about
>  1) walk function body and see if there are conditionals we know will be
>     optimized out (at the begining those can be only those which has one
>     arm reaching __bulitin_unreachable
>  2) if there are none, just proceed with fnsummary construction
>  3) if there were some, do non-cd-dce mark stage which will skip those
>     dead conditional identified in 1
>     and proceed to fnsummary construction with additional bitmap of
>     marked stmts.

So you need to feed it with extra info on the optimized out stmts because
as-is it will not remove __builtin_unreachable ().  That means you're
doing the find_obviously_necessary_stmts manually, skipping the
conditional and all stmts it controls to the __builtin_unreachable () path?

I also think you want something cheaper than non-cd-dce mark, you also don't
want to bother with stores/loads?

Also when you only do this conditional how do you plan to use the result?

Richard.

>
> This should be cheaper than unconditionally doing cd-dce and should
> handle common cases?
> Honza