From 498f448ef353eed906a100752b2c1598423a7276 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Mon, 21 Oct 2019 19:24:57 +0200 Subject: 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 @@ __all__ = [ '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 @@ class Microarchitecture(object): 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 @@ class Target(object): 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 @@ class Target(object): 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 -- cgit v1.2.3-70-g09d2