From 172c2059c25b81516187e143e7fa155712cfadfb Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Fri, 31 Oct 2025 12:11:47 +0000 Subject: [PATCH 01/16] Initial implementation of maximal parallel region trans. Tests TODO --- .../maximal_parallel_region_trans.py | 167 ++++++++++++++++++ .../omp_maximal_parallel_region_trans.py | 67 +++++++ 2 files changed, 234 insertions(+) create mode 100644 src/psyclone/psyir/transformations/maximal_parallel_region_trans.py create mode 100644 src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py diff --git a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py new file mode 100644 index 0000000000..e01474b0e3 --- /dev/null +++ b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py @@ -0,0 +1,167 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2017-2025, Science and Technology Facilities Council. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# ----------------------------------------------------------------------------- +# Authors A. B. G. Chalk, STFC Daresbury Lab + +'''This module contains the MaximalParallelRoutineTrans.''' + +import abc +from typing import Union, List + +from psyclone.psyir.nodes import ( + Node, + Schedule, + Loop, + IfBlock, +) +from psyclone.psyir.transformations.region_trans import RegionTrans +from psyclone.psyir.transformations.transformation_error import \ + TransformationError + + +class MaximalParallelRegionTrans(RegionTrans, metaclass=abc.ABCMeta): + '''Abstract transformation containing the functionality to add + the largest allowed parallel regions to the provided code segment. + + Subclasses should override the _parallel_transformation and _allowed_nodes + members to control the functionality. + + The _parallel_transformation should be a transformation class to apply to + the computed parallel regions. + + The _allowed_nodes is a tuple of Node classes that are allowed as + statements in the parallel region. Note that upon finding a Loop or + IfBlock, the node's children will be checked to determine whether its safe + to contain the Loop or IfBlock in the parallel section.''' + + # The type of parallel transformation to be applied to the input region. + _parallel_transformation = None + # Tuple of statement nodes allowed inside the _parallel_transformation + _allowed_nodes = () + + def _can_be_in_parallel_region(self, node: Node) -> bool: + '''Returns whether the provided node can be included in an + OpenMP parallel region. Most OpenMP directives can be included, + and loops and if statements are recursed into to check if their + children can be. + + Other statements are currently not added to an OpenMP parallel region + by this transformation. + + :param node: the candidate Node to be placed into a parallel region. + + :returns: whether it is safe to add the node to the parallel region. + ''' + + if isinstance(node, self._allowed_nodes): + return True + + if isinstance(node, Loop): + # Recurse through the loop body. + for child in node.loop_body: + if not self._can_be_in_parallel_region(child): + break + else: + return True + return False + + if isinstance(node, IfBlock): + # Recurse through the if_body and else_body + allowed = True + for child in node.if_body: + allowed = (allowed and self._can_be_in_parallel_region(child)) + if node.else_body and allowed: + for child in node.else_body: + allowed = (allowed and + self._can_be_in_parallel_region(child)) + return allowed + + # All other node types we default to False. + return False + + def validate(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): + '''Validates whether this transformation can be applied to the + nodes provided. + + :param nodes: can be a single node, a schedule or a list of nodes. + + :raises TransformationError: if the nodes provided don't all have the + same parent and aren't consecutive. + ''' + + self.validate_options(**kwargs) + node_list = self.get_node_list(nodes) + + node_parent = node_list[0].parent + prev_position = node_list[0].position + for child in node_list[1:]: + if child.parent is not node_parent: + raise TransformationError( + f"Error in {self.name} transformation: supplied nodes " + f"are not children of the same parent.") + if prev_position+1 != child.position: + raise TransformationError( + f"Children are not consecutive children of one parent: " + f"child '{child}' has position {child.position}, but " + f"previous child had position {prev_position}.") + prev_position = child.position + + def apply(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): + '''Applies the transformation to the nodes provided. + + :param nodes: can be a single node, a schedule or a list of nodes. + ''' + node_list = self.get_node_list(nodes) + + # Call validate. + self.validate(nodes, **kwargs) + + par_trans = self._parallel_transformation() + + # Find the largest sections we can surround with parallel regions. + current_block = [] + for child in node_list: + # If the child can be added to a parallel region then add it + # to the current block of nodes. + if self._can_be_in_parallel_region(child): + current_block.append(child) + else: + # Otherwise, if the current_block contains any children, + # add them to a parallel region and reset the current_block. + if current_block: + par_trans.apply(current_block) + current_block = [] + # If any nodes are left in the current block at the end of the + # node_list, then add them to a parallel region + if current_block: + par_trans.apply(current_block) diff --git a/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py new file mode 100644 index 0000000000..719cecd043 --- /dev/null +++ b/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py @@ -0,0 +1,67 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2017-2025, Science and Technology Facilities Council. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# ----------------------------------------------------------------------------- +# Authors A. B. G. Chalk, STFC Daresbury Lab + +'''This module contains the OMPMaximalParallelRegionTrans.''' + +from psyclone.psyir.nodes import ( + OMPTaskwaitDirective, + OMPBarrierDirective, + OMPSerialDirective, + OMPTaskloopDirective, + OMPDoDirective, + OMPLoopDirective, + OMPTaskDirective, + DynamicOMPTaskDirective, +) +from psyclone.psyir.transformations.maximal_parallel_region_trans import ( + MaximalParallelRegionTrans) +from psyclone.transformations import OMPParallelTrans + + +class OMPMaximalParallelRegionTrans(MaximalParallelRegionTrans): + '''TODO''' + # The type of parallel transformation to be applied to the input region. + _parallel_transformation = OMPParallelTrans + # Tuple of statement nodes allowed inside the _parallel_transformation + _allowed_nodes = ( + OMPTaskwaitDirective, + OMPBarrierDirective, + OMPSerialDirective, + OMPTaskloopDirective, + OMPDoDirective, + OMPLoopDirective, + OMPTaskDirective, + DynamicOMPTaskDirective, + ) From 8b19ef04b0e3faa73bd4f108f27fbeb5d072e9a1 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Fri, 31 Oct 2025 15:32:23 +0000 Subject: [PATCH 02/16] Tests for base MaximalParallelRegionTrans class --- .../maximal_parallel_region_trans_test.py | 152 ++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py diff --git a/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py b/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py new file mode 100644 index 0000000000..3780836705 --- /dev/null +++ b/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py @@ -0,0 +1,152 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2017-2025, Science and Technology Facilities Council. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# ----------------------------------------------------------------------------- +# Authors A. B. G. Chalk, STFC Daresbury Lab + +'''This module contains the tests for the MaximalParallelRegionTrans.''' + +import pytest + +from psyclone.psyir.nodes import ( + Assignment, + IfBlock, + Routine, + OMPParallelDirective, +) +from psyclone.psyir.transformations import ( + MaximalParallelRegionTrans, + TransformationError +) +from psyclone.transformations import OMPParallelTrans + + +# Dummy class to test MaxParallelRegionTrans' functionality. +class MaxParTrans(MaximalParallelRegionTrans): + # The apply function will do OMPParallelTrans around allowed regions. + _parallel_transformation = OMPParallelTrans + # We're only allowing assignment because its straightforward to test with. + _allowed_nodes = (Assignment, ) + + +@pytest.mark.parametrize( + "statement,expected", + [ + ("i = 1", True), + ("call a_function()", False), + ("do i = 1, 100\nj = j + 1\nend do", True), + ("do i = 1, 100\ncall a_function()\nend do", False), + ("if (.true.) then\nj=3\nend if", True), + ("if(.true.) then\nj=3\nelse\nj=3\nend if", True), + ("if(.true.) then\ncall a_function()\nelse\nj=3\nendif", False), + ("if(.true.) then\nj=3\nelse\ncall a_function()\nendif", False), + ] +) +def test_can_be_in_parallel_region(fortran_reader, statement, expected): + '''Test the _can_be_in_parallel_region function of + MaxParallelRegionTrans.''' + code = f""" + subroutine test + use some_module + integer :: i, j + {statement} + end subroutine test + """ + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + trans = MaxParTrans() + assert trans._can_be_in_parallel_region(routine.children[0]) == expected + + +def test_validate(fortran_reader): + '''Test the validate function of MaxParallelRegionTrans.''' + code = """ + subroutine test + integer :: i, j + i = 1 + j = 1 + k = i + 1 + if(.true.) then + k = i + j + end if + end subroutine test""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + trans = MaxParTrans() + # Validate should allow us to give the full children + trans.validate(routine.children) + + # Validate should not allow non consecutive children + with pytest.raises(TransformationError) as err: + trans.validate([routine.children[0], routine.children[2]]) + assert ("Children are not consecutive children of one parent: child " + "'k = i + 1' has position 2, but previous child had position 0." + in str(err.value)) + + # Validate should not allow children of different parents. + with pytest.raises(TransformationError) as err: + trans.validate([routine.children[0], + routine.children[3].if_body.children[0]]) + assert ("Error in MaxParTrans transformation: supplied nodes are not " + "children of the same parent" in str(err.value)) + + +def test_apply(fortran_reader): + '''Test the apply function of MaxParallelRegionTrans.''' + code = """ + subroutine test + use some_module + integer :: i, j + i = 1 + j = 1 + call a_function() + if(.true.) then + i = 1 + end if + j = 1 + end subroutine test + """ + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + trans = MaxParTrans() + trans.apply(routine) + # The result should be two OMPParallelDirectives, one containing + # i = 1 and j = 1, and another containing the IFBlock and the second j = 1 + dirs = routine.walk(OMPParallelDirective) + assert len(dirs) == 2 + + assert len(dirs[0].dir_body.children) == 2 + assert dirs[0].dir_body.children[0].debug_string() == "i = 1\n" + assert dirs[0].dir_body.children[1].debug_string() == "j = 1\n" + + assert isinstance(dirs[1].dir_body.children[0], IfBlock) + assert dirs[1].dir_body.children[1].debug_string() == "j = 1\n" From 38ef3e6a3340e4bac8b31847fdebc4f81fb26dfa Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Fri, 31 Oct 2025 15:56:34 +0000 Subject: [PATCH 03/16] Missed __init__ updates --- src/psyclone/psyir/transformations/__init__.py | 3 +++ .../psyir/transformations/maximal_parallel_region_trans.py | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/psyclone/psyir/transformations/__init__.py b/src/psyclone/psyir/transformations/__init__.py index 2b8143a379..49ce36c59b 100644 --- a/src/psyclone/psyir/transformations/__init__.py +++ b/src/psyclone/psyir/transformations/__init__.py @@ -94,6 +94,8 @@ from psyclone.psyir.transformations.loop_trans import LoopTrans from psyclone.psyir.transformations.value_range_check_trans import ( ValueRangeCheckTrans) +from psyclone.psyir.transformations.maximal_parallel_region_trans import ( + MaximalParallelRegionTrans) from psyclone.psyir.transformations.omp_loop_trans import OMPLoopTrans from psyclone.psyir.transformations.omp_minimise_sync_trans import \ OMPMinimiseSyncTrans @@ -174,4 +176,5 @@ "ParallelRegionTrans", "OMPTaskloopTrans", "OMPDeclareTargetTrans", + "MaximalParallelRegionTrans", ] diff --git a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py index e01474b0e3..87c629a725 100644 --- a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py @@ -132,8 +132,9 @@ def validate(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): if prev_position+1 != child.position: raise TransformationError( f"Children are not consecutive children of one parent: " - f"child '{child}' has position {child.position}, but " - f"previous child had position {prev_position}.") + f"child '{child.debug_string().rstrip()}' has position " + f"{child.position}, but previous child had position " + f"{prev_position}.") prev_position = child.position def apply(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): From 1b86da510928790a332398cff4311b5d2518852f Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Mon, 24 Nov 2025 15:17:13 +0000 Subject: [PATCH 04/16] Changes to fix unnecessary parallel additions and to recurse correctly on applying inside if/loop nodes --- .../maximal_parallel_region_trans.py | 29 +++++++++++++++++-- .../omp_maximal_parallel_region_trans.py | 8 +++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py index 87c629a725..0b0dda900a 100644 --- a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py @@ -43,6 +43,7 @@ Schedule, Loop, IfBlock, + WhileLoop, ) from psyclone.psyir.transformations.region_trans import RegionTrans from psyclone.psyir.transformations.transformation_error import \ @@ -68,6 +69,10 @@ class MaximalParallelRegionTrans(RegionTrans, metaclass=abc.ABCMeta): _parallel_transformation = None # Tuple of statement nodes allowed inside the _parallel_transformation _allowed_nodes = () + # Tuple of nodes that there must be at least one of inside the block + # to be parallelised, else the block can be ignored (e.g. a block of + # only barriers doesn't need to be parallelised). + _required_nodes = () def _can_be_in_parallel_region(self, node: Node) -> bool: '''Returns whether the provided node can be included in an @@ -158,11 +163,29 @@ def apply(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): current_block.append(child) else: # Otherwise, if the current_block contains any children, - # add them to a parallel region and reset the current_block. + # add them to a parallel region if we should and reset + # the current_block. if current_block: - par_trans.apply(current_block) + for node in current_block: + if node.walk(self._required_nodes, + stop_type=self._required_nodes): + par_trans.apply(current_block) + break current_block = [] + # Need to recurse on some node types + if isinstance(child, IfBlock): + self.apply(child.if_body) + if child.else_body: + self.apply(child.else_body) + if isinstance(child, Loop): + self.apply(child.loop_body) + if isinstance(child, WhileLoop): + self.apply(child.loop_body) # If any nodes are left in the current block at the end of the # node_list, then add them to a parallel region if current_block: - par_trans.apply(current_block) + for node in current_block: + if node.walk(self._required_nodes, + stop_type=self._required_nodes): + par_trans.apply(current_block) + break diff --git a/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py index 719cecd043..77866ddf4b 100644 --- a/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py @@ -65,3 +65,11 @@ class OMPMaximalParallelRegionTrans(MaximalParallelRegionTrans): OMPTaskDirective, DynamicOMPTaskDirective, ) + _required_nodes = ( + OMPSerialDirective, + OMPTaskloopDirective, + OMPDoDirective, + OMPLoopDirective, + OMPTaskDirective, + DynamicOMPTaskDirective, + ) From f3fa2a3a5ed49196c736cdf1c087a6567561edcd Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 26 Nov 2025 13:49:55 +0000 Subject: [PATCH 05/16] Fixed some of the behaviour to improve barrier removal --- .../omp_minimise_sync_trans.py | 15 +++++++++++++ .../omp_minimise_sync_trans_test.py | 22 ++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/psyclone/psyir/transformations/omp_minimise_sync_trans.py b/src/psyclone/psyir/transformations/omp_minimise_sync_trans.py index b107f07bdc..f9f0e17444 100644 --- a/src/psyclone/psyir/transformations/omp_minimise_sync_trans.py +++ b/src/psyclone/psyir/transformations/omp_minimise_sync_trans.py @@ -172,6 +172,18 @@ def validate(self, node: Routine, **kwargs) -> None: raise TypeError(f"OMPMinimiseSyncTrans expects a Routine input " f"but found '{type(node).__name__}'.") + def _eliminate_uncontained_barriers(self, routine: Routine) -> None: + ''' + Removes any OMPBarrierDirectives that are not inside an + OMPParallelRegion. + + :param routine: the routine to remove uncontainined barriers from. + ''' + barriers = routine.walk(OMPBarrierDirective) + for bar in barriers: + if bar.ancestor(OMPParallelDirective) is None: + bar.detach() + def _eliminate_adjacent_barriers(self, routine: Routine, bar_type: type) -> None: ''' @@ -537,6 +549,9 @@ def apply(self, node: Routine, **kwargs) -> None: # if its a OMPBarrierDirective as they are uneccessary. for parallel in node.walk(OMPParallelDirective): _eliminate_final_parallel_barrier(parallel) + # Finally eliminate any barriers leftover outside of parallel + # regions, as these are now superfluous + self._eliminate_uncontained_barriers(node) # Eliminate OMPTaskwaitDirectives for the gpu_directives if len(gpu_directives) > 0: self._eliminate_adjacent_barriers(node, OMPTaskwaitDirective) diff --git a/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py b/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py index 6ad2e00f61..c66ee62ec0 100644 --- a/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py @@ -42,8 +42,9 @@ OMPTargetDirective, OMPParallelDirective) from psyclone.psyir.transformations import ( OMPLoopTrans, OMPMinimiseSyncTrans, - OMPTargetTrans, TransformationError + OMPTargetTrans, TransformationError, ) +from psyclone.transformations import OMPParallelTrans from psyclone.psyir.transformations.omp_minimise_sync_trans import ( _eliminate_final_parallel_barrier ) @@ -67,6 +68,25 @@ def test_omp_remove_barrier_validate(): in str(excinfo.value)) +def test_omp_eliminate_uncontained_barriers(fortran_reader): + ''' + Test the _eliminite_uncontained_barriers routine of the + OMPMinimiseSyncTrans.''' + code = """subroutine test + + end subroutine + """ + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + routine.addchild(OMPBarrierDirective()) + routine.addchild(OMPBarrierDirective()) + partrans = OMPParallelTrans() + partrans.apply(routine.children[1]) + assert len(routine.walk(OMPBarrierDirective)) == 2 + OMPMinimiseSyncTrans()._eliminate_uncontained_barriers(routine) + assert len(routine.walk(OMPBarrierDirective)) == 1 + + def test_omp_eliminate_adjacent_barriers(fortran_reader): '''Test the _eliminate_adjacent_barriers routine of the OMPMinimiseSyncTrans.''' From cbdde6d6e7b9a43309157921831815a0575f55e6 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 26 Nov 2025 14:19:12 +0000 Subject: [PATCH 06/16] linting --- .../tests/psyir/transformations/omp_minimise_sync_trans_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py b/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py index c66ee62ec0..8a1729d0f1 100644 --- a/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/omp_minimise_sync_trans_test.py @@ -48,7 +48,6 @@ from psyclone.psyir.transformations.omp_minimise_sync_trans import ( _eliminate_final_parallel_barrier ) -from psyclone.transformations import OMPParallelTrans def test_omp_remove_barrier_trans_str(): From 0617319fd3dc49a34861bf746f576c45bd01925d Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Wed, 26 Nov 2025 15:49:21 +0000 Subject: [PATCH 07/16] Update the test to handle the new required node structure --- .../tests/psyir/nodes/maximal_parallel_region_trans_test.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py b/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py index 3780836705..c06c7e2d1a 100644 --- a/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py +++ b/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py @@ -56,6 +56,8 @@ class MaxParTrans(MaximalParallelRegionTrans): _parallel_transformation = OMPParallelTrans # We're only allowing assignment because its straightforward to test with. _allowed_nodes = (Assignment, ) + # Should parallelise any found region that contains an assignment. + _required_nodes = (Assignment, ) @pytest.mark.parametrize( From bdc9f12995aeadbffe61a4c843e149680ae2d8f6 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Mon, 12 Jan 2026 14:12:54 +0000 Subject: [PATCH 08/16] Added tests --- .../maximal_parallel_region_trans.py | 4 +- .../omp_maximal_parallel_region_trans.py | 7 +- .../maximal_parallel_region_trans_test.py | 224 ++++++++++++++++++ 3 files changed, 232 insertions(+), 3 deletions(-) create mode 100644 src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py diff --git a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py index 0b0dda900a..120246b903 100644 --- a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py @@ -1,7 +1,7 @@ # ----------------------------------------------------------------------------- # BSD 3-Clause License # -# Copyright (c) 2017-2025, Science and Technology Facilities Council. +# Copyright (c) 2025-2026, Science and Technology Facilities Council. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -91,7 +91,7 @@ def _can_be_in_parallel_region(self, node: Node) -> bool: if isinstance(node, self._allowed_nodes): return True - if isinstance(node, Loop): + if isinstance(node, (Loop, WhileLoop)): # Recurse through the loop body. for child in node.loop_body: if not self._can_be_in_parallel_region(child): diff --git a/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py index 77866ddf4b..2651314883 100644 --- a/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py @@ -51,7 +51,12 @@ class OMPMaximalParallelRegionTrans(MaximalParallelRegionTrans): - '''TODO''' + '''Applies OpenMP Parallel directives around the largest possible sections + of the input. + + At current, this will never place OpenMP parallel sections around + Assignments that are outside of another OpenMP directive. See #3157 and + the discussion on #3205 for more detail.''' # The type of parallel transformation to be applied to the input region. _parallel_transformation = OMPParallelTrans # Tuple of statement nodes allowed inside the _parallel_transformation diff --git a/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py b/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py new file mode 100644 index 0000000000..94ca61f8bd --- /dev/null +++ b/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py @@ -0,0 +1,224 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2025-2026, Science and Technology Facilities Council. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# ----------------------------------------------------------------------------- +# Authors A. B. G. Chalk, STFC Daresbury Lab + +'''This module contains the tests for the MaximalParallelRegionTrans.''' + +import pytest + +from psyclone.psyir.nodes import ( + Assignment, IfBlock, Call, Loop, OMPParallelDirective, Routine +) +from psyclone.psyir.transformations.maximal_parallel_region_trans import ( + MaximalParallelRegionTrans +) +from psyclone.psyir.transformations.transformation_error import ( + TransformationError +) +from psyclone.transformations import OMPParallelTrans + + +class DummyMaxTrans(MaximalParallelRegionTrans): + '''Test class to test the functionality of the MaximalParallelRegionTrans + ''' + _parallel_transformation = OMPParallelTrans + _allowed_nodes = (Assignment, IfBlock) + _required_nodes = (Assignment) + + +def test_can_be_in_parallel_region(fortran_reader): + '''Test the can_be_in_parallel_region function of the + MaximalParallelRegionTrans.''' + code = """subroutine x + use some_mod + integer :: i, j, k, l, m + + i = 2 + if(k == 3) then + call something() + end if + call something() + do j = k, l + i = 3 + end do + do j = k, l + call something() + end do + end subroutine x""" + psyir = fortran_reader.psyir_from_source(code) + mtrans = DummyMaxTrans() + # First case - Assignment is in _allowed_nodes so should be True + assert mtrans._can_be_in_parallel_region(psyir.walk(Assignment)[0]) + # Second case - IfBlock is in _allowed_nodes so should be True + assert mtrans._can_be_in_parallel_region(psyir.walk(IfBlock)[0]) + # Third case - Call is not in _allowed_nodes so should be False + assert not mtrans._can_be_in_parallel_region(psyir.walk(Call)[0]) + # Fourth case - Loop containing only nodes in _allowed_nodes so should be + # true. + assert mtrans._can_be_in_parallel_region(psyir.walk(Loop)[0]) + # Fifth case - Loop containing a node not in _allowed_nodes so should be + # False. + assert not mtrans._can_be_in_parallel_region(psyir.walk(Loop)[1]) + + +def test_validate(fortran_reader): + '''Test the validate function of the MaximalParallelRegionTrans.''' + code = """subroutine x + integer :: i, j, k, l, m + i = 1 + if(i == 3) then + i = 2 + end if + i = 3 + end subroutine x""" + psyir = fortran_reader.psyir_from_source(code) + assigns = psyir.walk(Assignment) + mtrans = DummyMaxTrans() + + with pytest.raises(TransformationError) as err: + mtrans.validate([assigns[0], assigns[1]]) + assert ("Error in DummyMaxTrans transformation: supplied nodes are not " + "children of the same parent." in str(err.value)) + + with pytest.raises(TransformationError) as err: + mtrans.validate([assigns[0], assigns[2]]) + assert ("Children are not consecutive children of one parent: " + "child 'i = 3' has position 2, but previous child had position 0." + in str(err.value)) + + +class DummyMaxTrans2(MaximalParallelRegionTrans): + '''Test class to test the functionality of the MaximalParallelRegionTrans + apply method + ''' + _parallel_transformation = OMPParallelTrans + _allowed_nodes = (Assignment) + _required_nodes = (Assignment) + + +def test_apply(fortran_reader): + '''Test the apply function of the MaximalParallelRegionTrans.''' + mtrans = DummyMaxTrans2() + + code = """subroutine x + integer :: i, j, k, l + + i = 1 + j = 2 + k = 3 + l = 4 + end subroutine x""" + psyir = fortran_reader.psyir_from_source(code) + assigns = psyir.walk(Assignment) + mtrans.apply(assigns) + assert len(psyir.walk(OMPParallelDirective)) == 1 + pdir = psyir.walk(OMPParallelDirective)[0] + # All the assignments should be in the parallel directive. + for assign in assigns: + assert assign.parent.parent is pdir + + code = """subroutine x + integer :: i, j, k, l + + i = 1 + do j = 2, 3 + k = 1 + end do + if (j == 2) then + k = 4 + end if + do while(j < 3) + j = j + 1 + end do + i = 4 + end subroutine x + """ + psyir = fortran_reader.psyir_from_source(code) + nodes = psyir.walk(Routine)[0].children[:] + mtrans.apply(nodes) + assert len(psyir.walk(OMPParallelDirective)) == 1 + pdir = psyir.walk(OMPParallelDirective)[0] + # All of the blocks here should be in the same ParallelDirective + for node in nodes: + assert node.parent.parent is pdir + + code = """subroutine x + use some_mod + integer :: i + + i = 1 + call something() + i = 2 + end subroutine x""" + psyir = fortran_reader.psyir_from_source(code) + nodes = psyir.walk(Routine)[0].children[:] + mtrans.apply(nodes) + pdirs = psyir.walk(OMPParallelDirective) + assert len(pdirs) == 2 + # All of the blocks here should be in the same ParallelDirective + assert nodes[0].parent.parent is pdirs[0] + assert not nodes[1].ancestor(OMPParallelDirective) + assert nodes[2].parent.parent is pdirs[1] + + code = """subroutine x + use some_mod + integer :: i, j + + if(i == 1) then + call something() + i = 2 + else + i = 3 + end if + + do i = 1,5 + call something() + j = 2 + end do + + do while(j == 3) + call something() + j = j + 2 + end do + end subroutine x""" + # Each of the nodes should contain OMPParallels inside them and there + # should be no top level OMPParallelDirective + psyir = fortran_reader.psyir_from_source(code) + nodes = psyir.walk(Routine)[0].children[:] + mtrans.apply(nodes) + assert len(psyir.walk(OMPParallelDirective)) == 4 + assert len(nodes[0].walk(OMPParallelDirective)) == 2 + assert len(nodes[0].if_body.children) == 2 + assert isinstance(nodes[0].if_body.children[1], OMPParallelDirective) + assert isinstance(nodes[0].else_body.children[0], OMPParallelDirective) From 67ae4c0f1aca0e77e381739cb60b90deddf90a98 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Mon, 12 Jan 2026 14:38:02 +0000 Subject: [PATCH 09/16] Fix test overlaps --- .../maximal_parallel_region_trans_test.py | 154 ---------------- .../maximal_parallel_region_trans_test.py | 166 ++++++++++-------- 2 files changed, 94 insertions(+), 226 deletions(-) delete mode 100644 src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py diff --git a/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py b/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py deleted file mode 100644 index c06c7e2d1a..0000000000 --- a/src/psyclone/tests/psyir/nodes/maximal_parallel_region_trans_test.py +++ /dev/null @@ -1,154 +0,0 @@ -# ----------------------------------------------------------------------------- -# BSD 3-Clause License -# -# Copyright (c) 2017-2025, Science and Technology Facilities Council. -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are met: -# -# * Redistributions of source code must retain the above copyright notice, this -# list of conditions and the following disclaimer. -# -# * Redistributions in binary form must reproduce the above copyright notice, -# this list of conditions and the following disclaimer in the documentation -# and/or other materials provided with the distribution. -# -# * Neither the name of the copyright holder nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS -# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE -# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, -# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, -# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; -# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER -# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT -# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN -# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE -# POSSIBILITY OF SUCH DAMAGE. -# ----------------------------------------------------------------------------- -# Authors A. B. G. Chalk, STFC Daresbury Lab - -'''This module contains the tests for the MaximalParallelRegionTrans.''' - -import pytest - -from psyclone.psyir.nodes import ( - Assignment, - IfBlock, - Routine, - OMPParallelDirective, -) -from psyclone.psyir.transformations import ( - MaximalParallelRegionTrans, - TransformationError -) -from psyclone.transformations import OMPParallelTrans - - -# Dummy class to test MaxParallelRegionTrans' functionality. -class MaxParTrans(MaximalParallelRegionTrans): - # The apply function will do OMPParallelTrans around allowed regions. - _parallel_transformation = OMPParallelTrans - # We're only allowing assignment because its straightforward to test with. - _allowed_nodes = (Assignment, ) - # Should parallelise any found region that contains an assignment. - _required_nodes = (Assignment, ) - - -@pytest.mark.parametrize( - "statement,expected", - [ - ("i = 1", True), - ("call a_function()", False), - ("do i = 1, 100\nj = j + 1\nend do", True), - ("do i = 1, 100\ncall a_function()\nend do", False), - ("if (.true.) then\nj=3\nend if", True), - ("if(.true.) then\nj=3\nelse\nj=3\nend if", True), - ("if(.true.) then\ncall a_function()\nelse\nj=3\nendif", False), - ("if(.true.) then\nj=3\nelse\ncall a_function()\nendif", False), - ] -) -def test_can_be_in_parallel_region(fortran_reader, statement, expected): - '''Test the _can_be_in_parallel_region function of - MaxParallelRegionTrans.''' - code = f""" - subroutine test - use some_module - integer :: i, j - {statement} - end subroutine test - """ - psyir = fortran_reader.psyir_from_source(code) - routine = psyir.walk(Routine)[0] - trans = MaxParTrans() - assert trans._can_be_in_parallel_region(routine.children[0]) == expected - - -def test_validate(fortran_reader): - '''Test the validate function of MaxParallelRegionTrans.''' - code = """ - subroutine test - integer :: i, j - i = 1 - j = 1 - k = i + 1 - if(.true.) then - k = i + j - end if - end subroutine test""" - psyir = fortran_reader.psyir_from_source(code) - routine = psyir.walk(Routine)[0] - trans = MaxParTrans() - # Validate should allow us to give the full children - trans.validate(routine.children) - - # Validate should not allow non consecutive children - with pytest.raises(TransformationError) as err: - trans.validate([routine.children[0], routine.children[2]]) - assert ("Children are not consecutive children of one parent: child " - "'k = i + 1' has position 2, but previous child had position 0." - in str(err.value)) - - # Validate should not allow children of different parents. - with pytest.raises(TransformationError) as err: - trans.validate([routine.children[0], - routine.children[3].if_body.children[0]]) - assert ("Error in MaxParTrans transformation: supplied nodes are not " - "children of the same parent" in str(err.value)) - - -def test_apply(fortran_reader): - '''Test the apply function of MaxParallelRegionTrans.''' - code = """ - subroutine test - use some_module - integer :: i, j - i = 1 - j = 1 - call a_function() - if(.true.) then - i = 1 - end if - j = 1 - end subroutine test - """ - psyir = fortran_reader.psyir_from_source(code) - routine = psyir.walk(Routine)[0] - trans = MaxParTrans() - trans.apply(routine) - # The result should be two OMPParallelDirectives, one containing - # i = 1 and j = 1, and another containing the IFBlock and the second j = 1 - dirs = routine.walk(OMPParallelDirective) - assert len(dirs) == 2 - - assert len(dirs[0].dir_body.children) == 2 - assert dirs[0].dir_body.children[0].debug_string() == "i = 1\n" - assert dirs[0].dir_body.children[1].debug_string() == "j = 1\n" - - assert isinstance(dirs[1].dir_body.children[0], IfBlock) - assert dirs[1].dir_body.children[1].debug_string() == "j = 1\n" diff --git a/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py b/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py index 94ca61f8bd..68cb28e58f 100644 --- a/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py @@ -1,7 +1,7 @@ # ----------------------------------------------------------------------------- # BSD 3-Clause License # -# Copyright (c) 2025-2026, Science and Technology Facilities Council. +# Copyright (c) 2017-2025, Science and Technology Facilities Council. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -38,98 +38,120 @@ import pytest from psyclone.psyir.nodes import ( - Assignment, IfBlock, Call, Loop, OMPParallelDirective, Routine + Assignment, + IfBlock, + Routine, + OMPParallelDirective, ) -from psyclone.psyir.transformations.maximal_parallel_region_trans import ( - MaximalParallelRegionTrans -) -from psyclone.psyir.transformations.transformation_error import ( +from psyclone.psyir.transformations import ( + MaximalParallelRegionTrans, TransformationError ) from psyclone.transformations import OMPParallelTrans -class DummyMaxTrans(MaximalParallelRegionTrans): - '''Test class to test the functionality of the MaximalParallelRegionTrans - ''' +# Dummy class to test MaxParallelRegionTrans' functionality. +class MaxParTrans(MaximalParallelRegionTrans): + # The apply function will do OMPParallelTrans around allowed regions. _parallel_transformation = OMPParallelTrans - _allowed_nodes = (Assignment, IfBlock) - _required_nodes = (Assignment) - - -def test_can_be_in_parallel_region(fortran_reader): - '''Test the can_be_in_parallel_region function of the - MaximalParallelRegionTrans.''' - code = """subroutine x - use some_mod - integer :: i, j, k, l, m - - i = 2 - if(k == 3) then - call something() - end if - call something() - do j = k, l - i = 3 - end do - do j = k, l - call something() - end do - end subroutine x""" + # We're only allowing assignment because its straightforward to test with. + _allowed_nodes = (Assignment, ) + # Should parallelise any found region that contains an assignment. + _required_nodes = (Assignment, ) + + +@pytest.mark.parametrize( + "statement,expected", + [ + ("i = 1", True), + ("call a_function()", False), + ("do i = 1, 100\nj = j + 1\nend do", True), + ("do i = 1, 100\ncall a_function()\nend do", False), + ("if (.true.) then\nj=3\nend if", True), + ("if(.true.) then\nj=3\nelse\nj=3\nend if", True), + ("if(.true.) then\ncall a_function()\nelse\nj=3\nendif", False), + ("if(.true.) then\nj=3\nelse\ncall a_function()\nendif", False), + ] +) +def test_can_be_in_parallel_region(fortran_reader, statement, expected): + '''Test the _can_be_in_parallel_region function of + MaxParallelRegionTrans.''' + code = f""" + subroutine test + use some_module + integer :: i, j + {statement} + end subroutine test + """ psyir = fortran_reader.psyir_from_source(code) - mtrans = DummyMaxTrans() - # First case - Assignment is in _allowed_nodes so should be True - assert mtrans._can_be_in_parallel_region(psyir.walk(Assignment)[0]) - # Second case - IfBlock is in _allowed_nodes so should be True - assert mtrans._can_be_in_parallel_region(psyir.walk(IfBlock)[0]) - # Third case - Call is not in _allowed_nodes so should be False - assert not mtrans._can_be_in_parallel_region(psyir.walk(Call)[0]) - # Fourth case - Loop containing only nodes in _allowed_nodes so should be - # true. - assert mtrans._can_be_in_parallel_region(psyir.walk(Loop)[0]) - # Fifth case - Loop containing a node not in _allowed_nodes so should be - # False. - assert not mtrans._can_be_in_parallel_region(psyir.walk(Loop)[1]) + routine = psyir.walk(Routine)[0] + trans = MaxParTrans() + assert trans._can_be_in_parallel_region(routine.children[0]) == expected def test_validate(fortran_reader): - '''Test the validate function of the MaximalParallelRegionTrans.''' - code = """subroutine x - integer :: i, j, k, l, m + '''Test the validate function of MaxParallelRegionTrans.''' + code = """ + subroutine test + integer :: i, j i = 1 - if(i == 3) then - i = 2 + j = 1 + k = i + 1 + if(.true.) then + k = i + j end if - i = 3 - end subroutine x""" + end subroutine test""" psyir = fortran_reader.psyir_from_source(code) - assigns = psyir.walk(Assignment) - mtrans = DummyMaxTrans() - - with pytest.raises(TransformationError) as err: - mtrans.validate([assigns[0], assigns[1]]) - assert ("Error in DummyMaxTrans transformation: supplied nodes are not " - "children of the same parent." in str(err.value)) + routine = psyir.walk(Routine)[0] + trans = MaxParTrans() + # Validate should allow us to give the full children + trans.validate(routine.children) + # Validate should not allow non consecutive children with pytest.raises(TransformationError) as err: - mtrans.validate([assigns[0], assigns[2]]) - assert ("Children are not consecutive children of one parent: " - "child 'i = 3' has position 2, but previous child had position 0." + trans.validate([routine.children[0], routine.children[2]]) + assert ("Children are not consecutive children of one parent: child " + "'k = i + 1' has position 2, but previous child had position 0." in str(err.value)) - -class DummyMaxTrans2(MaximalParallelRegionTrans): - '''Test class to test the functionality of the MaximalParallelRegionTrans - apply method - ''' - _parallel_transformation = OMPParallelTrans - _allowed_nodes = (Assignment) - _required_nodes = (Assignment) + # Validate should not allow children of different parents. + with pytest.raises(TransformationError) as err: + trans.validate([routine.children[0], + routine.children[3].if_body.children[0]]) + assert ("Error in MaxParTrans transformation: supplied nodes are not " + "children of the same parent" in str(err.value)) def test_apply(fortran_reader): - '''Test the apply function of the MaximalParallelRegionTrans.''' - mtrans = DummyMaxTrans2() + '''Test the apply function of MaxParallelRegionTrans.''' + code = """ + subroutine test + use some_module + integer :: i, j + i = 1 + j = 1 + call a_function() + if(.true.) then + i = 1 + end if + j = 1 + end subroutine test + """ + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + mtrans = MaxParTrans() + mtrans.apply(routine) + # The result should be two OMPParallelDirectives, one containing + # i = 1 and j = 1, and another containing the IFBlock and the second j = 1 + dirs = routine.walk(OMPParallelDirective) + assert len(dirs) == 2 + + assert len(dirs[0].dir_body.children) == 2 + assert dirs[0].dir_body.children[0].debug_string() == "i = 1\n" + assert dirs[0].dir_body.children[1].debug_string() == "j = 1\n" + + assert isinstance(dirs[1].dir_body.children[0], IfBlock) + assert dirs[1].dir_body.children[1].debug_string() == "j = 1\n" code = """subroutine x integer :: i, j, k, l From a275f480b8d08a92a73f22ab43a28b2045bd2a25 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Mon, 12 Jan 2026 15:18:29 +0000 Subject: [PATCH 10/16] Update the example --- examples/nemo/eg7/openmp_cpu_nowait_trans.py | 58 ++------------------ 1 file changed, 6 insertions(+), 52 deletions(-) diff --git a/examples/nemo/eg7/openmp_cpu_nowait_trans.py b/examples/nemo/eg7/openmp_cpu_nowait_trans.py index 6889cf43cc..c438c38ab7 100755 --- a/examples/nemo/eg7/openmp_cpu_nowait_trans.py +++ b/examples/nemo/eg7/openmp_cpu_nowait_trans.py @@ -41,68 +41,22 @@ ArrayAssignment2LoopsTrans, OMPLoopTrans, OMPMinimiseSyncTrans, - TransformationError + TransformationError, + OMPMaximalParallelRegionTrans ) -from psyclone.transformations import OMPParallelTrans from psyclone.psyir.nodes import ( Assignment, Directive, - IfBlock, Loop, - OMPBarrierDirective, - OMPDoDirective, Routine, ) -def add_parallel_region_to_contiguous_directives(schedule): - '''Adds OMPParallelDirective nodes around areas of the schedule with - contiguous OpenMP directives. - - :param schedule: The Schedule to add OpenMPParallelDirectives to. - :type schedule: :py:class:`psyclone.nodes.Schedule` - ''' - par_trans = OMPParallelTrans() - start = -1 - end = -1 - sets = [] - # Loop through the children, if its an OpenMP directive add it - # to the current set - for child in schedule: - if isinstance(child, (OMPDoDirective, OMPBarrierDirective)): - if start < 0: - start = child.position - end = child.position + 1 - else: - # If we have a non OMPDodirective/OMPBarrierDirective then add - # an OMPParallelDirective if needed. - if start >= 0: - sets.append((start, end)) - start = -1 - end = -1 - # Recurse appropriately to sub schedules: - if isinstance(child, Loop): - add_parallel_region_to_contiguous_directives(child.loop_body) - elif isinstance(child, IfBlock): - add_parallel_region_to_contiguous_directives(child.if_body) - if child.else_body: - add_parallel_region_to_contiguous_directives( - child.else_body - ) - # If we get to the end and need to enclose some nodes in a parallel - # directive we do it now - if start >= 0: - sets.append((start, end)) - - for subset in sets[::-1]: - par_trans.apply(schedule[subset[0]:subset[1]]) - - def trans(psyir): ''' Adds OpenMP Loop directives with nowait to Nemo loops over levels. - This is followed by applying OpenMP parallel directives as required, - before removing barriers where possible. - + This is followed by applying OpenMP parallel directives as required + with the OMPMaximalParallelRegionTrans, before removing barriers where + possible. :param psyir: the PSyIR of the provided file. :type psyir: :py:class:`psyclone.psyir.nodes.FileContainer` @@ -130,5 +84,5 @@ def trans(psyir): # Apply the largest possible parallel regions and remove any barriers that # can be removed. for routine in psyir.walk(Routine): - add_parallel_region_to_contiguous_directives(routine) + OMPMaximalParallelRegionTrans().apply(routine) minsync_trans.apply(routine) From f711ae98bf630012763b555c8d0d5cd80af4aed8 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Mon, 12 Jan 2026 15:23:28 +0000 Subject: [PATCH 11/16] Missing init update --- src/psyclone/psyir/transformations/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/psyclone/psyir/transformations/__init__.py b/src/psyclone/psyir/transformations/__init__.py index d24e602144..755bfd2152 100644 --- a/src/psyclone/psyir/transformations/__init__.py +++ b/src/psyclone/psyir/transformations/__init__.py @@ -128,6 +128,9 @@ OMPTaskloopTrans from psyclone.psyir.transformations.omp_declare_target_trans import \ OMPDeclareTargetTrans +from psyclone.psyir.transformations.omp_maximal_parallel_region_trans import ( + OMPMaximalParallelRegionTrans, +) # For AutoAPI documentation generation __all__ = [ @@ -180,4 +183,5 @@ "OMPDeclareTargetTrans", "MaximalParallelRegionTrans", "OMPCriticalTrans", + "OMPMaximalParallelRegionTrans", ] From c15d552399fe46f6fdcd753343f89e2cb0200273 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Tue, 27 Jan 2026 15:31:53 +0000 Subject: [PATCH 12/16] First changes towards review --- .../psyir/transformations/__init__.py | 12 ++-- ...y => maximal_omp_parallel_region_trans.py} | 25 ++++++-- ...egion_trans.py => maximal_region_trans.py} | 63 +++++++++---------- ...s_test.py => maximal_region_trans_test.py} | 14 ++--- 4 files changed, 63 insertions(+), 51 deletions(-) rename src/psyclone/psyir/transformations/{omp_maximal_parallel_region_trans.py => maximal_omp_parallel_region_trans.py} (80%) rename src/psyclone/psyir/transformations/{maximal_parallel_region_trans.py => maximal_region_trans.py} (75%) rename src/psyclone/tests/psyir/transformations/{maximal_parallel_region_trans_test.py => maximal_region_trans_test.py} (95%) diff --git a/src/psyclone/psyir/transformations/__init__.py b/src/psyclone/psyir/transformations/__init__.py index 86aa8568ab..a9f81d7961 100644 --- a/src/psyclone/psyir/transformations/__init__.py +++ b/src/psyclone/psyir/transformations/__init__.py @@ -94,8 +94,8 @@ from psyclone.psyir.transformations.loop_trans import LoopTrans from psyclone.psyir.transformations.value_range_check_trans import ( ValueRangeCheckTrans) -from psyclone.psyir.transformations.maximal_parallel_region_trans import ( - MaximalParallelRegionTrans) +from psyclone.psyir.transformations.maximal_region_trans import ( + MaximalRegionTrans) from psyclone.psyir.transformations.omp_critical_trans import ( OMPCriticalTrans) from psyclone.psyir.transformations.omp_loop_trans import OMPLoopTrans @@ -128,8 +128,8 @@ OMPTaskloopTrans from psyclone.psyir.transformations.omp_declare_target_trans import \ OMPDeclareTargetTrans -from psyclone.psyir.transformations.omp_maximal_parallel_region_trans import ( - OMPMaximalParallelRegionTrans +from psyclone.psyir.transformations.maximal_omp_parallel_region_trans import ( + MaximalOMPParallelRegionTrans ) from psyclone.psyir.transformations.omp_parallel_trans import ( OMPParallelTrans, @@ -184,8 +184,8 @@ "ParallelRegionTrans", "OMPTaskloopTrans", "OMPDeclareTargetTrans", - "MaximalParallelRegionTrans", + "MaximalRegionTrans", "OMPCriticalTrans", - "OMPMaximalParallelRegionTrans", + "MaximalOMPParallelRegionTrans", "OMPParallelTrans", ] diff --git a/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/maximal_omp_parallel_region_trans.py similarity index 80% rename from src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py rename to src/psyclone/psyir/transformations/maximal_omp_parallel_region_trans.py index 41093b203b..57a2d3496b 100644 --- a/src/psyclone/psyir/transformations/omp_maximal_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_omp_parallel_region_trans.py @@ -33,7 +33,9 @@ # ----------------------------------------------------------------------------- # Authors A. B. G. Chalk, STFC Daresbury Lab -'''This module contains the OMPMaximalParallelRegionTrans.''' +'''This module contains the MaximalOMPParallelRegionTrans.''' + +from typing import Union from psyclone.psyir.nodes import ( OMPTaskwaitDirective, @@ -44,13 +46,17 @@ OMPLoopDirective, OMPTaskDirective, DynamicOMPTaskDirective, + Node, + Schedule ) -from psyclone.psyir.transformations.maximal_parallel_region_trans import ( - MaximalParallelRegionTrans) +from psyclone.psyir.transformations.maximal_region_trans import ( + MaximalRegionTrans) from psyclone.psyir.transformations.omp_parallel_trans import OMPParallelTrans +from psyclone.utils import transformation_documentation_wrapper -class OMPMaximalParallelRegionTrans(MaximalParallelRegionTrans): +@transformation_documentation_wrapper +class MaximalOMPParallelRegionTrans(MaximalRegionTrans): '''Applies OpenMP Parallel directives around the largest possible sections of the input. @@ -58,8 +64,8 @@ class OMPMaximalParallelRegionTrans(MaximalParallelRegionTrans): Assignments that are outside of another OpenMP directive. See #3157 and the discussion on #3205 for more detail.''' # The type of parallel transformation to be applied to the input region. - _parallel_transformation = OMPParallelTrans - # Tuple of statement nodes allowed inside the _parallel_transformation + _transformation = OMPParallelTrans + # Tuple of statement nodes allowed inside the _transformation _allowed_nodes = ( OMPTaskwaitDirective, OMPBarrierDirective, @@ -78,3 +84,10 @@ class OMPMaximalParallelRegionTrans(MaximalParallelRegionTrans): OMPTaskDirective, DynamicOMPTaskDirective, ) + + def apply(self, nodes: Union[Node, Schedule, list[Node]], **kwargs): + '''Applies the transformation to the nodes provided. + + :param nodes: can be a single node, a schedule or a list of nodes. + ''' + super().apply(self, nodes, **kwargs) diff --git a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py b/src/psyclone/psyir/transformations/maximal_region_trans.py similarity index 75% rename from src/psyclone/psyir/transformations/maximal_parallel_region_trans.py rename to src/psyclone/psyir/transformations/maximal_region_trans.py index 120246b903..ee40f6467d 100644 --- a/src/psyclone/psyir/transformations/maximal_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_region_trans.py @@ -33,7 +33,7 @@ # ----------------------------------------------------------------------------- # Authors A. B. G. Chalk, STFC Daresbury Lab -'''This module contains the MaximalParallelRoutineTrans.''' +'''This module contains the MaximalRegionTrans.''' import abc from typing import Union, List @@ -48,44 +48,43 @@ from psyclone.psyir.transformations.region_trans import RegionTrans from psyclone.psyir.transformations.transformation_error import \ TransformationError +from psyclone.utils import transformation_documentation_wrapper -class MaximalParallelRegionTrans(RegionTrans, metaclass=abc.ABCMeta): +@transformation_documentation_wrapper +class MaximalRegionTrans(RegionTrans, metaclass=abc.ABCMeta): '''Abstract transformation containing the functionality to add - the largest allowed parallel regions to the provided code segment. + the largest allowed transformation to the provided code segment. - Subclasses should override the _parallel_transformation and _allowed_nodes + Subclasses should override the _transformation and _allowed_nodes members to control the functionality. - The _parallel_transformation should be a transformation class to apply to - the computed parallel regions. + The _transformation should be a transformation class to apply to + the computed set of regions. The _allowed_nodes is a tuple of Node classes that are allowed as - statements in the parallel region. Note that upon finding a Loop or + statements in the transformed region. Note that upon finding a Loop or IfBlock, the node's children will be checked to determine whether its safe - to contain the Loop or IfBlock in the parallel section.''' + to contain the Loop or IfBlock in the transformed section.''' - # The type of parallel transformation to be applied to the input region. - _parallel_transformation = None - # Tuple of statement nodes allowed inside the _parallel_transformation + # The type of transformation to be applied to the input region. + _transformation = None + # Tuple of statement nodes allowed inside the _transformation _allowed_nodes = () # Tuple of nodes that there must be at least one of inside the block - # to be parallelised, else the block can be ignored (e.g. a block of - # only barriers doesn't need to be parallelised). + # to be transformed, else the block can be ignored (e.g. a block of + # only barriers doesn't need to be transformed). _required_nodes = () - def _can_be_in_parallel_region(self, node: Node) -> bool: - '''Returns whether the provided node can be included in an - OpenMP parallel region. Most OpenMP directives can be included, - and loops and if statements are recursed into to check if their + def _can_be_in_region(self, node: Node) -> bool: + '''Returns whether the provided node can be included in a + region. Loops and if statements are recursed into to check if their children can be. - Other statements are currently not added to an OpenMP parallel region - by this transformation. + :param node: the candidate Node to be placed into a transformed + region. - :param node: the candidate Node to be placed into a parallel region. - - :returns: whether it is safe to add the node to the parallel region. + :returns: whether it is safe to add the node to a transformed region. ''' if isinstance(node, self._allowed_nodes): @@ -94,7 +93,7 @@ def _can_be_in_parallel_region(self, node: Node) -> bool: if isinstance(node, (Loop, WhileLoop)): # Recurse through the loop body. for child in node.loop_body: - if not self._can_be_in_parallel_region(child): + if not self._can_be_in_region(child): break else: return True @@ -104,11 +103,11 @@ def _can_be_in_parallel_region(self, node: Node) -> bool: # Recurse through the if_body and else_body allowed = True for child in node.if_body: - allowed = (allowed and self._can_be_in_parallel_region(child)) + allowed = (allowed and self._can_be_in_region(child)) if node.else_body and allowed: for child in node.else_body: allowed = (allowed and - self._can_be_in_parallel_region(child)) + self._can_be_in_region(child)) return allowed # All other node types we default to False. @@ -142,7 +141,7 @@ def validate(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): f"{prev_position}.") prev_position = child.position - def apply(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): + def apply(self, nodes: Union[Node, Schedule, list[Node]], **kwargs): '''Applies the transformation to the nodes provided. :param nodes: can be a single node, a schedule or a list of nodes. @@ -152,18 +151,18 @@ def apply(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): # Call validate. self.validate(nodes, **kwargs) - par_trans = self._parallel_transformation() + par_trans = self._transformation() - # Find the largest sections we can surround with parallel regions. + # Find the largest sections we can surround with the transformation. current_block = [] for child in node_list: - # If the child can be added to a parallel region then add it + # If the child can be added to a transformed region then add it # to the current block of nodes. - if self._can_be_in_parallel_region(child): + if self._can_be_in_region(child): current_block.append(child) else: # Otherwise, if the current_block contains any children, - # add them to a parallel region if we should and reset + # add them to a transformed region if we should and reset # the current_block. if current_block: for node in current_block: @@ -182,7 +181,7 @@ def apply(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): if isinstance(child, WhileLoop): self.apply(child.loop_body) # If any nodes are left in the current block at the end of the - # node_list, then add them to a parallel region + # node_list, then add them to a transformed region if current_block: for node in current_block: if node.walk(self._required_nodes, diff --git a/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py b/src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py similarity index 95% rename from src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py rename to src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py index 09ec2b850c..20dcaf7bc7 100644 --- a/src/psyclone/tests/psyir/transformations/maximal_parallel_region_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py @@ -33,7 +33,7 @@ # ----------------------------------------------------------------------------- # Authors A. B. G. Chalk, STFC Daresbury Lab -'''This module contains the tests for the MaximalParallelRegionTrans.''' +'''This module contains the tests for the MaximalRegionTrans.''' import pytest @@ -44,16 +44,16 @@ OMPParallelDirective, ) from psyclone.psyir.transformations import ( - MaximalParallelRegionTrans, + MaximalRegionTrans, TransformationError, OMPParallelTrans ) # Dummy class to test MaxParallelRegionTrans' functionality. -class MaxParTrans(MaximalParallelRegionTrans): +class MaxParTrans(MaximalRegionTrans): # The apply function will do OMPParallelTrans around allowed regions. - _parallel_transformation = OMPParallelTrans + _transformation = OMPParallelTrans # We're only allowing assignment because its straightforward to test with. _allowed_nodes = (Assignment, ) # Should parallelise any found region that contains an assignment. @@ -73,8 +73,8 @@ class MaxParTrans(MaximalParallelRegionTrans): ("if(.true.) then\nj=3\nelse\ncall a_function()\nendif", False), ] ) -def test_can_be_in_parallel_region(fortran_reader, statement, expected): - '''Test the _can_be_in_parallel_region function of +def test_can_be_in_region(fortran_reader, statement, expected): + '''Test the _can_be_in_region function of MaxParallelRegionTrans.''' code = f""" subroutine test @@ -86,7 +86,7 @@ def test_can_be_in_parallel_region(fortran_reader, statement, expected): psyir = fortran_reader.psyir_from_source(code) routine = psyir.walk(Routine)[0] trans = MaxParTrans() - assert trans._can_be_in_parallel_region(routine.children[0]) == expected + assert trans._can_be_in_region(routine.children[0]) == expected def test_validate(fortran_reader): From 321893c036c0151b317ed2585ccfb8abd1f51dc7 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Thu, 29 Jan 2026 15:47:28 +0000 Subject: [PATCH 13/16] Changes for review --- examples/nemo/eg7/openmp_cpu_nowait_trans.py | 4 +- examples/nemo/scripts/utils.py | 60 +------- .../maximal_omp_parallel_region_trans.py | 2 +- .../transformations/maximal_region_trans.py | 134 +++++++++++++----- 4 files changed, 106 insertions(+), 94 deletions(-) diff --git a/examples/nemo/eg7/openmp_cpu_nowait_trans.py b/examples/nemo/eg7/openmp_cpu_nowait_trans.py index f79459e410..aefe2311fb 100755 --- a/examples/nemo/eg7/openmp_cpu_nowait_trans.py +++ b/examples/nemo/eg7/openmp_cpu_nowait_trans.py @@ -42,7 +42,7 @@ OMPLoopTrans, OMPMinimiseSyncTrans, TransformationError, - OMPMaximalParallelRegionTrans + MaximalOMPParallelRegionTrans ) from psyclone.psyir.nodes import ( Assignment, @@ -84,5 +84,5 @@ def trans(psyir): # Apply the largest possible parallel regions and remove any barriers that # can be removed. for routine in psyir.walk(Routine): - OMPMaximalParallelRegionTrans().apply(routine) + MaximalOMPParallelRegionTrans().apply(routine) minsync_trans.apply(routine) diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index a7c703c66b..6d2d139bc6 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -474,6 +474,11 @@ def add_profiling(children: Union[List[Node], Schedule]): attempt to add profiling regions. ''' + class MaximalProfilingTrans(MaximalRegionTrans): + '''Applies Profiling to the largest possible region.''' + _allowed_nodes = [Assignment, Call, CodeBlock] + _transformation = ProfileTrans + if children and isinstance(children, Schedule): # If we are given a Schedule, we look at its children. children = children.children @@ -486,56 +491,5 @@ def add_profiling(children: Union[List[Node], Schedule]): parent_routine = children[0].ancestor(Routine) if parent_routine and parent_routine.return_symbol: return - - node_list = [] - for child in children[:]: - # Do we want this node to be included in a profiling region? - if child.walk((Directive, Return)): - # It contains a directive or return statement so we put what we - # have so far inside a profiling region. - add_profile_region(node_list) - # A node that is not included in a profiling region marks the - # end of the current candidate region so reset the list. - node_list = [] - # Now we go down a level and try again without attempting to put - # profiling below directives or within Assignments - if isinstance(child, IfBlock): - add_profiling(child.if_body) - add_profiling(child.else_body) - elif not isinstance(child, (Assignment, Directive)): - add_profiling(child.children) - else: - # We can add this node to our list for the current region - node_list.append(child) - add_profile_region(node_list) - - -def add_profile_region(nodes): - ''' - Attempt to put the supplied list of nodes within a profiling region. - - :param nodes: list of sibling PSyIR nodes to enclose. - :type nodes: list of :py:class:`psyclone.psyir.nodes.Node` - - ''' - if nodes: - # Check whether we should be adding profiling inside this routine - routine_name = nodes[0].ancestor(Routine).name.lower() - if any(ignore in routine_name for ignore in PROFILING_IGNORE): - return - if len(nodes) == 1: - if isinstance(nodes[0], CodeBlock) and \ - len(nodes[0].get_ast_nodes) == 1: - # Don't create profiling regions for CodeBlocks consisting - # of a single statement - return - if isinstance(nodes[0], IfBlock) and \ - "was_single_stmt" in nodes[0].annotations and \ - isinstance(nodes[0].if_body[0], CodeBlock): - # We also don't put single statements consisting of - # 'IF(condition) CALL blah()' inside profiling regions - return - try: - ProfileTrans().apply(nodes) - except TransformationError: - pass + + MaximalProfilingTrans.apply(children) diff --git a/src/psyclone/psyir/transformations/maximal_omp_parallel_region_trans.py b/src/psyclone/psyir/transformations/maximal_omp_parallel_region_trans.py index 57a2d3496b..d16f2b8ed6 100644 --- a/src/psyclone/psyir/transformations/maximal_omp_parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_omp_parallel_region_trans.py @@ -90,4 +90,4 @@ def apply(self, nodes: Union[Node, Schedule, list[Node]], **kwargs): :param nodes: can be a single node, a schedule or a list of nodes. ''' - super().apply(self, nodes, **kwargs) + super().apply(nodes, **kwargs) diff --git a/src/psyclone/psyir/transformations/maximal_region_trans.py b/src/psyclone/psyir/transformations/maximal_region_trans.py index ee40f6467d..fc4ea59fab 100644 --- a/src/psyclone/psyir/transformations/maximal_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_region_trans.py @@ -73,8 +73,8 @@ class MaximalRegionTrans(RegionTrans, metaclass=abc.ABCMeta): _allowed_nodes = () # Tuple of nodes that there must be at least one of inside the block # to be transformed, else the block can be ignored (e.g. a block of - # only barriers doesn't need to be transformed). - _required_nodes = () + # only barriers doesn't need to be transformed). Defaults to any Node. + _required_nodes = (Node) def _can_be_in_region(self, node: Node) -> bool: '''Returns whether the provided node can be included in a @@ -113,7 +113,83 @@ def _can_be_in_region(self, node: Node) -> bool: # All other node types we default to False. return False - def validate(self, nodes: Union[Node, Schedule, List[Node]], **kwargs): + def _compute_transformable_sections( + self, node_list: list[Node] + ) -> list[list[Node]]: + ''' + Computes the sections of the input node_list to apply the + transformation to. + + :param node_list: The node_list passed into this Transformation. + :returns: The list of node_lists to apply this class' + _transformation class to. + ''' + # Find the largest sections we can surround with the transformation. + all_blocks = [] + current_block = [] + for child in node_list: + # If the child can be added to a transformed region then add it + # to the current block of nodes. + if self._can_be_in_region(child): + current_block.append(child) + else: + # Otherwise, if the current_block contains any children, + # add them to the list of regions to be transformed and reset + # the current_block. + if current_block: + for node in current_block: + if node.walk(self._required_nodes, + stop_type=self._required_nodes): + all_blocks.append(current_block) + break + current_block = [] + # Need to recurse on some node types + if isinstance(child, IfBlock): + if_blocks = self._compute_transformable_sections( + child.if_body + ) + all_blocks.extend(if_blocks) + if child.else_body: + else_blocks = self._compute_transformable_sections( + child.else_body + ) + all_blocks.extend(else_blocks) + if isinstance(child, (Loop, WhileLoop)): + loop_blocks = self._compute_transformable_sections( + child.loop_body + ) + all_blocks.extend(loop_blocks) + # If any nodes are left in the current block at the end of the + # node_list, then add them to a transformed region + if current_block: + for node in current_block: + if node.walk(self._required_nodes, + stop_type=self._required_nodes): + all_blocks.append(current_block) + break + + return all_blocks + + + def _handle_invalid_block(self, err: TransformationError, + block: list[Node], + all_blocks: list[list[Node]]): + ''' + Function to handle what happens when a discovered block fails + validation for the relevant transformation. Children classes + are free to implement their own version of this routine. The + default implementation removes the block from the list of blocks + and continues. + + :param err: The TransformationError raised by the transformation's + validate function. + :param block: The block that failed validation. + :param all_blocks: The list of all of the blocks found during + application of this transformation. + ''' + all_blocks.remove(block) + + def validate(self, nodes: Union[Node, Schedule, list[Node]], **kwargs): '''Validates whether this transformation can be applied to the nodes provided. @@ -153,38 +229,20 @@ def apply(self, nodes: Union[Node, Schedule, list[Node]], **kwargs): par_trans = self._transformation() - # Find the largest sections we can surround with the transformation. - current_block = [] - for child in node_list: - # If the child can be added to a transformed region then add it - # to the current block of nodes. - if self._can_be_in_region(child): - current_block.append(child) - else: - # Otherwise, if the current_block contains any children, - # add them to a transformed region if we should and reset - # the current_block. - if current_block: - for node in current_block: - if node.walk(self._required_nodes, - stop_type=self._required_nodes): - par_trans.apply(current_block) - break - current_block = [] - # Need to recurse on some node types - if isinstance(child, IfBlock): - self.apply(child.if_body) - if child.else_body: - self.apply(child.else_body) - if isinstance(child, Loop): - self.apply(child.loop_body) - if isinstance(child, WhileLoop): - self.apply(child.loop_body) - # If any nodes are left in the current block at the end of the - # node_list, then add them to a transformed region - if current_block: - for node in current_block: - if node.walk(self._required_nodes, - stop_type=self._required_nodes): - par_trans.apply(current_block) - break + all_blocks = self._compute_transformable_sections(node_list) + + # Check that the transformation can be applied to all of the found + # blocks. + for block in all_blocks: + try: + par_trans.validate(block) + except TransformationError as err: + # Perform class specific behaviour for a block that fails + # transformation validation. + self._handle_invalid_block(err, block, all_blocks) + + # Apply the transformation to all of the blocks found. + for block in all_blocks: + par_trans.apply(block) + + From 44c7a94f0c9d8b500c72d75a626aee3444cb529d Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Thu, 29 Jan 2026 15:48:09 +0000 Subject: [PATCH 14/16] linting --- src/psyclone/psyir/transformations/maximal_region_trans.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/psyclone/psyir/transformations/maximal_region_trans.py b/src/psyclone/psyir/transformations/maximal_region_trans.py index fc4ea59fab..e29ccaee3d 100644 --- a/src/psyclone/psyir/transformations/maximal_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_region_trans.py @@ -36,7 +36,7 @@ '''This module contains the MaximalRegionTrans.''' import abc -from typing import Union, List +from typing import Union from psyclone.psyir.nodes import ( Node, @@ -170,7 +170,6 @@ def _compute_transformable_sections( return all_blocks - def _handle_invalid_block(self, err: TransformationError, block: list[Node], all_blocks: list[list[Node]]): @@ -244,5 +243,3 @@ def apply(self, nodes: Union[Node, Schedule, list[Node]], **kwargs): # Apply the transformation to all of the blocks found. for block in all_blocks: par_trans.apply(block) - - From e5214367e033aa694b2695e71b64f2150937194a Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Thu, 29 Jan 2026 15:59:05 +0000 Subject: [PATCH 15/16] linting --- examples/nemo/scripts/utils.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index 6d2d139bc6..1a1008a1ca 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -40,14 +40,14 @@ from psyclone.domain.common.transformations import KernelModuleInlineTrans from psyclone.psyir.nodes import ( - Assignment, Loop, Directive, Node, Reference, CodeBlock, Call, Return, - IfBlock, Routine, Schedule, IntrinsicCall, StructureReference) + Assignment, Loop, Directive, Node, Reference, CodeBlock, Call, + Routine, Schedule, IntrinsicCall, StructureReference) from psyclone.psyir.symbols import DataSymbol from psyclone.psyir.transformations import ( ArrayAssignment2LoopsTrans, HoistLoopBoundExprTrans, HoistLocalArraysTrans, HoistTrans, InlineTrans, Maxval2LoopTrans, ProfileTrans, OMPMinimiseSyncTrans, Reference2ArrayRangeTrans, - ScalarisationTrans, IncreaseRankLoopArraysTrans) + ScalarisationTrans, IncreaseRankLoopArraysTrans, MaximalRegionTrans) from psyclone.transformations import TransformationError # USE statements to chase to gather additional symbol information. @@ -491,5 +491,5 @@ class MaximalProfilingTrans(MaximalRegionTrans): parent_routine = children[0].ancestor(Routine) if parent_routine and parent_routine.return_symbol: return - + MaximalProfilingTrans.apply(children) From 3002bd6493ed0e843b15a5bd5e85eed681e7f545 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Fri, 30 Jan 2026 11:59:20 +0000 Subject: [PATCH 16/16] Changes for coverage --- .../transformations/maximal_region_trans.py | 2 +- .../maximal_ompparallel_region_trans_test.py | 54 ++++++++++++++++++ .../maximal_region_trans_test.py | 56 ++++++++++++++++++- 3 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 src/psyclone/tests/psyir/transformations/maximal_ompparallel_region_trans_test.py diff --git a/src/psyclone/psyir/transformations/maximal_region_trans.py b/src/psyclone/psyir/transformations/maximal_region_trans.py index e29ccaee3d..04363c13a8 100644 --- a/src/psyclone/psyir/transformations/maximal_region_trans.py +++ b/src/psyclone/psyir/transformations/maximal_region_trans.py @@ -232,7 +232,7 @@ def apply(self, nodes: Union[Node, Schedule, list[Node]], **kwargs): # Check that the transformation can be applied to all of the found # blocks. - for block in all_blocks: + for block in all_blocks[:]: try: par_trans.validate(block) except TransformationError as err: diff --git a/src/psyclone/tests/psyir/transformations/maximal_ompparallel_region_trans_test.py b/src/psyclone/tests/psyir/transformations/maximal_ompparallel_region_trans_test.py new file mode 100644 index 0000000000..a8ad9d88c8 --- /dev/null +++ b/src/psyclone/tests/psyir/transformations/maximal_ompparallel_region_trans_test.py @@ -0,0 +1,54 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2026, Science and Technology Facilities Council. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# ----------------------------------------------------------------------------- +# Authors A. B. G. Chalk, STFC Daresbury Lab + +'''This module contains the tests for the MaximalOMPParallelRegionTrans.''' + +from psyclone.psyir.nodes import ( + OMPParallelDirective, +) +from psyclone.psyir.transformations import ( + MaximalOMPParallelRegionTrans, +) + + +def test_maximal_ompparallel_region_trans_apply(fortran_reader): + ''' Test the apply method of the ompparallel region transformation.''' + code = """subroutine x + integer :: i + i = 1 + end subroutine x""" + psyir = fortran_reader.psyir_from_source(code) + MaximalOMPParallelRegionTrans().apply(psyir.children[0].children[:]) + assert len(psyir.walk(OMPParallelDirective)) == 0 diff --git a/src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py b/src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py index 20dcaf7bc7..dbe18f691c 100644 --- a/src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/maximal_region_trans_test.py @@ -1,7 +1,7 @@ # ----------------------------------------------------------------------------- # BSD 3-Clause License # -# Copyright (c) 2017-2025, Science and Technology Facilities Council. +# Copyright (c) 2026, Science and Technology Facilities Council. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -37,6 +37,7 @@ import pytest +from psyclone.psyGen import Transformation from psyclone.psyir.nodes import ( Assignment, IfBlock, @@ -244,3 +245,56 @@ def test_apply(fortran_reader): assert len(nodes[0].if_body.children) == 2 assert isinstance(nodes[0].if_body.children[1], OMPParallelDirective) assert isinstance(nodes[0].else_body.children[0], OMPParallelDirective) + + # Dummy class to test failing validation. + class Faketrans(Transformation): + '''Dummy transformation to test failing validation.''' + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._validate_count = 0 + + def validate(self, node, **kwargs): + if self._validate_count < 1: + self._validate_count = self._validate_count + 1 + return + raise TransformationError("") + + def apply(self, node, **kwargs): + OMPParallelTrans().apply(node, **kwargs) + + class OneParTrans(MaximalRegionTrans): + '''Dummy MaximalRegionTrans that uses our FakeTrans''' + _transformation = Faketrans + _allowed_nodes = (Assignment, ) + _required_nodes = (Assignment, ) + + code = """subroutine x + use some_mod + integer :: i, j + + if(i == 1) then + call something() + i = 2 + else + i = 3 + end if + + do i = 1,5 + call something() + j = 2 + end do + + do while(j == 3) + call something() + j = j + 2 + end do + end subroutine x""" + # Each of the nodes should contain OMPParallels inside them and there + # should be no top level OMPParallelDirective + psyir = fortran_reader.psyir_from_source(code) + nodes = psyir.walk(Routine)[0].children[:] + mtrans = OneParTrans() + mtrans.apply(nodes) + # Validate fails on all but the first try so we only get one resulting + # OMPParallelDirective + assert len(psyir.walk(OMPParallelDirective)) == 1