node_flag_propagation_candidate: special cases for multiple node attrs

Compiler flag test still fail

lib/spack/spack/solver/asp.py

@@ -948,6 +948,12 @@ def visit(node):
         self.control.load(os.path.join(parent_dir, "display.lp"))
         if not setup.concretize_everything:
             self.control.load(os.path.join(parent_dir, "when_possible.lp"))
+
+        for spec in specs:
+            if self._compiler_flags_has_propagation(spec.compiler_flags):
+                self.control.load(os.path.join(parent_dir, "propagation.lp"))
+                break
+
         timer.stop("load")
 
         # Grounding is the first step in the solve -- it turns our facts
@@ -1028,6 +1034,12 @@ def on_model(model):
 
         return result, timer, self.control.statistics
 
+    def _compiler_flags_has_propagation(self, flags):
+        for _, flag_vals in flags.items():
+            if any(val.propagate for val in flag_vals):
+                return True
+        return False
+
 
 class ConcreteSpecsByHash(collections.abc.Mapping):
     """Mapping containing concrete specs keyed by DAG hash.
@@ -1921,7 +1933,7 @@ class Head:
             node_compiler_version = fn.attr("node_compiler_version_set")
             node_flag = fn.attr("node_flag_set")
             node_flag_source = fn.attr("node_flag_source")
-            node_flag_propagate = fn.attr("node_flag_propagate")
+            node_flag_propagation_candidate = fn.attr("node_flag_propagation_candidate")
             variant_propagation_candidate = fn.attr("variant_propagation_candidate")
 
         class Body:
@@ -1935,7 +1947,7 @@ class Body:
             node_compiler_version = fn.attr("node_compiler_version")
             node_flag = fn.attr("node_flag")
             node_flag_source = fn.attr("node_flag_source")
-            node_flag_propagate = fn.attr("node_flag_propagate")
+            node_flag_propagation_candidate = fn.attr("node_flag_propagation_candidate")
             variant_propagation_candidate = fn.attr("variant_propagation_candidate")
 
         f = Body if body else Head
@@ -2020,7 +2032,9 @@ class Body:
                 clauses.append(f.node_flag(spec.name, flag_type, flag))
                 clauses.append(f.node_flag_source(spec.name, flag_type, spec.name))
                 if not spec.concrete and flag.propagate is True:
-                    clauses.append(f.node_flag_propagate(spec.name, flag_type))
+                    clauses.append(
+                        f.node_flag_propagation_candidate(spec.name, flag_type, flag, spec.name)
+                    )
 
         # dependencies
         if spec.concrete:

lib/spack/spack/solver/concretize.lp

@@ -318,9 +318,11 @@ trigger_condition_holds(ID, RequestorNode) :-
   attr(Name, node(X, A1))             : condition_requirement(ID, Name, A1),             condition_nodes(ID, PackageNode, node(X, A1));
   attr(Name, node(X, A1), A2)         : condition_requirement(ID, Name, A1, A2),         condition_nodes(ID, PackageNode, node(X, A1));
   attr(Name, node(X, A1), A2, A3)     : condition_requirement(ID, Name, A1, A2, A3),     condition_nodes(ID, PackageNode, node(X, A1)), not multiple_nodes_attribute(Name);
-  attr(Name, node(X, A1), A2, A3, A4) : condition_requirement(ID, Name, A1, A2, A3, A4), condition_nodes(ID, PackageNode, node(X, A1));
+  attr(Name, node(X, A1), A2, A3, A4) : condition_requirement(ID, Name, A1, A2, A3, A4), condition_nodes(ID, PackageNode, node(X, A1)), not multiple_nodes_attribute(Name);
   % Special cases
   attr("node_flag_source", node(X, A1), A2, node(Y, A3)) : condition_requirement(ID, "node_flag_source", A1, A2, A3), condition_nodes(ID, PackageNode, node(X, A1)), condition_nodes(ID, PackageNode, node(Y, A3));
+  attr("node_flag_propagation_candidate", node(X, A1), A2, A3, node(Y, A4)) : condition_reuqirement(ID, "node_flag_propagation_candidate", A1, A2, A3, A4), condition_nodes(ID, PackageNode, node(X, A1)), condition_nodes(ID, PackageNode, node(Y, A4));
+  % if the structure of the program ever could change so that node_flag_source can be in a condition, we will need a case for it here.
   not cannot_hold(ID, PackageNode).
 
 condition_holds(ConditionID, node(X, Package))
@@ -373,6 +375,15 @@ attr("node_flag_source", node(X, A1), A2, node(Y, A3))
      imposed_constraint(ID, "node_flag_source", A1, A2, A3),
      condition_set(node(Y, A3), node(X, A1)).
 
+% For node flag propagation, we need to look at the condition_set of the source, since it is the ancestor
+% of the package on which we want to impose the constraint
+attr("node_flag_propagation_candidate", node(X, A1), A2, A3, node(Y, A4))
+  :- impose(ID, node(X, A1)),
+     imposed_constraint(ID, "node_flag_propagation_candidate", A1, A2, A3, A4),
+     condition_set(node(Y, A4), node(X, A1)).
+
+% if the structure of the program ever could change so that node_flag_source can be in a condition, we will need a case for it here.
+
 % Provider set is relevant only for literals, since it's the only place where `^[virtuals=foo] bar`
 % might appear in the HEAD of a rule
 attr("provider_set", node(min_dupe_id, Provider), node(min_dupe_id, Virtual))
@@ -1212,46 +1223,16 @@ error(100, "Compiler {1}@{2} requested for {0} cannot be found. Set install_miss
 % Compiler flags
 %-----------------------------------------------------------------------------
 
-% propagate flags when compiler match
-can_inherit_flags(PackageNode, DependencyNode, FlagType)
-  :- same_compiler(PackageNode, DependencyNode),
-     not attr("node_flag_set", DependencyNode, FlagType, _),
-     flag_type(FlagType).
-
 same_compiler(PackageNode, DependencyNode)
   :- depends_on(PackageNode, DependencyNode),
      node_compiler(PackageNode, CompilerID),
      node_compiler(DependencyNode, CompilerID),
      compiler_id(CompilerID).
 
-node_flag_inherited(DependencyNode, FlagType, Flag)
-  :- attr("node_flag_set", PackageNode, FlagType, Flag),
-     can_inherit_flags(PackageNode, DependencyNode, FlagType),
-     attr("node_flag_propagate", PackageNode, FlagType).
-
-% Ensure propagation
-:- node_flag_inherited(PackageNode, FlagType, Flag),
-   can_inherit_flags(PackageNode, DependencyNode, FlagType),
-   attr("node_flag_propagate", PackageNode, FlagType).
-
-error(100, "{0} and {1} cannot both propagate compiler flags '{2}' to {3}", Source1, Source2, Package, FlagType) :-
-  depends_on(Source1, Package),
-  depends_on(Source2, Package),
-  attr("node_flag_propagate", Source1, FlagType),
-  attr("node_flag_propagate", Source2, FlagType),
-  can_inherit_flags(Source1, Package, FlagType),
-  can_inherit_flags(Source2, Package, FlagType),
-  Source1 < Source2.
-
 % remember where flags came from
 attr("node_flag_source", PackageNode, FlagType, PackageNode)
   :- attr("node_flag_set", PackageNode, FlagType, _).
 
-attr("node_flag_source", DependencyNode, FlagType, Q)
-  :- attr("node_flag_source", PackageNode, FlagType, Q),
-     node_flag_inherited(DependencyNode, FlagType, _),
-     attr("node_flag_propagate", PackageNode, FlagType).
-
 % compiler flags from compilers.yaml are put on nodes if compiler matches
 attr("node_flag", PackageNode, FlagType, Flag)
   :- compiler_flag(CompilerID, FlagType, Flag),
@@ -1272,7 +1253,30 @@ attr("node_flag_compiler_default", PackageNode)
 
 % if a flag is set to something or inherited, it's included
 attr("node_flag", PackageNode, FlagType, Flag) :- attr("node_flag_set", PackageNode, FlagType, Flag).
-attr("node_flag", PackageNode, FlagType, Flag) :- node_flag_inherited(PackageNode, FlagType, Flag).
+
+attr("node_flag_propagate", PackageNode, FlagType, Flag, Source) :-
+  attr("node_flag_propagation_candidate", PackageNode, FlagType, Flag, Source),
+  not attr("node_flag_set", PackageNode, FlagType, _).
+
+% source is a node() attribute
+attr("node_flag_propagation_candidate", PackageNode, FlagType, Flag, Source) :-
+  same_compiler(ParentNode, PackageNode),
+  attr("node_flag_propagation_candidate", ParentNode, FlagType, _, Source),
+  attr("node_flag", Source, FlagType, Flag),
+  flag_type(FlagType).
+
+error(100, "{0} and {1} cannot both propagate compiler flags '{2}' to {3}", Source1, Source2, PackageNode, FlagType) :-
+  same_compiler(Source1, PackageNode),
+  same_compiler(Source2, PackageNode),
+  attr("node_flag_propagate", _, FlagType, _, Source1),
+  attr("node_flag_propagate", _, FlagType, _, Source2),
+  Source1 < Source2.
+
+attr("node_flag_source", PackageNode, FlagType, Q)
+  :- attr("node_flag_propagate", PackageNode, FlagType, _, Q).
+
+attr("node_flag", PackageNode, FlagType, Flag) :- attr("node_flag_propagate", PackageNode, FlagType, Flag, _).
+
 
 % if no node flags are set for a type, there are no flags.
 attr("no_flags", PackageNode, FlagType)

lib/spack/spack/solver/propagation.lp

@@ -0,0 +1,31 @@
+% Copyright 2013-2023 Lawrence Livermore National Security, LLC and other
+% Spack Project Developers. See the top-level COPYRIGHT file for details.
+%
+% SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+%=============================================================================
+% TODO: Later
+%=============================================================================
+
+% propagate flags when compiler match
+attr("node_flag_propagate", PackageNode, FlagType, Flag, Source) :- % source is a node() attribute
+  attr("node_flag_propagation_candidate", PackageNode, FlagType, Flag, Source),
+  not attr("node_flag_set", PackageNode, FlagType, _).
+
+attr("node_flag_propagation_candidate", PackageNode, FlagType, Flag, Source) :- % source is a node() attribute
+  same_compiler(ParentNode, PackageNode),
+  attr("node_flag_propagation_candidate", ParentNode, FlagType, _, Source),
+  attr("node_flag", Source, FlagType, Flag),
+  flag_type(FlagType).
+
+error(100, "{0} and {1} cannot both propagate compiler flags '{2}' to {3}", Source1, Source2, PackageNode, FlagType) :-
+  same_compiler(Source1, PackageNode),
+  same_compiler(Source2, PackageNode),
+  attr("node_flag_propagate", _, FlagType, _, Source1),
+  attr("node_flag_propagate", _, FlagType, _, Source2),
+  Source1 < Source2.
+
+attr("node_flag_source", PackageNode, FlagType, Q)
+  :- attr("node_flag_propagate", PackageNode, FlagType, _, Q).
+
+attr("node_flag", PackageNode, FlagType, Flag) :- attr("node_flag_propagate", PackageNode, FlagType, Flag, _).

lib/spack/spack/test/concretize.py

@@ -353,10 +353,21 @@ def test_compiler_flags_differ_identical_compilers(self):
         "Optional compiler propagation isn't deprecated for original concretizer"
     )
     def test_concretize_compiler_flag_propagate(self):
-        spec = Spec("hypre cflags=='-g' ^openblas")
+        spec = Spec("callpath cflags=='-g'")
         spec.concretize()
 
-        assert spec.satisfies("^openblas cflags='-g'")
+        for dep in spec.traverse():
+            assert dep.satisfies("cflags='-g'")
+
+    @pytest.mark.only_clingo(
+        "Optional compiler propagation isn't deprecated for original concretizer"
+    )
+    def test_concretize_non_root_compiler_flag_propagate(self):
+        spec = Spec("callpath ^dyninst cflags=='-g'")
+        spec.concretize()
+
+        assert spec.satisfies("^libdwarf cflags='-g'")
+        assert spec.satisfies("^libelf cflags='-g'")
 
     @pytest.mark.only_clingo(
         "Optional compiler propagation isn't deprecated for original concretizer"
@@ -365,17 +376,30 @@ def test_concretize_compiler_flag_does_not_propagate(self):
         spec = Spec("hypre cflags='-g' ^openblas")
         spec.concretize()
 
-        assert not spec.satisfies("^openblas cflags='-g'")
+        for dep in spec.traverse(root=False):
+            assert not dep.satisfies("cflags='-g'")
 
     @pytest.mark.only_clingo(
         "Optional compiler propagation isn't deprecated for original concretizer"
     )
     def test_concretize_propagate_compiler_flag_not_passed_to_dependent(self):
-        spec = Spec("hypre cflags=='-g' ^openblas cflags='-O3'")
+        spec = Spec("callpath cflags=='-g' ^dyninst cflags='-O3'")
         spec.concretize()
 
         assert set(spec.compiler_flags["cflags"]) == set(["-g"])
-        assert spec.satisfies("^openblas cflags='-O3'")
+        assert spec.satisfies("^dyninst cflags='-O3'")
+        assert spec.satisfies("^libelf cflags='-g'")
+
+    @pytest.mark.only_clingo(
+        "Optional compiler propagation isn't deprecated for original concretizer"
+    )
+    def test_concretize_propagate_specified_compiler_flag(self):
+        spec = Spec("callpath cflags=='-g' cxxflags='-O3'")
+        spec.concretize()
+
+        for dep in spec.traverse(root=False):
+            assert dep.satisfies("cflags='-g'")
+            assert not dep.satisfies("cxxflags='-O3'")
 
     def test_mixing_compilers_only_affects_subdag(self):
         spack.config.set("packages:all:compiler", ["clang", "gcc"])