<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><br></div><div>In your first section please answer the three questions here: <a href="http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines">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><div><br></div><div>-- Sean Silva</div><div><br></div><div><br></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:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ping<br>
<br>
11.09.2014, 12:50, "Stepan Dyatkovskiy" <<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a>>:<br>
<div><div class="h5">> 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">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>