[PATCH] tidy the MCJIT tests now applyPermissions will invalidate cache

[David Tweed]

Hi,

 

Thanks for the review.

 

| Wouldn't it be better if the tests did the "right thing" and called
TheJIT->finalizeObject()?

 

I think you're probably right. It would also be good if the finalizeObject()
declaration had a comment mentioning that's what it's intended use is.
AFAICS the only comment is the oblique reference in SectionMemoryManager.h.

 

|I'm not sure how much I like SectionMemoryManager::applyPermissions() being
specified to also mean flushing cache, but I don't have a strong opinion on
that.

 

I think that it may be that applyPermissions might not be quite the right
name, but my basic reasoning is that it does do the cache invalidation
inside the body, so there should be commentary documenting this fact
(particularly since it's allowed for alternative implementations of the
memory manager to be used). As you can probably tell, in addition to the
professional interest in having MCJIT work well on ARM, I'm interested in
having LLVM's JITing facilities be documented enough to be usable by "casual
users" without them needing to become JIT implementation experts.

 

I'll wait a bit for other comments, then respin a patch.

 

Cheers,

Dave

 

-Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130513/b0508b49/attachment.html>