Re: [PATCH] irange_pool class

[Aldy Hernandez <aldyh@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1407 bytes --]

On 9/18/20 6:42 PM, Andrew MacLeod wrote:
> On 9/18/20 8:28 AM, David Malcolm wrote:I think of a "pool allocator" as 
> something that makes a small
>>>> number of
>>>> large allocation under the covers, and then uses that to serve
>>>> large
>>>> numbers of fixed sized small allocations and deallocations with
>>>> O(1)
>>>> using a free list.
>>> Ah, I didn't know pool had a different meaning.
>> See e.g. gcc/alloc-pool.h
> 
> The name originated when the original v1 version was based on using 
> alloc-pool.h.  when we went to varying sizes, we switched to and obstack 
> implementation  and never changed the name.
>   <...>
> 
>>>> I think it would be clearer to name this "irange_obstack", or
>>>> somesuch.
>>> I'd prefer something more generic.  We don't want to tie the name of
>>> the
>>> allocator to the underlying implementation.  What if we later change
>>> to
>>> malloc?  We'd have to change the name to irange_malloc.
>>> irange_allocator?  Or is there something more generically appropriate
>>> here?
>> How about "irange_bump_allocator?"   Rather long, but it expresses the
> 
> 
> 
> "irange_allocator" is sufficient .      The consumer should not care 
> what the implementation is, and we may decide to implement it 
> differently down the road. So I don't want to imply something specific 
> in the name or we'd have to change it again.

Updated patch attached.

Aldy

[-- Attachment #2: curr.patch --]
[-- Type: text/x-patch, Size: 3026 bytes --]

commit 343463f8887ab510503d9e230268963a299a84ef
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Sep 11 10:15:12 2020 +0200

    irange_allocator class
    
    This is the irange storage class.  It is used to allocate the
    minimum amount of storage needed for a given irange.  Storage is
    automatically freed at destruction of the storage class.
    
    It is meant for long term storage, as opposed to int_range_max
    which is meant for intermediate temporary results on the stack.
    
    The general gist is:
    
            irange_allocator alloc;
    
            // Allocate an irange of 5 sub-ranges.
            irange *p = alloc.allocate (5);
    
            // Allocate an irange of 3 sub-ranges.
            irange *q = alloc.allocate (3);
    
            // Allocate an irange with as many sub-ranges as are currently
            // used in "some_other_range".
            irange *r = alloc.allocate (some_other_range);

diff --git a/gcc/value-range.h b/gcc/value-range.h
index 8497791c7b3..c875e713d65 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -43,6 +43,7 @@ enum value_range_kind
 
 class irange
 {
+  friend class irange_allocator;
 public:
   // In-place setters.
   void set (tree, tree, value_range_kind = VR_RANGE);
@@ -619,4 +620,68 @@ vrp_val_min (const_tree type)
   return NULL_TREE;
 }
 
+// This is the irange storage class.  It is used to allocate the
+// minimum amount of storage needed for a given irange.  Storage is
+// automatically freed at destruction of the storage class.
+//
+// It is meant for long term storage, as opposed to int_range_max
+// which is meant for intermediate temporary results on the stack.
+//
+// The newly allocated irange is initialized to the empty set
+// (undefined_p() is true).
+
+class irange_allocator
+{
+public:
+  irange_allocator ();
+  ~irange_allocator ();
+  // Return a new range with NUM_PAIRS.
+  irange *allocate (unsigned num_pairs);
+  // Return a copy of SRC with the minimum amount of sub-ranges needed
+  // to represent it.
+  irange *allocate (const irange &src);
+private:
+  DISABLE_COPY_AND_ASSIGN (irange_allocator);
+  struct obstack m_obstack;
+};
+
+inline
+irange_allocator::irange_allocator ()
+{
+  obstack_init (&m_obstack);
+}
+
+inline
+irange_allocator::~irange_allocator ()
+{
+  obstack_free (&m_obstack, NULL);
+}
+
+// Return a new range with NUM_PAIRS.
+
+inline irange *
+irange_allocator::allocate (unsigned num_pairs)
+{
+  // Never allocate 0 pairs.
+  // Don't allocate 1 either, or we get legacy value_range's.
+  if (num_pairs < 2)
+    num_pairs = 2;
+
+  struct newir {
+    irange range;
+    tree mem[1];
+  };
+  size_t nbytes = (sizeof (newir) + sizeof (tree) * 2 * (num_pairs - 1));
+  struct newir *r = (newir *) obstack_alloc (&m_obstack, nbytes);
+  return new (r) irange (r->mem, num_pairs);
+}
+
+inline irange *
+irange_allocator::allocate (const irange &src)
+{
+  irange *r = allocate (src.num_pairs ());
+  *r = src;
+  return r;
+}
+
 #endif // GCC_VALUE_RANGE_H