Page MenuHomePhabricator

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

Authored by awarzynski on Jun 7 2022, 5:18 AM.

Details

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.

Diff Detail

Event Timeline

awarzynski created this revision.Jun 7 2022, 5:18 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
awarzynski requested review of this revision.Jun 7 2022, 5:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2022, 5:18 AM
ekieri added inline comments.Jun 7 2022, 11:54 AM
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 accepted this revision.Jun 8 2022, 3:35 AM

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.

This revision is now accepted and ready to land.Jun 8 2022, 3:35 AM
awarzynski added inline comments.Jun 8 2022, 5:45 AM
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 updated this revision to Diff 435115.Jun 8 2022, 5:47 AM

Remove redundant CHECK line

ekieri added inline comments.Jun 8 2022, 9:04 AM
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 added inline comments.Jun 10 2022, 3:20 AM
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 ;-)

This revision was landed with ongoing or failed builds.Jun 10 2022, 3:36 AM
This revision was automatically updated to reflect the committed changes.
sunho added a subscriber: sunho.Jun 17 2022, 5:20 PM

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

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!