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

[Kaylor, Andrew]

Looks good!

From: David Tweed [mailto:david.tweed at arm.com]
Sent: Thursday, May 16, 2013 2:57 AM
To: Kaylor, Andrew; 'Filip Pizlo'
Cc: llvm-commits at cs.uiuc.edu
Subject: RE: [PATCH v2] tidy the MCJIT tests now applyPermissions will invalidate cache

Hi,

|I forgot to comment on the name change to applyPermissions.  I don't really like updateMemoryAttributes.  What do you think about prepareMemoryForExecution?  It obscures the fact that some data sections might get marked as read-only, though that could be
|adequately covered in the documentation.  Something like finalizeMemory is very vague, but that could be a good thing since we don't really know what the memory manager might need to do.


Attached is a new patch. I'm not personally that keen on finalizeMemory() due to having had some brushes with Java/Python/NET where finalize brings to mind object desctruction, but the user visible API already has finalizeObject() so using finalize on an internal function I don't object to that much. I've XFAILED the specific multiple_function test and tested on both X86-64 and ARM. Ok to commit?

Cheers,
Dave


From: David Tweed [mailto:david.tweed at arm.com]
Sent: Wednesday, May 15, 2013 8:38 AM
To: Kaylor, Andrew; 'Filip Pizlo'
Cc: llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
Subject: RE: [PATCH v2] tidy the MCJIT tests now applyPermissions will invalidate cache

Hi, thanks of the feedback.

| I don't know if it's more than you wanted to take on, but I'd be very happy to have that fixed.  There are a few ExecutionEngine/MCJIT tests marked as XFAIL: ARM because of this.

I've opened PR 16013 to keep track of this more substantial issue. With the complexity this is clearly a separate issue to tidying the tests, so I'll rework the patch to address  comments and (for the moment) avoid that test on ARM.

Cheers,
Dave

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