[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:4788
+bool Driver::ShouldUseFlangCompiler(const JobAction &JA) const {
+  // Say "no" if there is not exactly one input of a type flang understands.
+  if (JA.size() != 1 ||
----------------
richard.barton.arm wrote:
> This first clause surprised me. Is this a temporary measure or do we never intend to support compiling more than one fortran file at once?
This function answers the question at the scope of a single JobAction. My understanding is that the compiler (as with clang -cc1) will be invoked once per input file.

This does not prevent multiple fortran files from being compiled with a single driver invocation.


================
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:37
+  if (isa<AssembleJobAction>(JA)) {
+    CmdArgs.push_back("-emit-obj");
+  } else if (isa<PreprocessJobAction>(JA)) {
----------------
richard.barton.arm wrote:
> F18 does not currently support these options that control the output like -emit-llvm and -emit-obj so this code doesn't do anything sensible at present. Would it not make more sense to add this later on once F18 or llvm/flang grows support for such options?
I've removed them.


================
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:44
+    if (JA.getType() == types::TY_Nothing) {
+      CmdArgs.push_back("-fsyntax-only");
+    } else if (JA.getType() == types::TY_LLVM_IR ||
----------------
richard.barton.arm wrote:
> Looks like the F18 option spelling for this is -fparse-only? Or maybe -fdebug-semantics? I know the intention is to throw away the 'throwaway driver' in F18, so perhaps you are preparing to replace this option spelling in the throwaway driver with -fsyntax-only so this would highlight that perhaps adding the code here before the F18 driver is ready to accept it is not the right approach?
In the RFC, the intent expressed was to mimick gfortran and clang options. I am making a spelling choice here that I think it should be called -fsyntax-only, which matches those. I intend to make the `flang -fc1` side match in the near future.

-fdebug-* could be supported through an `-Xflang ...` option to pass debug flags through.


================
Comment at: clang/lib/Driver/ToolChains/Flang.cpp:67
+  Args.AddAllArgs(CmdArgs, options::OPT_R_Group); // "Rpass-"" options passthrough.
+  Args.AddAllArgs(CmdArgs, options::OPT_gfortran_Group); // gfortran options passthrough.
+
----------------
richard.barton.arm wrote:
> Similarly to previous comment, do we want to be passing all gfortran options through to F18 in the immediate term or even the long term? I don't think there has been any agreement yet on what the options that F18 will support are (although I agree that gfortran-like options would be most sensible and in keeping with clang's philosophy)
I have deleted these options pass-throughs. I think you're right in general that we should only add options along with support for those things. The plan is now to add an OPT_flang_Group (or alike) later.


================
Comment at: clang/test/Driver/fortran.f95:1
-// Check that the clang driver can invoke gcc to compile Fortran.
+! Check that the clang driver can invoke gcc to compile Fortran.
 
----------------
richard.barton.arm wrote:
> ... when not in --driver-mode=fortran
Fixed.


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

https://reviews.llvm.org/D63607