diff --git a/changelog b/changelog index 56d32e873b..e3d24e129c 100644 --- a/changelog +++ b/changelog @@ -1,3 +1,6 @@ + 13) PR #3373 towards #3412. Extract argument expressions from NEMO iom_put + calls in order to parallelise/offload their implicit loops. + 12) PR #3408 for #2812. Updates the Min/Max to code transformations so that they use the datatype of their arguments rather than assuming real. Also skips treesitter tests for Python < 3.10. diff --git a/examples/nemo/scripts/omp_cpu_trans.py b/examples/nemo/scripts/omp_cpu_trans.py index 7bf3cb96ec..773a78e48b 100755 --- a/examples/nemo/scripts/omp_cpu_trans.py +++ b/examples/nemo/scripts/omp_cpu_trans.py @@ -117,7 +117,8 @@ def trans(psyir): loopify_array_intrinsics=True, convert_range_loops=True, hoist_expressions=False, - scalarise_loops=False + scalarise_loops=False, + hoist_argument_expressions=True ) if psyir.name not in PARALLELISATION_ISSUES: diff --git a/examples/nemo/scripts/omp_gpu_trans.py b/examples/nemo/scripts/omp_gpu_trans.py index dc5c078252..ba26aee5f9 100755 --- a/examples/nemo/scripts/omp_gpu_trans.py +++ b/examples/nemo/scripts/omp_gpu_trans.py @@ -197,7 +197,8 @@ def trans(psyir): loopify_array_intrinsics=True, convert_range_loops=True, increase_array_ranks=not NEMOV4, - hoist_expressions=True + hoist_expressions=True, + hoist_argument_expressions=True, ) # Perform module-inlining of called routines. if INLINING_ENABLED: diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index fec2f529f5..40005514a5 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -41,15 +41,15 @@ from psyclone.domain.common.transformations import KernelModuleInlineTrans from psyclone.psyir.nodes import ( Assignment, Loop, Directive, Node, Reference, CodeBlock, Call, - Routine, Schedule, IntrinsicCall, StructureReference, IfBlock) -from psyclone.psyir.symbols import DataSymbol + Routine, Schedule, IntrinsicCall, StructureReference, IfBlock, + Operation) +from psyclone.psyir.symbols import DataSymbol, ArrayType from psyclone.psyir.transformations import ( ArrayAssignment2LoopsTrans, HoistLoopBoundExprTrans, HoistLocalArraysTrans, HoistTrans, InlineTrans, Maxval2LoopTrans, Sum2LoopTrans, Minval2LoopTrans, Product2LoopTrans, ProfileTrans, OMPMinimiseSyncTrans, Reference2ArrayRangeTrans, ScalarisationTrans, IncreaseRankLoopArraysTrans, - MaximalRegionTrans) -from psyclone.transformations import TransformationError + MaximalRegionTrans, TransformationError, DataNodeToTempTrans) # USE statements to chase to gather additional symbol information. NEMO_MODULES_TO_IMPORT = [ @@ -181,6 +181,7 @@ def normalise_loops( scalarise_loops: bool = False, increase_array_ranks: bool = False, hoist_expressions: bool = True, + hoist_argument_expressions: bool = True, ): ''' Normalise all loops in the given schedule so that they are in an appropriate form for the Parallelisation transformations to analyse @@ -201,7 +202,14 @@ def normalise_loops( arrays. :param hoist_expressions: whether to hoist bounds and loop invariant statements out of the loop nest. + :param hoist_argument_expressions: whether to hoist array expressions + out of the containing Call. ''' + # TODO #3412: This is currently limited to iom_put, we want to expand it + # throughout the code + if hoist_argument_expressions: + iom_put_argument_to_temporary(schedule.walk(Call)) + if hoist_local_arrays and schedule.name not in CONTAINS_STMT_FUNCTIONS: # Apply the HoistLocalArraysTrans when possible, it cannot be applied # to files with statement functions because it will attempt to put the @@ -530,3 +538,25 @@ def _satisfies_minimum_region_rules(self, region: list[Node]) -> bool: routine_name = parent_routine.name if parent_routine else "" if routine_name not in PROFILING_IGNORE: MaximalProfilingOutsideDirectivesTrans().apply(children) + + +def iom_put_argument_to_temporary(calls: list[Call]): + '''Extracts the second argument of all iom_put calls and puts them + in a temporary if they are an Operation with an array datatype. + + :param calls: The list of calls in a subroutine whose arguments + may be moved into temporary storage to allow additional potential + parallelisation. + + ''' + for call in calls: + if call.symbol.name == "iom_put": + for arg in call.arguments: + dtype = arg.datatype + if (isinstance(dtype, ArrayType) and + (isinstance(arg, Operation) or + isinstance(arg, IntrinsicCall))): + try: + DataNodeToTempTrans().apply(arg, verbose=True) + except TransformationError: + pass diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 64aa7e8123..35c02d5366 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -1112,8 +1112,8 @@ def _fparser2_tree_from_fparser2_reader( except (FortranSyntaxError, NoMatchError) as err: raise ValueError( f"Failed to parse the provided source code:\n{source_code}" - "\nError was: {err}\nIs the input valid Fortran (note that" - f" CPP directives must be handled by a pre-processor)?" + f"\nError was: {err}\nIs the input valid Fortran (note " + f"that CPP directives must be handled by a pre-processor)?" ) from err try: # If it reaches this point a partial_code was provided, attempt diff --git a/src/psyclone/psyir/nodes/intrinsic_call.py b/src/psyclone/psyir/nodes/intrinsic_call.py index 9b42a183b2..1a84fd5c37 100644 --- a/src/psyclone/psyir/nodes/intrinsic_call.py +++ b/src/psyclone/psyir/nodes/intrinsic_call.py @@ -439,6 +439,8 @@ def _iparity_return_type(node: IntrinsicCall) -> DataType: :returns: the computed datatype for the IntrinsicCall. """ + # TODO #3415: Replace with _type_of_named_arg_accounting_for_dim_arg( + # node, "array"). dtype = ScalarType( node.argument_by_name("array").datatype.intrinsic, node.argument_by_name("array").datatype.precision, @@ -565,8 +567,13 @@ def _maxval_return_type(node: IntrinsicCall) -> DataType: :returns: the computed datatype for the IntrinsicCall. """ + # TODO #3415: Replace with _type_of_named_arg_accounting_for_dim_arg( + # node, "array"). + dtype = ScalarType( + node.argument_by_name("array").datatype.intrinsic, + node.argument_by_name("array").datatype.precision + ) arg = node.argument_by_name("array") - dtype = arg.datatype.elemental_type if "dim" not in node.argument_names: return dtype # We have a dimension specified. We don't know the resultant shape @@ -588,8 +595,8 @@ def _dot_product_return_type(node: IntrinsicCall) -> DataType: from psyclone.psyir.tools.type_info_computation import ( compute_scalar_type ) - veca_datatype = node.argument_by_name("vector_a").datatype - vecb_datatype = node.argument_by_name("vector_b").datatype + veca_datatype = node.argument_by_name("vector_a").datatype.elemental_type + vecb_datatype = node.argument_by_name("vector_b").datatype.elemental_type return compute_scalar_type( [ScalarType( veca_datatype.intrinsic, veca_datatype.precision @@ -3215,7 +3222,8 @@ class Intrinsic(IAttr, Enum): optional_args={"kind": DataNode}, return_type=lambda node: ( _type_of_scalar_with_optional_kind( - node, node.argument_by_name("l").datatype.intrinsic, + node, + node.argument_by_name("l").datatype.intrinsic, "kind", ) if "kind" in node.argument_names else _type_of_named_argument(node, "l") @@ -4897,7 +4905,20 @@ def datatype(self) -> DataType: if isinstance(self.intrinsic.return_type, Callable): try: return self.intrinsic.return_type(self) + except TypeError as err: + # If we get an invalid argument to a ScalarType constructor it + # means we attempted to pass either an UnresolvedType into the + # datatype + if ("ScalarType expected 'intrinsic' argument to be of type " + in str(err) + or "ScalarType expected 'precision' argument to be of " + "type " in str(err)): + return UnresolvedType() + # This should never happen, propogate as an InternalError. + outerr = err except AttributeError as err: + # This is to handle when we call .intrinsic or + # .precision on an UnresolvedType # If we get an attribute error, and its because of attempting # to lookup the precision or intrinsic, then it is likely # due to looking up the datatype elements of an Unresolved @@ -4908,13 +4929,15 @@ def datatype(self) -> DataType: and "NoneType" not in str(err)): return UnresolvedType() - # Can't use debug string due to this being a potentially - # incomplete IntrinsicCall - raise InternalError( - f"Failed to compute the datatype of a " - f"'{self.intrinsic.name}' intrinsic. This is likely due " - f"to not fully initialising the intrinsic correctly." - ) from err + outerr = err + # Fall through to the internalerror. + # Can't use debug string due to this being a potentially + # incomplete IntrinsicCall + raise InternalError( + f"Failed to compute the datatype of a " + f"'{self.intrinsic.name}' intrinsic. This is likely due " + f"to not fully initialising the intrinsic correctly." + ) from outerr else: return self.intrinsic.return_type diff --git a/src/psyclone/psyir/transformations/datanode_to_temp_trans.py b/src/psyclone/psyir/transformations/datanode_to_temp_trans.py index 21198c1a77..78505d31ef 100644 --- a/src/psyclone/psyir/transformations/datanode_to_temp_trans.py +++ b/src/psyclone/psyir/transformations/datanode_to_temp_trans.py @@ -42,10 +42,15 @@ Assignment, Call, DataNode, + IfBlock, IntrinsicCall, + Literal, + Loop, Range, Reference, Statement, + Schedule, + UnaryOperation, ) from psyclone.psyir.symbols.datatypes import ( ArrayType, @@ -111,6 +116,7 @@ def validate(self, node: DataNode, **kwargs): """ # Validate the input options and types. self.validate_options(**kwargs) + verbose = self.get_option("verbose", **kwargs) if not isinstance(node, DataNode): raise TypeError( @@ -123,21 +129,31 @@ def validate(self, node: DataNode, **kwargs): calls = node.walk(Call) for call in calls: if not call.is_pure: - raise TransformationError( + message = ( f"Input node to {self.name} contains a call " f"'{call.debug_string().strip()}' that is not guaranteed " f"to be pure. Input node is " f"'{node.debug_string().strip()}'." ) + if verbose: + node.ancestor(Statement).append_preceding_comment( + f"PSyclone Warning: {message}" + ) + raise TransformationError(message) if isinstance(dtype, ArrayType): for element in dtype.shape: if element in [ArrayType.Extent.DEFERRED, ArrayType.Extent.ATTRIBUTE]: - raise TransformationError( + message = ( f"Input node's datatype is an array of unknown size, " f"so the {self.name} cannot be applied. " f"Input node was '{node.debug_string().strip()}'." ) + if verbose: + node.ancestor(Statement).append_preceding_comment( + f"PSyclone Warning: {message}" + ) + raise TransformationError(message) # The shape must now be set by ArrayBounds, we need to # examine the symbols used to define those bounds. symbols = set() @@ -147,9 +163,11 @@ def validate(self, node: DataNode, **kwargs): symbols.update(element.upper.get_all_accessed_symbols()) # Compare the symbols in the array bounds with the symbols # already in the scope. - scope_symbols = node.scope.symbol_table.get_symbols() + scope_table = node.scope.symbol_table for sym in symbols: - scoped_name_sym = scope_symbols.get(sym.name, None) + scoped_name_sym = scope_table.lookup( + sym.name, otherwise=None + ) # If sym is not scoped_name_sym, then there is a # symbol collision from an imported symbol. if scoped_name_sym and sym is not scoped_name_sym: @@ -157,38 +175,54 @@ def validate(self, node: DataNode, **kwargs): # container then we can skip this. if scoped_name_sym.interface == sym.interface: continue - raise TransformationError( + message = ( f"The type of the node supplied to {self.name} " f"depends upon an imported symbol '{sym.name}' " f"which has a name clash with a symbol in the " f"current scope." ) + if verbose: + node.ancestor(Statement).append_preceding_comment( + f"PSyclone Warning: {message}" + ) + raise TransformationError(message) # If its not in the current scope, and its visibility is # private then we can't import it. if (not scoped_name_sym and sym.visibility == Symbol.Visibility.PRIVATE): - raise TransformationError( + message = ( f"The datatype of the node suppled to " f"{self.name} depends upon an imported symbol " f"'{sym.name}' that is declared as private in " f"its containing module, so cannot be imported." ) + if verbose: + node.ancestor(Statement).append_preceding_comment( + f"PSyclone Warning: {message}" + ) + raise TransformationError(message) # If its an imported symbol we need to check if its # the same import interface. if isinstance(sym.interface, ImportInterface): - scoped_name_sym = scope_symbols.get( - sym.interface.container_symbol.name, - None + scoped_name_sym = scope_table.lookup( + sym.interface.container_symbol.name, + otherwise=None ) if scoped_name_sym and not isinstance( scoped_name_sym, ContainerSymbol): - raise TransformationError( + message = ( f"Input node contains an imported symbol " f"'{sym.name}' whose containing module " f"collides with an existing symbol. Colliding " f"name is " f"'{sym.interface.container_symbol.name}'." ) + if verbose: + node.ancestor(Statement).\ + append_preceding_comment( + f"PSyclone Warning: {message}" + ) + raise TransformationError(message) if node.ancestor(Statement) is None: raise TransformationError( @@ -217,18 +251,26 @@ def validate(self, node: DataNode, **kwargs): f"RESOLVE_IMPORTS in the transformation script may " f"enable resolution of these symbols." ) + if verbose: + node.ancestor(Statement).append_preceding_comment( + f"PSyclone Warning: {message}" + ) raise TransformationError(message) - def apply(self, node: DataNode, storage_name: str = "", **kwargs): + def apply(self, node: DataNode, storage_name: str = "", + verbose: bool = False, **kwargs): """Applies the DataNodeToTempTrans to the input arguments. :param node: The datanode to extract. :param storage_name: The base name of the temporary variable to store the result of the input node in. The default is tmp(_...) based on the rules defined in the SymbolTable class. + :param verbose: Whether to add comments to the input node if + the transformation fails. """ # Call validate to check inputs are valid. - self.validate(node, storage_name=storage_name, **kwargs) + self.validate(node, storage_name=storage_name, verbose=verbose, + **kwargs) # Find the datatype datatype = node.datatype @@ -248,18 +290,20 @@ def apply(self, node: DataNode, storage_name: str = "", **kwargs): symbols.update(element.lower.get_all_accessed_symbols()) if isinstance(element.upper, DataNode): symbols.update(element.upper.get_all_accessed_symbols()) - scope_symbols = node.scope.symbol_table.get_symbols() + scope_table = node.scope.symbol_table for sym in symbols: - scoped_name_sym = scope_symbols.get(sym.name, None) + scoped_name_sym = scope_table.lookup( + sym.name, otherwise=None + ) # If no symbol with the name exists then create one. if not scoped_name_sym: sym_copy = sym.copy() if isinstance(sym_copy.interface, ImportInterface): # Check if the ContainerSymbol is already in the # interface - container = scope_symbols.get( + container = scope_table.lookup( sym_copy.interface.container_symbol.name, - None + otherwise=None ) if container is None: # Add the container symbol to the symbol table @@ -276,13 +320,27 @@ def apply(self, node: DataNode, storage_name: str = "", **kwargs): # the datatype to use the in-scope symbols datatype.replace_symbols_using(node.scope.symbol_table) - # We want to create an allocatable symbol for Array entities, so - # create a new datatype for the symbol and keep the - # datatype around for the ALLOCATE statement later. - allocatable_datatype = datatype - datatype = ArrayType(allocatable_datatype.elemental_type, - [ArrayType.Extent.DEFERRED for x in - allocatable_datatype.shape]) + # If any of the bound information aren't static then we need + # to create an allocatable array. + has_static_bounds = True + for element in datatype.shape: + if not isinstance(element.lower, Literal): + has_static_bounds = False + break + if not isinstance(element.upper, Literal): + has_static_bounds = False + break + if has_static_bounds: + datatype = ArrayType(datatype.elemental_type, + [x.copy() for x in datatype.shape]) + else: + # We want to create an allocatable symbol for Array entities, + # so create a new datatype for the symbol and keep the + # datatype around for the ALLOCATE statement later. + allocatable_datatype = datatype + datatype = ArrayType(allocatable_datatype.elemental_type, + [ArrayType.Extent.DEFERRED for x in + allocatable_datatype.shape]) # Create a symbol of the relevant type. if not storage_name: @@ -300,10 +358,11 @@ def apply(self, node: DataNode, storage_name: str = "", **kwargs): # Create a Reference to the new symbol new_ref = Reference(symbol) - # Find the parent and position of the statement containing the - # DataNode. - parent = node.ancestor(Statement).parent - pos = node.ancestor(Statement).position + # Find the containing schedule and position of the statement + # containing the DataNode. + schedule = node.ancestor(Schedule) + path = node.path_from(schedule) + pos = path[0] # Replace the datanode with the new reference node.replace_with(new_ref) @@ -312,11 +371,11 @@ def apply(self, node: DataNode, storage_name: str = "", **kwargs): assign = Assignment.create(new_ref.copy(), node) # Add the assignment into the tree. - parent.addchild(assign, pos) + schedule.addchild(assign, pos) # If the datatype is an array, we need to allocate the array - # before the statement too. - if isinstance(datatype, ArrayType): + # before the statement too if its not already allocated. + if isinstance(datatype, ArrayType) and not has_static_bounds: # Create an array reference to the symbol with the dimensions # returned by the datatype call earlier. ref = ArrayReference.create( @@ -329,9 +388,42 @@ def apply(self, node: DataNode, storage_name: str = "", **kwargs): IntrinsicCall.Intrinsic.ALLOCATE, (ref,) ) - # Add the allocate statement into the tree immediately before - # its use. - parent.addchild(intrinsic, pos) + allocated = IntrinsicCall.create( + IntrinsicCall.Intrinsic.ALLOCATED, + (Reference(symbol),) + ) + + ifblock = IfBlock.create( + UnaryOperation.create( + UnaryOperation.Operator.NOT, + allocated), + [intrinsic] + ) + # If the shape doesn't contain array references then we can hoist + # the allocate statement outside of any ancestor loops. + hoistable = True + for shape in ref.indices: + for ref2 in shape.walk(Reference): + if isinstance(ref2, ArrayReference): + hoistable = False + # If we can hoist the allocate, find the highest level Loop + # ancestor and set the schedule and position to place the + # allocate before this loop. + # TODO #1445: Use HositTrans to do this if its extended to support + # more node types. + if hoistable: + loop_anc = schedule.ancestor(Loop) + cursor = loop_anc + while cursor: + loop_anc = cursor + cursor = cursor.ancestor(Loop) + if loop_anc: + pos = loop_anc.position + schedule = loop_anc.ancestor(Schedule) + + # Add the allocate statement and the containing ifblock into the + # tree immediately before its use. + schedule.addchild(ifblock, pos) __all__ = ["DataNodeToTempTrans"] diff --git a/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py b/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py index 9b760c2ed6..ae1a7053ce 100644 --- a/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py +++ b/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py @@ -43,6 +43,7 @@ """ import pytest +from enum import Enum from psyclone.core.access_type import AccessType from psyclone.errors import InternalError @@ -51,11 +52,13 @@ Assignment, BinaryOperation, Call, + DataNode, Literal, Schedule, Reference, ) from psyclone.psyir.nodes.intrinsic_call import ( + ArgDesc, IntrinsicCall, IAttr, _type_of_arg_with_rank_minus_one, @@ -175,6 +178,80 @@ def test_intrinsiccall_datatype(fortran_reader): assert isinstance(call.datatype, UnresolvedType) +def test_intrinsiccall_datatype_typeerr_paths( + fortran_reader, monkeypatch +): + '''Test the TypeError raising paths inside the datatype function + of IntrinsicCall.''' + + def expected_typeerr(*args, **kwargs): + '''Raise one of the expected type errors that datatype can handle.''' + raise TypeError( + "ScalarType expected 'intrinsic' argument to be of type " + ) + + def unexpected_typeerr(*args, **kwargs): + '''Raise some other type error that datatype raises an error from.''' + raise TypeError( + "This is a bad type error." + ) + + class Intrinsic(IAttr, Enum): + '''Test class to override the base Intrinsic enum''' + MAX = IAttr( + name="MAX", + is_pure=True, + is_elemental=True, + is_inquiry=False, + # No upper limit on argument type so we don't store an + # argument list of names. + required_args=ArgDesc( + min_count=2, + max_count=None, + types=DataNode, + arg_names=((None,),)), + optional_args={}, + return_type=expected_typeerr, + reference_accesses=None, + ) + MIN = IAttr( + name="MIN", + is_pure=True, + is_elemental=True, + is_inquiry=False, + # No upper limit on argument type so we don't store an + # argument list of names. + required_args=ArgDesc( + min_count=2, + max_count=None, + types=DataNode, + arg_names=((None,),)), + optional_args={}, + return_type=unexpected_typeerr, + reference_accesses=None, + ) + + # Replace the Intrinsic Enum as its not modifiable. + monkeypatch.setattr(IntrinsicCall, "Intrinsic", Intrinsic) + + code = """subroutine test + integer, dimension(100) :: i + integer :: j + + j = MAX(i(1), i(2)) + j = MIN(i(3), i(4)) + end subroutine test""" + psyir = fortran_reader.psyir_from_source(code) + intrinsics = psyir.walk(IntrinsicCall) + + assert isinstance(intrinsics[0].datatype, UnresolvedType) + with pytest.raises(InternalError) as err: + _ = intrinsics[1].datatype + assert ("Failed to compute the datatype of a 'MIN' intrinsic. " + "This is likely due to not fully initialising the intrinsic " + "correctly." in str(err.value)) + + def test_intrinsiccall_reference_accesses_no_arg_names(): """Test the case of IntrinsicCall's reference_accesses method where the call to compute_argument_names fails.""" @@ -1322,6 +1399,7 @@ def test_iparity_return_type(fortran_reader): k = IPARITY(array) end subroutine x """ + # TODO #3415: Test is superfluous with this issue fixed. psyir = fortran_reader.psyir_from_source(code) # TODO #3268 Can't iparity directily with fortran reader, so need to # create the Intrinsics manually using the psyir from the generated code. @@ -1481,6 +1559,7 @@ def test_matmul_return_type(fortran_reader): def test_maxval_return_type(fortran_reader): '''Test for the _maxval_return_type function.''' + # TODO #3415: Test is superfluous with this issue fixed. code = """subroutine test integer, parameter :: wp = 8 integer*8, dimension(100,100) :: x diff --git a/src/psyclone/tests/psyir/transformations/datanode_to_temp_trans_test.py b/src/psyclone/tests/psyir/transformations/datanode_to_temp_trans_test.py index f40b56e832..647ecba4c3 100644 --- a/src/psyclone/tests/psyir/transformations/datanode_to_temp_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/datanode_to_temp_trans_test.py @@ -51,7 +51,7 @@ from psyclone.tests.utilities import Compile -def test_datanodetotemptrans_validate(fortran_reader, tmp_path): +def test_datanodetotemptrans_validate(fortran_reader): """Tests the non-import related functionality of the validate function of the DataNodeToTempTrans.""" dtrans = DataNodeToTempTrans() @@ -62,10 +62,12 @@ def test_datanodetotemptrans_validate(fortran_reader, tmp_path): psyir = fortran_reader.psyir_from_source(code) assign = psyir.walk(Assignment)[0] with pytest.raises(TransformationError) as err: - dtrans.validate(assign.rhs) + dtrans.validate(assign.rhs, verbose=True) assert ("Input node's datatype is an array of unknown size, so the " "DataNodeToTempTrans cannot be applied. Input node was " "'b + a'" in str(err.value)) + assert ("PSyclone Warning: Input node's datatype is an array" in + assign.preceding_comment) code = """subroutine test use some_mod @@ -74,13 +76,17 @@ def test_datanodetotemptrans_validate(fortran_reader, tmp_path): psyir = fortran_reader.psyir_from_source(code) assign = psyir.walk(Assignment)[0] with pytest.raises(TransformationError) as err: - dtrans.validate(assign.rhs) + dtrans.validate(assign.rhs, verbose=True) assert ("The datatype of the supplied node cannot be computed, so " "the DataNodeToTempTrans cannot be applied. Input node " "was 'b + a'. The following symbols in the input " "node have not been resolved by PSyclone: '['a', 'b']'. " "Setting RESOLVE_IMPORTS in the transformation script " "may enable resolution of these symbols." in str(err.value)) + assert ("PSyclone Warning: The datatype of the supplied node " in + assign.preceding_comment) + assert ("Setting RESOLVE_IMPORTS in the transformation script" in + assign.preceding_comment) code = """subroutine test complex :: a, b @@ -90,10 +96,12 @@ def test_datanodetotemptrans_validate(fortran_reader, tmp_path): psyir = fortran_reader.psyir_from_source(code) assign = psyir.walk(Assignment)[0] with pytest.raises(TransformationError) as err: - dtrans.validate(assign.rhs) + dtrans.validate(assign.rhs, verbose=True) assert ("The datatype of the supplied node cannot be computed, " "so the DataNodeToTempTrans cannot be applied. Input node " "was 'a'" in str(err.value)) + assert ("PSyclone Warning: The datatype of the supplied node " in + assign.preceding_comment) with pytest.raises(TypeError) as err: dtrans.validate("abc") @@ -128,11 +136,13 @@ def test_datanodetotemptrans_validate(fortran_reader, tmp_path): psyir = fortran_reader.psyir_from_source(code) assign = psyir.walk(Assignment)[2] with pytest.raises(TransformationError) as err: - dtrans.validate(assign.rhs) + dtrans.validate(assign.rhs, verbose=True) assert ("Input node to DataNodeToTempTrans contains a call " "'some_func(a, b)' that is not " "guaranteed to be pure. Input node is 'a + some_func(a, b)'." in str(err.value)) + assert ("PSyclone Warning: Input node to DataNodeToTempTrans contains a " + "call" in assign.preceding_comment) def test_datanodetotemptrans_validate_imports( @@ -160,10 +170,12 @@ def test_datanodetotemptrans_validate_imports( psyir.children[0].symbol_table.resolve_imports() assign = psyir.walk(Assignment)[0] with pytest.raises(TransformationError) as err: - dtrans.validate(assign.rhs) + dtrans.validate(assign.rhs, verbose=True) assert ("The type of the node supplied to DataNodeToTempTrans depends " "upon an imported symbol 'i' which has a name clash with a " "symbol in the current scope." in str(err.value)) + assert ("PSyclone Warning: The type of the node supplied to " + in assign.preceding_comment) # This should work if the i in scope is imported from the # some_mod already. @@ -240,11 +252,13 @@ def test_datanodetotemptrans_validate_imports( psyir = FortranReader(resolve_modules=True).psyir_from_source(code) assign = psyir.walk(Assignment)[0] with pytest.raises(TransformationError) as err: - dtrans.validate(assign.rhs) + dtrans.validate(assign.rhs, verbose=True) assert ("Input node contains an imported symbol 'i' whose containing " "module collides with an existing symbol. Colliding name is " "'tmpmod'." in str(err.value)) + assert ("PSyclone Warning: Input node contains an imported symbol 'i' " + in assign.preceding_comment) filename = tmp_path / "some_other_mod.f90" with open(filename, "w", encoding='UTF-8') as module: @@ -264,14 +278,15 @@ def test_datanodetotemptrans_validate_imports( psyir = FortranReader(resolve_modules=True).psyir_from_source(code) assign = psyir.walk(Assignment)[0] with pytest.raises(TransformationError) as err: - dtrans.validate(assign.rhs) + dtrans.validate(assign.rhs, verbose=True) assert ("The datatype of the node suppled to DataNodeToTempTrans depends " "upon an imported symbol 'dim1' that is declared as private in " "its containing module, so cannot be imported." in str(err.value)) + assert ("PSyclone Warning: The datatype of the node suppled to" in + assign.preceding_comment) -def test_datanodetotemptrans_apply(fortran_reader, fortran_writer, tmp_path, - monkeypatch): +def test_datanodetotemptrans_apply(fortran_reader, fortran_writer, tmp_path): """Tests the apply function of the DataNodeToTempTrans without imported symbols.""" dtrans = DataNodeToTempTrans() @@ -287,7 +302,9 @@ def test_datanodetotemptrans_apply(fortran_reader, fortran_writer, tmp_path, out = fortran_writer(psyir) assert """ integer, allocatable, dimension(:,:) :: tmp - ALLOCATE(tmp(1:SIZE(a, dim=1),1:SIZE(b, dim=2))) + if (.NOT.ALLOCATED(tmp)) then + ALLOCATE(tmp(1:SIZE(a, dim=1),1:SIZE(b, dim=2))) + end if tmp = MATMUL(a, b) d = c + tmp""" in out assert Compile(tmp_path).string_compiles(out) @@ -326,8 +343,10 @@ def test_datanodetotemptrans_apply(fortran_reader, fortran_writer, tmp_path, enddo""" in out code = """subroutine test() + integer, parameter :: i = 1 + integer, parameter :: j = 3 integer, dimension(2:6) :: a - integer, dimension(1:3) :: b + integer, dimension(i:j) :: b a(2:4) = 3 * b @@ -338,7 +357,9 @@ def test_datanodetotemptrans_apply(fortran_reader, fortran_writer, tmp_path, out = fortran_writer(psyir) assert """ integer, allocatable, dimension(:) :: tmp - ALLOCATE(tmp(1:3)) + if (.NOT.ALLOCATED(tmp)) then + ALLOCATE(tmp(i:j)) + end if tmp = 3 * b a(:4) = tmp""" in out assert Compile(tmp_path).string_compiles(out) @@ -395,9 +416,8 @@ def test_datanodetotemptrans_apply_imports( assign = psyir.walk(Assignment)[0] dtrans.apply(assign.rhs) out = fortran_writer(psyir) - assert """ integer, allocatable, dimension(:,:) :: tmp + assert """ integer, dimension(25,50) :: tmp - ALLOCATE(tmp(1:25,1:50)) tmp = some_var b = tmp""" in out @@ -424,7 +444,9 @@ def test_datanodetotemptrans_apply_imports( integer, dimension(25,50) :: b integer, allocatable, dimension(:,:) :: tmp - ALLOCATE(tmp(1:25,1:i)) + if (.NOT.ALLOCATED(tmp)) then + ALLOCATE(tmp(1:25,1:i)) + end if tmp = some_var b = tmp""" in out @@ -462,7 +484,9 @@ def test_datanodetotemptrans_apply_imports( use f_mod, only : some_var integer, allocatable, dimension(:,:) :: tmp - ALLOCATE(tmp(1:25,1:i)) + if (.NOT.ALLOCATED(tmp)) then + ALLOCATE(tmp(1:25,1:i)) + end if tmp = some_var j = tmp""" in out @@ -493,6 +517,70 @@ def test_datanodetotemptrans_apply_nemo_example(fortran_reader, out = fortran_writer(psyir) assert """real, allocatable, dimension(:,:,:) :: tmp - ALLOCATE(tmp(1:nie0 - nis0 + 1,1:nje0 - njs0 + 1,1:SIZE(rn2, dim=3))) + if (.NOT.ALLOCATED(tmp)) then + ALLOCATE(tmp(1:nie0 - nis0 + 1,1:nje0 - njs0 + 1,1:SIZE(rn2, dim=3))) + end if tmp = -avt_k(:,:,:) * rn2(nis0:nie0,njs0:nje0,:) * \ -wmask(nis0:nie0,njs0:nje0,:)""" in out +wmask(nis0:nie0,njs0:nje0,:) + call iom_put('estrat_k', tmp)""" in out + + +def test_datanodetotemptrans_hoistable_array(fortran_reader, + fortran_writer): + ''' + Takes an array sized datanode and ensures the allocate is hoisted out of + the containing loop if possible. + ''' + + code = """subroutine test + use some_mod, only: some_func + integer :: i, a, b + real, dimension(a,b) :: arr1, arr2 + + do i = 1, 100 + call some_func(arr1*arr2) + end do + end subroutine test""" + + psyir = fortran_reader.psyir_from_source(code) + dtrans = DataNodeToTempTrans() + dtrans.apply( + psyir.children[0].children[0].loop_body.children[0].arguments[0] + ) + + out = fortran_writer(psyir) + assert """ if (.NOT.ALLOCATED(tmp)) then + ALLOCATE(tmp(1:a,1:b)) + end if + do i = 1, 100, 1 + tmp = arr1 * arr2 + call some_func(tmp) + enddo""" in out + + # If the shape of the result contains an array expression then we + # shouldn't hoist. + + code = """subroutine test + use some_mod, only: some_func + integer :: i + real, dimension(100, 100) :: arr1, arr2 + integer, dimension(100) :: arr3 + + do i = 1, 100 + call some_func(arr1(1:arr3(i),:)*arr2(1:arr3(i),:)) + end do + end subroutine test""" + psyir = fortran_reader.psyir_from_source(code) + dtrans = DataNodeToTempTrans() + dtrans.apply( + psyir.children[0].children[0].loop_body.children[0].arguments[0] + ) + + out = fortran_writer(psyir) + assert """ do i = 1, 100, 1 + if (.NOT.ALLOCATED(tmp)) then + ALLOCATE(tmp(1:arr3(i),1:100)) + end if + tmp = arr1(:arr3(i),:) * arr2(:arr3(i),:) + call some_func(tmp) + enddo""" in out