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

[Bryan Chan via Phabricator via flang-commits]

bryanpkc added inline comments.


================
Comment at: flang/docs/DesignGuideline.md:51-53
+- Stick to Fortran well defined terms when talking about Fortran
+  (definitions of these terms can be found in Section 3. of Fortran 2018
+  standard)
----------------



================
Comment at: flang/docs/DesignGuideline.md:58-59
+- Illustrate with a few small Fortran code snippets if applicable
+- When dealing with lowering, illustrate lowering output with a few FIR,
+  and LLVM IR code snippets
+- Illustrations do not have to be fully functional programs, it is better to
----------------



================
Comment at: flang/docs/DesignGuideline.md:73
+- Check if the related constraints (Cxxx numbers in the standard) are enforced
+  by semantics. If not it is good idea to start by adding the related checks
+  (this does not require writing a design document).
----------------
rovka wrote:
> 
```
by semantic checks in the compiler. If not, it is a good idea to start by adding those checks
```


================
Comment at: flang/docs/DesignGuideline.md:79
+  with flang. It is not a goal to be 100% binary compatible with other
+  compilers outside of Fortran 77, but sources of incompatibility should be
+  known and justified.
----------------
Even for Fortran 77, can we really achieve binary compatibility with other compilers? I feel that binary compatibility shouldn't be a goal at all. As Kiran pointed out above, compatibility can mean different things; some important aspects where compatibility with other established compilers might be useful include: semantics of command-line options, default floating-point math behavior, runtime library functionality, etc.


================
Comment at: flang/docs/DesignGuideline.md:89-90
+  and what should happen when lowering the FIR to LLVM IR.
+- For inlined ops, look at how the existing dialects can be used,
+  if new FIR operations are required, justify their purpose.
+- Look at the related representation in Semantics (e.g., is some information
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130166