[llvm-commits] [llvm] r99883 - in /llvm/trunk: include/llvm/CodeGen/LiveInterval.h include/llvm/Support/Allocator.h lib/CodeGen/LiveInterval.cpp lib/CodeGen/LiveIntervalAnalysis.cpp lib/Support/Allocator.cpp

[Jakob Stoklund Olesen]

On Mar 30, 2010, at 4:17 AM, Torok Edwin wrote:

> Author: edwin
> Date: Tue Mar 30 06:17:48 2010
> New Revision: 99883
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=99883&view=rev
> Log:
> Reapply r99881 with some fixes: only call destructor in releaseMemory!
> 
> Modified:
>    llvm/trunk/include/llvm/CodeGen/LiveInterval.h
>    llvm/trunk/include/llvm/Support/Allocator.h
>    llvm/trunk/lib/CodeGen/LiveInterval.cpp
>    llvm/trunk/lib/CodeGen/LiveIntervalAnalysis.cpp
>    llvm/trunk/lib/Support/Allocator.cpp

Hi Török,

It is OK to defer destruction of VNInfos, and I think it is a good idea with a BumpPtrAllocator that can destroy all elements, but the current implementation seems unsafe. It only works correctly if everything allocated is of the same type and size.

Given the operator new() at the bottom of Allocator.h, that seems like a risky assertion.

A template class that is only capable of allocating one type would be safer.

/jakob