[flang][driver] Fix support for `-x`

[awarzynski]

Summary

Until now, -x wasn't really taken into account in Flang's compiler and
frontend drivers. flang-new and flang-new -fc1 only recently gained
powers to consume inputs other than Fortran files and that's probably
why this hasn't been noticed yet.

This patch makes sure that -x is supported correctly and consistently
with Clang. To this end, verification is added when reading LLVM IR
files (i.e. IR modules are verified with llvm::verifyModule). This
way, LLVM IR parsing errors are correctly reported to Flang users. This
also aids testing.

With the new functionality, we can verify that -x ir breaks
compilation for e.g. Fortran files and vice-versa. Tests are updated
accordingly.

[ekieri]

flang/lib/Frontend/CompilerInvocation.cpp

268

Is there a reason to change from "f90" to "f95"? In my understanding, "f90" is more idiomatic for free-form fortran of any standard >= 90.

[rovka]

LGTM, thanks.

flang/lib/Frontend/CompilerInvocation.cpp

268

At least for gfortran, f90 doesn't seem to be supported, only f77, f77-cpp-input, f95, f95-cpp-input are.
https://raw.githubusercontent.com/gcc-mirror/gcc/master/gcc/doc/invoke.texi#:~:text=f77%20%20f77%2Dcpp%2Dinput%20f95%20%20f95%2Dcpp%2Dinput

Note that these are not file extensions, but values for the -x option.

flang/test/Driver/parse-ir-error.f95

17

Nit: I think checking just the "Could not parse IR" message should be enough.

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

268

Note that these are not file extensions, but values for the -x option.

Indeed, thanks Diana! For clangDriver, the available values for Fortran are defined here.

flang/test/Driver/parse-ir-error.f95

17

👍🏻

[awarzynski]

Remove redundant CHECK line

[ekieri]

flang/lib/Frontend/CompilerInvocation.cpp

268

Thank you both! This still goes against my intuition, but I must admit we should prioritise compatibility with gfortran above my intuition :)

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

268

This still goes against my intuition,

And mine :)

we should prioritise compatibility with gfortran

That and with what's already in Clang ;-)

[sunho]

Hi! I'm not exactly sure what's going on. But, seems like build is failing here? https://lab.llvm.org/buildbot/#/builders/177/builds/5571

[awarzynski]

In D127207#3593665, @sunho wrote:

Hi! I'm not exactly sure what's going on. But, seems like build is failing here? https://lab.llvm.org/buildbot/#/builders/177/builds/5571

Thanks for flagging this up! I am also rather confused. I checked the list of changes for that buildbot job and it all looks totally unrelated to flang-new -x. The error is:

******************** TEST 'Flang :: Driver/input-from-stdin-llvm.ll' FAILED ********************
Script:
--
: 'RUN: at line 10';   cat /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll | /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/not /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/flang-new -S - -o -
: 'RUN: at line 11';   cat /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll | /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/not /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/flang-new -fc1 -S - -o -
: 'RUN: at line 14';   cat /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll | /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/flang-new -x ir -S -target aarch64-unknown-linux-gnu - -o - | /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll
: 'RUN: at line 15';   cat /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll | /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/flang-new -fc1 -x ir -S -triple aarch64-unknown-linux-gnu - -o - | /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/FileCheck /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll
--
Exit Code: 141
Command Output (stderr):
--
+ : 'RUN: at line 10'
+ /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/not /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/flang-new -S - -o -
+ cat /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll
error: Could not scan -
standard input:10:15: error: bad character ('|') in Fortran token
  ; RUN: cat %s | not %flang -S - -o -
                ^
standard input:11:15: error: bad character ('|') in Fortran token
  ; RUN: cat %s | not %flang_fc1 -S - -o -
                ^
standard input:14:15: error: bad character ('|') in Fortran token
  ; RUN: cat %s | %flang -x ir -S -target aarch64-unknown-linux-gnu - -o - | FileCheck %s
                ^
standard input:14:74: error: bad character ('|') in Fortran token
  ; RUN: cat %s | %flang -x ir -S -target aarch64-unknown-linux-gnu - -o - | FileCheck %s
                                                                           ^
standard input:15:15: error: bad character ('|') in Fortran token
  ; RUN: cat %s | %flang_fc1 -x ir -S -triple aarch64-unknown-linux-gnu - -o - | FileCheck %s
                ^
standard input:15:78: error: bad character ('|') in Fortran token
  ; RUN: cat %s | %flang_fc1 -x ir -S -triple aarch64-unknown-linux-gnu - -o - | FileCheck %s
                                                                               ^
standard input:26:20: error: bad character ('{') in Fortran token
  define void @foo() {
                     ^
standard input:28:1: error: bad character ('}') in Fortran token
  }
  ^
+ : 'RUN: at line 11'
+ /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/not /home/tcwg-buildbot/worker/flang-aarch64-dylib/build/bin/flang-new -fc1 -S - -o -
+ cat /home/tcwg-buildbot/worker/flang-aarch64-dylib/llvm-project/flang/test/Driver/input-from-stdin-llvm.ll
error: Invalid input type - expecting a Fortran file
--
********************

But lines 10 and 11 in input-from-stdin-llvm.ll are expected to fail and hence there's not %flang rather than %flang 🤔 . The subsequent buildbot job is fine, so probably no need to change anything just now. But I will monitor this!

Thanks again for letting me know!

clang/include/clang/Driver/Options.td

 def weak__framework : Separate<["-"], "weak_framework">, Flags<[LinkerInput]>;
 def weak__library : Separate<["-"], "weak_library">, Flags<[LinkerInput]>;
 def weak__reference__mismatches : Separate<["-"], "weak_reference_mismatches">;
 def whatsloaded : Flag<["-"], "whatsloaded">;
 def why_load : Flag<["-"], "why_load">;
 def whyload : Flag<["-"], "whyload">, Alias<why_load>;
 def w : Flag<["-"], "w">, HelpText<"Suppress all warnings">, Flags<[CC1Option]>,
   MarshallingInfoFlag<DiagnosticOpts<"IgnoreWarnings">>;
-def x : JoinedOrSeparate<["-"], "x">, Flags<[NoXarchOption,CC1Option]>,
+def x : JoinedOrSeparate<["-"], "x">,
+Flags<[NoXarchOption,CC1Option,FlangOption,FC1Option]>,
   HelpText<"Treat subsequent input files as having type <language>">,
   MetaVarName<"<language>">;
 def y : Joined<["-"], "y">;
 
 defm integrated_as : BoolFOption<"integrated-as",
   CodeGenOpts<"DisableIntegratedAS">, DefaultFalse,
   NegFlag<SetTrue, [CC1Option, FlangOption], "Disable">, PosFlag<SetFalse, [], "Enable">,
   BothFlags<[], " the integrated assembler">>;

clang/lib/Driver/ToolChains/Flang.cpp

 
 #include <cassert>
 
 using namespace clang::driver;
 using namespace clang::driver::tools;
 using namespace clang;
 using namespace llvm::opt;
 
+/// Add -x lang to \p CmdArgs for \p Input.
+static void addDashXForInput(const ArgList &Args, const InputInfo &Input,
+                             ArgStringList &CmdArgs) {
+  CmdArgs.push_back("-x");
+  // Map the driver type to the frontend type.
+  CmdArgs.push_back(types::getTypeName(Input.getType()));
+}
+
 void Flang::AddFortranDialectOptions(const ArgList &Args,
                                      ArgStringList &CmdArgs) const {
   Args.AddAllArgs(
       CmdArgs, {options::OPT_ffixed_form, options::OPT_ffree_form,
                 options::OPT_ffixed_line_length_EQ, options::OPT_fopenmp,
                 options::OPT_fopenacc, options::OPT_finput_charset_EQ,
                 options::OPT_fimplicit_none, options::OPT_fno_implicit_none,
                 options::OPT_fbackslash, options::OPT_fno_backslash,
void Flang::ConstructJob(Compilation &C, const JobAction &JA,
   if (Output.isFilename()) {
     CmdArgs.push_back("-o");
     CmdArgs.push_back(Output.getFilename());
   } else {
     assert(Output.isNothing() && "Invalid output.");
   }
 
   assert(Input.isFilename() && "Invalid input.");
+
+  addDashXForInput(Args, Input, CmdArgs);
+
   CmdArgs.push_back(Input.getFilename());
 
   const auto& D = C.getDriver();
   // TODO: Replace flang-new with flang once the new driver replaces the
   // throwaway driver
   const char *Exec = Args.MakeArgString(D.GetProgramPath("flang-new", TC));
   C.addCommand(std::make_unique<Command>(JA, *this,
                                          ResponseFileSupport::AtFileUTF8(),
                                          Exec, CmdArgs, Inputs, Output));
 }
 
 Flang::Flang(const ToolChain &TC) : Tool("flang-new", "flang frontend", TC) {}
 
 Flang::~Flang() {}

flang/lib/Frontend/CompilerInvocation.cpp

static bool parseFrontendArgs(FrontendOptions &opts, llvm::opt::ArgList &args,
 
   // Get the input kind (from the value passed via `-x`)
   InputKind dashX(Language::Unknown);
   if (const llvm::opt::Arg *a =
           args.getLastArg(clang::driver::options::OPT_x)) {
     llvm::StringRef xValue = a->getValue();
     // Principal languages.
     dashX = llvm::StringSwitch<InputKind>(xValue)
-                .Case("f90", Language::Fortran)
+                // Flang does not differentiate between pre-processed and not
+                // pre-processed inputs.
+                .Case("f95", Language::Fortran)
+                .Case("f95-cpp-input", Language::Fortran)
                 .Default(Language::Unknown);
 
     // Some special cases cannot be combined with suffixes.
     if (dashX.isUnknown())
       dashX = llvm::StringSwitch<InputKind>(xValue)
                   .Case("ir", Language::LLVM_IR)
                   .Default(Language::Unknown);
 

flang/lib/Frontend/FrontendActions.cpp

 #include "mlir/Target/LLVMIR/ModuleTranslation.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticFrontend.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriterPass.h"
 #include "llvm/IR/LegacyPassManager.h"
+#include "llvm/IR/Verifier.h"
 #include "llvm/IRReader/IRReader.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Passes/PassBuilder.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Target/TargetMachine.h"
 #include <memory>
 
bool PrescanAndSemaDebugAction::beginSourceFileAction() {
   // never do this in actions intended for end-users or otherwise regular
   // compiler workflows!
   return runPrescan() && runParse() && (runSemanticChecks() || true) &&
          (generateRtTypeTables() || true);
 }
 
 bool CodeGenAction::beginSourceFileAction() {
   llvmCtx = std::make_unique<llvm::LLVMContext>();
+  CompilerInstance &ci = this->getInstance();
 
   // If the input is an LLVM file, just parse it and return.
   if (this->getCurrentInput().getKind().getLanguage() == Language::LLVM_IR) {
     llvm::SMDiagnostic err;
     llvmModule = llvm::parseIRFile(getCurrentInput().getFile(), err, *llvmCtx);
+    if (!llvmModule || llvm::verifyModule(*llvmModule, &llvm::errs())) {
+      err.print("flang-new", llvm::errs());
+      unsigned diagID = ci.getDiagnostics().getCustomDiagID(
+          clang::DiagnosticsEngine::Error, "Could not parse IR");
+      ci.getDiagnostics().Report(diagID);
+      return false;
+    }
 
-    return (nullptr != llvmModule);
+    return true;
   }
 
   // Otherwise, generate an MLIR module from the input Fortran source
-  assert(getCurrentInput().getKind().getLanguage() == Language::Fortran &&
-         "Invalid input type - expecting a Fortran file");
+  if (getCurrentInput().getKind().getLanguage() != Language::Fortran) {
+    unsigned diagID = ci.getDiagnostics().getCustomDiagID(
+        clang::DiagnosticsEngine::Error,
+    ci.getDiagnostics().Report(diagID);
+    return false;
+  }
   bool res = runPrescan() && runParse() && runSemanticChecks() &&
              generateRtTypeTables();
   if (!res)
     return res;
 
-  CompilerInstance &ci = this->getInstance();
-
   // Load the MLIR dialects required by Flang
   mlir::DialectRegistry registry;
   mlirCtx = std::make_unique<mlir::MLIRContext>(registry);
   fir::support::registerNonCodegenDialects(registry);
   fir::support::loadNonCodegenDialects(*mlirCtx);
 
   // Create a LoweringBridge
   const common::IntrinsicTypeDefaultKinds &defKinds =

flang/test/Driver/input-from-stdin-llvm.ll

+; Verify that reading from stdin works as expected - LLVM input
+
+;----------
+; RUN LINES
+;----------
+; Input type is implicit - assumed to be Fortran. As the input is provided via
+; stdin, the file extension is not relevant here.
+; RUN: cat %s | not %flang -S - -o -
+; RUN: cat %s | not %flang_fc1 -S - -o -
+
+; Input type is explicit
+; RUN: cat %s | %flang -x ir -S - -o - | FileCheck %s
+; RUN: cat %s | %flang_fc1 -x ir -S - -o - | FileCheck %s
+
+;----------------
+; EXPECTED OUTPUT
+;----------------
+; CHECK-LABEL: foo:
+; CHECK: ret
+
+;------
+; INPUT
+;------
+define void @foo() {
+  ret void
+}

flang/test/Driver/input-from-stdin.f90

-! Verify that reading from stdin works as expected
+! Verify that reading from stdin works as expected - Fortran input
 
 !--------------------------
 ! FLANG DRIVER (flang)
 !--------------------------
 ! Input type is implicit
 ! RUN: cat %s | %flang -E -cpp - | FileCheck %s --check-prefix=PP-NOT-DEFINED
 ! RUN: cat %s | %flang -DNEW -E -cpp - | FileCheck %s --check-prefix=PP-DEFINED
 ! RUN: cat %s | %flang -DNEW -E - | FileCheck %s --check-prefix=PP-NOT-DEFINED

flang/test/Driver/linker-flags.f90

 ! Use `--ld-path` so that the linker location (used in the LABEL below) is deterministic.
 ! RUN: %flang -### -flang-experimental-exec --ld-path=/usr/bin/ld %S/Inputs/hello.f90 2>&1 | FileCheck %s
 
 !----------------
 ! EXPECTED OUTPUT
 !----------------
 ! Compiler invocation to generate the object file
 ! CHECK-LABEL: {{.*}} "-emit-obj"
-! CHECK-SAME:  "-o" "[[object_file:.*]]" {{.*}}Inputs/hello.f90
+! CHECK-SAME:  "-o" "[[object_file:.*\.o]]" {{.*}}Inputs/hello.f90
 
 ! Linker invocation to generate the executable
 ! CHECK-LABEL:  "/usr/bin/ld"
 ! CHECK-SAME: "[[object_file]]"
 ! CHECK-SAME: -lFortran_main
 ! CHECK-SAME: -lFortranRuntime
 ! CHECK-SAME: -lFortranDecimal
 ! CHECK-SAME: -lm

flang/test/Driver/parse-error.f95

-! Verify that parsing errors are correctly reported by the driver
-! Focuses on actions inheriting from the following:
-! * PrescanAndSemaAction (-fsyntax-only)
-! * PrescanAndParseAction (-fdebug-unparse-no-sema)
-
-! RUN: not %flang_fc1 -fdebug-unparse-no-sema %s 2>&1 | FileCheck %s --check-prefix=ERROR
-! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=ERROR
-
-! ERROR: Could not parse {{.*}}parse-error.f95
-
-"This file will not parse"

flang/test/Driver/parse-error.ll

+; This file is a valid LLVM IR file, but we force the driver to treat it as
+; Fortran (with the `-x` flag). This way we verify that the driver
+; correctly rejects invalid Fortran input.
+
+;----------
+; RUN LINES
+;----------
+; Input type is implicit (correctly assumed to be LLVM IR)
+; RUN: %flang_fc1 -S %s -o -
+
+; Input type is explicitly set as Fortran
+; Verify that parsing errors are correctly reported by the driver
+; Focuses on actions inheriting from the following:
+; * PrescanAndSemaAction (-fsyntax-only)
+; * PrescanAndParseAction (-fdebug-unparse-no-sema)
+; RUN: not %flang_fc1 -fdebug-unparse-no-sema -x f95 %s 2>&1 | FileCheck %s --check-prefix=ERROR
+; RUN: not %flang_fc1 -fsyntax-only %s -x f95 2>&1 | FileCheck %s --check-prefix=ERROR
+
+; ERROR: Could not parse {{.*}}parse-error.f95
+
+define void @foo() {
+  ret void
+}

flang/test/Driver/parse-ir-error.f95

+! This file is a valid Fortran file, but we force the driver to treat it as an
+! LLVM file (with the `-x` flag). This way we verify that the driver correctly
+! rejects invalid LLVM IR input.
+
+!----------
+! RUN LINES
+!----------
+! Input type is implicit (correctly assumed to be Fortran)
+! RUN: %flang_fc1 -S %s
+! Input type is explicitly set as LLVM IR
+! RUN: not %flang -S -x ir %s 2>&1 | FileCheck %s
+
+!----------------
+! EXPECTED OUTPUT
+!----------------
+! CHECK: error: Could not parse IR
+
+end program