[PATCH] D122008: [flang][driver] Add support for generating executables

Wed Apr 13 08:12:30 PDT 2022

awarzynski added a comment.

Thank you all for your feedback!

**CMake integration** 
I have a couple more data points. I've experimented with CMake using Tin's CMake PR <https://gitlab.kitware.com/cmake/cmake/-/merge_requests/6323>. I confirm that together with this patch, I was able to build a small CMake project:

  -- The Fortran compiler identification is LLVMFlang 15.0.0
  -- Detecting Fortran compiler ABI info
  -- Detecting Fortran compiler ABI info - done
  -- Check for working Fortran compiler: /home/build/release/bin/flang-new - skipped
  -- Configuring done
  -- Generating done
  -- Build files have been written to: /home/work/cmake_test

As you can see, the compiler discovery worked fine (I was able to build and executable using the CMake generated Make files). I also tried the same experiment with an extra flag (Option 2 above) and that didn't work. I investigated a bit and discovered that internally CMake calls try_compile <https://gitlab.kitware.com/cmake/cmake/blob/1c12694124aedc0f7cf5a98e635e27d4c338fce0/Modules/CMakeTestFortranCompiler.cmake#L49-51> (CMake docs <https://cmake.org/cmake/help/latest/command/try_compile.html>). I don't see  any way to pass custom compiler options there (i.e. `try_compile` expects `flang-new file.f90` to just work). This makes me rather hesitant to pursue Option 2 at all.

@sscalpone I hope that this answers your question. I'm happy to re-open that PR myself, but it would probably make more sense for Tin to do it (since he wrote it originally). Either way, we will be making sure that it's attributed to Tin.

I think that it's also worth taking into account CMake's release cycle. Based on the following, it looks like a new version is released every March, July and November:

- https://discourse.cmake.org/c/announcements/8
- https://github.com/Kitware/CMake/releases

It would be extremely helpful to align adding CMake support for LLVM Flang with LLVM releases.

**LLVM 15 Release**
While people seem to lean towards Option 3, I would like this change to be merged on time for LLVM 15. LLVM 16 will be out in 2023 - lets not wait that long. Also, with no public progress tracker for upstreaming, I would really like to make sure that there is a cut off date for this to land in LLVM.

**Other points**

In D122008#3427407 <https://reviews.llvm.org/D122008#3427407>, @sscalpone wrote:

> As I said this morning, I prefer to wait with this patch until the upstreaming is finished.  The reason is not only to do with upstreaming, but also with a concurrent effort to button up any assertion and runtime errors so the initial user experience is better.

Can you give us an indication when to expect this to be completed? Also, like @richard.barton.arm pointed out, people already can see the issues that you mentioned even without this patch (i.e. miscompilation errors are orthogonal  to this).

Note that this is only the first step. We still need to rename `flang-new` as `flang`. That's likely going to require a separate discussion. In fact, from my experience most people new to LLVM Flang use `flang` rather than `flang-new` and  most (all?) find it very confusing. This change is unlikely to change their perception of LLVM Flang because it only affects `flang-new`.

Lastly, I don't quite understand why can't we use project's documentation to communicate the level of support instead of artificially delaying patches like this one. This is what we currently claim (extracted from index.md <https://github.com/llvm/llvm-project/blob/main/flang/docs/index.md>, contributed in https://reviews.llvm.org/D120067 by myself):

> While it is capable of generating executables for a number of examples, some functionality is still missing.

I wrote that, perhaps naively, believing that this patch (or something similar) would soon be merged. In any case, if Flang developers are concerned how LLVM Flang is perceived by compiler users, then could we please fix this through better documentation and LLVM release notes?

**New proposal**
@rovka and @rouson , I know that you voted for Option 2, but after my CMake experiment I feel that we should really choose between Option 1 and 3. As there isn't enough support for Option 1, we should probably park this for now. Please let me know if Option 2 would unblock any of you (or anyone else!) and we can re-visit. Otherwise, lets wait a bit. Our goal is to have this merged in time for LLVM 15.

Thank you all for reading,
Andrzej

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008