-
Notifications
You must be signed in to change notification settings - Fork 306
A path to making OpenBLAS optarch aware. #1946
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,21 +3,43 @@ | |
|
|
||
| @author: Andrew Edmondson (University of Birmingham) | ||
| @author: Alex Domingo (Vrije Universiteit Brussel) | ||
| @author: Terje Kvernes (University of Oslo) | ||
| """ | ||
| import os | ||
| import re | ||
| from distutils.version import LooseVersion | ||
| from easybuild.base import fancylogger | ||
| from easybuild.easyblocks.generic.configuremake import ConfigureMake | ||
| from easybuild.framework.easyconfig import CUSTOM | ||
| from easybuild.tools.build_log import EasyBuildError, print_warning | ||
| from easybuild.tools.config import ERROR, build_option | ||
| from easybuild.tools.filetools import read_file | ||
| from easybuild.tools.systemtools import POWER, get_cpu_architecture, get_shared_lib_ext | ||
| from easybuild.tools.build_log import print_warning | ||
| from easybuild.tools.config import ERROR | ||
| from easybuild.tools.run import run_cmd, check_log_for_errors | ||
|
|
||
| _log = fancylogger.getLogger('systemtools', fname=False) | ||
|
|
||
| try: | ||
| from archspec import cpu as archspec_cpu | ||
| HAVE_ARCHSPEC = True | ||
| except ImportError as err: | ||
| _log.debug("Failed to import 'archspec' Python module: %s", err) | ||
| HAVE_ARCHSPEC = False | ||
|
|
||
| TARGET = 'TARGET' | ||
|
|
||
|
|
||
| class EB_OpenBLAS(ConfigureMake): | ||
| """Support for building/installing OpenBLAS.""" | ||
|
|
||
| @staticmethod | ||
| def extra_options(): | ||
| """Custom easyconfig parameters for OpenBLAS""" | ||
| extra_vars = { | ||
| 'targetfile': ['TargetList.txt', "File containing OpenBLAS target list", CUSTOM], | ||
| } | ||
| return ConfigureMake.extra_options(extra_vars) | ||
|
|
||
| def configure_step(self): | ||
| """ set up some options - but no configure command to run""" | ||
|
|
||
|
|
@@ -29,6 +51,15 @@ def configure_step(self): | |
| 'USE_THREAD': '1', | ||
| } | ||
|
|
||
| # Handle the possibility of setting the target architecture as dynamic, | ||
| # where OpenBLAS will optimize the kernel at runtime. | ||
| self._dynamic_target = False | ||
|
|
||
| compiler_optarch = self._optarch_for_compiler(build_option('optarch')) | ||
|
|
||
| # Retain the (m)arch part of the optarch settings across the entire object. | ||
| self._optarch_architecture = compiler_optarch | ||
|
|
||
| if '%s=' % TARGET in self.cfg['buildopts']: | ||
| # Add any TARGET in buildopts to default_opts, so it is passed to testopts and installopts | ||
| for buildopt in self.cfg['buildopts'].split(): | ||
|
|
@@ -39,6 +70,38 @@ def configure_step(self): | |
| # There doesn't seem to be a POWER9 option yet, but POWER8 should work. | ||
| print_warning("OpenBLAS 0.3.5 and lower have known issues on POWER systems") | ||
| default_opts[TARGET] = 'POWER8' | ||
| elif compiler_optarch: | ||
| compiler_family = self.toolchain.comp_family() | ||
| self.log.info("EasyBuild full optarch requested for %s: %s" % (compiler_family, compiler_optarch)) | ||
| optarch_as_target = self._parse_optarch(compiler_optarch) | ||
| mapped_target = None | ||
|
|
||
| if optarch_as_target: | ||
| # Note that _parse_optarch returns lowercased results, so GENERIC has become 'generic'. | ||
| if optarch_as_target == 'generic': | ||
| self._set_dynamic_architecture(default_opts) | ||
| mapped_target = 'generic' | ||
| else: | ||
| self.log.info("EasyBuild march: %s" % optarch_as_target) | ||
| openblas_targets = self._get_openblas_targets(self.cfg['targetfile']) | ||
| mapped_target = self._get_mapped_target(optarch_as_target, openblas_targets) | ||
| else: | ||
| self.log.info("Optarch specified for %s, but no march detected", compiler_family) | ||
|
|
||
| if mapped_target is None: | ||
| print_warning("optarch for %s given as '%s'\n" | ||
| "EasyBuild was unable to map this to an equivalent OpenBLAS target!\n" | ||
| "OpenBLAS will be built to optimize its kernel at runtime!\n" | ||
| % (self.toolchain.comp_family(), compiler_optarch)) | ||
| self.log.warning("Unable to map %s to an OpenBLAS target, falling back to runtime optimization." | ||
| % compiler_optarch) | ||
| self._set_dynamic_architecture(default_opts) | ||
| elif mapped_target == 'generic': | ||
| self.log.info("Optarch 'GENERIC' requested, will enable runtime optimization.") | ||
| else: | ||
| mapped_target = mapped_target.upper() | ||
| self.log.info("Optarch mapped between EasyBuild and OpenBLAS to: " + mapped_target) | ||
| default_opts[TARGET] = mapped_target | ||
|
|
||
| for key in sorted(default_opts.keys()): | ||
| for opts_key in ['buildopts', 'testopts', 'installopts']: | ||
|
|
@@ -57,6 +120,17 @@ def build_step(self): | |
| build_parts += ['re_lapack'] | ||
| build_parts += ['shared'] | ||
|
|
||
| # If we're doing either a dynamic build or utilizing optarch, | ||
| # strip march from all environment variables except the EBVAR-prefixed ones. | ||
| # For dynamic builds we should ignore optarch completely and for optarch-set builds | ||
| # we need to adhere to TARGET and not march. | ||
| if self._dynamic_target is True or self._optarch_architecture is True: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In which case can
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, yeah. I think this was a suggested change from the editor of best practices for python. There was something odd about that. I'll check it out. |
||
| self.log.info('Dynamic build requested, stripping march settings from environment variables') | ||
| for k in os.environ.keys(): | ||
| optarch_to_strip = '-' + self._optarch_architecture | ||
| if 'EBVAR' not in k and self._optarch_architecture in os.environ[k]: | ||
| os.environ[k] = os.environ[k].replace(optarch_to_strip, '') | ||
|
|
||
| # Pass CFLAGS through command line to avoid redefinitions (issue xianyi/OpenBLAS#818) | ||
| cflags = 'CFLAGS' | ||
| if os.environ[cflags]: | ||
|
|
@@ -72,7 +146,7 @@ def build_step(self): | |
| run_cmd(cmd, log_all=True, simple=True) | ||
|
|
||
| def test_step(self): | ||
| """ Mandatory test step plus optional runtest""" | ||
| """ Mandatory test step plus optional runtest """ | ||
|
|
||
| run_tests = ['tests'] | ||
| if self.cfg['runtest']: | ||
|
|
@@ -94,3 +168,132 @@ def sanity_check_step(self): | |
| 'dirs': [], | ||
| } | ||
| super(EB_OpenBLAS, self).sanity_check_step(custom_paths=custom_paths) | ||
|
|
||
| def _optarch_for_compiler(self, optarch): | ||
| """ | ||
| Extracts the appropriate optarch for the compiler currently being used. | ||
| If it is not compiler-specific it is returned as-is. | ||
| If no optarch is found, False is returned. | ||
| :param optarch: A complete optarch statement. | ||
| https://easybuild.readthedocs.io/en/latest/Controlling_compiler_optimization_flags.html | ||
| """ | ||
| if optarch is False: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For me (on POWER) optarch is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, certainly. |
||
| return False | ||
|
|
||
| compiler = self.toolchain.comp_family() | ||
| compiler_specific_optarch_string = '' | ||
|
|
||
| if type(optarch) == str: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can (and IIRC should) use
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, will do. |
||
| compiler_specific_optarch_string = optarch | ||
| elif type(optarch) == dict: | ||
| if compiler in optarch: | ||
| compiler_specific_optarch_string = optarch[compiler] | ||
| else: | ||
| raise EasyBuildError("optarch in an unexpected format: '%s'" % type(optarch)).__class__.__name__ | ||
terjekv marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please remove the |
||
|
|
||
| return compiler_specific_optarch_string | ||
|
|
||
| def _parse_optarch(self, compiler_optarch): | ||
| """ | ||
| Pick the march out of a given optarch. | ||
| Note that the result is lowercased. | ||
| :param compiler_optarch: An optarch for a given compiler. | ||
| https://easybuild.readthedocs.io/en/latest/Controlling_compiler_optimization_flags.html | ||
| """ | ||
|
|
||
| target_arch = '' | ||
| pieces = compiler_optarch.split() | ||
|
|
||
| for piece in pieces: | ||
| spec = piece.split('=') | ||
| if spec[0] == 'march' or spec[0] == '-march': | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. -mcpu is used, but it is a deprecated synonym for -mtune, which only takes two values on x86, generic and Intel, neither of which have an effect on OpenBLAS targeting, which only cares about CPU architecture. I have no idea what this option does on Power. :(
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See https://stackoverflow.com/questions/42718572/gcc-mtune-vs-march-vs-mcpu
|
||
| target_arch = spec[1] | ||
|
|
||
| return target_arch.lower() | ||
|
|
||
| def _get_openblas_targets(self, targetfile): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For all those functions: Could you make them free functions? In PyTorch/TensorFlow I found it a good idea so you can just import this from regular python and run/test it
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lexming (I think) suggested generalising a number of these functions and shipping them off to Framework. The ones that are OpenBLAS specific should probably suffer a similar fate. This will require another rewrite though, not sure about the ETA on that. |
||
| """ | ||
| Parse the openblas target file and generate a list of targets. | ||
| :param targetfile: A file with OpenBLAS targets. | ||
| """ | ||
| targets = [] | ||
|
|
||
| if os.path.isfile(targetfile): | ||
| # Assumption, the OpenBLAS TargetList.txt has one target per line and that | ||
| # single words on a line is a target if they match a simple regexp... | ||
| re_target = re.compile(r'^[A-Z0-9_]+$') | ||
| for line in read_file(targetfile).splitlines(): | ||
| match = re_target.match(line) | ||
| if match is not None: | ||
| targets.append(line.strip().lower()) | ||
| else: | ||
| print_warning("Unable to find OpenBLAS targetfile '%s'" % os.path.realpath(targetfile)) | ||
|
|
||
| return targets | ||
|
|
||
| def _set_dynamic_architecture(self, default_opts): | ||
| """ | ||
| Sets the DYNAMIC_ARCH option for OpenBLAS, building a library that chooses | ||
| an optimized kernel at runtime. Also removes any previous TARGET setting, if any. | ||
| """ | ||
| default_opts['DYNAMIC_ARCH'] = 1 | ||
| default_opts.pop(TARGET, None) | ||
| self._dynamic_target = True | ||
|
|
||
| def _get_mapped_target(self, march, openblas_targets): | ||
| """ | ||
| Attempts to match the given march in the list of openblas targets. | ||
| If archspec is installed, will try to match directly or follow ancestors for | ||
| architectures that will work. | ||
| Returns None if no target was found. | ||
| """ | ||
|
|
||
| result = None | ||
|
|
||
| if HAVE_ARCHSPEC: | ||
| self.log.info("Using archspec to match optarch to openblas targets.") | ||
|
|
||
| openblas_arch = set(['alpha', 'arm', 'ia64', 'mips', 'mips64', | ||
| 'power', 'sparc', 'zarch']) | ||
| openblas_arch_map = { | ||
| 'amd64': 'x86_64', | ||
| 'powerpc64': 'power', | ||
| 'i386': 'x86', | ||
| 'aarch64': 'arm64', | ||
| } | ||
| openblas_arch.update(openblas_arch_map.keys()) | ||
| openblas_arch.update(openblas_arch_map.values()) | ||
|
|
||
| skylake = set(["skylake"]) | ||
| available_targets = set(openblas_targets) | skylake | openblas_arch | ||
|
|
||
| try: | ||
| uarch = archspec_cpu.TARGETS[march] | ||
| except KeyError: | ||
| warning_string = "Archspec was asked to find '" + march + "' as an architecture, but failed!" | ||
| print_warning(warning_string) | ||
| self.log.warning(warning_string) | ||
| return None | ||
|
|
||
| if uarch.name in available_targets: | ||
| result = uarch.name | ||
| else: | ||
| self.log.info("No direct match for '" + march + "' between archspec and OpenBLAS, traversing ancestry.") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we use formatted logs instead of string concatting?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably? I'll have to look at that. |
||
| for uarch in uarch.ancestors: | ||
| if uarch.name in available_targets: | ||
| self.log.info("Ancestral match between '" + march + "' and '" + uarch.name + "'.") | ||
| result = uarch.name | ||
| break | ||
|
|
||
| # Skylake for OpenBLAS is called 'skylakex'. Handle this exception exceptionally. | ||
| if result == 'skylake': | ||
| result = 'skylakex' | ||
|
|
||
| else: | ||
| self.log.info("Unable to find archspec, optarch matching will be poor.") | ||
| if march == 'skylake': | ||
| result = 'skylakex' | ||
| elif march in openblas_targets: | ||
| result = march | ||
|
|
||
| return result | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm very unsure about this. If I build OpenBLAS currently, it seems to autodetect the current arch and optimizes for it. It seems to even overwrite our optarch settings etc which is fine when building for native. Additionally e.g. for POWER and OpenBLAS until 0.3.7 (IIRC) DYNAMIC_ARCH is broken to the point where even the simple tests fail (so in this case the EC wouldn't build)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks! With DYNAMIC_ARCH being broken on POWER, we should certainly it for Power.
But the thing is, if you ask for
optarch=haswelland OpenBLAS optimises for native, and you're on a skylake build host, your compute nodes will cease to function. For me (and I presume others),optarchis not something that may be ignored.My strong preference is that if an EasyBuild user sets
optarch=haswellEasyBuild offers the guarantee that the code produced will indeed run on haswell. If this can't be done for an EasyConfig, it is, at least for me, vastly preferable for the EasyConfig to fail during build.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright sure. I just wanted to highlight that the default (native) should still work as before (hence my restriction to "when building for native")