<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 12 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@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:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;}
@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""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<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""> Chandler Carruth [mailto:chandlerc@google.com]
<br>
<b>Sent:</b> Thursday, October 04, 2012 5:00 PM<br>
<b>To:</b> Villmow, Micah<br>
<b>Cc:</b> Kim Gräsman; Evan Cheng; llvm-commits@cs.uiuc.edu LLVM; Nadav Rotem; cfe-commits@cs.uiuc.edu cfe<br>
<b>Subject:</b> Re: [cfe-commits] [llvm-commits] [Patch] Move TargetData from Target to Support/VMCore<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">On Thu, Oct 4, 2012 at 4:48 PM, Villmow, Micah <<a href="mailto:Micah.Villmow@amd.com" target="_blank">Micah.Villmow@amd.com</a>> wrote:<o:p></o:p></p>
<div>
<div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Here is my first run at implementing TargetData with DataLayout.</span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""> </span><o:p></o:p></p>
<p class="MsoNormal" style="mso-margin-top-alt:auto;mso-margin-bottom-alt:auto"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">This should allow all the clients to start switching over to datalayout without any functionality breaking.</span><o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">There are comments in TargetData that may need updating.<o:p></o:p></p>
<p class="MsoNormal"><b><i><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">[Villmow, Micah] Ok, will do.</span></i></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">You've made the destructor virtual. I don't think you should need this. The code responsible for creating and destroying these will need to be the last migrated (which is fine) and that code should use TargetData consistently until every
client is updated to use DataLayout. Then it can switch, and the virtual destructor won't matter.<o:p></o:p></p>
<p class="MsoNormal"><b><i><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">[Villmow, Micah] Ok, I was only making it this way temporarily.</span></i></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Some comments:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">+ DataLayout(*reinterpret_cast<const DataLayout*>(&TD))</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">No, you should just call DataLayout(TD). Derived-to-base conversion will handle the rest.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><br>
</span>With those changes, if everything builds and tests pass, LGTM.<o:p></o:p></p>
<p class="MsoNormal"><b><i><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">[Villmow, Micah] Will have to wait till tomorrow to get the testing, having issues with windows testing now and this isn't a non-functional change.</span></i></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</div>
</body>
</html>