-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
add initial support for llvm-flang #13323
Conversation
mesonbuild/compilers/fortran.py
Outdated
@@ -430,7 +430,61 @@ | |||
'everything': default_warn_args + ['-Mdclchk']} | |||
|
|||
|
|||
class FlangFortranCompiler(ClangCompiler, FortranCompiler): | |||
class LlvmFlangFortranCompiler(ClangCompiler, FortranCompiler): |
Check warning
Code scanning / CodeQL
Conflicting attributes in base classes Warning
Function get_include_args
Function get_include_args
Base classes have conflicting values for attribute 'get_pic_args':
Function get_pic_args
Function get_pic_args
Base classes have conflicting values for attribute 'get_pch_use_args':
Function get_pch_use_args
Function get_pch_use_args
Base classes have conflicting values for attribute 'get_optimization_args':
Function get_optimization_args
Function get_optimization_args
Base classes have conflicting values for attribute 'get_debug_args':
Function get_debug_args
Function get_debug_args
Base classes have conflicting values for attribute 'compute_parameters_with_absolute_paths':
Function compute_parameters_with_absolute_paths
Function compute_parameters_with_absolute_paths
Base classes have conflicting values for attribute 'get_output_args':
Function get_output_args
Function get_output_args
Base classes have conflicting values for attribute 'get_compile_only_args':
Function get_compile_only_args
Function get_compile_only_args
Base classes have conflicting values for attribute 'get_has_func_attribute_extra_args':
Function get_has_func_attribute_extra_args
Function get_has_func_attribute_extra_args
Base classes have conflicting values for attribute 'linker_to_compiler_args':
Function linker_to_compiler_args
Function linker_to_compiler_args
Base classes have conflicting values for attribute 'get_compiler_dirs':
Function get_compiler_dirs
Function get_compiler_dirs
Base classes have conflicting values for attribute 'get_default_include_dirs':
Function get_default_include_dirs
Function get_default_include_dirs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let me know if you want the name of the class to be FlangLlvmFortranCompiler
(to go with flang-llvm
) or if I should leave it as LlvmFlangFortranCompiler
. Same question goes for ClassicFlangFortranCompiler
.
mesonbuild/compilers/fortran.py
Outdated
return search_dirs + ['-lFortranRuntime', '-lFortranDecimal'] | ||
|
||
|
||
class ClassicFlangFortranCompiler(ClangCompiler, FortranCompiler): |
Check warning
Code scanning / CodeQL
Conflicting attributes in base classes Warning
Function get_include_args
Function get_include_args
Base classes have conflicting values for attribute 'get_pic_args':
Function get_pic_args
Function get_pic_args
Base classes have conflicting values for attribute 'get_pch_use_args':
Function get_pch_use_args
Function get_pch_use_args
Base classes have conflicting values for attribute 'get_optimization_args':
Function get_optimization_args
Function get_optimization_args
Base classes have conflicting values for attribute 'get_debug_args':
Function get_debug_args
Function get_debug_args
Base classes have conflicting values for attribute 'compute_parameters_with_absolute_paths':
Function compute_parameters_with_absolute_paths
Function compute_parameters_with_absolute_paths
Base classes have conflicting values for attribute 'get_output_args':
Function get_output_args
Function get_output_args
Base classes have conflicting values for attribute 'get_compile_only_args':
Function get_compile_only_args
Function get_compile_only_args
Base classes have conflicting values for attribute 'get_has_func_attribute_extra_args':
Function get_has_func_attribute_extra_args
Function get_has_func_attribute_extra_args
Base classes have conflicting values for attribute 'linker_to_compiler_args':
Function linker_to_compiler_args
Function linker_to_compiler_args
Base classes have conflicting values for attribute 'get_compiler_dirs':
Function get_compiler_dirs
Function get_compiler_dirs
Base classes have conflicting values for attribute 'get_default_include_dirs':
Function get_default_include_dirs
Function get_default_include_dirs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some comments; there's more in the diff itself. This isn't actually functional yet (e.g. for compiling SciPy 1.13.1), because llvm/llvm-project@9d6837d broke the ability to link to Fortran_main
, which is required (even if perhaps spuriously so) by the scipy build.
mesonbuild/compilers/detect.py
Outdated
# capture help text for possible fallback | ||
try: | ||
_, help_out, help_err = Popen_safe_logged(compiler + ["--help"], msg='Detecting compiler via') | ||
except OSError as e: | ||
popen_exceptions[join_args(compiler + ["--help"])] = e | ||
help_out = help_err = "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eli-schwartz said in #12306:
As long as meson can tell the difference between classic flang and new flang by parsing the output of --help / --version [...] we are good to go because that's what we'll base identification off of, not the executable name.
Since I couldn't see anything else using this, I've tentatively added this now. This is because flang-new --version
outputs
flang-new version 17.0.6
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: E:\miniforge\envs\<env_name>\Library\bin
-- IOW, nothing llvm-specific (especially once the rename from flang-new
to flang
lands) --, while flang-new --help
outputs
OVERVIEW: flang LLVM compiler
USAGE: flang-new.exe [options] file...
OPTIONS:
[...]
mesonbuild/compilers/detect.py
Outdated
def _get_linker_try_windows(cls): | ||
linker = None | ||
if 'windows' in out or env.machines[for_machine].is_windows(): | ||
# If we're in a MINGW context this actually will use a gnu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I broke this out into a separate function rather than duplicating it for both flang flavours. Not sure where this should ideally go. If it's not defined as a nested function, it would need a pretty extensive signature.
There are several large potential changes still outstanding for flang, the biggest one of which will be the rename of the main binary from There's also a stalled refactor of the runtime libs which would change what we'd need to add under |
3efc2a7
to
d6ec347
Compare
Good news! With this PR + flang 19 built from main, I can build SciPy successfully. 🥳 Unfortunately flang 18 is broken for this purpose, and backports won't happen anymore. Now that I have the pieces together though, it should be much easier to test the rc's for flang 19 and ensure that they keep working. |
We are in RC freeze currently, so this will have to wait until the release. |
Is this ready for merging in your opinion? |
If you don't mind how I'm adding the readout of I'll ask again for the flang-new -> flang rename upstream (probably after llvm 19 branches later this month, it's already too tight for changes now), and there will certainly be other fixes, e.g. once upstream gains support for certain flags. Another potential point is whether meson would consider it its job to detect and error out for the problem in llvm/llvm-project#92496, as compiling e.g. SciPy with this PR + flang 18 will fail deep into compilation (but passes with flang 17 & 19). Other version-related concerns: flang <17 would require a Overall, if you are okay to ramp up support for flang gradually, and accept some sharp corners as things get ironed out over time, then I think we should merge this. |
I'd be happy to open a tracking issue with all the open potential improvements after this lands. |
Since we have settled on |
Good to go from my side at least. As I mentioned above, this will need some further testing and polish (e.g. follow-ups as currently unimplemented flags become available upstream), but it should be a good start. |
This needs a release note snippet, though. |
No point in having a separate commit for that. It only adds clutter. I can merge it with a squash, just remember this in the future. |
In this case, it turns out we can avoid the fearsome idea of squash merging resulting in utterly mangled commit messages. |
Thank you! 🙏 🥳 |
We could also keep using #12306 I suppose. |
PR for #12306