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?
> 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).
> +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
Dave
