<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Thanks, Philip. Please see below.<div><br><div><div>On Jun 10, 2014, at 10:17 AM, Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
Before anything else, the naming and documentation of this pass need
improved. I have no idea what "IM" stands for and the file level
documentation doesn't say. I finally got down to the pass
registration and found "Instruction Merger". This is far too vague
to be useful. <br></div></blockquote>I’m open for good suggestions, otherwise go with instruction merger. When you know what it is it becomes less vague. This is similar to GVN etc.<br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
For code comments, please put this on phabricator. I see a number
of small issues which need addressed. </div></blockquote><div><br></div><div>Alright, that sounds cool. Here is my first attempt at it: </div><div><a href="http://reviews.llvm.org/D4096">http://reviews.llvm.org/D4096</a></div><div><br></div><div><br></div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000"> <br>
<br>
This code seems like it duplicates lots of legality tests from other
places in the code base. I don't see obvious ommisions, but the
code duplication is questionable. Could you refactor out a single
shared copy?<br></div></blockquote><div><br></div>I know what you mean, but could not find good abstractions for a few consistency checks and decided to use the code as is. But if you have a specific proposal I’m happy to revisit.<br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
More generally, there's a lot of shared logic which other passes
which perform code hoisting and sinking (LICM for one). Why does
this need to be it's own pass rather than an extension to something
existing?<br></div></blockquote>It is a separate pass because there is no obvious way to hook it into an existing one. I tried a kind of piggyback in GVN, but agreed with Chandler that a separate pass is more appropriate. While there is a little duplication in logic it keeps passes independent and simple. And based on Chandler's claims below it should also have the nice side effect of making the review a snap for him ;-)</div><div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
Philip<br>
<br>
<div class="moz-cite-prefix">On 06/09/2014 08:42 PM, Gerolf
Hoflehner wrote:<br>
</div>
<blockquote cite="mid:89D865B3-4E4C-48D3-8B37-43E3DC77A9CB@apple.com" type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
Alright, time has come to move on with this one. The instruction
merge code is now a pass eagerly awaiting expert review eyes.
<div><br>
</div>
<div>Cheers</div>
<div>Gerolf</div>
<div><br>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<div><br>
<div>
<div>On May 2, 2014, at 5:13 PM, Chandler Carruth <<a moz-do-not-send="true" href="mailto:chandlerc@google.com">chandlerc@google.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, May 2, 2014 at 4:44 PM,
Gerolf Hoflehner <span dir="ltr"><<a moz-do-not-send="true" href="mailto:ghoflehner@apple.com" target="_blank">ghoflehner@apple.com</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">I
can see the arguments pro independent pass, but why
do you assert it will make the code much easier to
review? The code is coherent as it is and won’t
change whether it is part of GVN or its own phase. </blockquote>
</div>
<br>
Because then I don't have to even think about GVN?
Anyways, it should be *much* easier to test.</div>
</div>
</blockquote>
</div>
<br>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
</div>
</blockquote></div><br></div></body></html>