Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support LLVM flang #12306

Open
8 tasks
h-vetinari opened this issue Sep 29, 2023 · 22 comments
Open
8 tasks

Support LLVM flang #12306

h-vetinari opened this issue Sep 29, 2023 · 22 comments

Comments

@h-vetinari
Copy link
Contributor

h-vetinari commented Sep 29, 2023

Update, now that #13323 has been merged, I'm turning this into a list of further improvements, as opening a follow-up issue was deemed not necessary.

Blocked on upstream

Improvements

  • currently meson says that flang cannot handle fortran_std=legacy (workaround is using fortran_std=none)
  • automatically link to compiler-rt on windows (and other potential configuration).

For reference, in SciPy I currently use the following:

:: default flags for flang in conda-forge (in compiler activation)
set "FC=flang-new"
set "LD=lld-link.exe"
set "FFLAGS=-D_CRT_SECURE_NO_WARNINGS -fms-runtime-lib=dll -fuse-ld=lld -I%LIBRARY_INC%"
set "LDFLAGS=-Wl,-defaultlib:%CONDA_PREFIX:\=/%/lib/clang/@MAJOR_VER@/lib/windows/clang_rt.builtins-x86_64.lib"

:: in SciPy recipe
set "CC=clang-cl"
set "CXX=clang-cl"
:: https://github.com/conda-forge/scipy-feedstock/pull/253#issuecomment-1732578945
set "MESON_RSP_THRESHOLD=320000"

python -m build -w -n -x ^
    [...]
    -Csetup-args=-Dfortran_std=none

Note that -fms-runtime-lib is only supported since flang 18 (llvm/llvm-project@cf1e342)

Correctly handle non-functional versions

  • flang <17 requires a -flang-experimental-exec flag to compile things, and before flang 15/16, it cannot actually do any code generation itself, but relies on an external compiler, c.f. here. Realistically, not much before flang 17 is going to be functional (though apparently some people managed to build SciPy with flang 16).
  • flang 18 is broken for anything that wants to combine -shared with builds that require linking to Fortran_main (SciPy falls into this category); if it's possible to diagnose this and error out that would be great, or perhaps there are work-arounds to suppress/modify the linkage somehow.

Expand testing

  • test on more platforms
    • windows: pretty OK (what I've been testing mainly)
    • linux: should work, but haven't tested it
    • etc.

Previously:

This is a continuation of #10839, which got closed without any changes.

Flang has a long history, the relevant bits of which are that there's nowadays a "classic" flang, and a new LLVM-based flang (called f18 before it got merged into LLVM). Though they share a name, and developed largely by the same people, they're essentially completely different compilers.

As of LLVM 17, flang is starting to become useable (no more experimental flag necessary to generate executable), however, the naming still has not fully settled, and llvm-flang is currently still using flang-new as the name of the binary until some planned refactoring work is done.

One of the main differences that appeared pretty much immediately when testing this in conda-forge, is that llvm-flang does not use the PGI-style -Minform=inform and will throw an error upon that.

Furthermore, it doesn't support -module, but requires -module-dir. Finally, the runtime libs that need to be linked are different:

-        return search_dirs + ['-lflang', '-lpgmath']
+        return search_dirs + ['-lFortranRuntime', '-lFortranDecimal']

The runtime libs are due for a relatively large overhaul, so in addition with the binary name, supporting flang probably means:

  • having a separate class in fortran.py
  • doing some version-based logic that will switch runtime libs and binary name for LLVM 18/19 (or whenever these things land)

Finally, using llvm-flang also runs into #10778, and into some rsp-limitations resp. path mangling issues (for now worked around by raising MESON_RSP_THRESHOLD to ridiculous levels).

@h-vetinari
Copy link
Contributor Author

Ah yes, and according to meson, flang cannot handle fortran_std=legacy (worked around by using fortran_std=None).

@h-vetinari
Copy link
Contributor Author

In terms of feasibility, I'm happy to note that with a hacked-up version of meson (roughly according to what I'm describing in the OP), I managed to build scipy with meson & flang. :)

So if someone tells me how an acceptable PR for flang-support would roughly look like, I'm happy to give it a shot!

The runtime libs are due for a relatively large overhaul

This landed now as llvm/llvm-project@6403287, but I realised one thing: we probably don't need to add ['-lFortranRuntime', '-lFortranDecimal'] (now ['-lfortran-rt']) at all, because flang should take of this itself. For example, I had forgotten to specify -lFortran_main in the hacked-up version I used for scipy, but it still worked because flang is adding this itself (see the first hunk in the linked commit).

@eli-schwartz
Copy link
Member

but I realised one thing: we probably don't need to add ['-lFortranRuntime', '-lFortranDecimal'] (now ['-lfortran-rt']) at all, because flang should take of this itself.

IIRC the reason we add this isn't for when you build and link with the Fortran compiler, but rather when you build with the Fortran compiler and link with a different compiler as part of a multi-language target. Usually the C++ compiler.

@eli-schwartz
Copy link
Member

So if someone tells me how an acceptable PR for flang-support would roughly look like, I'm happy to give it a shot!

Roughly speaking, I think we want to insert naming bikeshed add a compilers/fortran.py class defining the flang-new compiler that does whatever is needed, and indicates itself as fc.get_id() such that people know this is "new flang". We have existing compiler IDs for flang, clang, and llvm, and I'm not sure what the difference is between the latter two...

compilers/detect.py is also going to want to be able to tell based on running $FC --version, that it is "new flang".

There's a list of possible command names for searching for a compiler when $FC has not been defined, but I think we want to hold off on that until we're sure what the final name is going to be.

@h-vetinari
Copy link
Contributor Author

h-vetinari commented Oct 2, 2023

Thanks for the feedback!

but I think we want to hold off on that until we're sure what the final name is going to be.

It's going to be flang. There was a long discussion that ultimately got resolved together with LLVM's BDFL; see particularly point 3. here (note also the relatively subdued reception for the proposed intermediate name llvm-flang in the following comments, because very few people much less distros have classic flang).

The only question is how long things will take. There's already an RFC however which proposes to do exactly what's been agreed upon as the final criterion to do the rename, but whether that'll be LLVM 18 or 19 (or later still), who knows. I get the feeling it's not too far off though - in most cases the new lowering already outperforms the old one, and they're just hunting down any remaining perf regressions.

@bilderbuchi
Copy link

The runtime libs are due for a relatively large overhaul

This landed now as llvm/llvm-project@6403287, but I realised one thing: we probably don't need to add ['-lFortranRuntime', '-lFortranDecimal'] (now ['-lfortran-rt']) at all, because flang should take of this itself. For example, I had forgotten to specify -lFortran_main in the hacked-up version I used for scipy, but it still worked because flang is adding this itself (see the first hunk in the linked commit).

I only want to make sure you're aware: AFAICT, the thing you consider landed was reverted pretty quickly (https://reviews.llvm.org/D154869#4652255) -- I don't know if a fixed version has landed in the meantime.

@h-vetinari
Copy link
Contributor Author

Yeah, I'm following along. It's pretty standard for big PRs in LLVM to get involved because the merge breaks something somewhere unforeseen. The issue with this one is that the person driving that change has apparently moved on already. In any case, it shouldn't concern the meson setup too much, as I mentioned above, the driver should already add the right linkage to the runtime libs by itself.

@dcbaker
Copy link
Member

dcbaker commented Oct 6, 2023

but I realised one thing: we probably don't need to add ['-lFortranRuntime', '-lFortranDecimal'] (now ['-lfortran-rt']) at all, because flang should take of this itself.

IIRC the reason we add this isn't for when you build and link with the Fortran compiler, but rather when you build with the Fortran compiler and link with a different compiler as part of a multi-language target. Usually the C++ compiler.

Yes, this is exactly what these are for. If the final link is done with a different language (c, c++, rust, etc) these are the flags we need to add manually because normally the native compiler would do this for us

@eli-schwartz
Copy link
Member

What's the upstream status on this?

@h-vetinari
Copy link
Contributor Author

You have good timing, I just started working on this again yesterday 😅

What's the upstream status on this?

Neither the rename nor the runtime lib refactor has happened yet. So far I was working on hacking enough on meson to get SciPy to compile with flang 18 (which got a lot stricter and will simply fail on unknown flags).

However, I haven't yet looked into how to distinguish llvm-flang (binary currently named flang-new, but will be named flang at some unspecified time in the future), and currently-in-meson flang (binary called flang, now referred to as "classic flang"). Aside from having to switch behaviour based on the version number somehow, there's a collision on the name. Thoughts on this would be appreciated!

If it helps, I can open a WIP PR, but it's going to be in a very rough state.

@eli-schwartz
Copy link
Member

Well, technically meson doesn't really care what the binary is named except inasmuch as we need to know what to run to try to check for a compiler. There are e.g. environments where gcc is clang because that's all the environment provides and they wanted to "seamlessly make things work for existing users", which is... definitely a choice.

There's also more boring reasons why a simple name check doesn't work, such as the complexity of handling x86_64-pc-linux-gnu-gcc-12 or musl-gcc, the fact that /usr/bin/cc is traditionally a symlink to whichever of GCC/clang are the system default...

As long as meson can tell the difference between classic flang and new flang by parsing the output of --help / --version (I assume checking the version number is good enough given classic flang isn't really based on llvm 18?) we are good to go because that's what we'll base identification off of, not the executable name.

@LaserEyess
Copy link

@h-vetinari please open a WIP PR with your patches. I was also looking into this and I'd like to help if possible. I can test on linux/windows, too.

@h-vetinari
Copy link
Contributor Author

I've opened a pull request now: #13323

It's even less functional than I'd want to - i.e. I cannot even build SciPy with it and flang 18 (v17 worked with a prior set of the patches, haven't tested the updated version yet). But perhaps it gives people something to discuss. I'm also building a flang from the upstream main branch to test, so that I can get better feedback on the open issue I have for this.

@h-vetinari
Copy link
Contributor Author

h-vetinari commented Jul 2, 2024

As a quick update: #13323 + the in-progress flang 19 can build SciPy (and pass the test suite). Unfortunately flang 18 is not usable, and the regression fix won't be backported. Otherwise, this should now slowly be approaching something like usability!

@eli-schwartz
Copy link
Member

The preliminary support has now been merged to git master. Let's discuss what else we need to get this polished.

@h-vetinari
Copy link
Contributor Author

I was about to close this and open a follow-up issue, but after your comment I edited the OP instead with the open points I'm aware of for improvements.

And of course, testing this on various platforms would be very helpful!

@gorloffslava
Copy link

gorloffslava commented Aug 15, 2024

@h-vetinari Wanted to say you a big-big thanks for this amazing work!

After finding a workaround for this issue, we were able to build SciPy using LLVM 20 on Amazon Linux (Fedora-based) w/ no extra patches to either SciPy or LLVM nor too much configuration.

  1. Tested SciPy versions: tagged release 1.14.0 and commit 9e9d534b7afac90e8d2fa23be1d0ab1201c1bde1 from the main branch (commit made on Aug 15).
  2. We build it against OpenBLAS, also compiled using flang-new. Tested versions: tagged releases 0.3.28 and 0.3.27 as well as 4944148e663719967755f151c60f5215172bdad7 commit from the develop branch (commit made on Aug 15).

BTW, transition to flang-new from gfortran reduced our scipy buildtime from 7-10 minutes to 1-2.

CC: @BwL1289

@rgommers
Copy link
Contributor

Awesome!

BTW, transition to flang-new from gfortran reduced our scipy buildtime from 7-10 minutes to 1-2.

I'll note that this was probably due to a hot cache, different build settings, or something like that. Not that much time is spent compiling or linking Fortran code, the heaviest components are C++.

@gorloffslava
Copy link

@rgommers yeah, I have the similar feeling. It's not so huge amount of Fortran code in SciPy.

Probably, this is something related to that flang better interoperates with LLVM ecosystem than gfortran (even if everything else is built with LLVM in both cases), but it's only my hypothesis.

@BwL1289
Copy link

BwL1289 commented Aug 15, 2024

thanks @rgommers!

@h-vetinari
Copy link
Contributor Author

The rename from flang-new to flang finally landed in LLVM 20: llvm/llvm-project@06eb10d 🥳

@h-vetinari
Copy link
Contributor Author

Also, the runtime libs refactor is progressing and almost there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants