[PATCH] Introduce needsCleanup() for APFloat and APInt

[Shuxin Yang]

Just FYI,   You never know if the APFloat allocate a memory or not after 
construction, meaning
  you may start by calling APFloat with single-precision semantics 
(which place its
significant bit in place, in stead of calling allocation); later on, 
some arithmetic may extend
the precision for some reasons,  then it has to allocate some memory, and
it never reclaim the memory until destructor.

Depending on when u register the call-back function, you are still able 
to leak memory.


On 5/13/13 10:49 AM, Shuxin Yang wrote:
>
> On 5/13/13 10:37 AM, Manuel Klimek wrote:
>> On Mon, May 13, 2013 at 6:26 PM, Shuxin Yang <shuxin.llvm at gmail.com 
>> <mailto:shuxin.llvm at gmail.com>> wrote:
>>
>>
>>     On 5/13/13 10:21 AM, Manuel Klimek wrote:
>>>     +richard smith, who proposed to implement it this way
>>>
>>>     On Mon, May 13, 2013 at 6:08 PM, Shuxin Yang
>>>     <shuxin.llvm at gmail.com <mailto:shuxin.llvm at gmail.com>> wrote:
>>>
>>>         I'm afraid this change is going to open a big can of worms.
>>>         You are essentially promote
>>>         private member function to be public. Such practice should
>>>         be generally avoided even for
>>>         the local/private class which is used in small scope, let
>>>         alone these fundamental structures
>>>         which are extensively used in the compiler.
>>>
>>>         > object needs the destructor called after its memory was freed
>>>
>>>         If the block of memory containing APFloat/APInt is already
>>>         freed, how do you know
>>>         the state of these objects are still valid? Depending on the
>>>         implementation, free-operation
>>>         might clobber the memory with some special value for
>>>         debugging purpose. If it were safe
>>>         to call needsCleanup(), why not directly call the
>>>         destructor? Is needsCleanup() sufficient
>>>         and/or necessary condition for calling the destructor? How
>>>         do we keep them in sync in the
>>>         future?
>>>
>>>
>>>     APFloat/APInt is placement new'ed in the code in question, and
>>>     thus we need to call the destructors of any objects that do
>>>     memory allocation themselves. We can save those destructor calls
>>>     otherwise.
>>     I understand that if you place a non-plain-old-data in a union,
>>     you have to construct it via placement new,
>>     and explicitly destruct it.  I come across similar problem when I
>>     implement unsafe fadd/fsub optimization
>>     half year ago.
>>
>>     My questions are :
>>       - why do you need to call function xyz() before calling the
>>     destructor.
>>       - if the memory is already freed, why do you know it is safe to
>>     call syz().
>>          The object may not in valid state.
>>
>>
>> Ah, now I understand the question :)
>>
>> So, in clang, the lifetime of the placement new'ed APFloat/APInt is 
>> basically coupled to the buffer of where they are placement new'ed 
>> into, which is very different from where the objects are created. 
>> Thus, when the objects are placement-new'ed, clang registers cleanup 
>> callbacks with that structure (which basically just calls the 
>> destructor for those objects before the underlying 
>> placement-new-buffer is deallocated). Since there might be a ton of 
>> APFloat/APInt values constructed inside a C++ AST, we want to 
>> minimize the number of registered callbacks (all of them use memory 
>> and precious runtime).
> I'm sorry, I don't know the internal of clang.
> How about using a enum is keep track of the state of the buffer (if it 
> contains a valid APFloat/APInt/whatever), and so
> you just need to register one call-back like this:
>
>  my-call-back(the-buffer) {
>      switch(buffer-state)
>      case is_APFloat:
>           ((APFloat*)&buffer-state->data)::~APFloat();
>           buffer-state = invalid;
>      case is_APint:
>            ...
>      case is_plain-old-data:
>           do nothing
>  }
>
>> To that end, we call needsCleanup() in order to check whether we need 
>> to register a "destructor callback" that needs to be called before 
>> throwing away the memory. (see the clang review CL I pointed to)
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130513/394375c0/attachment.html>