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

[Richard Barton via Phabricator via cfe-commits]

richard.barton.arm added a comment.

Thanks for the updates. I think this is now looking good and matches the RFC already posted.

One thought that occurs to me when reviewing this. I think we will assume that F18 <https://reviews.llvm.org/F18>/flang when it lands in the LLVM project will be an optional thing to build and will be an opt-in project at the start that is off by default. What happens when clang --driver-mode=flang is called and flang has not been built? And where would we put a test for that behaviour? If flang were not to be built, should --driver-mode=flang be unsupported/unrecognised and emit an error message?

Might not be something we need to address with this patch, but have you considered this?



================
Comment at: clang/include/clang/Driver/Driver.h:186
+  /// Other modes fall back to calling gcc which in turn calls gfortran.
+  bool IsFlangMode() const { return Mode == FlangMode; }
+
----------------
Need to update the cover letter for the patch then as it still talks about 'fortran mode' rather than 'flang mode'.


================
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 ||
----------------
peterwaller-arm wrote:
> 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.
Righto - thanks for the explanation.


================
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 ||
----------------
peterwaller-arm wrote:
> 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.
OK - happy with this approach. So -fsyntax-only and -emit-ast will be the new names for f18's -fdebug-semantics and -fdebug-dump-parse-tree I guess.


================
Comment at: clang/lib/Driver/Types.cpp:220
+
+  case TY_Fortran: case TY_PP_Fortran:
+    return true;
----------------
kiranchandramohan wrote:
> Now that there is a 2018 standard, I am assuming that f18 and F18 are also valid fortran extensions. Can these be added to the File types?
> 
> Also, should the TypeInfo file be extended to handle all Fortran file types? ./include/clang/Driver/Types.def
> 
> Also, should we capture some information about the standards from the filename extension?
Agree with those first two, but that last one is surely a new feature that needs adding when such a feature is enabled in the rest of F18. 

We'd need to think that through carefully too. Classic flang never supported such an option and GCC's `-std` option from C/C++ does not work for Fortran. Also, would that go in the driver or in flang -fc1?

I suggest holding fire on this until we are ready to do it properly.


================
Comment at: clang/test/Driver/fortran.f95:2
+! Check that the clang driver can invoke gcc to compile Fortran when in
+! --driver-mode=clang. This is legacy behaviour - see also --driver-mode=fortran.
 
----------------
Need to change "see also --drver-mode=fortran" to "--driver-mode=flang" here.


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

https://reviews.llvm.org/D63607