diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index 6c48a6dffd..e97c8b81d5 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -1000,14 +1000,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. @@ -1089,19 +1091,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 da2f6e1434..274e9d099c 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -3575,7 +3575,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 = ( @@ -3636,7 +3636,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 6d73da43a3..c1d59340c7 100644 --- a/src/psyclone/psyir/nodes/codeblock.py +++ b/src/psyclone/psyir/nodes/codeblock.py @@ -48,7 +48,10 @@ from psyclone.errors import InternalError 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): @@ -73,7 +76,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. @@ -111,6 +114,35 @@ def __init__( self._parse_tree_nodes = [parse_tree] # 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)) @staticmethod def create(*args, **kwargs) -> CodeBlock: @@ -184,10 +216,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 @@ -195,9 +223,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) -> str: @@ -294,12 +326,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). @@ -407,3 +453,10 @@ def get_fortran_lines(self) -> list[str]: for node in self._parse_tree_nodes: output.extend(str(node.text, encoding="utf8").split("\n")) return output + + def get_symbol_names(self) -> list[str]: + ''' + :returns: the name of all symbols accessed in the CodeBlock. + ''' + # TODO #3038 Treesitter support is incomplete + return [] diff --git a/src/psyclone/psyir/nodes/reference.py b/src/psyclone/psyir/nodes/reference.py index 718cae644d..8a40872b67 100644 --- a/src/psyclone/psyir/nodes/reference.py +++ b/src/psyclone/psyir/nodes/reference.py @@ -130,6 +130,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. @@ -144,6 +145,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 dc0f5951d3..79d1d558e9 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -589,17 +589,14 @@ def _compute_forward_uses(self, basic_block_list: list[Node]): if defs_out[sig] is not None: self._defsout[sig].append(defs_out[sig]) return - for i, ref in enumerate(self._references[:]): - if ( - ref.symbol.name - in reference.get_symbol_names() - ): - # Assume the worst for a CodeBlock and we count - # them as killed and defsout and uses. - sig = self._reference_signatures[i] - if defs_out[sig] is not None: - self._killed[sig].append(defs_out[sig]) - defs_out[sig] = reference + elif isinstance(reference.parent, CodeBlock): + for i, _ in enumerate(self._references[:]): + # Assume the worst for a CodeBlock and we count + # them as killed and defsout and uses. + sig = self._reference_signatures[i] + if defs_out[sig] is not None: + self._killed[sig].append(defs_out[sig]) + defs_out[sig] = reference 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. @@ -888,17 +885,14 @@ def _compute_backward_uses(self, basic_block_list: list[Node]): "DefinitionUseChains can't handle code containing" " GOTO statements." ) - for i, ref in enumerate(self._references[:]): - if ( - ref.symbol.name - in reference.get_symbol_names() - ): - # Assume the worst for a CodeBlock and we count - # them as killed and defsout and uses. - sig = self._reference_signatures[i] - if defs_out[sig] is not None: - self._killed[sig].append(defs_out[sig]) - defs_out[sig] = reference + elif isinstance(reference.parent, CodeBlock): + for i, _ in enumerate(self._references[:]): + # Assume the worst for a CodeBlock and we count + # them as killed and defsout and uses. + sig = self._reference_signatures[i] + if defs_out[sig] is not None: + self._killed[sig].append(defs_out[sig]) + defs_out[sig] = reference 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 904d689067..79bf05ce6f 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -1157,7 +1157,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 496d1dd786..d09b770301 100644 --- a/src/psyclone/psyir/transformations/scalarisation_trans.py +++ b/src/psyclone/psyir/transformations/scalarisation_trans.py @@ -36,9 +36,9 @@ '''This module provides the sclarization transformation class.''' 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, ScalarType 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 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 9519fb2a07..dadb8e7ce2 100644 --- a/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_gen_decls_test.py @@ -252,9 +252,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_char(fortran_writer): @@ -347,9 +347,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 b5583b6294..69765557ac 100644 --- a/src/psyclone/tests/psyir/backend/fortran_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_test.py @@ -958,18 +958,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", ScalarType.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 819cf236d0..45441c1982 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py @@ -618,12 +618,33 @@ 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") + def test_module_contains_comment_before_subroutine( fortran_writer @@ -643,7 +664,6 @@ def test_module_contains_comment_before_subroutine( fortran_reader = FortranReader(ignore_comments=False) psyir = fortran_reader.psyir_from_source(code) assert "test" in psyir.children[0].symbol_table - assert "subsub" not in psyir.children[0].symbol_table out = fortran_writer(psyir) assert """ contains ! PSyclone CodeBlock (unsupported code) reason: @@ -654,6 +674,9 @@ def test_module_contains_comment_before_subroutine( SUBROUTINE subsub END SUBROUTINE END SUBROUTINE test""" in out + if "subsub" in psyir.children[0].symbol_table: + pytest.xfail("TODO #2205: When support for Routines inside Routines" + " is added, the symbol will not be propagated") def test_module_contains_comment_before_function( @@ -675,7 +698,6 @@ def test_module_contains_comment_before_function( fortran_reader = FortranReader(ignore_comments=False) psyir = fortran_reader.psyir_from_source(code) assert "test" in psyir.children[0].symbol_table - assert "subsub" not in psyir.children[0].symbol_table out = fortran_writer(psyir) assert """ contains ! PSyclone CodeBlock (unsupported code) reason: @@ -686,3 +708,6 @@ def test_module_contains_comment_before_function( FUNCTION subsub() END FUNCTION END FUNCTION test""" in out + if "subsub" 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 6011df9f55..2c55e97fe2 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 30eb62f643..8bf3daa74a 100644 --- a/src/psyclone/tests/psyir/nodes/codeblock_test.py +++ b/src/psyclone/tests/psyir/nodes/codeblock_test.py @@ -43,6 +43,7 @@ from fparser.common.readfortran import FortranStringReader from psyclone.configuration import Config from psyclone.psyir.frontend.fortran import FortranReader +from psyclone.psyir.nodes import Reference, Schedule from psyclone.psyir.frontend.fparser2 import Fparser2Reader from psyclone.psyir.frontend.fortran_treesitter_reader import \ FortranTreeSitterReader @@ -65,6 +66,7 @@ def test_codeblock_create(): cb = CodeBlock.create("3 + 3", partial_code="expression") assert isinstance(cb, Fparser2CodeBlock) assert "3 + 3" in cb.get_fortran_lines() + assert cb.get_symbol_names() == [] cb = CodeBlock.create("a => b", partial_code="pointer_assignment") assert isinstance(cb, Fparser2CodeBlock) @@ -116,7 +118,7 @@ def test_codeblock_constructor_and_getastnodes(): ''' original = ["hello", "there"] - cblock = CodeBlock(original, CodeBlock.Structure.EXPRESSION) + cblock = Fparser2CodeBlock(original, CodeBlock.Structure.EXPRESSION) result = cblock.parse_tree_nodes assert result == original # Check that the list is a copy not a reference. @@ -124,7 +126,7 @@ def test_codeblock_constructor_and_getastnodes(): # If only one element is provided, this is added to a list original = 3 - cblock = CodeBlock(original, CodeBlock.Structure.EXPRESSION) + cblock = Fparser2CodeBlock(original, CodeBlock.Structure.EXPRESSION) assert cblock.parse_tree_nodes == [3] @@ -147,8 +149,8 @@ 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_abstract_methods(): @@ -199,15 +201,16 @@ def test_codeblock_get_fortran_lines(): assert "end subroutine" in block.get_fortran_lines() -def test_codeblock_get_symbol_names(): +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.''' - prog = Fparser2Reader().generate_parse_tree_from_source(''' + 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" @@ -216,18 +219,35 @@ def test_codeblock_get_symbol_names(): END IF myifblock END DO myloop end subroutine mytest''') - block = Fparser2CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) + prog = parser(reader) + scope = Schedule() + block = Fparser2CodeBlock(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(): @@ -318,9 +338,9 @@ def test_codeblock_equality(parser): a = b + sqrt(c) end subroutine mytest''') prog = parser(reader) - block = CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) - block2 = CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) - block3 = CodeBlock(prog.children, CodeBlock.Structure.EXPRESSION) + block = Fparser2CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) + block2 = Fparser2CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) + block3 = Fparser2CodeBlock(prog.children, CodeBlock.Structure.EXPRESSION) assert block == block2 assert block != block3 reader = FortranStringReader(''' @@ -328,7 +348,7 @@ def test_codeblock_equality(parser): a = b + c end subroutine mytest''') prog = parser(reader) - block4 = CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) + block4 = Fparser2CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) assert block != block4 diff --git a/src/psyclone/tests/psyir/nodes/omp_directives_test.py b/src/psyclone/tests/psyir/nodes/omp_directives_test.py index d10eb03642..71a88092be 100644 --- a/src/psyclone/tests/psyir/nodes/omp_directives_test.py +++ b/src/psyclone/tests/psyir/nodes/omp_directives_test.py @@ -703,34 +703,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) @@ -741,7 +748,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 d4fc23c93f..356ab36bb8 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 7123e1b1d8..5d9805b78c 100644 --- a/src/psyclone/tests/psyir/nodes/routine_test.py +++ b/src/psyclone/tests/psyir/nodes/routine_test.py @@ -564,29 +564,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 840de59c47..d8fa63e7b0 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 @@ -473,7 +473,7 @@ def test_definition_use_chain_find_backward_accesses_codeblock( chains = DefinitionUseChain(ref) reaches = chains.find_backward_accesses()[sig] 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( @@ -548,7 +548,7 @@ def test_definition_use_chain_find_backward_accesses_codeblock_and_call_local( chains = DefinitionUseChain(ref) reaches = chains.find_backward_accesses()[sig] 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 d180962673..566acb885d 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 @@ -837,7 +837,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock( chains = DefinitionUseChain(ref) reaches = chains.find_forward_accesses()[sig] 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( @@ -860,7 +860,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock_and_call_nlocal( chains = DefinitionUseChain(ref) reaches = chains.find_forward_accesses()[sig] 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( @@ -886,7 +886,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock_and_call_cflow( chains = DefinitionUseChain(ref) reaches = chains.find_forward_accesses()[sig] 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] @@ -911,7 +911,7 @@ def test_definition_use_chain_find_forward_accesses_codeblock_and_call_local( chains = DefinitionUseChain(ref) reaches = chains.find_forward_accesses()[sig] 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 63e03d88f6..40cd6478b5 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) @@ -2654,7 +2654,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" @@ -2675,11 +2676,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 " diff --git a/src/psyclone/tests/psyir/transformations/scalarisation_trans_test.py b/src/psyclone/tests/psyir/transformations/scalarisation_trans_test.py index 06e551f327..25a0f788ab 100644 --- a/src/psyclone/tests/psyir/transformations/scalarisation_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/scalarisation_trans_test.py @@ -78,13 +78,6 @@ def test_scalararizationtrans_is_local_array(fortran_reader): assert ScalarisationTrans._is_local_array(Signature("local"), var_accesses) - # Test b - the RHS of the assignment is a codeblock so we do not - # count it as a local array and invalidate it, as otherwise the - # local array test can fail. Also we can't safely transform the - # CodeBlock anyway. - assert not ScalarisationTrans._is_local_array(Signature("b"), - var_accesses) - # Test x - the return value is not classed as a local array. assert not ScalarisationTrans._is_local_array(Signature("x"), var_accesses) @@ -103,8 +96,9 @@ def test_scalararizationtrans_is_local_array(fortran_reader): lambda sig: ScalarisationTrans._is_local_array(sig, var_accesses), var_accesses) local_arrays = list(local_arrays) - assert len(local_arrays) == 1 - assert local_arrays[0].var_name == "local" + assert len(local_arrays) == 2 + assert local_arrays[0].var_name == "b" + assert local_arrays[1].var_name == "local" def test_scalarisationtrans_have_same_unmodified_index(fortran_reader):