[PATCH] BumpPtrAllocator: remove redundant no-slabs-allocated-yet check

[Hans Wennborg]

We already handle the no-slabs case when checking whether the current slab is large enough: if no slabs have been allocated, CurPtr and End are both 0. alignPtr(0), will still be 0, and so "if (Ptr + Size <= End)" fails.

I measured a 1.5% speed-up with "perf stat -r10 clang -fsyntax-only -w gcc.c".

The thing I'd like review for is whether "Ptr + Size <= End" is strictly correct. It seems to me that Ptr + Size would often point outside the slab object (especially now when Ptr can be 0), and that comparing that with End would be undefined behaviour. Perhaps we should really be using intptr_t here?

http://reviews.llvm.org/D4943

Files:
  include/llvm/Support/Allocator.h
  unittests/Support/AllocatorTest.cpp

Index: include/llvm/Support/Allocator.h
===================================================================
--- include/llvm/Support/Allocator.h
+++ include/llvm/Support/Allocator.h
@@ -201,9 +201,6 @@
 
   /// \brief Allocate space at the specified alignment.
   void *Allocate(size_t Size, size_t Alignment) {
-    if (!CurPtr) // Start a new slab if we haven't allocated one already.
-      StartNewSlab();
-
     // Keep track of how many bytes we've allocated.
     BytesAllocated += Size;
 
Index: unittests/Support/AllocatorTest.cpp
===================================================================
--- unittests/Support/AllocatorTest.cpp
+++ unittests/Support/AllocatorTest.cpp
@@ -112,7 +112,7 @@
   BumpPtrAllocator Alloc;
 
   Alloc.Allocate(8000, 0);
-  EXPECT_EQ(2U, Alloc.GetNumSlabs());
+  EXPECT_EQ(1U, Alloc.GetNumSlabs());
 }
 
 // Mock slab allocator that returns slabs aligned on 4096 bytes.  There is no
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4943.12587.patch
Type: text/x-patch
Size: 927 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140817/a8e1a23d/attachment.bin>