<html dir="ltr"><head>
<meta http-equiv="Content-Type" content="text/html; charset=Windows-1252">
<style>@font-face {
font-family: Helvetica;
}
@font-face {
font-family: Cambria Math;
}
@font-face {
font-family: Calibri;
}
@font-face {
font-family: Tahoma;
}
@page WordSection1 {margin: 56.7pt 42.5pt 56.7pt 85.05pt; }
P.MsoNormal {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
LI.MsoNormal {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
DIV.MsoNormal {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
A:link {
COLOR: blue; TEXT-DECORATION: underline
}
SPAN.MsoHyperlink {
COLOR: blue; TEXT-DECORATION: underline
}
A:visited {
COLOR: blue; TEXT-DECORATION: underline
}
SPAN.MsoHyperlinkFollowed {
COLOR: blue; TEXT-DECORATION: underline
}
P.MsoAcetate {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Tahoma","sans-serif"; FONT-SIZE: 8pt
}
LI.MsoAcetate {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Tahoma","sans-serif"; FONT-SIZE: 8pt
}
DIV.MsoAcetate {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Tahoma","sans-serif"; FONT-SIZE: 8pt
}
P.MsoListParagraph {
MARGIN: 0in 0in 0pt 0.5in; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
LI.MsoListParagraph {
MARGIN: 0in 0in 0pt 0.5in; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
DIV.MsoListParagraph {
MARGIN: 0in 0in 0pt 0.5in; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
SPAN.BalloonTextChar {
FONT-FAMILY: "Tahoma","sans-serif"
}
P.balloontext {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
LI.balloontext {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
DIV.balloontext {
MARGIN: 0in 0in 0pt; FONT-FAMILY: "Times New Roman","serif"; FONT-SIZE: 12pt
}
SPAN.balloontextchar0 {
FONT-FAMILY: "Tahoma","sans-serif"
}
SPAN.EmailStyle22 {
FONT-FAMILY: "Arial","sans-serif"; COLOR: navy
}
SPAN.EmailStyle23 {
FONT-FAMILY: "Arial","sans-serif"; COLOR: navy
}
SPAN.EmailStyle24 {
FONT-FAMILY: "Calibri","sans-serif"; COLOR: #1f497d
}
SPAN.EmailStyle25 {
FONT-FAMILY: "Arial","sans-serif"; COLOR: navy
}
SPAN.apple-converted-space {
}
SPAN.EmailStyle27 {
FONT-FAMILY: "Calibri","sans-serif"; COLOR: #1f497d
}
SPAN.EmailStyle28 {
FONT-FAMILY: "Calibri","sans-serif"; COLOR: #1f497d
}
SPAN.EmailStyle29 {
FONT-FAMILY: "Calibri","sans-serif"; COLOR: #1f497d
}
.MsoChpDefault {
FONT-SIZE: 10pt
}
DIV.WordSection1 {
}
</style>
<meta name="GENERATOR" content="MSHTML 8.00.6001.19088">
<style id="owaTempEditStyle"></style><style title="owaParaStyle"><!--P {
MARGIN-TOP: 0px; MARGIN-BOTTOM: 0px
}
--></style>
</head>
<body lang="EN-US" link="blue" vlink="blue" ocsi="x">
<div style="FONT-FAMILY: Tahoma; DIRECTION: ltr; COLOR: #000000; FONT-SIZE: x-small">
<div>Hi Andrew,</div>
<div><font face="tahoma"></font> </div>
<div><font face="tahoma">I moved includes ErrorHandling.h, DynamicLibrary.h and config.h to the top of file and still only includes linked with StatSymbols.</font></div>
<div><font face="tahoma">Attached move_getPointerToNamedFunction-05.patch moved all includes to the top of file.</font></div>
<div><font face="tahoma"></font> </div>
<div><font face="tahoma"></font> </div>
<div><font face="tahoma">Regards,</font></div>
<div><font face="tahoma">Danil</font></div>
<div><font face="tahoma"></font> </div>
<div dir="ltr"><font color="#000000" size="2" face="Tahoma"></font> </div>
<div style="DIRECTION: ltr" id="divRpF758444">
<hr tabindex="-1">
<font color="#000000" size="2" face="Tahoma"><b>From:</b> Kaylor, Andrew [andrew.kaylor@intel.com]<br>
<b>Sent:</b> Wednesday, March 28, 2012 12:28 PM<br>
<b>To:</b> Danil Malyshev; llvm-commits@cs.uiuc.edu; Jim Grosbach<br>
<b>Subject:</b> RE: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.<br>
</font><br>
</div>
<div></div>
<div>
<div class="WordSection1">
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">The patch still shows ‘include’ statements in the middle of the JITMemoryManager.cpp file. Maybe you didn’t regenerate the patch after making those changes?</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Otherwise, it looks good.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">-Andy</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<div>
<div style="BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt">From:</span></b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt"> Danil Malyshev [mailto:dmalyshev@accesssoftek.com]
<br>
<b>Sent:</b> Tuesday, March 27, 2012 6:01 PM<br>
<b>To:</b> Kaylor, Andrew; llvm-commits@cs.uiuc.edu; Jim Grosbach<br>
<b>Subject:</b> RE: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.</span></p>
</div>
</div>
<p class="MsoNormal"> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Hi Andrew,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> There are some ugly issues with ownership of the JITMemoryManager pointer. The EngineBuilder documentation says that the</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> execution engine takes ownership of the pointer upon successful create, but it seems that in the case of the legacy JIT code it</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> delegates that ownership to the JITEmitter. That was fine in the old implementation because the JIT class didn’t keep a copy of</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> the pointer. With your new implementation the situation is complicated by the fact that both the JIT and the JITEmitter have</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> copies of the pointer.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> I feel like this code needs some clean-up in the near future anyway to disentangle the memory management from the symbol</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> resolution. For now maybe it is sufficient to add comments in the JIT code to make the ownership of the JMM pointer absolutely clear.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Well, It's looks not so good as I thought before start doing these changes.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">The JCE is a JITCodeEmitter and its hasn't any members linked with JMM, so the comments in the JIT code about JMM is a ownership of the JCE looks a bit strange.
For the same reason I cannot use something like JCE->getEmitterMemoryManager() in JIT instead of keep copy of JMM pointer.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> Also, I’m confused by the placement of the StatSymbols class and related declarations. It’s showing up between</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> the comment block for the ‘getPointerToNamedFunction() implementation’ and the actual implementation. I wasn’t</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> sure if this was a cut-and-paste error or if you meant to do that because that code was related to getPointerToNamedFunction.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">The StatSymbols is a part of getPointerToNamedFunction implementation and used only for it.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">It's was in Intercept.cpp before, but the DefaultJITMemoryManager was declared in the anonymous namespace, so it's implementation should be in the same file
and Intercept.cpp cannot used for DefaultJITMemoryManager members implementation.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">> In any event, I think at least the ‘include’ statements need to be moved to the top of the file and the function comments should be kept together.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">I did it.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Please find attached the patch with these changes. I changed some comments of getPointerToNamedFunction also.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Regards,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Danil</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<div>
<div style="BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt">From:</span></b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt"> Kaylor, Andrew [<a href="mailto:andrew.kaylor@intel.com">mailto:andrew.kaylor@intel.com</a>]
<br>
<b>Sent:</b> Monday, March 26, 2012 1:17 PM<br>
<b>To:</b> Danil Malyshev; <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>; Jim Grosbach<br>
<b>Subject:</b> RE: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.</span></p>
</div>
</div>
<p class="MsoNormal"> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Hi Danil,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">There are some ugly issues with ownership of the JITMemoryManager pointer. The EngineBuilder documentation says that the execution engine takes ownership
of the pointer upon successful create, but it seems that in the case of the legacy JIT code it delegates that ownership to the JITEmitter. That was fine in the old implementation because the JIT class didn’t keep a copy of the pointer. With your new implementation
the situation is complicated by the fact that both the JIT and the JITEmitter have copies of the pointer.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">I feel like this code needs some clean-up in the near future anyway to disentangle the memory management from the symbol resolution. For now maybe it is
sufficient to add comments in the JIT code to make the ownership of the JMM pointer absolutely clear.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Also, I’m confused by the placement of the StatSymbols class and related declarations. It’s showing up between the comment block for the ‘getPointerToNamedFunction()
implementation’ and the actual implementation. I wasn’t sure if this was a cut-and-paste error or if you meant to do that because that code was related to getPointerToNamedFunction. In any event, I think at least the ‘include’ statements need to be moved
to the top of the file and the function comments should be kept together.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Otherwise, I think your changes look good, and I think they could be committed after minor cleanup for the issues described above.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">-Andy</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<div>
<div style="BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt">From:</span></b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt"> Danil Malyshev [<a href="mailto:dmalyshev@accesssoftek.com">mailto:dmalyshev@accesssoftek.com</a>]
<br>
<b>Sent:</b> Saturday, March 24, 2012 7:32 PM<br>
<b>To:</b> Kaylor, Andrew; <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>; Jim Grosbach<br>
<b>Subject:</b> RE: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.</span></p>
</div>
</div>
<p class="MsoNormal"> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Hi Andrew,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">I agree, also because the JMM can be added to the JIT only in the constructor, so if a NULL will used instead of value, it's can not be changed.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Please review attached the patch.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Regards,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Danil</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<div>
<div style="TEXT-ALIGN: center" class="MsoNormal" align="center"><span lang="RU">
<hr align="center" size="2" width="100%">
</span></div>
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt" lang="RU">From:</span></b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt" lang="RU"> Kaylor, Andrew [<a href="mailto:andrew.kaylor@intel.com">mailto:andrew.kaylor@intel.com</a>]
<br>
<b>Sent:</b> Friday, March 23, 2012 4:36 AM<br>
<b>To:</b> Danil Malyshev; <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>; Jim Grosbach<br>
<b>Subject:</b> RE: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.</span><span lang="RU"></span></p>
</div>
<p class="MsoNormal"><span lang="RU"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">Hi Danil,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">It isn’t entirely clear to me what the old expected behavior was, but the fact that you had to update the tests seems like a red flag.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">There are likely other users of this code who are creating JIT execution engines without creating a memory manager. I think it would be better to have the
JIT constructor create a default memory manager if NULL is passed in for that argument.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">The check for a null JMM before use in getPointerToNamed function is good, but the function will still likely fail in many cases if JMM is null.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt">-Andy</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Calibri','sans-serif'; COLOR: #1f497d; FONT-SIZE: 11pt"></span> </p>
<div>
<div style="BORDER-BOTTOM: medium none; BORDER-LEFT: medium none; PADDING-BOTTOM: 0in; PADDING-LEFT: 0in; PADDING-RIGHT: 0in; BORDER-TOP: #b5c4df 1pt solid; BORDER-RIGHT: medium none; PADDING-TOP: 3pt">
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt">From:</span></b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt"> Danil Malyshev [<a href="mailto:dmalyshev@accesssoftek.com">mailto:dmalyshev@accesssoftek.com</a>]
<br>
<b>Sent:</b> Thursday, March 22, 2012 3:56 PM<br>
<b>To:</b> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>; Jim Grosbach; Kaylor, Andrew<br>
<b>Subject:</b> RE: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.</span></p>
</div>
</div>
<p class="MsoNormal"> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Hello everyone,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">The move_getPointerToNamedFunction-01.patch has causing failures in the LazyLoadedJITTest.MaterializableAvailableExternallyFunctionIsntCompiled and MultiJitTest.JitPool
unit tests.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">This tests uses JIT.getPointerToNamedFunctions() for JIT with NULL memory manager, so call JMM->getPointerToNamedFunctions() raise exception.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">This patch move the implementation of getPointerToNamedFunctions from JIT to defaultJMM, so I think, the best way is change this tests to use JIT with default
memory manager.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Please review attached move_getPointerToNamedFunction-02.patch with these changes.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">For easily looking I also attached the changes.diff. It's contains only changes linked with the problem.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Tested in Ubuntu 64 and Mac OS 64.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Regards,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Danil</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<div>
<div style="TEXT-ALIGN: center" class="MsoNormal" align="center"><span lang="RU">
<hr align="center" size="2" width="100%">
</span></div>
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt" lang="RU">From:</span></b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt" lang="RU"> Danil Malyshev
<br>
<b>Sent:</b> Wednesday, March 21, 2012 10:41 PM<br>
<b>To:</b> 'Jim Grosbach'<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> RE: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.</span><span lang="RU"></span></p>
</div>
<p class="MsoNormal"><span lang="RU"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Hello,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Committed at r153205, thank you.</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Regards,</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt">Danil</span></p>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; COLOR: navy; FONT-SIZE: 10pt"></span> </p>
<div>
<div style="TEXT-ALIGN: center" class="MsoNormal" align="center"><span lang="RU">
<hr align="center" size="2" width="100%">
</span></div>
<p class="MsoNormal"><b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt" lang="RU">From:</span></b><span style="FONT-FAMILY: 'Tahoma','sans-serif'; FONT-SIZE: 10pt" lang="RU"> Jim Grosbach [<a href="mailto:grosbach@apple.com">mailto:grosbach@apple.com</a>]
<br>
<b>Sent:</b> Wednesday, March 21, 2012 1:46 AM<br>
<b>To:</b> Danil Malyshev<br>
<b>Cc:</b> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [llvm-commits] Move getPointerToNamedFunction() from JIT/MCJIT to JITMemoryManager.</span><span lang="RU"></span></p>
</div>
<p class="MsoNormal"><span lang="RU"></span> </p>
<p class="MsoNormal"><span lang="RU">LGTM. Thanks!</span></p>
<div>
<div>
<p class="MsoNormal"><span lang="RU">On Mar 20, 2012, at 11:48 AM, Danil Malyshev <<a href="mailto:dmalyshev@accesssoftek.com">dmalyshev@accesssoftek.com</a>> wrote:</span></p>
</div>
<p style="MARGIN-BOTTOM: 12pt" class="MsoNormal"><span lang="RU"></span> </p>
<div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">Hello everyone,</span><span lang="RU"></span></p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt"></span><span lang="RU"></span> </p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">Based on this discussion:</span><span class="apple-converted-space"> </span><span lang="RU"><a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120305/138477.html" target="_blank"><span style="COLOR: purple" lang="EN-US">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120305/138477.html</span></a></span></p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">Please find attached the patch for review.</span><span lang="RU"></span></p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">This patch doing following:</span><span lang="RU"></span></p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">1. Declares a virtual function getPointerToNamedFunction() in JITMemoryManager</span><span lang="RU"></span></p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">2. Moves the implementation of getPointerToNamedFunction() form JIT/MCJIT to DefaultJITMemoryManager.</span><span lang="RU"></span></p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt"></span><span lang="RU"></span> </p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt"></span><span lang="RU"></span> </p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">Regards,</span><span lang="RU"></span></p>
</div>
<div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Arial','sans-serif'; FONT-SIZE: 10pt">Danil</span><span lang="RU"></span></p>
</div>
<p class="MsoNormal"><span style="FONT-FAMILY: 'Helvetica','sans-serif'; FONT-SIZE: 13.5pt" lang="RU"><move_getPointerToNamedFunction-01.patch>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu"><span style="COLOR: purple">llvm-commits@cs.uiuc.edu</span></a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank"><span style="COLOR: purple">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</span></a></span></p>
</div>
</div>
<p class="MsoNormal"><span lang="RU"></span> </p>
</div>
</div>
</div>
</body>
</html>