[PATCH] D104229: [flang][docs] Add documentation for the new Flang driver

[Andrzej Warzynski via Phabricator via llvm-commits]

awarzynski marked 7 inline comments as done.
awarzynski added a comment.

Thank you for taking a look @richard.barton.arm

In D104229#2817089 <https://reviews.llvm.org/D104229#2817089>, @richard.barton.arm wrote:

> More diagrams would help to explain the structure of Actions, and perhaps the relationship between the compiler driver and other jobs/binaries it could call.

I agree, but I'm finding it difficult to come up with something useful. IMO, the diagrams that I created in the past wouldn't be that helpful here. Also, should we be documenting `clangDriver` here? I've added a short paragraph on `flang`, but that's meant to be just a quick intro.

Another reason for not getting into too much detail on the other compilation jobs (e.g. backend, assembler, linker) is that Flang does not support them yet.  Also, there's a great diagram and more documentation in Clang: https://clang.llvm.org/docs/DriverInternals.html#design-overview.



================
Comment at: flang/docs/FlangDriver.md:22
+preprocessing, frontend code-generation, middlend/backend code-optimisation and
+lowering, linking). For frontend specific tasks, the compiler driver creates a
+frontend job and delegates it to `flang -fc1`, the frontend driver.
----------------
richard.barton.arm wrote:
> Is that the right word choice? I think it's for Fortran compilation jobs isn't it?
> 
> I wonder if "toolchain driver" == flang and "compiler driver" -> flang -fc1 might work better?
I find this a bit tricky - I've never seen Clang being referred to as a "toolchain driver". LLDB is tool that belongs to the LLVM toolchain, but it's not driven by Clang. On the other hand, Clang does drive the compiler (i.e. Clang & LLVM) **and** the linker, so it's more than just a compiler driver.


================
Comment at: flang/docs/FlangDriver.md:44
+
+Internally, a `clangDriver` based compiler driver works by creating actions
+that correspond to various compiler phases (e.g. `PreprocessJobClass`,
----------------
richard.barton.arm wrote:
> I think a diagram would really help explain this part.
I find the output generated with `-ccc-print-phases` more helpful than any of the diagrams that I've generated so far :) Let me try that instead.


================
Comment at: flang/docs/FlangDriver.md:132
+`FlangOption` among its `Flags`) that's meant for the frontend, make sure that
+it is either forwarded to `flang -fc1` or translated into some other option
+that is accepted by the frontend driver. You can implement this in
----------------
richard.barton.arm wrote:
> I guess you do this by adding FlangFc1Option do you?
No, these flags are not used in this process. They are not used much beyond defining the output of `{flang|clang} -help`. Also, the semantics for option flags are not well defined, so we shouldn't rely on them too much.


================
Comment at: flang/docs/FlangDriver.md:156
+
+When you are using `%flang` instead of `%flang_fc1`, the compiler driver will
+add extra flags to the frontend driver invocation (i.e. `flang -fc1
----------------
richard.barton.arm wrote:
> Not sure what this para is trying to say.
> 
> Are you suggesting that all tests for flang (i.e. the Fortran compiler part of it) ought to use -fc1 command lines and only tests for compiler driver should invoke the compiler driver? So tests in flang/test/Driver should use `%flang -###` and all the others use `%flang_fc1`? If so, do you think that will work?
> 
> Or are you trying to teach a newcomer how to add a tests for the compiler driver and for frontend driver options handling specifically? If so, then I think it needs a bit more prescriptive detail.
I just want to highlight that there are differences between `%flang` and `%flang_fc1`. In some cases the observable behavior will be identical, so IMO it's worthwhile to have this written down. I will rephrase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104229