Skip to content

Commit

Permalink
Support compilers that do not support OpenMP.
Browse files Browse the repository at this point in the history
  • Loading branch information
hiker committed Nov 8, 2024
1 parent f165e46 commit df5f63a
Show file tree
Hide file tree
Showing 12 changed files with 144 additions and 34 deletions.
3 changes: 2 additions & 1 deletion source/fab/build_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ def __init__(self, project_label: str,
self._openmp = openmp
self.two_stage = two_stage
self.verbose = verbose
compiler = tool_box.get_tool(Category.FORTRAN_COMPILER, mpi=mpi)
compiler = tool_box.get_tool(Category.FORTRAN_COMPILER, mpi=mpi,
openmp=openmp)
project_label = Template(project_label).safe_substitute(
compiler=compiler.name,
two_stage=f'{int(two_stage)+1}stage')
Expand Down
2 changes: 1 addition & 1 deletion source/fab/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def _generic_build_config(folder: Path, kwargs=None) -> BuildConfig:
# Set the default Fortran compiler as linker (otherwise e.g. the
# C compiler might be used in linking, requiring additional flags)
tr = ToolRepository()
fc = tr.get_default(Category.FORTRAN_COMPILER, mpi=False)
fc = tr.get_default(Category.FORTRAN_COMPILER, mpi=False, openmp=False)
# TODO: This assumes a mapping of compiler name to the corresponding
# linker name (i.e. `linker-gfortran` or `linker-ifort`). Still, that's
# better than hard-coding gnu here.
Expand Down
5 changes: 3 additions & 2 deletions source/fab/steps/compile_c.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ def compile_c(config, common_flags: Optional[List[str]] = None,
# No need to look for compiler etc if there is nothing to do
return

compiler = config.tool_box.get_tool(Category.C_COMPILER, config.mpi)
compiler = config.tool_box.get_tool(Category.C_COMPILER, mpi=config.mpi,
openmp=config.openmp)
logger.info(f'C compiler is {compiler}')

mp_payload = MpCommonArgs(config=config, flags=flags)
Expand Down Expand Up @@ -146,7 +147,7 @@ def _compile_file(arg: Tuple[AnalysedC, MpCommonArgs]):
compiler.compile_file(analysed_file.fpath, obj_file_prebuild,
openmp=config.openmp,
add_flags=flags)
except Exception as err:
except RuntimeError as err:
return FabException(f"error compiling "
f"{analysed_file.fpath}:\n{err}")

Expand Down
3 changes: 2 additions & 1 deletion source/fab/steps/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ def link_exe(config, flags=None, source: Optional[ArtefactsGetter] = None):
output from compiler steps, which typically is the expected behaviour.
"""
linker = config.tool_box.get_tool(Category.LINKER, config.mpi)
linker = config.tool_box.get_tool(Category.LINKER, mpi=config.mpi,
openmp=config.openmp)
logger.info(f'Linker is {linker.name}')

flags = flags or []
Expand Down
9 changes: 8 additions & 1 deletion source/fab/tools/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ class Compiler(CompilerSuiteTool):
compilation (not linking).
:param output_flag: the compilation flag to use to indicate the name
of the output file
:param openmp_flag: the flag to use to enable OpenMP
:param openmp_flag: the flag to use to enable OpenMP. If no flag is
specified, it is assumed that the compiler does not support OpenMP.
'''

# pylint: disable=too-many-arguments
Expand All @@ -61,6 +62,12 @@ def get_hash(self) -> int:
return (zlib.crc32(self.name.encode()) +
zlib.crc32(self.get_version_string().encode()))

@property
def openmp(self) -> bool:
''':returns: if the compiler supports openmp or not
'''
return self._openmp_flag != ""

@property
def openmp_flag(self) -> str:
''':returns: The flag to enable OpenMP for this compiler.
Expand Down
13 changes: 9 additions & 4 deletions source/fab/tools/tool_box.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,18 @@ def add_tool(self, tool: Tool,
f"'{tool}'.")
self._all_tools[tool.category] = tool

def get_tool(self, category: Category, mpi: Optional[bool] = None) -> Tool:
def get_tool(self, category: Category, mpi: Optional[bool] = None,
openmp: Optional[bool] = None) -> Tool:
'''Returns the tool for the specified category.
:param category: the name of the category in which to look
for the tool.
:param mpi: if no compiler or linker is specified when requesting one,
use the MPI setting to find an appropriate default.
:param mpi: if no compiler or linker is explicitly specified in this
tool box, use the MPI and OpenMP setting to find an appropriate
default from the tool repository.
:param mpi: if no compiler or linker is explicitly specified in this
tool box, use the MPI and OpenMP setting to find an appropriate
default from the tool repository.
:raises KeyError: if the category is not known.
'''
Expand All @@ -69,6 +74,6 @@ def get_tool(self, category: Category, mpi: Optional[bool] = None) -> Tool:
# from the ToolRepository, and add it, so we don't need to look
# it up again later.
tr = ToolRepository()
tool = tr.get_default(category, mpi=mpi)
tool = tr.get_default(category, mpi=mpi, openmp=openmp)
self._all_tools[category] = tool
return tool
26 changes: 23 additions & 3 deletions source/fab/tools/tool_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,8 @@ def set_default_compiler_suite(self, suite: str):
f"in the suite '{suite}'.")

def get_default(self, category: Category,
mpi: Optional[bool] = None):
mpi: Optional[bool] = None,
openmp: Optional[bool] = None):
'''Returns the default tool for a given category. For most tools
that will be the first entry in the list of tools. The exception
are compilers and linker: in this case it must be specified if
Expand All @@ -141,6 +142,7 @@ def get_default(self, category: Category,
:param category: the category for which to return the default tool.
:param mpi: if a compiler or linker is required that supports MPI.
:param open: if a compiler or linker is required that supports OpenMP.
:raises KeyError: if the category does not exist.
:raises RuntimeError: if no compiler/linker is found with the
Expand All @@ -159,11 +161,29 @@ def get_default(self, category: Category,
raise RuntimeError(f"Invalid or missing mpi specification "
f"for '{category}'.")

if not isinstance(openmp, bool):
raise RuntimeError(f"Invalid or missing openmp specification "
f"for '{category}'.")

for tool in self[category]:
# If the tool supports/does not support MPI, return the first one
# If OpenMP is request, but the tool does not support openmp,
# ignore it.
if openmp and not tool.openmp:
continue
# If the tool supports/does not support MPI, return it.
if mpi == tool.mpi:
return tool

# Don't bother returning an MPI enabled tool if no-MPI is requested -
# that seems to be an unlikely scenario.
raise RuntimeError(f"Could not find '{category}' that supports MPI.")
if mpi:
if openmp:
raise RuntimeError(f"Could not find '{category}' that "
f"supports MPI and OpenMP.")
raise RuntimeError(f"Could not find '{category}' that "
f"supports MPI.")

if openmp:
raise RuntimeError(f"Could not find '{category}' that "
f"supports OpenMP.")
raise RuntimeError(f"Could not find any '{category}'.")
14 changes: 9 additions & 5 deletions tests/unit_tests/steps/test_archive_objects.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ def test_for_exes(self):

# ensure the correct artefacts were created
assert config.artefact_store[ArtefactSet.OBJECT_ARCHIVES] == {
target: set([str(config.build_output / f'{target}.a')]) for target in targets}
target: set([str(config.build_output / f'{target}.a')])
for target in targets}

def test_for_library(self):
'''As used when building an object archive or archiving before linking
Expand All @@ -65,12 +66,15 @@ def test_for_library(self):
mock_result = mock.Mock(returncode=0, return_value=123)
with mock.patch('fab.tools.tool.subprocess.run',
return_value=mock_result) as mock_run_command, \
pytest.warns(UserWarning, match="_metric_send_conn not set, cannot send metrics"):
archive_objects(config=config, output_fpath=config.build_output / 'mylib.a')
pytest.warns(UserWarning, match="_metric_send_conn not set, "
"cannot send metrics"):
archive_objects(config=config,
output_fpath=config.build_output / 'mylib.a')

# ensure the correct command line calls were made
mock_run_command.assert_called_once_with([
'ar', 'cr', str(config.build_output / 'mylib.a'), 'util1.o', 'util2.o'],
'ar', 'cr', str(config.build_output / 'mylib.a'),
'util1.o', 'util2.o'],
capture_output=True, env=None, cwd=None, check=False)

# ensure the correct artefacts were created
Expand All @@ -83,7 +87,7 @@ def test_incorrect_tool(self):

config = BuildConfig('proj', ToolBox())
tool_box = config.tool_box
cc = tool_box.get_tool(Category.C_COMPILER, config.mpi)
cc = tool_box.get_tool(Category.C_COMPILER, config.mpi, config.openmp)
# And set its category to C_COMPILER
cc._category = Category.AR
# So overwrite the C compiler with the re-categories Fortran compiler
Expand Down
2 changes: 1 addition & 1 deletion tests/unit_tests/steps/test_compile_c.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def test_exception_handling(self, content):
compiler = config.tool_box[Category.C_COMPILER]
# mock the run command to raise an exception
with pytest.raises(RuntimeError):
with mock.patch.object(compiler, "run", side_effect=Exception):
with mock.patch.object(compiler, "run", side_effect=RuntimeError):
with mock.patch('fab.steps.compile_c.send_metric') as mock_send_metric:
with mock.patch('pathlib.Path.mkdir'):
compile_c(config=config)
Expand Down
27 changes: 27 additions & 0 deletions tests/unit_tests/tools/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,33 @@ def test_compiler():
in str(err.value))


def test_compiler_openmp():
'''Test that the openmp flag is correctly reflected in the test if
a compiler supports OpenMP or not.'''
cc = CCompiler("gcc", "gcc", "gnu", openmp_flag="-fopenmp")
assert cc.openmp_flag == "-fopenmp"
assert cc.openmp
cc = CCompiler("gcc", "gcc", "gnu", openmp_flag=None)
assert cc.openmp_flag == ""
assert not cc.openmp
cc = CCompiler("gcc", "gcc", "gnu")
assert cc.openmp_flag == ""
assert not cc.openmp

fc = FortranCompiler("gfortran", "gfortran", "gnu", openmp_flag="-fopenmp",
module_folder_flag="-J")
assert fc.openmp_flag == "-fopenmp"
assert fc.openmp
fc = FortranCompiler("gfortran", "gfortran", "gnu", openmp_flag=None,
module_folder_flag="-J")
assert fc.openmp_flag == ""
assert not fc.openmp
fc = FortranCompiler("gfortran", "gfortran", "gnu",
module_folder_flag="-J")
assert fc.openmp_flag == ""
assert not fc.openmp


def test_compiler_check_available():
'''Check if check_available works as expected. The compiler class uses
internally get_version to test if a compiler works or not. Check the
Expand Down
5 changes: 3 additions & 2 deletions tests/unit_tests/tools/test_tool_box.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ def test_tool_box_get_tool():
'''Tests get_tool.'''
tb = ToolBox()
# No tool is defined, so the default Fortran compiler must be returned:
default_compiler = tb.get_tool(Category.FORTRAN_COMPILER, mpi=False)
default_compiler = tb.get_tool(Category.FORTRAN_COMPILER,
mpi=False, openmp=False)
tr = ToolRepository()
assert default_compiler is tr.get_default(Category.FORTRAN_COMPILER,
mpi=False)
mpi=False, openmp=False)
# Check that dictionary-like access works as expected:
assert tb[Category.FORTRAN_COMPILER] == default_compiler

Expand Down
69 changes: 56 additions & 13 deletions tests/unit_tests/tools/test_tool_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
from unittest import mock
import pytest


from fab.tools import (Ar, Category, Gcc, Gfortran, Ifort, Linker,
ToolRepository)
from fab.tools import (Ar, Category, FortranCompiler, Gcc, Gfortran, Ifort,
Linker, ToolRepository)


def test_tool_repository_get_singleton_new():
Expand Down Expand Up @@ -59,14 +58,15 @@ def test_tool_repository_get_tool_error():
def test_tool_repository_get_default():
'''Tests get_default.'''
tr = ToolRepository()
gfortran = tr.get_default(Category.FORTRAN_COMPILER, mpi=False)
gfortran = tr.get_default(Category.FORTRAN_COMPILER, mpi=False,
openmp=False)
assert isinstance(gfortran, Gfortran)

gcc_linker = tr.get_default(Category.LINKER, mpi=False)
gcc_linker = tr.get_default(Category.LINKER, mpi=False, openmp=False)
assert isinstance(gcc_linker, Linker)
assert gcc_linker.name == "linker-gcc"

gcc = tr.get_default(Category.C_COMPILER, mpi=False)
gcc = tr.get_default(Category.C_COMPILER, mpi=False, openmp=False)
assert isinstance(gcc, Gcc)

# Test a non-compiler
Expand All @@ -88,19 +88,62 @@ def test_tool_repository_get_default_error_missing_mpi():
parameter is missing (which is required for a compiler).'''
tr = ToolRepository()
with pytest.raises(RuntimeError) as err:
tr.get_default(Category.FORTRAN_COMPILER)
tr.get_default(Category.FORTRAN_COMPILER, openmp=True)
assert ("Invalid or missing mpi specification for 'FORTRAN_COMPILER'"
in str(err.value))
with pytest.raises(RuntimeError) as err:
tr.get_default(Category.FORTRAN_COMPILER, mpi="123")
assert ("Invalid or missing mpi specification for 'FORTRAN_COMPILER'"
in str(err.value))


def test_tool_repository_get_default_error_missing_compiler():
def test_tool_repository_get_default_error_missing_openmp():
'''Tests error handling in get_default when the optional openmp
parameter is missing (which is required for a compiler).'''
tr = ToolRepository()
with pytest.raises(RuntimeError) as err:
tr.get_default(Category.FORTRAN_COMPILER, mpi=True)
assert ("Invalid or missing openmp specification for 'FORTRAN_COMPILER'"
in str(err.value))
with pytest.raises(RuntimeError) as err:
tr.get_default(Category.FORTRAN_COMPILER, mpi=True, openmp="123")
assert ("Invalid or missing openmp specification for 'FORTRAN_COMPILER'"
in str(err.value))


@pytest.mark.parametrize("mpi, openmp, message",
[(False, False, "any 'FORTRAN_COMPILER'."),
(False, True,
"'FORTRAN_COMPILER' that supports OpenMP"),
(True, False,
"'FORTRAN_COMPILER' that supports MPI"),
(True, True, "'FORTRAN_COMPILER' that supports MPI "
"and OpenMP.")])
def test_tool_repository_get_default_error_missing_compiler(mpi, openmp,
message):
'''Tests error handling in get_default when there is no compiler
that fulfils the requirements.'''
that fulfils the requirements with regards to OpenMP and MPI.'''
tr = ToolRepository()
with mock.patch.dict(tr, {Category.FORTRAN_COMPILER: []}), \
pytest.raises(RuntimeError) as err:
tr.get_default(Category.FORTRAN_COMPILER, mpi=True)
assert ("Could not find 'FORTRAN_COMPILER' that supports MPI."
tr.get_default(Category.FORTRAN_COMPILER, mpi=mpi, openmp=openmp)

assert f"Could not find {message}" in str(err.value)


def test_tool_repository_get_default_error_missing_openmp_compiler():
'''Tests error handling in get_default when there is a compiler, but it
does not support OpenMP (which triggers additional tests in the
ToolRepository.'''
tr = ToolRepository()
fc = FortranCompiler("gfortran", "gfortran", "gnu", openmp_flag=None,
module_folder_flag="-J")

with mock.patch.dict(tr, {Category.FORTRAN_COMPILER: [fc]}), \
pytest.raises(RuntimeError) as err:
tr.get_default(Category.FORTRAN_COMPILER, mpi=False, openmp=True)

assert ("Could not find 'FORTRAN_COMPILER' that supports OpenMP."
in str(err.value))


Expand All @@ -110,13 +153,13 @@ def test_tool_repository_default_compiler_suite():
tr.set_default_compiler_suite("gnu")
for cat in [Category.C_COMPILER, Category.FORTRAN_COMPILER,
Category.LINKER]:
def_tool = tr.get_default(cat, mpi=False)
def_tool = tr.get_default(cat, mpi=False, openmp=False)
assert def_tool.suite == "gnu"

tr.set_default_compiler_suite("intel-classic")
for cat in [Category.C_COMPILER, Category.FORTRAN_COMPILER,
Category.LINKER]:
def_tool = tr.get_default(cat, mpi=False)
def_tool = tr.get_default(cat, mpi=False, openmp=False)
assert def_tool.suite == "intel-classic"
with pytest.raises(RuntimeError) as err:
tr.set_default_compiler_suite("does-not-exist")
Expand Down

0 comments on commit df5f63a

Please sign in to comment.