From 498f448ef353eed906a100752b2c1598423a7276 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Mon, 21 Oct 2019 19:24:57 +0200
Subject: [PATCH] microarchitectures: fix custom compiler versions (#13222)
Custom string versions for compilers were raising a ValueError on
conversion to int. This commit fixes the behavior by trying to detect
the underlying compiler version when in presence of a custom string
version.
* Refactor code that deals with custom versions for better readability
* Partition version components with a regex
* Fix semantic of custom compiler versions with a suffix
* clang@x.y-apple has been special-cased
* Add unit tests
---
lib/spack/llnl/util/cpu/__init__.py | 4 ++-
lib/spack/llnl/util/cpu/microarchitecture.py | 27 +++++++++++++++++---
lib/spack/spack/architecture.py | 18 ++++++++++++-
lib/spack/spack/test/architecture.py | 26 +++++++++++++++++++
lib/spack/spack/test/data/compilers.yaml | 18 +++++++++++++
lib/spack/spack/test/llnl/util/cpu.py | 12 +++++++++
6 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/lib/spack/llnl/util/cpu/__init__.py b/lib/spack/llnl/util/cpu/__init__.py
index eda6fa9c36..cf3c3ef50c 100644
--- a/lib/spack/llnl/util/cpu/__init__.py
+++ b/lib/spack/llnl/util/cpu/__init__.py
@@ -5,6 +5,7 @@
from .microarchitecture import Microarchitecture, UnsupportedMicroarchitecture
from .microarchitecture import targets, generic_microarchitecture
+from .microarchitecture import version_components
from .detect import host
__all__ = [
@@ -12,5 +13,6 @@
'UnsupportedMicroarchitecture',
'targets',
'generic_microarchitecture',
- 'host'
+ 'host',
+ 'version_components'
]
diff --git a/lib/spack/llnl/util/cpu/microarchitecture.py b/lib/spack/llnl/util/cpu/microarchitecture.py
index e14e1a8c66..3d1590376a 100644
--- a/lib/spack/llnl/util/cpu/microarchitecture.py
+++ b/lib/spack/llnl/util/cpu/microarchitecture.py
@@ -4,6 +4,7 @@
# SPDX-License-Identifier: (Apache-2.0 OR MIT)
import functools
import platform
+import re
import warnings
try:
@@ -219,9 +220,9 @@ def satisfies_constraint(entry, version):
min_version, max_version = entry['versions'].split(':')
# Check version suffixes
- min_version, _, min_suffix = min_version.partition('-')
- max_version, _, max_suffix = max_version.partition('-')
- version, _, suffix = version.partition('-')
+ min_version, min_suffix = version_components(min_version)
+ max_version, max_suffix = version_components(max_version)
+ version, suffix = version_components(version)
# If the suffixes are not all equal there's no match
if ((suffix != min_suffix and min_version) or
@@ -277,6 +278,26 @@ def generic_microarchitecture(name):
)
+def version_components(version):
+ """Decomposes the version passed as input in version number and
+ suffix and returns them.
+
+ If the version number of the suffix are not present, an empty
+ string is returned.
+
+ Args:
+ version (str): version to be decomposed into its components
+ """
+ match = re.match(r'([\d.]*)(-?)(.*)', str(version))
+ if not match:
+ return '', ''
+
+ version_number = match.group(1)
+ suffix = match.group(3)
+
+ return version_number, suffix
+
+
def _known_microarchitectures():
"""Returns a dictionary of the known micro-architectures. If the
current host platform is unknown adds it too as a generic target.
diff --git a/lib/spack/spack/architecture.py b/lib/spack/spack/architecture.py
index dd99cfa1e3..0e318bbccf 100644
--- a/lib/spack/spack/architecture.py
+++ b/lib/spack/spack/architecture.py
@@ -192,6 +192,8 @@ def optimization_flags(self, compiler):
compiler (CompilerSpec or Compiler): object that contains both the
name and the version of the compiler we want to use
"""
+ # Mixed toolchains are not supported yet
+ import spack.compilers
if isinstance(compiler, spack.compiler.Compiler):
if spack.compilers.is_mixed_toolchain(compiler):
msg = ('microarchitecture specific optimizations are not '
@@ -200,8 +202,22 @@ def optimization_flags(self, compiler):
warnings.warn(msg.format(compiler))
return ''
+ # Try to check if the current compiler comes with a version number or
+ # has an unexpected suffix. If so, treat it as a compiler with a
+ # custom spec.
+ compiler_version = compiler.version
+ version_number, suffix = cpu.version_components(compiler.version)
+ if not version_number or suffix not in ('', 'apple'):
+ # Try to deduce the correct version. Depending on where this
+ # function is called we might get either a CompilerSpec or a
+ # fully fledged compiler object
+ import spack.spec
+ if isinstance(compiler, spack.spec.CompilerSpec):
+ compiler = spack.compilers.compilers_for_spec(compiler).pop()
+ compiler_version = compiler.cc_version(compiler.cc)
+
return self.microarchitecture.optimization_flags(
- compiler.name, str(compiler.version)
+ compiler.name, str(compiler_version)
)
diff --git a/lib/spack/spack/test/architecture.py b/lib/spack/spack/test/architecture.py
index 7e120000d4..35980d8a86 100644
--- a/lib/spack/spack/test/architecture.py
+++ b/lib/spack/spack/test/architecture.py
@@ -187,3 +187,29 @@ def test_optimization_flags(
compiler = spack.compilers.compilers_for_spec(compiler_spec).pop()
opt_flags = target.optimization_flags(compiler)
assert opt_flags == expected_flags
+
+
+@pytest.mark.parametrize('compiler,real_version,target_str,expected_flags', [
+ (spack.spec.CompilerSpec('gcc@9.2.0'), None, 'haswell',
+ '-march=haswell -mtune=haswell'),
+ # Check that custom string versions are accepted
+ (spack.spec.CompilerSpec('gcc@foo'), '9.2.0', 'icelake',
+ '-march=icelake-client -mtune=icelake-client'),
+ # Check that we run version detection (4.4.0 doesn't support icelake)
+ (spack.spec.CompilerSpec('gcc@4.4.0-special'), '9.2.0', 'icelake',
+ '-march=icelake-client -mtune=icelake-client'),
+ # Check that the special case for Apple's clang is treated correctly
+ # i.e. it won't try to dtect the version again
+ (spack.spec.CompilerSpec('clang@9.1.0-apple'), None, 'x86_64',
+ '-march=x86-64 -mcpu=generic'),
+])
+def test_optimization_flags_with_custom_versions(
+ compiler, real_version, target_str, expected_flags, monkeypatch, config
+):
+ target = spack.architecture.Target(target_str)
+ if real_version:
+ monkeypatch.setattr(
+ spack.compiler.Compiler, 'cc_version', lambda x, y: real_version
+ )
+ opt_flags = target.optimization_flags(compiler)
+ assert opt_flags == expected_flags
diff --git a/lib/spack/spack/test/data/compilers.yaml b/lib/spack/spack/test/data/compilers.yaml
index f619c8d351..7aec138473 100644
--- a/lib/spack/spack/test/data/compilers.yaml
+++ b/lib/spack/spack/test/data/compilers.yaml
@@ -135,3 +135,21 @@ compilers:
f77: None
fc: None
modules: 'None'
+- compiler:
+ spec: gcc@foo
+ operating_system: redhat6
+ paths:
+ cc: /path/to/gcc
+ cxx: /path/to/g++
+ f77: /path/to/gfortran
+ fc: /path/to/gfortran
+ modules: 'None'
+- compiler:
+ spec: gcc@4.4.0-special
+ operating_system: redhat6
+ paths:
+ cc: /path/to/gcc
+ cxx: /path/to/g++
+ f77: /path/to/gfortran
+ fc: /path/to/gfortran
+ modules: 'None'
diff --git a/lib/spack/spack/test/llnl/util/cpu.py b/lib/spack/spack/test/llnl/util/cpu.py
index 5713580a72..68185532b4 100644
--- a/lib/spack/spack/test/llnl/util/cpu.py
+++ b/lib/spack/spack/test/llnl/util/cpu.py
@@ -242,3 +242,15 @@ def test_automatic_conversion_on_comparisons(operation, expected_result):
target = llnl.util.cpu.targets[target]
code = 'target ' + operator + 'other_target'
assert eval(code) is expected_result
+
+
+@pytest.mark.parametrize('version,expected_number,expected_suffix', [
+ ('4.2.0', '4.2.0', ''),
+ ('4.2.0-apple', '4.2.0', 'apple'),
+ ('my-funny-name-with-dashes', '', 'my-funny-name-with-dashes'),
+ ('10.3.56~svnr64537', '10.3.56', '~svnr64537')
+])
+def test_version_components(version, expected_number, expected_suffix):
+ number, suffix = llnl.util.cpu.version_components(version)
+ assert number == expected_number
+ assert suffix == expected_suffix
--
GitLab