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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to