[flang-commits] [flang] 3e782ba - [flang][driver] Fix support for `-x`
Andrzej Warzynski via flang-commits
flang-commits at lists.llvm.org
Fri Jun 10 04:48:06 PDT 2022
On 10/06/2022 14:32, Diana Picus wrote:
> On Fri, 10 Jun 2022 at 12:36, Andrzej Warzynski via flang-commits
> <flang-commits at lists.llvm.org> wrote:
>>
>> Author: Andrzej Warzynski
>> Date: 2022-06-10T10:36:25Z
>> New Revision: 3e782ba21be4b88c761d3b0854df130d7ca08a56
>>
>> URL: https://github.com/llvm/llvm-project/commit/3e782ba21be4b88c761d3b0854df130d7ca08a56
>> DIFF: https://github.com/llvm/llvm-project/commit/3e782ba21be4b88c761d3b0854df130d7ca08a56.diff
>>
>> LOG: [flang][driver] Fix support for `-x`
>>
>> 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.
>>
>> Differential Revision: https://reviews.llvm.org/D127207
>>
>> Added:
>> flang/test/Driver/input-from-stdin-llvm.ll
>> flang/test/Driver/parse-error.ll
>> flang/test/Driver/parse-ir-error.f95
>>
>> Modified:
>> clang/include/clang/Driver/Options.td
>> clang/lib/Driver/ToolChains/Flang.cpp
>> flang/lib/Frontend/CompilerInvocation.cpp
>> flang/lib/Frontend/FrontendActions.cpp
>> flang/test/Driver/input-from-stdin.f90
>> flang/test/Driver/linker-flags.f90
>>
>> Removed:
>> flang/test/Driver/parse-error.f95
>>
>>
>> ################################################################################
>> diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
>> index 6707714f170b3..95840760f7746 100644
>> --- a/clang/include/clang/Driver/Options.td
>> +++ b/clang/include/clang/Driver/Options.td
>> @@ -4166,7 +4166,8 @@ 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">;
>>
>> diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
>> index 63e3c080a7e7f..ab9287d880a95 100644
>> --- a/clang/lib/Driver/ToolChains/Flang.cpp
>> +++ b/clang/lib/Driver/ToolChains/Flang.cpp
>> @@ -19,6 +19,14 @@ 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(
>> @@ -126,6 +134,9 @@ void Flang::ConstructJob(Compilation &C, const JobAction &JA,
>> }
>>
>> assert(Input.isFilename() && "Invalid input.");
>> +
>> + addDashXForInput(Args, Input, CmdArgs);
>> +
>> CmdArgs.push_back(Input.getFilename());
>>
>> const auto& D = C.getDriver();
>>
>> diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
>> index 9ea4810d64950..cc376f3b7d851 100644
>> --- a/flang/lib/Frontend/CompilerInvocation.cpp
>> +++ b/flang/lib/Frontend/CompilerInvocation.cpp
>> @@ -263,7 +263,10 @@ static bool parseFrontendArgs(FrontendOptions &opts, llvm::opt::ArgList &args,
>> llvm::StringRef xValue = a->getValue();
>> // Principal languages.
>> dashX = llvm::StringSwitch<InputKind>(xValue)
>> - .Case("f90", Language::Fortran)
>> + // Flang does not
>> diff erentiate 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.
>>
>> diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
>> index 334849ceeb54c..57b6203f94b2e 100644
>> --- a/flang/lib/Frontend/FrontendActions.cpp
>> +++ b/flang/lib/Frontend/FrontendActions.cpp
>> @@ -41,6 +41,7 @@
>> #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"
>> @@ -78,25 +79,36 @@ bool PrescanAndSemaDebugAction::beginSourceFileAction() {
>>
>> 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,
>> + "Invalid input type - expecting a Fortran file");
>> + 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);
>>
>> diff --git a/flang/test/Driver/input-from-stdin-llvm.ll b/flang/test/Driver/input-from-stdin-llvm.ll
>> new file mode 100644
>> index 0000000000000..142c332af1e53
>> --- /dev/null
>> +++ b/flang/test/Driver/input-from-stdin-llvm.ll
>> @@ -0,0 +1,26 @@
>> +; 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
>> +}
>>
>> diff --git a/flang/test/Driver/input-from-stdin.f90 b/flang/test/Driver/input-from-stdin.f90
>> index f05318bdec4c1..58bf03b46f219 100644
>> --- a/flang/test/Driver/input-from-stdin.f90
>> +++ b/flang/test/Driver/input-from-stdin.f90
>> @@ -1,4 +1,4 @@
>> -! Verify that reading from stdin works as expected
>> +! Verify that reading from stdin works as expected - Fortran input
>>
>> !--------------------------
>> ! FLANG DRIVER (flang)
>>
>> diff --git a/flang/test/Driver/linker-flags.f90 b/flang/test/Driver/linker-flags.f90
>> index 1210554927399..c46331f37029b 100644
>> --- a/flang/test/Driver/linker-flags.f90
>> +++ b/flang/test/Driver/linker-flags.f90
>> @@ -20,7 +20,7 @@
>> !----------------
>> ! 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"
>>
>> diff --git a/flang/test/Driver/parse-error.f95 b/flang/test/Driver/parse-error.f95
>> deleted file mode 100644
>> index 3b9ad93ea5147..0000000000000
>> --- a/flang/test/Driver/parse-error.f95
>> +++ /dev/null
>> @@ -1,11 +0,0 @@
>> -! 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"
>>
>> diff --git a/flang/test/Driver/parse-error.ll b/flang/test/Driver/parse-error.ll
>> new file mode 100644
>> index 0000000000000..374b218fc098f
>> --- /dev/null
>> +++ b/flang/test/Driver/parse-error.ll
>> @@ -0,0 +1,23 @@
>> +; 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
>> +}
>>
>> diff --git a/flang/test/Driver/parse-ir-error.f95 b/flang/test/Driver/parse-ir-error.f95
>> new file mode 100644
>> index 0000000000000..d10d71dc77542
>> --- /dev/null
>> +++ b/flang/test/Driver/parse-ir-error.f95
>> @@ -0,0 +1,18 @@
>> +! 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
> This is leaving a parse-ir-error.s file around. I've sent a quick
> commit to redirect the output to /dev/null, but you might want to
> consider sending it to FileCheck.
Thanks Diana, much appreciated! And :doh:!
FileCheck wouldn't be good - the output will differ depending on the
architecture. But we could also output to stdout like here:
https://github.com/llvm/llvm-project/blob/main/flang/test/Driver/parse-error.ll#L9
>
>> +! 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
>>
>>
>>
>> _______________________________________________
>> flang-commits mailing list
>> flang-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-commits
More information about the flang-commits
mailing list