[flang-commits] [PATCH] D130166: [flang] Adding a guideline for flang design documentation

[Jean Perier via Phabricator via flang-commits]

jeanPerier added a comment.

In D130166#3668764 <https://reviews.llvm.org/D130166#3668764>, @kiranchandramohan wrote:

> I like the idea of good design docs. We should have something like this for OpenMP as well. The only difficulty is that the OpenMP dialect and OpenMP IRBuilder sit outside the flang tree and can be worked on independently.  So maybe any such discussions can be in discourse but before implementation in Flang (parse-tree to OpenMP + FIR dialects) we can make this a requirement for OpenMP as well subject to agreement of this proposal and consensus among OpenMP team members.

Good point, what about adding:

Features impacting projects outside of flang (like work OpenMP or OpenACC that may require touching parts outside of flang tree) should follow the general LLVM process, or the related subproject process. There should still be a related flang design docs if part of the solution impacts flang in significant way (e.g. it involves changes in the lowering from the parse-tree to OpenMP or FIR dialects that are not simply about mapping a parse-tree node to a related dialect operation).

In D130166#3668723 <https://reviews.llvm.org/D130166#3668723>, @awarzynski wrote:

> Currently there are two code-owners in LLVM Flang: @sscalpone and @schweitz (from CODE_OWNERS.TXT). Should it be updated?

Right, not my call, I think it is probably fine as it is. I turned to Kiran as my OpenMP point of contact, any official code owner is of course welcome to speak up here :) I guess that given the codeowners, and Kiran's point, this document should apply to all feature changes where significant choices have to be made in flang tree. Basically, everything that will have a task with the "design" label under the future flang related subproject should rule what we requires a design doc. I do not want to turn this document into a feature list :)



================
Comment at: flang/docs/DesignGuideline.md:21
+
+The preferred organization of such documents is:
+1) Problem description
----------------
rovka wrote:
> Do we have any examples in tree already?
Not with this exact format, the current documents have various structures. There are documents whose structure is very close like https://github.com/llvm/llvm-project/blob/main/flang/docs/RuntimeDescriptor.md

The purpose of this document is precisely to get to a more unified structure in the future (Note that I do not think it is worth changing the structure of existing docs at least not until we are far enough so that we can spend more time polishing the past).



================
Comment at: flang/docs/DesignGuideline.md:25
+3) Implementation details overview
+4) Testing plan
+
----------------
awarzynski wrote:
> jeanPerier wrote:
> > kiranchandramohan wrote:
> > > Should this have end to end tests? If so should we think of setting up a mechanism to run end-to-end tests?
> > Yes, I think it would be great if we could add end-to-end feature tests in the llvm test suite.
> > 
> > Would it make sense if we asked people to create a folder for the feature in https://github.com/llvm/llvm-test-suite/tree/main/Fortran/UnitTests and add their end to end tests there ?
> > 
> > In my opinion though, the testing plan may include running/benchmarking applications that may not be easily added there, and documenting the results might be enough for those.
> > 
> > Would it make sense if we asked people to create a folder for the feature in https://github.com/llvm/llvm-test-suite/tree/main/Fortran/UnitTests and add their end to end tests there ?
> 
> IMO, it would. But I would also ask the wider community first (that's outside the Flang sub-project). @Meinersbur helped a lot adding the existing Fortran tests there - could you chime in? More importantly, how do we make sure that any new tests in [[ https://github.com/llvm/llvm-test-suite |  llvm-test-suite  ]] are actually run by one of the bots?
> 
> > In my opinion though, the testing plan may include running/benchmarking applications that may not be easily added there, and documenting the results might be enough for those.
> 
> So this is not unit testing, is it? It's more like a validation plan. This would make perfect sense for optimisations (to demonstrate the benefits), but what about new features that can be tested with regular LIT tests. It would be good to clarify this point. In particular, testing in LLVM is fairly well documented. What additional testing would you require?
> So this is not unit testing, is it? It's more like a validation plan. This would make perfect sense for optimisations (to demonstrate the benefits), but what about new features that can be tested with regular LIT tests. It would be good to clarify this point. In particular, testing in LLVM is fairly well documented. What additional testing would you require?

This is testing in general. What kind of testing needs to be done should be a feature by feature decision, some aspect may be proven via unit tests, some other via validation tests. The purpose of the test plan is precisely  to propose which testing should be done. This guideline could list available test options, but I do not think it should dictate more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130166