From 4ce80b95f3cbb8b6e8ce6bb4546dee76b1f398dc Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Tue, 7 Nov 2023 00:17:31 +0100 Subject: [PATCH] spack compiler find --[no]-mixed-toolchain (#40902) Currently there's some hacky logic in the AppleClang compiler that makes it also accept `gfortran` as a fortran compiler if `flang` is not found. This is guarded by `if sys.platform` checks s.t. it only applies to Darwin. But on Linux the feature of detecting mixed toolchains is highly requested too, cause it's rather annoying to run into a failed build of `openblas` after dozens of minutes of compiling its dependencies, just because clang doesn't have a fortran compiler. In particular in CI where the system compilers may change during system updates, it's typically impossible to fix compilers in a hand-written compilers.yaml config file: the config will almost certainly be outdated sooner or later, and maintaining one config file per target machine and writing logic to select the correct config is rather undesirable too. --- This PR introduces a flag `spack compiler find --mixed-toolchain` that fills out missing `fc` and `f77` entries in `clang` / `apple-clang` by picking the best matching `gcc`. It is enabled by default on macOS, but not on Linux, matching current behavior of `spack compiler find`. The "best matching gcc" logic and compiler path updates are identical to how compiler path dictionaries are currently flattened "horizontally" (per compiler id). This just adds logic to do the same "vertically" (across different compiler ids). So, with this change on Ubuntu 22.04: ``` $ spack compiler find --mixed-toolchain ==> Added 6 new compilers to /home/harmen/.spack/linux/compilers.yaml gcc@13.1.0 gcc@12.3.0 gcc@11.4.0 gcc@10.5.0 clang@16.0.0 clang@15.0.7 ==> Compilers are defined in the following files: /home/harmen/.spack/linux/compilers.yaml ``` you finally get: ``` compilers: - compiler: spec: clang@=15.0.7 paths: cc: /usr/bin/clang cxx: /usr/bin/clang++ f77: /usr/bin/gfortran fc: /usr/bin/gfortran flags: {} operating_system: ubuntu23.04 target: x86_64 modules: [] environment: {} extra_rpaths: [] - compiler: spec: clang@=16.0.0 paths: cc: /usr/bin/clang-16 cxx: /usr/bin/clang++-16 f77: /usr/bin/gfortran fc: /usr/bin/gfortran flags: {} operating_system: ubuntu23.04 target: x86_64 modules: [] environment: {} extra_rpaths: [] ``` The "best gcc" is automatically default system gcc, since it has no suffixes / prefixes. --- lib/spack/spack/bootstrap/config.py | 4 +- lib/spack/spack/cmd/compiler.py | 17 +++- lib/spack/spack/compilers/__init__.py | 140 +++++++++++++++++++------- lib/spack/spack/compilers/aocc.py | 12 --- lib/spack/spack/compilers/clang.py | 18 +--- lib/spack/spack/test/cmd/compiler.py | 20 ++-- share/spack/spack-completion.bash | 4 +- share/spack/spack-completion.fish | 12 ++- 8 files changed, 151 insertions(+), 76 deletions(-) diff --git a/lib/spack/spack/bootstrap/config.py b/lib/spack/spack/bootstrap/config.py index e38c5669d9..6786bc0d3e 100644 --- a/lib/spack/spack/bootstrap/config.py +++ b/lib/spack/spack/bootstrap/config.py @@ -143,7 +143,9 @@ def _bootstrap_config_scopes() -> Sequence["spack.config.ConfigScope"]: def _add_compilers_if_missing() -> None: arch = spack.spec.ArchSpec.frontend_arch() if not spack.compilers.compilers_for_arch(arch): - new_compilers = spack.compilers.find_new_compilers() + new_compilers = spack.compilers.find_new_compilers( + mixed_toolchain=sys.platform == "darwin" + ) if new_compilers: spack.compilers.add_compilers_to_config(new_compilers, init_config=False) diff --git a/lib/spack/spack/cmd/compiler.py b/lib/spack/spack/cmd/compiler.py index 07006afc2c..76eb8d3150 100644 --- a/lib/spack/spack/cmd/compiler.py +++ b/lib/spack/spack/cmd/compiler.py @@ -31,6 +31,19 @@ def setup_parser(subparser): aliases=["add"], help="search the system for compilers to add to Spack configuration", ) + mixed_toolchain_group = find_parser.add_mutually_exclusive_group() + mixed_toolchain_group.add_argument( + "--mixed-toolchain", + action="store_true", + default=sys.platform == "darwin", + help="Allow mixed toolchains (for example: clang, clang++, gfortran)", + ) + mixed_toolchain_group.add_argument( + "--no-mixed-toolchain", + action="store_false", + dest="mixed_toolchain", + help="Do not allow mixed toolchains (for example: clang, clang++, gfortran)", + ) find_parser.add_argument("add_paths", nargs=argparse.REMAINDER) find_parser.add_argument( "--scope", @@ -86,7 +99,9 @@ def compiler_find(args): # Below scope=None because we want new compilers that don't appear # in any other configuration. - new_compilers = spack.compilers.find_new_compilers(paths, scope=None) + new_compilers = spack.compilers.find_new_compilers( + paths, scope=None, mixed_toolchain=args.mixed_toolchain + ) if new_compilers: spack.compilers.add_compilers_to_config(new_compilers, scope=args.scope, init_config=False) n = len(new_compilers) diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py index 3f9663d21e..6366fc321b 100644 --- a/lib/spack/spack/compilers/__init__.py +++ b/lib/spack/spack/compilers/__init__.py @@ -10,7 +10,7 @@ import itertools import multiprocessing.pool import os -from typing import Dict, List +from typing import Dict, List, Optional, Tuple import archspec.cpu @@ -21,6 +21,7 @@ import spack.compiler import spack.config import spack.error +import spack.operating_systems import spack.paths import spack.platforms import spack.spec @@ -223,13 +224,16 @@ def all_compiler_specs(scope=None, init_config=True): ] -def find_compilers(path_hints=None): +def find_compilers( + path_hints: Optional[List[str]] = None, *, mixed_toolchain=False +) -> List["spack.compiler.Compiler"]: """Return the list of compilers found in the paths given as arguments. Args: - path_hints (list or None): list of path hints where to look for. - A sensible default based on the ``PATH`` environment variable - will be used if the value is None + path_hints: list of path hints where to look for. A sensible default based on the ``PATH`` + environment variable will be used if the value is None + mixed_toolchain: allow mixing compilers from different toolchains if otherwise missing for + a certain language """ if path_hints is None: path_hints = get_path("PATH") @@ -250,7 +254,7 @@ def find_compilers(path_hints=None): finally: tp.close() - def valid_version(item): + def valid_version(item: Tuple[Optional[DetectVersionArgs], Optional[str]]) -> bool: value, error = item if error is None: return True @@ -262,25 +266,37 @@ def valid_version(item): pass return False - def remove_errors(item): + def remove_errors( + item: Tuple[Optional[DetectVersionArgs], Optional[str]] + ) -> DetectVersionArgs: value, _ = item + assert value is not None return value - return make_compiler_list(map(remove_errors, filter(valid_version, detected_versions))) + return make_compiler_list( + [remove_errors(detected) for detected in detected_versions if valid_version(detected)], + mixed_toolchain=mixed_toolchain, + ) -def find_new_compilers(path_hints=None, scope=None): +def find_new_compilers( + path_hints: Optional[List[str]] = None, + scope: Optional[str] = None, + *, + mixed_toolchain: bool = False, +): """Same as ``find_compilers`` but return only the compilers that are not already in compilers.yaml. Args: - path_hints (list or None): list of path hints where to look for. - A sensible default based on the ``PATH`` environment variable - will be used if the value is None - scope (str): scope to look for a compiler. If None consider the - merged configuration. + path_hints: list of path hints where to look for. A sensible default based on the ``PATH`` + environment variable will be used if the value is None + scope: scope to look for a compiler. If None consider the merged configuration. + mixed_toolchain: allow mixing compilers from different toolchains if otherwise missing for + a certain language """ - compilers = find_compilers(path_hints) + compilers = find_compilers(path_hints, mixed_toolchain=mixed_toolchain) + return select_new_compilers(compilers, scope) @@ -638,7 +654,9 @@ def all_compiler_types(): ) -def arguments_to_detect_version_fn(operating_system, paths): +def arguments_to_detect_version_fn( + operating_system: spack.operating_systems.OperatingSystem, paths: List[str] +) -> List[DetectVersionArgs]: """Returns a list of DetectVersionArgs tuples to be used in a corresponding function to detect compiler versions. @@ -646,8 +664,7 @@ def arguments_to_detect_version_fn(operating_system, paths): function by providing a method called with the same name. Args: - operating_system (spack.operating_systems.OperatingSystem): the operating system - on which we are looking for compilers + operating_system: the operating system on which we are looking for compilers paths: paths to search for compilers Returns: @@ -656,10 +673,10 @@ def arguments_to_detect_version_fn(operating_system, paths): compilers in this OS. """ - def _default(search_paths): - command_arguments = [] + def _default(search_paths: List[str]) -> List[DetectVersionArgs]: + command_arguments: List[DetectVersionArgs] = [] files_to_be_tested = fs.files_in(*search_paths) - for compiler_name in spack.compilers.supported_compilers_for_host_platform(): + for compiler_name in supported_compilers_for_host_platform(): compiler_cls = class_for_compiler_name(compiler_name) for language in ("cc", "cxx", "f77", "fc"): @@ -684,7 +701,9 @@ def _default(search_paths): return fn(paths) -def detect_version(detect_version_args): +def detect_version( + detect_version_args: DetectVersionArgs, +) -> Tuple[Optional[DetectVersionArgs], Optional[str]]: """Computes the version of a compiler and adds it to the information passed as input. @@ -693,8 +712,7 @@ def detect_version(detect_version_args): needs to be checked by the code dispatching the calls. Args: - detect_version_args (DetectVersionArgs): information on the - compiler for which we should detect the version. + detect_version_args: information on the compiler for which we should detect the version. Returns: A ``(DetectVersionArgs, error)`` tuple. If ``error`` is ``None`` the @@ -710,7 +728,7 @@ def _default(fn_args): path = fn_args.path # Get compiler names and the callback to detect their versions - callback = getattr(compiler_cls, "{0}_version".format(language)) + callback = getattr(compiler_cls, f"{language}_version") try: version = callback(path) @@ -736,13 +754,15 @@ def _default(fn_args): return fn(detect_version_args) -def make_compiler_list(detected_versions): +def make_compiler_list( + detected_versions: List[DetectVersionArgs], mixed_toolchain: bool = False +) -> List["spack.compiler.Compiler"]: """Process a list of detected versions and turn them into a list of compiler specs. Args: - detected_versions (list): list of DetectVersionArgs containing a - valid version + detected_versions: list of DetectVersionArgs containing a valid version + mixed_toolchain: allow mixing compilers from different toolchains if langauge is missing Returns: list: list of Compiler objects @@ -751,7 +771,7 @@ def make_compiler_list(detected_versions): sorted_compilers = sorted(detected_versions, key=group_fn) # Gather items in a dictionary by the id, name variation and language - compilers_d = {} + compilers_d: Dict[CompilerID, Dict[NameVariation, dict]] = {} for sort_key, group in itertools.groupby(sorted_compilers, key=group_fn): compiler_id, name_variation, language = sort_key by_compiler_id = compilers_d.setdefault(compiler_id, {}) @@ -760,7 +780,7 @@ def make_compiler_list(detected_versions): def _default_make_compilers(cmp_id, paths): operating_system, compiler_name, version = cmp_id - compiler_cls = spack.compilers.class_for_compiler_name(compiler_name) + compiler_cls = class_for_compiler_name(compiler_name) spec = spack.spec.CompilerSpec(compiler_cls.name, f"={version}") paths = [paths.get(x, None) for x in ("cc", "cxx", "f77", "fc")] # TODO: johnwparent - revist the following line as per discussion at: @@ -782,13 +802,14 @@ def _default_make_compilers(cmp_id, paths): getattr(variation, "suffix", None), ) - compilers = [] + # Flatten to a list of compiler id, primary variation and compiler dictionary + flat_compilers: List[Tuple[CompilerID, NameVariation, dict]] = [] for compiler_id, by_compiler_id in compilers_d.items(): ordered = sorted(by_compiler_id, key=sort_fn) selected_variation = ordered[0] selected = by_compiler_id[selected_variation] - # fill any missing parts from subsequent entries + # Fill any missing parts from subsequent entries (without mixing toolchains) for lang in ["cxx", "f77", "fc"]: if lang not in selected: next_lang = next( @@ -797,14 +818,63 @@ def _default_make_compilers(cmp_id, paths): if next_lang: selected[lang] = next_lang - operating_system, _, _ = compiler_id - make_compilers = getattr(operating_system, "make_compilers", _default_make_compilers) + flat_compilers.append((compiler_id, selected_variation, selected)) - compilers.extend(make_compilers(compiler_id, selected)) + # Next, fill out the blanks of missing compilers by creating a mixed toolchain (if requested) + if mixed_toolchain: + make_mixed_toolchain(flat_compilers) + + # Finally, create the compiler list + compilers = [] + for compiler_id, _, compiler in flat_compilers: + make_compilers = getattr(compiler_id.os, "make_compilers", _default_make_compilers) + compilers.extend(make_compilers(compiler_id, compiler)) return compilers +def make_mixed_toolchain(compilers: List[Tuple[CompilerID, NameVariation, dict]]) -> None: + """Add missing compilers across toolchains when they are missing for a particular language. + This currently only adds the most sensible gfortran to (apple)-clang if it doesn't have a + fortran compiler (no flang).""" + + # First collect the clangs that are missing a fortran compiler + clangs_without_flang = [ + (id, variation, compiler) + for id, variation, compiler in compilers + if id.compiler_name in ("clang", "apple-clang") + and "f77" not in compiler + and "fc" not in compiler + ] + if not clangs_without_flang: + return + + # Filter on GCCs with fortran compiler + gccs_with_fortran = [ + (id, variation, compiler) + for id, variation, compiler in compilers + if id.compiler_name == "gcc" and "f77" in compiler and "fc" in compiler + ] + + # Sort these GCCs by "best variation" (no prefix / suffix first) + gccs_with_fortran.sort( + key=lambda x: (getattr(x[1], "prefix", None), getattr(x[1], "suffix", None)) + ) + + # Attach the optimal GCC fortran compiler to the clangs that don't have one + for clang_id, _, clang_compiler in clangs_without_flang: + gcc_compiler = next( + (gcc[2] for gcc in gccs_with_fortran if gcc[0].os == clang_id.os), None + ) + + if not gcc_compiler: + continue + + # Update the fc / f77 entries + clang_compiler["f77"] = gcc_compiler["f77"] + clang_compiler["fc"] = gcc_compiler["fc"] + + def is_mixed_toolchain(compiler): """Returns True if the current compiler is a mixed toolchain, False otherwise. diff --git a/lib/spack/spack/compilers/aocc.py b/lib/spack/spack/compilers/aocc.py index a642960b7d..326522c93c 100644 --- a/lib/spack/spack/compilers/aocc.py +++ b/lib/spack/spack/compilers/aocc.py @@ -5,7 +5,6 @@ import os import re -import sys import llnl.util.lang @@ -114,17 +113,6 @@ def extract_version_from_output(cls, output): return ".".join(match.groups()) return "unknown" - @classmethod - def fc_version(cls, fortran_compiler): - if sys.platform == "darwin": - return cls.default_version("clang") - - return cls.default_version(fortran_compiler) - - @classmethod - def f77_version(cls, f77): - return cls.fc_version(f77) - @property def stdcxx_libs(self): return ("-lstdc++",) diff --git a/lib/spack/spack/compilers/clang.py b/lib/spack/spack/compilers/clang.py index a9356227de..71837bfe5e 100644 --- a/lib/spack/spack/compilers/clang.py +++ b/lib/spack/spack/compilers/clang.py @@ -5,7 +5,6 @@ import os import re -import sys import llnl.util.lang @@ -39,10 +38,10 @@ class Clang(Compiler): cxx_names = ["clang++"] # Subclasses use possible names of Fortran 77 compiler - f77_names = ["flang", "gfortran", "xlf_r"] + f77_names = ["flang"] # Subclasses use possible names of Fortran 90 compiler - fc_names = ["flang", "gfortran", "xlf90_r"] + fc_names = ["flang"] version_argument = "--version" @@ -182,16 +181,3 @@ def extract_version_from_output(cls, output): if match: ver = match.group(match.lastindex) return ver - - @classmethod - def fc_version(cls, fc): - # We could map from gcc/gfortran version to clang version, but on macOS - # we normally mix any version of gfortran with any version of clang. - if sys.platform == "darwin": - return cls.default_version("clang") - else: - return cls.default_version(fc) - - @classmethod - def f77_version(cls, f77): - return cls.fc_version(f77) diff --git a/lib/spack/spack/test/cmd/compiler.py b/lib/spack/spack/test/cmd/compiler.py index 9bc2049fdf..1cea72d3b2 100644 --- a/lib/spack/spack/test/cmd/compiler.py +++ b/lib/spack/spack/test/cmd/compiler.py @@ -4,12 +4,14 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import os import shutil -import sys import pytest +import spack.cmd.compiler import spack.compilers import spack.main +import spack.spec +import spack.util.pattern import spack.version compiler = spack.main.SpackCommand("compiler") @@ -146,7 +148,7 @@ def test_compiler_add(mutable_config, mock_packages, mock_executable): compilers_before_find = set(spack.compilers.all_compiler_specs()) args = spack.util.pattern.Bunch( - all=None, compiler_spec=None, add_paths=[str(root_dir)], scope=None + all=None, compiler_spec=None, add_paths=[str(root_dir)], scope=None, mixed_toolchain=False ) spack.cmd.compiler.compiler_find(args) compilers_after_find = set(spack.compilers.all_compiler_specs()) @@ -159,10 +161,15 @@ def test_compiler_add(mutable_config, mock_packages, mock_executable): @pytest.mark.not_on_windows("Cannot execute bash script on Windows") @pytest.mark.regression("17590") -def test_compiler_find_mixed_suffixes(no_compilers_yaml, working_env, compilers_dir): +@pytest.mark.parametrize("mixed_toolchain", [True, False]) +def test_compiler_find_mixed_suffixes( + mixed_toolchain, no_compilers_yaml, working_env, compilers_dir +): """Ensure that we'll mix compilers with different suffixes when necessary.""" os.environ["PATH"] = str(compilers_dir) - output = compiler("find", "--scope=site") + output = compiler( + "find", "--scope=site", "--mixed-toolchain" if mixed_toolchain else "--no-mixed-toolchain" + ) assert "clang@11.0.0" in output assert "gcc@8.4.0" in output @@ -176,9 +183,8 @@ def test_compiler_find_mixed_suffixes(no_compilers_yaml, working_env, compilers_ assert clang["paths"] == { "cc": str(compilers_dir / "clang"), "cxx": str(compilers_dir / "clang++"), - # we only auto-detect mixed clang on macos - "f77": gfortran_path if sys.platform == "darwin" else None, - "fc": gfortran_path if sys.platform == "darwin" else None, + "f77": gfortran_path if mixed_toolchain else None, + "fc": gfortran_path if mixed_toolchain else None, } assert gcc["paths"] == { diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index 91ed9dd172..20bb886b10 100755 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -756,7 +756,7 @@ _spack_compiler() { _spack_compiler_find() { if $list_options then - SPACK_COMPREPLY="-h --help --scope" + SPACK_COMPREPLY="-h --help --mixed-toolchain --no-mixed-toolchain --scope" else SPACK_COMPREPLY="" fi @@ -765,7 +765,7 @@ _spack_compiler_find() { _spack_compiler_add() { if $list_options then - SPACK_COMPREPLY="-h --help --scope" + SPACK_COMPREPLY="-h --help --mixed-toolchain --no-mixed-toolchain --scope" else SPACK_COMPREPLY="" fi diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish index 7ea1d18484..769768c04c 100755 --- a/share/spack/spack-completion.fish +++ b/share/spack/spack-completion.fish @@ -1045,18 +1045,26 @@ complete -c spack -n '__fish_spack_using_command compiler' -s h -l help -f -a he complete -c spack -n '__fish_spack_using_command compiler' -s h -l help -d 'show this help message and exit' # spack compiler find -set -g __fish_spack_optspecs_spack_compiler_find h/help scope= +set -g __fish_spack_optspecs_spack_compiler_find h/help mixed-toolchain no-mixed-toolchain scope= complete -c spack -n '__fish_spack_using_command compiler find' -s h -l help -f -a help complete -c spack -n '__fish_spack_using_command compiler find' -s h -l help -d 'show this help message and exit' +complete -c spack -n '__fish_spack_using_command compiler find' -l mixed-toolchain -f -a mixed_toolchain +complete -c spack -n '__fish_spack_using_command compiler find' -l mixed-toolchain -d 'Allow mixed toolchains (for example: clang, clang++, gfortran)' +complete -c spack -n '__fish_spack_using_command compiler find' -l no-mixed-toolchain -f -a mixed_toolchain +complete -c spack -n '__fish_spack_using_command compiler find' -l no-mixed-toolchain -d 'Do not allow mixed toolchains (for example: clang, clang++, gfortran)' complete -c spack -n '__fish_spack_using_command compiler find' -l scope -r -f -a '_builtin defaults system site user command_line' complete -c spack -n '__fish_spack_using_command compiler find' -l scope -r -d 'configuration scope to modify' # spack compiler add -set -g __fish_spack_optspecs_spack_compiler_add h/help scope= +set -g __fish_spack_optspecs_spack_compiler_add h/help mixed-toolchain no-mixed-toolchain scope= complete -c spack -n '__fish_spack_using_command compiler add' -s h -l help -f -a help complete -c spack -n '__fish_spack_using_command compiler add' -s h -l help -d 'show this help message and exit' +complete -c spack -n '__fish_spack_using_command compiler add' -l mixed-toolchain -f -a mixed_toolchain +complete -c spack -n '__fish_spack_using_command compiler add' -l mixed-toolchain -d 'Allow mixed toolchains (for example: clang, clang++, gfortran)' +complete -c spack -n '__fish_spack_using_command compiler add' -l no-mixed-toolchain -f -a mixed_toolchain +complete -c spack -n '__fish_spack_using_command compiler add' -l no-mixed-toolchain -d 'Do not allow mixed toolchains (for example: clang, clang++, gfortran)' complete -c spack -n '__fish_spack_using_command compiler add' -l scope -r -f -a '_builtin defaults system site user command_line' complete -c spack -n '__fish_spack_using_command compiler add' -l scope -r -d 'configuration scope to modify'