Merge pull request #3431 from MetBenjaminWent/force_private_parallel

Add basic means to force promote variables to parallel regions

Author: LonelyCat124

src/psyclone/psyir/transformations/omp_parallel_trans.py

@@ -34,12 +34,13 @@
 # Authors R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab
 #         A. B. G. Chalk STFC Daresbury Lab
 #         J. Henrichs, Bureau of Meteorology
-# Modified I. Kavcic, J. G. Wallwork, O. Brunt and L. Turner, Met Office
-#          S. Valat, Inria / Laboratoire Jean Kuntzmann
+# Modified I. Kavcic, J. G. Wallwork, O. Brunt and L. Turner, B. Went,
+#          Met Office, S. Valat, Inria / Laboratoire Jean Kuntzmann
 #          M. Schreiber, Univ. Grenoble Alpes / Inria / Lab. Jean Kuntzmann
 #          J. Dendy, Met Office
 '''This module provides the OMPParallelTrans transformation.'''
 
+from collections.abc import Iterable
 from psyclone import psyGen
 from psyclone.psyir.nodes import (
     ACCDirective,
@@ -48,6 +49,7 @@
     OMPParallelDirective,
     OMPDirective,
     Return,
+    RegionDirective,
 )
 from psyclone.psyir.transformations.parallel_region_trans import (
     ParallelRegionTrans)
@@ -121,14 +123,31 @@ def validate(self, nodes: list[Node], options=None, **kwargs):
         # TODO #2668: Remove options.
         super().validate(nodes, options, **kwargs)
 
-    def apply(self, nodes: list[Node], options=None, **kwargs):
+    def apply(
+            self, nodes: list[Node],
+            options=None, force_private: Iterable[str] = (),
+            **kwargs):
         '''
         Surrounds the provided node list with an OpenMP Parallel region.
 
         :param nodes: list of Nodes to put within parallel region.
+        :param force_private: list of symbols explicitly requested to
+            be private.
         '''
         # TODO #2668: Remove options.
         super().apply(nodes, options, **kwargs)
 
+        # Privatise the provided variables for the new RegionDirective, if they
+        # are found within the symbol table of the ancestor Routine.
+        if force_private:
+            new_region_directive = nodes[0].ancestor(RegionDirective)
+            if new_region_directive:
+                region_set = self._check_symbol_table_vars(
+                        new_region_directive,
+                        force_private)
+                if region_set:
+                    new_region_directive.explicitly_private_symbols.update(
+                        region_set)
+
 
 __all__ = ["OMPParallelTrans"]

src/psyclone/psyir/transformations/parallel_region_trans.py

@@ -34,22 +34,24 @@
 # Authors R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab
 #         A. B. G. Chalk, V. K. Atkinson, STFC Daresbury Lab
 #         J. Henrichs, Bureau of Meteorology
-# Modified I. Kavcic, J. G. Wallwork, O. Brunt and L. Turner, Met Office
-#          S. Valat, Inria / Laboratoire Jean Kuntzmann
+# Modified I. Kavcic, J. G. Wallwork, O. Brunt and L. Turner, B. Went,
+#          Met Office, S. Valat, Inria / Laboratoire Jean Kuntzmann
 #          M. Schreiber, Univ. Grenoble Alpes / Inria / Lab. Jean Kuntzmann
 #          J. Dendy, Met Office
 
 '''
 This module provides the implementation of ParallelRegionTrans
 
 '''
-
+import logging
+from collections.abc import Iterable
 from abc import ABC, abstractmethod
 from psyclone.psyir.transformations.transformation_error import (
     TransformationError)
 from psyclone import psyGen
 from psyclone.psyir.transformations.region_trans import RegionTrans
-from psyclone.psyir.nodes import CodeBlock, Node, Return
+from psyclone.psyir.nodes import CodeBlock, Node, Return, RegionDirective
+from psyclone.psyir.symbols import DataSymbol
 from psyclone.utils import transformation_documentation_wrapper
 
 
@@ -76,6 +78,35 @@ def __str__(self) -> str:
 
         '''
 
+    def _check_symbol_table_vars(
+            self, region_node: RegionDirective,
+            force_private: Iterable[str] = ()) -> set[DataSymbol]:
+        '''
+        Check that the symbol table of the provided region node contains the
+        variable variables in the provided list. Return a set of DataSymbols.
+
+        This is intended to be used as part of privatising the variables
+        contained in the list for the provided region in the child classes.
+        '''
+        explicitly_private_symbols = set()
+
+        for symbol_name in force_private:
+            sym = None
+            try:
+                sym = region_node.scope.symbol_table.lookup(
+                    symbol_name.lower())
+            except KeyError as err:
+                # This is not an error, but we will log the missed string
+                logger = logging.getLogger(__name__)
+                logger.warning(
+                    f"Error: {err} This has been provided with the "
+                    f"'{symbol_name}' in the 'force_private' option, "
+                    "but there is no such symbol in this scope.")
+            if sym:
+                explicitly_private_symbols.add(sym)
+
+        return explicitly_private_symbols
+
     def validate(self, nodes: list[Node], options=None, **kwargs):
         # pylint: disable=arguments-renamed
         '''

src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py

@@ -32,6 +32,7 @@
 # POSSIBILITY OF SUCH DAMAGE.
 # -----------------------------------------------------------------------------
 # Authors A. B. G. Chalk, STFC Daresbury Lab
+# Modified B. Went, Met Office
 
 '''This module contains the tests for the MaximalRegionTrans.'''
 
@@ -42,12 +43,14 @@
     Assignment,
     IfBlock,
     Routine,
+    Loop,
     OMPParallelDirective,
 )
 from psyclone.psyir.transformations import (
     MaximalRegionTrans,
     TransformationError,
-    OMPParallelTrans
+    OMPParallelTrans,
+    OMPLoopTrans,
 )
 
 
@@ -345,3 +348,41 @@ class OneParTrans(MaximalRegionTrans):
     assert isinstance(routine.children[0], OMPParallelDirective)
     assert isinstance(routine.children[1], Assignment)
     assert isinstance(routine.children[2], OMPParallelDirective)
+
+
+def test_apply_force_private(fortran_reader):
+    '''Test the apply function of MaxParallelRegionTrans
+       with force privates.'''
+    code = """subroutine x
+    integer :: i, ii, k, l, block
+    integer :: array_l(8)
+    block = 2
+    do ii = 1, 4
+        do k = 4, 1, -1
+            l = 0
+            do i = ii, min(ii+block -1, 4)
+                l = l + 1
+                array_l(l) = 1 + 2
+            end do
+        end do
+    end do
+    end subroutine x
+    """
+    psyir = fortran_reader.psyir_from_source(code)
+    # Apply loop_trans to all the loops possible.
+    ltrans = OMPLoopTrans(omp_schedule="static")
+    ltrans.apply(
+        psyir.walk(Loop)[0],
+        ignore_dependencies_for=["array_l"],
+        nowait=True)
+    # Apply maximum transformation to code
+    mtrans = MaxParTrans()
+    routine = psyir.walk(Routine)
+    # Note, i, ii, k, l seem to be set as first private
+    mtrans.apply(routine, force_private=["i", "ii", "k", "l", "array_l"])
+    # assertions
+    assert len(psyir.walk(OMPParallelDirective)) == 1
+    nodes = psyir.walk(Routine)[0].children[:]
+    assert isinstance(nodes[0], OMPParallelDirective) is True
+    pdir = nodes[0]
+    assert len(pdir.explicitly_private_symbols) == 5

src/psyclone/tests/psyir/transformations/parallel_region_trans_test.py

@@ -34,8 +34,8 @@
 # Authors R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab
 #         A. B. G. Chalk, V. K. Atkinson, STFC Daresbury Lab
 #         J. Henrichs, Bureau of Meteorology
-# Modified I. Kavcic, J. G. Wallwork, O. Brunt and L. Turner, Met Office
-#          S. Valat, Inria / Laboratoire Jean Kuntzmann
+# Modified I. Kavcic, J. G. Wallwork, O. Brunt and L. Turner, B. Went,
+#          Met Office, S. Valat, Inria / Laboratoire Jean Kuntzmann
 #          M. Schreiber, Univ. Grenoble Alpes / Inria / Lab. Jean Kuntzmann
 #          J. Dendy, Met Office
 
@@ -44,11 +44,11 @@
 
 '''
 
+import logging
 import pytest
 from psyclone.psyir.transformations.transformation_error import (
     TransformationError)
-from psyclone.psyir.nodes import CodeBlock
-from psyclone.psyir.nodes import (Literal, Loop)
+from psyclone.psyir.nodes import (CodeBlock, Literal, Loop)
 from psyclone.psyir.transformations import OMPParallelTrans
 from psyclone.psyir.symbols import DataSymbol, ScalarType
 
@@ -69,3 +69,32 @@ def test_parallelregion_refuse_codeblock():
         otrans.validate([parent])
     assert ("Nodes of type 'CodeBlock' cannot be enclosed by a "
             "OMPParallelTrans transformation" in str(err.value))
+
+
+def test_parallelregion_check_symtab_var(fortran_reader, caplog):
+    '''
+    Check ParallelRegionTrans._check_symbol_table_vars try and except,
+    if the logging message produces a warning when a variable is not
+    in the routine scope.We use OMPParallelTrans as ParallelRegionTrans
+    is abstract.
+    '''
+    otrans = OMPParallelTrans()
+    code = """subroutine test
+    integer :: i
+    do i = 1, 100
+
+    end do
+    end subroutine"""
+    psyir = fortran_reader.psyir_from_source(code)
+    otrans.apply(psyir.children[0].children[0])
+    parallel = psyir.children[0].children[0]
+    caplog.clear()
+    with caplog.at_level(logging.WARNING,
+                         logger="psyclone.psyir.transformations"):
+        otrans._check_symbol_table_vars(parallel, ("j"))
+    long_string = (
+        "Error: \"Could not find 'j' in the Symbol Table.\" This has been "
+        "provided with the 'j' in the 'force_private' option, "
+        "but there is no such symbol in this scope."
+    )
+    assert long_string in caplog.text