Add new flang driver main() entrypoint

[CarolineConcatto]

Important: This patch requires changes in upstream clang to land in order to have the correct behaviour.

---

Note' : This a working in progress to have a flang driver for 18. More changes will be needed to fully support the new compiler.

Note'': Lit regression tests are added along with related CMakeLists infrastructure, this may be the first patch to do so.

This work implements the "driver" part of an RFC sent to cfe-dev in [1].

The new entrypoint adds a binary called bin/flang. The old flang.sh is removed from the install to make way for this.

bin/flang is implemented in terms of clang::Driver, defaulting to the --driver-mode=flang mode recently added to clang in [1]. This ensures that the driver runs in flang mode regardless of the name of the binary inferred from argv[0]. "flang -fc1" will be implemented as part of bin/flang in later patches.

If compiled against an old libclangDriver which is not flang-mode aware, the driver fails with:

"unsupported argument 'flang' to option '--driver-mode='"

Regression tests are added in test/driver, which mirror those in the clang/test/Driver directory. The primary difference is that the clang tests run clang --driver-mode=flang <input>, whereas these tests run flang <input>.

In order to add regression tests, the relevant CMakeLists infrastructure is included.

The configuration for the tests are in lit.cfg files. These rely on the presence of llvm-lit and FileCheck, as long as F18 is not a part of the monorepo it needs requires -DLLVM_EXTERNAL_LIT to be defined at cmake configuration time to point to /bin/llvm-lit from an LLVM build.

[1] "RFC: Adding a fortran mode to the clang driver for flang" http://lists.llvm.org/pipermail/cfe-dev/2019-June/062669.html [2] [clang][driver] Add basic --driver-mode=flang support for fortran https://reviews.llvm.org/D63607

Signed-off-by: Caroline Concatto [email protected]

[peterwaller-arm]

I tried to run the tests compiling against llvm-project master, but they didn't work for me. It can't find FileCheck. I included in my CMake configuration:

-DLLVM_EXTERNAL_LIT=/path/to/llvm/build/bin/llvm-lit
-DLLVM_TOOLS_BINARY_DIR=/path/to/llvm/build/bin

Using cmake --trace-expand, I can see that LLVM_TOOLS_BINARY_DIR is overriding my -D option in AddLLVM to be the <install prefix>/bin.

I think this addition should to include some minimal instructions about how to run the tests currently towards the end of README.md.

[peterwaller-arm]

Note: The first iteration of --driver-mode=fortran has now landed in upstream clang: llvm/llvm-project@6bf55804924d5a1d902925ad080b1a2b57c5c75c.

[CarolineConcatto]

We have a dependency with Clang Frontend that was removed with the latest patch.

[CarolineConcatto]

When we created this patch we wanted it to be able to call clang --driver-mode=flang from f18 side. And we would have for a while 2 flangs. flang-tmp that in the future would call fc1 and flang that is what we have currently inside f18. This 2 should coexist for a while until we could replace flang by using only flang-tmp.

We had this discussion on: reviewed on 25 Sep 2019 Collaborator peterwaller-arm left a comment This was just discussed on a private call: we need to leave bin/flang.sh in place. In the meantime we can give this new entrypoint a name like flang-tmp.

[RichBarton-Arm]

When we created this patch we wanted it to be able to call clang --driver-mode=flang from f18 side. And we would have for a while 2 flangs. flang-tmp that in the future would call fc1 and flang that is what we have currently inside f18. This 2 should coexist for a while until we could replace flang by using only flang-tmp.

I agree with all that and remember the discussion. My point is that whilst we have the two flangs, the new flang needs to be internally consistent with its own design. I don't think it is ever intended for the new flang-tmp to call to the old flang? That is what would happen with this patch committed.

I think the intention is for the new flang-tmp to call something similar to the current f18 binary. Maybe this binary is still TODO or maybe it can be the f18 binary itself. I would expect flang-tmp to either: a) call some binary that does not exist yet, with the -fc1 argument. Calling flang without -### would emit an error, which we could perhaps catch and report a nicer version of depending on how it looks. b) call the current f18 binary (without -fc1 argument)

I mildly prefer the former as it gives us a clean separation between the two drivers while under development, but I am open to suggestions otherwise.

[CarolineConcatto]

After me and @RichBarton-Arm talked we discover that my patch is calling flang because Flang.cpp in llvm-project(llvm-project/clang/lib/Driver/ToolChains/Flang.cpp). It has a hard code "flang" implemented in it. So it will always call flang. const char* Exec = Args.MakeArgString(D.GetProgramPath("flang", TC)) instead of something similar to this: const char *Exec = D.getClangProgramPath(); A solution will be to change things inside llvm-project.

[CarolineConcatto]

In my implementation, I assumed that the flang driver will select its frontend without users intervention and it will have the same name as the driver. For instance, flang -fortran-fe test -### will print flang -fc1 and not test -fc1 the flang driver completely ignores test and will set frontran-fe = flang. Is it ok to set fortran-fe in flang driver, or not? Like -driver-mode is fixed to flang. Or should the command line be able to overwrite the flag? If that is ok, there is a second question. With the current implementation, the frontend will have the same name as the driver. I was trying to follow clang behaviour when clang calls clang -fc1. Is this an ok assumption? According to the RFC proposed by Peter Waller, it looks like this is alright. If that is not ok, the further question will come.

[1]http://lists.llvm.org/pipermail/cfe-dev/2019-June/062669.html

[CarolineConcatto]

I did some mistake with my repo when I tried to do git pull --rebase.

[CarolineConcatto]

@peterwaller-arm , @RichBarton-Arm , @kiranchandramohan and @DavidTruby Do you mind if I close this PR and create a new one for this work? Lots have happened since I did this PR. First I rebased to lit, later to master. It now depends on other PR in llvm-project[1]. I would like to have a clean PR again. But is up to you. [1]b7c8b9d

[RichBarton-Arm]

@peterwaller-arm , @RichBarton-Arm , @kiranchandramohan and @DavidTruby Do you mind if I close this PR and create a new one for this work? Lots have happened since I did this PR. First I rebased to lit, later to master. It now depends on other PR in llvm-project[1]. I would like to have a clean PR again. But is up to you. [1]b7c8b9d

Starting afresh does sound like the cleanest way forward. You can link back to this for the full review trial.

[RichBarton-Arm]

In my implementation, I assumed that the flang driver will select its frontend without users intervention and it will have the same name as the driver. For instance, flang -fortran-fe test -### will print flang -fc1 and not test -fc1 the flang driver completely ignores test and will set frontran-fe = flang. Is it ok to set fortran-fe in flang driver, or not? Like -driver-mode is fixed to flang. Or should the command line be able to overwrite the flag? If that is ok, there is a second question. With the current implementation, the frontend will have the same name as the driver. I was trying to follow clang behaviour when clang calls clang -fc1. Is this an ok assumption? According to the RFC proposed by Peter Waller, it looks like this is alright. If that is not ok, the further question will come.

[1]http://lists.llvm.org/pipermail/cfe-dev/2019-June/062669.html

I think the behaviour you want is to be able to set the default flang-fe inside the driver code to be "flang-tmp" by calling Driver.GenericFortranFE = "flang-tmp" in F18's driver.

So you want flang-tmp (with no other options - i.e. the default) to call "flang-tmp" and flang-tmp --fortran-de="blah" to call "blah". Looking at the way you have implemented the default in https://reviews.llvm.org/D73951, it looks like you can't do this. If you follow David's suggestion there and have the default value of GenericFortranFE be handled by the constructor then calling GetProgramPath on the class variable from inside ConstructJob() then the F18 driver could set GenericFortranFE to "flang-tmp" before calling constructJob and it would all work out. This line would later be removed when we rename flang-tmp to flang.

[CarolineConcatto]

The file f18/tools/flang/main.cpp has this line: // Give to the driver the name of the frontend argv.push_back("-fortran-fe"); argv.push_back(basename(argv[0])); F18 driver over-writes the flag internally.

The f18 driver will over-write the "user" option for fortran-fe, because I am not checking fortran-fe before setting it to basename.

Have in mind that this is not the same behaviour in the llvm/clang side. Clang driver in flang mode (|https://reviews.llvm.org/D73951): clang --driver-mode=fortran -fortran-fe=foo -> calls frontend foo clang --driver-mode=fortran -> call frontend flang

F18 as it is: flang-tmp -fortran-fe=foo -> calls frontend flang-tmp
flang-tmp ->calls frontend flang-tmp

But should it be F18 like this? flang-tmp -fortran-fe=foo-> calls frontend foo flang-tmp -> call frontend flang-tmp

To be honest I like the first option for now. Where f18 driver sets the frontend. My reasons are: 1) simplicity and 2)it is similar to driver-more too in main.cpp. clang::driver::ParsedClangName TargetandMode("flang", "--driver-mode=flang");

[RichBarton-Arm]

But should it be F18 like this? flang-tmp -fortran-fe=foo-> calls frontend foo flang-tmp -> call frontend flang-tmp

I think it should be this one because:

• It mirrors the clang --driver-mode=flang intention, i.e. "call the Flang Fortran frontend from the driver" from the flang driver.
• It still allows the user to do something different if they want to override the default, same as they can for clang --driver-mode=flang
• It naturally supports renaming of the flang compiler binary

To make that work, I think you would need to either:

1. Check that the user has not already set the --fortran-fe option before setting the default in the current code.
2. (on the clang side) Add a setter method to Driver for setting the GenericFortranFE member of Driver then call that setter (on the flang side) to set the new default value to argv[0] before you call Driver.BuildCompilation.

I think I mildly prefer option 2 as it kinda fits the existing pattern for setTargetAndMode in that class, but don't hold that opinion strongly.