[PATCH] D73595: [mlir][Linalg][doc] Add Design Document for the Linalg Dialect

River Riddle via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 29 09:49:12 PST 2020


rriddle added a comment.

In D73595#1847182 <https://reviews.llvm.org/D73595#1847182>, @ftynse wrote:

> In D73595#1847054 <https://reviews.llvm.org/D73595#1847054>, @rriddle wrote:
>
> > In D73595#1846944 <https://reviews.llvm.org/D73595#1846944>, @nicolasvasilache wrote:
> >
> > > @ftynse I fully agree hence my pushing back on the github comments.
> >
> >
> > If you had started submitting this incrementally, you wouldn't have this problem ;)
>
>
> From my experience, you cannot write this kind of document (essentially a position paper more than documentation) incrementally in the same sense you would write code. There are no really separable components (functions, classes, features) that can be implemented independently. Worse, having only some parts available might make it convey a different message than what is actually intended. I would love to hear suggestions on how to iterate on such things.


It depends really, but I had some things in mind when making that comment. For essay-like/position pieces it is really difficult to split up I definitely agree. Though the more infra-documentation bits could be split up however, if they are similar to the other documentation on dialects: types,  operations, etc. This document seems to lack much of dry-langref style documentation so I 100% agree with you.

Now as for where to have this review, it is difficult.

- One big Phab review:

Pro: It can be treated the same as the review for everything else
Pro: The comment and revision history is easy to see
Con: It is difficult when there are multiple collaborators as ownership/updates are difficult to make

- Multiple Phab reviews:

Pro: It is much easier to review
Con: The entire picture may not be clear if each of the reviews rely on each other for context and deeply interconnect

- Google Docs:

Pro: Much easier to collaboratively modify
Con: Comment/Revision history is difficult to view/recover

The above isn't exhaustive by any means and I don't necessarily have a preference in any of those cases. I'd just like to encourage a culture where we the community can more easily collaborate and review.

> 
> 
>> 
>> 
>>> @rriddle can we please move forward?
>>>  I'll integrate the current review and then we can iterate like Alex suggests?
>> 
>> Is there a rush here? I have some strong concerns with the way this document is structured as-is, which are largely enumerated by Uday on the github commit:
>> 
>>> This file doesn't appear to be just the documentation or design doc of Linalg but also a rationale doc + past survey/experience + related work discussion + opinion + future thoughts > / side comments and some amount of vision. Writing these up is useful, but I think it's both misnamed and misplaced in the directory tree. For eg., other docs/Dialects/*.md file have > vastly different content.
>> 
>> What is the plan to resolve these?
> 
> I think most of this doc is the rationale for building Linalg, _not_ dialect documentation. It lists specific design decisions Linalg takes and needs the review of state-of-the-art to justify those decisions. How about renaming it to "Linalg Rationale" and linking to design decisions (aka core guiding principles) with an explanatory phrase that, unless you know the field well, you should read the survey first?

I don't think we necessarily need to have a disclaimer, but I'm +1 to this being named "Linalg Rationale" as it fits much better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73595/new/

https://reviews.llvm.org/D73595





More information about the llvm-commits mailing list