diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index acac8c384f..aa47d5a953 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -985,14 +985,16 @@ def gen_decls(self, except KeyError: internal_interface_symbol = None if unresolved_symbols and not ( - symbol_table.wildcard_imports() or internal_interface_symbol): + symbol_table.wildcard_imports() or + internal_interface_symbol or + (symbol_table.node and symbol_table.node.walk(CodeBlock))): symbols_txt = ", ".join( ["'" + sym.name + "'" for sym in unresolved_symbols]) raise VisitorError( f"The following symbols are not explicitly declared or " f"imported from a module and there are no wildcard " - f"imports which could be bringing them into scope: " - f"{symbols_txt}") + f"imports, generic interfaces or CodeBlocks which could be " + f"bringing them into scope: {symbols_txt}") # Check that the names of all symbols are less than the limit # imposed by the Fortran standard. @@ -1074,19 +1076,23 @@ def filecontainer_node(self, node): :rtype: str :raises VisitorError: if the attached symbol table contains - any non-routine symbols. + any symbols that can not be declard in a FileContainer. :raises VisitorError: if more than one child is a Routine Node with is_program set to True. ''' for symbol in node.symbol_table.symbols: - # TODO #2201 - ContainerSymbols should be accepted but - # currently are stored in its containing scope. - if not isinstance(symbol, RoutineSymbol): + # Only RoutineSymbols and ContainerSymbol can be declared here + # pylint: disable=unidiomatic-typecheck + if type(symbol) is Symbol and symbol.is_unresolved: + # However we also accept symbols that we don't know where + # they are declared, so we propagated upwards. + continue + if not isinstance(symbol, (RoutineSymbol, ContainerSymbol)): raise VisitorError( f"In the Fortran backend, a file container should not " - f"have any symbols associated with it other than " - f"RoutineSymbols, but found {str(symbol)}.") + f"have any data symbols associated with it, " + f"but found {str(symbol)}.") program_nodes = len([child for child in node.children if isinstance(child, Routine) and child.is_program]) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index f90a6588bd..0a35314657 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -3379,7 +3379,7 @@ def _do_construct_handler(self, node, parent): self.process_comment(child, preceding_comments) continue if isinstance(child, Fortran2003.Directive) and not found_do_stmt: - directive = self._directive_handler(child, None) + directive = self._directive_handler(child, loop.parent) # Add the directive before the loop. loop.parent.addchild(directive) directive.preceding_comment = ( @@ -3440,7 +3440,7 @@ def _if_construct_handler(self, node, parent): if isinstance(child, Fortran2003.Comment): self.process_comment(child, preceding_comments) if isinstance(child, Fortran2003.Directive): - direc = self._directive_handler(child, None) + direc = self._directive_handler(child, parent) parent.addchild(direc) direc.preceding_comment = ( self._comments_list_to_string(preceding_comments) diff --git a/src/psyclone/psyir/nodes/codeblock.py b/src/psyclone/psyir/nodes/codeblock.py index 2203ded286..8df9000cfc 100644 --- a/src/psyclone/psyir/nodes/codeblock.py +++ b/src/psyclone/psyir/nodes/codeblock.py @@ -47,12 +47,16 @@ from psyclone.core import AccessType, Signature, VariablesAccessMap from psyclone.psyir.nodes.statement import Statement from psyclone.psyir.nodes.datanode import DataNode +from psyclone.psyir.nodes.reference import Reference +from psyclone.psyir.nodes.node import Node +from psyclone.psyir.symbols import ( + SymbolTable, SymbolError, UnresolvedInterface) class CodeBlock(Statement, DataNode): '''Node representing some generic Fortran code that PSyclone does not - attempt to manipulate. As such it is a leaf in the PSyIR and therefore - has no children. + attempt to manipulate. It has a child Reference for each symbol used in + the CodeBlock. :param fp2_nodes: the fparser2 parse-tree nodes representing the Fortran code constituting the code block. @@ -69,7 +73,7 @@ class CodeBlock(Statement, DataNode): ''' #: Textual description of the node. - _children_valid_format = "" + _children_valid_format = "[Reference]*" _text_name = "CodeBlock" _colour = "red" #: The annotations that are supported by this node. @@ -105,6 +109,35 @@ def __init__(self, fp2_nodes, structure, parent=None, annotations=None): self.ast_end = None # Store the structure of the code block. self._structure = structure + # Capture all symbols used inside the Codeblock as children References + self._insert_representative_references() + + @staticmethod + def _validate_child(position: int, child: Node) -> bool: + ''' + :param position: the position to be validated. + :param child: a child to be validated. + + :return: whether the given child and position are valid for this node. + + ''' + return isinstance(child, Reference) + + def _insert_representative_references(self): + ''' Insert Reference children under this codeblock that + represent each of the symbols used inside the CodeBlock. + ''' + for symbol_name in self.get_symbol_names(): + try: + symtab = self.scope.symbol_table + except SymbolError: + # Needed for detached CodeBlocks, mainly used in testing + symtab = SymbolTable() + symbol = symtab.find_or_create( + symbol_name, interface=UnresolvedInterface()) + ref = Reference(symbol) + if ref not in self.children: + self.addchild(Reference(symbol)) def __eq__(self, other): ''' @@ -186,12 +219,26 @@ def get_symbol_names(self) -> List[str]: node is node.parent.children[0]): result.append(node.string) elif not isinstance(node.parent, + # We don't want labels associated with loop or + # branch control. (Fortran2003.Cycle_Stmt, Fortran2003.End_Do_Stmt, Fortran2003.Exit_Stmt, Fortran2003.Else_Stmt, Fortran2003.End_If_Stmt)): - # We don't want labels associated with loop or branch control. + + # Check if this name is a structure accessor instead of a + # symbol + if isinstance(node.parent, Fortran2003.Part_Ref): + # Also account for array fields name%array(i) + check = node.parent + else: + check = node + if isinstance(check.parent, Fortran2003.Data_Ref): + # The first child is the base reference, the others are + # accessor names, which are not symbols + if check.parent.children[0] is not check: + continue result.append(node.string) # Precision on literals requires special attention since they are just # stored in the tree as str (fparser/#456). @@ -235,10 +282,6 @@ def reference_accesses(self) -> VariablesAccessMap: TODO #2863 - it would be better to use AccessType.UNKNOWN here but currently VariablesAccessMap does not consider that type of access. - This method makes use of - :py:meth:`~psyclone.psyir.nodes.CodeBlock.get_symbol_names` and is - therefore subject to the same limitations as that method. - :returns: a map of all the symbol accessed inside this node, the keys are Signatures (unique identifiers to a symbol and its structure accessors) and the values are AccessSequence @@ -246,9 +289,13 @@ def reference_accesses(self) -> VariablesAccessMap: ''' var_accesses = VariablesAccessMap() - for name in self.get_symbol_names(): - var_accesses.add_access(Signature(name), AccessType.READWRITE, - self) + # All symbols accessed within the CodeBlock are captured as Reference + # nodes and stored as children of the CodeBlock node + for child in self.children: + var_accesses.add_access( + Signature(child.name), + AccessType.READWRITE, + child) return var_accesses def __str__(self): diff --git a/src/psyclone/psyir/nodes/reference.py b/src/psyclone/psyir/nodes/reference.py index 6d70fdb560..03709bd112 100644 --- a/src/psyclone/psyir/nodes/reference.py +++ b/src/psyclone/psyir/nodes/reference.py @@ -119,6 +119,7 @@ def is_write(self): # pylint: disable=import-outside-toplevel from psyclone.psyir.nodes.assignment import Assignment from psyclone.psyir.nodes.call import Call + from psyclone.psyir.nodes.codeblock import CodeBlock from psyclone.psyir.nodes.intrinsic_call import IntrinsicCall parent = self.parent # pure or inquiry IntrinsicCall nodes do not write to their arguments. @@ -133,6 +134,9 @@ def is_write(self): # The reference that is the LHS of an assignment is a write. if isinstance(parent, Assignment) and parent.lhs is self: return True + # Assume the worst for CodeBlocks + if isinstance(parent, CodeBlock): + return True return False @property diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index d74f782103..97df411831 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -481,16 +481,18 @@ def _compute_forward_uses(self, basic_block_list: list[Node]): if defs_out is not None: self._defsout.append(defs_out) return - if ( - self._reference.symbol.name - in reference.get_symbol_names() - ): - # Assume the worst for a CodeBlock and we count them - # as killed and defsout and uses. - if defs_out is not None: - self._killed.append(defs_out) - defs_out = reference - continue + for child in reference.children: + if self._reference.symbol is child.symbol: + # It refers to this symbol, assume the worst and + # consider it defout and killed + if defs_out is not None: + self._killed.append(defs_out) + defs_out = child + break + elif isinstance(reference.parent, CodeBlock): + # We have already dealt with References inside Codeblocks + # above, so skip this reference + pass elif isinstance(reference, Call): # If its a local variable we can ignore it as we'll catch # the Reference later if its passed into the Call. @@ -764,16 +766,18 @@ def _compute_backward_uses(self, basic_block_list: list[Node]): "DefinitionUseChains can't handle code containing" " GOTO statements." ) - if ( - self._reference.symbol.name - in reference.get_symbol_names() - ): - # Assume the worst for a CodeBlock and we count them - # as killed and defsout and uses. - if defs_out is not None: - self._killed.append(defs_out) - defs_out = reference - continue + for child in reference.children: + if self._reference.symbol is child.symbol: + # It refers to this symbol, assume the worst and + # consider it defout and killed + if defs_out is not None: + self._killed.append(defs_out) + defs_out = child + break + elif isinstance(reference.parent, CodeBlock): + # We have already dealt with References inside Codeblocks + # above, so skip this reference + pass elif isinstance(reference, Call): # If its a local variable we can ignore it as we'll catch # the Reference later if its passed into the Call. diff --git a/src/psyclone/psyir/transformations/inline_trans.py b/src/psyclone/psyir/transformations/inline_trans.py index aefe4e1b81..36c796f37b 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -1161,7 +1161,7 @@ def validate( continue exprn = prev.ancestor(Statement, include_self=True) stmt = exprn.debug_string().strip() - if isinstance(prev, (CodeBlock, Call, Kern, Loop)): + if isinstance(prev, (Kern, Loop)): raise TransformationError( f"Cannot inline routine '{routine.name}' " f"because one or more of its declarations " diff --git a/src/psyclone/psyir/transformations/scalarisation_trans.py b/src/psyclone/psyir/transformations/scalarisation_trans.py index 25884cf246..17a6814e20 100644 --- a/src/psyclone/psyir/transformations/scalarisation_trans.py +++ b/src/psyclone/psyir/transformations/scalarisation_trans.py @@ -38,9 +38,9 @@ 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, \ - IfBlock, Loop, Node, Range, Reference, Routine, StructureReference +from psyclone.psyir.nodes import ( + Literal, IfBlock, Loop, Node, Range, Reference, Routine, + StructureReference) 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 @@ -110,12 +110,6 @@ def _is_local_array(signature: Signature, ''' if not var_accesses[signature].has_indices(): return False - # If any of the accesses are to a CodeBlock then we stop. This can - # happen if there is a string access inside a string concatenation, - # e.g. NEMO4. - for access in var_accesses[signature]: - if isinstance(access.node, CodeBlock): - return False base_symbol = var_accesses[signature][0].node.symbol if not base_symbol.is_automatic: return False @@ -307,13 +301,6 @@ def _value_unused_after_loop(sig: Signature, if has_complex_index: return False - # If next access is a Call or CodeBlock or Kern then - # we have to assume the value is used. These nodes don't - # have the is_read property that Reference has, so we need - # to be explicit. - if isinstance(next_access, (CodeBlock, Call, Kern)): - return False - # If the access is a read, then return False if next_access.is_read: return False @@ -389,6 +376,7 @@ def apply(self, node: Loop, options: Optional[Dict[str, Any]] = None) \ :param options: a dictionary with options for transformations. ''' + self.validate(node) # 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 diff --git a/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py b/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py index 9720e7a8e9..a70f5f0904 100644 --- a/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py @@ -163,7 +163,7 @@ def test_gen_param_decls_case_insensitive(fortran_reader, "integer(kind=an_int), parameter :: a_second_int = 5_i_def\n" "integer, parameter :: nfieldnames3d = 4\n" "integer, dimension(nfieldnames3d), parameter :: " - "interpolationlevels = [2, 0, HUGE(InterpolationLevels) / 3, 0]\n") + "InterpolationLevels = [2, 0, HUGE(InterpolationLevels) / 3, 0]\n") def test_gen_decls(fortran_writer): @@ -250,9 +250,9 @@ def test_gen_decls(fortran_writer): with pytest.raises(VisitorError) as excinfo: _ = fortran_writer.gen_decls(symbol_table) assert ("The following symbols are not explicitly declared or imported " - "from a module and there are no wildcard " - "imports which could be bringing them into scope: " - "'unknown'" in str(excinfo.value)) + "from a module and there are no wildcard imports, generic " + "interfaces or CodeBlocks which could be bringing them into scope:" + " 'unknown'" in str(excinfo.value)) def test_gen_decls_array(fortran_writer): @@ -300,9 +300,10 @@ def test_gen_decls_nested_scope(fortran_writer): # be brought into scope with pytest.raises(VisitorError) as err: fortran_writer.gen_decls(inner_table) - assert ("symbols are not explicitly declared or imported from a module " - "and there are no wildcard imports which " - "could be bringing them into scope: 'unknown1'" in str(err.value)) + assert ("The following symbols are not explicitly declared or imported " + "from a module and there are no wildcard imports, generic " + "interfaces or CodeBlocks which could be bringing them into scope:" + " 'unknown1'" in str(err.value)) # Add a ContainerSymbol with a wildcard import in the outermost scope csym = ContainerSymbol("other_mod") csym.wildcard_import = True diff --git a/src/psyclone/tests/psyir/backend/fortran_test.py b/src/psyclone/tests/psyir/backend/fortran_test.py index 8aeced553c..df04937681 100644 --- a/src/psyclone/tests/psyir/backend/fortran_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_test.py @@ -950,18 +950,18 @@ def test_fw_filecontainer_2(fortran_writer): def test_fw_filecontainer_error1(fortran_writer): '''Check that an instance of the FortranWriter class raises the expected exception if the symbol table associated with a - FileContainer node contains any symbols. + FileContainer node contains any data symbols. ''' symbol_table = SymbolTable() - symbol_table.add(Symbol("x")) + symbol_table.add(DataSymbol("x", INTEGER_TYPE)) file_container = FileContainer.create("None", symbol_table, []) with pytest.raises(VisitorError) as info: _ = fortran_writer(file_container) assert ( "In the Fortran backend, a file container should not have any " - "symbols associated with it other than RoutineSymbols, but found " - "x: Symbol." in str(info.value)) + "data symbols associated with it, but found x: DataSymbol" + in str(info.value)) # Check that a routine symbol is fine. symbol_table = SymbolTable() diff --git a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py index a633d769c0..35a14fefb5 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py @@ -583,8 +583,29 @@ def test_module_contains_subroutine_contains_subroutine( end function func_a end module my_mod""" psyir = fortran_reader.psyir_from_source(code) - # s symbol should not be in my_mod - with pytest.raises(KeyError): - psyir.children[0].symbol_table.lookup("s") + out = fortran_writer(psyir) assert "subroutine func_a" not in out + + # It is unclear if Codeblock represent their own scope (by definition + # we don't know what a Codeblock represents). Some of them do (e.g. + # associate blocks, unsupported functions, ...) and some don't. + + # So there is no good way to decide if symbols should be propagated to its + # parent scope or not, but it seems preferable to let the symbol escape the + # codeblock because: + # - If it is not its own scope and we don't propagate it, it won't be part + # of the symbol table, and it will be possible to create a double + # declaration with symbols with overlapping names. + # - If it is its own scope and we wrongly propate it: + # * if a symbol exist with the same name, it must be a location where + # shadowing is possible (otherwise this would be invalid Fortran) and + # it will create a false dependency. + # * if a symbol does not exist, it will unnecessary reserve the name in + # the parent symbol table. + # The consequences of propagating it are not fatal, while the ones of not + # propagating it are. + + if "s" in psyir.children[0].symbol_table: + pytest.xfail("TODO #2205: When support for Routines inside Routines" + " is added, the symbol will not be propagated") diff --git a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py index 878e94371f..c891ace55d 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py @@ -1257,55 +1257,6 @@ def test_nested_where(fortran_reader, fortran_writer, tmp_path): " 'UnresolvedType'. Consider adding the name of the file " "containing the declaration of this quantity to RESOLVE_IMPORTS.") - # If some information is provided, one of the WHEREs can be resolved, but - # the other still is a CodeBlock - code = ("module my_mod\n" - " use some_mod\n" - "contains\n" - "subroutine my_sub()\n" - " type :: mytype\n" - " real, dimension(10,10,10) :: D11\n" - " real, dimension(10,10,10) :: D12\n" - " real, dimension(10,10,10) :: D22\n" - " end type\n" - " type(mytype) :: p_dal\n" - " WHERE ( z_lenp4(:,:) <= 0.0_wp )\n" - " p_dal%D12(:,:,1) = 0.0_wp\n" - " z_lenp2(:,:) = SQRT( p_dal%D11(:,:,1) * p_dal%D22(:,:,1) )\n" - " WHERE ( z_lenp2(:,:) == 0.0_wp )\n" - " p_dal%D11(:,:,1) = p_ens%D11_min(:,:)\n" - " p_dal%D22(:,:,1) = p_ens%D22_min(:,:)\n" - " z_lenp2(:,:) = z_lenp2_min(:,:)\n" - " END WHERE\n" - " END WHERE\n" - "end subroutine my_sub\n" - "end module my_mod\n") - psyir = fortran_reader.psyir_from_source(code) - code = fortran_writer(psyir) - assert """ - do widx2 = 1, SIZE(z_lenp4, dim=2), 1 - do widx1 = 1, SIZE(z_lenp4, dim=1), 1 - if (z_lenp4(LBOUND(z_lenp4, dim=1) + widx1 - 1,LBOUND(z_lenp4, dim=2) \ -+ widx2 - 1) <= 0.0_wp) then - p_dal%D12(widx1,widx2,1) = 0.0_wp - z_lenp2(LBOUND(z_lenp2, dim=1) + widx1 - 1,LBOUND(z_lenp2, dim=2) + \ -widx2 - 1) = SQRT(p_dal%D11(widx1,widx2,1) * p_dal%D22(widx1,widx2,1)) - - ! PSyclone CodeBlock (unsupported code) reason: - ! - WHERE not supported because 'p_ens' cannot be converted to an \ -array due to: Transformation Error: The supplied node should be a Reference \ -to a symbol of known type, but 'p_ens%D11_min(:,:)' is 'UnresolvedType'. \ -Consider adding the name of the file containing the declaration of this \ -quantity to RESOLVE_IMPORTS. - WHERE (z_lenp2(:, :) == 0.0_wp) - p_dal % D11(:, :, 1) = p_ens % D11_min(:, :) - p_dal % D22(:, :, 1) = p_ens % D22_min(:, :) - z_lenp2(:, :) = z_lenp2_min(:, :) - END WHERE - end if - enddo - enddo""" in code - # If enough information is provided, both WHEREs are resolved and nested code = ("module my_mod\n" "contains\n" diff --git a/src/psyclone/tests/psyir/nodes/codeblock_test.py b/src/psyclone/tests/psyir/nodes/codeblock_test.py index bf49cda8ac..a4467244b8 100644 --- a/src/psyclone/tests/psyir/nodes/codeblock_test.py +++ b/src/psyclone/tests/psyir/nodes/codeblock_test.py @@ -41,7 +41,7 @@ import pytest from fparser.common.readfortran import FortranStringReader from psyclone.psyir.frontend.fortran import FortranReader -from psyclone.psyir.nodes import CodeBlock +from psyclone.psyir.nodes import CodeBlock, Reference, Schedule from psyclone.psyir.nodes.node import colored from psyclone.errors import GenerationError @@ -98,19 +98,20 @@ def test_codeblock_children_validation(): cblock = CodeBlock([], "dummy") with pytest.raises(GenerationError) as excinfo: cblock.addchild(CodeBlock([], "dummy2")) - assert ("Item 'CodeBlock' can't be child 0 of 'CodeBlock'. CodeBlock is a" - " LeafNode and doesn't accept children.") in str(excinfo.value) + assert ("Item 'CodeBlock' can't be child 0 of 'CodeBlock'. The valid " + "format is: '[Reference]*'.") in str(excinfo.value) -def test_codeblock_get_symbol_names(parser): +def test_codeblock_get_symbol_names_and_representative_references(parser): '''Test that the get_symbol_names methods returns the names of the symbols used inside the CodeBlock. This is slightly subtle as we have to avoid - any labels on loop and branching statements.''' + any labels and structure accessors names. Also check that this information + is used to create the appropriate symbols and representative references.''' reader = FortranStringReader(''' subroutine mytest myloop: DO i = 1, 10 a = b + sqrt(c) - myifblock: IF(this_is_true)THEN + myifblock: IF(this_is_true%really_true(nested%field)%for_real)THEN EXIT myloop ELSE IF(that_is_true)THEN myifblock write(*,*) "Bye" @@ -120,18 +121,34 @@ def test_codeblock_get_symbol_names(parser): END DO myloop end subroutine mytest''') prog = parser(reader) - block = CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) + scope = Schedule() + block = CodeBlock(prog.children, CodeBlock.Structure.STATEMENT, + parent=scope) sym_names = block.get_symbol_names() - assert "a" in sym_names - assert "b" in sym_names - assert "c" in sym_names - assert "mytest" in sym_names - assert "subroutine" not in sym_names - assert "sqrt" not in sym_names - assert "myloop" not in sym_names - assert "myifblock" not in sym_names - assert "this_is_true" in sym_names - assert "that_is_true" in sym_names + refs = block.walk(Reference) + + # Check that all refs are immediate children of the codeblock + for ref in refs: + assert ref.parent is block + + # Check strings that are symbols + for name in ['a', 'b', 'c', 'i', 'mytest', 'this_is_true', 'nested', + 'that_is_true']: + # The name is reported by get_symbol_names + assert name in sym_names + # It has been added to the scope + symbol = scope.symbol_table.lookup(name) + # There is a virtual reference to it + assert Reference(symbol) in refs + # The 8 symbols mentioned above, this also checks references to the same + # symbol are not repeated, e.g. 'mytest' + assert len(refs) == 8 + + # Check strings that are not symbols, e.g. keywords, labels, accessors + for name in ['subroutine', 'sqrt', 'myloop', 'myifblock', + 'really_true', 'for_real', 'field']: + assert name not in sym_names + assert scope.symbol_table.lookup(name, otherwise=None) is None def test_codeblock_get_symbol_names_comments_and_directives(): diff --git a/src/psyclone/tests/psyir/nodes/omp_directives_test.py b/src/psyclone/tests/psyir/nodes/omp_directives_test.py index d40de24f01..4710397d04 100644 --- a/src/psyclone/tests/psyir/nodes/omp_directives_test.py +++ b/src/psyclone/tests/psyir/nodes/omp_directives_test.py @@ -665,34 +665,41 @@ def test_infer_sharing_attributes_with_explicitly_private_symbols( assert "scalar2" in [x.name for x in fpvars] -def test_infer_sharing_attributes_with_codeblocks( - fortran_reader, fortran_writer): +@pytest.mark.parametrize("code, loop_idx", [ + (lambda x, y: f"write(*,*) {x}, {y}\n", 0), + (lambda x, y: f"do {x} = 1,2\nenddo\ndo {y} = 1,2\nenddo\n", 2) +]) +def test_infer_sharing_attributes_with_hidden_references( + code, loop_idx, fortran_reader, fortran_writer): ''' Tests the infer_sharing_attributes() method when some of the loops have - Codeblocks inside it. We check that the infer_sharing attribute analysis - succeed by assuming worst case. + potentially hidden references (e.g. inside codeblocks or loop variables). + We check that the infer_sharing attribute analysis succeed by assuming + worst case. ''' - psyir = fortran_reader.psyir_from_source(''' + psyir = fortran_reader.psyir_from_source(f''' subroutine my_subroutine() use other, only: mystruct integer :: i, j, scalar1 = 1, scalar2 = 2 real, dimension(10) :: array, array2 - write(*,*) scalar1, scalar2 + {code("scalar2", "scalar2")} do j = 1, 10 do i = 1, size(array, 1) - write(*,*) scalar2, mystruct(i)%field2 + {code("scalar2", "scalar2")} + write(*,*) mystruct(i)%field2 if (.true.) then scalar1 = 1 - write(*,*) scalar1, mystruct(i)%field1 + {code("scalar1", "scalar1")} + write(*,*) mystruct(i)%field1 end if scalar2 = scalar1 + 1 - write(*,*) scalar1, scalar2 + {code("scalar1", "scalar2")} enddo enddo - write(*,*) scalar1, scalar2 + {code("scalar1", "scalar2")} end subroutine''') omplooptrans = OMPLoopTrans() omplooptrans.omp_directive = "paralleldo" - loop = psyir.walk(Loop)[0] + loop = psyir.walk(Loop)[loop_idx] # Make sure that the write statements inside the loop are CodeBlocks, # otherwise we need a new test example assert loop.has_descendant(nodes.CodeBlock) @@ -703,7 +710,7 @@ def test_infer_sharing_attributes_with_codeblocks( # over with CodeBlocks. This will often still be defensively firstprivate # as we condiser all codeblock accesses as READWRITE. output = fortran_writer(psyir) - assert "firstprivate(scalar1,scalar2)" in output + assert "firstprivate(scalar1,scalar2)" in output, output # Add many more Codeblocks. For performance reasons this will skip the # analysis, but still return them all as firstprivate diff --git a/src/psyclone/tests/psyir/nodes/reference_test.py b/src/psyclone/tests/psyir/nodes/reference_test.py index 9658bce946..96d3948fa8 100644 --- a/src/psyclone/tests/psyir/nodes/reference_test.py +++ b/src/psyclone/tests/psyir/nodes/reference_test.py @@ -347,7 +347,7 @@ def test_reference_next_accesses_with_codeblock(fortran_reader): a = routine.children[0].lhs codeblock = routine.children[1] assert len(a.next_accesses()) == 1 - assert a.next_accesses()[0] is codeblock + assert a.next_accesses()[0] is codeblock.children[0] def test_reference_previous_accesses(fortran_reader): @@ -520,7 +520,7 @@ def test_reference_previous_accesses_with_codeblock(fortran_reader): routine = psyir.children[0] a = routine.children[1].lhs codeblock = routine.walk(CodeBlock)[0] - assert a.previous_accesses()[0] is codeblock + assert a.previous_accesses()[0].is_descendant_of(codeblock) def test_reference_replace_symbols_using(): diff --git a/src/psyclone/tests/psyir/nodes/routine_test.py b/src/psyclone/tests/psyir/nodes/routine_test.py index 1106dfdf29..ae3c99e2ae 100644 --- a/src/psyclone/tests/psyir/nodes/routine_test.py +++ b/src/psyclone/tests/psyir/nodes/routine_test.py @@ -555,29 +555,22 @@ def test_outer_scope_accesses_unresolved(fortran_reader): ''' psyir = fortran_reader.psyir_from_source('''\ module my_mod - use another_mod contains subroutine call_it() - write(*,*) unresolved() call a_routine() end subroutine call_it end module my_mod ''') rt0 = psyir.children[0].children[0] - sym = rt0.symbol_table.lookup("a_routine") - assert sym.is_unresolved - call = Call.create(RoutineSymbol("a_routine"), []) - # The access to 'unresolved' is in a CodeBlock and we don't have a - # Symbol for it. + call = rt0.children[0] + + # Mistakenly add symbols without adding them to the symbol table + rt0.addchild(Assignment.create(Reference(Symbol("a")), + Reference(Symbol("b")))) with pytest.raises(SymbolError) as err: rt0.check_outer_scope_accesses(call, "call") - assert ("'call_it' contains accesses to 'unresolved' but the origin of " + assert ("'call_it' contains accesses to 'a' but the origin of " "this" in str(err.value)) - # Remove the CodeBlock and repeat. - rt0.children[0].detach() - rt0.check_outer_scope_accesses(call, "call") - # The interface should have been left unchanged. - assert sym.is_unresolved def test_outer_scope_accesses_multi_wildcards(fortran_reader): diff --git a/src/psyclone/tests/psyir/tools/call_tree_utils_test.py b/src/psyclone/tests/psyir/tools/call_tree_utils_test.py index e6972693c9..fd0fa25e80 100644 --- a/src/psyclone/tests/psyir/tools/call_tree_utils_test.py +++ b/src/psyclone/tests/psyir/tools/call_tree_utils_test.py @@ -217,6 +217,7 @@ def test_call_tree_get_used_symbols_from_modules(): expected = set([ ("unknown", "constants_mod", "eps"), ("unknown", "module_with_var_mod", "module_const"), + ('unknown', 'module_with_var_mod', 'module_function'), ("reference", "testkern_import_symbols_mod", "dummy_module_variable"), ('routine', 'testkern_import_symbols_mod', "local_func"), @@ -226,7 +227,8 @@ def test_call_tree_get_used_symbols_from_modules(): ("routine", "testkern_import_symbols_mod", "local_subroutine"), ("routine", None, "unknown_subroutine")] ) - assert non_locals_without_access == expected + for x in non_locals_without_access: + assert x in expected, str(x) + " not found" # Check the handling of a symbol that is not found: _compute_all_non_locals # should return None: diff --git a/src/psyclone/tests/psyir/tools/definition_use_chains_backward_dependence_test.py b/src/psyclone/tests/psyir/tools/definition_use_chains_backward_dependence_test.py index d7940c0df5..4fa40448b6 100644 --- a/src/psyclone/tests/psyir/tools/definition_use_chains_backward_dependence_test.py +++ b/src/psyclone/tests/psyir/tools/definition_use_chains_backward_dependence_test.py @@ -452,7 +452,7 @@ def test_definition_use_chain_find_backward_accesses_codeblock( chains = DefinitionUseChain(routine.walk(Assignment)[1].lhs) reaches = chains.find_backward_accesses() assert len(reaches) == 1 - assert reaches[0] is routine.walk(CodeBlock)[0] + assert reaches[0] is routine.walk(CodeBlock)[0].children[0] def test_definition_use_chain_find_backward_accesses_codeblock_and_call_nlocal( @@ -521,7 +521,7 @@ def test_definition_use_chain_find_backward_accesses_codeblock_and_call_local( chains = DefinitionUseChain(routine.walk(Assignment)[0].rhs.children[0]) reaches = chains.find_backward_accesses() assert len(reaches) == 1 - assert reaches[0] is routine.walk(CodeBlock)[0] + assert reaches[0] is routine.walk(CodeBlock)[0].children[0] def test_definition_use_chain_find_backward_accesses_call_and_codeblock_nlocal( diff --git a/src/psyclone/tests/psyir/tools/definition_use_chains_forward_dependence_test.py b/src/psyclone/tests/psyir/tools/definition_use_chains_forward_dependence_test.py index d4d1d6f3c8..4f39a8f8bb 100644 --- a/src/psyclone/tests/psyir/tools/definition_use_chains_forward_dependence_test.py +++ b/src/psyclone/tests/psyir/tools/definition_use_chains_forward_dependence_test.py @@ -783,7 +783,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock( chains = DefinitionUseChain(routine.walk(Assignment)[0].lhs) reaches = chains.find_forward_accesses() assert len(reaches) == 1 - assert reaches[0] is routine.walk(CodeBlock)[0] + assert reaches[0] is routine.walk(CodeBlock)[0].children[0] def test_definition_use_chain_find_forward_accesses_codeblock_and_call_nlocal( @@ -804,7 +804,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock_and_call_nlocal( chains = DefinitionUseChain(routine.walk(Assignment)[0].lhs) reaches = chains.find_forward_accesses() assert len(reaches) == 1 - assert reaches[0] is routine.walk(CodeBlock)[0] + assert reaches[0] is routine.walk(CodeBlock)[0].children[0] def test_definition_use_chain_find_forward_accesses_codeblock_and_call_cflow( @@ -828,7 +828,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock_and_call_cflow( chains = DefinitionUseChain(routine.walk(Assignment)[0].lhs) reaches = chains.find_forward_accesses() assert len(reaches) == 2 - assert reaches[0] is routine.walk(CodeBlock)[0] + assert reaches[0] is routine.walk(CodeBlock)[0].children[0] assert reaches[1] is routine.walk(Call)[1] @@ -851,7 +851,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock_and_call_local( chains = DefinitionUseChain(routine.walk(Assignment)[0].lhs) reaches = chains.find_forward_accesses() assert len(reaches) == 1 - assert reaches[0] is routine.walk(CodeBlock)[0] + assert reaches[0] is routine.walk(CodeBlock)[0].children[0] def test_definition_use_chain_find_forward_accesses_call_and_codeblock_nlocal( diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index 68620f7a2b..8d66104e6e 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -45,7 +45,7 @@ from psyclone.psyGen import Kern from psyclone.psyir.backend.fortran import FortranWriter from psyclone.psyir.nodes import ( - Assignment, Call, CodeBlock, IntrinsicCall, Loop, Node, Reference, + Assignment, Call, IntrinsicCall, Loop, Node, Reference, Routine, Statement) from psyclone.psyir.symbols import ( AutomaticInterface, DataSymbol, ImportInterface, UnresolvedType) @@ -2623,7 +2623,8 @@ def test_validate_automatic_array_sized_by_arg(fortran_reader, monkeypatch): " ndim = 5\n" " ! A read access to ndim is fine.\n" " zdim = ndim + mdim\n" - " write(*,*) ndim\n" + " do ndim = 1, 10\n" + " enddo\n" " call sub(var, ndim, ndim)\n" "end subroutine main\n" "subroutine sub(x, ilen, jlen)\n" @@ -2644,11 +2645,10 @@ def test_validate_automatic_array_sized_by_arg(fortran_reader, monkeypatch): inline_trans.validate(call) assert ("Cannot inline routine 'sub' because one or more of its " "declarations depends on 'ilen' which is passed by argument and " - "may be written to before the call ('! PSyclone CodeBlock" + "may be written to before the call ('do ndim =" in str(err.value)) - # Remove the CodeBlock so the Assignment is found. - cblock = psyir.walk(CodeBlock)[0] - cblock.detach() + # Remove the Loop so the Assignment is found. + psyir.walk(Loop)[0].detach() with pytest.raises(TransformationError) as err: inline_trans.validate(call) assert ("Cannot inline routine 'sub' because one or more of its "