[flang][driver] Support fixed form detection

[FarisRehman]

Summary

Currently the new flang driver always runs in free form mode. Add support for fixed form mode based on the file extensions that f18 looks for, and treat "for", "FOR", "fpp" and "FPP" file extensions as fixed form files like gfortran does.

Additionally add support for the missing file extensions "ff", "FOR", "for", "f77", "ff90", "fpp", "FPP" to be compatible with f18 in this respect, along with extensions "f03", "F03", "f08", "F08" as the Cray Fortran compiler supports it.

The following file extensions will be processed as fixed form: .f, .F, .ff, .for, .FOR, .fpp, .FPP
Any other file type will be processed as free form.

Summary of changes:

• Add isFixedFormSuffix and isFreeFormSuffix to FrontendTool/Utils.h
• Set Fortran::parser::Options#isFixedForm according to the file type
• Change FrontendOptions#GetInputKindForExtension to support the missing file extensions that f18 supports and some additional file extensions

[sameeranjoshi]

flang/lib/Frontend/FrontendOptions.cpp

15–16

What about .f08 extension or maybe f03?
It seems to be in one of the popular compilers[1]

[1]https://github.com/llvm/llvm-project/blob/master/flang/docs/OptionComparison.md#notes

23

Why is this repeated from above Cases statement?
Why are all options not inside a single Cases statement?
If it's an issue mentioned from the comments, would it be better to differentiate the above options in 2 different Cases based on free and fixed form and add a one liner comment on top about the form they belong to?

[FarisRehman]

flang/lib/Frontend/FrontendOptions.cpp

23

Correct, it is because of the issue mentioned in the comments.
I agree and I have made the changes to separate the cases based on form, thanks.

[FarisRehman]

Support more extensions

Further support more extensions, and address the comments made by @sameeranjoshi

Summary of changes:

• Add support for f08, F08, f03 and F03 extensions
• Seperate Cases based on fixed and free form
• Fix a unit test failing due to the fixed form change

[sscalpone]

Are you able to make a small table for us to look at the whole list of supported fixed and free form sources?

I believe the "-x" option allows different language types to be supported. Do we think -x will differentiate between free/fixed, or do we anticipate adding special options to instruct the compiler to handle fixed/free source?

More information about other compilers is available here:
https://github.com/llvm/llvm-project/blob/main/flang/docs/OptionComparison.md

[awarzynski]

@sscalpone @sameeranjoshi Let me just point out that currently Flang does not differentiate between different versions of Fortran. So file.f95 and file.f03 is internally simply treated as Language::Fortran.

In D94228#2490643, @sscalpone wrote:

I believe the "-x" option allows different language types to be supported. Do we think -x will differentiate between free/fixed, or do we anticipate adding special options to instruct the compiler to handle fixed/free source?

gfortran uses -ffixed-form & -ffree-form and that's what we proposed for Flang [1] [2]. The table below effectively lists all options supported by -x. For Fortran we differentiate between TY_Fortran (regular Fortran input, not preprocessed) and TY_PP_Fortran (regular Fortran input, preprocessed). In order to differentiate fixed and free form source file with -x we'd have to extend clang::driver::types::ID. IMO such change is not yet required.

• https://github.com/llvm/llvm-project/blob/80dee7965dffdfb866afa9d74f3a4a97453708b2/clang/include/clang/Driver/Types.h#L23

More information about other compilers is available here:
https://github.com/llvm/llvm-project/blob/main/flang/docs/OptionComparison.md

This document was very helpful when compiling [1] and [2]. Kudos to @PeteSteinfeld for preparing it!

[1] https://docs.google.com/spreadsheets/d/1JRe39lP_KhtkYxFEIvwrCFlE5v1Ofa_krOHI-XXXWPY/edit#gid=0
[2] https://lists.llvm.org/pipermail/flang-dev/2020-November/000588.html

flang/lib/Frontend/FrontendOptions.cpp

15–16

Thanks for the comment. I just wanted to point out that currently Flang does not differentiate between different versions of Fortran. Indeed, only F2018 grammar is mentioned in the docs: https://github.com/llvm/llvm-project/blob/main/flang/docs/f2018-grammar.md

So, whether other file extension are added or not, internally we are using Language::Fortran. That's unlikely to change any time soon. Adding more file extensions here is just a _syntactic_ sugar at this stage.

[awarzynski]

@FarisRehman , thank you for working on this!

I think that it would be useful to add a test like this:
Input name: `input.f:

program ShouldFail
end

Run line:

flang-new -E input.f

This should fail as flang-new will identify the input it as fixed-form, yet the actual file is free form.

flang/test/Flang-Driver/Inputs/free-form-test.f90

2

I think that fixed-free-detection.f90 will be easier to read if you use different name here. Perhaps instead of A, you could use: FixedForm and FreeForm?

flang/test/Flang-Driver/fixed-free-detection.f90

2

This tests verifies the driver correctly switches between the free and fixed forms (based on the file extension). This is tested by exploiting the fact that the preprocessor (more generally, the frontend, but not the driver itself) treats white-spaces differently for free and fixed form input files.

Would you mind clarifying this?

[tskeith]

flang/lib/Frontend/FrontendAction.cpp

60

This set of fixed form suffixes is duplicated in FrontendOptions.cpp. I think it would be better two have two predicates, say IsFixedFormSuffix and IsFreeFormSuffix, so that the former can be used in both places.

[awarzynski]

flang/lib/Frontend/FrontendAction.cpp

60

I would prefer if we kept it as is.

In FrontendOptions.cpp (in particular in GetInputKindForExtension), we don't intend to and should not differentiate between the fixed and free forms. Nor should we leave any comments that would suggest otherwise. GetInputKindForExtension is about differentiating between e.g. Fortran, CUDA, LLVM IR inputs. For reference, see similar method in Clang:

• https://github.com/llvm/llvm-project/blob/cbea6737d5130724c7c8cf8ee4ccf1c3dd099450/clang/lib/Frontend/FrontendOptions.cpp#L15-L36.

If something like IsFixedFormSuffix and IsFreeFormSuffix is introduced and used in GetInputKindForExtension, then that will suggest that the differentiation between fixed and free form is significant in that method. That's not the case. Also, we would have to abandon the llvm::StringSwitch API there, which IMO fits there very well.

The only reason that there are two cases for Language::Fortran in GetInputKindForExtension is that LLVM's Cases allows 9 entries at most:

• https://github.com/llvm/llvm-project/blob/cbea6737d5130724c7c8cf8ee4ccf1c3dd099450/llvm/include/llvm/ADT/StringSwitch.h#L132-L137

There's more than 9 file extensions that will lead to Language::Fortran.

flang/lib/Frontend/FrontendOptions.cpp

23

@sameeranjoshi @FarisRehman

Could I suggest that we actually remove // Free form files and // Fixed form files? These comments suggest that the split into fixed and free form matters here, but that is not the case here (see also my reply to @tskeith 's comment).

The key comment here is this:

// FIXME: Currently this API allows at most 9 items per case.

Link to the API: https://github.com/llvm/llvm-project/blob/cbea6737d5130724c7c8cf8ee4ccf1c3dd099450/llvm/include/llvm/ADT/StringSwitch.h#L132-L137. This is the only reason why we have two cases here instead of one. Perhaps we should just update that comment:

//  llvm::StringSwithc::Cases allows at most 9 strings per entry and hence we need multiple cases for Language::Fortran.

[FarisRehman]

In D94228#2492906, @awarzynski wrote:

I think that it would be useful to add a test like this:
Input name: `input.f:

program ShouldFail
end

Run line:

flang-new -E input.f

This should fail as flang-new will identify the input it as fixed-form, yet the actual file is free form.

Thanks for the suggestion! However, currently the driver does not print non-fatal errors so this test is not possible as incorrect input form is non-fatal.

[FarisRehman]

flang/lib/Frontend/FrontendOptions.cpp

23

Based on how the discussion evolved, I also feel that it is best to not have these comments here, and I have removed them. cc @sameeranjoshi

[FarisRehman]

Update regression test

Update the regression test to address comments.

Summary of changes:

• Rename test program names for greater readability
• Expand on the description of the regression test
• Remove comments separating fixed form from free form in FrontendOptions.cpp

[tskeith]

flang/lib/Frontend/FrontendAction.cpp

60

In FrontendOptions.cpp (in particular in GetInputKindForExtension), we don't intend to and should not differentiate between the fixed and free forms. Nor should we leave any comments that would suggest otherwise. GetInputKindForExtension is about differentiating between e.g. Fortran, CUDA, LLVM IR inputs. For reference, see similar method in Clang:

• https://github.com/llvm/llvm-project/blob/cbea6737d5130724c7c8cf8ee4ccf1c3dd099450/clang/lib/Frontend/FrontendOptions.cpp#L15-L36.

If something like IsFixedFormSuffix and IsFreeFormSuffix is introduced and used in GetInputKindForExtension, then that will suggest that the differentiation between fixed and free form is significant in that method.

If GetInputKindForExtension contained code like:
if (IsFixedFormSuffix(suffix) || IsFreeFormSuffix(suffix)) { return Language::Fortran; }
it would suggest to me that source form is not significant as they are both being treated the same way.

Also, we would have to abandon the llvm::StringSwitch API there, which IMO fits there very well.

It would fit better if the API weren't so clumsy. Wanting to use a favorite class is not a reason to duplicate code.

I'm fine with a different approach, but we shouldn't be listing the fixed-form suffixes twice in different places.

[FarisRehman]

flang/lib/Frontend/FrontendAction.cpp

60

From this discussion, it seems in this case moving away from StringSwitch and having helper methods isFixedFormSuffix and isFreeFormSuffix is the best solution. I have made these changes.

[FarisRehman]

Add helper methods to Utils

Add methods isFixedFormSuffix and isFreeFormSuffix to address the comment from @tskeith

Summary of changes:

• Create isFixedFormSuffix and isFreeFormSuffix methods in FrontendTool/Utils.h
• Use isFixedFormSuffix and isFreeFormSuffix methods instead of StringSwitch in FrontendOptions#GetInputKindForExtension

[sameeranjoshi]

Thank you for working on this.
It build and tests successfully.
Please wait for other reviewers to accept.

flang/include/flang/FrontendTool/Utils.h

17 ↗

(On Diff #316961)

Do you really need this?

flang/lib/Frontend/FrontendAction.cpp

13

Is this really needed?

[FarisRehman]

flang/include/flang/FrontendTool/Utils.h

17 ↗

(On Diff #316961)

This is needed as llvm::StringRef is used in isFixedFormSuffix and isFreeFormSuffix methods.

[awarzynski]

Thank you for working on this @FarisRehman ! LGTM.

[awarzynski]

Sadly, this patch is causing shared library builds to fail: http://lab.llvm.org:8011/#/builders/134/builds/1234

FAILED: lib/libflangFrontend.so.12git 
: && /usr/local/bin/c++ -fPIC -fPIC -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-copy -fno-strict-aliasing -fno-semantic-interposition -O3  -fno-semantic-interposition  -Wl,-z,defs -Wl,-z,nodelete   -Wl,-rpath-link,/home/tcwg-buildslave/worker/flang-aarch64-ubuntu-sharedlibs/build/./lib  -Wl,-O3 -Wl,--gc-sections -shared -Wl,-soname,libflangFrontend.so.12git -o lib/libflangFrontend.so.12git tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/CompilerInstance.cpp.o tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/CompilerInvocation.cpp.o tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/FrontendAction.cpp.o tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/FrontendActions.cpp.o tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/FrontendOptions.cpp.o tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/TextDiagnosticPrinter.cpp.o tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/TextDiagnosticBuffer.cpp.o tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/TextDiagnostic.cpp.o  -Wl,-rpath,"\$ORIGIN/../lib"  lib/libFortranSemantics.so.12git  lib/libclangDriver.so.12git  lib/libLLVMOption.so.12git  lib/libFortranParser.so.12git  lib/libFortranCommon.so.12git  lib/libclangBasic.so.12git  lib/libLLVMSupport.so.12git  -Wl,-rpath-link,/home/tcwg-buildslave/worker/flang-aarch64-ubuntu-sharedlibs/build/lib && :
tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/FrontendAction.cpp.o: In function `Fortran::frontend::FrontendAction::Execute()':
FrontendAction.cpp:(.text._ZN7Fortran8frontend14FrontendAction7ExecuteEv+0x38c): undefined reference to `Fortran::frontend::isFixedFormSuffix(llvm::StringRef)'
tools/flang/lib/Frontend/CMakeFiles/obj.flangFrontend.dir/FrontendOptions.cpp.o: In function `Fortran::frontend::FrontendOptions::GetInputKindForExtension(llvm::StringRef)':
FrontendOptions.cpp:(.text._ZN7Fortran8frontend15FrontendOptions24GetInputKindForExtensionEN4llvm9StringRefE+0x14): undefined reference to `Fortran::frontend::isFixedFormSuffix(llvm::StringRef)'
FrontendOptions.cpp:(.text._ZN7Fortran8frontend15FrontendOptions24GetInputKindForExtensionEN4llvm9StringRefE+0x38): undefined reference to `Fortran::frontend::isFreeFormSuffix(llvm::StringRef)'
collect2: error: ld returned 1 exit status

I'm guessing that's because isFixedFormSuffix and isFreeFormSuffix are implemented in flangFrontendTool, but used in flangFrontend. However, flangFrontendTool depends on flangFrontend: CmakeLists.tx. So basically it's a circular dependency.

EDIT: I've uploaded a fix: https://reviews.llvm.org/D94968

flang/lib/Frontend/FrontendAction.cpp

 //===--- FrontendAction.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Frontend/FrontendAction.h"
 #include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/FrontendActions.h"
 #include "llvm/Support/Errc.h"
 
 using namespace Fortran::frontend;
 
 void FrontendAction::set_currentInput(const FrontendInputFile &currentInput) {
   this->currentInput_ = currentInput;
 }
 
 // Call this method if BeginSourceFile fails.
 // Deallocate compiler instance, input and output descriptors

 
 llvm::Error FrontendAction::Execute() {
   CompilerInstance &ci = this->instance();
 
   std::string currentInputPath{GetCurrentFileOrBufferName()};
 
   Fortran::parser::Options parserOptions =
       this->instance().invocation().fortranOpts();
+  // Set the fixed form flag based on the file extension
+  auto pathDotIndex{currentInputPath.rfind(".")};
+  if (pathDotIndex != std::string::npos) {
+    std::string pathSuffix{currentInputPath.substr(pathDotIndex + 1)};
+    parserOptions.isFixedForm = pathSuffix == "f" || pathSuffix == "F" ||
+        pathSuffix == "ff" || pathSuffix == "for" || pathSuffix == "FOR" ||
+        pathSuffix == "fpp" || pathSuffix == "FPP";
+  }
 
   // Prescan. In case of failure, report and return.
   ci.parsing().Prescan(currentInputPath, parserOptions);
 
   if (ci.parsing().messages().AnyFatalError()) {
     const unsigned diagID = ci.diagnostics().getCustomDiagID(
         clang::DiagnosticsEngine::Error, "Could not scan %0");
     ci.diagnostics().Report(diagID) << GetCurrentFileOrBufferName();

flang/lib/Frontend/FrontendOptions.cpp

 //===- FrontendOptions.cpp ------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Frontend/FrontendOptions.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace Fortran::frontend;
 
 InputKind FrontendOptions::GetInputKindForExtension(llvm::StringRef extension) {
   return llvm::StringSwitch<InputKind>(extension)
-      // TODO: Should match the list in flang/test/lit.cfg.py
-      // FIXME: Currently this API allows at most 9 items per case.
-      .Cases("f", "F", "f77", "f90", "F90", "f95", "F95", "ff95", "f18", "F18",
+      // Note: Keep this list in-sync with flang/test/lit.cfg.py
+      // TODO: Add Cuda Fortan files (i.e. `*.cuf` and `*.CUF`).
+      // llvm::StringSwitch::Cases allows at most 9 strings per entry and hence
+      // we need multiple cases for Language::Fortran.
+      .Cases("f", "F", "ff", "for", "FOR", "fpp", "FPP", Language::Fortran)
+      .Cases("f77", "f90", "F90", "ff90", "f95", "F95", "ff95", "f18", "F18",
           Language::Fortran)
+      .Cases("f03", "F03", "f08", "F08", Language::Fortran)
       .Default(Language::Unknown);
 }

flang/test/Flang-Driver/Inputs/fixed-form-test.f

+      program FixedForm
+      end
 No newline at end of file

flang/test/Flang-Driver/Inputs/free-form-test.f90

+program FreeForm
+end
 No newline at end of file

flang/test/Flang-Driver/fixed-free-detection.f90

+! Ensure the driver correctly switches between fixed and free form based on the file extension.
+! This test exploits the fact that the preprocessor treats white-spaces differently for free
+! and fixed form input files.
+
+! REQUIRES: new-flang-driver
+
+!--------------------------
+! FLANG DRIVER (flang-new)
+!--------------------------
+! RUN: %flang-new -E %S/Inputs/free-form-test.f90  2>&1 | FileCheck %s --check-prefix=FREEFORM
+! RUN: %flang-new -E %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s --check-prefix=FIXEDFORM
+! RUN: %flang-new -E %S/Inputs/free-form-test.f90 %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s --check-prefix=MULTIPLEFORMS
+
+!-----------------------------------------
+! FRONTEND FLANG DRIVER (flang-new -fc1)
+!-----------------------------------------
+! RUN: %flang-new -fc1 -E %S/Inputs/free-form-test.f90  2>&1 | FileCheck %s --check-prefix=FREEFORM
+! RUN: %flang-new -fc1 -E %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s --check-prefix=FIXEDFORM
+! RUN: %flang-new -fc1 -E %S/Inputs/free-form-test.f90 %S/Inputs/fixed-form-test.f  2>&1 | FileCheck %s --check-prefix=MULTIPLEFORMS
+
+!-------------------------------------
+! EXPECTED OUTPUT FOR A FREE FORM FILE
+!-------------------------------------
+! FREEFORM:program freeform
+! FREEFORM-NOT:programfixedform
+
+!---------------------------------------
+! EXPECTED OUTPUT FOR A FIXED FORM FILE
+!---------------------------------------
+! FIXEDFORM:programfixedform
+! FIXEDFORM-NOT:program freeform
+
+!------------------------------------------------
+! EXPECTED OUTPUT FOR 2 FILES OF DIFFERENT FORMS
+!------------------------------------------------
+! MULTIPLEFORMS:program freeform
+! MULTIPLEFORMS-NOT:programfixedform
+! MULTIPLEFORMS-NEXT:end
+! MULTIPLEFORMS-NEXT:programfixedform
+! MULTIPLEFORMS-NOT:program freeform
 No newline at end of file

flang/test/lit.cfg.py

 # testFormat: The test format to use to interpret tests.
 #
 # For now we require '&&' between commands, until they get globally killed and
 # the test runner updated.
 config.test_format = lit.formats.ShTest(not llvm_config.use_lit_shell)
 
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = ['.c', '.cpp', '.f', '.F', '.ff', '.FOR', '.for', '.f77', '.f90', '.F90',
-                   '.ff90', '.f95', '.F95', '.ff95', '.fpp', '.FPP', '.cuf',
-                   '.CUF', '.f18', '.F18', '.fir']
+                   '.ff90', '.f95', '.F95', '.ff95', '.fpp', '.FPP', '.cuf'
+                   '.CUF', '.f18', '.F18', '.fir', '.f03', '.F03', '.f08', '.F08']
 
 config.substitutions.append(('%PATH%', config.environment['PATH']))
 
 llvm_config.use_default_substitutions()
 
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
 # directories.

flang/unittests/Frontend/FrontendActionTest.cpp

 //===- unittests/Frontend/PrintPreprocessedTest.cpp  FrontendAction tests--===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "gtest/gtest.h"
 #include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/CompilerInvocation.h"
 #include "flang/Frontend/FrontendOptions.h"
 #include "flang/FrontendTool/Utils.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace Fortran::frontend;
protected:
 
   CompilerInstance compInst_;
   std::shared_ptr<CompilerInvocation> invocation_;
 
   void SetUp() override {
     // Generate a unique test file name.
     const testing::TestInfo *const test_info =
         testing::UnitTest::GetInstance()->current_test_info();
-    inputFileName_ = std::string(test_info->name()) + "_test-file.f";
+    inputFileName_ = std::string(test_info->name()) + "_test-file.f90";
 
     // Create the input file stream. Note that this stream is populated
     // separately in every test (i.e. the input is test specific).
     inputFileOs_ = std::make_unique<llvm::raw_fd_ostream>(
         inputFileName_, ec_, llvm::sys::fs::OF_None);
     if (ec_)
       FAIL() << "Failed to create the input file";