[PATCH] SectionMemoryManager is not a JITMemoryManager

[Kaylor, Andrew]

There definitely is some clean-up that should be done to both lli and the unit tests with regard to explicit cache invalidation, but I don't think that's related to this patch.

I had been waiting to make sure the cache invalidation change "stuck" before purging the tests of redundant calls to it.  It looks like that has happened.  Now I just need to find time to do it.

David, if you want to do the clean-up, I'll take a grateful look at it when you're done.  Otherwise, I'll probably get to that within the next week or so.

-Andy

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of David Tweed
Sent: Tuesday, May 07, 2013 2:37 AM
To: 'Filip Pizlo'
Cc: llvm-commits at cs.uiuc.edu
Subject: RE: [PATCH] SectionMemoryManager is not a JITMemoryManager

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






_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits