Add `intel-oneapi-runtime`, allow injecting virtual dependencies by alalazo · Pull Request #42062 · spack/spack

alalazo · 2024-01-12T09:59:22Z

This PR adds:

A new runtime for %oneapi compilers, called intel-oneapi-runtime
Information to both gcc-runtime and intel-oneapi-runtime, to ensure that we don't mix compilers using different soname for either libgfortran or libifcore

To do so, the following internal mechanisms have been implemented:

Possibility to inject virtual dependencies from the runtime_constraints callback on packages

Information has been added to gcc-runtime to provide the correct soname under different conditions on its %gcc.

Rules injected into the solver looks like:

Injection of virtual dependency

% Add a dependency on 'gfortran@5' for nodes compiled with gcc@=13.2.0 and using the 'fortran' language
attr("dependency_holds", node(ID, Package), "gfortran", "link") :-
  attr("node", node(ID, Package)),
  attr("node_compiler", node(ID, Package), "gcc"),
  attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"),
  not external(node(ID, Package)),
  not runtime(Package),
  attr("language", node(ID, Package), "fortran").

attr("virtual_node", node(RuntimeID, "gfortran")) :-
  attr("depends_on", node(ID, Package), ProviderNode, "link"),
  provider(ProviderNode, node(RuntimeID, "gfortran")),
  attr("node", node(ID, Package)),
  attr("node_compiler", node(ID, Package), "gcc"),
  attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"),
  not external(node(ID, Package)),
  not runtime(Package),
  attr("language", node(ID, Package), "fortran").

attr("node_version_satisfies", node(RuntimeID, "gfortran"), "5") :-
  attr("depends_on", node(ID, Package), ProviderNode, "link"),
  provider(ProviderNode, node(RuntimeID, "gfortran")),
  attr("node", node(ID, Package)),
  attr("node_compiler", node(ID, Package), "gcc"),
  attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"),
  not external(node(ID, Package)),
  not runtime(Package),
  attr("language", node(ID, Package), "fortran").

spackbot-app · 2024-01-12T09:59:37Z

@byrnHDF can you review this PR?

This PR modifies the following package(s), for which you are listed as a maintainer:

gcc
gcc-runtime
hdf5

lib/spack/spack/solver/display.lp

alalazo · 2024-01-23T10:49:49Z

First benchmark as of 661f908

Spack: 0.22.0.dev0 (2f935b325f80c159655c757837c218aa7e750760)
Python: 3.11.5
Platform: linux-ubuntu20.04-icelake
Concretizer: clingo

develop: 6681346
PR: 661f908

radiuss.develop.csv
radiuss.pr.csv
radiuss.txt

Adding more rules to the runtimes impacts the solve time

alalazo · 2024-01-23T10:57:34Z

On the other hand, with 661f908 we have:

spack:
  specs:
  - hdf5%oneapi+fortran~mpi
  packages:
    cmake:
      require: "%gcc"
  view: true
  concretizer:
    unify: true

Not that in the graph we introduced intel-oneapi-runtime, with a dependency on gcc-runtime (which in principle allows the selection of the underlying gcc).

alalazo · 2024-01-23T20:40:31Z

@spackbot run pipeline

spackbot-app · 2024-01-23T20:40:40Z

I've started that pipeline for you!

alalazo · 2024-01-24T05:53:34Z

@spackbot run pipeline

The intel-oneapi-runtime depends on gcc-runtime at link time. This dependency can be used later to dynamically select which GCC to use with the %oneapi compiler. intel-oneapi-runtime provides ifort for packages that need fortran

Also: provides sycl, add libiomp5 to the list of libraries

For now maintain a CompilerSpec, since it allows to avoid a double conversion from compiler name to pkg name and vice-versa

tgamblin

@alalazo I'm merging this as it is far enough along that I think we can merge without hurting anything, and it can be improved once compilers are full packages.

I am still not sure that the interface is 100% yet. Specifically, so we don't forget:

The deptype of a depends_on() on a language either shouldn't matter or should just be default. The deptype(s) is (are) entirely up to the compiler package... not the caller, so I don't see any point in forcing an arbitrary convention. Omitting is better.
pkg("*").depends_on() shouldn't take a language parameter and we should not make languages special. They're just virtuals like any other... all we're adding is a means for dependencies to inject additional dependencies on their dependents. There is some fanciness I think we will need to do in solver setup to enable this but we should not force callers to know these special cases -- it should just work.
I think likewise we shouldn't need to special case languages in the solve, at least not after compilers are proper dependencies. That can change later, too.

More comments/details below, but this is looking better and is moving along!

tgamblin · 2024-03-25T05:25:25Z

lib/spack/spack/directives.py


    """
+    if spack.spec.Spec(spec).name in SUPPORTED_LANGUAGES:
+        assert type == "build", "languages must be of 'build' type"


Why is this needed? It's not really a build dependency; it's a build dependency on the compiler and (maybe) a link dependency on the runtime, depending on what the compiler decides to do.

Languages should just omit the deptype. In practice, it doesn't matter b/c it's determined by the compiler's package (and some may not need a runtime). The caller of depends_on should not have to do anything based on how the language is implemented. they should just say they depend on it.

Why is this needed? It's not really a build dependency; it's a build dependency on the compiler and (maybe) a link dependency on the runtime, depending on what the compiler decides to do.

Depending on a language just brings in the compiler, so as you say above it's a build dependency on the compiler. Then the compiler may or may not decide to inject further dependencies. If we don't make the language dependency a build dependency we won't be able to have duplicates (since link dependency virtuals are unified, whereas build are not).

Languages should just omit the deptype.

But depends_on have that argument, so a user may specify it. That was one of the reason for which I started implementing this using another directive. If we use depends_on I think people need to get used to adding type="build" as they do e.g. for cmake. If we use a wrapper directive we can take control of the type, and avoid exposing it.

I think people need to get used to adding type="build" as they do e.g. for cmake. If we use a wrapper directive we can take control of the type, and avoid exposing it.

You can just do that with depends_on. No extra directive, no extra convention. Don't have them specify the deptype. It's the "default" deptype, and the machinery underneath should take care of the rest.

tgamblin · 2024-03-25T05:28:53Z

lib/spack/spack/package_base.py

    provided_together: Dict["spack.spec.Spec", List[Set[str]]]
    patches: Dict["spack.spec.Spec", List["spack.patch.Patch"]]
    variants: Dict[str, Tuple["spack.variant.Variant", "spack.spec.Spec"]]
+    languages: Dict["spack.spec.Spec", Set[str]]


This should be TODO to remove later.

tgamblin · 2024-03-25T05:29:51Z

lib/spack/spack/solver/asp.py

+            if compiler.name not in ("gcc", "oneapi"):
                continue
+
+            compiler_with_different_cls_names = {"oneapi": "intel-oneapi-compilers"}


This still needs to be consolidated - it should come from compilers/__init__.py

tgamblin · 2024-03-25T05:32:33Z

lib/spack/spack/solver/asp.py

+        if languages:
+            body_str += ",\n"
+            for language in languages:
+                body_str += f'  attr("language", {node_variable}, "{language}")'


this, like depends_on() should be using SUPPORTED_LANGUAGES an not forcing extra parameters on the caller.

TODO: the final implementation of this should not have an extra languages parameter and should not have any special cases for "languages". There should be one general mechanism for packages to inject additional dependencies, and that's all we should need.

tgamblin · 2024-03-25T05:34:14Z

lib/spack/spack/solver/concretize.lp

+attr("language", node(X, Package), Language) :-
+  condition_holds(ConditionID, node(X, Package)),
+  pkg_fact(Package,language(ConditionID, Language)).


This should eventually be unnecessary once we have compilers modeled as packages.

tgamblin · 2024-03-25T05:41:19Z

lib/spack/spack/solver/counter.py

+        runtime_pkgs = spack.repo.PATH.packages_with_tags("runtime")
+        runtime_virtuals = set()
+        for x in runtime_pkgs:
+            pkg_class = spack.repo.PATH.get_pkg_class(x)
+            runtime_virtuals.update(pkg_class.provided_virtual_names())
+
+        self.specs = specs + [spack.spec.Spec(x) for x in runtime_pkgs]


This requires too much effort from the packager. They shouldn't have to tag runtimes.

I'm assuming the reason we do this is because we need the full set of possible virtuals before the solve, but you have that after you call runtime_constraints() on all the packages that have that method. You can just see if any of the global dependencies added with pkg("*").depends_on() are virtual, and get the same (actually, possibly smaller) list of runtime_virtuals that you have here. So this logic should go away and we should just look at what was injected.

I can remove the tags for an extra method in e.g. gcc that returns what the package could possibly inject.

tgamblin · 2024-03-25T05:43:13Z

var/spack/repos/builtin/packages/gcc/package.py

+            gfortran_str = "libgfortran@4"
+
+        for fortran_virtual in ("fortran-rt", gfortran_str):
+            pkg("*").depends_on(


This depends_on() is the one that should alert the solver to the fact that libgfortran and fortran-rt are possible virtuals. I think the pkg object should keep track of that and we should use it once all runtime_constraints() have been called in setup.

tgamblin · 2024-03-25T05:49:40Z

var/spack/repos/builtin/packages/hdf5/package.py

+    depends_on("cxx", type="build", when="+cxx")
+    depends_on("fortran", type="build", when="+fortran")


@alalazo Why do these have to be build dependencies? I think a default deptype is fine here, as this will eventually resolve to a build dependency on a compiler and link dependencies on libraries.

Discussed in DMs. We might want to consider a new deptype. Currently the dependency maps to the compiler only, which needs to be build due to unification rules (once compilers are real dependencies).

…ck#42062) This PR adds: - A new runtime for `%oneapi` compilers, called `intel-oneapi-runtime` - Information to both `gcc-runtime` and `intel-oneapi-runtime`, to ensure that we don't mix compilers using different soname for either `libgfortran` or `libifcore` To do so, the following internal mechanisms have been implemented: - Possibility to inject virtual dependencies from the `runtime_constraints` callback on packages Information has been added to `gcc-runtime` to provide the correct soname under different conditions on its `%gcc`. Rules injected into the solver looks like: ```prolog % Add a dependency on 'gfortran@5' for nodes compiled with gcc@=13.2.0 and using the 'fortran' language attr("dependency_holds", node(ID, Package), "gfortran", "link") :- attr("node", node(ID, Package)), attr("node_compiler", node(ID, Package), "gcc"), attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"), not external(node(ID, Package)), not runtime(Package), attr("language", node(ID, Package), "fortran"). attr("virtual_node", node(RuntimeID, "gfortran")) :- attr("depends_on", node(ID, Package), ProviderNode, "link"), provider(ProviderNode, node(RuntimeID, "gfortran")), attr("node", node(ID, Package)), attr("node_compiler", node(ID, Package), "gcc"), attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"), not external(node(ID, Package)), not runtime(Package), attr("language", node(ID, Package), "fortran"). attr("node_version_satisfies", node(RuntimeID, "gfortran"), "5") :- attr("depends_on", node(ID, Package), ProviderNode, "link"), provider(ProviderNode, node(RuntimeID, "gfortran")), attr("node", node(ID, Package)), attr("node_compiler", node(ID, Package), "gcc"), attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"), not external(node(ID, Package)), not runtime(Package), attr("language", node(ID, Package), "fortran"). ```

balay · 2024-04-02T20:49:34Z

I'm seeing the following behavior with this PR change.

svcpetsc@petsc-gpu-02:/scratch/svcpetsc/spack.a$ ./bin/spack spec [email protected] %[email protected] |grep hdf5@
 -           ^[email protected]%[email protected]~cxx+fortran+hl~ipo~java~map+mpi+shared~subfiling~szip~threadsafe+tools api=default build_system=cmake build_type=Release generator=make arch=linux-ubuntu22.04-x86_64

Note the [email protected]%[email protected] above. This breaks dependent pkgs that attempt to use hdf5 as the f90 module is no-longer compatible [with ifc].

With the snapshot prior to this change - I get:

svcpetsc@petsc-gpu-02:/scratch/svcpetsc/spack.a$ ./bin/spack spec [email protected] %[email protected] ^hdf5 %[email protected] |grep hdf5@
 -           ^[email protected]%[email protected]~cxx+fortran+hl~ipo~java~map+mpi+shared~subfiling~szip~threadsafe+tools api=default build_system=cmake build_type=Release generator=make arch=linux-ubuntu22.04-x86_64

Any idea if hdf5 needs a fix to restore this behavior?

Alternative I'm having to use now:

svcpetsc@petsc-gpu-02:/scratch/svcpetsc/spack.a$ ./bin/spack spec [email protected] %[email protected] ^hdf5 %[email protected] |grep hdf5@
 -           ^[email protected]%[email protected]~cxx+fortran+hl~ipo~java~map+mpi+shared~subfiling~szip~threadsafe+tools api=default build_system=cmake build_type=Release generator=make arch=linux-ubuntu22.04-x86_64

alalazo · 2024-04-03T12:17:58Z

@balay I'll check if I can reproduce this default behavior, and why it happens. Thanks for reporting.

…ck#42062) This PR adds: - A new runtime for `%oneapi` compilers, called `intel-oneapi-runtime` - Information to both `gcc-runtime` and `intel-oneapi-runtime`, to ensure that we don't mix compilers using different soname for either `libgfortran` or `libifcore` To do so, the following internal mechanisms have been implemented: - Possibility to inject virtual dependencies from the `runtime_constraints` callback on packages Information has been added to `gcc-runtime` to provide the correct soname under different conditions on its `%gcc`. Rules injected into the solver looks like: ```prolog % Add a dependency on 'gfortran@5' for nodes compiled with gcc@=13.2.0 and using the 'fortran' language attr("dependency_holds", node(ID, Package), "gfortran", "link") :- attr("node", node(ID, Package)), attr("node_compiler", node(ID, Package), "gcc"), attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"), not external(node(ID, Package)), not runtime(Package), attr("language", node(ID, Package), "fortran"). attr("virtual_node", node(RuntimeID, "gfortran")) :- attr("depends_on", node(ID, Package), ProviderNode, "link"), provider(ProviderNode, node(RuntimeID, "gfortran")), attr("node", node(ID, Package)), attr("node_compiler", node(ID, Package), "gcc"), attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"), not external(node(ID, Package)), not runtime(Package), attr("language", node(ID, Package), "fortran"). attr("node_version_satisfies", node(RuntimeID, "gfortran"), "5") :- attr("depends_on", node(ID, Package), ProviderNode, "link"), provider(ProviderNode, node(RuntimeID, "gfortran")), attr("node", node(ID, Package)), attr("node_compiler", node(ID, Package), "gcc"), attr("node_compiler_version", node(ID, Package), "gcc", "13.2.0"), not external(node(ID, Package)), not runtime(Package), attr("language", node(ID, Package), "fortran"). ```

mabraham · 2024-11-26T11:09:56Z

Ah, working now: #47806

spackbot-app bot added core PR affects Spack core functionality defaults dependencies directives update-package virtual-dependencies labels Jan 12, 2024

spackbot-app bot requested review from brtnfld, gheber, haampie, hyoklee, lkurz, lrknox and michaelkuhn January 12, 2024 09:59

alalazo commented Jan 12, 2024

View reviewed changes

lib/spack/spack/solver/display.lp Outdated Show resolved Hide resolved

alalazo force-pushed the features/gcc-runtime-and-languages branch from 579d704 to e026365 Compare January 12, 2024 11:06

tgamblin self-assigned this Jan 18, 2024

alalazo force-pushed the features/gcc-runtime-and-languages branch from e026365 to 3ad227e Compare January 22, 2024 16:59

spackbot-app bot added intel new-package labels Jan 22, 2024

alalazo force-pushed the features/gcc-runtime-and-languages branch from 3ad227e to 661f908 Compare January 22, 2024 21:50

alalazo marked this pull request as ready for review January 23, 2024 11:56

alalazo force-pushed the features/gcc-runtime-and-languages branch from 661f908 to eef4c70 Compare January 23, 2024 11:59

alalazo changed the title ~~gcc-runtime: add constraints on language runtimes~~ Add intel-oneapi-runtime and language directive Jan 23, 2024

hyoklee approved these changes Jan 23, 2024

View reviewed changes

alalazo added 15 commits March 15, 2024 11:03

intel-oneapi-runtime: add a new runtime package

bf902c3

The intel-oneapi-runtime depends on gcc-runtime at link time. This dependency can be used later to dynamically select which GCC to use with the %oneapi compiler. intel-oneapi-runtime provides ifort for packages that need fortran

Add a fortran-rt virtual for all Fortran runtimes

903d0b6

Use a symlink to avoid licensing issues

ef58a4a

Group addition of dynamically injected virtual packages into a function

1ec92eb

intel-oneapi-runtime: copy libraries, instead of symlonking

52035fd

Also: provides sycl, add libiomp5 to the list of libraries

intel-oneapi-runtime: replace install time check by directives

e32ea68

Renamed gfortran -> libgfortran

3acd1b4

Don't hardcode names when counting possible packages

681302a

Remove the "language" directive, in favor of "depends_on"

2ce6e79

Fix wording in docstring

70c5d48

Remove needless check on supported compilers

3c34818

gcc-runtime: fix issue introduced with renaming

d0fb08d

intel-oneapi-runtime: add stub libs, as for gcc-runtime

b42c383

Renamed ifcore -> libifcore

dd2c493

Pass a Spec instead of a Compiler

4511679

For now maintain a CompilerSpec, since it allows to avoid a double conversion from compiler name to pkg name and vice-versa

alalazo force-pushed the features/gcc-runtime-and-languages branch from 9f1ce19 to 4511679 Compare March 15, 2024 12:54

alalazo changed the title ~~Add intel-oneapi-runtime and language directive~~ Add intel-oneapi-runtime, allow injecting virtual dependencies Mar 18, 2024

tgamblin reviewed Mar 25, 2024

View reviewed changes

tgamblin approved these changes Mar 25, 2024

View reviewed changes

tgamblin merged commit 0c9a53b into spack:develop Mar 25, 2024

alalazo deleted the features/gcc-runtime-and-languages branch March 25, 2024 07:35

This was referenced Apr 29, 2024

Implement relocation for compiler runtime paths #7120

Closed

python: always use a venv #40773

Merged

scheibelp mentioned this pull request Dec 19, 2024

Test compiler mixing #48060

Open

		depends_on("cxx", type="build", when="+cxx")
		depends_on("fortran", type="build", when="+fortran")

Conversation

alalazo commented Jan 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spackbot-app bot commented Jan 12, 2024

Uh oh!

Uh oh!

alalazo commented Jan 23, 2024

Uh oh!

alalazo commented Jan 23, 2024

Uh oh!

alalazo commented Jan 23, 2024

Uh oh!

spackbot-app bot commented Jan 23, 2024

Uh oh!

alalazo commented Jan 24, 2024

Uh oh!

tgamblin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

balay commented Apr 2, 2024

Uh oh!

alalazo commented Apr 3, 2024

Uh oh!

mabraham commented Nov 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

alalazo commented Jan 12, 2024 •

edited

Loading