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

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?

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

Thanks,

Evan

On Jul 1, 2009, at 11:48 AM, Evan Cheng wrote:
> Sorry I am late to the thread. I've been meaning to find the time to
> respond properly.
>
> 1. Yes, the default behavior is to keep GV and function in the same
> memory buffer. The reason is the JIT can be used in a client and
> server environment. This makes it possible without doing expensive
> relocation on the fly. In fact, the currently implementation doesn't
> allow the client to do relocation since relocation is done by the JIT.
> That can be changed, of course.
> 2. There is a hook to change the behavior, i.e. allocating GV
> separately with malloc. The API is not fully flushed out so it's done
> with malloc (which is how it was done before #1).
> 3. We *can* consider changing the default. But we need to do it
> gently. I'd prefer to complete the API to support both models and
> *then* consider making the change. We do not want to disrupt existing
> clients.
> 4. Most *real* implementation should use its down custom memory
> manager. The default manager (or malloc for GVs) works for lli. But
> that's a command line tool for testing, it isn't necessarily how
> things must be done.
>
> I am glad to see the push to design and implement a proper API to
> support both models. I intend to review the code later today. Sorry
> about the delay.
>
> Evan
>
> On Jul 1, 2009, at 9:57 AM, Reid Kleckner wrote:
>
>>> We have been JITing kernels and having a memory manager for globals
>>> would be a big win there (clean up a few hacks to make things go in
>>> the correct locations).
>>
>> I'm also guessing that Dale's client at Apple is using a custom
>> memory
>> manager, since without doing that there's no way to get the size of
>> the code block in order to send it over the wire.  Adding an
>> allocateGlobal method would probably allow them to trap it and also
>> send it over the wire, but they say they don't want to make any
code
>> changes.
>>
>> I'd like to get some guidance on what's acceptable before I
send a
>> patch to llvm-commits, though.
>>
>> Reid
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

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?
> 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?

Reid

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