[PATCH] Introduce needsCleanup() for APFloat and APInt

[Shuxin Yang]

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/0e346bf5/attachment.html>