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

Thu Jul 21 07:19:08 PDT 2022

kiranchandramohan added a comment.

In D130166#3667742 <https://reviews.llvm.org/D130166#3667742>, @jeanPerier wrote:

> In D130166#3665793 <https://reviews.llvm.org/D130166#3665793>, @kiranchandramohan wrote:
>
>> Do you want to clarify whether the scope of this document is limited to Lowering (parse-tree to MLIR, FIR, FIR transformations, FIR to LLVM)? Or is this a general guideline for all of Flang including for eg. Semantics, Runtime, OpenMP, OpenACC etc?
>
> I wrote it with Lowering in mind because that is what I am working on, but I think it is generic guideline and up to the code owner of the different part to enforce it. @kiranchandramohan What is your take for OpenMP, is this how you want people to work ?

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.

> In D130166#3665793 <https://reviews.llvm.org/D130166#3665793>, @kiranchandramohan wrote:
>
>> Would it be good to mention how a person can find what is a feature to pick up? For the intrinsics work we have a spreadsheet, would it make sense to have a spreadsheet or projects page in github? This page can have unimplemented features from Fortran 95, 2003 etc.
>
> A Github project page sounds like a great idea to me. I have not created any yet, and I see that there are now two options (beta and classic). @kiranchandramohan Do you have an opinion on which one we should use.

I have been using Classic mostly. No issues with beta as well but have not spent time understanding it.

================
Comment at: flang/docs/DesignGuideline.md:30-33
+The design document should be added to the `docs` folder as a markdown document,
+ideally using the name of the feature as the document name. Its approval on
+Phabricator is the pre-requisite to submitting patches implementing new
+features.
----------------
awarzynski wrote:
> jeanPerier wrote:
> > jeanPerier wrote:
> > > clementval wrote:
> > > > unterumarmung wrote:
> > > > > So, one should create a design doc first, merge it to the repository after necessary approvals and only then start to work on the feature?
> > > > > I'm with @awarzynski on this. I think it's more agile to first create a discourse discussion with RFC and/or design doc and then, after a mutual agreement that a feature is needed, start to work on the feature and the final design doc in the same revision.
> > > > I think discussion on Discourse are nice but I find it hard to summarize if you come later in the discussion or even after it. IMO The document is a better place to have the summary of what is agreed on. To get it it in tree phab is the only way. What about make a phab review for the document and advertise it in a Discourse post?
> > > > So, one should create a design doc first, merge it to the repository after necessary approvals and only then start to work on the feature?
> > > > I'm with @awarzynski on this. I think it's more agile to first create a discourse discussion with RFC and/or design doc and then, after a mutual agreement that a feature is needed, start to work on the feature and the final design doc in the same revision.
> > > 
> > > No, the guideline tells that you can first discuss on discourse if you want to, but I do not think we should mandate the discussions to happen on discourse if the person working on the feature prefers the discussions to happen as a design document review. Although, I agree with @awarzynski  point that discourse is more visible, and it makes sense to me to require people to at least advertise those in discourse.
> > > So, one should create a design doc first, merge it to the repository after necessary approvals and only then start to work on the feature?
> > > I'm with @awarzynski on this. I think it's more agile to first create a discourse discussion with RFC and/or design doc and then, after a mutual agreement that a feature is needed, start to work on the feature and the final design doc in the same revision.
> > 
> > 
> I suggest that:
> # We require all new proposal/design-docs to be //advertised// on Discourse.
> # People posting new proposals decide for themselves whether they prefer the discussion to continue on Discourse or Phabricator.
> 
> Basically, we let people decide what works for them best. 
For me, advertising in discourse should be made compulsory so that people are aware. 

================
Comment at: flang/docs/DesignGuideline.md:75
+  (this does not require writing a design document).
+- Identify if the feature affects compatibility with programs compiled by other
+  Fortran compilers, or if a given solution for flang could not be changed in
----------------
awarzynski wrote:
> jeanPerier wrote:
> > kiranchandramohan wrote:
> > > Is there are preference for compatibility with gfortran over other fortran compilers?
> > I do not want to make it a general rule to avoid inciting people to just do the same as gfortran ABI without thinking it through. But if being compatible with gfortran does not come at extra cost, then I would say it is probably the safe choice.
> > I do not want to make it a general rule to avoid inciting people to just do the same as gfortran ABI without thinking it through. 
> 
> That's a good point, but I think that we also want to avoid a situation in which some parts of "LLVM Flang" align with GFortran, something else with "Classic Flang" etc. To me, it's either "align with GFortran" or "do something custom". Consulting other compilers is important and should be taken into account, but I would still prioritise GFortran.
> 
> This is what Clang folks did in the early days. IIRC, `clang` used to be a drop-in replacement for `gcc` for a while. Then, gradually, they started diverging. 
It might be good to clarify this point further. Given that the descriptor, module format, etc., are not compatible, exactly what compatibility are we referring to here?

GCC has a dominant position in the C/C++ world, but I am not sure about the prominence of Gfortran in the Fortran world. A lot of our community comes from Classic Flang/nvfortran. It will be good to check compatibility there as well.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130166