[PATCH] SectionMemoryManager is not a JITMemoryManager

[David Tweed]

|It does update unit tests. :-). MCJITTest Base is changed. 
| Are there other places that I don't know about?

Ah, I was wondering if any of the actual code in MCJITTEst.cpp now needed,
or perhaps more
likely, would be simplified by changing: it looks
like the
'static_cast<SectionMemoryManager*>(MM)->invalidateInstructionCache()'
instructions
can go now that MM is definitely a SectionMemoryManager* whose
applyPermissions() does that at
the end of the it. (Since the purpose is to test the built-in "default"
manager it seems
worthwhile to use its behaviour rather than write code that might be needed
if a "user"
memory manager were being used.)

Cheers,
Dave

-Filip

On May 7, 2013, at 12:58 AM, David Tweed <david.tweed at arm.com> wrote:

> Hi,
> 
> I haven't had time to do more than skim the patch, but one thing is that I
> notice there aren't any changes to any of the tests. I know that we've
> unfortunately got a situation where several of the MCJIT using tests just
> copy and paste whatever memory management code lli was using at the time
so
> they easily end up out-of-sync. If after this patch is applied they still
> need updating I'll have a go at it, although it might be preferable if
> someone more knowledgeable has a look... (I just about know how to what
> needs to be done one ARM, but I may well not understand the other
supported
> but less mainstream architectures.)
> 
> Will try to come up with other comments on the patch today,
> 
> Cheers,
> Dave
> 
> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu
> [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Filip Pizlo
> Sent: 04 May 2013 20:37
> To: llvm-commits at cs.uiuc.edu
> Subject: [PATCH] SectionMemoryManager is not a JITMemoryManager
> 
> This patch cleans up the relationship between RTDyldMemoryManager,
> JITMemoryManager, and EngineBuilder.
> 
> As I understand it, JITMemoryManager is meant to go away at some point,
and
> RTDyldMemoryManager is not meant to be predestined to have to support all
of
> the things that JITMemoryManager had; it may have some of the same things,
> but also maybe some new things, and probably not all of the old things.
> 
> But right now EngineBuilder doesn't know anything about
RTDyldMemoryManager
> and assumes that JITMemoryManager is the way memory is managed in both
MCJIT
> and the old JIT.  This means that if you want to pass an
RTDyldMemoryManager
> to the MCJIT via the EngineBuilder API, you have to actually create a
> JITMemoryManager and stub out a *bunch* of methods.  SectionMemoryManager
> does this, for example: SectionMemoryManager is a subtype of
> JITMemoryManager that cannot actually be used as a JITMemoryManager,
except
> passing it through EngineBuilder so that MCJIT can use it as an
> RTDyldMemoryManager. :-/
> 
> This patch attempts to fix the problem:
> 
> - EngineBuilder now knows that RTDyldMemoryManager and JITMemoryManager
are
> different but similar things.  It knows that if you set a
JITMemoryManager,
> then it can be used for the MCJIT.  It knows that an RTDyldMemoryManager
can
> also be used for the MCJIT, but cannot be used for old JIT.  It will
ensure
> that you cannot set both at the same time since that would be confusing
> (setting one overrides the other).
> 
> - SectionMemoryManager is no longer a subtype of JITMemoryManager, and no
> longer has a ton of stub unimplemented methods.
> 
> - lli and various tests are changed to know that a SectionMemoryManager is
> not a JITMemoryManager.
> 
> This is a change to the C++ API, and I actually don't know that the
policies
> on this are!  But I hope that this is something that we can change, since
it
> kills some gnarly code and concentrates whatever little ugliness remains
> into EngineBuilder (it's only ugly because it has to know that there are
two
> kinds of ways of saying that you want to manager memory yourself).
> 
> -Filip
> 
> 
> 
>