[ADT][NFC] Use empty base optimisation in BumpPtrAllocatorImpl

[njames93]

Summary

Most uses of this class just use the default MallocAllocator.
As this contains no fields, we can use the empty base optimisation for BumpPtrAllocatorImpl and save 8 bytes of padding for most use cases.

This prevents using a class that is marked as final as the AllocatorT template argument.
In one must use an allocator that has been marked as final, the simplest way around this is a proxy class.
The class should have all the methods that AllocaterBase expects and should forward the calls to your own allocator instance.

[dblaikie]

I think for full generality the standard library does some things to check if a type can be derived from before using it in this way - but we probably don't need all that machinery for LLVM-internal allocator stuff like this?

[njames93]

In D94439#2491075, @dblaikie wrote:

I think for full generality the standard library does some things to check if a type can be derived from before using it in this way - but we probably don't need all that machinery for LLVM-internal allocator stuff like this?

We just have to make sure people don't use an allocator that's defined with the final keyword. I doubt we are going to run into many cases of that.
I have done the template magic to get it make it work if someone wants to use an allocator marked final, should I include it?

[dblaikie]

In D94439#2491180, @njames93 wrote:

In D94439#2491075, @dblaikie wrote:

I think for full generality the standard library does some things to check if a type can be derived from before using it in this way - but we probably don't need all that machinery for LLVM-internal allocator stuff like this?

We just have to make sure people don't use an allocator that's defined with the final keyword. I doubt we are going to run into many cases of that.
I have done the template magic to get it make it work if someone wants to use an allocator marked final, should I include it?

Probably wouldn't bother - might be worth leaving it here or in the commit message, or in a comment in the code as a "here's how to do it if it turns out someone needs it".

[njames93]

In D94439#2491194, @dblaikie wrote:

In D94439#2491180, @njames93 wrote:

In D94439#2491075, @dblaikie wrote:

I think for full generality the standard library does some things to check if a type can be derived from before using it in this way - but we probably don't need all that machinery for LLVM-internal allocator stuff like this?

We just have to make sure people don't use an allocator that's defined with the final keyword. I doubt we are going to run into many cases of that.
I have done the template magic to get it make it work if someone wants to use an allocator marked final, should I include it?

Probably wouldn't bother - might be worth leaving it here or in the commit message, or in a comment in the code as a "here's how to do it if it turns out someone needs it".

To make the class work is code than I'd be happy to tack onto the commit message. I'll just make a note in the commit message about it.
Basically just create a class that has an instance of the final Allocator type, then create calls that forward to that instance.

llvm/include/llvm/Support/Allocator.h

 /// use a custom allocator.
 ///
 /// The GrowthDelay specifies after how many allocated slabs the allocator
 /// increases the size of the slabs.
 template <typename AllocatorT = MallocAllocator, size_t SlabSize = 4096,
           size_t SizeThreshold = SlabSize, size_t GrowthDelay = 128>
 class BumpPtrAllocatorImpl
     : public AllocatorBase<BumpPtrAllocatorImpl<AllocatorT, SlabSize,
-                                                SizeThreshold, GrowthDelay>> {
+                                                SizeThreshold, GrowthDelay>>,
+      private AllocatorT {
 public:
   static_assert(SizeThreshold <= SlabSize,
                 "The SizeThreshold must be at most the SlabSize to ensure "
                 "that objects larger than a slab go into their own memory "
                 "allocation.");
   static_assert(GrowthDelay > 0,
                 "GrowthDelay must be at least 1 which already increases the"
                 "slab size after each allocated slab.");
 
   BumpPtrAllocatorImpl() = default;
 
   template <typename T>
   BumpPtrAllocatorImpl(T &&Allocator)
-      : Allocator(std::forward<T &&>(Allocator)) {}
+      : AllocatorT(std::forward<T &&>(Allocator)) {}
 
   // Manually implement a move constructor as we must clear the old allocator's
   // slabs as a matter of correctness.
   BumpPtrAllocatorImpl(BumpPtrAllocatorImpl &&Old)
-      : CurPtr(Old.CurPtr), End(Old.End), Slabs(std::move(Old.Slabs)),
+      : AllocatorT(static_cast<AllocatorT &&>(Old)), CurPtr(Old.CurPtr),
+        End(Old.End), Slabs(std::move(Old.Slabs)),
         CustomSizedSlabs(std::move(Old.CustomSizedSlabs)),
-        BytesAllocated(Old.BytesAllocated), RedZoneSize(Old.RedZoneSize),
-        Allocator(std::move(Old.Allocator)) {
+        BytesAllocated(Old.BytesAllocated), RedZoneSize(Old.RedZoneSize) {
     Old.CurPtr = Old.End = nullptr;
     Old.BytesAllocated = 0;
     Old.Slabs.clear();
     Old.CustomSizedSlabs.clear();
   }
 
   ~BumpPtrAllocatorImpl() {
     DeallocateSlabs(Slabs.begin(), Slabs.end());
     DeallocateCustomSizedSlabs();
   }
 
   BumpPtrAllocatorImpl &operator=(BumpPtrAllocatorImpl &&RHS) {
     DeallocateSlabs(Slabs.begin(), Slabs.end());
     DeallocateCustomSizedSlabs();
 
     CurPtr = RHS.CurPtr;
     End = RHS.End;
     BytesAllocated = RHS.BytesAllocated;
     RedZoneSize = RHS.RedZoneSize;
     Slabs = std::move(RHS.Slabs);
     CustomSizedSlabs = std::move(RHS.CustomSizedSlabs);
-    Allocator = std::move(RHS.Allocator);
+    AllocatorT::operator=(static_cast<AllocatorT &&>(RHS));
 
     RHS.CurPtr = RHS.End = nullptr;
     RHS.BytesAllocated = 0;
     RHS.Slabs.clear();
     RHS.CustomSizedSlabs.clear();
     return *this;
   }
 
if (Adjustment + SizeToAllocate <= size_t(End - CurPtr)) {
       // Similarly, tell ASan about this space.
       __asan_unpoison_memory_region(AlignedPtr, Size);
       return AlignedPtr;
     }
 
     // If Size is really big, allocate a separate slab for it.
     size_t PaddedSize = SizeToAllocate + Alignment.value() - 1;
     if (PaddedSize > SizeThreshold) {
-      void *NewSlab = Allocator.Allocate(PaddedSize, alignof(std::max_align_t));
+      void *NewSlab =
+          AllocatorT::Allocate(PaddedSize, alignof(std::max_align_t));
       // We own the new slab and don't want anyone reading anyting other than
       // pieces returned from this method.  So poison the whole slab.
       __asan_poison_memory_region(NewSlab, PaddedSize);
       CustomSizedSlabs.push_back(std::make_pair(NewSlab, PaddedSize));
 
       uintptr_t AlignedAddr = alignAddr(NewSlab, Alignment);
       assert(AlignedAddr + Size <= (uintptr_t)NewSlab + PaddedSize);
       char *AlignedPtr = (char*)AlignedAddr;
private:
   ///
   /// Used so that we can compute how much space was wasted.
   size_t BytesAllocated = 0;
 
   /// The number of bytes to put between allocations when running under
   /// a sanitizer.
   size_t RedZoneSize = 1;
 
-  /// The allocator instance we use to get slabs of memory.
-  AllocatorT Allocator;
-
   static size_t computeSlabSize(unsigned SlabIdx) {
     // Scale the actual allocated slab size based on the number of slabs
     // allocated. Every GrowthDelay slabs allocated, we double
     // the allocated size to reduce allocation frequency, but saturate at
     // multiplying the slab size by 2^30.
     return SlabSize *
            ((size_t)1 << std::min<size_t>(30, SlabIdx / GrowthDelay));
   }
 
   /// Allocate a new slab and move the bump pointers over into the new
   /// slab, modifying CurPtr and End.
   void StartNewSlab() {
     size_t AllocatedSlabSize = computeSlabSize(Slabs.size());
 
     void *NewSlab =
-        Allocator.Allocate(AllocatedSlabSize, alignof(std::max_align_t));
+        AllocatorT::Allocate(AllocatedSlabSize, alignof(std::max_align_t));
     // We own the new slab and don't want anyone reading anything other than
     // pieces returned from this method.  So poison the whole slab.
     __asan_poison_memory_region(NewSlab, AllocatedSlabSize);
 
     Slabs.push_back(NewSlab);
     CurPtr = (char *)(NewSlab);
     End = ((char *)NewSlab) + AllocatedSlabSize;
   }
 
   /// Deallocate a sequence of slabs.
   void DeallocateSlabs(SmallVectorImpl<void *>::iterator I,
                        SmallVectorImpl<void *>::iterator E) {
     for (; I != E; ++I) {
       size_t AllocatedSlabSize =
           computeSlabSize(std::distance(Slabs.begin(), I));
-      Allocator.Deallocate(*I, AllocatedSlabSize, alignof(std::max_align_t));
+      AllocatorT::Deallocate(*I, AllocatedSlabSize, alignof(std::max_align_t));
     }
   }
 
   /// Deallocate all memory for custom sized slabs.
   void DeallocateCustomSizedSlabs() {
     for (auto &PtrAndSize : CustomSizedSlabs) {
       void *Ptr = PtrAndSize.first;
       size_t Size = PtrAndSize.second;
-      Allocator.Deallocate(Ptr, Size, alignof(std::max_align_t));
+      AllocatorT::Deallocate(Ptr, Size, alignof(std::max_align_t));
     }
   }
 
   template <typename T> friend class SpecificBumpPtrAllocator;
 };
 
 /// The standard BumpPtrAllocator which just uses the default template
 /// parameters.