LLVM Documentation: MergeFunctions pass

[Sean Silva]

On Mon, Sep 15, 2014 at 3:07 PM, Nick Lewycky <nlewycky at google.com> wrote:

> On 15 September 2014 15:02, Sean Silva <chisophugis at gmail.com> wrote:
>
>> Wow, this is a really detailed document. Great work!
>>
>> 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.
>>
>
> 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?
>

Hopefully this should get answered once Stepan an updates to answer the
three questions:
http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines


> 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?
>

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.

-- Sean Silva


>
> Nick
>
> In your first section please answer the three questions here:
>> http://llvm.org/docs/SphinxQuickstartTemplate.html#guidelines
>>
>> 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?
>>
>> Some random comments:
>>
>> > Sometimes code contains functions that does exactly the same thing even
>> though
>> > they are non-equal on the binary level.
>>
>> This confuses me; do you mean non-equal on the source level, but equal on
>> the binary level?
>>
>> > If we will track every numbers and flags to be compared we would be
>> able to get
>> > numbers chain and then create the hash number. So, once again,
>> *total-ordering*
>> > could be considered as a milestone for even faster (in theory)
>> random-access
>> > approach.
>>
>> 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?
>>
>> > #. For two trees *T1* and *T2* we perform *depth-first-trace* and have
>> two
>> >    chains as a product: "*T1Items*" and "*T2Items*".
>>
>> I think most readers would be more comfortable with the terms
>> "depth-first-traversal" instead of "depth-first-trace" and "sequences"
>> instead of "chains".
>>
>> > Consider modification of *cmpType* method.
>>
>> What does this paragraph mean?
>>
>> -- Sean Silva
>>
>>
>>
>> On Sun, Sep 14, 2014 at 11:02 PM, <llvm at dyatkovskiy.com> wrote:
>>
>>> ping
>>>
>>> 11.09.2014, 12:50, "Stepan Dyatkovskiy" <stpworld at narod.ru>:
>>> > Reattached as patch.
>>> >
>>> > Stepan Dyatkovskiy wrote:
>>> >>  Hello everyone,
>>> >>  Please review the MergeFunctions pass documentation in attachment.
>>> Hope
>>> >>  doc is clear enough :-)
>>> >>
>>> >>  - Stepan
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140915/0a9c4f58/attachment.html>