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

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