Skip to content

Commit

Permalink
Replace openmp argument in all compiler with config in preparation to…
Browse files Browse the repository at this point in the history
… use compiler profile from Config.
  • Loading branch information
hiker committed Feb 15, 2025
1 parent 30e3678 commit bac068d
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 33 deletions.
2 changes: 1 addition & 1 deletion source/fab/steps/compile_c.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def _compile_file(arg: Tuple[AnalysedC, MpCommonArgs]):
log_or_dot(logger, f'CompileC compiling {analysed_file.fpath}')
try:
compiler.compile_file(analysed_file.fpath, obj_file_prebuild,
openmp=config.openmp,
config=config,
add_flags=flags)
except RuntimeError as err:
return FabException(f"error compiling "
Expand Down
2 changes: 1 addition & 1 deletion source/fab/steps/compile_fortran.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def compile_file(analysed_file, flags, output_fpath, mp_common_args):
compiler = config.tool_box[Category.FORTRAN_COMPILER]

compiler.compile_file(input_file=analysed_file, output_file=output_fpath,
openmp=config.openmp,
config=config,
add_flags=flags,
syntax_only=mp_common_args.syntax_only)

Expand Down
15 changes: 9 additions & 6 deletions source/fab/tools/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from typing import cast, List, Optional, Tuple, Union
import zlib

from fab.build_config import BuildConfig
from fab.tools.category import Category
from fab.tools.flags import Flags
from fab.tools.tool import CompilerSuiteTool
Expand Down Expand Up @@ -98,7 +99,7 @@ def get_hash(self) -> int:

def compile_file(self, input_file: Path,
output_file: Path,
openmp: bool,
config: BuildConfig,
add_flags: Union[None, List[str]] = None):
'''Compiles a file. It will add the flag for compilation-only
automatically, as well as the output directives. The current working
Expand All @@ -109,12 +110,13 @@ def compile_file(self, input_file: Path,
:param input_file: the path of the input file.
:param output_file: the path of the output file.
:param opemmp: whether OpenMP should be used or not.
:param config: The BuildConfig, from which compiler profile and OpenMP
status are taken.
:param add_flags: additional compiler flags.
'''

params: List[Union[Path, str]] = [self._compile_flag]
if openmp:
if config.openmp:
params.append(self.openmp_flag)
if add_flags:
if self.openmp_flag in add_flags:
Expand Down Expand Up @@ -319,14 +321,15 @@ def set_module_output_path(self, path: Path):

def compile_file(self, input_file: Path,
output_file: Path,
openmp: bool,
config: BuildConfig,
add_flags: Union[None, List[str]] = None,
syntax_only: Optional[bool] = False):
'''Compiles a file.
:param input_file: the name of the input file.
:param output_file: the name of the output file.
:param openmp: if compilation should be done with OpenMP.
:param config: The BuildConfig, from which compiler profile and OpenMP
status are taken.
:param add_flags: additional flags for the compiler.
:param syntax_only: if set, the compiler will only do
a syntax check
Expand All @@ -348,7 +351,7 @@ def compile_file(self, input_file: Path,
if self._module_folder_flag and self._module_output_path:
params.append(self._module_folder_flag)
params.append(self._module_output_path)
super().compile_file(input_file, output_file, openmp=openmp,
super().compile_file(input_file, output_file, config=config,
add_flags=params)


Expand Down
10 changes: 6 additions & 4 deletions source/fab/tools/compiler_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from pathlib import Path
from typing import cast, List, Optional, Tuple, Union

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
Expand Down Expand Up @@ -130,7 +131,7 @@ def set_module_output_path(self, path: Path):

def compile_file(self, input_file: Path,
output_file: Path,
openmp: bool,
config: BuildConfig,
add_flags: Union[None, List[str]] = None,
syntax_only: Optional[bool] = None):
# pylint: disable=too-many-arguments
Expand All @@ -140,7 +141,8 @@ def compile_file(self, input_file: Path,
:param input_file: the name of the input file.
:param output_file: the name of the output file.
:param openmp: if compilation should be done with OpenMP.
:param config: The BuildConfig, from which compiler profile and OpenMP
status are taken.
:param add_flags: additional flags for the compiler.
:param syntax_only: if set, the compiler will only do
a syntax check
Expand All @@ -160,15 +162,15 @@ def compile_file(self, input_file: Path,
# (or a CompilerWrapper in case of nested CompilerWrappers,
# which also supports the syntax_only flag anyway)
self._compiler = cast(FortranCompiler, self._compiler)
self._compiler.compile_file(input_file, output_file, openmp=openmp,
self._compiler.compile_file(input_file, output_file, config=config,
add_flags=self.flags + add_flags,
syntax_only=syntax_only,
)
else:
if syntax_only is not None:
raise RuntimeError(f"Syntax-only cannot be used with compiler "
f"'{self.name}'.")
self._compiler.compile_file(input_file, output_file, openmp=openmp,
self._compiler.compile_file(input_file, output_file, config=config,
add_flags=self.flags+add_flags
)
self._compiler.change_exec_name(orig_compiler_name)
Expand Down
9 changes: 9 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,15 @@ def fixture_tool_box(mock_c_compiler, mock_fortran_compiler, mock_linker):
return tool_box


@pytest.fixture(name="mock_config")
def fixture_mock_config(tool_box):
'''Provides a dummy fixture with mock ToolBox.'''
# Avoid cylic import
# pylint: disable=import-outside-toplevel
from fab.build_config import BuildConfig
return BuildConfig(project_label="label", tool_box=tool_box)


@pytest.fixture(name="psyclone_lfric_api")
def fixture_psyclone_lfric_api():
'''A simple fixture to provide the name of the LFRic API for
Expand Down
24 changes: 15 additions & 9 deletions tests/unit_tests/tools/test_compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def test_compiler_syntax_only():
assert fc._syntax_only_flag == "-fsyntax-only"


def test_compiler_without_openmp():
def test_compiler_without_openmp(mock_config):
'''Tests that the openmp flag is not used when openmp is not enabled. '''
fc = FortranCompiler("gfortran", "gfortran", "gnu",
version_regex="something",
Expand All @@ -172,14 +172,15 @@ def test_compiler_without_openmp():
syntax_only_flag="-fsyntax-only")
fc.set_module_output_path("/tmp")
fc.run = mock.Mock()
fc.compile_file(Path("a.f90"), "a.o", openmp=False, syntax_only=True)
mock_config._openmp = False
fc.compile_file(Path("a.f90"), "a.o", config=mock_config, syntax_only=True)
fc.run.assert_called_with(cwd=Path('.'),
additional_parameters=['-c', '-fsyntax-only',
"-J", '/tmp', 'a.f90',
'-o', 'a.o', ])


def test_compiler_with_openmp():
def test_compiler_with_openmp(mock_config):
'''Tests that the openmp flag is used as expected if openmp is enabled.
'''
fc = FortranCompiler("gfortran", "gfortran", "gnu",
Expand All @@ -189,27 +190,30 @@ def test_compiler_with_openmp():
syntax_only_flag="-fsyntax-only")
fc.set_module_output_path("/tmp")
fc.run = mock.Mock()
fc.compile_file(Path("a.f90"), "a.o", openmp=True, syntax_only=False)
mock_config._openmp = True
fc.compile_file(Path("a.f90"), "a.o", config=mock_config,
syntax_only=False)
fc.run.assert_called_with(cwd=Path('.'),
additional_parameters=['-c', '-fopenmp',
"-J", '/tmp', 'a.f90',
'-o', 'a.o', ])


def test_compiler_module_output():
def test_compiler_module_output(mock_config):
'''Tests handling of module output_flags.'''
fc = FortranCompiler("gfortran", "gfortran", suite="gnu",
version_regex="something", module_folder_flag="-J")
fc.set_module_output_path("/module_out")
assert fc._module_output_path == "/module_out"
fc.run = mock.MagicMock()
fc.compile_file(Path("a.f90"), "a.o", openmp=False, syntax_only=True)
mock_config._openmp = False
fc.compile_file(Path("a.f90"), "a.o", config=mock_config, syntax_only=True)
fc.run.assert_called_with(cwd=PosixPath('.'),
additional_parameters=['-c', '-J', '/module_out',
'a.f90', '-o', 'a.o'])


def test_compiler_with_add_args():
def test_compiler_with_add_args(mock_config):
'''Tests that additional arguments are handled as expected.'''
fc = FortranCompiler("gfortran", "gfortran", suite="gnu",
version_regex="something",
Expand All @@ -218,20 +222,22 @@ def test_compiler_with_add_args():
fc.set_module_output_path("/module_out")
assert fc._module_output_path == "/module_out"
fc.run = mock.MagicMock()
mock_config._openmp = False
with pytest.warns(UserWarning, match="Removing managed flag"):
fc.compile_file(Path("a.f90"), "a.o", add_flags=["-J/b", "-O3"],
openmp=False, syntax_only=True)
config=mock_config, syntax_only=True)
# Notice that "-J/b" has been removed
fc.run.assert_called_with(cwd=PosixPath('.'),
additional_parameters=['-c', "-O3",
'-J', '/module_out',
'a.f90', '-o', 'a.o'])
mock_config._openmp = True
with pytest.warns(UserWarning,
match="explicitly provided. OpenMP should be enabled in "
"the BuildConfiguration"):
fc.compile_file(Path("a.f90"), "a.o",
add_flags=["-fopenmp", "-O3"],
openmp=True, syntax_only=True)
config=mock_config, syntax_only=True)


# ============================================================================
Expand Down
30 changes: 18 additions & 12 deletions tests/unit_tests/tools/test_compiler_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,17 @@ def test_compiler_wrapper_module_output():
assert "'gcc' has no 'set_module_output_path' function" in str(err.value)


def test_compiler_wrapper_fortran_with_add_args():
def test_compiler_wrapper_fortran_with_add_args(mock_config):
'''Tests that additional arguments are handled as expected in
a wrapper.'''
mpif90 = ToolRepository().get_tool(Category.FORTRAN_COMPILER,
"mpif90-gfortran")
mpif90.set_module_output_path("/module_out")
mock_config._openmp = False
with mock.patch.object(mpif90.compiler, "run", mock.MagicMock()):
with pytest.warns(UserWarning, match="Removing managed flag"):
mpif90.compile_file(Path("a.f90"), "a.o",
add_flags=["-J/b", "-O3"], openmp=False,
add_flags=["-J/b", "-O3"], config=mock_config,
syntax_only=True)
# Notice that "-J/b" has been removed
mpif90.compiler.run.assert_called_with(
Expand All @@ -185,58 +186,61 @@ def test_compiler_wrapper_fortran_with_add_args():
'a.f90', '-o', 'a.o'])


def test_compiler_wrapper_fortran_with_add_args_unnecessary_openmp():
def test_compiler_wrapper_fortran_add_args_unnecessary_openmp(mock_config):
'''Tests that additional arguments are handled as expected in
a wrapper if also the openmp flags are specified.'''
mpif90 = ToolRepository().get_tool(Category.FORTRAN_COMPILER,
"mpif90-gfortran")
mpif90.set_module_output_path("/module_out")
mock_config._openmp = True
with mock.patch.object(mpif90.compiler, "run", mock.MagicMock()):
with pytest.warns(UserWarning,
match="explicitly provided. OpenMP should be "
"enabled in the BuildConfiguration"):
mpif90.compile_file(Path("a.f90"), "a.o",
add_flags=["-fopenmp", "-O3"],
openmp=True, syntax_only=True)
config=mock_config, syntax_only=True)
mpif90.compiler.run.assert_called_with(
cwd=Path('.'),
additional_parameters=['-c', '-fopenmp', '-fopenmp', '-O3',
'-fsyntax-only', '-J', '/module_out',
'a.f90', '-o', 'a.o'])


def test_compiler_wrapper_c_with_add_args():
def test_compiler_wrapper_c_with_add_args(mock_config):
'''Tests that additional arguments are handled as expected in a
compiler wrapper. Also verify that requesting Fortran-specific options
like syntax-only with the C compiler raises a runtime error.
'''

mpicc = ToolRepository().get_tool(Category.C_COMPILER,
"mpicc-gcc")
mock_config._openmp = False
with mock.patch.object(mpicc.compiler, "run", mock.MagicMock()):
# Normal invoke of the C compiler, make sure add_flags are
# passed through:
mpicc.compile_file(Path("a.f90"), "a.o", openmp=False,
mpicc.compile_file(Path("a.f90"), "a.o", config=mock_config,
add_flags=["-O3"])
mpicc.compiler.run.assert_called_with(
cwd=Path('.'), additional_parameters=['-c', "-O3", 'a.f90',
'-o', 'a.o'])
# Invoke C compiler with syntax-only flag (which is only supported
# by Fortran compilers), which should raise an exception.
with pytest.raises(RuntimeError) as err:
mpicc.compile_file(Path("a.f90"), "a.o", openmp=False,
mpicc.compile_file(Path("a.f90"), "a.o", config=mock_config,
add_flags=["-O3"], syntax_only=True)
assert ("Syntax-only cannot be used with compiler 'mpicc-gcc'."
in str(err.value))

# Check that providing the openmp flag in add_flag raises a warning:
mock_config._openmp = True
with mock.patch.object(mpicc.compiler, "run", mock.MagicMock()):
with pytest.warns(UserWarning,
match="explicitly provided. OpenMP should be "
"enabled in the BuildConfiguration"):
mpicc.compile_file(Path("a.f90"), "a.o",
add_flags=["-fopenmp", "-O3"],
openmp=True)
config=mock_config)
mpicc.compiler.run.assert_called_with(
cwd=Path('.'),
additional_parameters=['-c', '-fopenmp', '-fopenmp', '-O3',
Expand Down Expand Up @@ -278,7 +282,7 @@ def test_compiler_wrapper_flags_independent():
assert mpicc.flags == ["-a", "-b", "-d", "-e"]


def test_compiler_wrapper_flags_with_add_arg():
def test_compiler_wrapper_flags_with_add_arg(mock_config):
'''Tests that flags set in the base compiler will be accessed in the
wrapper if also additional flags are specified.'''
gcc = Gcc()
Expand All @@ -289,16 +293,17 @@ def test_compiler_wrapper_flags_with_add_arg():
# Check that the flags are assembled in the right order in the
# actual compiler call: first the wrapper compiler flag, then
# the wrapper flag, then additional flags
mock_config._openmp = True
with mock.patch.object(mpicc.compiler, "run", mock.MagicMock()):
mpicc.compile_file(Path("a.f90"), "a.o", add_flags=["-f"],
openmp=True)
config=mock_config)
mpicc.compiler.run.assert_called_with(
cwd=Path('.'),
additional_parameters=["-c", "-fopenmp", "-a", "-b", "-d",
"-e", "-f", "a.f90", "-o", "a.o"])


def test_compiler_wrapper_flags_without_add_arg():
def test_compiler_wrapper_flags_without_add_arg(mock_config):
'''Tests that flags set in the base compiler will be accessed in the
wrapper if no additional flags are specified.'''
gcc = Gcc()
Expand All @@ -308,9 +313,10 @@ def test_compiler_wrapper_flags_without_add_arg():
# Check that the flags are assembled in the right order in the
# actual compiler call: first the wrapper compiler flag, then
# the wrapper flag, then additional flags
mock_config._openmp = True
with mock.patch.object(mpicc.compiler, "run", mock.MagicMock()):
# Test if no add_flags are specified:
mpicc.compile_file(Path("a.f90"), "a.o", openmp=True)
mpicc.compile_file(Path("a.f90"), "a.o", config=mock_config)
mpicc.compiler.run.assert_called_with(
cwd=Path('.'),
additional_parameters=["-c", "-fopenmp", "-a", "-b", "-d",
Expand Down

0 comments on commit bac068d

Please sign in to comment.