From 628fb7e3b85b1abb5e5e5541a1cdb0f2efa061b7 Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Thu, 23 Apr 2026 14:09:18 +0100 Subject: [PATCH 1/3] Finished non-PsyDataTrans transformation option deprecation --- .../transformations/omp_parallel_trans.py | 24 ++++--- .../transformations/omp_taskloop_trans.py | 31 +++++---- .../transformations/omp_taskwait_trans.py | 67 ++++++++++--------- .../transformations/parallel_region_trans.py | 34 +++++----- .../replace_induction_variables_trans.py | 19 +++--- .../replace_reference_by_literal_trans.py | 9 ++- .../transformations/scalarisation_trans.py | 20 ++++-- 7 files changed, 119 insertions(+), 85 deletions(-) diff --git a/src/psyclone/psyir/transformations/omp_parallel_trans.py b/src/psyclone/psyir/transformations/omp_parallel_trans.py index 91a43f5386..c6fe3843d7 100644 --- a/src/psyclone/psyir/transformations/omp_parallel_trans.py +++ b/src/psyclone/psyir/transformations/omp_parallel_trans.py @@ -53,8 +53,10 @@ ParallelRegionTrans) from psyclone.psyir.transformations.transformation_error import ( TransformationError) +from psyclone.utils import transformation_documentation_wrapper +@transformation_documentation_wrapper class OMPParallelTrans(ParallelRegionTrans): ''' Create an OpenMP PARALLEL region by inserting directives. For @@ -108,28 +110,34 @@ def name(self) -> str: ''' return "OMPParallelTrans" - def validate(self, node_list: list[Node], options=None): + def validate(self, nodes: list[Node], options=None, **kwargs): ''' Perform OpenMP-specific validation checks. - :param node_list: list of Nodes to put within parallel region. - :type node_list: list of :py:class:`psyclone.psyir.nodes.Node` + :param nodes: list of Nodes to put within parallel region. :param options: a dictionary with options for transformations. :type options: Optional[Dict[str, Any]] - :param bool options["node-type-check"]: this flag controls if the \ - type of the nodes enclosed in the region should be tested \ - to avoid using unsupported nodes inside a region. :raises TransformationError: if the target Nodes are already within \ some OMP parallel region. ''' - if node_list[0].ancestor(OMPDirective): + if nodes[0].ancestor(OMPDirective): raise TransformationError("Error in OMPParallel transformation:" + " cannot create an OpenMP PARALLEL " + "region within another OpenMP region.") # Now call the general validation checks - super().validate(node_list, options) + # TODO #2668: Remove options. + super().validate(nodes, options, **kwargs) + + def apply(self, nodes: list[Node], options=None, **kwargs): + ''' + Surrounds the provided node list with an OpenMP Parallel region. + + :param nodes: list of Nodes to put within parallel region. + ''' + # TODO #2668: Remove options. + super().apply(nodes, options, **kwargs) __all__ = ["OMPParallelTrans"] diff --git a/src/psyclone/psyir/transformations/omp_taskloop_trans.py b/src/psyclone/psyir/transformations/omp_taskloop_trans.py index 05ff94cc4d..05bd2e3b20 100644 --- a/src/psyclone/psyir/transformations/omp_taskloop_trans.py +++ b/src/psyclone/psyir/transformations/omp_taskloop_trans.py @@ -18,7 +18,7 @@ This module provides the implementation of OMPTaskloopTrans ''' - +from typing import Union from psyclone.psyir.transformations.parallel_loop_trans import ( ParallelLoopTrans) @@ -27,8 +27,10 @@ from psyclone.psyir.nodes import ( OMPTaskloopDirective) +from psyclone.utils import transformation_documentation_wrapper +@transformation_documentation_wrapper class OMPTaskloopTrans(ParallelLoopTrans): ''' Adds an OpenMP taskloop directive to a loop. Only one of grainsize or @@ -230,7 +232,8 @@ def _directive(self, children, collapse=None): nogroup=self.omp_nogroup) return _directive - def apply(self, node, options=None, **kwargs): + def apply(self, node, options=None, nogroup: Union[bool, None] = None, + **kwargs): '''Apply the OMPTaskloopTrans transformation to the specified node in a Schedule. This node must be a Loop since this transformation corresponds to wrapping the generated code with directives like so: @@ -246,32 +249,36 @@ def apply(self, node, options=None, **kwargs): At code-generation time (when lowering is called), this node must be within (i.e. a child of) an OpenMP SERIAL region. - If the keyword "nogroup" is specified in the options, it will cause a - nogroup clause be generated if it is set to True. This will override + If the nogroup option is True, it will cause a + nogroup clause be generated. This will override the value supplied to the constructor, but will only apply to the apply call to which the value is supplied. - :param node: the supplied node to which we will apply the \ + :param node: the supplied node to which we will apply the OMPTaskloopTrans transformation :type node: :py:class:`psyclone.psyir.nodes.Node` - :param options: a dictionary with options for transformations\ + :param options: a dictionary with options for transformations and validation. :type options: Optional[Dict[str, Any]] - :param bool options["nogroup"]: + :param nogroup: indicating whether a nogroup clause should be applied to - this taskloop. + this taskloop. Defaults to None, which means to use the + option defined by the transformations omp_nogroup member. ''' - if not options: - options = {} current_nogroup = self.omp_nogroup # If nogroup is specified it overrides that supplied to the # constructor of the Transformation, but will be reset at the # end of this function - self.omp_nogroup = options.get("nogroup", current_nogroup) + # TODO #2668: Remove options + if not options: + if nogroup is not None: + self.omp_nogroup = nogroup + else: + self.omp_nogroup = options.get("nogroup", current_nogroup) try: - super().apply(node, options, **kwargs) + super().apply(node, options, nogroup=self.omp_nogroup, **kwargs) finally: # Reset the nogroup value to the original value self.omp_nogroup = current_nogroup diff --git a/src/psyclone/psyir/transformations/omp_taskwait_trans.py b/src/psyclone/psyir/transformations/omp_taskwait_trans.py index 5a7bca14b7..13d2858693 100644 --- a/src/psyclone/psyir/transformations/omp_taskwait_trans.py +++ b/src/psyclone/psyir/transformations/omp_taskwait_trans.py @@ -38,17 +38,24 @@ applied to an OMPParallelDirective to satisfy any task-based dependencies created by OpenMP Taskloops.''' +import warnings + from psyclone.core import VariablesAccessMap from psyclone.errors import LazyString, InternalError from psyclone.psyGen import Transformation from psyclone.psyir import nodes -from psyclone.psyir.nodes import Loop, Schedule, \ - OMPDoDirective, OMPTaskloopDirective, OMPSerialDirective, \ +from psyclone.psyir.nodes import ( + Loop, Schedule, + OMPDoDirective, OMPTaskloopDirective, OMPSerialDirective, OMPTaskwaitDirective, OMPSingleDirective, OMPParallelDirective -from psyclone.psyir.transformations.transformation_error import \ - TransformationError +) +from psyclone.psyir.transformations.transformation_error import ( + TransformationError +) +from psyclone.utils import transformation_documentation_wrapper +@transformation_documentation_wrapper class OMPTaskwaitTrans(Transformation): ''' Adds zero or more OpenMP Taskwait directives to an OMP parallel region. @@ -99,31 +106,29 @@ def __str__(self): "region to satisfy 'OpenMP TASKLOOP' dependencies") return rval - def validate(self, node, options=None): + def validate(self, node: OMPParallelDirective, options=None, **kwargs): ''' Validity checks for input arguments. :param node: the OMPParallelDirective node to validate. - :type node: :py:class:`psyclone.psyir.nodes.OMPParallelDirective` :param options: a dictionary with options for transformations. :type options: Optional[Dict[str, Any]] - :param bool options["fail_on_no_taskloop"]: - indicating whether this should throw an error if no \ - OMPTaskloop nodes are found in this tree. This can be \ - safely disabled as if there are no Taskloop nodes the \ - result of this transformation is valid OpenMP code. Default \ - is True. - - :raises TransformationError: If the supplied node is not an \ + + :raises TransformationError: If the supplied node is not an OMPParallelDirective - :raises TransformationError: If there are no OMPTaskloopDirective \ + :raises TransformationError: If there are no OMPTaskloopDirective nodes in this tree. - :raises TransformationError: If taskloop dependencies can't be \ - satisfied due to dependencies across \ + :raises TransformationError: If taskloop dependencies can't be + satisfied due to dependencies across barrierless OpenMP Serial Regions. ''' - fail_on_no_taskloop = True - if options is not None: + # TODO #2668: Remove options dict. + if options is None: + self.validate_options(**kwargs) + fail_on_no_taskloop = self.get_option("fail_on_no_taskloop", + **kwargs) + else: + warnings.warn(self._deprecation_warning, DeprecationWarning, 2) fail_on_no_taskloop = options.get("fail_on_no_taskloop", True) # Check the supplied node is an OMPParallelDirective if not isinstance(node, nodes.OMPParallelDirective): @@ -315,7 +320,7 @@ def _eliminate_unneeded_dependencies(taskloop_positions, :type taskloop_positions: list of int :param dependence_positions: positions of the taskloops' dependencies. :type dependence_positions: list of int - :param dependence_nodes: the nodes representing the forward \ + :param dependence_nodes: the nodes representing the forward dependency of each taskloop node. :type dependence_nodes: list of :py:class:`psyclone.psyir.nodes.Node` @@ -366,7 +371,8 @@ def _eliminate_unneeded_dependencies(taskloop_positions, break return dependence_positions, dependence_nodes - def apply(self, node, options=None): + def apply(self, node: OMPParallelDirective, options=None, + fail_on_no_taskloop: bool = True, **kwargs): ''' Apply an OMPTaskwait Transformation to the supplied node (which must be an OMPParallelDirective). In the generated code this @@ -384,19 +390,19 @@ def apply(self, node, options=None): !$OMP END PARALLEL :param node: the node to which to apply the transformation. - :type node: :py:class:`psyclone.psyir.nodes.OMPParallelDirective` - :param options: a dictionary with options for transformations\ + :param options: a dictionary with options for transformations and validation. :type options: Optional[Dict[str, Any]] - :param bool options["fail_on_no_taskloop"]: - indicating whether this should throw an error if no \ - OMPTaskloop nodes are found in this tree. This can be \ - safely disabled as if there are no Taskloop nodes the \ - result of this transformation is valid OpenMP code. Default \ + :param fail_on_no_taskloop: + indicating whether this should throw an error if no + OMPTaskloop nodes are found in this tree. This can be + safely disabled as if there are no Taskloop nodes the + result of this transformation is valid OpenMP code. Default is True ''' - self.validate(node, options=options) + self.validate(node, options=options, + fail_on_no_taskloop=fail_on_no_taskloop, **kwargs) # Find all the OpenMP Single & Master regions task_regions = node.walk(OMPSerialDirective) @@ -453,10 +459,11 @@ def apply(self, node, options=None): dependence_position[i] = forward_dep.abs_position dependence_node[i] = forward_dep # Forward dependency positions are now computed for this region. - dependence_position, dependence_node = \ + dependence_position, dependence_node = ( OMPTaskwaitTrans._eliminate_unneeded_dependencies( taskloop_positions, dependence_position, dependence_node) + ) # dependence_position now contains only the required dependencies # to satisfy the full superset of dependencies. We can loop over # these by index, and if dependence_position[i] is not None then diff --git a/src/psyclone/psyir/transformations/parallel_region_trans.py b/src/psyclone/psyir/transformations/parallel_region_trans.py index 2fa1ccc1d5..48b78482e5 100644 --- a/src/psyclone/psyir/transformations/parallel_region_trans.py +++ b/src/psyclone/psyir/transformations/parallel_region_trans.py @@ -49,9 +49,11 @@ TransformationError) from psyclone import psyGen from psyclone.psyir.transformations.region_trans import RegionTrans -from psyclone.psyir.nodes import CodeBlock, Return +from psyclone.psyir.nodes import CodeBlock, Node, Return +from psyclone.utils import transformation_documentation_wrapper +@transformation_documentation_wrapper class ParallelRegionTrans(RegionTrans, ABC): ''' Base class for transformations that create a parallel region. @@ -74,24 +76,20 @@ def __str__(self) -> str: ''' - def validate(self, node_list, options=None): + def validate(self, nodes: list[Node], options=None, **kwargs): # pylint: disable=arguments-renamed ''' Check that the supplied list of Nodes are eligible to be put inside a parallel region. - :param list node_list: list of nodes to put into a parallel region - :param options: a dictionary with options for transformations.\ - :type options: Optional[Dict[str, Any]] - :param bool options["node-type-check"]: this flag controls whether \ - or not the type of the nodes enclosed in the region should be \ - tested to avoid using unsupported nodes inside a region. + :param list nodes: list of nodes to put into a parallel region + :param options: a dictionary with options for transformations. - :raises TransformationError: if the supplied nodes are not all \ + :raises TransformationError: if the supplied nodes are not all children of the same parent (siblings). ''' - node_list = self.get_node_list(node_list) + node_list = self.get_node_list(nodes) node_parent = node_list[0].parent @@ -100,30 +98,28 @@ def validate(self, node_list, options=None): raise TransformationError( f"Error in {self.name} transformation: supplied nodes are " f"not children of the same parent.") - super().validate(node_list, options) + # TODO #2668: Remove options. + super().validate(node_list, options, **kwargs) - def apply(self, target_nodes, options=None): + def apply(self, nodes: list[Node], options=None, **kwargs): # pylint: disable=arguments-renamed ''' Apply this transformation to a subset of the nodes within a schedule - i.e. enclose the specified Loops in the schedule within a single parallel region. - :param target_nodes: a single Node or a list of Nodes. - :type target_nodes: (list of) :py:class:`psyclone.psyir.nodes.Node` + :param nodes: a single Node or a list of Nodes. :param options: a dictionary with options for transformations. :type options: Optional[Dict[str, Any]] - :param bool options["node-type-check"]: this flag controls if the \ - type of the nodes enclosed in the region should be tested \ - to avoid using unsupported nodes inside a region. ''' # Check whether we've been passed a list of nodes or just a # single node. If the latter then we create ourselves a # list containing just that node. - node_list = self.get_node_list(target_nodes) - self.validate(node_list, options) + node_list = self.get_node_list(nodes) + # TODO #2668: Remove options. + self.validate(node_list, options, **kwargs) # Keep a reference to the parent of the nodes that are to be # enclosed within a parallel region. Also keep the index of diff --git a/src/psyclone/psyir/transformations/replace_induction_variables_trans.py b/src/psyclone/psyir/transformations/replace_induction_variables_trans.py index 4e10c9d22e..fd4b3f86b3 100644 --- a/src/psyclone/psyir/transformations/replace_induction_variables_trans.py +++ b/src/psyclone/psyir/transformations/replace_induction_variables_trans.py @@ -41,10 +41,13 @@ from psyclone.psyGen import Transformation from psyclone.psyir.nodes import (ArrayReference, Assignment, BinaryOperation, Call, CodeBlock, Loop, Reference) -from psyclone.psyir.transformations.transformation_error \ - import TransformationError +from psyclone.psyir.transformations.transformation_error import ( + TransformationError +) +from psyclone.utils import transformation_documentation_wrapper +@transformation_documentation_wrapper class ReplaceInductionVariablesTrans(Transformation): '''Move all supported induction variables out of the loop, and replace their usage inside the loop. For example: @@ -197,17 +200,17 @@ def _is_induction_variable(assignment, accesses_in_loop_body): return True # ------------------------------------------------------------------------ - def apply(self, node, options=None): + def apply(self, node, options=None, **kwargs): '''Apply the ReplaceInductionVariablesTrans transformation to the specified node. The node must be a loop. In case of nested loops, the transformation might need to be applied several times, from the inner-most loop outwards. :param node: a Loop node. - :type node: :py:class:`psyclone.psyir.nodes.Loop` ''' - self.validate(node, options) + # TODO #2668: Remove options. + self.validate(node, options, **kwargs) loop_var = node.variable.name # Find assignments that are directly part of the loop (this @@ -263,18 +266,18 @@ def apply(self, node, options=None): # statement in the body. # ------------------------------------------------------------------------ - def validate(self, node, options=None): + def validate(self, node: Loop, options=None, **kwargs): '''Perform various checks to ensure that it is valid to apply the ReplaceInductionVariablesTrans transformation to the supplied PSyIR Node. :param node: the node that is being checked. - :type node: :py:class:`psyclone.psyir.nodes.Assignment` - :raises TransformationError: if the node argument is not a \ + :raises TransformationError: if the node argument is not a Loop. ''' + self.validate_options(**kwargs) if not isinstance(node, Loop): raise TransformationError( f"Error in {self.name} transformation. The supplied node " diff --git a/src/psyclone/psyir/transformations/replace_reference_by_literal_trans.py b/src/psyclone/psyir/transformations/replace_reference_by_literal_trans.py index 6c5050942f..7b76c9c7af 100644 --- a/src/psyclone/psyir/transformations/replace_reference_by_literal_trans.py +++ b/src/psyclone/psyir/transformations/replace_reference_by_literal_trans.py @@ -55,8 +55,10 @@ from psyclone.psyir.transformations.transformation_error import ( TransformationError, ) +from psyclone.utils import transformation_documentation_wrapper +@transformation_documentation_wrapper class ReplaceReferenceByLiteralTrans(Transformation): ''' This transformation takes a psyir Routine and replace all Reference psyir @@ -230,7 +232,8 @@ def _replace_bounds( return new_shape # ------------------------------------------------------------------------ - def apply(self, node: Routine, options: Optional[Dict[str, Any]] = None): + def apply(self, node: Routine, options: Optional[Dict[str, Any]] = None, + **kwargs): """Applies the transformation to a Routine node: * First update a dictionary (param_table) with the Literal of constant (parameter) symbol from node.parent symbol_table, and from @@ -248,6 +251,7 @@ def apply(self, node: Routine, options: Optional[Dict[str, Any]] = None): :param node: node on which the transformation is applied :param options: a dictionary with options for transformations. """ + # TODO 2668: Remove options. ## Reset the param table for the current Routine self._param_table = {} self.validate(node, options) @@ -279,7 +283,7 @@ def apply(self, node: Routine, options: Optional[Dict[str, Any]] = None): new_shape) # ------------------------------------------------------------------------ - def validate(self, node, options=None): + def validate(self, node: Routine, options=None, **kwargs): """Perform various checks to ensure that it is valid to apply the ReplaceReferenceByLiteralTrans transformation to the supplied PSyIR Node. @@ -290,6 +294,7 @@ def validate(self, node, options=None): :raises TransformationError: if the node argument is not a Routine. """ + self.validate_options(**kwargs) if not isinstance(node, Routine): raise TransformationError( f"Error in {self.name} transformation. The supplied node " diff --git a/src/psyclone/psyir/transformations/scalarisation_trans.py b/src/psyclone/psyir/transformations/scalarisation_trans.py index 25884cf246..3847baf11f 100644 --- a/src/psyclone/psyir/transformations/scalarisation_trans.py +++ b/src/psyclone/psyir/transformations/scalarisation_trans.py @@ -35,8 +35,6 @@ '''This module provides the sclarization transformation class.''' -from typing import Optional, Dict, Any, List, Tuple - from psyclone.core import VariablesAccessMap, Signature, SymbolicMaths from psyclone.psyGen import Kern from psyclone.psyir.nodes import Call, CodeBlock, Literal, \ @@ -44,8 +42,10 @@ from psyclone.psyir.nodes.array_mixin import ArrayMixin from psyclone.psyir.symbols import DataSymbol, RoutineSymbol, INTEGER_TYPE from psyclone.psyir.transformations.loop_trans import LoopTrans +from psyclone.utils import transformation_documentation_wrapper +@transformation_documentation_wrapper class ScalarisationTrans(LoopTrans): '''This transformation takes a Loop and converts any array accesses to scalar if the results of the loop are unused, and the initial value @@ -197,7 +197,7 @@ def _check_first_access_is_write(signature: Signature, @staticmethod def _get_index_values_from_indices( - node: ArrayMixin, indices: List[Node]) -> Tuple[bool, List[Node]]: + node: ArrayMixin, indices: list[Node]) -> tuple[bool, list[Node]]: ''' Compute a list of index values for a given node. Looks at loop bounds and range declarations to attempt to convert loop variables to an @@ -367,8 +367,16 @@ def _value_unused_after_loop(sig: Signature, return True - def apply(self, node: Loop, options: Optional[Dict[str, Any]] = None) \ - -> None: + def validate(self, node: Loop, **kwargs): + ''' + Validate the options provided to the ScalarisationTrans. + + :param node: the supplied loop to apply scalarisation to. + + ''' + self.validate_options(**kwargs) + + def apply(self, node: Loop, **kwargs) -> None: ''' Apply the scalarisation transformation to a loop. All of the array accesses that are identified as being able to be @@ -386,9 +394,9 @@ def apply(self, node: Loop, options: Optional[Dict[str, Any]] = None) \ 4. The array symbol is a local variable. :param node: the supplied loop to apply scalarisation to. - :param options: a dictionary with options for transformations. ''' + self.validate(node, **kwargs) # For each array reference in the Loop: # Find every access to the same symbol in the loop # They all have to be accessed with the same index statement, and From 570ab9572eb213b026cfb603a26b5878d8cdeecf Mon Sep 17 00:00:00 2001 From: LonelyCat124 <3043914+LonelyCat124@users.noreply.github.com.> Date: Thu, 23 Apr 2026 14:26:28 +0100 Subject: [PATCH 2/3] Updates for omp_taskloop_trans_coverage --- .../tests/psyir/transformations/omp_taskloop_trans_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/psyclone/tests/psyir/transformations/omp_taskloop_trans_test.py b/src/psyclone/tests/psyir/transformations/omp_taskloop_trans_test.py index 5a91f1a944..71b7f08772 100644 --- a/src/psyclone/tests/psyir/transformations/omp_taskloop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/omp_taskloop_trans_test.py @@ -165,7 +165,7 @@ def validate(self, options, **kwargs): _, invoke_info = parse(os.path.join(GOCEAN_BASE_PATH, "single_invoke.f90"), api="gocean") schedule = psy.invokes.invoke_list[0].schedule - taskloop.apply(schedule[0], {"nogroup": True}) + taskloop.apply(schedule[0], nogroup=True) assert "Fake error" in str(excinfo.value) assert taskloop._nogroup is False From d07fd5246714b888b7aa086f039c1061174b70cc Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Thu, 7 May 2026 11:56:47 +0100 Subject: [PATCH 3/3] #2668 Update changelog --- changelog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/changelog b/changelog index 473e06957f..908ebf8d2f 100644 --- a/changelog +++ b/changelog @@ -1,3 +1,5 @@ + 18) PR #3410 towards #2668. Update more Transformations to use kwargs. + 17) PR #3427. Preserve directive format casing. 16) PR #3422 for #440. Updates tests to use the Fortran backend to generate