<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">You may be right about the naming of applyPermissions, and you are definitely right about the poor documentation.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I agree with Filip that when the MCJIT engine is being used the client should call finalizeObject rather than using applyPermissions directly.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">My thinking with the interfaces is that both the module within the MCJIT engine and the memory within the memory manager go through a series of state transitions
(which obviously need to be better documented). In the execution engine it goes something like this:<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">raw module -> code and data generated (or loaded from cache) -> object image loaded into memory -> object prepared for execution<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The third state above is necessarily vague and open as we need to allow for clients moving the memory around, and possibly copying it to an external process
or a remote system. We currently lack a well-defined method to transition from the first state to the second or the second to the third. Right now, the second and third states are coupled inside MCJIT such that we always go straight into the third state
as soon as we enter the second. Several methods will implicitly trigger this transition (for instance, getPointerToFunction). That’s not by design so much as a historical legacy. Right now, relocations are applied in both the third state and the fourth
state. Again, that’s kind of sloppy and is just an artifact of how we got things working. Ideally, I don’t think relocations should be applied until we go into the fourth state. The finalizeObject method is intended as the canonical way to get us into the
fourth and final state.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Things are a bit simpler with the memory manager side of things. The intention is that the memory manager will allocate memory as RW such that we can use it
to load the object and get it ready for execution, then when applyPermissions is called the memory manager will do whatever is necessary to get things in a state that’s ready for execution. That’s why the cache invalidation went there. The name does obscure
some of what’s happening. Something like ‘finalizeMemory’ or ‘prepareForExecution’ might be better.
<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I didn’t intend for clients to call applyPermissions directly. Clients that do so are likely to miss the relocation step in the execution engine when that
gets moved out of the code generation process.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-Andy<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Tweed [mailto:david.tweed@arm.com]
<br>
<b>Sent:</b> Monday, May 13, 2013 9:01 AM<br>
<b>To:</b> 'Filip Pizlo'<br>
<b>Cc:</b> llvm-commits@cs.uiuc.edu; Kaylor, Andrew<br>
<b>Subject:</b> RE: [PATCH] tidy the MCJIT tests now applyPermissions will invalidate cache<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D">Hi,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D">Thanks for the review.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D">| </span><span lang="EN-GB">Wouldn't it be better if the tests did the "right thing" and called TheJIT->finalizeObject()?<o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D">|</span><span lang="EN-GB">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.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-GB" style="color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">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.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I'll wait a bit for other comments, then respin a patch.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Cheers,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Dave<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-GB" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-GB">-Filip<o:p></o:p></span></p>
</div>
</div>
</body>
</html>