spack / spack

A flexible package manager that supports multiple versions, configurations, platforms, and compilers.
https://spack.io
Other
4.42k stars 2.29k forks source link

Spack environment: cannot combine spec cflags with compiler cflags #38664

Open scheibelp opened 1 year ago

scheibelp commented 1 year ago

Steps to reproduce

Using the following spack.yaml:

spack:
  # add package specs to the `specs` list
  specs:
  - zlib cflags=-O3
  view: true
  concretizer:
    unify: true
  'compilers:':
  - compiler:
      spec: gcc@=9.4.0
      paths:
        cc: /usr/bin/gcc
        cxx: /usr/bin/g++
        f77: null
        fc: null
      flags:
        cflags: "-What"
      operating_system: ubuntu20.04
      target: x86_64
      modules: []
      environment: {}
      extra_rpaths: []

(in my case I did this on an Ubuntu 20 docker image)

and do

spack env create t1 spack.yaml 
spack -e t1 concretize -f -U

(nothing happens)

Error message

There is no error message, but nothing is concretized, and no spack.lock appears.

If I remove the flag from either the spec in the spec list, or set cflags to "" in the compiler entry, then the concretization succeeds.

If I run in debug mode (spack -d...) then most of the output looks like normal config loading, although the following unusual message appears (not reported as an error and doesn't show up without -d):

==> [2023-06-30-23:22:35.561111] UNKNOWN SYMBOL: attr("node_compiler", zlib, gcc)

Information on your system

$ spack debug report
* **Spack:** 0.21.0.dev0 (0c2b98ca7044588d56e9e4c9c178f411b6dc15c1)
* **Python:** 3.8.10
* **Platform:** linux-ubuntu20.04-skylake
* **Concretizer:** clingo

General information

scheibelp commented 1 year ago

Note that combining the compiler config with command-line invocation works:

spack -e t1 spec zlib cflags=-O3
Concretized
--------------------------------
 -   zlib@1.2.13%gcc@9.4.0 cflags="-What -O3" +optimize+pic+shared build_system=makefile arch=linux-ubuntu20.04-skylake

but the environment will not concretize.

adrienbernede commented 1 year ago

@becker33 this is preventing me from moving to environments: I have specs with extra flags on top of those defined at compiler definition.

adrienbernede commented 1 year ago

@becker33 @scheibelp @tldahlgren Please allow me to insist on the fact that this is critical for RADIUSS open-source software at LLNL. We can’t move on to environments because of this. Thank you.

scheibelp commented 1 year ago

I can look into this next week

alalazo commented 9 months ago

Adding some more information for the record, as I'm looking into this while reviewing #42655.

The bug here is in FlagMap.satisfies (as @scheibelp notes in #41049) , specifically in:

Spec("zlib cflags='-What -O3'").satisfies("zlib cflags='-O3'")

which returns False, while a True is expected in:

https://github.com/spack/spack/blob/3d1d5f755ff0c0f39764620d3c769b41bd786c67/lib/spack/spack/solver/asp.py#L469

From there the spec builder takes the wrong branch and so we get an "unused spec" while we expected none.

alalazo commented 9 months ago

I tried to look into the docs if we define somewhere what should be the semantic of combining flags from different sources, but couldn't find anything. Did I miss any relevant section?

Basically, my thoughts are that for flags order is important and it seems we don't specify anywhere how:

  1. Command line flags
  2. Compiler flags
  3. Flags from packages

are composed.

scheibelp commented 9 months ago

if we define somewhere what should be the semantic of combining flags from different sources, but couldn't find anything. Did I miss any relevant section?

I don't think we do. IMO it would be easier to bluntly append all these sources in a fixed order, at least to start, and raise an error if the combined list has conflicting flags. Overall the approach I suggest is:

I predict this will work smoothly so long as package.py files don't start encoding cflags into their depends_on statements (there are currently no cases of this in the builtin repo).

~The~ Some use cases to consider:

alalazo commented 9 months ago

@scheibelp Do we define the meaning of satisfies, intersects and constrain for compiler flags in the approach you propose above? It's not clear to me that we do so, and I think it's important to have a clear model (if we have that all the rest will follow).

Taking into account as a simple example the optimization flag -O3, gcc returns the following:

$ gcc -Q -O3 --help=optimizers | grep ftree-pre
  -ftree-pre                        [enabled]
$ gcc -Q -fno-tree-pre -O3 --help=optimizers | grep ftree-pre
  -ftree-pre                        [disabled]

So does "-fno-tree-pre -O3".satisfies("-O3") ? In my opinion the correct answer should be "no".

The reasoning is that, if the answer is "yes" we can disable explicitly another optimization from -O3 until, by induction, we disabled them all. At that point "<no-optimizations>".satisfies("-O3"). That's clearly a wrong model.

In principle, I think we should try to get a canonical form for the flags[^1] and have the flags behave like a multi-valued variant. I'm not sure though this is easy to obtain for each compiler we support.

If we agree on a model, but that is difficult to implement, we can opt for some "wrong" (or approximate) implementation that is better for our "practical" purposes. But at least we'll know what we would like to obtain from the model, and how the implementation falls short of that.

[^1]: e.g. the one obtained with gcc -Q ... --help=<section> on GCC

scheibelp commented 9 months ago

If we agree on a model, but that is difficult to implement, we can opt for some "wrong" (or approximate) implementation that is better for our "practical" purposes. But at least we'll know what we would like to obtain from the model, and how the implementation falls short of that.

I think the easiest way to define the model is to describe the use cases we want to satisfy. I tried to do that in my proposal. I think more use cases will come up naturally as this proposal (or any proposal) draws criticism (i.e. I think defining an implementation will help us talk about how to model things).

In principle, I think we should try to get a canonical form for the flags and have the flags behave like a multi-valued variant.

I take "canonical" to mean a unique representation: do you mean that for a given unordered set of flags, that we should know how to order them? I don't think we can actually guarantee doing that (at least, we can't do it for -lx -ly for example because lib orderings can have different meanings).

Do we define the meaning of satisfies, intersects and constrain for compiler flags in the approach you propose above?

In my proposal .satisfies does not handle issues like the possible conflict between -fno-tree-pre and -O3: For two flag specifications X and Y, X.satisfies(Y) if all flags in Y appear in X.

does "-fno-tree=pre -O3".satisfies("-O3")? In my opinion the correct answer should be "no".

This example is interesting (do you want me to break out all use cases into a separate comment here?): this means that e.g. there is no conflict if -fno-tree-pree -O3 comes from one source, but it does conflict if it is assembled from two different sources, which makes it impossible to model w/ our logic for multi-valued variants (e.g. it is not covered by conflicts("cflags=-O2", when "cflags=-O3").

I think the answer should be "yes" in terms of what the solver and "spec.satisfies" says, and I think my proposal can make that work:

The reasoning is that, if the answer is "yes" we can disable explicitly another optimization from -O3 until, by induction, we disabled them all.

I think my explanation for -fno-tree-pre addresses this, but let me know if you think it doesn't

alalazo commented 9 months ago

do you mean that for a given unordered set of flags, that we should know how to order them?

My idea is that from a given ordered set of flags, we can get a unique unordered set that represents it. gcc has ways to obtain that representation for different set of flags (e.g. optimizations, target specific flags, warnings), where it expands options like -OX into the single options that are enabled or disabled, and it reduces conflicting flags. Not sure if other compilers expose anything similar.

scheibelp commented 8 months ago

My idea is that from a given ordered set of flags, we can get a unique unordered set that represents it.

This might be true of optimization flags, but isn't true for linking flags. We could still do it for the optimization flags, but committing ourselves to this level of fidelity seems problematic:

so it feels like we'd be doing a lot of work for something that can immediately be undone.

Hopefully this does not come across as dismissive: I think what you point out is important, but I think that the best response to this specific issue is for us to explicitly agree not to handle it.

alalazo commented 8 months ago

but isn't true for linking flags.

Correct. Also, any path-like flag doesn't fit in this model. Are you aware of any use of ldflags="-L/path1 -L/path2 -lfoo -lbar" or similar? Not seeing them in here I thought we dealt with customizing flags like that in other ways. If we need to model linking flags too in specs, then I don't think we can map flags to multi-valued variants. At least not in a formal model (which may differ from our implementation).

I agree though that we can avoid handling certain cases and be aware of that, which means our model has deficiencies users need to be aware of. That wouldn't be dissimilar from the current way e.g. compilers are treated.

adrienbernede commented 8 months ago

@alalazo @scheibelp it looks like this is turning into deeper design considerations that the issue first suggested.

I think I get the issue about the complexity of comparing a list of flags. One thing I am not sure to understand however, is why would spack have 2 different specs to compare here in the first place.

@alalazo you said the issues is that Spec("zlib cflags='-What -O3'").satisfies("zlib cflags='-O3'") returns false. And the discussion then demonstrates that there is no obvious policy, since -What could actually alter the meaning of -O3.

My (possibly naive) question is as follow. Given the environment in this reproducer, to me it looks like there is only one spec to consider: zlib cflags='-What -O3'. Then why should we care about satisfying zlib cflags='-O3' ?

alalazo commented 8 months ago

Then why should we care about satisfying zlib cflags='-O3' ?

It's the abstract spec requested by the user. We check that to associate a concrete spec with the corresponding abstract spec.

adrienbernede commented 8 months ago

@alalazo I understand what you’re saying, but still, by asking for this compiler (defined with the additional flag -What) the user is asking for -O3 -What even if that is done implicitly.

To reuse your point above, if we had 2 compilers, one with no flags and one with -fno-tree-pre, by choosing the second one, the user is expecting "slightly less than -O3".

I am not saying I am right to think that way, but to me, it looks like you are debating defining "satisfies" in a way that will either suppose full knowledge of how compilers deal with flags, or accept compromise, while I think the solution might be to have the compilers flags applied to the abstract spec because that’s what the users asked (knowingly or not).

alalazo commented 8 months ago

@adrienbernede I am fine with a workaround to get past this problem (I think @scheibelp is working on one, happy to review it when ready), but the above considerations mean to me we don't have a clean formal model of flags. This makes discussing flags more difficult, because flags are inherently ordered, and later flags could potentially alter the meaning or previous flags.

I could go with counterexamples to your "slightly less than -O3" but I guess you got the point: it's difficult to define "slightly less", or make sure "slightly less" means the same thing for all the users, without a formal model :slightly_smiling_face:

adrienbernede commented 8 months ago

@alalazo my comments may just be too naive. I'll trust you with doing what's best for spack.

alalazo commented 8 months ago

my comments may just be too naive. I'll trust you with doing what's best for spack.

I wouldn't dismiss them as too naive. I'm just trying to explain why I'm looking for a model for flags which defines how Flag.satisfies and Flag.constrain work. Currently a flag satisfies another if it is exactly the same string which, as this issue proofs, is too restrictive and prevents composing flags from different sources.

That said, one practical solution might be to explicitly agree to not handle the semantic inconsistencies discussed above, as @scheibelp says proposes in https://github.com/spack/spack/issues/38664#issuecomment-1967866664 We may get a model which is not 100% satisfactory, but we'll be documenting how the model works and in which cases it may give unexpected results. This would still be a big improvement from the state in this issue.

scheibelp commented 8 months ago

I am fine with a workaround to get past this problem (I think @scheibelp is working on one, happy to review it when ready),

To be clear: I don't have a workaround yet. My only strategy so far in this thread was to decide how to handle concretization of flags. Exploring workarounds is also worthwhile IMO:

scheibelp commented 8 months ago

Compiler flag combination semantics

The following attempts to describe cases where two sets of compiler flags are complementary based on my reading of GCC. Loosely speaking, something like

-O3 -Wall and -O3

are complementary. In words, I would take complementary to mean cases where one list of compiler flags would be accepted in place of another.

@alalazo this is my attempt to more-fully capture the flag semantics so that we may be aware of the possible deficiencies in an implementation. Overall, I still think https://github.com/spack/spack/issues/38664#issuecomment-1963541947 is a fully-specified and reasonable implementation for dealing with flag combination.

Modifiers: one flag partially undoing the effect of another

Conflicts: flags that are precisely opposite in their effect

Ordering: when does it matter

The --start-group linking flag

(UPDATE March 21: I added this section)

--start-group -la -lb --end-group --start-group -lc -ld --end-group is different from --start-group -la -lb -lc -ld --end-group

This would complicate a set-based representation because --start-group appears multiple times (and that is important for the linking behavior).

However, for purposes of representation in the concretizer, one could consolidate into a pseudo flag like --ld-group="-la -lb" --ld-group="-lc -ld".

I made a separate section for this because I don't know any other flags that have this behavior.

scheibelp commented 8 months ago

Broad approach based on discussion of above between @tgamblin @alalazo @psakievich @becker33 (each step here can be a separate PR):

koysean commented 6 months ago

Since I don't see a workaround mentioned in this thread, here's one that I've found useful in the meantime.

From the original issue, this fails:

  specs:
    - zlib cflags="-O3"

but using a require: statement to set the needed cflags does concretize and the concretized spec concatenates the flags from the compiler and package configurations:

  packages:
    zlib:
      require: 'cflags="-O3"'
  specs:
    - zlib

This doesn't take care of any flag conflicts or ordering. It seems it also can't propagate flags to dependencies automatically like you could with an abstract spec (with cflags==...). However, it's been good enough for what I need to do.