Skip to content

Commit

Permalink
Linker wrapper new (#375)
Browse files Browse the repository at this point in the history
* Support new and old style of PSyclone command line (no more nemo api etc)

* Fix mypy errors.

* Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa.

* Updated comment.

* Removed mixing, use a simple regex instead.

* Added support for ifx/icx compiler as intel-llvm class.

* Added support for nvidia compiler.

* Add preliminary support for Cray compiler.

* Added Cray compiler wrapper ftn and cc.

* Follow a more consistent naming scheme for crays, even though the native compiler names are longer (crayftn-cray, craycc-cray).

* Changed names again.

* Renamed cray compiler wrapper to be CrayCcWrapper and CrayFtnWrapper, to avoid confusion with Craycc.

* Fixed incorrect name in comments.

* Additional compilers (#349)

* Moved OBJECT_ARCHIVES from constants to ArtefactSet.

* Moved PRAGMAD_C from constants to ArtefactSet.

* Turned 'all_source' into an enum.

* Allow integer as revision.

* Fixed flake8 error.

* Removed specific functions to add/get fortran source files etc.

* Removed non-existing and unneccessary collections.

* Try to fix all run_configs.

* Fixed rebase issues.

* Added replace functionality to ArtefactStore, updated test_artefacts to cover all lines in that file.

* Started to replace artefacts when files are pre-processed.

* Removed linker argument from linking step in all examples.

* Try to get jules to link.

* Fixed build_jules.

* Fixed other issues raised in reviews.

* Try to get jules to link.

* Fixed other issues raised in reviews.

* Simplify handling of X90 files by replacing the X90 with x90, meaning only one artefact set is involved when running PSyclone.

* Make OBJECT_ARCHIVES also a dict, migrate more code to replace/add files to the default build artefact collections.

* Fixed some examples.

* Fix flake8 error.

* Fixed failing tests.

* Support empty comments.

* Fix preprocessor to not unnecessary remove and add files that are already in the output directory.

* Allow find_soure_files to be called more than once by adding files (not replacing artefact).

* Updated lfric_common so that files created by configurator are written in build (not source).

* Use c_build_files instead of pragmad_c.

* Removed unnecessary str.

* Documented the new artefact set handling.

* Fixed typo.

* Make the PSyclone API configurable.

* Fixed formatting of documentation, properly used ArtefactSet names.

* Support .f and .F Fortran files.

* Removed setter for tool.is_available, which was only used for testing.

* #3 Fix documentation and coding style issues from review.

* Renamed Categories into Category.

* Minor coding style cleanup.

* Removed more unnecessary ().

* Re-added (invalid) grab_pre_build call.

* Fixed typo.

* Renamed set_default_vendor to set_default_compiler_suite.

* Renamed VendorTool to CompilerSuiteTool.

* Also accept a Path as exec_name specification for a tool.

* Move the check_available function into the base class.

* Fixed some types and documentation.

* Fix typing error.

* Added explanation for meta-compiler.

* Improved error handling and documentation.

* Replace mpiifort with mpifort to be a tiny bit more portable.

* Use classes to group tests for git/svn/fcm together.

* Fixed issue in get_transformation script, and moved script into lfric_common to remove code duplication.

* Code improvement as suggested by review.

* Fixed run config

* Added reference to ticket.

* Updated type information.

* More typing fixes.

* Fixed typing warnings.

* As requested by reviewer removed is_working_copy functionality.

* Issue a warning (which can be silenced) when a tool in a toolbox is replaced.

* Fixed flake8.

* Fixed flake8.

* Fixed failing test.

* Addressed issues raised in review.

* Removed now unnecessary operations.

* Updated some type information.

* Fixed all references to APIs to be consistent with PSyclone 2.5.

* Added api to the checksum computation.

* Fixed type information.

* Added test to verify that changing the api changes the checksum.

* Make compiler version a tuple of integers

* Update some tests to use tuple versions

* Explicitly test handling of bad version format

* Fix formatting

* Tidying up

* Make compiler raise an error for any invalid version string

Assume these compilers don't need to be hashed.
Saves dealing with empty tuples.

* Check compiler version string for compiler name

* Fix formatting

* Add compiler.get_version_string() method

Includes other cleanup from PR comments

* Add mpi and openmp settings to BuildConfig, made compiler MPI aware.

* Looks like the circular dependency has been fixed.

* Revert "Looks like the circular dependency has been fixed." ...
while it works with the tests, a real application still triggered it.

This reverts commit 150dc37.

* Don't even try to find a C compiler if no C files are to be compiled.

* Updated gitignore to ignore (recently renamed) documentation.

* Fixed failing test.

* Return from compile Fortran early if there are no files to compiles. Fixed coding style.

* Add MPI enables wrapper for intel and gnu compiler.

* Fixed test.

* Automatically add openmp flag to compiler and linker based on BuildConfig.

* Removed enforcement of keyword parameters, which is not supported in python 3.7.

* Fixed failing test.

* Support more than one tool of a given suite by sorting them.

* Use different version checkout for each compiler vendor with mixins

* Refactoring, remove unittest compiler class

* Fix some mypy errors

* Use 'Union' type hint to fix build checks

* Added option to add flags to a tool.

* Introduce proper compiler wrapper, used this to implement properly wrapper MPI compiler.

* Fixed typo in types.

* Return run_version_command to base Compiler class

Provides default version command that can be overridden for other compilers.
Also fix some incorrect tests
Other tidying

* Add a missing type hint

* Added (somewhat stupid) 'test' to reach 100% coverage of PSyclone tool.

* Simplified MPI support in wrapper.

* More compiler wrapper coverage.

* Removed duplicated function.

* Removed debug print.

* Removed permanently changing compiler attributes, which can cause test failures later.

* More test for C compiler wrapper.

* More work on compiler wrapper tests.

* Fixed version and availability handling, added missing tests for 100% coverage.

* Fixed typing error.

* Try to fix python 3.7.

* Tried to fix failing tests.

* Remove inheritance from mixins and use protocol

* Simplify compiler inheritance

Mixins have static methods with unique names,
overrides only happen in concrete classes

* Updated wrapper and tests to handle error raised in get_version.

* Simplified regular expressions (now tests cover detection of version numbers with only a major version).

* Test for missing mixin.

* Use the parsing mixing from the compiler in a compiler wrapper.

* Use setattr instead of assignment to make mypy happy.

* Simplify usage of compiler-specific parsing mixins.

* Minor code cleanup.

* Updated documentation.

* Simplify usage of compiler-specific parsing mixins.

* Test for missing mixin.

* Fixed test.

* Added missing openmp_flag property to compiler wrapper.

* Don't use isinstance for consistency check, which does not work for CompilerWrappers.

* Fixed isinstance test for C compilation which doesn't work with a CompilerWrapper.

* Use a linker's compiler to determine MPI support. Removed mpi property from CompilerSuite.

* Added more tests for invalid version numbers.

* Added more test cases for invalid version number, improved regex to work as expected.

* Fixed typo in test.

* Fixed flake/mypy errors.

* Combine wrapper flags with flags from wrapped compiler.

* Made mypy happy.

* Fixed test.

* Split tests into smaller individual ones, fixed missing asssert in test.

* Parameterised compiler version tests to also test wrapper.

* Added missing MPI parameter when getting the compiler.

* Fixed comments.

* Order parameters to be in same order for various compiler classes.

* Remove stray character

* Added getter for wrapped compiler.

* Fixed small error that would prevent nested compiler wrappers from being used.

* Added a cast to make mypy happy.

* Add simple getter for linker library flags

* Add getter for linker flags by library

* Fix formatting

* Add optional libs argument to link function

* Reorder and clean up linker tests

* Make sure `Linker.link()` raises for unknown lib

* Add missing type

* Fix typing error

* Add 'libs' argument to link_exe function

* Try to add documentation for the linker libs feature

* Use correct list type in link_exe hint

* Add silent replace option to linker.add_lib_flags

* Fixed spelling mistake in option.

* Clarified documentation.

* Removed unnecessary functions in CompilerWrapper.

* Fixed failing test triggered by executing them in specific order (tools then steps)

* Fixed line lengths.

* Add tests for linker LDFLAG

* Add pre- and post- lib flags to link function

* Fix syntax in built-in lib flags

* Remove netcdf as a built-in linker library

Bash-style substitution is not currently handled

* Configure pre- and post-lib flags on the Linker object

Previously they were passed into the Linker.link() function

* Use more realistic linker lib flags

* Formatting fix

* Removed mixing, use a simple regex instead.

* Added support for ifx/icx compiler as intel-llvm class.

* Added support for nvidia compiler.

* Add preliminary support for Cray compiler.

* Added Cray compiler wrapper ftn and cc.

* Made mpi and openmp default to False in the BuildConfig constructor.

* Removed white space.

* Follow a more consistent naming scheme for crays, even though the native compiler names are longer (crayftn-cray, craycc-cray).

* Changed names again.

* Support compilers that do not support OpenMP.

* Added documentation for openmp parameter.

* Renamed cray compiler wrapper to be CrayCcWrapper and CrayFtnWrapper, to avoid confusion with Craycc.

* Fixed incorrect name in comments.

---------

Co-authored-by: jasonjunweilyu <[email protected]>
Co-authored-by: Luke Hoffmann <[email protected]>
Co-authored-by: Luke Hoffmann <[email protected]>

* Support new and old style of PSyclone command line (no more nemo api etc)

* Fix mypy errors.

* Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa.

* Added shell tool.

* Try to make mypy happy.

* Removed debug code.

* ToolRepository now only returns default that are available. Updated tests to make tools as available.

* Fixed typos and coding style.

* Support new and old style of PSyclone command line (no more nemo api etc)

* Fix mypy errors.

* Added missing tests for calling psyclone, and converting old style to new stle arguments and vice versa.

* Updated comment.

* Fixed failing tests.

* Updated fparser dependency to version 0.2.

* Replace old code for handling sentinels with triggering this behaviour in fparser. Require config in constructor of Analyser classes.

* Fixed tests for latest changes.

* Removed invalid openmp continuation line - since now fparser fails when trying to parse this line.

* Added test for disabled openmp parsing. Updated test to work with new test file.

* Coding style changes.

* Fix flake issues.

* Fixed double _.

* Make Linker inherit CompilerWrapper

* Fix up tests for new Linker inheritence

* Fix a flake error

* Use linker wrapping to combine flags from the wrapped linker with the linker wrapper.

* Minor code cleanup.

* Created linker wrapper in ToolRepository.

* Try making linker a CompilerSuiteTool instead of a CompilerWrapper.

* Updated tests.

* Fix support for post-libs.

* Fixed mypy.

* Removed more accesses to private members.

* Added missing type hint.

* Make flake8 happy.

* Added missing openmp handling in linker.

* Addressed issues raised in review.

* Forgot that file in previous commit.

---------

Co-authored-by: Luke Hoffmann <[email protected]>
Co-authored-by: jasonjunweilyu <[email protected]>
Co-authored-by: Luke Hoffmann <[email protected]>
  • Loading branch information
4 people authored Jan 29, 2025
1 parent 54a234b commit c585ff7
Show file tree
Hide file tree
Showing 10 changed files with 369 additions and 150 deletions.
7 changes: 6 additions & 1 deletion source/fab/tools/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def __init__(self, name: str,
self._compile_flag = compile_flag if compile_flag else "-c"
self._output_flag = output_flag if output_flag else "-o"
self._openmp_flag = openmp_flag if openmp_flag else ""
self.flags.extend(os.getenv("FFLAGS", "").split())
self.add_flags(os.getenv("FFLAGS", "").split())
self._version_regex = version_regex

@property
Expand All @@ -83,6 +83,11 @@ def openmp_flag(self) -> str:
'''Returns the flag to enable OpenMP.'''
return self._openmp_flag

@property
def output_flag(self) -> str:
'''Returns the flag that specifies the output flag.'''
return self._output_flag

def get_hash(self) -> int:
''':returns: a hash based on the compiler name and version.
'''
Expand Down
4 changes: 2 additions & 2 deletions source/fab/tools/compiler_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
# which you should have received as part of this distribution
##############################################################################

"""This file contains the base class for any compiler, and derived
classes for gcc, gfortran, icc, ifort
"""This file contains the base class for any compiler-wrapper, including
the derived classes for mpif90, mpicc, and CrayFtnWrapper and CrayCcWrapper.
"""

from pathlib import Path
Expand Down
203 changes: 153 additions & 50 deletions source/fab/tools/linker.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@
"""This file contains the base class for any Linker.
"""

from __future__ import annotations

import os
from pathlib import Path
from typing import cast, Dict, List, Optional
from typing import cast, Dict, List, Optional, Union
import warnings

from fab.tools.category import Category
Expand All @@ -18,60 +20,107 @@


class Linker(CompilerSuiteTool):
'''This is the base class for any Linker. If a compiler is specified,
its name, executable, and compile suite will be used for the linker (if
not explicitly set in the constructor).
:param name: the name of the linker.
:param exec_name: the name of the executable.
:param suite: optional, the name of the suite.
:param compiler: optional, a compiler instance
:param output_flag: flag to use to specify the output name.
'''This is the base class for any Linker. It takes either another linker
instance, or a compiler instance as parameter in the constructor. Exactly
one of these must be provided.
:param compiler: an optional compiler instance
:param linker: an optional linker instance
:param name: name of the linker
:raises RuntimeError: if both compiler and linker are specified.
:raises RuntimeError: if neither compiler nor linker is specified.
'''

# pylint: disable=too-many-arguments
def __init__(self, name: Optional[str] = None,
def __init__(self, compiler: Optional[Compiler] = None,
linker: Optional[Linker] = None,
exec_name: Optional[str] = None,
suite: Optional[str] = None,
compiler: Optional[Compiler] = None,
output_flag: str = "-o"):
if (not name or not exec_name or not suite) and not compiler:
raise RuntimeError("Either specify name, exec name, and suite "
"or a compiler when creating Linker.")
# Make mypy happy, since it can't work out otherwise if these string
# variables might still be None :(
compiler = cast(Compiler, compiler)
name: Optional[str] = None):

if linker and compiler:
raise RuntimeError("Both compiler and linker is specified in "
"linker constructor.")
if not linker and not compiler:
raise RuntimeError("Neither compiler nor linker is specified in "
"linker constructor.")
self._compiler = compiler
self._linker = linker

search_linker = self
while search_linker._linker:
search_linker = search_linker._linker
final_compiler = search_linker._compiler
if not name:
name = compiler.name
assert final_compiler # make mypy happy
name = f"linker-{final_compiler.name}"

if not exec_name:
exec_name = compiler.exec_name
if not suite:
suite = compiler.suite
self._output_flag = output_flag
super().__init__(name, exec_name, suite, Category.LINKER)
self._compiler = compiler
self.flags.extend(os.getenv("LDFLAGS", "").split())
# This will search for the name in linker or compiler
exec_name = self.get_exec_name()

super().__init__(
name=name,
exec_name=exec_name,
suite=self.suite,
category=Category.LINKER)

self.add_flags(os.getenv("LDFLAGS", "").split())

# Maintain a set of flags for common libraries.
self._lib_flags: Dict[str, List[str]] = {}
# Allow flags to include before or after any library-specific flags.
self._pre_lib_flags: List[str] = []
self._post_lib_flags: List[str] = []

@property
def mpi(self) -> bool:
''':returns: whether the linker supports MPI or not.'''
return self._compiler.mpi

def check_available(self) -> bool:
'''
:returns: whether the linker is available or not. We do this
by requesting the linker version.
''':returns: whether this linker is available by asking the wrapped
linker or compiler.
'''
if self._compiler:
return self._compiler.check_available()
assert self._linker # make mypy happy
return self._linker.check_available()

def get_exec_name(self) -> str:
''':returns: the name of the executable by asking the wrapped
linker or compiler.'''
if self._compiler:
return self._compiler.exec_name
assert self._linker # make mypy happy
return self._linker.exec_name

@property
def suite(self) -> str:
''':returns: the suite this linker belongs to by getting it from
the wrapper compiler or linker.'''
return cast(CompilerSuiteTool, (self._compiler or self._linker)).suite

@property
def mpi(self) -> bool:
''':returns" whether this linker supports MPI or not by checking
with the wrapper compiler or linker.'''
if self._compiler:
return self._compiler.mpi
assert self._linker # make mypy happy
return self._linker.mpi

return super().check_available()
@property
def openmp(self) -> bool:
''':returns" whether this linker supports OpenMP or not by checking
with the wrapper compiler or linker.'''
if self._compiler:
return self._compiler.openmp
assert self._linker # make mypy happy
return self._linker.openmp

@property
def output_flag(self) -> str:
''':returns: the flag that is used to specify the output name.
'''
if self._compiler:
return self._compiler.output_flag
assert self._linker # make mypy happy
return self._linker.output_flag

def get_lib_flags(self, lib: str) -> List[str]:
'''Gets the standard flags for a standard library
Expand All @@ -85,6 +134,10 @@ def get_lib_flags(self, lib: str) -> List[str]:
try:
return self._lib_flags[lib]
except KeyError:
# If a lib is not defined here, but this is a wrapper around
# another linker, return the result from the wrapped linker
if self._linker:
return self._linker.get_lib_flags(lib)
raise RuntimeError(f"Unknown library name: '{lib}'")

def add_lib_flags(self, lib: str, flags: List[str],
Expand Down Expand Up @@ -128,6 +181,47 @@ def add_post_lib_flags(self, flags: List[str]):
'''
self._post_lib_flags.extend(flags)

def get_pre_link_flags(self) -> List[str]:
'''Returns the list of pre-link flags. It will concatenate the
flags for this instance with all potentially wrapped linkers.
This wrapper's flag will come first - the assumption is that
the pre-link flags are likely paths, so we need a wrapper to
be able to put a search path before the paths from a wrapped
linker.
:returns: List of pre-link flags of this linker and all
wrapped linkers
'''
params: List[str] = []
if self._pre_lib_flags:
params.extend(self._pre_lib_flags)
if self._linker:
# If we are wrapping a linker, get the wrapped linker's
# pre-link flags and append them to the end (so the linker
# wrapper's settings come before the setting from the
# wrapped linker).
params.extend(self._linker.get_pre_link_flags())
return params

def get_post_link_flags(self) -> List[str]:
'''Returns the list of post-link flags. It will concatenate the
flags for this instance with all potentially wrapped linkers.
This wrapper's flag will be added to the end.
:returns: List of post-link flags of this linker and all
wrapped linkers
'''
params: List[str] = []
if self._linker:
# If we are wrapping a linker, get the wrapped linker's
# post-link flags and add them first (so this linker
# wrapper's settings come after the setting from the
# wrapped linker).
params.extend(self._linker.get_post_link_flags())
if self._post_lib_flags:
params.extend(self._post_lib_flags)
return params

def link(self, input_files: List[Path], output_file: Path,
openmp: bool,
libs: Optional[List[str]] = None) -> str:
Expand All @@ -141,21 +235,30 @@ def link(self, input_files: List[Path], output_file: Path,
:returns: the stdout of the link command
'''
if self._compiler:
# Create a copy:
params = self._compiler.flags[:]
if openmp:
params.append(self._compiler.openmp_flag)
else:
params = []

params: List[Union[str, Path]] = []

# Find the compiler by following the (potentially
# layered) linker wrapper.
linker = self
while linker._linker:
linker = linker._linker
# Now we must have a compiler
compiler = linker._compiler
assert compiler # make mypy happy
params.extend(compiler.flags)

if openmp:
params.append(compiler.openmp_flag)

# TODO: why are the .o files sorted? That shouldn't matter
params.extend(sorted(map(str, input_files)))
params.extend(self.get_pre_link_flags())

if self._pre_lib_flags:
params.extend(self._pre_lib_flags)
for lib in (libs or []):
params.extend(self.get_lib_flags(lib))
if self._post_lib_flags:
params.extend(self._post_lib_flags)
params.extend([self._output_flag, str(output_file)])

params.extend(self.get_post_link_flags())
params.extend([self.output_flag, str(output_file)])

return self.run(params)
2 changes: 1 addition & 1 deletion source/fab/tools/preprocessor.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class CppFortran(Preprocessor):
'''
def __init__(self):
super().__init__("cpp", "cpp", Category.FORTRAN_PREPROCESSOR)
self.flags.extend(["-traditional-cpp", "-P"])
self.add_flags(["-traditional-cpp", "-P"])


# ============================================================================
Expand Down
35 changes: 27 additions & 8 deletions source/fab/tools/tool_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
from fab.tools.tool import Tool
from fab.tools.category import Category
from fab.tools.compiler import Compiler
from fab.tools.compiler_wrapper import (CrayCcWrapper, CrayFtnWrapper,
Mpif90, Mpicc)
from fab.tools.compiler_wrapper import (CompilerWrapper, CrayCcWrapper,
CrayFtnWrapper, Mpif90, Mpicc)
from fab.tools.linker import Linker
from fab.tools.versioning import Fcm, Git, Subversion
from fab.tools import (Ar, Cpp, CppFortran, Craycc, Crayftn,
Expand Down Expand Up @@ -81,12 +81,12 @@ def __init__(self):
# Now create the potential mpif90 and Cray ftn wrapper
all_fc = self[Category.FORTRAN_COMPILER][:]
for fc in all_fc:
mpif90 = Mpif90(fc)
self.add_tool(mpif90)
if not fc.mpi:
mpif90 = Mpif90(fc)
self.add_tool(mpif90)
# I assume cray has (besides cray) only support for Intel and GNU
if fc.name in ["gfortran", "ifort"]:
crayftn = CrayFtnWrapper(fc)
print("NEW NAME", crayftn, crayftn.name)
self.add_tool(crayftn)

# Now create the potential mpicc and Cray cc wrapper
Expand Down Expand Up @@ -114,9 +114,28 @@ def add_tool(self, tool: Tool):

# If we have a compiler, add the compiler as linker as well
if tool.is_compiler:
tool = cast(Compiler, tool)
linker = Linker(name=f"linker-{tool.name}", compiler=tool)
self[linker.category].append(linker)
compiler = cast(Compiler, tool)
if isinstance(compiler, CompilerWrapper):
# If we have a compiler wrapper, create a new linker using
# the linker based on the wrappped compiler. For example, when
# creating linker-mpif90-gfortran, we want this to be based on
# linker-gfortran (and not on the compiler mpif90-gfortran),
# since the linker-gfortran might have library definitions
# that should be reused. So we first get the existing linker
# (since the compiler exists, a linker for this compiler was
# already created and must exist).
other_linker = self.get_tool(
category=Category.LINKER,
name=f"linker-{compiler.compiler.name}")
other_linker = cast(Linker, other_linker)
linker = Linker(linker=other_linker,
exec_name=compiler.exec_name,
name=f"linker-{compiler.name}")
self[linker.category].append(linker)
else:
linker = Linker(compiler=compiler,
name=f"linker-{compiler.name}")
self[linker.category].append(linker)

def get_tool(self, category: Category, name: str) -> Tool:
''':returns: the tool with a given name in the specified category.
Expand Down
7 changes: 3 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import pytest

from fab.tools import Category, CCompiler, FortranCompiler, Linker, ToolBox
from fab.tools import CCompiler, FortranCompiler, Linker, ToolBox


# This avoids pylint warnings about Redefining names from outer scope
Expand Down Expand Up @@ -44,10 +44,9 @@ def fixture_mock_fortran_compiler():


@pytest.fixture(name="mock_linker")
def fixture_mock_linker():
def fixture_mock_linker(mock_fortran_compiler):
'''Provides a mock linker.'''
mock_linker = Linker("mock_linker", "mock_linker.exe",
Category.FORTRAN_COMPILER)
mock_linker = Linker(mock_fortran_compiler)
mock_linker.run = mock.Mock()
mock_linker._version = (1, 2, 3)
mock_linker.add_lib_flags("netcdf", ["-lnetcdff", "-lnetcdf"])
Expand Down
Loading

0 comments on commit c585ff7

Please sign in to comment.