Improve default selection of compilers

This splits optimization on providers, and
puts selection of default compilers at lower
priority wrt selection of e.g. mpi or lapack
provider.

This is so that, on systems where 2 or more
compilers are used (e.g. macOS), clingo would
not make weird choices for mpi or lapack, in
order to minimize compiler penalty due to
using different compilers on the same node.

lib/spack/spack/modules/lmod.py

@@ -6,6 +6,7 @@
 import itertools
 import os
 import pathlib
+import warnings
 from typing import Dict, List, Optional, Tuple
 
 import llnl.util.filesystem as fs
@@ -102,15 +103,19 @@ class LmodConfiguration(BaseConfiguration):
     def __init__(self, spec: spack.spec.Spec, module_set_name: str, explicit: bool) -> None:
         super().__init__(spec, module_set_name, explicit)
 
-        # FIXME (compiler as nodes): make this a bit more robust
         candidates = collections.defaultdict(list)
         for node in spec.traverse(deptype=("link", "run")):
             candidates["c"].extend(node.dependencies(virtuals=("c",)))
             candidates["cxx"].extend(node.dependencies(virtuals=("c",)))
 
-        # FIXME (compiler as nodes): decide what to do when we have more than one C compiler
+        if candidates["c"]:
-        if candidates["c"] and len(set(candidates["c"])) == 1:
             self.compiler = candidates["c"][0]
+            if len(set(candidates["c"])) > 1:
+                warnings.warn(
+                    f"{spec.short_spec} uses more than one compiler, and might not fit the "
+                    f"LMod hierarchy. Using {self.compiler.short_spec} as the LMod compiler."
+                )
+
         elif not candidates["c"]:
             self.compiler = None
 

lib/spack/spack/solver/concretize.lp

@@ -1246,10 +1246,6 @@ error(100, "Cannot propagate the variant '{0}' from the package: {1} because pac
 % 1. The same flag type is not set on this node
 % 2. This node has the same compilers as the propagation source
 
-compiler_penalty(PackageNode, C-1) :-
-   C = #count { CompilerNode : node_compiler(PackageNode, CompilerNode) },
-   node_compiler(PackageNode, _).
-
 node_compiler(node(X, Package), node(Y, Compiler)) :-
      attr("virtual_on_edge", node(X, Package), node(Y, Compiler), Language),
      attr("version", node(Y, Compiler), Version),
@@ -1295,6 +1291,12 @@ compiler_used_as_a_library(node(X, Child), Hash) :-
   compiler_package(Child), % Used to restrict grounding for this rule
   attr("depends_on", _, node(X, Child), "link").
 
+% If a compiler is used for C on a package, it must provide C++ too, if need be, and vice-versa
+:-  attr("virtual_on_edge", PackageNode, CompilerNode1, "c"),
+    attr("virtual_on_edge", PackageNode, CompilerNode2, "cxx"),
+    CompilerNode1 != CompilerNode2.
+
+
 %-----------------------------------------------------------------------------
 % Runtimes
 %-----------------------------------------------------------------------------
@@ -1660,9 +1662,9 @@ opt_criterion(60, "preferred providers for roots").
 #minimize{ 0@260: #true }.
 #minimize{ 0@60: #true }.
 #minimize{
-    Weight@60+Priority,ProviderNode,Virtual
+    Weight@60+Priority,ProviderNode,X,Virtual
-    : provider_weight(ProviderNode, Virtual, Weight),
+    : provider_weight(ProviderNode, node(X, Virtual), Weight),
-      attr("root", ProviderNode),
+      attr("root", ProviderNode), not language(Virtual),
       build_priority(ProviderNode, Priority)
 }.
 
@@ -1687,36 +1689,50 @@ opt_criterion(50, "number of non-default variants (non-roots)").
       build_priority(PackageNode, Priority)
 }.
 
-% Minimize the ids of the providers, i.e. use as much as
+% Minimize the weights of the providers, i.e. use as much as
-% possible the first providers
+% possible the most preferred providers
-opt_criterion(48, "number of duplicate virtuals needed").
+opt_criterion(48, "preferred providers (non-roots)").
 #minimize{ 0@248: #true }.
 #minimize{ 0@48: #true }.
 #minimize{
-    Weight@48+Priority,ProviderNode,Virtual
+    Weight@48+Priority,ProviderNode,X,Virtual
-    : provider(ProviderNode, node(Weight, Virtual)),
+    : provider_weight(ProviderNode, node(X, Virtual), Weight),
+      not attr("root", ProviderNode), not language(Virtual),
       build_priority(ProviderNode, Priority)
 }.
 
 % Minimize the number of compilers used on nodes
-opt_criterion(49, "minimize the number of compilers used on the same node").
+
-#minimize{ 0@249: #true }.
+compiler_penalty(PackageNode, C-1) :-
-#minimize{ 0@49: #true }.
+   C = #count { CompilerNode : node_compiler(PackageNode, CompilerNode) },
+   node_compiler(PackageNode, _).
+
+opt_criterion(46, "number of compilers used on the same node").
+#minimize{ 0@246: #true }.
+#minimize{ 0@46: #true }.
 #minimize{
-    Penalty@49+Priority,PackageNode
+    Penalty@46+Priority,PackageNode
-    : compiler_penalty(PackageNode, Penalty),
+    : compiler_penalty(PackageNode, Penalty), build_priority(PackageNode, Priority)
-    build_priority(PackageNode, Priority)
 }.
 
-% Minimize the weights of the providers, i.e. use as much as
+% Minimize the ids of the providers, i.e. use as much as
-% possible the most preferred providers
+% possible the first providers
-opt_criterion(45, "preferred providers (non-roots)").
+opt_criterion(45, "number of duplicate virtuals needed").
 #minimize{ 0@245: #true }.
 #minimize{ 0@45: #true }.
 #minimize{
     Weight@45+Priority,ProviderNode,Virtual
-    : provider_weight(ProviderNode, Virtual, Weight),
+    : provider(ProviderNode, node(Weight, Virtual)),
-      not attr("root", ProviderNode),
+      build_priority(ProviderNode, Priority)
+}.
+
+opt_criterion(40, "preferred compilers").
+#minimize{ 0@240: #true }.
+#minimize{ 0@40: #true }.
+#minimize{
+    Weight@40+Priority,ProviderNode,X,Virtual
+    : provider_weight(ProviderNode, node(X, Virtual), Weight),
+      language(Virtual),
       build_priority(ProviderNode, Priority)
 }.
 

lib/spack/spack/test/concretization/core.py

@@ -741,12 +741,51 @@ def test_virtual_is_fully_expanded_for_mpileaks(self):
         assert all(not d.dependencies(name="mpi") for d in spec.traverse())
         assert all(x in spec for x in ("zmpi", "mpi"))
 
-    @pytest.mark.parametrize("compiler_str", ["%clang", "%gcc", "%gcc@10.2.1", "%clang@:15.0.0"])
+    @pytest.mark.parametrize(
-    def test_compiler_inheritance(self, compiler_str):
+        "spec_str,expected,not_expected",
-        spec_str = f"mpileaks {compiler_str}"
+        [
+            # clang only provides C, and C++ compilers, while gcc has also fortran
+            #
+            # If we ask mpileaks%clang, then %gcc must be used for fortran, and since
+            # %gcc is preferred to clang in config, it will be used for most nodes
+            (
+                "mpileaks %clang",
+                {"mpileaks": "%clang", "libdwarf": "%gcc", "libelf": "%gcc"},
+                {"libdwarf": "%clang", "libelf": "%clang"},
+            ),
+            (
+                "mpileaks %clang@:15.0.0",
+                {"mpileaks": "%clang", "libdwarf": "%gcc", "libelf": "%gcc"},
+                {"libdwarf": "%clang", "libelf": "%clang"},
+            ),
+            (
+                "mpileaks %gcc",
+                {"mpileaks": "%gcc", "libdwarf": "%gcc", "libelf": "%gcc"},
+                {"mpileaks": "%clang", "libdwarf": "%clang", "libelf": "%clang"},
+            ),
+            (
+                "mpileaks %gcc@10.2.1",
+                {"mpileaks": "%gcc", "libdwarf": "%gcc", "libelf": "%gcc"},
+                {"mpileaks": "%clang", "libdwarf": "%clang", "libelf": "%clang"},
+            ),
+            # dyninst doesn't require fortran, so %clang is propagated
+            (
+                "dyninst %clang",
+                {"dyninst": "%clang", "libdwarf": "%clang", "libelf": "%clang"},
+                {"libdwarf": "%gcc", "libelf": "%gcc"},
+            ),
+        ],
+    )
+    def test_compiler_inheritance(self, spec_str, expected, not_expected):
+        """Spack tries to propagate compilers as much as possible, but prefers using a single
+        toolchain on a node, rather than mixing them.
+        """
         spec = spack.concretize.concretize_one(spec_str)
-        assert spec["libdwarf"].satisfies(compiler_str)
+        for name, constraint in expected.items():
-        assert spec["libelf"].satisfies(compiler_str)
+            assert spec[name].satisfies(constraint)
+
+        for name, constraint in not_expected.items():
+            assert not spec[name].satisfies(constraint)
 
     def test_external_package(self):
         """Tests that an external is preferred, if present, and that it does not

lib/spack/spack/test/env.py

@@ -921,3 +921,31 @@ def test_environment_from_name_or_dir(mock_packages, mutable_mock_env_path, tmp_
 
     with pytest.raises(ev.SpackEnvironmentError, match="no such environment"):
         _ = ev.environment_from_name_or_dir("fake-env")
+
+
+def test_using_multiple_compilers_on_a_node_is_discouraged(
+    tmp_path, mutable_config, mock_packages
+):
+    """Tests that when we specify %<compiler> Spack tries to use that compiler for all the
+    languages needed by that node.
+    """
+    manifest = tmp_path / "spack.yaml"
+    manifest.write_text(
+        """\
+spack:
+  specs:
+    - mpileaks%clang ^mpich%gcc
+  concretizer:
+    unify: true
+"""
+    )
+    with ev.Environment(tmp_path) as e:
+        e.concretize()
+        mpileaks = e.concrete_roots()[0]
+
+    assert not mpileaks.satisfies("%gcc") and mpileaks.satisfies("%clang")
+    assert len(mpileaks.dependencies(virtuals=("c", "cxx"))) == 1
+
+    mpich = mpileaks["mpich"]
+    assert mpich.satisfies("%gcc") and not mpich.satisfies("%clang")
+    assert len(mpich.dependencies(virtuals=("c", "cxx"))) == 1