<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 15, 2014 at 3:07 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">On 15 September 2014 15:02, Sean Silva <span dir="ltr"><<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>Wow, this is a really detailed document. Great work!</div><div><br></div><div>I wouldn't typically recommend a document to go into this much detail, but I think that in this particular case, it is fine to have this detail since the document can double as a "in-depth walkthrough of a specific LLVM pass", which I'm sure will be useful for newbies to get a feel for things.</div></div></blockquote><div><br></div></span><div>Actually, I have questions on this point before I get into reviewing the contents. This is the first piece of pass documentation. Who is the intended audience? What is the desired level of detail and why?</div></div></div></div></blockquote><div><br></div><div>Hopefully this should get answered once Stepan an updates to answer the three questions: <a href="http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines">http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines</a></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> At what point should implementation details be found by reading the code instead of being in the documentation? Or is this supposed to be a higher-level understanding of the algorithm like an academic paper but without the tone (or impenetrable writing)? What is the burden for updating this document as the implementation changes and why is that a good tradeoff?</div></div></div></div></blockquote><div><br></div><div>I really don't have a good answer to this. I sort of lean towards the "informal paper" interpretation. My gut right now is that this would be worth having as a hold-your-hand walkthrough for newbies, and would continue to be so even if details of the code changed underneath it. But I really don't have a good way to weight that against the downsides, like the ongoing maintenance commitment, if any. Any ideas are welcome.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><font color="#888888"><div><br></div><div>Nick</div></font></span><div><div class="h5"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>In your first section please answer the three questions here: <a href="http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines" target="_blank">http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines</a><br></div><div><br></div><div>I don't know that much about the pass (especially the new implementation), so Nick, could you skim over the content to make sure it is covering all the main bases? </div><div><br></div><div>Some random comments:</div><div><br></div><div>> Sometimes code contains functions that does exactly the same thing even though</div><div>> they are non-equal on the binary level.</div><div><br></div><div>This confuses me; do you mean non-equal on the source level, but equal on the binary level?</div><div><br></div><div><div>> If we will track every numbers and flags to be compared we would be able to get</div><div>> numbers chain and then create the hash number. So, once again, *total-ordering*</div><div>> could be considered as a milestone for even faster (in theory) random-access</div><div>> approach.</div></div><div><br></div><div>I'm not sure this makes sense. I imagine that part of the benefit of the comparison-based approach is that the comparisons can return early once they find a difference. Hashing always has to look at everything. Does the current comparison routine look at the entire function before actually doing any comparisons?</div><div><br></div><div><div>> #. For two trees *T1* and *T2* we perform *depth-first-trace* and have two</div><div>> chains as a product: "*T1Items*" and "*T2Items*".</div></div><div><br></div><div>I think most readers would be more comfortable with the terms "depth-first-traversal" instead of "depth-first-trace" and "sequences" instead of "chains".</div><div><br></div><div>> Consider modification of *cmpType* method.</div><div><br></div><div>What does this paragraph mean?</div><span><font color="#888888"><div><br></div><div>-- Sean Silva</div><div><br></div><div><br></div></font></span></div><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Sep 14, 2014 at 11:02 PM, <span dir="ltr"><<a href="mailto:llvm@dyatkovskiy.com" target="_blank">llvm@dyatkovskiy.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">ping<br>
<br>
11.09.2014, 12:50, "Stepan Dyatkovskiy" <<a href="mailto:stpworld@narod.ru" target="_blank">stpworld@narod.ru</a>>:<br>
<div><div>> Reattached as patch.<br>
><br>
> Stepan Dyatkovskiy wrote:<br>
>> Hello everyone,<br>
>> Please review the MergeFunctions pass documentation in attachment. Hope<br>
>> doc is clear enough :-)<br>
>><br>
>> - Stepan<br>
</div></div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
</div></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>