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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits