[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Eric Christopher via Phabricator via cfe-commits]

echristo added a comment.

Sadly I'm just noticing this:

I don't really agree with the rationale for adding fortran support to the clang 
driver. My proposed design here would be to move the aspects you need out of 
clang and into a separate library in llvm that you can use instead.

I don't necessarily want to remove this immediately, but I think work should be 
done to move forward in this other direction and eventually migrate this out.

Thanks!


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I had some time to take a look. D69636  makes 
things work on mac.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks!

Looks like it's just the `-E` test that's causing problems; the test passes 
with that disabled:

  $ git diff
  diff --git a/clang/test/Driver/flang/flang.f90 
b/clang/test/Driver/flang/flang.f90
  index 97e4847f843..4cbc2cd8754 100644
  --- a/clang/test/Driver/flang/flang.f90
  +++ b/clang/test/Driver/flang/flang.f90
  @@ -1,6 +1,5 @@
   ! D63607 made mac builders unhappy by failing this test, and it isn't
   ! yet obvious why. Mark as unsupported as a temporary measure.
  -! UNSUPPORTED: darwin
   
   ! Check that flang -fc1 is invoked when in --driver-mode=flang.
   
  @@ -21,10 +20,9 @@
   
   ! Check that f90 files are not treated as "previously preprocessed"
   ! ... in --driver-mode=flang.
  -! RUN: %clang --driver-mode=flang -### -E  %s 2>&1 | 
FileCheck --check-prefixes=ALL,CHECK-E %s
  +! RUN: %clang --driver-mode=flang -###   %s 2>&1 | FileCheck 
--check-prefixes=ALL,CHECK-E %s
   ! CHECK-E-NOT: previously preprocessed input
  -! CHECK-E-DAG: "-E"
  -! CHECK-E-DAG: "-o" "-"
  +! CHECK-E-DAG: "-o"
   
   ! RUN: %clang --driver-mode=flang -### -emit-ast   %s 2>&1 | 
FileCheck --check-prefixes=ALL,CHECK-EMIT-AST %s
   ! CHECK-EMIT-AST-DAG: "-triple"

I'll debug a bit more.




Comment at: clang/lib/Driver/Driver.cpp:4883
+  // And say "no" if this is not a kind of action flang understands.
+  if (!isa(JA) && !isa(JA) && 
!isa(JA))
+return false;

(please run clang-format on your new code -- it might also answer why some 
enums have a comma after the last entry ;) )


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

@thakis: I've marked the test unsupported in c75cd3c7f0f 
. 
Hopefully that makes your builder happy! I'll figure out what is going on and 
fix it.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

I've thought about it for a moment and I'm currently at a loss to quickly 
explain why this would only fail on darwin. In my patch, the change to 
LookupTypeForExtension should prevent clang from reaching this state where it 
complains about a preprocessed input.

I don't have access to a darwin machine right now to dive in, understand and 
fix it.

I propose to mark the test `UNSUPPORTED: darwin` until more is understood about 
the breakage. I assume there is no issue with that?


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

> @thakis: I can't make quick sense of the failure from those logs alone. I 
> would appreciate it very much to see the output of the `clang -###` run lines 
> to see what's missing. It should be showing that it would invoke flang.

It's the very first run line that's failing. The output looks like so:

  $ out/gn/bin/clang --driver-mode=flang -### -E 
/Users/thakis/src/llvm-project/clang/test/Driver/flang/flang.f90
  clang version 10.0.0 
  Target: x86_64-apple-darwin18.7.0
  Thread model: posix
  InstalledDir: /Users/thakis/src/llvm-project/out/gn/bin
  clang: warning: 
/Users/thakis/src/llvm-project/clang/test/Driver/flang/flang.f90: previously 
preprocessed input [-Wunused-command-line-argument]


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

@thakis: I found the thread at 
https://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20120430/057199.html

@thakis: I can't make quick sense of the failure from those logs alone. I would 
appreciate it very much to see the output of the `clang -###` run lines to see 
what's missing. It should be showing that it would invoke flang.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D63607#1727037 , @peterwaller-arm 
wrote:

> Thanks for chiming in.
>
> > Sorry, I had missed the RfC, but it looks like there wasn't a lot of 
> > discussion on it anyways.
>
> Apologies @thakis - did I jump the gun? And if so, what could I have done 
> differently?


Whoops, sorry! I had meant to delete that bit. I started writing that, then 
went back to the old discussion thread, and saw that there was actually 
agreement on giving clang's driver knowledge about fortran (but not the 
preprocessor etc). So this is all good as far as I can tell. Please ignore that 
part.

The test is still failing fairly consistently on my bot, even after 6c0a160c 
 . (And 
that change required me to change my bots from `git merge --ff-only 
origin/master` to `git reset --hard origin/master` after fetching, but that's a 
good change to do anyways ;) ). Does http://45.33.8.238/mac/2528/step_6.txt 
make any sense to you, or should I ssh in and debug?


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

> Interesting, I had missed that conversation! Google and DDG fail to find it 
> for me even quoting sentences out of it. Found it by wgetting the gzip 
> mailing list archives and grepping them - is there a better way? :)

(If you want, I can forward you the thread. But as I said, I just failed to 
delete my toplevel comment; all good.)


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

Thanks for chiming in.

> Sorry, I had missed the RfC, but it looks like there wasn't a lot of 
> discussion on it anyways.

Apologies @thakis - did I jump the gun? And if so, what could I have done 
differently?

> Adding fortran support to clang's driver has been suggested and decided 
> against before, see "[cfe-commits] [RFC and PATCH] Fortran"

Interesting, I had missed that conversation! Google and DDG fail to find it for 
me even quoting sentences out of it. Found it by wgetting the gzip mailing list 
archives and grepping them - is there a better way? :)

That conversation was in 2012, and since then flang has been accepted as a 
subproject to LLVM in "[llvm-dev] f18 is accepted as part of LLVM project! 
". Since the 
acceptance, the project has renamed to flang, and is in the process of actively 
being integrated into the llvm project.

As discussed in the RFC, I understand that libclangDriver was always intended 
to be a flexible driver used by other frontends, so in light of that, does the 
change still seem unreasonable? My expectation is that the footprint of fortran 
on libclangDriver should only be quite modest.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

6c0a160c2d33e54aecf1538bf7c85d8da92051be 
 renamed 
the file (thanks Jeremy).

In D63607#1726978 , @peterwaller-arm 
wrote:

> In D63607#1726895 , @teemperor wrote:
>
> > Could we have the `clang/test/Driver/flang/flang.F90` and 
> > `clang/test/Driver/flang/flang.f90` files in different directories please? 
> > As macOS's FS is case-insensitive, those two files have the same path from 
> > macOS perspective which is causing a whole bunch of issues (including 
> > breaking git even with 'core.ignorecase').
>
>
> This was fixed by Jeremy Morse in 6c0a160c2d33e54aecf1538bf7c85d8da92051be 
>  - 
> thanks.
>
> I'm investigating the non-deterministic failure but don't have access to a 
> mac to test.
>
> @thakis is the flaky test resolved by 6c0a160c 
>  or is 
> it still outstanding?


Green dragon survived the change (because it doesn't run `git pull` I assume) 
and is running the tests soon-ish: 
http://green.lab.llvm.org/green/view/Clang/job/clang-stage1-cmake-RA-incremental/4685/console


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

In D63607#1726895 , @teemperor wrote:

> Could we have the `clang/test/Driver/flang/flang.F90` and 
> `clang/test/Driver/flang/flang.f90` files in different directories please? As 
> macOS's FS is case-insensitive, those two files have the same path from macOS 
> perspective which is causing a whole bunch of issues (including breaking git 
> even with 'core.ignorecase').


This was fixed by Jeremy Morse in 6c0a160c2d33e54aecf1538bf7c85d8da92051be 
 - thanks.

I'm investigating the non-deterministic failure but don't have access to a mac 
to test.

@thakis is the flaky test resolved by 6c0a160c 
 or is it 
still outstanding?


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

Re: [PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Jeremy Morse via cfe-commits]

FYI, in llvmorg-10-init-8612-g6c0a160c2d3 [0] I've renamed one of the
two files, Windows too has a case-insensitive filesystem layer and
this broke our CI. (I think I've preserved the original intention of
the tests).

[0] 
https://github.com/llvm/llvm-project/commit/6c0a160c2d33e54aecf1538bf7c85d8da92051be

--
Thanks,
Jeremy
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Sorry, I had missed the RfC, but it looks like there wasn't a lot of discussion 
on it anyways.

Adding fortran support to clang's driver has been suggested and decided against 
before, see "[cfe-commits] [RFC and PATCH] Fortran"

In D63607#1726895 , @teemperor wrote:

> Could we have the `clang/test/Driver/flang/flang.F90` and 
> `clang/test/Driver/flang/flang.f90` files in different directories please? As 
> macOS's FS is case-insensitive, those two files have the same path from macOS 
> perspective which is causing a whole bunch of issues (including breaking git 
> even with 'core.ignorecase').


One issue is that the test flakily fails about half the time

http://45.33.8.238/mac/2498/summary.html fail
http://45.33.8.238/mac/2499/summary.html pass
http://45.33.8.238/mac/2500/summary.html fail
http://45.33.8.238/mac/2501/summary.html pass
http://45.33.8.238/mac/2502/summary.html fail
http://45.33.8.238/mac/2503/summary.html pass
http://45.33.8.238/mac/2504/summary.html fail


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

Could we have the `clang/test/Driver/flang/flang.F90` and 
`clang/test/Driver/flang/flang.f90` files in different directories please? As 
macOS's FS is case-insensitive, those two files have the same path from macOS 
perspective which is causing a whole bunch of issues (including breaking git 
even with 'core.ignorecase').


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm closed this revision.
peterwaller-arm added a comment.

Submitted in 6bf55804924d5a1d902925ad080b1a2b57c5c75c 
.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

Revoking my previous request: I now have commit access and I intend to submit 
this shortly.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm marked an inline comment as done.
peterwaller-arm added a comment.

I'm still awaiting commit access, please can someone submit this on my behalf?


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm marked 2 inline comments as done.
peterwaller-arm added inline comments.



Comment at: clang/test/Driver/flang/flang-not-installed.f90:11
+! shell syntax.
+! UNSUPPORTED: windows
+

hfinkel wrote:
> I believe that you can write:
> 
>   REQUIRES: shell
> 
> for this.
I've removed this test for now because I discovered that setting PATH="" is not 
enough to make clang lose the flang binary, if the flang binary is in the same 
directory as clang (which will be likely once flang is in-tree).


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm updated this revision to Diff 226270.
peterwaller-arm added a comment.

I have rebased the patch for conflicts to master and all the tests are passing.

While doing so, I discovered that the test for flang-not-installed was not fit 
for purpose, because clang actually doesn't first check the PATH, it can find a 
flang binary which lives in the same directory as itself. I conclude that 
having such a test is more trouble than it is worth. Adding such a test would 
involve adding some questionable test-specific machinery. Therefore I've 
removed that test for now.

I'll leave the patch over the weekend in case anyone objects, otherwise I 
intend to submit early next week.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63607/new/

https://reviews.llvm.org/D63607

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/flang/Inputs/one.f90
  clang/test/Driver/flang/Inputs/other.c
  clang/test/Driver/flang/Inputs/two.f90
  clang/test/Driver/flang/flang.F90
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/test/Driver/fortran.f95
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/test/Driver/fortran.f95
===
--- clang/test/Driver/fortran.f95
+++ clang/test/Driver/fortran.f95
@@ -1,21 +1,22 @@
-// Check that the clang driver can invoke gcc to compile Fortran.
+! Check that the clang driver can invoke gcc to compile Fortran when in
+! --driver-mode=clang. This is legacy behaviour - see also --driver-mode=flang.
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
-// CHECK-OBJECT: gcc
-// CHECK-OBJECT: "-c"
-// CHECK-OBJECT: "-x" "f95"
-// CHECK-OBJECT-NOT: cc1as
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
+! CHECK-OBJECT: gcc
+! CHECK-OBJECT: "-c"
+! CHECK-OBJECT: "-x" "f95"
+! CHECK-OBJECT-NOT: cc1as
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: gcc
-// CHECK-ASM: "-S"
-// CHECK-ASM: "-x" "f95"
-// CHECK-ASM-NOT: cc1
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+! CHECK-ASM: gcc
+! CHECK-ASM: "-S"
+! CHECK-ASM: "-x" "f95"
+! CHECK-ASM-NOT: cc1
 
-// RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
-// CHECK-WARN: gcc
-// CHECK-WARN-NOT: "-Wall"
-// CHECK-WARN: ld
-// CHECK-WARN-NOT: "-Wall"
+! RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+! CHECK-WARN: gcc
+! CHECK-WARN-NOT: "-Wall"
+! CHECK-WARN: ld
+! CHECK-WARN-NOT: "-Wall"
Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -0,0 +1,7 @@
+! Check that flang driver can handle multiple inputs at once.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/two.f90 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/two.f90"
Index: clang/test/Driver/flang/multiple-inputs-mixed.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs-mixed.f90
@@ -0,0 +1,7 @@
+! Check that flang can handle mixed C and fortran inputs.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/other.c 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang{{[^"/]*}}" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}clang{{[^"/]*}}" "-cc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/other.c"

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Hal Finkel via Phabricator via cfe-commits]

hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

In D63607#1709258 , @peterwaller-arm 
wrote:

> Friendly ping to everybody watching. I'd like to get this in soon if possible.
>
> Hal - do you think this is close to being accepted? Note that I consider this 
> "the beginning" rather than "the end", since there will be more functionality 
> to add piecewise before this is fully functional. In the meantime, since it 
> is new functionality, it should not break anything.


One comment on the test, but otherwise LGTM.




Comment at: clang/test/Driver/flang/flang-not-installed.f90:11
+! shell syntax.
+! UNSUPPORTED: windows
+

I believe that you can write:

  REQUIRES: shell

for this.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

Friendly ping to everybody watching. I'd like to get this in soon if possible.

Hal - do you think this is close to being accepted? Note that I consider this 
"the beginning" rather than "the end", since there will be more functionality 
to add piecewise before this is fully functional. In the meantime, since it is 
new functionality, it should not break anything.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added a comment.

Hi All, I'm going on leave for two weeks, returning October 14th. I can 
plausibly respond to comments until 14:00 UTC tomorrow (Thu 25th Sept).

As a quick note, another piece of the puzzle has been submitted for review at 
https://github.com/flang-compiler/f18/pull/759, which is the flang-side binary 
which will link to this code and default to running the driver in flang mode.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Richard Barton via Phabricator via cfe-commits]

richard.barton.arm added a comment.

I think the behaviour for missing flang is fine for now, and I think we can 
improve on it later on. 
We ought to codify (if it is not done already) where flang looks for tools to 
exec, because I think PATH is probably not the only place it could look to 
(directory of clang binary, other relative paths, other environment variables 
and commandline options, etc.) The new test will need revising once we get 
there.

I think this patch is looking good to be committed - what do you think @hfinkel 
?


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added inline comments.



Comment at: clang/include/clang/Driver/Driver.h:69
+CLMode,
+FlangMode,
   } Mode;

kiranchandramohan wrote:
> Is the comma by choice?
It was not. Fixed.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:37
+  if (isa(JA)) {
+CmdArgs.push_back("-emit-obj");
+  } else if (isa(JA)) {

kiranchandramohan wrote:
> peterwaller-arm wrote:
> > richard.barton.arm wrote:
> > > F18 does not currently support these options that control the output like 
> > > -emit-llvm and -emit-obj so this code doesn't do anything sensible at 
> > > present. Would it not make more sense to add this later on once F18 or 
> > > llvm/flang grows support for such options?
> > I've removed them.
> Can it be removed from the Summary of this PR?
They have now been reintroduced after we found that it had an unpleasant 
failure mode. Better to implement them now, after all, and fail gracefully in 
the frontend.



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 ||

richard.barton.arm wrote:
> 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.
Agreed. They can still be changed later if necessary.



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?
My reading of the code is that both [fF]90 and [fF]95 both get turned into 
TY_Fortran/TY_PP_Fortran.

Both of these in the Types.def end up coerced to "f95", currently. I don't 
think the driver wants an entry in the Types.def for each fortran standard - it 
doesn't have them for other languages, for example, and I'm unsure what the 
benefit of that would be. The main purpose of that table as far as I see is to 
determine what phases need to run.

My feeling is that the name in the types table should be "fortran". I don't 
want to make that change in the scope of this PR, since it might be a breaking 
change. It looks to me like the name works its way out towards human eyeballs 
but is otherwise unused.

I would like to implement -std inference from the file extension and additional 
fortran filename extensions (f18 etc) in another change request.



Comment at: clang/lib/Driver/Types.cpp:220
+
+  case TY_Fortran: case TY_PP_Fortran:
+return true;

richard.barton.arm wrote:
> peterwaller-arm wrote:
> > 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?
> > My reading of the code is that both [fF]90 and [fF]95 both get turned into 
> > TY_Fortran/TY_PP_Fortran.
> > 
> > Both of these in the Types.def end up coerced to "f95", currently. I don't 
> > think the driver wants an entry in the Types.def for each fortran standard 
> > - it doesn't have them for other languages, for example, and I'm unsure 
> > what the benefit of that would be. The main purpose of that table as far as 
> > I see is to determine what phases need to run.
> > 
> > My feeling is that the name in the types table should be "fortran". I don't 
> > want to make that change in the scope of this PR, since it might be a 
> > breaking change. It looks 

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm updated this revision to Diff 220856.
peterwaller-arm marked 14 inline comments as done.
peterwaller-arm edited the summary of this revision.
peterwaller-arm added a comment.

Thanks everyone for your comments.

Changes since last patch:

- Reintroduce handling of (no phase arg specified), -S, -emit-llvm, etc.

  The code implementing it was reordered for clarity.

  After internal discussion we concluded it was preferable to land these now 
even though they are unimplemented on the frontend side, in order to avoid 
giving assertion errors for unimplemented behaviour.

  It is planned that the frontend will emit sensible errors for unimplemented 
behaviour.

- Fixes to the cover letter for --driver-mode=fortran => --driver-mode=flang

- Add a test (flang-not-installed.f90) which demonstrates the error message if 
clang is invoked with no flang available, by unsetting PATH. This is a 
non-"-###" test which also does not depend on the presence of flang.
  - Since it uses shell syntax, it is marked unsupported on windows.

- In response to a private comment, Flang mode now coerces TY_PP_Fortran to 
TY_Fortran. This:
  - Leaves the legacy behaviour unchanged
  - Ensures that we do not emit a warning when passing TY_PP_Fortran inputs to 
-E
  - Should ensure that TY_PP_Fortran is treated exactly as a fortran file
  - Is consistent with f18's intent to not have any notion of preprocessed 
sources
  - Now the flang.f90 and flang.F90  tests are 
identical except for the filename

- Various minor whitespace changes and documentation fixes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63607/new/

https://reviews.llvm.org/D63607

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/flang/Inputs/one.f90
  clang/test/Driver/flang/Inputs/other.c
  clang/test/Driver/flang/Inputs/two.f90
  clang/test/Driver/flang/flang-not-installed.f90
  clang/test/Driver/flang/flang.F90
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/test/Driver/fortran.f95
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/test/Driver/fortran.f95
===
--- clang/test/Driver/fortran.f95
+++ clang/test/Driver/fortran.f95
@@ -1,21 +1,22 @@
-// Check that the clang driver can invoke gcc to compile Fortran.
+! Check that the clang driver can invoke gcc to compile Fortran when in
+! --driver-mode=clang. This is legacy behaviour - see also --driver-mode=flang.
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
-// CHECK-OBJECT: gcc
-// CHECK-OBJECT: "-c"
-// CHECK-OBJECT: "-x" "f95"
-// CHECK-OBJECT-NOT: cc1as
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
+! CHECK-OBJECT: gcc
+! CHECK-OBJECT: "-c"
+! CHECK-OBJECT: "-x" "f95"
+! CHECK-OBJECT-NOT: cc1as
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: gcc
-// CHECK-ASM: "-S"
-// CHECK-ASM: "-x" "f95"
-// CHECK-ASM-NOT: cc1
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+! CHECK-ASM: gcc
+! CHECK-ASM: "-S"
+! CHECK-ASM: "-x" "f95"
+! CHECK-ASM-NOT: cc1
 
-// RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
-// CHECK-WARN: gcc
-// CHECK-WARN-NOT: "-Wall"
-// CHECK-WARN: ld
-// CHECK-WARN-NOT: "-Wall"
+! RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+! CHECK-WARN: gcc
+! CHECK-WARN-NOT: "-Wall"
+! CHECK-WARN: ld
+! CHECK-WARN-NOT: "-Wall"
Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -0,0 +1,7 @@
+! Check that flang driver can handle multiple inputs at once.
+
+! RUN: %clang 

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

> One thought that occurs to me when reviewing this. I think we will assume 
> that 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?

The Clang tests should not actually run flang regardless - they should just 
test the command line that should be run. An error might be emitted if "flang" 
isn't found in the relevant search paths, however, that makes sense to me 
(although we might override this for the purpose of the tests?).


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Richard Barton via Phabricator via cfe-commits]

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 /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 ) 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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm updated this revision to Diff 220681.
peterwaller-arm marked 6 inline comments as done.
peterwaller-arm added a comment.

- Fixed spurious comma
- Fixed incorrect comment and changed comment wrapping.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63607/new/

https://reviews.llvm.org/D63607

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/flang/Inputs/one.f90
  clang/test/Driver/flang/Inputs/other.c
  clang/test/Driver/flang/Inputs/two.f90
  clang/test/Driver/flang/flang.F90
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/test/Driver/fortran.f95
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/test/Driver/fortran.f95
===
--- clang/test/Driver/fortran.f95
+++ clang/test/Driver/fortran.f95
@@ -1,21 +1,22 @@
-// Check that the clang driver can invoke gcc to compile Fortran.
+! 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.
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
-// CHECK-OBJECT: gcc
-// CHECK-OBJECT: "-c"
-// CHECK-OBJECT: "-x" "f95"
-// CHECK-OBJECT-NOT: cc1as
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
+! CHECK-OBJECT: gcc
+! CHECK-OBJECT: "-c"
+! CHECK-OBJECT: "-x" "f95"
+! CHECK-OBJECT-NOT: cc1as
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: gcc
-// CHECK-ASM: "-S"
-// CHECK-ASM: "-x" "f95"
-// CHECK-ASM-NOT: cc1
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+! CHECK-ASM: gcc
+! CHECK-ASM: "-S"
+! CHECK-ASM: "-x" "f95"
+! CHECK-ASM-NOT: cc1
 
-// RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
-// CHECK-WARN: gcc
-// CHECK-WARN-NOT: "-Wall"
-// CHECK-WARN: ld
-// CHECK-WARN-NOT: "-Wall"
+! RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+! CHECK-WARN: gcc
+! CHECK-WARN-NOT: "-Wall"
+! CHECK-WARN: ld
+! CHECK-WARN-NOT: "-Wall"
Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -0,0 +1,7 @@
+! Check that flang driver can handle multiple inputs at once.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/two.f90 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/two.f90"
Index: clang/test/Driver/flang/multiple-inputs-mixed.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs-mixed.f90
@@ -0,0 +1,7 @@
+! Check that flang can handle mixed C and fortran inputs.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/other.c 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang{{[^"/]*}}" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}clang{{[^"/]*}}" "-cc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/other.c"
Index: clang/test/Driver/flang/flang.f90
===
--- /dev/null
+++ clang/test/Driver/flang/flang.f90
@@ -0,0 +1,31 @@
+! Check that flang -fc1 is invoked when in --driver-mode=flang.
+
+! See also: flang.F90 - "an input which also requires preprocessing".
+
+! Test output types:
+! * -E
+! * -fsyntax-only
+
+! Most tests use ALL-LABEL to bracket the checks with the compiler invocation
+! and the source, which appear 

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added a comment.

Some minor comments about Filetypes and file extensions. Can be ignored or 
considered for a separate commit.




Comment at: clang/include/clang/Driver/Driver.h:69
+CLMode,
+FlangMode,
   } Mode;

Is the comma by choice?



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:37
+  if (isa(JA)) {
+CmdArgs.push_back("-emit-obj");
+  } else if (isa(JA)) {

peterwaller-arm wrote:
> richard.barton.arm wrote:
> > F18 does not currently support these options that control the output like 
> > -emit-llvm and -emit-obj so this code doesn't do anything sensible at 
> > present. Would it not make more sense to add this later on once F18 or 
> > llvm/flang grows support for such options?
> I've removed them.
Can it be removed from the Summary of this PR?



Comment at: clang/lib/Driver/Types.cpp:220
+
+  case TY_Fortran: case TY_PP_Fortran:
+return true;

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?



Comment at: clang/test/Driver/lit.local.cfg:1
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', 
'.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']

For completion .F95 also?
Should we add f03,F03,f08,F08,f18,F18. May be not now.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm updated this revision to Diff 220646.
peterwaller-arm added a comment.

Updated comment for IsFlangMode.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63607/new/

https://reviews.llvm.org/D63607

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/flang/Inputs/one.f90
  clang/test/Driver/flang/Inputs/other.c
  clang/test/Driver/flang/Inputs/two.f90
  clang/test/Driver/flang/flang.F90
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/test/Driver/fortran.f95
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/test/Driver/fortran.f95
===
--- clang/test/Driver/fortran.f95
+++ clang/test/Driver/fortran.f95
@@ -1,21 +1,22 @@
-// Check that the clang driver can invoke gcc to compile Fortran.
+! 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.
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
-// CHECK-OBJECT: gcc
-// CHECK-OBJECT: "-c"
-// CHECK-OBJECT: "-x" "f95"
-// CHECK-OBJECT-NOT: cc1as
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
+! CHECK-OBJECT: gcc
+! CHECK-OBJECT: "-c"
+! CHECK-OBJECT: "-x" "f95"
+! CHECK-OBJECT-NOT: cc1as
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: gcc
-// CHECK-ASM: "-S"
-// CHECK-ASM: "-x" "f95"
-// CHECK-ASM-NOT: cc1
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+! CHECK-ASM: gcc
+! CHECK-ASM: "-S"
+! CHECK-ASM: "-x" "f95"
+! CHECK-ASM-NOT: cc1
 
-// RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
-// CHECK-WARN: gcc
-// CHECK-WARN-NOT: "-Wall"
-// CHECK-WARN: ld
-// CHECK-WARN-NOT: "-Wall"
+! RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+! CHECK-WARN: gcc
+! CHECK-WARN-NOT: "-Wall"
+! CHECK-WARN: ld
+! CHECK-WARN-NOT: "-Wall"
Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -0,0 +1,7 @@
+! Check that flang driver can handle multiple inputs at once.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/two.f90 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/two.f90"
Index: clang/test/Driver/flang/multiple-inputs-mixed.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs-mixed.f90
@@ -0,0 +1,7 @@
+! Check that flang can handle mixed C and fortran inputs.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/other.c 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang{{[^"/]*}}" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}clang{{[^"/]*}}" "-cc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/other.c"
Index: clang/test/Driver/flang/flang.f90
===
--- /dev/null
+++ clang/test/Driver/flang/flang.f90
@@ -0,0 +1,34 @@
+! Check that flang -fc1 is invoked when in --driver-mode=flang.
+
+! See also: flang.F90 - "an input which also requires preprocessing".
+
+! Test various output types:
+! * The default (-emit-obj)
+! * -fsyntax-only
+! * -emit-llvm
+! * -emit-llvm -S
+! * -S
+
+! Most tests use ALL-LABEL to bracket the checks with the compiler invocation and the source, which appear at the beginning and end.

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4788
+bool Driver::ShouldUseFlangCompiler(const JobAction ) const {
+  // Say "no" if there is not exactly one input of a type flang understands.
+  if (JA.size() != 1 ||

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.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:37
+  if (isa(JA)) {
+CmdArgs.push_back("-emit-obj");
+  } else if (isa(JA)) {

richard.barton.arm wrote:
> F18 does not currently support these options that control the output like 
> -emit-llvm and -emit-obj so this code doesn't do anything sensible at 
> present. Would it not make more sense to add this later on once F18 or 
> llvm/flang grows support for such options?
I've removed them.



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 ||

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.



Comment at: clang/lib/Driver/ToolChains/Flang.cpp:67
+  Args.AddAllArgs(CmdArgs, options::OPT_R_Group); // "Rpass-"" options 
passthrough.
+  Args.AddAllArgs(CmdArgs, options::OPT_gfortran_Group); // gfortran options 
passthrough.
+

richard.barton.arm wrote:
> Similarly to previous comment, do we want to be passing all gfortran options 
> through to F18 in the immediate term or even the long term? I don't think 
> there has been any agreement yet on what the options that F18 will support 
> are (although I agree that gfortran-like options would be most sensible and 
> in keeping with clang's philosophy)
I have deleted these options pass-throughs. I think you're right in general 
that we should only add options along with support for those things. The plan 
is now to add an OPT_flang_Group (or alike) later.



Comment at: clang/test/Driver/fortran.f95:1
-// Check that the clang driver can invoke gcc to compile Fortran.
+! Check that the clang driver can invoke gcc to compile Fortran.
 

richard.barton.arm wrote:
> ... when not in --driver-mode=fortran
Fixed.


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

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm updated this revision to Diff 220645.
peterwaller-arm marked 6 inline comments as done.
peterwaller-arm retitled this revision from "[clang][driver] Add basic 
--driver-mode=fortran support for flang" to "[clang][driver] Add basic 
--driver-mode=flang support for fortran".
peterwaller-arm added a comment.

Updated for review comments and spotted a couple of things myself.

Changes since last time:

- Removed various options which aren't yet implemented on the f18 side.
- --driver-mode=fortran is now --driver-mode=flang, which is more consistent 
with other modes.
- Added tests which show that multiple inputs (and mixed C+Fortran inputs) are 
supported by the driver.
- Updated comments.
- Removed a branch in getTool which didn't make sense.
- Updated commit message.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63607/new/

https://reviews.llvm.org/D63607

Files:
  clang/include/clang/Driver/Driver.h
  clang/include/clang/Driver/ToolChain.h
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/CMakeLists.txt
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/Flang.cpp
  clang/lib/Driver/ToolChains/Flang.h
  clang/lib/Driver/Types.cpp
  clang/test/Driver/flang/Inputs/one.f90
  clang/test/Driver/flang/Inputs/other.c
  clang/test/Driver/flang/Inputs/two.f90
  clang/test/Driver/flang/flang.F90
  clang/test/Driver/flang/flang.f90
  clang/test/Driver/flang/multiple-inputs-mixed.f90
  clang/test/Driver/flang/multiple-inputs.f90
  clang/test/Driver/fortran.f95
  clang/test/Driver/lit.local.cfg

Index: clang/test/Driver/lit.local.cfg
===
--- clang/test/Driver/lit.local.cfg
+++ clang/test/Driver/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.f95',
+config.suffixes = ['.c', '.cpp', '.h', '.m', '.mm', '.S', '.s', '.f90', '.F90', '.f95',
'.cu', '.rs', '.cl', '.hip']
 config.substitutions = list(config.substitutions)
 config.substitutions.insert(0,
Index: clang/test/Driver/fortran.f95
===
--- clang/test/Driver/fortran.f95
+++ clang/test/Driver/fortran.f95
@@ -1,21 +1,22 @@
-// Check that the clang driver can invoke gcc to compile Fortran.
+! 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.
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
-// CHECK-OBJECT: gcc
-// CHECK-OBJECT: "-c"
-// CHECK-OBJECT: "-x" "f95"
-// CHECK-OBJECT-NOT: cc1as
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -c %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-OBJECT %s
+! CHECK-OBJECT: gcc
+! CHECK-OBJECT: "-c"
+! CHECK-OBJECT: "-x" "f95"
+! CHECK-OBJECT-NOT: cc1as
 
-// RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
-// RUN:   | FileCheck --check-prefix=CHECK-ASM %s
-// CHECK-ASM: gcc
-// CHECK-ASM: "-S"
-// CHECK-ASM: "-x" "f95"
-// CHECK-ASM-NOT: cc1
+! RUN: %clang -target x86_64-unknown-linux-gnu -integrated-as -S %s -### 2>&1 \
+! RUN:   | FileCheck --check-prefix=CHECK-ASM %s
+! CHECK-ASM: gcc
+! CHECK-ASM: "-S"
+! CHECK-ASM: "-x" "f95"
+! CHECK-ASM-NOT: cc1
 
-// RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
-// CHECK-WARN: gcc
-// CHECK-WARN-NOT: "-Wall"
-// CHECK-WARN: ld
-// CHECK-WARN-NOT: "-Wall"
+! RUN: %clang -Wall -target x86_64-unknown-linux-gnu -integrated-as %s -o %t -### 2>&1 | FileCheck --check-prefix=CHECK-WARN %s
+! CHECK-WARN: gcc
+! CHECK-WARN-NOT: "-Wall"
+! CHECK-WARN: ld
+! CHECK-WARN-NOT: "-Wall"
Index: clang/test/Driver/flang/multiple-inputs.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs.f90
@@ -0,0 +1,7 @@
+! Check that flang driver can handle multiple inputs at once.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/two.f90 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/two.f90"
Index: clang/test/Driver/flang/multiple-inputs-mixed.f90
===
--- /dev/null
+++ clang/test/Driver/flang/multiple-inputs-mixed.f90
@@ -0,0 +1,7 @@
+! Check that flang can handle mixed C and fortran inputs.
+
+! RUN: %clang --driver-mode=flang -### -fsyntax-only %S/Inputs/one.f90 %S/Inputs/other.c 2>&1 | FileCheck --check-prefixes=CHECK-SYNTAX-ONLY %s
+! CHECK-SYNTAX-ONLY-LABEL: "{{[^"]*}}flang{{[^"/]*}}" "-fc1"
+! CHECK-SYNTAX-ONLY: "{{[^"]*}}/Inputs/one.f90"
+!