From 2ab1f54a343df8fa20ee98e490c60dbb95805983 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Sat, 15 Feb 2025 18:11:07 +1100 Subject: [PATCH] Started to add new ProfileFlags to compiler. --- source/fab/build_config.py | 8 +++ source/fab/steps/link.py | 2 +- source/fab/tools/__init__.py | 3 +- source/fab/tools/compiler.py | 10 +++- source/fab/tools/compiler_wrapper.py | 7 ++- source/fab/tools/flags.py | 85 +++++++++++++++++++++++++++- source/fab/tools/linker.py | 11 +++- tests/unit_tests/tools/test_flags.py | 78 ++++++++++++++++++++++++- 8 files changed, 195 insertions(+), 9 deletions(-) diff --git a/source/fab/build_config.py b/source/fab/build_config.py index c98c8d9b..0a0dc6d1 100644 --- a/source/fab/build_config.py +++ b/source/fab/build_config.py @@ -44,6 +44,7 @@ def __init__(self, project_label: str, tool_box: ToolBox, mpi: bool = False, openmp: bool = False, + profile: str = "default", multiprocessing: bool = True, n_procs: Optional[int] = None, reuse_artefacts: bool = False, @@ -63,6 +64,7 @@ def __init__(self, project_label: str, (if none is explicitly set in the ToolBox). The compiler-specific flag to enable OpenMP will automatically be added when compiling and linking. + :param profile: the name of a compiler profile to use. :param multiprocessing: An option to disable multiprocessing to aid debugging. :param n_procs: @@ -87,6 +89,7 @@ def __init__(self, project_label: str, self._tool_box = tool_box self._mpi = mpi self._openmp = openmp + self._profile = profile self.two_stage = two_stage self.verbose = verbose compiler = tool_box.get_tool(Category.FORTRAN_COMPILER, mpi=mpi, @@ -199,6 +202,11 @@ def openmp(self) -> bool: ''':returns: whether OpenMP is requested or not in this config.''' return self._openmp + @property + def profile(self) -> str: + ''':returns: the name of the compiler profile to use.''' + return self._profile + def add_current_prebuilds(self, artefacts: Iterable[Path]): """ Mark the given file paths as being current prebuilds, not to be diff --git a/source/fab/steps/link.py b/source/fab/steps/link.py index 902defd9..c5a2999e 100644 --- a/source/fab/steps/link.py +++ b/source/fab/steps/link.py @@ -73,7 +73,7 @@ def link_exe(config, target_objects = source_getter(config.artefact_store) for root, objects in target_objects.items(): exe_path = config.project_workspace / f'{root}' - linker.link(objects, exe_path, openmp=config.openmp, libs=libs) + linker.link(objects, exe_path, config=config, libs=libs) config.artefact_store.add(ArtefactSet.EXECUTABLES, exe_path) diff --git a/source/fab/tools/__init__.py b/source/fab/tools/__init__.py index f6830d85..7b663046 100644 --- a/source/fab/tools/__init__.py +++ b/source/fab/tools/__init__.py @@ -14,7 +14,7 @@ Icx, Ifort, Ifx, Nvc, Nvfortran) from fab.tools.compiler_wrapper import (CompilerWrapper, CrayCcWrapper, CrayFtnWrapper, Mpicc, Mpif90) -from fab.tools.flags import Flags +from fab.tools.flags import Flags, ProfileFlags from fab.tools.linker import Linker from fab.tools.psyclone import Psyclone from fab.tools.rsync import Rsync @@ -55,6 +55,7 @@ "Nvc", "Nvfortran", "Preprocessor", + "ProfileFlags", "Psyclone", "Rsync", "Shell", diff --git a/source/fab/tools/compiler.py b/source/fab/tools/compiler.py index 204e6429..293984f8 100644 --- a/source/fab/tools/compiler.py +++ b/source/fab/tools/compiler.py @@ -17,7 +17,7 @@ from fab.build_config import BuildConfig from fab.tools.category import Category -from fab.tools.flags import Flags +from fab.tools.flags import Flags, ProfileFlags from fab.tools.tool import CompilerSuiteTool @@ -66,8 +66,14 @@ def __init__(self, name: str, self._output_flag = output_flag if output_flag else "-o" self._openmp_flag = openmp_flag if openmp_flag else "" self.add_flags(os.getenv("FFLAGS", "").split()) + self._profile_flags = ProfileFlags() self._version_regex = version_regex + @property + def profile_flags(self) -> ProfileFlags: + ''':returns; the ProfileFlags for this compiler.''' + return self._profile_flags + @property def mpi(self) -> bool: ''':returns: whether this compiler supports MPI or not.''' @@ -126,6 +132,8 @@ def compile_file(self, input_file: Path, f"instead.") params += add_flags + params.extend(self.profile_flags[config.profile]) + params.extend([input_file.name, self._output_flag, str(output_file)]) diff --git a/source/fab/tools/compiler_wrapper.py b/source/fab/tools/compiler_wrapper.py index f7c35ed8..a470d940 100644 --- a/source/fab/tools/compiler_wrapper.py +++ b/source/fab/tools/compiler_wrapper.py @@ -14,7 +14,7 @@ from fab.build_config import BuildConfig from fab.tools.category import Category from fab.tools.compiler import Compiler, FortranCompiler -from fab.tools.flags import Flags +from fab.tools.flags import Flags, ProfileFlags class CompilerWrapper(Compiler): @@ -91,6 +91,11 @@ def flags(self) -> Flags: ''':returns: the flags to be used with this tool.''' return Flags(self._compiler.flags + self._flags) + @property + def profile_flags(self) -> ProfileFlags: + ''':returns: the flags to be used with this tool.''' + return self._compiler.profile_flags + @property def suite(self) -> str: ''':returns: the compiler suite of this tool.''' diff --git a/source/fab/tools/flags.py b/source/fab/tools/flags.py index d83d57cc..ad7e1eb0 100644 --- a/source/fab/tools/flags.py +++ b/source/fab/tools/flags.py @@ -10,7 +10,7 @@ ''' import logging -from typing import List, Optional, Union +from typing import Dict, List, Optional, Union import warnings from fab.util import string_checksum @@ -91,3 +91,86 @@ def remove_flag(self, remove_flag: str, has_parameter: bool = False): del self[i] continue i += 1 + + +class ProfileFlags: + + def __init__(self): + # Stores the flags for each profile mode. The key is the (lower case) + # name of the profile mode, and it contains a list of flags + self._profiles: Dict[str, Flags] = {} + + # This dictionary stores an optional inheritance, where one mode + # 'inherits' the flags from a different mode (recursively) + self._inherit_from: Dict[str, str] = {} + self.define_profile("default") + + def __getitem__(self, profile: str) -> List[str]: + profile = profile.lower() + + # First add any flags that we inherit. This will recursively call + # this __getitem__ to resolve inheritance chains. + if profile in self._inherit_from: + inherit_from = self._inherit_from[profile] + flags = self[inherit_from][:] + else: + flags = [] + # Now add the flags from this ProfileFlags. It is possible that this + # compiler does not have a profile defined, only the base one might. + if profile in self._profiles: + flags.extend(self._profiles[profile]) + + return flags + + def define_profile(self, + name: str, + inherit_from: Optional[str] = None): + if name in self._profiles: + raise KeyError(f"Profile '{name}' is already defined.") + self._profiles[name.lower()] = Flags() + + if inherit_from: + self._inherit_from[name.lower()] = inherit_from.lower() + + def add_flags(self, + profile: str, + new_flags: Union[str, List[str]]): + '''Adds the specified flags to the list of flags. + + :param new_flags: A single string or list of strings which are the + flags to be added. + ''' + + if not profile.lower() in self._profiles: + raise KeyError(f"add_flags: Profile '{profile}' is not defined.") + + if isinstance(new_flags, str): + new_flags = [new_flags] + + self._profiles[profile.lower()].add_flags(new_flags) + + def remove_flag(self, + profile: str, + remove_flag: str, + has_parameter: bool = False): + '''Adds the specified flags to the list of flags. + + :param new_flags: A single string or list of strings which are the + flags to be added. + ''' + + if not profile.lower() in self._profiles: + raise KeyError(f"remove_flag: Profile '{profile}' is not defined.") + + self._profiles[profile.lower()].remove_flag(remove_flag, + has_parameter) + + def checksum(self, profile: str) -> str: + """ + :returns: a checksum of the flags. + + """ + if not profile.lower() in self._profiles: + raise KeyError(f"checksum: Profile '{profile}' is not defined.") + + return self._profiles[profile.lower()].checksum() diff --git a/source/fab/tools/linker.py b/source/fab/tools/linker.py index 0fb0f61d..a489dacf 100644 --- a/source/fab/tools/linker.py +++ b/source/fab/tools/linker.py @@ -14,6 +14,7 @@ from typing import Dict, List, Optional, Union import warnings +from fab.build_config import BuildConfig from fab.tools.category import Category from fab.tools.compiler import Compiler from fab.tools.tool import CompilerSuiteTool @@ -181,14 +182,15 @@ def get_post_link_flags(self) -> List[str]: return params def link(self, input_files: List[Path], output_file: Path, - openmp: bool, + config: BuildConfig, libs: Optional[List[str]] = None) -> str: '''Executes the linker with the specified input files, creating `output_file`. :param input_files: list of input files to link. :param output_file: output file. - :param openm: whether OpenMP is requested or not. + :param config: The BuildConfig, from which compiler profile and OpenMP + status are taken. :param libs: additional libraries to link with. :returns: the stdout of the link command @@ -196,9 +198,12 @@ def link(self, input_files: List[Path], output_file: Path, params: List[Union[str, Path]] = [] + # TODO: For now we pick up both flags from ProfileFlags and the + # standard ones from Tool. params.extend(self._compiler.flags) + params.extend(self._compiler.profile_flags[config.profile]) - if openmp: + if config.openmp: params.append(self._compiler.openmp_flag) # TODO: why are the .o files sorted? That shouldn't matter diff --git a/tests/unit_tests/tools/test_flags.py b/tests/unit_tests/tools/test_flags.py index 0ff97ca2..ccf1f1f9 100644 --- a/tests/unit_tests/tools/test_flags.py +++ b/tests/unit_tests/tools/test_flags.py @@ -9,7 +9,7 @@ import pytest -from fab.tools import Flags +from fab.tools import Flags, ProfileFlags def test_flags_constructor(): @@ -71,3 +71,79 @@ def test_flags_checksum(): # I think this is a poor testing pattern. flags = Flags(['one', 'two', 'three', 'four']) assert flags.checksum() == 3011366051 + + +def test_profile_flags(): + '''Tests adding flags.''' + pf = ProfileFlags() + pf.define_profile("base") + assert pf["base"] == [] + pf.add_flags("base", "-base") + assert pf["base"] == ["-base"] + pf.add_flags("base", ["-base2", "-base3"]) + assert pf["base"] == ["-base", "-base2", "-base3"] + + +def test_profile_flags_inheriting(): + '''Tests adding flags.''' + pf = ProfileFlags() + pf.define_profile("base") + assert pf["base"] == [] + pf.add_flags("base", "-base") + assert pf["base"] == ["-base"] + + pf.define_profile("derived", "base") + assert pf["derived"] == ["-base"] + pf.add_flags("derived", "-derived") + assert pf["derived"] == ["-base", "-derived"] + + pf.define_profile("derived2", "derived") + assert pf["derived2"] == ["-base", "-derived"] + pf.add_flags("derived2", "-derived2") + assert pf["derived2"] == ["-base", "-derived", "-derived2"] + + +def test_profile_flags_removing(): + '''Tests adding flags.''' + pf = ProfileFlags() + pf.define_profile("base") + assert pf["base"] == [] + pf.add_flags("base", ["-base1", "-base2"]) + warn_message = "Removing managed flag '-base1'." + with pytest.warns(UserWarning, match=warn_message): + pf.remove_flag("base", "-base1") + assert pf["base"] == ["-base2"] + + +def test_profile_flags_checksum(): + '''Tests computation of the checksum.''' + pf = ProfileFlags() + pf.define_profile("base") + pf.add_flags("base", ['one', 'two', 'three', 'four']) + assert pf.checksum("base") == 3011366051 + + +def test_profile_flags_errors_invalid_profile_name(): + '''Tests that given undefined profile names will raise + KeyError in call functions. + ''' + pf = ProfileFlags() + pf.define_profile("base",) + with pytest.raises(KeyError) as err: + pf.define_profile("base") + assert "Profile 'base' is already defined." in str(err.value) + + with pytest.raises(KeyError) as err: + pf.add_flags("does not exist", []) + assert ("add_flags: Profile 'does not exist' is not defined." + in str(err.value)) + + with pytest.raises(KeyError) as err: + pf.remove_flag("does not exist", []) + assert ("remove_flag: Profile 'does not exist' is not defined." + in str(err.value)) + + with pytest.raises(KeyError) as err: + pf.checksum("does not exist") + assert ("checksum: Profile 'does not exist' is not defined." + in str(err.value))