Re: [PATCH] Tame path_range_query::compute_imports

[Richard Biener <rguenther@suse.de>]

On Tue, 16 Aug 2022, Aldy Hernandez wrote:

> On Mon, Aug 15, 2022 at 11:53 AM Richard Biener <rguenther@suse.de> wrote:
> >
> > The remaining issue I have with the path_range_query is that
> > we re-use the same instance in the back threader but the
> > class doesn't provide any way to "restart", aka give m_path
> > a lifetime.  The "start a new path" API seems to essentially
> > be compute_ranges (), but there's no convenient way to end.
> > It might be more appropriate to re-instantiate the path_range_query,
> > though that comes at a cost.  Or abstract an actual query, like
> > adding a
> 
> Yes, compute_ranges() is the way to start a new path.  It resets exit
> dependencies, the path, relations, etc.  I think it would be clearer
> to name it set_path (or reset_path if we want to share nomenclature
> with the path_oracle).
> 
> Instantiating a new path_range_query per path is fine, as long as you
> allocate the ranger it uses yourself, instead of letting
> path_range_query allocate it.  Instantiating a new ranger does have a
> cost, and it's best to let path_range_query re-use a ranger from path
> to path.  This is why path_range_query is (class) global in the
> backwards threader.  Andrew mentioned last year making the ranger
> start-up 0-cost, but it still leaves the internal caching the ranger
> will do from path to path (well, the stuff outside the current path,
> cause the stuff inside the path is irrelevant since it'll get
> recalculated).
> 
> However, why can't you use compute_ranges (or whatever we rename it to ;-))??

I've added

   auto_bb_flag m_on_path;

to the path query and at set_path time set m_on_path on each BB so
the m_path->contains () linear walks go away.  But I need to clear
the flag for which I would need something like finish_path (),
doing it just at the point we deallocate the path query object
or when we set the next path via compute_ranges doesn't look right
(and in fact it doesn't work out-of-the-box without adjusting the
lifetime of the path query object).

So a more incremental thing would be to add such finish_path ()
or to make the whole path query object single-shot, thus remove
compute_ranges and instead use the CTOR for this.

Probably not too important (for short paths).

Richard.

> Aldy
> 
> >
> >   query start (const vec<basic_block> &);
> >
> > and make range_of_* and friends members of a new 'query' class
> > instantiated by path_range_query.  I ran into this when trying
> > to axe the linear array walks for the .contains() query on the
> > path where I need a convenient way to "clenanup" after a path
> > query is done.
> >
> > Richard.
> >