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

Hans Wennborg hans at chromium.org
Sun Aug 17 11:45:52 PDT 2014


On Sat, Aug 16, 2014 at 10:56 PM, Chandler Carruth <chandlerc at google.com> wrote:
>
> On Sat, Aug 16, 2014 at 8:01 PM, Hans Wennborg <hans at chromium.org> wrote:
>>
>> 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".
>
>
> LGTM, please submit.

Thanks! r215841

>> 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?
>
> Technically, yes... but it can't possibly matter. ;]
>
> The correct fix is to compare the difference of Ptr and End to Size, not to
> use intptr_t. Feel free to submit that too if you like. =]

We've already bumped Ptr for the alignment, so it might already be
invalid. But like you said this probably doesn't matter so I won't
worry about it now.

 - Hans



More information about the llvm-commits mailing list