On 9/17/20 4:36 AM, Aldy Hernandez via Gcc-patches wrote:
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.
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_pool pool;
// Allocate an irange of 5 sub-ranges.
irange *p = pool.allocate (5);
// Allocate an irange of 3 sub-ranges.
irange *q = pool.allocate (3);
// Allocate an irange with as many sub-ranges as are currently
// used in "some_other_range".
irange *r = pool.allocate (some_other_range);
I used this as an opportunity to learn more about the irange classes,
so I have more to say than this little change alone might justify.
OK?
Aldy
---
gcc/value-range.h | 63 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/gcc/value-range.h b/gcc/value-range.h
index 8497791c7b3..88cb3075bf0 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -43,6 +43,7 @@ enum value_range_kind
class irange
{
+ friend class irange_pool;
public:
// In-place setters.
void set (tree, tree, value_range_kind = VR_RANGE);
@@ -619,4 +620,66 @@ 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.
+//
+// It is meant for long term storage, as opposed to int_range_max
+// which is meant for intermediate temporary results on the stack.
+
+class irange_pool
+{
+public:
+ irange_pool ();
+ ~irange_pool ();
+ // 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:
+ struct obstack irange_obstack;
+};
Since the class owns a resource, its copy ctor and assignment should
either transfer it or copy it between instances, or be disabled if
copying isn't intended.
I don't know much about the obstack APIs but if it's anything like
malloc/free or new/delete I'm guessing the class isn't meant to be
copied or assigned. If that's correct, I would suggest to make it
an error by making its copy ctor and copy assignment operator
private or deleted, e.g., by making use of DISABLE_COPY_AND_ASSIGN.
Otherwise, if the implicitly provided copy ctor and assignment work
correctly, I'd suggest to add a comment to the class making it clear
that copying and assignment are in fact safe.
+
+inline
+irange_pool::irange_pool ()
+{
+ obstack_init (&irange_obstack);
+}
+
+inline
+irange_pool::~irange_pool ()
+{
+ obstack_free (&irange_obstack, NULL);
+}
+
+// Return a new range with NUM_PAIRS.
+
+inline irange *
+irange_pool::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];
+ };
+ struct newir *r
+ = (struct newir *) obstack_alloc (&irange_obstack,
+ sizeof (struct newir)
+ + sizeof (tree) * 2 * (num_pairs - 1));
+ return new ((irange *) r) irange (&(r->mem[0]), num_pairs);
FWIW, it took me a minute before I understood what this dense code
does. Rewriting it like this helped:
size_t nbytes
= (sizeof (newir) + sizeof (tree) * 2 * (num_pairs - 1));
struct newir *r
= (newir *) obstack_alloc (&irange_obstack, nbytes);
return new (r) irange (r->mem, num_pairs);
The non-member placement new takes a void* argument so the cast from
r's type to irange* isn't necessary. &(r->mem[0]) is the same as
r->mem which also reads a little clearer to me. In C++, the class-
id ("struct" or "class") can be omitted when there's no ambiguity.
The allocated memory is passed to the irange ctor uninitialized but
the ctor doesn't access it, and (AFAICS) neither do any other irange
members, so that's fine. I like that the irange class is safe to
use even when the array it manages is uninitialized. In fact, it
might be a helpful comment to add to the irange and int_range ctors:
that the array of trees passed to irange doesn't have to be
initialized and the classes work correctly and treat a newly default
constructed range as empty (undefined_p() is true).
Martin
+}
+
+inline irange *
+irange_pool::allocate (const irange &src)
+{
+ irange *r = allocate (src.num_pairs ());
+ *r = src;
+ return r;
+}
+
#endif // GCC_VALUE_RANGE_H
