<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr">On Sat, Dec 22, 2012 at 2:21 AM, Vane, Edwin <span dir="ltr"><<a href="mailto:edwin.vane@intel.com" target="_blank">edwin.vane@intel.com</a>></span> wrote:<br>
<div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I’m not expecting any support and I just want to make progress at this point so I’ll make the changes and update the design doc to not mention about llvm::Pass-like
registration.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">Since not much of this patch is likely to remain, the first patch may just get abandoned in favour of something more like the in-progress patch I provided.</span></p>
</div></div></blockquote><div><br></div><div style>Thanks. Looking forward to reviewing it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple">
<div><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></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""> Manuel Klimek [mailto:<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>]
<br>
<b>Sent:</b> Friday, December 21, 2012 3:58 PM</span></p><div><div class="h5"><br>
<b>To:</b> Vane, Edwin<br>
<b>Cc:</b> <a href="mailto:reviews%2BD221%2Bpublic%2B4c9537db733ade53@llvm-reviews.chandlerc.com" target="_blank">reviews+D221+public+4c9537db733ade53@llvm-reviews.chandlerc.com</a>; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [cfe-commits] [PATCH] Foundation for Transform registration infrastructure<u></u><u></u></div></div><p></p><div><div class="h5">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">Vane, I looked at the in-progress change - I like where this is going, but I still think that having 1 place in main where you have to add the new transform leads to significantly
simpler to follow code (no magic!).<u></u><u></u></span></p>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">If you find more people who would find the completely decoupling better, I'm still happy to be outvoted.<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">Cheers,<u></u><u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">/Manuel<u></u><u></u></span></p>
</div>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""><u></u> <u></u></span></p>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">On Fri, Dec 21, 2012 at 7:11 PM, Vane, Edwin <<a href="mailto:edwin.vane@intel.com" target="_blank">edwin.vane@intel.com</a>> wrote:<u></u><u></u></span></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">To help clear up the confusion, I’ve posted some preliminary work on phabricator:</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><a href="http://llvm-reviews.chandlerc.com/D235" target="_blank">http://llvm-reviews.chandlerc.com/D235</a></span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">This is my WIP porting loop-convert into cpp11-migrate. It still requires polish but the things to
note are the makefiles, what the Transform class looks like, and how adding a new Transform is pretty much independent from any other code with the exception of some changes to the build system. I’ll submit a proper review for this code later. The purpose
of this patch is to help get the initial patch through.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></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"">
<a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a> [mailto:<a href="mailto:cfe-commits-bounces@cs.uiuc.edu" target="_blank">cfe-commits-bounces@cs.uiuc.edu</a>]
<b>On Behalf Of </b>Vane, Edwin<br>
<b>Sent:</b> Friday, December 21, 2012 9:22 AM<br>
<b>To:</b> Manuel Klimek</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""><br>
<b>Cc:</b> <a href="mailto:reviews%2BD221%2Bpublic%2B4c9537db733ade53@llvm-reviews.chandlerc.com" target="_blank">
reviews+D221+public+4c9537db733ade53@llvm-reviews.chandlerc.com</a>; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">
cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [cfe-commits] [PATCH] Foundation for Transform registration infrastructure<u></u><u></u></span></p>
</div>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">There’s some confusion I’ll try to clear up here. I don’t want to use Tablegen. It was just something
Sean said about TableGenBackends.h where I thought he was suggesting I use TableGen. After looking into the header I’m pretty sure he didn’t actually mean that.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">I really do want a simple design with subclasses of Transform implementing specific transformations
and those subclasses being instantiated by main based on what args are on the command-line. However, to satisfy the design requirement of making transforms easy to add and not having to change any central code every time a transform is added, I wanted to use
something like LLVM’s Pass mechanism for registration so the the pairing between command-line arg and class to instantiate is contained within transform code. This way a developer only has to add code to the tool and not have to change anything in the existing
code when adding a new transform.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The registration mechanism as it exists on phabricator now works for this purpose except that all
sources need to be linked into the tool directly and not via static libs or else the global registration variables don’t get included. I have a solution in cmake which is very straightforward but the legacy Makefile system is giving me issues. I’m debugging
it now.</span><u></u><u></u></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"> </span><u></u><u></u></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""> Manuel Klimek
<a href="mailto:[mailto:klimek@google.com]" target="_blank">[mailto:klimek@google.com]</a>
<br>
<b>Sent:</b> Thursday, December 20, 2012 11:06 AM<br>
<b>To:</b> Vane, Edwin<br>
<b>Cc:</b> <a href="mailto:reviews+D221+public+4c9537db733ade53@llvm-reviews.chandlerc.com" target="_blank">
reviews+D221+public+4c9537db733ade53@llvm-reviews.chandlerc.com</a>; <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">
cfe-commits@cs.uiuc.edu</a><br>
<b>Subject:</b> Re: [cfe-commits] [PATCH] Foundation for Transform registration infrastructure</span><u></u><u></u></p>
<p class="MsoNormal"> <u></u><u></u></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">On Thu, Dec 20, 2012 at 3:39 PM, Vane, Edwin <<a href="mailto:edwin.vane@intel.com" target="_blank">edwin.vane@intel.com</a>>
wrote:</span><u></u><u></u></p>
<div>
<div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">I wasn't planning on dynamically loading transforms. Yesterday when I tried turning a transform into a static library
to link into cpp11-migrate I was reminded that unused global variables like the RegisterTransform<...> vars won't get included in the link. My next goal is just to set up the build system to slurp through subdirectories adding all sources to the executable
as one monolithic entity. I'm not sure how this is any less complex than using tablegen although I admit I don't have much experience with it.</span><u></u><u></u></p>
</blockquote>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">Why tablegen? I'm still not sure why we need anything but simple classes that can be instantiated...</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">Can you give examples of what you want to do that this wouldn't fulfill?</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""> </span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">Thanks,</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">/Manuel</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""> </span><u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt">
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""><br>
-----Original Message-----<br>
From: Manuel Klimek [mailto:<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>]</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">Sent: Thursday, December 20, 2012 2:43 AM<br>
To: <a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>; Vane, Edwin</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">Cc:
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>;
<a href="mailto:gribozavr@gmail.com" target="_blank">gribozavr@gmail.com</a>; <a href="mailto:silvas@purdue.edu" target="_blank">
silvas@purdue.edu</a><br>
Subject: Re: [PATCH] Foundation for Transform registration infrastructure</span><u></u><u></u></p>
</div>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""> Sean's main point is my primary question: why do we need to load stuff dynamically?<br>
<br>
I would expect to have a Transform class at some point, and have main basically look like:<br>
if (Flag1)<br>
Transforms.push_back(new T1);<br>
...<br>
and then do something with the vector of transforms we've built - that wouldn't need any plugin infrastructure.<br>
<br>
PS: regarding my rants - many people in llvm disagree with my positions here :) There's a lot of taste and personal preference to them, so if you agree with my reasoning, cool, but always feel free to argue if you don't agree :) I'm happy to learn new points
of view<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D221" target="_blank">http://llvm-reviews.chandlerc.com/D221</a></span><u></u><u></u></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif"">_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></span><u></u><u></u></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""> </span><u></u><u></u></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Arial","sans-serif""><u></u> <u></u></span></p>
</div>
</div>
</div></div></div>
</div>
</blockquote></div><br></div></div></div>