Merge pull request #222 from ecmwf-ifs/naml-ecphys-dev

Enabling SCC-Stack for EC-physics, part 2

loki/backend/fgen.py

@@ -605,10 +605,13 @@ def visit_Conditional(self, o, **kwargs):
         if o.inline:
             # No indentation and only a single body node
             cond = self.visit(o.condition, **kwargs)
+            d = self.depth
+            self.depth = 0
             body = self.visit(o.body, **kwargs)
-            line = self.format_line(f'IF ({cond}) ') + body.lstrip()
-            # Ensure we run multi-line formatter, in case the body is complex
-            return self.join_lines(line)
+            self.depth = d
+            # Undo the indentation, so that we may re-format and re-indent
+            line = f'IF ({cond}) ' + ''.join(body.lstrip().split('&\n&'))
+            return self.format_line(line)
 
         name = kwargs.pop('name', f' {o.name}' if o.name else '')
         is_elseif = kwargs.pop('is_elseif', False)

loki/bulk/item.py

@@ -97,6 +97,8 @@ def __init__(self, name, source, config=None):
         self.config = config or {}
         self.trafo_data = {}
 
+        self.is_ignored = False
+
     def __repr__(self):
         return f'loki.bulk.Item<{self.name}>'
 

loki/bulk/scheduler.py

@@ -369,12 +369,11 @@ def _add_children(self, item, children):
 
             # Append child to work queue if expansion is configured
             if item.expand:
-                # Do not propagate to dependencies marked as "ignore"
-                # Note that, unlike blackisted items, "ignore" items
-                # are still marked as targets during bulk-processing,
-                # so that calls to "ignore" routines will be renamed.
-                if child.local_name in item.ignore:
-                    continue
+                # Mark children as "ignored", which means they may be
+                # used for certain analysis passes, but are not part
+                # of the injected changes for this batch-transformation
+                if item.is_ignored or child.local_name in item.ignore:
+                    child.is_ignored = True
 
                 if child not in self.item_map:
                     new_items += [child]
@@ -575,9 +574,15 @@ def process(self, transformation):
                             continue
 
                     _item = items[0]
+                    if _item.is_ignored and not transformation.process_ignored_items:
+                        continue
+
                     transformation.apply(items[0].source, items=items)
             else:
                 for item in traversal:
+                    if item.is_ignored and not transformation.process_ignored_items:
+                        continue
+
                     if item_filter and not isinstance(item, item_filter):
                         continue
 
@@ -709,6 +714,9 @@ def write_cmake_plan(self, filepath, mode, buildpath, rootpath):
         sources_to_transform = []
 
         for item in self.items:
+            if item.is_ignored:
+                continue
+
             sourcepath = item.path.resolve()
             newsource = sourcepath.with_suffix(f'.{mode.lower()}.F90')
             if buildpath:

loki/transform/build_system_transform.py

@@ -11,108 +11,10 @@
 
 from pathlib import Path
 
-from loki.logging import info
 from loki.transform.transformation import Transformation
 from loki.bulk.item import SubroutineItem, GlobalVarImportItem
 
-__all__ = ['CMakePlanner', 'FileWriteTransformation']
-
-
-class CMakePlanner(Transformation):
-    """
-    Generates a list of files to add, remove or replace in a CMake
-    target's list of sources
-
-    This is intended to be used in a :any:`Scheduler` traversal triggered
-    during the configuration phase of a build (e.g., using ``execute_process``)
-    to generate a CMake plan file (using :meth:`write_planfile`).
-    This file set variables ``LOKI_SOURCES_TO_TRANSFORM``,
-    ``LOKI_SOURCES_TO_APPEND``, and ``LOKI_SOURCES_TO_REMOVE`` that can then
-    be used to update a target's ``SOURCES`` property via
-    ``get_target_property`` and ``set_property``.
-
-    Attributes
-    ----------
-    sources_to_append : list of str
-        Newly generated source files that need to be added to the target
-    sources_to_remove : list of str
-        The source files that are replaced and must be removed from the target
-    sources_to_transform : list of str
-        The source files that are going to be transformed by Loki
-        transformations
-
-    Parameters
-    ----------
-    rootpath : :any:`pathlib.Path` or str
-        The base directory of the source tree
-    mode : str
-        The name of the transformation mode (which is going to be inserted
-        into the file name of new source files)
-    build : :any:`pathlib.Path` or str
-        The target directory for generate source files
-    """
-
-    def __init__(self, rootpath, mode, build=None):
-        self.build = None if build is None else Path(build)
-        self.mode = mode
-
-        self.rootpath = Path(rootpath).resolve()
-        self.sources_to_append = []
-        self.sources_to_remove = []
-        self.sources_to_transform = []
-
-    def transform_subroutine(self, routine, **kwargs):
-        """
-        Insert the current subroutine into the lists of source files to
-        process, add and remove, if part of the plan
-
-        Parameters
-        ----------
-        routine : :any:`Subroutine`
-            The subroutine object to process
-        item : :any:`Item`
-            The corresponding work item from the :any:`Scheduler`
-        role : str
-            The routine's role
-        """
-        item = kwargs['item']
-        role = kwargs.get('role')
-
-        sourcepath = item.path.resolve()
-        newsource = sourcepath.with_suffix(f'.{self.mode.lower()}.F90')
-        if self.build is not None:
-            newsource = self.build/newsource.name
-
-        # Make new CMake paths relative to source again
-        sourcepath = sourcepath.relative_to(self.rootpath)
-
-        info(f'Planning:: {routine.name} (role={role}, mode={self.mode})')
-
-        self.sources_to_transform += [sourcepath]
-
-        # Inject new object into the final binary libs
-        if item.replicate:
-            # Add new source file next to the old one
-            self.sources_to_append += [newsource]
-        else:
-            # Replace old source file to avoid ghosting
-            self.sources_to_append += [newsource]
-            self.sources_to_remove += [sourcepath]
-
-    def write_planfile(self, filepath):
-        """
-        Write the CMake plan file at :data:`filepath`
-        """
-        info(f'[Loki] CMakePlanner writing plan: {filepath}')
-        with Path(filepath).open('w') as f:
-            s_transform = '\n'.join(f'    {s}' for s in self.sources_to_transform)
-            f.write(f'set( LOKI_SOURCES_TO_TRANSFORM \n{s_transform}\n   )\n')
-
-            s_append = '\n'.join(f'    {s}' for s in self.sources_to_append)
-            f.write(f'set( LOKI_SOURCES_TO_APPEND \n{s_append}\n   )\n')
-
-            s_remove = '\n'.join(f'    {s}' for s in self.sources_to_remove)
-            f.write(f'set( LOKI_SOURCES_TO_REMOVE \n{s_remove}\n   )\n')
+__all__ = ['FileWriteTransformation']
 
 
 class FileWriteTransformation(Transformation):

loki/transform/transform_hoist_variables.py

@@ -113,6 +113,8 @@ class HoistVariablesAnalysis(Transformation):
     # Apply in reverse order to recursively find all variables to be hoisted.
     reverse_traversal = True
 
+    process_ignored_items = True
+
     def __init__(self, key=None):
         if key is not None:
             self._key = key

loki/transform/transformation.py

@@ -66,6 +66,10 @@ class Transformation:
     recurse_to_internal_procedures : bool
         Apply transformation to all internal :any:`Subroutine` objects
         when processing :any:`Subroutine` objects (default ``False``)
+    process_ignored_items : bool
+        Apply transformation to "ignored" :any:`Item` objects for analysis.
+        This might be needed if IPO-information needs to be passed across
+        library boundaries.
     """
 
     # Forces scheduler traversal in reverse order from the leaf nodes upwards
@@ -82,6 +86,8 @@ class Transformation:
     recurse_to_procedures = False  # Recurse from Sourcefile/Module to subroutines and functions
     recurse_to_internal_procedures = False  # Recurse to subroutines in ``contains`` clause
 
+    # Option to process "ignored" items for analysis
+    process_ignored_items = False
 
     def transform_subroutine(self, routine, **kwargs):
         """

scripts/loki_transform.py

@@ -320,7 +320,7 @@ def transform_subroutine(self, routine, **kwargs):
 
 @cli.command('plan')
 @click.option('--mode', '-m', default='sca',
-              type=click.Choice(['idem', 'sca', 'claw', 'scc', 'scc-hoist']))
+              type=click.Choice(['idem', 'sca', 'claw', 'scc', 'scc-hoist', 'scc-stack']))
 @click.option('--config', '-c', type=click.Path(),
               help='Path to configuration file.')
 @click.option('--header', '-I', type=click.Path(), multiple=True,
@@ -354,6 +354,8 @@ def plan(mode, config, header, source, build, root, cpp, directive, frontend, ca
     paths += [Path(h).resolve().parent for h in header]
     scheduler = Scheduler(paths=paths, config=config, frontend=frontend, full_parse=False, preprocess=cpp)
 
+    mode = mode.replace('-', '_')  # Sanitize mode string
+
     # Construct the transformation plan as a set of CMake lists of source files
     scheduler.write_cmake_plan(filepath=plan_file, mode=mode, buildpath=build, rootpath=root)
 

tests/test_fgen.py

@@ -146,6 +146,30 @@ def test_multiline_inline_conditional(frontend):
     out = fgen(routine, linewidth=132)
     for line in out.splitlines():
         assert line.count('&') <= 2
+        if line.count('&') == 2:
+            assert len(line.split('&')[1]) > 60
+
+
+@pytest.mark.parametrize('frontend', available_frontends(xfail=[(OMNI, 'Loki likes only valid code')]))
+def test_multiline_inline_conditional_long(frontend):
+    """
+    Test correct formatting of an inline :any:`Conditional` that
+    that creates a particularly long line.
+    """
+    fcode = """
+subroutine test_inline_multiline_long(array, flag)
+  real, intent(inout) :: array
+  logical, intent(in) :: flag
+
+  if (flag) call a_subroutine_with_an_exquisitely_loong_and_expertly_chosen_name_and_a_few_keyword_arguments(my_favourite_array=array)
+end subroutine test_inline_multiline_long
+    """.strip()
+    routine = Subroutine.from_source(fcode, frontend=frontend)
+
+    out = fgen(routine, linewidth=132)
+    for line in out.splitlines():
+        assert len(line) < 132
+        assert line.count('&') <= 2
 
 
 @pytest.mark.parametrize('frontend', available_frontends())

tests/test_scheduler.py

@@ -57,7 +57,7 @@
 from loki import (
     Scheduler, SchedulerConfig, DependencyTransformation, FP, OFP,
     HAVE_FP, HAVE_OFP, REGEX, Sourcefile, FindNodes, CallStatement,
-    fexprgen, Transformation, BasicType, CMakePlanner, Subroutine,
+    fexprgen, Transformation, BasicType, Subroutine,
     SubroutineItem, ProcedureBindingItem, gettempdir, ProcedureSymbol,
     ProcedureType, DerivedType, TypeDef, Scalar, Array, FindInlineCalls,
     Import, Variable, GenericImportItem, GlobalVarImportItem, flatten,
@@ -827,8 +827,10 @@ def test_scheduler_dependencies_ignore(here, frontend):
         'driverB_mod#driverB', 'kernelB_mod#kernelB',
         'compute_l1_mod#compute_l1', 'compute_l2_mod#compute_l2'
     ])
-    assert 'ext_driver_mod#ext_driver' not in schedulerA.items
-    assert 'ext_kernel_mod#ext_kernel' not in schedulerA.items
+    assert 'ext_driver_mod#ext_driver' in schedulerA.items
+    assert 'ext_kernel_mod#ext_kernel' in schedulerA.items
+    assert schedulerA['ext_driver_mod#ext_driver'].is_ignored
+    assert schedulerA['ext_kernel_mod#ext_kernel'].is_ignored
 
     assert all(n in schedulerB.items for n in ['ext_driver_mod#ext_driver', 'ext_kernel_mod#ext_kernel'])
 
@@ -843,7 +845,9 @@ def test_scheduler_dependencies_ignore(here, frontend):
     assert schedulerA.items[0].source.all_subroutines[0].name == 'driverB'
     assert schedulerA.items[1].source.all_subroutines[0].name == 'kernelB_test'
     assert schedulerA.items[2].source.all_subroutines[0].name == 'compute_l1_test'
-    assert schedulerA.items[3].source.all_subroutines[0].name == 'compute_l2_test'
+    assert schedulerA.items[3].source.all_subroutines[0].name == 'ext_driver'
+    assert schedulerA.items[4].source.all_subroutines[0].name == 'compute_l2_test'
+    assert schedulerA.items[5].source.all_subroutines[0].name == 'ext_kernel'
 
     # For the second target lib, we want the driver to be converted
     for transformation in transformations:
@@ -892,24 +896,16 @@ def test_scheduler_cmake_planner(here, frontend):
     builddir.mkdir(exist_ok=True)
     planfile = builddir/'loki_plan.cmake'
 
-    planner = CMakePlanner(rootpath=sourcedir, mode='foobar', build=builddir)
-    scheduler.process(transformation=planner)
+    scheduler.write_cmake_plan(
+        filepath=planfile, mode='foobar', buildpath=builddir, rootpath=sourcedir
+    )
 
     # Validate the generated lists
     expected_files = {
         proj_a/'module/driverB_mod.f90', proj_a/'module/kernelB_mod.F90',
         proj_a/'module/compute_l1_mod.f90', proj_a/'module/compute_l2_mod.f90'
     }
 
-    assert set(planner.sources_to_remove) == {f.relative_to(sourcedir) for f in expected_files}
-    assert set(planner.sources_to_append) == {
-        (builddir/f.stem).with_suffix('.foobar.F90') for f in expected_files
-    }
-    assert set(planner.sources_to_transform) == {f.relative_to(sourcedir) for f in expected_files}
-
-    # Write the plan file
-    planner.write_planfile(planfile)
-
     # Validate the plan file content
     plan_pattern = re.compile(r'set\(\s*(\w+)\s*(.*?)\s*\)', re.DOTALL)
 

tests/test_transform_hoist_variables.py

@@ -165,9 +165,11 @@ def test_hoist_disable(here, frontend, config):
     """
 
     disable = ("device1", "device2")
-    config['routines']['kernel2'] = {'role': 'kernel', 'ignore': disable}
+    config['routines']['kernel2'] = {'role': 'kernel', 'block': disable}
     proj = here/'sources/projHoist'
-    scheduler = Scheduler(paths=[proj], config=config, seed_routines=['driver', 'another_driver'], frontend=frontend)
+    scheduler = Scheduler(
+        paths=[proj], config=config, seed_routines=['driver', 'another_driver'], frontend=frontend
+    )
 
     # Transformation: Analysis
     scheduler.process(transformation=HoistVariablesAnalysis())