Re: [PATCH][RFC] Add dynamic edge/bb flag allocation

[David Malcolm <dmalcolm at redhat dot com>]

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:

class auto_edge_flag
{
public:
   auto_edge_flag (function *fun)
   : m_flag (alloc_edge_flag (fun)), m_fun (fun)
   {}

   ~auto_edge_flag ()
   {
      free_edge_flag (m_fun, m_flag);
   }

   operator int () const { return m_flag; }

private:
  int m_flag;
  function *m_fun;
};


> Templating the core flag allocator comes to my mind (up to max
> HOST_WIDE_INT) as well as moving the implementation elsewhere
> (hwi.h?).

Maybe wrap the data type up in a class?  Passing around an "int *"
seemed a bit low-level to me.  (But maybe that's me overthinking it)

[...snip...]

> diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c
> index 8af793c6015..64ad42c83ca 100644
> --- a/gcc/cfgloop.c
> +++ b/gcc/cfgloop.c
> @@ -1539,6 +1539,7 @@ verify_loop_structure (void)
>    /* Check irreducible loops.  */
>    if (loops_state_satisfies_p
> (LOOPS_HAVE_MARKED_IRREDUCIBLE_REGIONS))
>      {
> +      int saved_irr_mask = alloc_edge_flag (cfun);

         auto_edge_flag saved_irr_mask (cfun);

[...snip...]


Dave