llvm dev - Jul 2009 - [LLVMdev] JIT allocates global data in function body memory

On Jul 2, 2009, at 5:20 PM, Reid Kleckner wrote:
> On Thu, Jul 2, 2009 at 3:09 PM, Evan Cheng<evan.cheng at apple.com>  
> wrote:
>> The patch looks good to me. But we cannot allow AllocateGVsWithCode  
>> to
>> be initialized to be false yet. Can you add a mechanism to define the
>> behavior when the JIT is created / initialized?
>
> That makes four optional arguments to ExecutionEngine::create.  Do you
> mind if I go ahead and add an EngineBuilder?
Please, thanks.
>
>> Also, I am not crazy about this being moved to JITMemoryManager.h.
>> This seems like implementation detail that should be kept hidden.
>>
>> +  // If the PoisonMemory parameter is true, freed memory should be
>> overwritten
>> +  // with garbage.  This parameter is useful for testing and  
>> debugging.
>> +#ifdef NDEBUG
>> +#define POISON true
>> +#else
>> +#define POISON false
>> +#endif
>
> I wasn't crazy about it either, but it was hard to construct a
> unittest that fails properly under release mode without it.  It also
> seems like it would be a useful feature for writing unit tests for
> other custom memory managers.  I can either axe it and let the test
> only do proper testing in debug mode, or I could add a
> setPoisonMemory(bool poison) method to the JITMemoryManager interface.
> Which would you prefer?
My preference would be to add a method to JITMemoryManager.

Thanks,

Evan
>
> Reid
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

>> That makes four optional arguments to ExecutionEngine::create.  Do you
>> mind if I go ahead and add an EngineBuilder?
>
> Please, thanks.
This turned out to be somewhat involved and should probably be a
separate patch.  I started it, so I'll send it along later.  The patch
I'd like to submit now is attached and linked.
http://codereview.appspot.com/90053/show

Reid
-------------- next part --------------
A non-text attachment was scrubbed...
Name: issue90053_2001.diff
Type: text/x-diff
Size: 26719 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20090706/6c790172/attachment.diff>

Looks fine to me. Thanks. Can you commit it?

Evan

On Jul 6, 2009, at 4:28 PM, Reid Kleckner wrote:
>>> That makes four optional arguments to ExecutionEngine::create.  Do
>>> you
>>> mind if I go ahead and add an EngineBuilder?
>>
>> Please, thanks.
>
> This turned out to be somewhat involved and should probably be a
> separate patch.  I started it, so I'll send it along later.  The patch
> I'd like to submit now is attached and linked.
> http://codereview.appspot.com/90053/show
>
> Reid
> <issue90053_2001.diff>_______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev