diff --git a/doc/developer_guide/dependency.rst b/doc/developer_guide/dependency.rst index 3ae20cb50f..6abf2c2af3 100644 --- a/doc/developer_guide/dependency.rst +++ b/doc/developer_guide/dependency.rst @@ -344,7 +344,7 @@ AccessInfo The class `SingleVariableAccessInfo` uses a list of `psyclone.core.AccessInfo` instances to store all accesses to a single variable. A new instance of `AccessInfo` -is appended to the list whenever `add_access_with_location()` +is appended to the list whenever `add_access()` is called. .. autoclass:: psyclone.core.AccessInfo @@ -446,51 +446,6 @@ wrapped in an outer loop over all accesses. Index 'i' is used. -Access Location ---------------- - -Variable accesses are stored in the order in which they happen. For example, -an assignment `a=a+1` will store two access for the variable `a`, the -first one being a READ access, followed by a WRITE access, since this is the -order in which the accesses are executed. -Additionally, the function `reference_accesses()` keeps track of the location -at which the accesses happen. A location is an integer number, starting with 0, -which is increased for each new statement. This makes it possible to -compare accesses to variables: if two accesses have the same location value, -it means the accesses happen in the same statement, for example `a=a+1`: -the READ and WRITE access to `a` will have the same location number. If on the -other hand the accesses happen in two separate statements, e.g. `a=b+1; c=a+1` -then the first access to `a` (and the access to `b`) will have a smaller -location number than the second access to `a` (and the access to `c`). -If two statements have consecutive locations, this does not necessarily mean -that the statements are executed one after another. For example in if-statements -the statements in the if-body are counted first, then the statements in the -else-body. It is the responsibility of the user to handle these cases - for -example by creating separate `VariablesAccessMap` for statements in the if-body -and for the else-body. - -.. note:: When using different instances for an if- and else-body, the first - statement of the if-body will - have the same location number as the first statement of the else-body. So - you can only compare location numbers from the same `VariablesAccessMap` - instance. If you merge two instances together, the locations of the merged-in - instance will be appropriately increased to follow the locations of the - instance to which it is merged. - - -The location number is not exactly a line number - several statements can be -on one line, which will get different location numbers. And certain lines -will not have a location number (e.g. comment lines). - -As stated above, one instance of `VariablesAccessMap` can be extended by adding -additional variable information. It is the responsibility of the user to make -sure the accesses are added in the right order - the `VariablesAccessMap` object -will always assume accesses happen at the current location, and a call to -`next_location()` is required (internally) to increase the location number. - -.. note:: It is not possible to add access information about an earlier - usage to an existing `VariablesAccessMap` object. - Access Examples --------------- diff --git a/src/psyclone/core/single_variable_access_info.py b/src/psyclone/core/single_variable_access_info.py index 701d95c42a..06b05dfa0a 100644 --- a/src/psyclone/core/single_variable_access_info.py +++ b/src/psyclone/core/single_variable_access_info.py @@ -40,38 +40,31 @@ '''This module provides management of variable access information.''' +from __future__ import annotations +from typing import TYPE_CHECKING, Optional + from psyclone.core.access_type import AccessType from psyclone.core.component_indices import ComponentIndices from psyclone.errors import InternalError +if TYPE_CHECKING: # pragma: no cover + from psyclone.psyir.nodes import Node class AccessInfo(): - '''This class stores information about a single access - pattern of one variable (e.g. variable is read at a certain location). - A location is a number which can be used to compare different accesses - (i.e. if one access happens before another). Each consecutive - location will have an increasing location number, but read and write - accesses in the same statement will have the same location number. - If the variable accessed is an array, this class will also store - the indices used in the access. - Note that the name of the variable is not stored in this class. - It is a helper class used in the `SingleVariableAccessInfo` class, - which stores all `AccessInfo` objects for a variable, and it stores - the name of the variable. + ''' This class stores information about an access to a variable (the node + where it happens and the type of access, and the index accessed if + available). :param access: the access type. - :type access_type: :py:class:`psyclone.core.access_type.AccessType` - :param int location: a number used in ordering the accesses. :param node: Node in PSyIR in which the access happens. - :type node: :py:class:`psyclone.psyir.nodes.Node` :param component_indices: indices used in the access, defaults to None. - :type component_indices: None, [], a list or a list of lists of \ - :py:class:`psyclone.psyir.nodes.Node` objects, or an object of type \ - :py:class:`psyclone.core.component_indices.ComponentIndices` ''' - def __init__(self, access_type, location, node, component_indices=None): - self._location = location + def __init__( + self, access_type: AccessType, node: 'Node', + component_indices: Optional[list[list['Node']] | + ComponentIndices] = None + ): self._access_type = access_type self._node = node if not isinstance(component_indices, ComponentIndices): @@ -80,9 +73,7 @@ def __init__(self, access_type, location, node, component_indices=None): self.component_indices = component_indices def __str__(self): - '''Returns a string representation showing the access mode - and location, e.g.: WRITE(5).''' - return f"{self._access_type}({self._location})" + return f"{self._access_type}" def change_read_to_write(self): '''This changes the access mode from READ to WRITE. @@ -114,7 +105,7 @@ def component_indices(self): return self._component_indices @component_indices.setter - def component_indices(self, component_indices): + def component_indices(self, component_indices: ComponentIndices): '''Sets the indices for this AccessInfo instance. The component_indices contains a list of indices for each component of the signature, e.g. for `a(i)%b(j,k)%c` the component_indices will be @@ -122,8 +113,6 @@ def component_indices(self, component_indices): index expression). :param component_indices: indices used in the access. - :type component_indices: \ - :py:class:`psyclone.core.component_indices.ComponentIndices` :raises InternalError: if component_indices is not an instance \ of :py:class:`psyclone.core.component_indices.ComponentIndices`. @@ -160,14 +149,6 @@ def is_data_access(self) -> bool: ''' return self._access_type not in AccessType.non_data_accesses() - @property - def location(self): - ''':returns: the location information for this access.\ - Please see the Developers' Guide for more information. - :rtype: int - ''' - return self._location - @property def node(self): ''':returns: the PSyIR node at which this access happens. @@ -205,21 +186,15 @@ class SingleVariableAccessInfo(): def __init__(self, signature): self._signature = signature # This is the list of AccessInfo instances for this variable. - self._accesses = [] def __str__(self): '''Returns a string representation of this object with the format: - var_name:WRITE(2),WRITE(3),READ(5) where the numbers indicate - the 'location' of the corresponding access. The location is an - integer number that enumerates each statement in a program unit, - and can be used to compare if an access is earlier, later or in - the same statement as another access. - + var_name:[WRITE,WRITE,READ] ''' all_accesses = ",".join([str(access) for access in self._accesses]) - return f"{self._signature}:{all_accesses}" + return f"{self._signature}:[{all_accesses}]" def __repr__(self): return ",".join([str(access) for access in self._accesses]) @@ -337,24 +312,19 @@ def all_write_accesses(self): return [access for access in self._accesses if access.access_type in AccessType.all_write_accesses()] - def add_access_with_location(self, access_type, location, node, - component_indices): + def add_access( + self, access_type: AccessType, node: 'Node', + component_indices: Optional[list[list['Node']] | + ComponentIndices] = None + ): '''Adds access information to this variable. :param access_type: the type of access (READ, WRITE, ....) - :type access_type: \ - :py:class:`psyclone.core.access_type.AccessType` - :param location: location information - :type location: int :param node: Node in PSyIR in which the access happens. - :type node: :py:class:`psyclone.psyir.nodes.Node` :param component_indices: indices used for each component of the \ access. - :type component_indices: \ - :py:class:`psyclone.core.component_indices.ComponentIndices` ''' - self._accesses.append(AccessInfo(access_type, location, node, - component_indices)) + self._accesses.append(AccessInfo(access_type, node, component_indices)) def change_read_to_write(self): '''This function is only used when analysing an assignment statement. @@ -425,83 +395,6 @@ def is_array(self, index_variable=None): # The index variable is not used in any index in any access: return False - def is_written_before(self, reference): - '''Returns True if this variable is written before the specified - reference, and False if not. - - :param reference: the reference at which to stop for access checks. - :type reference: :py:class:`psyclone.psyir.nodes.Reference` - - :returns: True if this variable is written before the specified \ - reference, and False if not. - :rtype: bool - - :raises ValueError: if the specified reference is not in the list of \ - all accesses. - - ''' - result = False - - for access in self._accesses: - if access.node is reference: - return result - if access.access_type == AccessType.WRITE: - result = True - raise ValueError(f"Reference not found in 'is_written_before' for " - f"variable '{self.var_name}'.") - - def is_read_before(self, reference): - '''Returns True if this variable is read before the specified - reference, and False if not. - - :param reference: the reference at which to stop for access checks. - :type reference: :py:class:`psyclone.psyir.nodes.Reference` - - :returns: True if this variable is read before the specified \ - reference, and False if not. - :rtype: bool - - :raises ValueError: if the specified reference is not in the list of \ - all accesses. - - ''' - result = False - - for access in self._accesses: - if access.node is reference: - return result - if access.access_type == AccessType.READ: - result = True - raise ValueError(f"Reference not found in 'is_read_before' for " - f"variable '{self.var_name}'.") - - def is_accessed_before(self, reference): - '''Returns True if this variable is accessed before the specified - reference, and False if not. This is equivalent to testing that - 'reference' is the very first access, but this function will also - verify that 'reference' is indeed in the list of accesses. - - :param reference: the reference at which to stop for access checks. - :type reference: :py:class:`psyclone.psyir.nodes.Reference` - - :returns: True if this variable is read before the specified \ - reference, and False if not. - :rtype: bool - - :raises ValueError: if the specified reference is not in the list of \ - all accesses. - - ''' - - result = False - - for access in self._accesses: - if access.node is reference: - return result - result = True - raise ValueError(f"Reference not found in 'is_accessed_before' for " - f"variable '{self.var_name}'.") - # ---------- Documentation utils -------------------------------------------- # # The list of module members that we wish AutoAPI to generate diff --git a/src/psyclone/core/variables_access_map.py b/src/psyclone/core/variables_access_map.py index 5bba1f3669..e05c0b33e4 100644 --- a/src/psyclone/core/variables_access_map.py +++ b/src/psyclone/core/variables_access_map.py @@ -50,17 +50,10 @@ class VariablesAccessMap(dict): ''' This dictionary stores `SingleVariableAccessInfo` instances indexed by - their signature. It also maintains 'location' information, which is an - integer number that is increased for each new statement. It can be used to - easily determine if one access is syntactically before another. + their signature. ''' - def __init__(self): - super().__init__() - # Stores the current location information - self._location = 0 - def __str__(self): '''Gives a shortened visual representation of all variables and their access mode. The output is one of: READ, WRITE, READ+WRITE, @@ -97,20 +90,6 @@ def __str__(self): output_list.append(f"{signature}: {mode}") return ", ".join(output_list) - @property - def location(self): - '''Returns the current location of this instance, which is - the location at which the next accesses will be stored. - See the Developers' Guide for more information. - - :returns: the current location of this object. - :rtype: int''' - return self._location - - def next_location(self): - '''Increases the location number.''' - self._location = self._location + 1 - def add_access(self, signature, access_type, node, component_indices=None): '''Adds access information for the variable with the given signature. If the `component_indices` parameter is not an instance of @@ -172,13 +151,10 @@ def add_access(self, signature, access_type, node, component_indices=None): f"requires {len(signature)} elements.") if signature in self: - self[signature].add_access_with_location(access_type, - self._location, node, - component_indices) + self[signature].add_access(access_type, node, component_indices) else: var_info = SingleVariableAccessInfo(signature) - var_info.add_access_with_location(access_type, self._location, - node, component_indices) + var_info.add_access(access_type, node, component_indices) self[signature] = var_info @property @@ -206,9 +182,7 @@ def all_data_accesses(self) -> List[Signature]: def update(self, other_access_info): ''' Updates this dictionary with the entries in the provided VariablesAccessMap. If there are repeated signatures, the provided - values are appeneded to the existing sequence of accesses. The - 'location' property of the provided accesses (values in the dictionary) - will be updated to be after the existing entries. + values are appended to the existing sequence of accesses. :param other_access_info: the other VariablesAccessMap instance. :type other_access_info: :py:class:`psyclone.core.VariablesAccessMap` @@ -217,22 +191,15 @@ def update(self, other_access_info): for signature in other_access_info.all_signatures: var_info = other_access_info[signature] for access_info in var_info.all_accesses: - new_location = access_info.location + self._location if signature in self: var_info = self[signature] else: var_info = SingleVariableAccessInfo(signature) self[signature] = var_info - var_info.add_access_with_location(access_info.access_type, - new_location, - access_info.node, - access_info. - component_indices) - # Increase the current location of this instance by the amount of - # locations just merged in - # pylint: disable=protected-access - self._location = self._location + other_access_info._location + var_info.add_access(access_info.access_type, + access_info.node, + access_info.component_indices) def is_called(self, signature: Signature) -> bool: ''' diff --git a/src/psyclone/domain/lfric/lfric_builtins.py b/src/psyclone/domain/lfric/lfric_builtins.py index 362bf7c1ba..a925d47028 100644 --- a/src/psyclone/domain/lfric/lfric_builtins.py +++ b/src/psyclone/domain/lfric/lfric_builtins.py @@ -251,9 +251,6 @@ def reference_accesses(self) -> VariablesAccessMap: var_accesses.add_access(Signature(name), arg.access, self) # Now merge the write access to the end of all other accesses: var_accesses.update(written) - # Forward location pointer to next index, since this built-in kernel - # finishes a statement - var_accesses.next_location() return var_accesses def load(self, call, parent=None): diff --git a/src/psyclone/domain/lfric/lfric_kern.py b/src/psyclone/domain/lfric/lfric_kern.py index 79fd71f3fc..245f84c80e 100644 --- a/src/psyclone/domain/lfric/lfric_kern.py +++ b/src/psyclone/domain/lfric/lfric_kern.py @@ -151,9 +151,6 @@ def reference_accesses(self) -> VariablesAccessMap: create_arg_list.generate(var_accesses) var_accesses.update(super().reference_accesses()) - # Set the current location index to the next location, since after - # this kernel a new statement starts. - var_accesses.next_location() return var_accesses def load(self, call, parent=None): diff --git a/src/psyclone/gocean1p0.py b/src/psyclone/gocean1p0.py index b16308804c..f1910f1c99 100644 --- a/src/psyclone/gocean1p0.py +++ b/src/psyclone/gocean1p0.py @@ -1067,7 +1067,6 @@ def reference_accesses(self) -> VariablesAccessMap: self, [Reference(symbol_i), Reference(symbol_j)]) var_accesses.update(super().reference_accesses()) - var_accesses.next_location() return var_accesses @property diff --git a/src/psyclone/psyGen.py b/src/psyclone/psyGen.py index 1682e7142a..aff4abbc5e 100644 --- a/src/psyclone/psyGen.py +++ b/src/psyclone/psyGen.py @@ -56,7 +56,7 @@ from psyclone.utils import stringify_annotation from psyclone.configuration import Config, LFRIC_API_NAMES, GOCEAN_API_NAMES -from psyclone.core import AccessType, VariablesAccessMap +from psyclone.core import AccessType from psyclone.errors import GenerationError, InternalError, FieldNotFoundError from psyclone.parse.algorithm import BuiltInCall from psyclone.psyir.backend.fortran import FortranWriter @@ -1019,18 +1019,6 @@ def node_str(self, colour=True): return (self.coloured_name(colour) + " " + self.name + "(" + self.arguments.names + ")") - def reference_accesses(self) -> VariablesAccessMap: - ''' - :returns: a map of all the symbol accessed inside this node, the - keys are Signatures (unique identifiers to a symbol and its - structure acccessors) and the values are SingleVariableAccessInfo - (a sequence of AccessTypes). - - ''' - var_accesses = super().reference_accesses() - var_accesses.next_location() - return var_accesses - @property def is_reduction(self): ''' diff --git a/src/psyclone/psyir/nodes/assignment.py b/src/psyclone/psyir/nodes/assignment.py index 23c7bff764..2c39b9c9c4 100644 --- a/src/psyclone/psyir/nodes/assignment.py +++ b/src/psyclone/psyir/nodes/assignment.py @@ -209,7 +209,6 @@ def reference_accesses(self) -> VariablesAccessMap: # location otherwise, but the order is still important) rhs_accesses = self.rhs.reference_accesses() rhs_accesses.update(lhs_accesses) - rhs_accesses.next_location() return rhs_accesses @property diff --git a/src/psyclone/psyir/nodes/call.py b/src/psyclone/psyir/nodes/call.py index 79169d4f79..6e561fb97d 100644 --- a/src/psyclone/psyir/nodes/call.py +++ b/src/psyclone/psyir/nodes/call.py @@ -340,8 +340,6 @@ def reference_accesses(self) -> VariablesAccessMap: # an impure routine in which case any arguments to that Call # will have READWRITE access.) var_accesses.update(arg.reference_accesses()) - # Make sure that the next statement will be on the next location - var_accesses.next_location() return var_accesses @property diff --git a/src/psyclone/psyir/nodes/if_block.py b/src/psyclone/psyir/nodes/if_block.py index 4a7acf7cea..011b30024f 100644 --- a/src/psyclone/psyir/nodes/if_block.py +++ b/src/psyclone/psyir/nodes/if_block.py @@ -190,11 +190,8 @@ def reference_accesses(self) -> VariablesAccessMap: ''' var_accesses = self.condition.reference_accesses() - var_accesses.next_location() var_accesses.update(self.if_body.reference_accesses()) - var_accesses.next_location() if self.else_body: var_accesses.update(self.else_body.reference_accesses()) - var_accesses.next_location() return var_accesses diff --git a/src/psyclone/psyir/nodes/loop.py b/src/psyclone/psyir/nodes/loop.py index 0e087a996b..bc1c723331 100644 --- a/src/psyclone/psyir/nodes/loop.py +++ b/src/psyclone/psyir/nodes/loop.py @@ -509,11 +509,9 @@ def reference_accesses(self) -> VariablesAccessMap: var_accesses.update(self.start_expr.reference_accesses()) var_accesses.update(self.stop_expr.reference_accesses()) var_accesses.update(self.step_expr.reference_accesses()) - var_accesses.next_location() for child in self.loop_body.children: var_accesses.update(child.reference_accesses()) - var_accesses.next_location() return var_accesses def independent_iterations(self, diff --git a/src/psyclone/psyir/nodes/while_loop.py b/src/psyclone/psyir/nodes/while_loop.py index 79b9f247ca..efc9941124 100644 --- a/src/psyclone/psyir/nodes/while_loop.py +++ b/src/psyclone/psyir/nodes/while_loop.py @@ -152,7 +152,5 @@ def reference_accesses(self) -> VariablesAccessMap: ''' # The first child is the loop condition - all variables are read-only var_accesses = self.condition.reference_accesses() - var_accesses.next_location() var_accesses.update(self.loop_body.reference_accesses()) - var_accesses.next_location() return var_accesses diff --git a/src/psyclone/psyir/transformations/hoist_trans.py b/src/psyclone/psyir/transformations/hoist_trans.py index 41ae1de084..dcd85421d7 100644 --- a/src/psyclone/psyir/transformations/hoist_trans.py +++ b/src/psyclone/psyir/transformations/hoist_trans.py @@ -46,6 +46,7 @@ Loop, Assignment, Schedule, Call, CodeBlock) from psyclone.psyir.transformations.transformation_error \ import TransformationError +from psyclone.psyir.tools.definition_use_chains import DefinitionUseChain class HoistTrans(Transformation): @@ -226,13 +227,15 @@ def _validate_dependencies(self, statement, parent_loop): f"('{written_sig}') that is both " f"read and written.") - # Check if the variable is written or read before the first - # access in the statement to be hoisted: + # Check if any of the written variables could be used inside the + # loop before the statement that we are hoisting, this could be + # after the statement if there are conditional control flows. written_node = accesses_in_statement[0].node - # Get all access to that variable in the whole loop before the - # first write access that is to be hoisted: accesses_in_loop = all_loop_vars[written_sig] - if accesses_in_loop.is_accessed_before(written_node): + chains = DefinitionUseChain( + written_node, parent_loop.children[:] + ) + if chains.find_backward_accesses(): code = statement.debug_string().strip() raise TransformationError(f"The statement '{code}' can't be " f"hoisted as variable " diff --git a/src/psyclone/tests/core/single_variable_access_info_test.py b/src/psyclone/tests/core/single_variable_access_info_test.py index 31832b07f1..1f58e5536b 100644 --- a/src/psyclone/tests/core/single_variable_access_info_test.py +++ b/src/psyclone/tests/core/single_variable_access_info_test.py @@ -51,15 +51,13 @@ def test_access_info(): '''Test the AccessInfo class. ''' - location = 12 - access_info = AccessInfo(AccessType.READ, location, Node()) + access_info = AccessInfo(AccessType.READ, Node()) assert access_info.access_type == AccessType.READ - assert access_info.location == location assert access_info.component_indices.indices_lists == [[]] assert not access_info.is_array() - assert str(access_info) == "READ(12)" + assert str(access_info) == "READ" access_info.change_read_to_write() - assert str(access_info) == "WRITE(12)" + assert str(access_info) == "WRITE" assert access_info.access_type == AccessType.WRITE with pytest.raises(InternalError) as err: access_info.change_read_to_write() @@ -72,21 +70,18 @@ def test_access_info(): assert access_info.component_indices == component_indices assert access_info.is_array() - access_info = AccessInfo(AccessType.UNKNOWN, location, Node()) + access_info = AccessInfo(AccessType.UNKNOWN, Node()) assert access_info.access_type == AccessType.UNKNOWN - assert access_info.location == location assert access_info.component_indices.indices_lists == [[]] - access_info = AccessInfo(AccessType.UNKNOWN, location, Node(), + access_info = AccessInfo(AccessType.UNKNOWN, Node(), [["i", "j"]]) assert access_info.access_type == AccessType.UNKNOWN - assert access_info.location == location assert access_info.component_indices.indices_lists == [["i", "j"]] - access_info = AccessInfo(AccessType.UNKNOWN, location, Node(), + access_info = AccessInfo(AccessType.UNKNOWN, Node(), ComponentIndices([["i", "j"]])) assert access_info.access_type == AccessType.UNKNOWN - assert access_info.location == location assert access_info.component_indices.indices_lists == [["i", "j"]] @@ -94,21 +89,17 @@ def test_access_info(): def test_access_info_exceptions(): '''Test that the right exceptions are raised. ''' - location = 12 with pytest.raises(InternalError) as err: - _ = AccessInfo(AccessType.READ, location, Node(), - component_indices=123) + _ = AccessInfo(AccessType.READ, Node(), component_indices=123) assert "Index object in ComponentIndices constructor must be None, " \ "a list or list of lists, got '123'" in str(err.value) with pytest.raises(InternalError) as err: - _ = AccessInfo(AccessType.READ, location, Node(), - component_indices=[[], 123]) + _ = AccessInfo(AccessType.READ, Node(), component_indices=[[], 123]) assert "ComponentIndices: Invalid list parameter '[[], 123]'" \ in str(err.value) - location = 1 - access_info = AccessInfo(AccessType.READ, location, Node()) + access_info = AccessInfo(AccessType.READ, Node()) with pytest.raises(InternalError) as err: access_info.component_indices = 123 assert "The component_indices object in the setter of AccessInfo must " \ @@ -119,17 +110,16 @@ def test_access_info_description(): ''' Test for the description() method of AccessInfo. ''' - location = 12 - # When associated location is not a Statement. - ainfo = AccessInfo(AccessType.READ, location, Node()) + # When associated node is not a Statement. + ainfo = AccessInfo(AccessType.READ, Node()) assert "< node[] >" in ainfo.description.lower() # When it is a Statement. - ainfo = AccessInfo(AccessType.READ, location, Return()) + ainfo = AccessInfo(AccessType.READ, Return()) assert "return" in ainfo.description.lower() # When it is a Symbol. osym = Symbol("something") asym = DataSymbol("test", INTEGER_TYPE, initial_value=Reference(osym)) - ainfo = AccessInfo(AccessType.INQUIRY, location, asym) + ainfo = AccessInfo(AccessType.INQUIRY, asym) assert ("definition of symbol 'test: datasymbol