From 16b4b395a5a7254dd6dc94098e55f61553d6a85b Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 10 Aug 2021 14:33:45 +0200 Subject: [PATCH 1/6] Allow optarch values to be partial maps including vector extensions Instead of only setting optimal compiler arguments based on architecture and CPU family/vendor also include the supported vector extensions as criteria to choose flags. To simplify specifying generic flags allow partial matches, e.g. a fallback setting for any x86 arch which doesn't have more specific values. Example: COMPILER_OPTIMAL_ARCHITECTURE_OPTION = { (systemtools.X86_64, ): 'xHost', (systemtools.X86_64, systemtools.AMD, systemtools.AVX2): 'mavx2', } --- easybuild/tools/systemtools.py | 45 ++++++++++++++++++++++++ easybuild/tools/toolchain/compiler.py | 50 ++++++++++++++++++++------- test/framework/systemtools.py | 24 +++++++++++++ 3 files changed, 106 insertions(+), 13 deletions(-) diff --git a/easybuild/tools/systemtools.py b/easybuild/tools/systemtools.py index 73eb657510..496d27c5c2 100644 --- a/easybuild/tools/systemtools.py +++ b/easybuild/tools/systemtools.py @@ -99,6 +99,13 @@ ARCH_KEY_PREFIX = 'arch=' +# Vector extension constants +SSE = 'sse' +SSE2 = 'sse2' +AVX = 'avx' +AVX2 = 'avx2' +AVX512F = 'avx512f' + # Vendor constants AMD = 'AMD' APM = 'Applied Micro' @@ -133,6 +140,8 @@ CPU_ARCHITECTURES = [AARCH32, AARCH64, POWER, RISCV32, RISCV64, X86_64] CPU_FAMILIES = [AMD, ARM, INTEL, POWER, POWER_LE, RISCV] CPU_VENDORS = [AMD, APM, APPLE, ARM, BROADCOM, CAVIUM, DEC, IBM, INTEL, MARVELL, MOTOROLA, NVIDIA, QUALCOMM] +# Vector extensions of CPUs in ascending order (later => better) +CPU_VECTOR_EXTS = [SSE, SSE2, AVX, AVX2, AVX512F] # ARM implementer IDs (i.e., the hexadeximal keys) taken from ARMv8-A Architecture Reference Manual # (ARM DDI 0487A.j, Section G6.2.102, Page G6-4493) VENDOR_IDS = { @@ -610,6 +619,13 @@ def get_cpu_features(): return cpu_feat +def get_cpu_vector_exts(): + """Get list of (relevant) CPU vector extensions""" + cpu_features = set(get_cpu_features()) + # Values of the vector extension constants purposely match the cpu feature value + return [i for i in CPU_VECTOR_EXTS if i in cpu_features] + + def get_gpu_info(): """ Get the GPU info @@ -1322,6 +1338,35 @@ def pick_dep_version(dep_version): return result +def pick_opt_arch(options, arch, cpu_family, vector_exts): + """ + Pick the best matching entry from the options dict based on CPU arch and features + + :param options: Dictionary mapping tuples of system specs to optarchs. + E.g. (X86_64, AMD, AVX2): 'mavx2' + :param arch: Current CPU arch + :param cpu_family: Current CPU family + :param vector_exts: Vector extensions supported by current CPU + """ + def create_possible_keys(): + """Yield a list of possible keys from most specific to least specific""" + for ext in vector_exts: + yield (arch, cpu_family, ext) + yield (arch, cpu_family) + yield (arch, ) + # Also allow single string entry + yield arch + + result = None + for key in create_possible_keys(): + try: + result = options[key] + break + except KeyError: + pass + return result + + def det_pypkg_version(pkg_name, imported_pkg, import_name=None): """Determine version of a Python package.""" diff --git a/easybuild/tools/toolchain/compiler.py b/easybuild/tools/toolchain/compiler.py index faf8f03d88..854fbc643d 100644 --- a/easybuild/tools/toolchain/compiler.py +++ b/easybuild/tools/toolchain/compiler.py @@ -155,12 +155,29 @@ class Compiler(Toolchain): def __init__(self, *args, **kwargs): """Compiler constructor.""" Toolchain.base_init(self) - self.arch = systemtools.get_cpu_architecture() - self.cpu_family = systemtools.get_cpu_family() + self._arch, self._cpu_family, self._cpu_vector_exts = (None, None, None) # list of compiler prefixes self.prefixes = [] super(Compiler, self).__init__(*args, **kwargs) + @property + def arch(self): + if self._arch is None: + self._arch = systemtools.get_cpu_architecture() + return self._arch + + @property + def cpu_family(self): + if self._cpu_family is None: + self._cpu_family = systemtools.get_cpu_family() + return self._cpu_family + + @property + def cpu_vector_exts(self): + if self._cpu_vector_exts is None: + self._cpu_vector_exts = systemtools.get_cpu_vector_exts() + return self._cpu_vector_exts + def set_options(self, options): """Process compiler toolchain options.""" self._set_compiler_toolchainoptions() @@ -303,6 +320,14 @@ def _set_compiler_flags(self): raise EasyBuildError("toolchainopts %s: '%s' must start with a '-'." % (extra, extraflags)) self.variables.nappend_el(var, extraflags[1:]) + def _pick_optarch_entry(self, optarch_value): + """ + Get the best matching entry from optarch_value when it is a dict, else return it unchanged + """ + if isinstance(optarch_value, dict): + optarch_value = systemtools.pick_opt_arch(optarch_value, self.arch, self.cpu_family, self.cpu_vector_exts) + return optarch_value + def _set_optimal_architecture(self, default_optarch=None): """ Get options for the current architecture @@ -346,25 +371,24 @@ def _set_optimal_architecture(self, default_optarch=None): raise EasyBuildError("optarch is neither an string or a dict %s. This should never happen", optarch) if use_generic: - if (self.arch, self.cpu_family) in (self.COMPILER_GENERIC_OPTION or []): - optarch = self.COMPILER_GENERIC_OPTION[(self.arch, self.cpu_family)] + optarch = self._pick_optarch_entry(self.COMPILER_GENERIC_OPTION) + elif optarch is None: + # no --optarch specified or no option found for the current compiler + if default_optarch: + # Specified optarch default value + optarch = default_optarch else: - optarch = None - # Specified optarch default value - elif default_optarch and optarch is None: - optarch = default_optarch - # no --optarch specified, no option found for the current compiler, and no default optarch - elif optarch is None and (self.arch, self.cpu_family) in (self.COMPILER_OPTIMAL_ARCHITECTURE_OPTION or []): - optarch = self.COMPILER_OPTIMAL_ARCHITECTURE_OPTION[(self.arch, self.cpu_family)] + optarch = self._pick_optarch_entry(self.COMPILER_OPTIMAL_ARCHITECTURE_OPTION) + optarch_target = '%s/%s/%s' % (self.arch, self.cpu_family, self.cpu_vector_exts) if optarch is not None: optarch_log_str = optarch or 'no flags' self.log.info("_set_optimal_architecture: using %s as optarch for %s/%s.", - optarch_log_str, self.arch, self.cpu_family) + optarch_log_str, optarch_target, self.cpu_family) self.options.options_map['optarch'] = optarch elif self.options.options_map.get('optarch', None) is None: optarch_flags_str = "%soptarch flags" % ('', 'generic ')[use_generic] - error_msg = "Don't know how to set %s for %s/%s! " % (optarch_flags_str, self.arch, self.cpu_family) + error_msg = "Don't know how to set %s for %s! " % (optarch_flags_str, optarch_target) error_msg += "Use --optarch='' to override (see " error_msg += "http://easybuild.readthedocs.io/en/latest/Controlling_compiler_optimization_flags.html " error_msg += "for details) and consider contributing your settings back (see " diff --git a/test/framework/systemtools.py b/test/framework/systemtools.py index deece97702..9c2c415cb3 100644 --- a/test/framework/systemtools.py +++ b/test/framework/systemtools.py @@ -971,6 +971,30 @@ def test_pick_system_specific_value(self): self.assertErrorRegex(EasyBuildError, error_pattern, pick_system_specific_value, 'test-desc', {'foo': '1', 'arch=POWER': '2'}) + def test_pick_opt_arch(self): + """Test pick_opt_arch""" + options = { + (st.X86_64, ): 'opt-x86', + (st.X86_64, st.AMD): 'opt-amd', + (st.X86_64, st.AMD, st.AVX2): 'opt-amd-avx2', + (st.AARCH32, st.INTEL): 'opt-aarch32', + st.AARCH64: 'opt-aarch64', + (st.AARCH64, st.INTEL, st.SSE): 'opt-aarch64-sse', + } + avx = [st.SSE, st.SSE2, st.AVX] + avx2 = avx + [st.AVX2] + avx512 = avx2 + [st.AVX512F] + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.INTEL, avx2), 'opt-x86') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.INTEL, []), 'opt-x86') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx), 'opt-amd') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx2), 'opt-amd-avx2') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx512), 'opt-amd-avx2') + self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.AMD, avx2), 'opt-aarch64') + self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.INTEL, avx2), 'opt-aarch64-sse') + self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.INTEL, [st.SSE]), 'opt-aarch64-sse') + self.assertEqual(st.pick_opt_arch(options, st.AARCH32, st.AMD, avx2), None) + self.assertEqual(st.pick_opt_arch(options, st.POWER, st.IBM, []), None) + def test_check_os_dependency(self): """Test check_os_dependency.""" From 38c072d4d8ded2da835dc2604002688ce4c7fd37 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Tue, 10 Aug 2021 15:09:10 +0200 Subject: [PATCH 2/6] Add better optimization flags for Intel compilers on AMD systems Intel will use SSE2 when passed -xHost on AMD systems Be more specific here by passing e.g. AVX2 when that is available. --- easybuild/toolchains/compiler/inteliccifort.py | 10 ++++++---- easybuild/tools/systemtools.py | 5 ++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/easybuild/toolchains/compiler/inteliccifort.py b/easybuild/toolchains/compiler/inteliccifort.py index 01015a0d83..997b740d13 100644 --- a/easybuild/toolchains/compiler/inteliccifort.py +++ b/easybuild/toolchains/compiler/inteliccifort.py @@ -74,13 +74,15 @@ class IntelIccIfort(Compiler): # used when 'optarch' toolchain option is enabled (and --optarch is not specified) COMPILER_OPTIMAL_ARCHITECTURE_OPTION = { - (systemtools.X86_64, systemtools.AMD): 'xHost', - (systemtools.X86_64, systemtools.INTEL): 'xHost', + systemtools.X86_64: 'xHost', + # Intel compilers don't auto-detect AMD features and default to SSE2 + (systemtools.X86_64, systemtools.AMD, systemtools.SSSE3): 'mssse3', + (systemtools.X86_64, systemtools.AMD, systemtools.SSE4_2): 'msse4.2', + (systemtools.X86_64, systemtools.AMD, systemtools.AVX2): 'march=core-avx2', } # used with --optarch=GENERIC COMPILER_GENERIC_OPTION = { - (systemtools.X86_64, systemtools.AMD): 'xSSE2', - (systemtools.X86_64, systemtools.INTEL): 'xSSE2', + systemtools.X86_64: 'xSSE2', } COMPILER_CC = 'icc' diff --git a/easybuild/tools/systemtools.py b/easybuild/tools/systemtools.py index 496d27c5c2..49f28c40ea 100644 --- a/easybuild/tools/systemtools.py +++ b/easybuild/tools/systemtools.py @@ -102,6 +102,9 @@ # Vector extension constants SSE = 'sse' SSE2 = 'sse2' +SSSE3 = 'ssse3' +SSE4_1 = 'sse4_1' +SSE4_2 = 'sse4_2' AVX = 'avx' AVX2 = 'avx2' AVX512F = 'avx512f' @@ -141,7 +144,7 @@ CPU_FAMILIES = [AMD, ARM, INTEL, POWER, POWER_LE, RISCV] CPU_VENDORS = [AMD, APM, APPLE, ARM, BROADCOM, CAVIUM, DEC, IBM, INTEL, MARVELL, MOTOROLA, NVIDIA, QUALCOMM] # Vector extensions of CPUs in ascending order (later => better) -CPU_VECTOR_EXTS = [SSE, SSE2, AVX, AVX2, AVX512F] +CPU_VECTOR_EXTS = [SSE, SSE2, SSSE3, SSE4_1, SSE4_2, AVX, AVX2, AVX512F] # ARM implementer IDs (i.e., the hexadeximal keys) taken from ARMv8-A Architecture Reference Manual # (ARM DDI 0487A.j, Section G6.2.102, Page G6-4493) VENDOR_IDS = { From f4016ddaa275a3440420cfce7e6a7bdd4eac3afe Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 11 Aug 2021 17:04:25 +0200 Subject: [PATCH 3/6] Allow passing arch-specific optarch on cmdline/config --- easybuild/tools/options.py | 79 ++++++++++++++++++++------- easybuild/tools/systemtools.py | 2 + easybuild/tools/toolchain/compiler.py | 17 +++--- test/framework/options.py | 27 ++++++++- test/framework/systemtools.py | 28 ++++++---- 5 files changed, 113 insertions(+), 40 deletions(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 68ddba632a..65432ed9a9 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -104,7 +104,8 @@ from easybuild.tools.toolchain.compiler import DEFAULT_OPT_LEVEL, OPTARCH_MAP_CHAR, OPTARCH_SEP, Compiler from easybuild.tools.toolchain.toolchain import SYSTEM_TOOLCHAIN_NAME from easybuild.tools.repository.repository import avail_repositories -from easybuild.tools.systemtools import UNKNOWN, check_python_version, get_cpu_architecture, get_cpu_family +from easybuild.tools.systemtools import UNKNOWN, CPU_ARCHITECTURES, CPU_FAMILIES, CPU_VECTOR_EXTS +from easybuild.tools.systemtools import check_python_version, get_cpu_architecture, get_cpu_family from easybuild.tools.systemtools import get_cpu_features, get_gpu_info, get_system_info from easybuild.tools.version import this_is_easybuild @@ -987,29 +988,67 @@ def postprocess(self): cleanup_and_exit(self.tmpdir) def _postprocess_optarch(self): - """Postprocess --optarch option.""" - optarch_parts = self.options.optarch.split(OPTARCH_SEP) - - # we expect to find a ':' in every entry in optarch, in case optarch is specified on a per-compiler basis - n_parts = len(optarch_parts) - map_char_cnts = [p.count(OPTARCH_MAP_CHAR) for p in optarch_parts] - if (n_parts > 1 and any(c != 1 for c in map_char_cnts)) or (n_parts == 1 and map_char_cnts[0] > 1): + """ + Postprocess --optarch option. + + Format: map := (;*)* + entry := (, + Example values: + "GENERIC" + "march=native" + "Intel,x86:xHost; Intel,x86,AMD,AVX2:mavx2 -fma; GCC:march=native" + """ + # Split into entries, and each entry into key-value pairs + optarch_parts = [i.strip().split(OPTARCH_MAP_CHAR) for i in self.options.optarch.split(OPTARCH_SEP)] + + # We should either have a single value or a list of key-value pairs, and nothing else + is_single_value = len(optarch_parts) == 1 and len(optarch_parts[0]) == 1 + if not is_single_value and any(len(i) != 2 for i in optarch_parts): raise EasyBuildError("The optarch option has an incorrect syntax: %s", self.options.optarch) + + # if there are options for different compilers, we set up a dict + if is_single_value: + # if optarch is not in mapping format, we do nothing and just keep the string + self.log.info("Keeping optarch raw: %s", self.options.optarch) else: - # if there are options for different compilers, we set up a dict - if OPTARCH_MAP_CHAR in optarch_parts[0]: - optarch_dict = {} - for compiler, compiler_opt in [p.split(OPTARCH_MAP_CHAR) for p in optarch_parts]: - if compiler in optarch_dict: - raise EasyBuildError("The optarch option contains duplicated entries for compiler %s: %s", - compiler, self.options.optarch) + optarch_dict = {} + for key, compiler_opt in optarch_parts: + # key can be either a compiler (only) or compiler and archspec(s) + key_parts = key.split(',') + compiler = key_parts.pop(0) + if key_parts: + for part, allowed_values in zip(key_parts, (CPU_ARCHITECTURES, CPU_FAMILIES, CPU_VECTOR_EXTS)): + if part not in allowed_values: + raise EasyBuildError("The optarch option has an incorrect syntax: %s\n" + "'%s' of '%s' is not in allowed values: %s", + self.options.optarch, part, key, allowed_values) + arch_specs = tuple(key_parts) if len(key_parts) > 1 else key_parts[0] + else: + arch_specs = None + try: + compiler_dict = optarch_dict[compiler] + # Convert single entry to dict if required + if not isinstance(compiler_dict, dict): + compiler_dict = {None: compiler_dict} + optarch_dict[compiler] = compiler_dict + if arch_specs in compiler_dict: + if arch_specs is None: + raise EasyBuildError("The optarch option contains a duplicated entry for compiler %s: %s", + compiler, self.options.optarch) + else: + raise EasyBuildError("The optarch option contains a duplicated entry %s " + "for compiler %s: %s", + arch_specs, compiler, self.options.optarch) else: + compiler_dict[arch_specs] = compiler_opt + except KeyError: + # Keep the dict flat when no archspecs are given + if arch_specs is None: optarch_dict[compiler] = compiler_opt - self.options.optarch = optarch_dict - self.log.info("Transforming optarch into a dict: %s", self.options.optarch) - # if optarch is not in mapping format, we do nothing and just keep the string - else: - self.log.info("Keeping optarch raw: %s", self.options.optarch) + else: + optarch_dict[compiler] = {arch_specs: compiler_opt} + self.options.optarch = optarch_dict + self.log.info("Transforming optarch into a dict: %s", self.options.optarch) def _postprocess_close_pr_reasons(self): """Postprocess --close-pr-reasons options""" diff --git a/easybuild/tools/systemtools.py b/easybuild/tools/systemtools.py index 49f28c40ea..73bd45375e 100644 --- a/easybuild/tools/systemtools.py +++ b/easybuild/tools/systemtools.py @@ -1359,6 +1359,8 @@ def create_possible_keys(): yield (arch, ) # Also allow single string entry yield arch + # Default fallback for any arch + yield None result = None for key in create_possible_keys(): diff --git a/easybuild/tools/toolchain/compiler.py b/easybuild/tools/toolchain/compiler.py index 854fbc643d..f015027fb5 100644 --- a/easybuild/tools/toolchain/compiler.py +++ b/easybuild/tools/toolchain/compiler.py @@ -335,7 +335,7 @@ def _set_optimal_architecture(self, default_optarch=None): :param default_optarch: default value to use for optarch, rather than using default value based on architecture (--optarch and --optarch=GENERIC still override this value) """ - ec_optarch = self.options.get('optarch', False) + ec_optarch = self.options.get('optarch') if isinstance(ec_optarch, string_type): if OPTARCH_MAP_CHAR in ec_optarch: error_msg = "When setting optarch in the easyconfig (found %s), " % ec_optarch @@ -351,17 +351,18 @@ def _set_optimal_architecture(self, default_optarch=None): if isinstance(optarch, dict): # optarch has been validated as complex string with multiple compilers and converted to a dictionary # first try module names, then the family in optarch - current_compiler_names = (getattr(self, 'COMPILER_MODULE_NAME', []) + - [getattr(self, 'COMPILER_FAMILY', None)]) + current_compiler_names = self.COMPILER_MODULE_NAME or [] + if self.COMPILER_FAMILY: + current_compiler_names.append(self.COMPILER_FAMILY) + compiler_optarch = None for current_compiler in current_compiler_names: if current_compiler in optarch: - optarch = optarch[current_compiler] + compiler_optarch = optarch[current_compiler] break - # still a dict: no option for this compiler - if isinstance(optarch, dict): - optarch = None + if compiler_optarch is None: self.log.info("_set_optimal_architecture: no optarch found for compiler %s. Ignoring option.", - current_compiler) + current_compiler_names) + optarch = self._pick_optarch_entry(compiler_optarch) if isinstance(optarch, string_type): use_generic = (optarch == OPTARCH_GENERIC) diff --git a/test/framework/options.py b/test/framework/options.py index b2f3da43c1..8a1294f0b8 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -5611,10 +5611,14 @@ def test_parse_optarch(self): options.options.optarch = 'Intel:something:somethingelse' self.assertErrorRegex(EasyBuildError, error_msg, options.postprocess) - error_msg = "The optarch option contains duplicated entries for compiler" + error_msg = "The optarch option contains a duplicated entry for compiler" options.options.optarch = 'Intel:something;GCC:somethingelse;Intel:anothersomething' self.assertErrorRegex(EasyBuildError, error_msg, options.postprocess) + error_msg = "'x86' of 'x86' is not in allowed values" + options.options.optarch = 'Intel,x86:something' + self.assertErrorRegex(EasyBuildError, error_msg, options.postprocess) + # Check the parsing itself gcc_generic_flags = "march=x86-64 -mtune=generic" test_cases = [ @@ -5624,7 +5628,26 @@ def test_parse_optarch(self): ('Intel:xHost', {'Intel': 'xHost'}), ('Intel:GENERIC', {'Intel': 'GENERIC'}), ('Intel:xHost;GCC:%s' % gcc_generic_flags, {'Intel': 'xHost', 'GCC': gcc_generic_flags}), - ('Intel:;GCC:%s' % gcc_generic_flags, {'Intel': '', 'GCC': gcc_generic_flags}), + # Allow empty values and spaces + ('Intel:; GCC:%s' % gcc_generic_flags, {'Intel': '', 'GCC': gcc_generic_flags}), + # Arch specific values. With Compiler only selection at different places in the list + ( + 'Intel,x86_64,Intel,sse:i86iSSE; Intel,x86_64,AMD:i86AMD; Intel,POWER:iPwr; Intel:xHost; ' + 'GCC:GENERIC; Clang:cl; Clang,POWER:clPwr', + { + 'Intel': { + ('x86_64', 'Intel', 'sse'): 'i86iSSE', + ('x86_64', 'AMD'): 'i86AMD', + 'POWER': 'iPwr', + None: 'xHost', + }, + 'GCC': 'GENERIC', + 'Clang': { + None: 'cl', + 'POWER': 'clPwr', + }, + }, + ), ] for optarch_string, optarch_parsed in test_cases: diff --git a/test/framework/systemtools.py b/test/framework/systemtools.py index 9c2c415cb3..6875181787 100644 --- a/test/framework/systemtools.py +++ b/test/framework/systemtools.py @@ -984,16 +984,24 @@ def test_pick_opt_arch(self): avx = [st.SSE, st.SSE2, st.AVX] avx2 = avx + [st.AVX2] avx512 = avx2 + [st.AVX512F] - self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.INTEL, avx2), 'opt-x86') - self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.INTEL, []), 'opt-x86') - self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx), 'opt-amd') - self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx2), 'opt-amd-avx2') - self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx512), 'opt-amd-avx2') - self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.AMD, avx2), 'opt-aarch64') - self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.INTEL, avx2), 'opt-aarch64-sse') - self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.INTEL, [st.SSE]), 'opt-aarch64-sse') - self.assertEqual(st.pick_opt_arch(options, st.AARCH32, st.AMD, avx2), None) - self.assertEqual(st.pick_opt_arch(options, st.POWER, st.IBM, []), None) + for include_none in (False, True): + if include_none: + # Special fallback option when no entry matches + options[None] = 'opt-none' + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.INTEL, avx2), 'opt-x86') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.INTEL, []), 'opt-x86') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx), 'opt-amd') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx2), 'opt-amd-avx2') + self.assertEqual(st.pick_opt_arch(options, st.X86_64, st.AMD, avx512), 'opt-amd-avx2') + self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.AMD, avx2), 'opt-aarch64') + self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.INTEL, avx2), 'opt-aarch64-sse') + self.assertEqual(st.pick_opt_arch(options, st.AARCH64, st.INTEL, [st.SSE]), 'opt-aarch64-sse') + if include_none: + self.assertEqual(st.pick_opt_arch(options, st.AARCH32, st.AMD, avx2), 'opt-none') + self.assertEqual(st.pick_opt_arch(options, st.POWER, st.IBM, []), 'opt-none') + else: + self.assertEqual(st.pick_opt_arch(options, st.AARCH32, st.AMD, avx2), None) + self.assertEqual(st.pick_opt_arch(options, st.POWER, st.IBM, []), None) def test_check_os_dependency(self): """Test check_os_dependency.""" From ff08ad7115bb91963114fde6e09b2cdc01aacbfa Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Aug 2021 16:28:43 +0200 Subject: [PATCH 4/6] Allow arch-specific optarch in ECs --- easybuild/tools/options.py | 66 +----------- easybuild/tools/toolchain/compiler.py | 140 +++++++++++++++++++++----- test/framework/options.py | 10 +- test/framework/toolchain.py | 26 +++-- test/framework/utilities.py | 11 ++ 5 files changed, 159 insertions(+), 94 deletions(-) diff --git a/easybuild/tools/options.py b/easybuild/tools/options.py index 65432ed9a9..44ed508b1b 100644 --- a/easybuild/tools/options.py +++ b/easybuild/tools/options.py @@ -47,7 +47,7 @@ import pwd import easybuild.tools.environment as env -from easybuild.base import fancylogger # build_log should always stay there, to ensure EasyBuildLog +from easybuild.base import fancylogger from easybuild.base.fancylogger import setLogLevel from easybuild.base.generaloption import GeneralOption from easybuild.framework.easyblock import MODULE_ONLY_STEPS, SOURCE_STEP, FETCH_STEP, EasyBlock @@ -101,11 +101,10 @@ from easybuild.tools.robot import det_robot_path from easybuild.tools.run import run_cmd from easybuild.tools.package.utilities import avail_package_naming_schemes -from easybuild.tools.toolchain.compiler import DEFAULT_OPT_LEVEL, OPTARCH_MAP_CHAR, OPTARCH_SEP, Compiler +from easybuild.tools.toolchain.compiler import DEFAULT_OPT_LEVEL, Compiler, parse_optarch_string from easybuild.tools.toolchain.toolchain import SYSTEM_TOOLCHAIN_NAME from easybuild.tools.repository.repository import avail_repositories -from easybuild.tools.systemtools import UNKNOWN, CPU_ARCHITECTURES, CPU_FAMILIES, CPU_VECTOR_EXTS -from easybuild.tools.systemtools import check_python_version, get_cpu_architecture, get_cpu_family +from easybuild.tools.systemtools import UNKNOWN, check_python_version, get_cpu_architecture, get_cpu_family from easybuild.tools.systemtools import get_cpu_features, get_gpu_info, get_system_info from easybuild.tools.version import this_is_easybuild @@ -990,65 +989,8 @@ def postprocess(self): def _postprocess_optarch(self): """ Postprocess --optarch option. - - Format: map := (;*)* - entry := (, - Example values: - "GENERIC" - "march=native" - "Intel,x86:xHost; Intel,x86,AMD,AVX2:mavx2 -fma; GCC:march=native" """ - # Split into entries, and each entry into key-value pairs - optarch_parts = [i.strip().split(OPTARCH_MAP_CHAR) for i in self.options.optarch.split(OPTARCH_SEP)] - - # We should either have a single value or a list of key-value pairs, and nothing else - is_single_value = len(optarch_parts) == 1 and len(optarch_parts[0]) == 1 - if not is_single_value and any(len(i) != 2 for i in optarch_parts): - raise EasyBuildError("The optarch option has an incorrect syntax: %s", self.options.optarch) - - # if there are options for different compilers, we set up a dict - if is_single_value: - # if optarch is not in mapping format, we do nothing and just keep the string - self.log.info("Keeping optarch raw: %s", self.options.optarch) - else: - optarch_dict = {} - for key, compiler_opt in optarch_parts: - # key can be either a compiler (only) or compiler and archspec(s) - key_parts = key.split(',') - compiler = key_parts.pop(0) - if key_parts: - for part, allowed_values in zip(key_parts, (CPU_ARCHITECTURES, CPU_FAMILIES, CPU_VECTOR_EXTS)): - if part not in allowed_values: - raise EasyBuildError("The optarch option has an incorrect syntax: %s\n" - "'%s' of '%s' is not in allowed values: %s", - self.options.optarch, part, key, allowed_values) - arch_specs = tuple(key_parts) if len(key_parts) > 1 else key_parts[0] - else: - arch_specs = None - try: - compiler_dict = optarch_dict[compiler] - # Convert single entry to dict if required - if not isinstance(compiler_dict, dict): - compiler_dict = {None: compiler_dict} - optarch_dict[compiler] = compiler_dict - if arch_specs in compiler_dict: - if arch_specs is None: - raise EasyBuildError("The optarch option contains a duplicated entry for compiler %s: %s", - compiler, self.options.optarch) - else: - raise EasyBuildError("The optarch option contains a duplicated entry %s " - "for compiler %s: %s", - arch_specs, compiler, self.options.optarch) - else: - compiler_dict[arch_specs] = compiler_opt - except KeyError: - # Keep the dict flat when no archspecs are given - if arch_specs is None: - optarch_dict[compiler] = compiler_opt - else: - optarch_dict[compiler] = {arch_specs: compiler_opt} - self.options.optarch = optarch_dict - self.log.info("Transforming optarch into a dict: %s", self.options.optarch) + self.options.optarch = parse_optarch_string(self.options.optarch, include_compiler=True) def _postprocess_close_pr_reasons(self): """Postprocess --close-pr-reasons options""" diff --git a/easybuild/tools/toolchain/compiler.py b/easybuild/tools/toolchain/compiler.py index f015027fb5..5e6494fcfd 100644 --- a/easybuild/tools/toolchain/compiler.py +++ b/easybuild/tools/toolchain/compiler.py @@ -31,10 +31,12 @@ * Kenneth Hoste (Ghent University) * Damian Alvarez (Forschungszentrum Juelich GmbH) """ +from easybuild.base import fancylogger from easybuild.tools import systemtools from easybuild.tools.build_log import EasyBuildError from easybuild.tools.config import build_option from easybuild.tools.py2vs3 import string_type +from easybuild.tools.systemtools import CPU_ARCHITECTURES, CPU_FAMILIES, CPU_VECTOR_EXTS from easybuild.tools.toolchain.constants import COMPILER_VARIABLES from easybuild.tools.toolchain.toolchain import Toolchain @@ -49,6 +51,8 @@ OPTARCH_SEP = ';' OPTARCH_MAP_CHAR = ':' +_log = fancylogger.getLogger('compiler', fname=False) + def mk_infix(prefix): """Create an infix based on the given prefix.""" @@ -58,6 +62,99 @@ def mk_infix(prefix): return infix +def parse_optarch_string(optarch, include_compiler): + """ + Parse an optarch string into a dict if multiple options are found, else keeps it as a string + + Format: optarch := | + map := (;*)* + include_compiler is True: + entry := (,(,?(,)?)?)?: + include_compiler is False: + entry := (,(,)?)?: + Example values: + "GENERIC" + "march=native" + "Intel,x86:xHost; Intel,x86,AMD,AVX2:mavx2 -fma; GCC:march=native" + """ + # Split into entries, and each entry into key-value pairs + optarch_parts = [i.strip().split(OPTARCH_MAP_CHAR) for i in optarch.split(OPTARCH_SEP)] + + # We should either have a single value or a list of key-value pairs, and nothing else + is_single_value = len(optarch_parts) == 1 and len(optarch_parts[0]) == 1 + if not is_single_value and any(len(i) != 2 for i in optarch_parts): + raise EasyBuildError("The optarch option has an incorrect syntax: %s", optarch) + + # if there are options for different compilers, we set up a dict + if is_single_value: + # if optarch is not in mapping format, we do nothing and just keep the string + _log.info("Keeping optarch raw: %s", optarch) + else: + optarch_dict = {} + for key, compiler_opt in optarch_parts: + # key can be either a compiler (only) or compiler and archspec(s) + key_parts = key.split(',') + if include_compiler: + compiler = key_parts.pop(0) + elif not key: + # When no compiler is present an empty key is the default fallback + key_parts = None + if key_parts: + # Validate + allowed_values_list = (CPU_ARCHITECTURES, CPU_FAMILIES, CPU_VECTOR_EXTS) + if len(key_parts) > len(allowed_values_list): + raise EasyBuildError("The optarch option has an incorrect syntax: %s\n" + "'%s' should be of the format: ,, " + "where trailing elements can be ommitted.", + optarch, key_parts) + for i, (part, allowed_values) in enumerate(zip(key_parts, allowed_values_list)): + if part not in allowed_values: + # Try again fixing up casing + try: + idx = [value.lower() for value in allowed_values].index(part.lower()) + except ValueError: # Thrown when item does not exist + raise EasyBuildError("The optarch option has an incorrect syntax: %s\n" + "'%s' of '%s' is not in allowed values: %s", + optarch, part, key, allowed_values) + key_parts[i] = allowed_values[idx] + arch_specs = tuple(key_parts) if len(key_parts) > 1 else key_parts[0] + else: + arch_specs = None + # If we include the compiler, the optarch dict has it as the first level + # Else we use the optarch dict directly + if include_compiler: + try: + compiler_dict = optarch_dict[compiler] + except KeyError: + # Keep the dict flat when no archspecs are given + if arch_specs is None: + optarch_dict[compiler] = compiler_opt + else: + optarch_dict[compiler] = {arch_specs: compiler_opt} + # All done + continue + # Convert single entry to dict if required + if not isinstance(compiler_dict, dict): + compiler_dict = {None: compiler_dict} + optarch_dict[compiler] = compiler_dict + else: + compiler_dict = optarch_dict + # Disallow duplicates + if arch_specs in compiler_dict: + if arch_specs is None: + raise EasyBuildError("The optarch option contains a duplicated entry for compiler %s: %s", + compiler, optarch) + else: + raise EasyBuildError("The optarch option contains a duplicated entry %s " + "for compiler %s: %s", + arch_specs, compiler, optarch) + else: + compiler_dict[arch_specs] = compiler_opt + _log.info("Transforming optarch into a dict: %s", optarch_dict) + optarch = optarch_dict + return optarch + + class Compiler(Toolchain): """General compiler-like class can't be used without creating new class C(Compiler,Toolchain) @@ -337,32 +434,29 @@ def _set_optimal_architecture(self, default_optarch=None): """ ec_optarch = self.options.get('optarch') if isinstance(ec_optarch, string_type): - if OPTARCH_MAP_CHAR in ec_optarch: - error_msg = "When setting optarch in the easyconfig (found %s), " % ec_optarch - error_msg += "the syntax is not allowed. " % OPTARCH_MAP_CHAR - error_msg += "Use (omitting the first dash) for the specific compiler." - raise EasyBuildError(error_msg) - else: - optarch = ec_optarch + optarch = parse_optarch_string(ec_optarch, include_compiler=False) + optarch = self._pick_optarch_entry(optarch) else: + # TODO: Maybe parse optarch from cmdline here? optarch = build_option('optarch') - # --optarch is specified with flags to use - if isinstance(optarch, dict): - # optarch has been validated as complex string with multiple compilers and converted to a dictionary - # first try module names, then the family in optarch - current_compiler_names = self.COMPILER_MODULE_NAME or [] - if self.COMPILER_FAMILY: - current_compiler_names.append(self.COMPILER_FAMILY) - compiler_optarch = None - for current_compiler in current_compiler_names: - if current_compiler in optarch: - compiler_optarch = optarch[current_compiler] - break - if compiler_optarch is None: - self.log.info("_set_optimal_architecture: no optarch found for compiler %s. Ignoring option.", - current_compiler_names) - optarch = self._pick_optarch_entry(compiler_optarch) + # --optarch is specified with flags to use + if isinstance(optarch, dict): + # optarch has been validated as complex string with multiple compilers and converted to a dictionary + # first try module names, then the family in optarch + current_compiler_names = self.COMPILER_MODULE_NAME or [] + if self.COMPILER_FAMILY: + # Create new list to NOT modify self.COMPILER_MODULE_NAME + current_compiler_names = current_compiler_names + [self.COMPILER_FAMILY] + compiler_optarch = None + for current_compiler in current_compiler_names: + if current_compiler in optarch: + compiler_optarch = optarch[current_compiler] + break + if compiler_optarch is None: + self.log.info("_set_optimal_architecture: no optarch found for compiler %s. Ignoring option.", + current_compiler_names) + optarch = self._pick_optarch_entry(compiler_optarch) if isinstance(optarch, string_type): use_generic = (optarch == OPTARCH_GENERIC) diff --git a/test/framework/options.py b/test/framework/options.py index 8a1294f0b8..957e451d42 100644 --- a/test/framework/options.py +++ b/test/framework/options.py @@ -5615,7 +5615,7 @@ def test_parse_optarch(self): options.options.optarch = 'Intel:something;GCC:somethingelse;Intel:anothersomething' self.assertErrorRegex(EasyBuildError, error_msg, options.postprocess) - error_msg = "'x86' of 'x86' is not in allowed values" + error_msg = "'x86' of 'Intel,x86' is not in allowed values" options.options.optarch = 'Intel,x86:something' self.assertErrorRegex(EasyBuildError, error_msg, options.postprocess) @@ -5648,6 +5648,14 @@ def test_parse_optarch(self): }, }, ), + ( + # Case insensitive arch values + 'Intel,X86_64,intel,SSE:i86iSSE; Clang,POWER:clPwr', + { + 'Intel': {('x86_64', 'Intel', 'sse'): 'i86iSSE'}, + 'Clang': {'POWER': 'clPwr'}, + }, + ), ] for optarch_string, optarch_parsed in test_cases: diff --git a/test/framework/toolchain.py b/test/framework/toolchain.py index 0e53639491..e63c45875e 100644 --- a/test/framework/toolchain.py +++ b/test/framework/toolchain.py @@ -36,7 +36,7 @@ import tempfile from itertools import product from unittest import TextTestRunner -from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, find_full_path, init_config +from test.framework.utilities import EnhancedTestCase, TestLoaderFiltered, find_full_path, init_config, mock_object import easybuild.tools.modules as modules import easybuild.tools.toolchain as toolchain @@ -867,16 +867,26 @@ def test_easyconfig_optarch_flags(self): test_ec = os.path.join(self.test_prefix, 'test.eb') toy_txt = read_file(eb_file) - # check that an optarch map raises an error - write_file(test_ec, toy_txt + "\ntoolchainopts = {'optarch': 'GCC:march=sandrybridge;Intel:xAVX'}") - msg = "syntax is not allowed" + # check that using compilers in optarch raises an error + write_file(test_ec, toy_txt + "\ntoolchainopts = {'optarch': 'GCC:march=sandybridge;Intel:xAVX'}") + msg = "The optarch option has an incorrect syntax" self.assertErrorRegex(EasyBuildError, msg, self.eb_main, [test_ec], raise_error=True, do_build=True) # check that setting optarch flags work - write_file(test_ec, toy_txt + "\ntoolchainopts = {'optarch': 'march=sandybridge'}") - out = self.eb_main([test_ec], raise_error=True, do_build=True) - regex = re.compile("_set_optimal_architecture: using march=sandybridge as optarch for x86_64") - self.assertTrue(regex.search(out), "Pattern '%s' found in: %s" % (regex.pattern, out)) + test_cases = [ + ('march=sandybridge', 'march=sandybridge'), + # ,, + ('POWER,POWER:march=ppc; x86_64,Intel,AVX:march=avx', 'march=avx'), + ('POWER,POWER:march=ppc; x86_64,Intel,AVX2:march=avx; :GENERIC', 'march=x86-64 -mtune=generic'), + ] + with mock_object(st, 'get_cpu_arch_name', lambda: st.X86_64), \ + mock_object(st, 'get_cpu_family', lambda: st.INTEL), \ + mock_object(st, 'get_cpu_vector_exts', lambda: [st.SSE, st.SSE2, st.AVX]): + for optarch, expected in test_cases: + write_file(test_ec, toy_txt + "\ntoolchainopts = {'optarch': '%s'}" % optarch) + out = self.eb_main([test_ec, '--extended-dry-run'], raise_error=True, do_build=True) + msg = "_set_optimal_architecture: using %s as optarch for x86_64" % expected + self.assertIn(msg, out) def test_misc_flags_unique_fortran(self): """Test whether unique Fortran compiler flags are set correctly.""" diff --git a/test/framework/utilities.py b/test/framework/utilities.py index d07c2202b1..9338bc4d1a 100644 --- a/test/framework/utilities.py +++ b/test/framework/utilities.py @@ -80,6 +80,17 @@ os.environ[newkey] = val +@contextmanager +def mock_object(target, attribute, new_value): + """Patch the named attribute/member of the given target object with the new_value""" + old_value = getattr(target, attribute) + setattr(target, attribute, new_value) + try: + yield + finally: + setattr(target, attribute, old_value) + + class EnhancedTestCase(TestCase): """Enhanced test case, provides extra functionality (e.g. an assertErrorRegex method).""" From 68fdedbe4b9f8e4293457edcfa49da99f9c803c7 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Aug 2021 18:41:12 +0200 Subject: [PATCH 5/6] Restore tmp dir after init_config call in test --- test/framework/toolchain.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/framework/toolchain.py b/test/framework/toolchain.py index e63c45875e..469763f1a8 100644 --- a/test/framework/toolchain.py +++ b/test/framework/toolchain.py @@ -808,10 +808,18 @@ def test_compiler_dependent_optarch(self): optarch_var['Intel'] = intel_flags optarch_var['GCC'] = gcc_flags optarch_var['GCCcore'] = gcccore_flags + + # Save current tmp dir + old_vars = {name: os.environ.get(name) for name in ('TMPDIR', 'TEMP', 'TMP')} + init_config(build_options={'optarch': optarch_var, 'silent': True}) tc = self.get_toolchain(toolchain_name, version=toolchain_ver) tc.set_options({'optarch': enable}) tc.prepare() + + # Restore tmp dir + os.environ.update(old_vars) + flags = None if toolchain_name == 'iccifort': flags = intel_flags_exp From 3a17370c7377de6659da823725264c8f2d568f56 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Fri, 12 May 2023 13:35:48 +0200 Subject: [PATCH 6/6] Increase lock timeout in test Avoid spurious failures if the lock gets removed before the code checks for it. --- test/framework/toy_build.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/framework/toy_build.py b/test/framework/toy_build.py index 1bd114d9d7..61cd8c5f38 100644 --- a/test/framework/toy_build.py +++ b/test/framework/toy_build.py @@ -3560,8 +3560,8 @@ def __exit__(self, type, value, traceback): all_args = extra_args + opts - # use context manager to remove lock after 3 seconds - with RemoveLockAfter(3, toy_lock_path): + # use context manager to remove lock after 5 seconds + with RemoveLockAfter(5, toy_lock_path): self.mock_stderr(True) self.mock_stdout(True) self.test_toy_build(extra_args=all_args, verify=False, raise_error=True, testing=False) @@ -3576,7 +3576,7 @@ def __exit__(self, type, value, traceback): wait_matches = wait_regex.findall(stdout) # we can't rely on an exact number of 'waiting' messages, so let's go with a range... - self.assertIn(len(wait_matches), range(1, 5)) + self.assertIn(len(wait_matches), range(1, 7)) self.assertTrue(ok_regex.search(stdout), "Pattern '%s' found in: %s" % (ok_regex.pattern, stdout))