<div>Hi Nick,</div><div>Could you please look at pass documentation..</div><div>Thanks!</div><div>-Stepan</div><div> </div><div>01.11.2014, 03:44, "Sean Silva" <chisophugis@gmail.com>:</div><blockquote type="cite"><div>I'm okay with it. Nick?</div><div><br /><div>On Fri, Oct 31, 2014 at 1:14 PM, <span><<a href="mailto:llvm@dyatkovskiy.com" target="_blank">llvm@dyatkovskiy.com</a>></span> wrote:<br /><blockquote style="margin:0 0 0 0.8ex;border-left:1px #ccc solid;padding-left:1ex;">ping<br /> <br /> 20.10.2014, 13:36, "<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a>" <<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a>>:<br /><div><div>> Ping.<br /> > -Stepan<br /> ><br /> > 07.10.2014, 13:30, "Stepan Dyatkovskiy" <<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a>>:<br /> >>  ping<br /> >>  On 03 Oct 2014, at 11:42, Stepan Dyatkovskiy <<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a>> wrote:<br /> >>>   Hi Sean,<br /> >>>   Both issues you mentioned has been fixed. Final patch has been reattached.<br /> >>><br /> >>>   Thanks for reviews!<br /> >>>   -Stepan.<br /> >>><br /> >>>   On 03 Oct 2014, at 03:27, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>> wrote:<br /> >>><br /> >>>   On Thu, Oct 2, 2014 at 12:40 AM, Stepan Dyatkovskiy <<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a><mailto:<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a>>> wrote:<br /> >>>   Hi Sean,<br /> >>>>>   Sometimes code contains functions that does exactly the same thing even though<br /> >>>>>   they are non-equal on the binary level.<br /> >>>>   This confuses me; do you mean non-equal on the source level, but equal on the binary level?<br /> >>>   I mean equal on output. As if you treat function as a black-box with only inputs and outputs present. Functions could be different on binary level but equal on output, e.g:<br /> >>><br /> >>>   int foo_0(int a) {<br /> >>>    return a + a;<br /> >>>   }<br /> >>><br /> >>>   int foo_1(int a) {<br /> >>>    return a * 2;<br /> >>>   }<br /> >>><br /> >>>   int foo_2(int a) {<br /> >>>    return a << 1;<br /> >>>   }<br /> >>><br /> >>>   It also happens that such functions are different on one stage, and become equal after optimisation pass.<br /> >>><br /> >>>   I have rephrased text you mentioned as follows:<br /> >>><br /> >>>   [quote]<br /> >>>   Sometimes code contains equal functions, or functions that does exactly the same<br /> >>>   thing even though they are non-equal on the IR level (e.g.: multiplication on 2<br /> >>>   and 'shl 2’).<br /> >>>   [/quote]<br /> >>><br /> >>>   Should be `shl 1`, but otherwise this fixes the issue I mentioned.<br /> >>>>>   If we will track every numbers and flags to be compared we would be able to get<br /> >>>>>   numbers chain and then create the hash number. So, once again, *total-ordering*<br /> >>>>>   could be considered as a milestone for even faster (in theory) random-access<br /> >>>>>   approach.<br /> >>>>   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?<br /> >>>   Nope, it behaves exactly as you imagined: comparison returns result once it find a difference.<br /> >>><br /> >>>   As I mentioned in article I tried random-access approach, it works a bit slower. But it has complexity O(N), so one day somebody could decide that he knows how to create fast random-access implementation. I think its just important to explain briefly why logarithmical search is used now, and what are the possible ways to improve current implementation. Taking into account your question I have rephrased this text:<br /> >>><br /> >>>   [quote]<br /> >>>   We can use the same comparison algorithm. During comparison we exit once we find<br /> >>>   the difference, but here we have to scan whole function body every time (note,<br /> >>>   it could be slower). Like in "total-ordering", we will track every numbers and<br /> >>>   flags, but instead of comparison, we should get numbers sequence and then<br /> >>>   create the hash number. So, once again, *total-ordering* could be considered as<br /> >>>   a milestone for even faster (in theory) random-access approach.<br /> >>>   [/quote]<br /> >>><br /> >>>   This sounds good, but please say "but here we might have to scan whole function body every time"; otherwise it sounds contradictory.<br /> >>><br /> >>>   I have also updated Passes.rst (paragraph about MergeFunctions):<br /> >>><br /> >>>   [quote]<br /> >>><br /> >>>   This pass looks for equivalent functions that are mergable and folds them.<br /> >>>   Total-ordering is introduced among the functions set: we define comparison that answers for every two functions which of them is greater. It allows to arrange functions into the binary tree.<br /> >>>   For every new function we check for equivalent in tree.<br /> >>>   If equivalent exists we fold such functions. If both functions are overridable, we move the functionality into a new internal function and leave two overridable thunks to it.<br /> >>>   If there is no equivalent, then we add this function to tree.<br /> >>>   Lookup routine has O(log(n)) complexity, while whole merging process has complexity of O(n*log(n)).<br /> >>>   Read this(link) article for more details.<br /> >>><br /> >>>   [/quote]<br /> >>><br /> >>>   Thanks!<br /> >>>   Stepan<br /> >>><br /> >>>   On 30 Sep 2014, at 02:03, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>>> wrote:<br /> >>><br /> >>>   Thanks for answering those questions; that really helps. Could you please address the "random comments" that I mentioned in my original reply?<br /> >>><br /> >>>   As it stands, I'm currently in favor of committing this (with the "random comments" fixed); Nick, what do you think?<br /> >>><br /> >>>   -- Sean Silva<br /> >>><br /> >>>   On Mon, Sep 29, 2014 at 2:26 AM, Stepan Dyatkovskiy <<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a><mailto:<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a>><mailto:<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a><mailto:<a href="mailto:sdyatkovskiy@accesssoftek.com">sdyatkovskiy@accesssoftek.com</a>>>> wrote:<br /> >>>   Hi Nick and Silva.<br /> >>>   Sorry again for such a latency.<br /> >>><br /> >>>   In new version I have answered on three questions mentioned in<br /> >>>   <a href="http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines" target="_blank">http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines</a><br /> >>><br /> >>>   Mostly it answers on Nick’s questions as well. I would like to stop specially on next question:<br /> >>>>   What is the burden for updating this document as the implementation changes and why is that a good tradeoff?<br /> >>>   I tried to describe common cases. I quoted a little of comments and described functions implementation, but I tried to cut off places where we potentially could change logic, proposing reader to view the sources for more details. Anyways, if it happen to be, I’ll try to cut such extra details from documentation and replace it with more generic form.<br /> >>><br /> >>>   This article is extension to source code and to comments we’ve added there. And it's been written on higher level than comments in source code.<br /> >>>   (Frankly, I started it as a prove of total-ordering approach we used in MergeFunctions, but then just extended it and got full-featured article :-) )<br /> >>><br /> >>>   Below are the answers quoted from article:<br /> >>><br /> >>>   [quote]<br /> >>><br /> >>>   1. Why would I want to read this document?<br /> >>>   Document is the extension to pass comments and describes the pass logic. It describes algorithm that is used in order to compare functions, and contains the explanations of how we could then combine equal functions correctly, keeping module valid.<br /> >>>   Material brought in top-down form, so reader could start learn pass from ideas and end up with low-level algorithm details, thus preparing him for reading the sources.<br /> >>>   So main goal is do describe algorithm and logic here; the concept. This document is good for you, if you don’t want to read the source code, but want to understand pass algorithms. Author tried not to repeat the source-code and cover only common cases, and thus avoid cases when after minor code changes we need to update this document.<br /> >>><br /> >>>   2. What should I know to be able to follow along with this document?<br /> >>>   Reader should be familiar with common compile-engineering principles and LLVM code fundamentals. In this article we suppose reader is familiar with Single Static Assingment concepts. Understanding of IR structure is also important.<br /> >>>   We will use such terms as “module”, “function”, “basic block”, “user”, “value”, “instruction”.<br /> >>>   As a good start point, Kaleidoscope tutorial could be used (link).<br /> >>>   Especially it’s important to understand chapter 3 of tutorial (link).<br /> >>>   Reader also should know how passes work in LLVM, he could use next article as a reference and start point here (link).<br /> >>>   What else? Well perhaps reader also should have some experience in LLVM pass debugging and bug-fixing.<br /> >>><br /> >>>   3. What I gain by reading this document?<br /> >>>   Main purpose is to provide reader with comfortable form of algorithms description, namely the human reading text. Since it could be hard to understand algorithm straight from the source code: pass uses some principles that have to be explained first.<br /> >>>   Author wishes to everybody to avoid case, when you read code from top to bottom again and again, and yet you don’t understand why we implemented it that way.<br /> >>>   We hope that after this article reader could easily debug and improve MergeFunctions pass and thus help LLVM project.<br /> >>><br /> >>>   [/quote]<br /> >>><br /> >>>   Thanks!<br /> >>>   -Stepan<br /> >>><br /> >>>   On 16 Sep 2014, at 05:16, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>>>> wrote:<br /> >>><br /> >>>   On Mon, Sep 15, 2014 at 3:07 PM, Nick Lewycky <<a href="mailto:nlewycky@google.com">nlewycky@google.com</a><mailto:<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>><mailto:<a href="mailto:nlewycky@google.com">nlewycky@google.com</a><mailto:<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>>><mailto:<a href="mailto:nlewycky@google.com">nlewycky@google.com</a><mailto:<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>><mailto:<a href="mailto:nlewycky@google.com">nlewycky@google.com</a><mailto:<a href="mailto:nlewycky@google.com">nlewycky@google.com</a>>>>> wrote:<br /> >>>   On 15 September 2014 15:02, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>>><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a><mailto:<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>>>>> wrote:<br /> >>>   Wow, this is a really detailed document. Great work!<br /> >>><br /> >>>   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.<br /> >>><br /> >>>   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?<br /> >>><br /> >>>   Hopefully this should get answered once Stepan an updates to answer the three questions: <a href="http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines" target="_blank">http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines</a><br /> >>><br /> >>>   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?<br /> >>><br /> >>>   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.<br /> >>><br /> >>>   -- Sean Silva<br /> >>><br /> >>>   Nick<br /> >>><br /> >>>   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 /> >>><br /> >>>   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?<br /> >>><br /> >>>   Some random comments:<br /> >>>>   Sometimes code contains functions that does exactly the same thing even though<br /> >>>>   they are non-equal on the binary level.<br /> >>>   This confuses me; do you mean non-equal on the source level, but equal on the binary level?<br /> >>>>   If we will track every numbers and flags to be compared we would be able to get<br /> >>>>   numbers chain and then create the hash number. So, once again, *total-ordering*<br /> >>>>   could be considered as a milestone for even faster (in theory) random-access<br /> >>>>   approach.<br /> >>>   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?<br /> >>>>   #. For two trees *T1* and *T2* we perform *depth-first-trace* and have two<br /> >>>>     chains as a product: "*T1Items*" and "*T2Items*".<br /> >>>   I think most readers would be more comfortable with the terms "depth-first-traversal" instead of "depth-first-trace" and "sequences" instead of "chains".<br /> >>>>   Consider modification of *cmpType* method.<br /> >>>   What does this paragraph mean?<br /> >>><br /> >>>   -- Sean Silva<br /> >>><br /> >>>   On Sun, Sep 14, 2014 at 11:02 PM, <<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a><mailto:<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a>><mailto:<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a><mailto:<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a>>><mailto:<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a><mailto:<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a>><mailto:<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a><mailto:<a href="mailto:llvm@dyatkovskiy.com">llvm@dyatkovskiy.com</a>>>>> wrote:<br /> >>>   ping<br /> >>><br /> >>>   11.09.2014, 12:50, "Stepan Dyatkovskiy" <<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a><mailto:<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a>><mailto:<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a><mailto:<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a>>><mailto:<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a><mailto:<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a>><mailto:<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a><mailto:<a href="mailto:stpworld@narod.ru">stpworld@narod.ru</a>>>>>:<br /> >>>>   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 /> >>>   _______________________________________________<br /> >>>   llvm-commits mailing list<br /> >>>   <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><mailto:<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><mailto:<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><mailto:<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>>><mailto:<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><mailto:<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><mailto:<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><mailto:<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 /> >>><br /> >>>   <<span>2014-10-03</span>-mergefunc-doc.patch><br /> ><br /> > _______________________________________________<br /> > llvm-commits mailing list<br /> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a></div></div>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></div></blockquote>