<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=utf-8">
<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-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@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">Hi Lang,<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’m heading out for vacation this afternoon and won’t be back until June 26, so I won’t be able to review these changes in detail but the direction as you’ve
described in the e-mail below sounds very good to me.<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">Thanks,<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>
<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"><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""> Lang Hames [mailto:lhames@gmail.com]
<br>
<b>Sent:</b> Thursday, June 12, 2014 1:35 PM<br>
<b>To:</b> Commit Messages and Patches for LLVM; Kaylor, Andrew; Rafael Ávila de Espíndola<br>
<b>Subject:</b> [PATCH] Substantial RuntimeDyldMachO cleanup.<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">Hi All,<o:p></o:p></p>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Attached is a patch that I think substantially cleans up RuntimeDyldMachO and, if the general direction is agreed with, could be used as a template for cleaning up RuntimeDyldELF.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">At the moment all MCJIT relocation handling logic for MachO is contained in the RuntimeDyldMachO class (see lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.{h,cpp}). Mixing the logic for all different MachO targets in a single class creates
two problems:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">(1) Poor readability/maintainability: Any change to the relocation logic for a particular target has to be inspected carefully to make sure it won't alter the behavior for any other target. Some of my recent patches for X86 have exacerbated
this problem, so I'm very happy to be able to offer this patch as penance.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">(2) Poor performance: we're querying the target architecture in multiple places for every relocation, despite it being a known quantity as soon as we load an object file.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">To fix these issues, this patch splits RuntimeDyldMachO's functionality across the following class hierarchy:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">RuntimeDyldMachO</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Implemented in RuntimeDyldMachO.{h,cpp}<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Contains logic that is completely independent of the target. This consists mostly of MachO helper utilities which the derived classes use to get their work done.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">t<span style="font-family:"Courier New"">emplate <typename Impl></span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">RuntimeDyldMachOCRTPBase<Impl> : public RuntimeDyldMachO</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Implemented in RuntimeDyldMachO.h<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Contains generic MachO algorithms/data structures that defer to the Impl class for target-specific behaviors. For the curious, and as the name suggests, this acts as the base of a CRTP[1] instance.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">RuntimeDyldMachOARM : public RuntimeDyldMachOCRTPBase<RuntimeDyldMachOARM></span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">RuntimeDyldMachOARM64 : public RuntimeDyldMachOCRTPBase<RuntimeDyldMachOARM64></span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">RuntimeDyldMachOI386 : public RuntimeDyldMachOCRTPBase<RuntimeDyldMachOI386></span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">RuntimeDyldMachOX86_64 : public RuntimeDyldMachOCRTPBase<RuntimeDyldMachOX86_64></span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Implemented in their respective *.h files in lib/ExecutionEngine/RuntimeDyld/MachOTargets<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Each of these contains the relocation logic specific to their target architecture.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">These target specific classes keep the target specific craziness mercifully well contained, compared to what we have today. Each target can be modified independently, without concern for breaking other targets. The 'switch (Arch) ...' anti-pattern
is gone, presumably with a small performance benefit. There's still lots of work to do, but I think this is a solid first step.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Please let me know what you think.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Cheers,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Lang.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">[1] <a href="http://en.wikipedia.org/wiki/Curiously_recurring_template_pattern">http://en.wikipedia.org/wiki/Curiously_recurring_template_pattern</a><o:p></o:p></p>
</div>
</div>
</div>
</body>
</html>