On Tue, 22 May 2018, David Malcolm wrote:
> On Tue, 2018-05-22 at 10:43 +0200, Richard Biener wrote:
> > On Mon, 21 May 2018, Jeff Law wrote:
> >
> > > On 05/18/2018 07:15 AM, David Malcolm wrote:
> > > > On Fri, 2018-05-18 at 13:11 +0200, Richard Biener wrote:
> > > > > The following adds a simple alloc/free_flag machinery
> > > > > allocating
> > > > > bits from an int typed pool and applies that to bb->flags and
> > > > > edge-
> > > > > > flags.
> > > > >
> > > > > This should allow infrastructure pieces to use egde/bb flags
> > > > > temporarily
> > > > > without worrying that users might already use it as for example
> > > > > BB_VISITED and friends. It converts one clever user to the new
> > > > > interface.
> > > > >
> > > > > The allocation state is per CFG but we could also make it
> > > > > global
> > > > > or merge the two pools so one allocates a flag that can be used
> > > > > for
> > > > > bbs and edges at the same time.
> > > > >
> > > > > Thus - any opinions welcome. I'm mainly targeting cfganal
> > > > > algorithms
> > > > > where I want to add a few region-based ones that to be
> > > > > O(region-size)
> > > > > complexity may not use sbitmaps for visited sets because of the
> > > > > clearing
> > > > > overhead and bitmaps are probably more expensive to use than a
> > > > > BB/edge
> > > > > flag that needs to be cleared afterwards.
> > > > >
> > > > > Built on x86_64, otherwise untested.
> > > > >
> > > > > Any comments?
> > > >
> > > > Rather than putting alloc/free pairs at the usage sites, how
> > > > about an
> > > > RAII class? Something like this:
> > >
> > > Yes, please if at all possible we should be using RAII.
> >
> > So like the following? (see comments in the hwint.h hunk for
> > extra C++ questions...)
> >
> > I dropped the non-RAII interface - it's very likely never needed.
> >
> > Better suggestions for placement of auto_flag welcome.
>
> Do you have ideas for other uses? If not, maybe just put it in cfg.h
> right in front of auto_edge_flag and auto_bb_flag, for simplicity?
I don't have more users but of course gimple stmts and tree nodes
would come to my mind ;) Basically nodes in any data structure we walk
and that we can (cheaply) re-walk to clear flags in the end. Cost
comparison would always be to a simple pointer-set or bitmap.
But sure, I'll stick it to cfg.h for the moment. As said, my
main use case didn't materialize on trunk yet but is in a patchset
I have to bring up-to-date.
> > Thanks,
> > Richard.
> [...snip...]
>
> The new classes are missing leading comments. I think it's worth
> noting that the auto_flag (and thus their subclasses) hold a pointer
> into a control_flow_graph instance, but they don't interact with the
> garbage collector, so there's an implicit assumption that the auto_flag
> instances are short-lived and that the underlying storage is kept alive
> some other way (e.g. as cfun is kept alive by cfun being a GC root).
Ah, yes - missed a comment.
>
> > +class auto_edge_flag : public auto_flag<int>
> > +{
> > +public:
> > + auto_edge_flag (function *fun)
> > + : auto_flag (&fun->cfg->edge_flags_allocated) {}
> > +};
> > +
> > +class auto_bb_flag : public auto_flag<int>
> > +{
> > +public:
> > + auto_bb_flag (function *fun)
> > + : auto_flag (&fun->cfg->bb_flags_allocated) {}
> > +};
> > +
> > #endif /* GCC_CFG_H */
>
> [...snip...]
>
> Hope this is constructive
Sure!
Thanks,
Richard.
