From 097433e99a2f1ee67ce05eeb3a07630778aa69bd Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Thu, 14 Nov 2024 12:00:18 +0000 Subject: [PATCH 01/12] Initial implementation of part of the tests and functionality for backward accesses --- .../psyir/tools/definition_use_chains.py | 147 ++++ ...ion_use_chains_backward_dependence_test.py | 651 ++++++++++++++++++ 2 files changed, 798 insertions(+) create mode 100644 src/psyclone/tests/psyir/tools/definition_use_chains_backward_dependence_test.py diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index ee8fd7402c..0aff78b174 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -621,3 +621,150 @@ def _find_basic_blocks(self, nodelist): basic_blocks.append(current_block) control_flow_nodes.append(None) return control_flow_nodes, basic_blocks + + def _compute_backward_uses(self, basic_block_list): + """ + Compute the backward uses for self._reference for the + basic_block_list provided. This function will not work + correctly if there is control flow inside the + basic_block_list provided. + The basic_block_list will be reversed to find the backward + accesses. + Reads to the reference that occur before a write will + be added to the self._uses array, the earliest write will + be provided as self._defsout and all previous writes + will be inside self._killed. + + :param basic_block_list: The list of nodes that make up the basic + block to find the forward uses in. + :type basic_block_list: list[:py:class:`psyclone.psyir.nodes.Node`] + + :raises NotImplementedError: If a GOTO statement is found in the code + region. + """ + sig, _ = self._reference.get_signature_and_indices() + # For a basic block we will only ever have one defsout + defs_out = None + # Working backwards so reverse the basic_block_list + basic_block_list.reverse() + for region in basic_block_list: + region_list = region.walk((Reference, Call, CodeBlock, Return)) + # If the regoin contains any Return, Exit or Cycle statements then + # we modify the stop position to only look at statements that + # occur before this statement. + # FIXME: This doesn't work correctly if the Reference that + # is having its backwards dependencies analysed occurs after + # one of these such statements in a basic block, however + # since they're unreachable maybe we don't care? + stop_position = self._stop_point + for reference in region_list: + if isinstance(reference, Return): + stop_position = reference.abs_position + if isinstance(reference, CodeBlock): + if isinstance(reference._fp2_nodes[0], (Exit_Stmt, + Cycle_Stmt)): + stop_position = reference.abs_position + # Reverse the list + region_list.reverse() + for reference in region_list: + # Store the position instead of computing it twice. + abs_pos = reference.abs_position + # TODO Fix start and stop positions + if abs_pos <= self._start_point or abs_pos >= stop_position: + continue + if isinstance(reference, Return): + # We can ignore Return statements as we only check for + # nodes that occur before them. + continue + if isinstance(reference, CodeBlock): + # CodeBlocks only find symbols, so we can only do as good + # as checking the symbol - this means we can get false + # positives for structure accesses inside CodeBlocks. + if isinstance(reference._fp2_nodes[0], Goto_Stmt): + raise NotImplementedError("DefinitionUseChains can't " + "handle code containing GOTO" + " statements.") + # If we find an Exit or Cycle statement, only check for + # nodes that occur before them so we can skip. + if isinstance(reference._fp2_nodes[0], (Exit_Stmt, + Cycle_Stmt)): + continue + 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 + 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. + if self._reference.symbol.is_automatic: + continue + if isinstance(reference, IntrinsicCall): + # IntrinsicCall can only do stuff to arguments, these + # will be caught by Reference walk already. + # Note that this assumes two symbols are not + # aliases of each other. + continue + # For now just assume calls are bad if we have a non-local + # variable and we treat them as though they were a write. + if defs_out is not None: + self._killed.append(defs_out) + defs_out = reference + continue + elif reference.get_signature_and_indices()[0] == sig: + # Work out if its read only or not. + assign = reference.ancestor(Assignment) + # TODO Need to invert how we think about assignments. + # RHS reads occur "before" LHS writes, so if we + # hit the LHS or an assignment then we won't have + # a dependency to the value used from the LHS. + if assign is not None: + if assign.lhs is reference: + # This is a write to the reference, so kill the + # previous defs_out and set this to be the + # defs_out. + print("OK") + if defs_out is not None: + self._killed.append(defs_out) + defs_out = reference + elif ( + assign.lhs.get_signature_and_indices()[0] + == sig + and assign.lhs is not self._reference + ): + # Reference is on the rhs of an assignment such as + # a = a + 1. Since we're looping through the tree + # walk in reverse, we find the a on the RHS of the + # statement before the a on the LHS. Since the LHS + # of the statement is a write to this symbol, the + # RHS needs to not be a dependency when working + # backwards. + continue + else: + # Read only, so if we've not yet set written to + # this variable this is a use. NB. We need to + # check the if the write is the LHS of the parent + # assignment and if so check if we killed any + # previous assignments. + if defs_out is None: + self._uses.append(reference) + elif reference.ancestor(Call): + # It has a Call ancestor so assume read/write access + # for now. + # We can do better for IntrinsicCalls realistically. + if defs_out is not None: + self._killed.append(defs_out) + defs_out = reference + else: + # Reference outside an Assignment - read only + # This could be References inside a While loop + # condition for example. + if defs_out is None: + self._uses.append(reference) + if defs_out is not None: + self._defsout.append(defs_out) 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 new file mode 100644 index 0000000000..971833812c --- /dev/null +++ b/src/psyclone/tests/psyir/tools/definition_use_chains_backward_dependence_test.py @@ -0,0 +1,651 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2024-2024, Science and Technology Facilities Council. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# +# * Redistributions of source code must retain the above copyright notice, this +# list of conditions and the following disclaimer. +# +# * Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# +# * Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +# FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +# COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +# INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +# BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +# ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +# POSSIBILITY OF SUCH DAMAGE. +# ----------------------------------------------------------------------------- +# Author: A. B. G. Chalk, STFC Daresbury Lab +# ----------------------------------------------------------------------------- +'''This module contains the tests for the DefinitionUseChain class's +backward_accesses routine.''' + +import pytest +from psyclone.psyir.nodes import ( + Routine, + Reference, + Assignment, + WhileLoop, +) +from psyclone.psyir.tools.definition_use_chains import DefinitionUseChain + + +def test_definition_use_chain_compute_backward_uses(fortran_reader): + """ Test the _compute_backward_uses functionality.""" + + # First test is a simple Reference with a following read. + code = """ + subroutine x() + integer :: a, b + a = a + 1 + b = a + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + a_3 = psyir.walk(Reference)[3] + # Check this is the lhs of the assignment + assert a_3 is psyir.walk(Assignment)[1].rhs + + duc = DefinitionUseChain( + a_3, control_flow_region=[routine] + ) + basic_block_list = routine.children[:] + # Need to set the start point and stop points similar to what + # backward_accesses would do + duc._start_point = routine.children[0].abs_position + duc._stop_point = a_3.abs_position-1 + duc._compute_backward_uses(basic_block_list) + assert len(duc.defsout) == 1 + assert duc.defsout[0] is psyir.walk(Reference)[0] # The lhs of a = a + 1 + + # Next we test a Reference with a write then a read - we should only get + # the write, which should be in uses and defsout. + code = """ + subroutine x() + integer :: a, b, c + c = a + a = 2 + b = a + end subroutine""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + a_3 = psyir.walk(Reference)[4] + + duc = DefinitionUseChain( + a_3, control_flow_region=[routine] + ) + basic_block_list = routine.children[:] + # Need to set the start point and stop points similar to what + # backward_accesses would do + duc._start_point = routine.children[0].abs_position + duc._stop_point = a_3.abs_position - 1 + duc._compute_backward_uses(basic_block_list) + assert len(duc.uses) == 0 + assert len(duc.defsout) == 1 + assert len(duc.killed) == 0 + assert duc.defsout[0] is psyir.walk(Reference)[2] # The lhs of a = 2 + + +def test_definition_use_chain_find_backward_accesses_basic_example( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the basic functionality of the routine.""" + + code = """ +subroutine foo(a, b) +real, intent(inout) :: a +real, intent(inout) :: b +real :: c, d, e, f +c = a + 1.0 +e = a**2 +f = cos(e) +c = d * a +d = c + 2.0 +b = c + d +call bar(c, b) +b = b + c +e = a**3 +a = 2 +end subroutine foo +subroutine bar(x, y) +real, intent(in) :: x +real, intent(inout) :: y +x = x + 1.0 +y = exp(x**2) +end subroutine bar +""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Creating use chain for the a in a = 2 + chains = DefinitionUseChain( + routine.children[9].lhs, [routine] + ) + reaches = chains.find_backward_accesses() + # We find 2 results + # the a in e = a**3 + # The call bar(c, b) as a isn't local and we can't guarantee its behaviour. + assert len(reaches) == 2 + assert reaches[0] is routine.children[8].rhs.children[0] + assert reaches[2] is routine.children[6] + + # Create use chain for c in b = c + d + chains = DefinitionUseChain(routine.children[5].rhs.children[0], [routine]) + reaches = chains.find_backward_accesses() + # We should find 2 results + # C = d * a + # d = C + 2.0 + assert reaches[0] is routine.children[4].rhs.children[0] + assert reaches[1] is routine.children[3].lhs + assert len(reaches) == 2 + + +def test_definition_use_chain_find_backward_accesses_assignment( + fortran_reader, +): + + code = """ + subroutine x() + integer :: a + a = 1 + a = a * a + end subroutine + """ + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start chain from A = a * a + chains = DefinitionUseChain(routine.children[0].lhs) + reaches = chains.find_backward_accesses() + # We should find 3 results, both 3 references in + # a = A * A + # and A = 1 + assert len(reaches) == 3 + assert reaches[0] is routine.children[1].rhs.children[0] + assert reaches[1] is routine.children[1].rhs.children[1] + assert reaches[2] is routine.children[0].lhs + + +def test_definition_use_chain_find_backward_accesses_ifelse_example( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour when there is an if/else block.""" + + code = """ + subroutine x() + integer :: a, b, c, d, e, f + a = 1 + b = a + c + if ( d > e) then + a = 3 + else + a = 4 + end if + b = a + d + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from b = A + d. + chains = DefinitionUseChain(routine.children[3].rhs.children[0]) + reaches = chains.find_backward_accesses() + # TODO #2760 For now the if statement doesn't kill the accesses, + # even though it will always be written to. + assert len(reaches) == 4 + assert reaches[2] is routine.children[1].rhs.children[0] + assert reaches[0] is routine.children[2].if_body.children[0].lhs + assert reaches[1] is routine.children[2].else_body.children[0].lhs + assert reaches[3] is routine.children[0].lhs + + # Also check that a = 4 backward access is not a = 3. + a_3 = routine.children[2].if_body.children[0].lhs + a_4 = routine.children[2].else_body.children[0].rhs + chains = DefinitionUseChain(a_4) + reaches = chains.find_backward_accesses() + assert len(reaches) == 2 + assert reaches[0] is not a_3 + assert reaches[1] is not a_3 + + +def test_definition_use_chain_find_backward_accesses_loop_example( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour when there is a loop.""" + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + + a = 1 + do i = 1, 100 + a = a + i + b = a + 2 + end do + c = a + b + end subroutine x""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from A = a + i. + chains = DefinitionUseChain( + routine.children[1].loop_body.children[1].rhs.children[0] + ) + reaches = chains.find_backward_accesses() + # We should have 4? reaches + # First b = A + 2 + # Second a = A + i + # Third (?) is A = a + i + # Last is A = 1 + assert len(reaches) == 4 + assert ( + reaches[0] is routine.children[1].loop_body.children[1].rhs.children[0] + ) + assert ( + reaches[1] is routine.children[1].loop_body.children[0].rhs.children[0] + ) + assert ( + reaches[2] is routine.children[1].loop_body.children[0].lhs + ) + assert reaches[3] is routine.children[0].lhs + + # Check if we access a loop variable + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + + do i = 1, 100 + a = a + i + b = a + 2 + end do + i = 1231 + end subroutine x""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from I = 1231. + chains = DefinitionUseChain( + routine.children[1].lhs + ) + reaches = chains.find_backward_accesses() + # We should have 1 reaches + # It should be the loop + assert len(reaches) == 1 + assert reaches[0] is routine.children[0] + + +def test_definition_use_chain_find_backward_accesses_while_loop_example( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour when there is a while loop.""" + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + i = 100 + a = 1 + do while (a < i) + a = a + 3 + end do + end subroutine""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from A = a + 3. + chains = DefinitionUseChain(routine.children[2].loop_body.children[0].lhs) + reaches = chains.find_backward_accesses() + + assert len(reaches) == 4 + assert reaches[2] is routine.children[2].condition.children[0] + assert ( + reaches[0] is routine.children[2].loop_body.children[0].rhs.children[0] + ) + assert reaches[1] is routine.children[2].loop_body.children[0].lhs + assert reaches[3] is routine.children[1].lhs + + +def test_definition_use_chain_foward_accesses_nested_loop_example( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour when there is a nested loop.""" + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + i = 100 + a = 1 + do while(a < i) + a = a + 3 + do while(b < 5 * i) + b = b + a + end do + end do + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from b = b + A. + loops = routine.walk(WhileLoop) + chains = DefinitionUseChain(loops[1].loop_body.children[0].rhs.children[1]) + reaches = chains.find_backward_accesses() + # Results should be A = A + 3 and the a < i condition, then the A = 1 + assert len(reaches) == 4 + assert reaches[2] is loops[0].condition.children[0] + assert reaches[0] is loops[0].loop_body.children[0].rhs.children[0] + assert reaches[1] is loops[0].loop_body.children[0].lhs + assert reaches[3] is routine.children[1].lhs + + +def test_definition_use_chain_find_backward_accesses_structure_example( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour when a structureReference is provided.""" + code = """ + subroutine x() + use some_mod + a%b = 1 + a%c = 2 + a%b = 3 + end subroutine""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[2].lhs) + reaches = chains.find_backward_accesses() + assert len(reaches) == 1 + assert reaches[0] is routine.children[0].lhs + + +def test_definition_use_chain_find_backward_accesses_no_control_flow_example( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour for a simple case with no control flow with + an assignment.""" + code = """ + subroutine x() + integer :: a + a = a + 2 + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[0].lhs) + reaches = chains.find_backward_accesses() + assert len(reaches) == 1 + assert reaches[0] is routine.children[0].rhs.children[0] + + +def test_definition_use_chain_find_backward_accesses_codeblock( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour for a simple case with a CodeBlock.""" + code = """ + subroutine x() + integer :: a + a = a + 2 + print *, a + a = 3 + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[2].lhs) + reaches = chains.find_backward_accesses() + assert len(reaches) == 1 + assert reaches[0] is routine.children[1] + + +def test_definition_use_chain_find_backward_accesses_codeblock_and_call_nlocal( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour for a simple case with a CodeBlock and a Call, and + where the variable is not a local variable.""" + code = """ + subroutine x() + use some_mod + print *, a + call b(a) + a = a + 2 + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[2].rhs.children[0]) + reaches = chains.find_backward_accesses() + assert len(reaches) == 1 + assert reaches[0] is routine.children[1] + + +def test_definition_use_chain_find_backward_accesses_codeblock_and_call_cflow( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour for a simple case with a CodeBlock and a Call inside + control flow, and where the variable is not a local variable.""" + code = """ + subroutine x() + use some_mod + call c(a) + if(cond) then + print *, a + call b(a) + end if + a = a + 2 + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[2].rhs.children[0]) + reaches = chains.find_backward_accesses() + assert len(reaches) == 2 + assert reaches[0] is routine.children[1].if_body.children[0] + assert reaches[1] is routine.children[0] + + +def test_definition_use_chain_find_backward_accesses_codeblock_and_call_local( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour for a simple case with a CodeBlock and a Call, and + where the variable is a local variable.""" + code = """ + subroutine x() + use some_mod + integer :: a + call b(a) + print *, a + a = a + 2 + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[2].rhs.children[0]) + reaches = chains.find_backward_accesses() + assert len(reaches) == 1 + assert reaches[0] is routine.children[1] + + +def test_definition_use_chain_find_backward_accesses_call_and_codeblock_nlocal( + fortran_reader, +): + """Functionality test for the find_backward_accesses routine. This + tests the behaviour for a simple case with a Call then a Codeblock, and + where the variable is not a local variable.""" + code = """ + subroutine x() + use some_mod + print *, a + call b(a) + a = 2 + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[2].lhs) + reaches = chains.find_backward_accesses() + assert len(reaches) == 1 + assert reaches[0] is routine.children[1] + + +def test_definition_use_chains_goto_statement( + fortran_reader, +): + """Tests that we get an error when a region contains a GOTO statement.""" + code = """ + subroutine x() + integer :: a + GOTO 100 + 100 a = a + 3 + a = 2 + end subroutine""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain(routine.children[2].lhs) + with pytest.raises(NotImplementedError) as excinfo: + chains.find_backward_accesses() + assert ("DefinitionUseChains can't handle code containing GOTO statements" + in str(excinfo.value)) + + +def test_definition_use_chains_exit_statement( + fortran_reader, +): + """Check that DefinitionUseChains ignore statements after an exit statement + in a loop.""" + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + + a = 1 + do i = 1, 100 + a = a + i + exit + b = a + 2 + end do + c = a + b + end subroutine x""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from a = A +i. + chains = DefinitionUseChain( + routine.children[1].loop_body.children[0].rhs.children[0] + ) + reaches = chains.find_backward_accesses() + # We should have 2 reaches + # First is A = a + i + # Second is A = 1 + assert len(reaches) == 2 + assert reaches[0] is routine.children[1].loop_body.children[0].lhs + assert reaches[1] is routine.children[0].lhs + pytest.xfail(reason="Issue #2760: DefinitionUseChains should not search " + "again in a loop when there is a guaranteed exit " + "statement") + + +def test_definition_use_chains_cycle_statement( + fortran_reader, +): + """Check that DefinitionUseChains ignore statements after a cycle statement + in a loop.""" + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + + a = 1 + do i = 1, 100 + a = a + i + a = b * 4 + cycle + b = a + 2 + end do + c = a + b + end subroutine x""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from a = A +i. + chains = DefinitionUseChain( + routine.children[1].loop_body.children[0].rhs + ) + reaches = chains.find_backward_accesses() + # We should have 2 reaches + # A = b * 4 + # A = 1 + assert len(reaches) == 2 + assert reaches[0] is routine.children[1].loop_body.children[1].lhs + assert reaches[1] is routine.children[0].lhs + + +def test_definition_use_chains_return_statement( + fortran_reader, +): + """Check that DefinitionUseChains ignore statements after a cycle statement + in a loop.""" + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + + a = 1 + do i = 1, 100 + a = a + i + a = b + 4 + return + b = a + 2 + end do + c = a + b + end subroutine x""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Start the chain from a = A +i. + chains = DefinitionUseChain( + routine.children[1].loop_body.children[0].rhs + ) + reaches = chains.find_backward_accesses() + # We should have 2 reaches + # A = b * 4 + # A = 1 + assert len(reaches) == 2 + assert reaches[0] is routine.children[1].loop_body.children[1].lhs + assert reaches[1] is routine.children[0].lhs + + +def test_definition_use_chains_backward_accesses_multiple_routines( + fortran_reader, +): + '''Test the backward_accesses function doesn't find accesses outside of the + containing subroutine.''' + code = """ +module my_mod + integer :: a, b + contains + subroutine test() + a = 1 + end subroutine + subroutine test2() + b = a + end subroutine +end module +""" + + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[1] + chains = DefinitionUseChain( + routine.children[0].rhs + ) + reaches = chains.find_backward_accesses() + assert len(reaches) == 0 From 13c42b5cd3c3674517dd75b65ca937c6a4e21643 Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Mon, 18 Nov 2024 14:40:08 +0000 Subject: [PATCH 02/12] More changes towards backwards accesses --- .../psyir/tools/definition_use_chains.py | 229 ++++++++++++++++++ 1 file changed, 229 insertions(+) diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index 0aff78b174..0e3f9c729b 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -768,3 +768,232 @@ def _compute_backward_uses(self, basic_block_list): self._uses.append(reference) if defs_out is not None: self._defsout.append(defs_out) + + + def find_backward_accesses(self): + """ + Find all the backward accesses for the reference defined in this + DefinitionUseChain. + Backward accesses are all of the prior References or Calls that read + or write to the symbol of the reference up to the point that a + write to the symbol is guaranteed to occur. + PSyclone assumes all control flow may not be taken, so writes + that occur inside control flow do not end the backward access + chain. + + :returns: the backward accesses of the reference given to this + DefinitionUseChain + :rtype: list[:py:class:`psyclone.psyir.nodes.Node`] + """ + # Setup the start and stop positions + save_start_position = self._start_point + save_stop_position = self._stop_point + # If there is no set start point, then we look for all + # accesses after the Reference. + if self._stop_point is None: + self._stop_point = self._reference_abs_pos + # If there is no set stop point, then any Reference after + # the start point can potentially be a forward access. + if self._start_point is None: + self._start_point = self._scope[0].abs_position + if not self.is_basic_block: + # If this isn't a basic block, then we find all of the basic + # blocks. + control_flow_nodes, basic_blocks = self._find_basic_blocks( + self._scope + ) + chains = [] + # If this is the top level access, we need to check if the + # reference has an ancestor loop. If it does, we find the + # highest ancestor Loop in the tree and add a + # DefinitionUseChain block at the start to search for things + # before the Reference that can also be looped back to. + # We should probably have this be any top level time this is + # called but thats hard to otherwise track. + if ( + isinstance(self._scope[0], Routine) + or self._scope[0] is self._reference.root + ): + # Check if there is an ancestor Loop/WhileLoop. + ancestor = self._reference.ancestor((Loop, WhileLoop)) + if ancestor is not None: + next_ancestor = ancestor.ancestor((Loop, WhileLoop)) + while next_ancestor is not None: + ancestor = next_ancestor + next_ancestor = ancestor.ancestor((Loop, WhileLoop)) + # Create a basic block for the ancestor Loop. + body = ancestor.loop_body.children[:] + control_flow_nodes.insert(0, ancestor) + # Find the stop point - this needs to be the node after + # the ancestor statement. + sub_stop_point = ( + self._reference.ancestor(Statement) + .walk(Node)[-1] + .abs_position + + 1 + ) + # We make a copy of the reference to have a detached + # node to avoid handling the special cases based on + # the parents of the reference. + chain = DefinitionUseChain( + self._reference.copy(), + body, + start_point=reference.abs_position+1, + stop_point=sub_stop_point, + ) + chains.insert(0, chain) + # If its a while loop, create a basic block for the while + # condition. + if isinstance(ancestor, WhileLoop): + control_flow_nodes.insert(0, None) + sub_stop_point = ancestor.loop_body.abs_position + chain = DefinitionUseChain( + self._reference.copy(), + [ancestor.condition], + start_point=ancestor.abs_position, + stop_point=sub_stop_point, + ) + chains.insert(0, chain) + + # Check if there is an ancestor Assignment. + ancestor = self._reference.ancestor(Assignment) + if ancestor is not None: + # If the reference is not the lhs then we can ignore the RHS. + if ancestor.lhs is not self._reference: + # Find the last node in the assignment + last_node = ancestor.walk(Node)[-1] + # Modify the start_point to only include the node after + # this assignment. + self._start_point = last_node.abs_position + else: + end = ancestor.rhs.abs_position.children[-1] + while len(end.children) > 0: + end = end.children[-1] + # Add the rhs as a potential basic block with + # different start and stop positions. + chain = DefinitionUseChain( + self._reference, + ancestor.rhs.children[:], + start_point=ancestor.rhs.abs_position, + stop_point=end.abs_position + 1, + ) + control_flow_nodes.append(None) + chains.append(chain) + # N.B. For now this assumes that for an expression + # b = a * a, that next_access to the first Reference + # to a should not return the second Reference to a. + # Now add all the other standardly handled basic_blocks to the + # list of chains. + for block in basic_blocks: + chain = DefinitionUseChain( + self._reference, + block, + start_point=self._start_point, + stop_point=self._stop_point, + ) + chains.append(chain) + + # For backwards we want to reverse the order. + chains.reverse() + for i, chain in enumerate(chains): + # Compute the defsout, killed and reaches for the block. + chain.find_backward_accesses() + cfn = control_flow_nodes[i] + + if cfn is None: + # We're outside a control flow region, updating the reaches + # here is to find all the reached nodes. + for ref in chain._reaches: + # Add unique references to reaches. Since we're not + # in a control flow region, we can't have added + # these references into the reaches array yet so + # they're guaranteed to be unique. + self._reaches.append(ref) + # If we have a defsout in the chain then we can stop as we + # will never get past the write as its not conditional. + if len(chain.defsout) > 0: + # Reset the start and stop points before returning + # the result. + self._start_point = save_start_position + self._stop_point = save_stop_position + return self._reaches + else: + # We assume that the control flow here could not be taken, + # i.e. that this doesn't kill the chain. + # TODO #2760: In theory we could analyse loop structures + # or if block structures to see if we're guaranteed to + # write to the symbol. + # If the control flow node is a Loop we have to check + # if the variable is the same symbol as the _reference. + if isinstance(cfn, Loop): + if cfn.variable == self._reference.symbol: + # The loop variable is always written to and so + # we're done if its reached. + self._reaches.append(cfn) + self._start_point = save_start_position + self._stop_point = save_stop_position + return self._reaches + + for ref in chain._reaches: + # We will only ever reach a reference once, so + # we don't need to check uniqueness. + self._reaches.append(ref) + else: + # Check if there is an ancestor Assignment. + ancestor = self._reference.ancestor(Assignment) + if ancestor is not None: + # If we get here to check the start part of a loop we need + # to handle this differently. + if self._start_point != self._reference_abs_pos: + pass + # If the reference is the lhs then we can ignore the RHS. + if ancestor.lhs is not self._reference: + # Find the last node in the assignment + last_node = ancestor.walk(Node)[-1] + # Modify the start_point to only include the node after + # this assignment. + self._start_point = last_node.abs_position + elif ancestor.lhs is self._scope[0] and len(self._scope) == 1: + # If the ancestor LHS is the scope of this chain then we + # do nothing. + pass + else: + # Add the lhs as a potential basic block with different + # start and stop positions. + chain = DefinitionUseChain( + self._reference, + [ancestor.rhs.children], + start_point=ancestor.rhs.abs_position, + stop_point=ancestor.lhs.abs_position + 1, + ) + # Find any forward_accesses in the lhs. + chain.find_backward_accesses() + for ref in chain._reaches: + self._reaches.append(ref) + # If we have a defsout in the chain then we can stop as we + # will never get past the write as its not conditional. + if len(chain.defsout) > 0: + # Reset the start and stop points before returning + # the result. + self._start_point = save_start_position + self._stop_point = save_stop_position + return self._reaches + # We can compute the rest of the accesses + self._compute_backward_uses(self._scope) + for ref in self._uses: + self._reaches.append(ref) + # If this block doesn't kill any accesses, then we add + # the defsout into the reaches array. + if len(self.killed) == 0: + for ref in self._defsout: + self._reaches.append(ref) + else: + # If this block killed any accesses, then the first element + # of the killed writes is the access access that we're + # dependent with. + self._reaches.append(self.killed[0]) + + # Reset the start and stop points before returning the result. + self._start_point = save_start_position + self._stop_point = save_stop_position + return self._reaches From 7b9d201836c8545b71dad2c3800976fefba5c32e Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Mon, 2 Dec 2024 14:23:08 +0000 Subject: [PATCH 03/12] Initial implementation of backward dependencies --- .../psyir/tools/definition_use_chains.py | 222 ++++++++++-------- ...ion_use_chains_backward_dependence_test.py | 62 +++-- 2 files changed, 166 insertions(+), 118 deletions(-) diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index 0e3f9c729b..59190c81a9 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -33,7 +33,7 @@ # ----------------------------------------------------------------------------- # Author: A. B. G. Chalk, STFC Daresbury Lab # ----------------------------------------------------------------------------- -'''This module contains the DefinitionUseChain class''' +"""This module contains the DefinitionUseChain class""" import sys @@ -90,22 +90,28 @@ def __init__( stop_point=None, ): if not isinstance(reference, Reference): - raise TypeError(f"The reference passed into a DefinitionUseChain " - f"must be a Reference but found " - f"'{type(reference).__name__}'.") + raise TypeError( + f"The reference passed into a DefinitionUseChain " + f"must be a Reference but found " + f"'{type(reference).__name__}'." + ) self._reference = reference # Store the absolute position for later. self._reference_abs_pos = reference.abs_position # To enable loops to work correctly we can set the start/stop point # and not just use base it on the reference's absolute position if start_point and not isinstance(start_point, int): - raise TypeError(f"The start_point passed into a " - f"DefinitionUseChain must be an int but found " - f"'{type(start_point).__name__}'.") + raise TypeError( + f"The start_point passed into a " + f"DefinitionUseChain must be an int but found " + f"'{type(start_point).__name__}'." + ) if stop_point and not isinstance(stop_point, int): - raise TypeError(f"The stop_point passed into a " - f"DefinitionUseChain must be an int but found " - f"'{type(stop_point).__name__}'.") + raise TypeError( + f"The stop_point passed into a " + f"DefinitionUseChain must be an int but found " + f"'{type(stop_point).__name__}'." + ) self._start_point = start_point self._stop_point = stop_point if control_flow_region is None: @@ -443,13 +449,16 @@ def _compute_forward_uses(self, basic_block_list): # as checking the symbol - this means we can get false # positives for structure accesses inside CodeBlocks. if isinstance(reference._fp2_nodes[0], Goto_Stmt): - raise NotImplementedError("DefinitionUseChains can't " - "handle code containing GOTO" - " statements.") + raise NotImplementedError( + "DefinitionUseChains can't " + "handle code containing GOTO" + " statements." + ) # If we find an Exit or Cycle statement, we can't # reach further in this code region so we can return. - if isinstance(reference._fp2_nodes[0], (Exit_Stmt, - Cycle_Stmt)): + if isinstance( + reference._fp2_nodes[0], (Exit_Stmt, Cycle_Stmt) + ): if defs_out is not None: self._defsout.append(defs_out) return @@ -602,8 +611,18 @@ def _find_basic_blocks(self, nodelist): # No control for the condition - we always check that. control_flow_nodes.append(None) basic_blocks.append([node.condition]) - control_flow_nodes.append(node) - basic_blocks.append(node.if_body.children[:]) + # Check if the node is in the else_body + in_else_body = False + if node.else_body: + refs = node.else_body.walk(Reference) + for ref in refs: + if ref is self._reference: + in_else_body = True + break + # If its in the else_body we don't add the if_body + if not in_else_body: + control_flow_nodes.append(node) + basic_blocks.append(node.if_body.children[:]) # Check if the node is in the if_body in_if_body = False refs = node.if_body.walk(Reference) @@ -647,30 +666,34 @@ def _compute_backward_uses(self, basic_block_list): defs_out = None # Working backwards so reverse the basic_block_list basic_block_list.reverse() + stop_position = self._stop_point for region in basic_block_list: region_list = region.walk((Reference, Call, CodeBlock, Return)) - # If the regoin contains any Return, Exit or Cycle statements then + # If the region contains any Return, Exit or Cycle statements then # we modify the stop position to only look at statements that # occur before this statement. - # FIXME: This doesn't work correctly if the Reference that + # This doesn't work correctly if the Reference that # is having its backwards dependencies analysed occurs after # one of these such statements in a basic block, however # since they're unreachable maybe we don't care? - stop_position = self._stop_point for reference in region_list: if isinstance(reference, Return): - stop_position = reference.abs_position + stop_position = min(reference.abs_position, stop_position) if isinstance(reference, CodeBlock): - if isinstance(reference._fp2_nodes[0], (Exit_Stmt, - Cycle_Stmt)): - stop_position = reference.abs_position + if isinstance( + reference._fp2_nodes[0], (Exit_Stmt, Cycle_Stmt) + ): + stop_position = min( + reference.abs_position, stop_position + ) + for region in basic_block_list: + region_list = region.walk((Reference, Call, CodeBlock, Return)) # Reverse the list region_list.reverse() for reference in region_list: # Store the position instead of computing it twice. abs_pos = reference.abs_position - # TODO Fix start and stop positions - if abs_pos <= self._start_point or abs_pos >= stop_position: + if abs_pos < self._start_point or abs_pos >= stop_position: continue if isinstance(reference, Return): # We can ignore Return statements as we only check for @@ -681,13 +704,16 @@ def _compute_backward_uses(self, basic_block_list): # as checking the symbol - this means we can get false # positives for structure accesses inside CodeBlocks. if isinstance(reference._fp2_nodes[0], Goto_Stmt): - raise NotImplementedError("DefinitionUseChains can't " - "handle code containing GOTO" - " statements.") + raise NotImplementedError( + "DefinitionUseChains can't " + "handle code containing GOTO" + " statements." + ) # If we find an Exit or Cycle statement, only check for # nodes that occur before them so we can skip. - if isinstance(reference._fp2_nodes[0], (Exit_Stmt, - Cycle_Stmt)): + if isinstance( + reference._fp2_nodes[0], (Exit_Stmt, Cycle_Stmt) + ): continue if ( self._reference.symbol.name @@ -725,16 +751,28 @@ def _compute_backward_uses(self, basic_block_list): # a dependency to the value used from the LHS. if assign is not None: if assign.lhs is reference: + # Check if the RHS contains the self._reference. + # Can't use in since equality is not what we want + # here. + found = False + for ref in assign.rhs.walk(Reference): + if ( + ref is self._reference + and self._stop_point == ref.abs_position + ): + found = True + # If the RHS contains the self._reference, then + # this LHS is "after" so we skip it + if found: + continue # This is a write to the reference, so kill the # previous defs_out and set this to be the # defs_out. - print("OK") if defs_out is not None: self._killed.append(defs_out) defs_out = reference elif ( - assign.lhs.get_signature_and_indices()[0] - == sig + assign.lhs.get_signature_and_indices()[0] == sig and assign.lhs is not self._reference ): # Reference is on the rhs of an assignment such as @@ -769,7 +807,6 @@ def _compute_backward_uses(self, basic_block_list): if defs_out is not None: self._defsout.append(defs_out) - def find_backward_accesses(self): """ Find all the backward accesses for the reference defined in this @@ -803,6 +840,16 @@ def find_backward_accesses(self): self._scope ) chains = [] + # Now add all the other standardly handled basic_blocks to the + # list of chains. + for block in basic_blocks: + chain = DefinitionUseChain( + self._reference, + block, + start_point=self._start_point, + stop_point=self._stop_point, + ) + chains.append(chain) # If this is the top level access, we need to check if the # reference has an ancestor loop. If it does, we find the # highest ancestor Loop in the tree and add a @@ -823,29 +870,30 @@ def find_backward_accesses(self): next_ancestor = ancestor.ancestor((Loop, WhileLoop)) # Create a basic block for the ancestor Loop. body = ancestor.loop_body.children[:] - control_flow_nodes.insert(0, ancestor) - # Find the stop point - this needs to be the node after - # the ancestor statement. - sub_stop_point = ( - self._reference.ancestor(Statement) - .walk(Node)[-1] - .abs_position - + 1 - ) + # Find the stop point - this needs to be the last node + # in the ancestor loop + sub_stop_point = ancestor.walk(Node)[-1].abs_position + 1 # We make a copy of the reference to have a detached # node to avoid handling the special cases based on # the parents of the reference. + if self._reference.ancestor(Assignment) is not None: + sub_start_point = self._reference.ancestor( + Assignment + ).abs_position + else: + sub_start_point = self._reference.abs_position chain = DefinitionUseChain( self._reference.copy(), body, - start_point=reference.abs_position+1, + start_point=sub_start_point, stop_point=sub_stop_point, ) - chains.insert(0, chain) + chains.append(chain) + control_flow_nodes.append(ancestor) # If its a while loop, create a basic block for the while # condition. if isinstance(ancestor, WhileLoop): - control_flow_nodes.insert(0, None) + control_flow_nodes.append(None) sub_stop_point = ancestor.loop_body.abs_position chain = DefinitionUseChain( self._reference.copy(), @@ -853,26 +901,23 @@ def find_backward_accesses(self): start_point=ancestor.abs_position, stop_point=sub_stop_point, ) - chains.insert(0, chain) + chains.append(chain) # Check if there is an ancestor Assignment. ancestor = self._reference.ancestor(Assignment) if ancestor is not None: - # If the reference is not the lhs then we can ignore the RHS. + # If the reference is not the lhs then we can ignore + # the RHS. if ancestor.lhs is not self._reference: - # Find the last node in the assignment - last_node = ancestor.walk(Node)[-1] - # Modify the start_point to only include the node after - # this assignment. - self._start_point = last_node.abs_position + pass else: - end = ancestor.rhs.abs_position.children[-1] + end = ancestor.rhs while len(end.children) > 0: end = end.children[-1] # Add the rhs as a potential basic block with # different start and stop positions. chain = DefinitionUseChain( - self._reference, + self._reference.copy(), ancestor.rhs.children[:], start_point=ancestor.rhs.abs_position, stop_point=end.abs_position + 1, @@ -882,19 +927,10 @@ def find_backward_accesses(self): # N.B. For now this assumes that for an expression # b = a * a, that next_access to the first Reference # to a should not return the second Reference to a. - # Now add all the other standardly handled basic_blocks to the - # list of chains. - for block in basic_blocks: - chain = DefinitionUseChain( - self._reference, - block, - start_point=self._start_point, - stop_point=self._stop_point, - ) - chains.append(chain) # For backwards we want to reverse the order. chains.reverse() + control_flow_nodes.reverse() for i, chain in enumerate(chains): # Compute the defsout, killed and reaches for the block. chain.find_backward_accesses() @@ -908,7 +944,13 @@ def find_backward_accesses(self): # in a control flow region, we can't have added # these references into the reaches array yet so # they're guaranteed to be unique. - self._reaches.append(ref) + found = False + for ref2 in self._reaches: + if ref is ref2: + found = True + break + if not found: + self._reaches.append(ref) # If we have a defsout in the chain then we can stop as we # will never get past the write as its not conditional. if len(chain.defsout) > 0: @@ -935,53 +977,49 @@ def find_backward_accesses(self): return self._reaches for ref in chain._reaches: - # We will only ever reach a reference once, so - # we don't need to check uniqueness. - self._reaches.append(ref) + found = False + for ref2 in self._reaches: + if ref is ref2: + found = True + break + if not found: + self._reaches.append(ref) else: # Check if there is an ancestor Assignment. ancestor = self._reference.ancestor(Assignment) if ancestor is not None: + # TODO Need to fix this - not sure what is going on here. # If we get here to check the start part of a loop we need # to handle this differently. - if self._start_point != self._reference_abs_pos: - pass # If the reference is the lhs then we can ignore the RHS. if ancestor.lhs is not self._reference: - # Find the last node in the assignment - last_node = ancestor.walk(Node)[-1] - # Modify the start_point to only include the node after - # this assignment. - self._start_point = last_node.abs_position - elif ancestor.lhs is self._scope[0] and len(self._scope) == 1: - # If the ancestor LHS is the scope of this chain then we + pass + elif ancestor.rhs is self._scope[0] and len(self._scope) == 1: + # If the ancestor RHS is the scope of this chain then we # do nothing. pass else: - # Add the lhs as a potential basic block with different + # Add the rhs as a potential basic block with different # start and stop positions. chain = DefinitionUseChain( self._reference, - [ancestor.rhs.children], + [ancestor.rhs], start_point=ancestor.rhs.abs_position, - stop_point=ancestor.lhs.abs_position + 1, + stop_point=sys.maxsize, ) - # Find any forward_accesses in the lhs. + # Find any forward_accesses in the rhs. chain.find_backward_accesses() for ref in chain._reaches: self._reaches.append(ref) - # If we have a defsout in the chain then we can stop as we - # will never get past the write as its not conditional. - if len(chain.defsout) > 0: - # Reset the start and stop points before returning - # the result. - self._start_point = save_start_position - self._stop_point = save_stop_position - return self._reaches # We can compute the rest of the accesses self._compute_backward_uses(self._scope) for ref in self._uses: - self._reaches.append(ref) + found = False + for ref2 in self._reaches: + if ref is ref2: + found = True + if not found: + self._reaches.append(ref) # If this block doesn't kill any accesses, then we add # the defsout into the reaches array. if len(self.killed) == 0: 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 971833812c..e3124a38eb 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 @@ -145,7 +145,7 @@ def test_definition_use_chain_find_backward_accesses_basic_example( # The call bar(c, b) as a isn't local and we can't guarantee its behaviour. assert len(reaches) == 2 assert reaches[0] is routine.children[8].rhs.children[0] - assert reaches[2] is routine.children[6] + assert reaches[1] is routine.children[6] # Create use chain for c in b = c + d chains = DefinitionUseChain(routine.children[5].rhs.children[0], [routine]) @@ -172,14 +172,14 @@ def test_definition_use_chain_find_backward_accesses_assignment( psyir = fortran_reader.psyir_from_source(code) routine = psyir.walk(Routine)[0] # Start chain from A = a * a - chains = DefinitionUseChain(routine.children[0].lhs) + chains = DefinitionUseChain(routine.children[1].lhs) reaches = chains.find_backward_accesses() # We should find 3 results, both 3 references in # a = A * A # and A = 1 assert len(reaches) == 3 - assert reaches[0] is routine.children[1].rhs.children[0] - assert reaches[1] is routine.children[1].rhs.children[1] + assert reaches[0] is routine.children[1].rhs.children[1] + assert reaches[1] is routine.children[1].rhs.children[0] assert reaches[2] is routine.children[0].lhs @@ -210,13 +210,13 @@ def test_definition_use_chain_find_backward_accesses_ifelse_example( # even though it will always be written to. assert len(reaches) == 4 assert reaches[2] is routine.children[1].rhs.children[0] - assert reaches[0] is routine.children[2].if_body.children[0].lhs - assert reaches[1] is routine.children[2].else_body.children[0].lhs + assert reaches[1] is routine.children[2].if_body.children[0].lhs + assert reaches[0] is routine.children[2].else_body.children[0].lhs assert reaches[3] is routine.children[0].lhs # Also check that a = 4 backward access is not a = 3. a_3 = routine.children[2].if_body.children[0].lhs - a_4 = routine.children[2].else_body.children[0].rhs + a_4 = routine.children[2].else_body.children[0].lhs chains = DefinitionUseChain(a_4) reaches = chains.find_backward_accesses() assert len(reaches) == 2 @@ -245,7 +245,7 @@ def test_definition_use_chain_find_backward_accesses_loop_example( routine = psyir.walk(Routine)[0] # Start the chain from A = a + i. chains = DefinitionUseChain( - routine.children[1].loop_body.children[1].rhs.children[0] + routine.children[1].loop_body.children[0].lhs ) reaches = chains.find_backward_accesses() # We should have 4? reaches @@ -253,15 +253,18 @@ def test_definition_use_chain_find_backward_accesses_loop_example( # Second a = A + i # Third (?) is A = a + i # Last is A = 1 + print(reaches[0].parent.debug_string()) + print(reaches[1].parent.debug_string()) + print(reaches[2].parent.debug_string()) assert len(reaches) == 4 assert ( reaches[0] is routine.children[1].loop_body.children[1].rhs.children[0] ) assert ( - reaches[1] is routine.children[1].loop_body.children[0].rhs.children[0] + reaches[2] is routine.children[1].loop_body.children[0].rhs.children[0] ) assert ( - reaches[2] is routine.children[1].loop_body.children[0].lhs + reaches[1] is routine.children[1].loop_body.children[0].lhs ) assert reaches[3] is routine.children[0].lhs @@ -312,9 +315,9 @@ def test_definition_use_chain_find_backward_accesses_while_loop_example( reaches = chains.find_backward_accesses() assert len(reaches) == 4 - assert reaches[2] is routine.children[2].condition.children[0] + assert reaches[0] is routine.children[2].condition.children[0] assert ( - reaches[0] is routine.children[2].loop_body.children[0].rhs.children[0] + reaches[2] is routine.children[2].loop_body.children[0].rhs.children[0] ) assert reaches[1] is routine.children[2].loop_body.children[0].lhs assert reaches[3] is routine.children[1].lhs @@ -343,11 +346,15 @@ def test_definition_use_chain_foward_accesses_nested_loop_example( loops = routine.walk(WhileLoop) chains = DefinitionUseChain(loops[1].loop_body.children[0].rhs.children[1]) reaches = chains.find_backward_accesses() - # Results should be A = A + 3 and the a < i condition, then the A = 1 + # TODO #2760 The backwards accesses should not continue past a = a + 3 as + # to reach the b = b + a statement we must have passed through the + # a = a + 3 statement. + # Results should be b = b + A, A = A + 3 and the a < i condition + # then the A = 1 assert len(reaches) == 4 - assert reaches[2] is loops[0].condition.children[0] - assert reaches[0] is loops[0].loop_body.children[0].rhs.children[0] - assert reaches[1] is loops[0].loop_body.children[0].lhs + assert reaches[0] is loops[0].condition.children[0] + assert reaches[1] is loops[1].loop_body.children[0].rhs.children[1] + assert reaches[2] is loops[0].loop_body.children[0].lhs assert reaches[3] is routine.children[1].lhs @@ -429,7 +436,7 @@ def test_definition_use_chain_find_backward_accesses_codeblock_and_call_nlocal( chains = DefinitionUseChain(routine.children[2].rhs.children[0]) reaches = chains.find_backward_accesses() assert len(reaches) == 1 - assert reaches[0] is routine.children[1] + assert reaches[0] is routine.children[1].children[1] def test_definition_use_chain_find_backward_accesses_codeblock_and_call_cflow( @@ -441,7 +448,7 @@ def test_definition_use_chain_find_backward_accesses_codeblock_and_call_cflow( code = """ subroutine x() use some_mod - call c(a) + call c() if(cond) then print *, a call b(a) @@ -453,7 +460,8 @@ def test_definition_use_chain_find_backward_accesses_codeblock_and_call_cflow( chains = DefinitionUseChain(routine.children[2].rhs.children[0]) reaches = chains.find_backward_accesses() assert len(reaches) == 2 - assert reaches[0] is routine.children[1].if_body.children[0] + print([x.parent.debug_string() for x in reaches]) + assert reaches[0] is routine.children[1].if_body.children[1].children[1] assert reaches[1] is routine.children[0] @@ -489,7 +497,7 @@ def test_definition_use_chain_find_backward_accesses_call_and_codeblock_nlocal( subroutine x() use some_mod print *, a - call b(a) + call b() a = 2 end subroutine""" psyir = fortran_reader.psyir_from_source(code) @@ -513,7 +521,7 @@ def test_definition_use_chains_goto_statement( end subroutine""" psyir = fortran_reader.psyir_from_source(code) routine = psyir.walk(Routine)[0] - chains = DefinitionUseChain(routine.children[2].lhs) + chains = DefinitionUseChain(routine.children[1].lhs) with pytest.raises(NotImplementedError) as excinfo: chains.find_backward_accesses() assert ("DefinitionUseChains can't handle code containing GOTO statements" @@ -559,8 +567,8 @@ def test_definition_use_chains_exit_statement( def test_definition_use_chains_cycle_statement( fortran_reader, ): - """Check that DefinitionUseChains ignore statements after a cycle statement - in a loop.""" + """Check that DefinitionUseChains ignore statements after a return + statement in a loop.""" code = """ subroutine x() integer :: a, b, c, d, e, f, i @@ -579,7 +587,7 @@ def test_definition_use_chains_cycle_statement( routine = psyir.walk(Routine)[0] # Start the chain from a = A +i. chains = DefinitionUseChain( - routine.children[1].loop_body.children[0].rhs + routine.children[1].loop_body.children[0].rhs.children[0] ) reaches = chains.find_backward_accesses() # We should have 2 reaches @@ -613,15 +621,17 @@ def test_definition_use_chains_return_statement( routine = psyir.walk(Routine)[0] # Start the chain from a = A +i. chains = DefinitionUseChain( - routine.children[1].loop_body.children[0].rhs + routine.children[1].loop_body.children[0].rhs.children[0] ) reaches = chains.find_backward_accesses() # We should have 2 reaches # A = b * 4 # A = 1 assert len(reaches) == 2 - assert reaches[0] is routine.children[1].loop_body.children[1].lhs assert reaches[1] is routine.children[0].lhs + # Search backwards in loop isn't working correctly - we're finding + # the LHS of a = a + i instead of A = b + 4 + assert reaches[0] is routine.children[1].loop_body.children[1].lhs def test_definition_use_chains_backward_accesses_multiple_routines( From 6df819ae8269266f72a11336db1a81a57ae0fb6c Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:12:29 +0000 Subject: [PATCH 04/12] removed dead code and coverage improvements --- .../psyir/tools/definition_use_chains.py | 10 ------ ...ion_use_chains_backward_dependence_test.py | 32 +++++++++++++++++++ 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index 59190c81a9..98eac3f766 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -695,10 +695,6 @@ def _compute_backward_uses(self, basic_block_list): abs_pos = reference.abs_position if abs_pos < self._start_point or abs_pos >= stop_position: continue - if isinstance(reference, Return): - # We can ignore Return statements as we only check for - # nodes that occur before them. - continue if isinstance(reference, CodeBlock): # CodeBlocks only find symbols, so we can only do as good # as checking the symbol - this means we can get false @@ -709,12 +705,6 @@ def _compute_backward_uses(self, basic_block_list): "handle code containing GOTO" " statements." ) - # If we find an Exit or Cycle statement, only check for - # nodes that occur before them so we can skip. - if isinstance( - reference._fp2_nodes[0], (Exit_Stmt, Cycle_Stmt) - ): - continue if ( self._reference.symbol.name in reference.get_symbol_names() 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 e3124a38eb..d8c4f0b7ae 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 @@ -659,3 +659,35 @@ def test_definition_use_chains_backward_accesses_multiple_routines( ) reaches = chains.find_backward_accesses() assert len(reaches) == 0 + + +def test_definition_use_chains_backward_accesses_nonassign_reference_in_loop( + fortran_reader, +): + '''Coverage completion to handle the case where the passed reference is + not part of an assignment and within a loop.''' + code = """ + subroutine x() + integer :: a, b, c, d, e, f, i + + a = 1 + do i = 1, 100 + a = a + i + call p(a) + return + b = a + 2 + end do + c = a + b + end subroutine x""" + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + chains = DefinitionUseChain( + routine.children[1].loop_body.children[1].children[1] + ) + reaches = chains.find_backward_accesses() + # TODO #2760 The backwards accesses should not continue past a = a + i + # when searching backwards in the loop, or to a = 1 + assert len(reaches) == 3 + assert reaches[0] is routine.children[1].loop_body.children[1].children[1] + assert reaches[1] is routine.children[1].loop_body.children[0].lhs + assert reaches[2] is routine.children[0].lhs From be3292b72b55ec9380f91b48e65e37359029393a Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Mon, 2 Dec 2024 15:29:30 +0000 Subject: [PATCH 05/12] Removed final bit of dead code --- src/psyclone/psyir/tools/definition_use_chains.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index 98eac3f766..4e78488e76 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -997,19 +997,14 @@ def find_backward_accesses(self): start_point=ancestor.rhs.abs_position, stop_point=sys.maxsize, ) - # Find any forward_accesses in the rhs. + # Find any backward_accesses in the rhs. chain.find_backward_accesses() for ref in chain._reaches: self._reaches.append(ref) # We can compute the rest of the accesses self._compute_backward_uses(self._scope) for ref in self._uses: - found = False - for ref2 in self._reaches: - if ref is ref2: - found = True - if not found: - self._reaches.append(ref) + self._reaches.append(ref) # If this block doesn't kill any accesses, then we add # the defsout into the reaches array. if len(self.killed) == 0: From 27c0c38316155daa8d928c9bd092228c1f6b546b Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Tue, 3 Dec 2024 14:40:11 +0000 Subject: [PATCH 06/12] Fixed a bug with loop variables being pushed into the wrong usage if they weren't inside the search area, and added tests for previous_accesses in Reference class --- src/psyclone/psyir/nodes/reference.py | 32 ++----- .../psyir/tools/definition_use_chains.py | 27 +++--- .../tests/psyir/nodes/reference_test.py | 89 +++++++++++++------ 3 files changed, 85 insertions(+), 63 deletions(-) diff --git a/src/psyclone/psyir/nodes/reference.py b/src/psyclone/psyir/nodes/reference.py index 6efabfa5dc..fe1f3404e8 100644 --- a/src/psyclone/psyir/nodes/reference.py +++ b/src/psyclone/psyir/nodes/reference.py @@ -233,34 +233,18 @@ def datatype(self): return UnresolvedType() return self.symbol.datatype - def previous_access(self): + def previous_accesses(self): ''' - :returns: the previous reference to the same symbol. - :rtype: Optional[:py:class:`psyclone.psyir.nodes.Node`] + :returns: the nodes accessing the same symbol directly before this + reference. It can be multiple nodes if the control flow + diverges and there are multiple possible accesses. + :rtype: List[:py:class:`psyclone.psyir.nodes.Node`] ''' # Avoid circular import # pylint: disable=import-outside-toplevel - from psyclone.psyir.nodes.routine import Routine - # The scope is as far as the Routine that contains this - # Reference. - routine = self.ancestor(Routine) - # Handle the case when this is a subtree without an ancestor - # Routine - if routine is None: - routine = self.root - var_access = VariablesAccessInfo(nodes=routine) - signature, _ = self.get_signature_and_indices() - all_accesses = var_access[signature].all_accesses - index = -1 - # Find my position in the VariablesAccesInfo - for i, access in enumerate(all_accesses): - if access.node is self: - index = i - break - - if index > 0: - return all_accesses[index-1].node - return None + from psyclone.psyir.tools import DefinitionUseChain + chain = DefinitionUseChain(self) + return chain.find_backward_accesses() def next_accesses(self): ''' diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index 4e78488e76..6405dab2a3 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -339,7 +339,12 @@ def find_forward_accesses(self): # If the control flow node is a Loop we have to check # if the variable is the same symbol as the _reference. if isinstance(cfn, Loop): - if cfn.variable == self._reference.symbol: + cfn_abs_pos = cfn.abs_position + if ( + cfn.variable == self._reference.symbol + and cfn_abs_pos >= self._start_point + and cfn_abs_pos < self._stop_point + ): # The loop variable is always written to and so # we're done if its reached. self._reaches.append(cfn) @@ -862,7 +867,7 @@ def find_backward_accesses(self): body = ancestor.loop_body.children[:] # Find the stop point - this needs to be the last node # in the ancestor loop - sub_stop_point = ancestor.walk(Node)[-1].abs_position + 1 + sub_stop_point = ancestor.walk(Node)[-1].abs_position # We make a copy of the reference to have a detached # node to avoid handling the special cases based on # the parents of the reference. @@ -898,19 +903,15 @@ def find_backward_accesses(self): if ancestor is not None: # If the reference is not the lhs then we can ignore # the RHS. - if ancestor.lhs is not self._reference: - pass - else: - end = ancestor.rhs - while len(end.children) > 0: - end = end.children[-1] + if ancestor.lhs is self._reference: + end = ancestor.walk(Node)[-1] # Add the rhs as a potential basic block with # different start and stop positions. chain = DefinitionUseChain( self._reference.copy(), ancestor.rhs.children[:], start_point=ancestor.rhs.abs_position, - stop_point=end.abs_position + 1, + stop_point=end.abs_position, ) control_flow_nodes.append(None) chains.append(chain) @@ -958,7 +959,12 @@ def find_backward_accesses(self): # If the control flow node is a Loop we have to check # if the variable is the same symbol as the _reference. if isinstance(cfn, Loop): - if cfn.variable == self._reference.symbol: + cfn_abs_pos = cfn.abs_position + if ( + cfn.variable == self._reference.symbol + and cfn_abs_pos >= self._start_point + and cfn_abs_pos < self._stop_point + ): # The loop variable is always written to and so # we're done if its reached. self._reaches.append(cfn) @@ -1001,6 +1007,7 @@ def find_backward_accesses(self): chain.find_backward_accesses() for ref in chain._reaches: self._reaches.append(ref) + # We can compute the rest of the accesses self._compute_backward_uses(self._scope) for ref in self._uses: diff --git a/src/psyclone/tests/psyir/nodes/reference_test.py b/src/psyclone/tests/psyir/nodes/reference_test.py index 8babd6a70d..d01cef56fb 100644 --- a/src/psyclone/tests/psyir/nodes/reference_test.py +++ b/src/psyclone/tests/psyir/nodes/reference_test.py @@ -256,6 +256,24 @@ def test_reference_next_accesses(fortran_reader): assert len(b.next_accesses()) == 1 assert b.next_accesses()[0] == b + # Check that a loop accessing a variable before + # the reference doesn't result in a false positive. + code = '''subroutine my_sub() + integer :: a + integer :: b + do a = 0, 10 + b = a + end do + a = 1 + end subroutine''' + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.children[0] + a = routine.children[1].lhs + loop = routine.children[0] + b = loop.loop_body.children[0].lhs + a_2 = loop.loop_body.children[0].rhs + assert len(a.next_accesses()) == 0 + # Check the function for basic structures code = '''subroutine my_sub() type :: x @@ -359,8 +377,8 @@ def test_reference_next_accesses_with_codeblock(fortran_reader): assert a.next_accesses()[0] is codeblock -def test_reference_previous_access(fortran_reader): - '''Test the previous_access function for a Reference''' +def test_reference_previous_accesses(fortran_reader): + '''Test the previous_accesses function for a Reference''' code = '''subroutine my_sub() integer :: a integer :: b @@ -374,11 +392,11 @@ def test_reference_previous_access(fortran_reader): b = routine.children[1].lhs a_2 = routine.children[2].lhs b_2 = routine.children[2].rhs - assert a.previous_access() is None - assert b.previous_access() is None - assert a_2.previous_access() is a - assert b_2.previous_access() is b - + assert len(a.previous_accesses()) == 0 + assert len(b.previous_accesses()) == 0 + assert a_2.previous_accesses()[0] is a + assert b_2.previous_accesses()[0] is b + code = '''subroutine my_sub() integer :: a integer :: b @@ -393,9 +411,11 @@ def test_reference_previous_access(fortran_reader): loop = routine.children[1] b_a = loop.loop_body.children[0].lhs a_2 = loop.loop_body.children[0].rhs - assert a.previous_access() is None - assert b_a.previous_access() is None - assert a_2.previous_access() is loop + assert len(a.previous_accesses()) == 0 + assert len(b_a.previous_accesses()) == 1 + assert b_a.previous_accesses()[0] is b_a + assert len(a_2.previous_accesses()) == 1 + assert a_2.previous_accesses()[0] is loop # Check the function for basic structures code = '''subroutine my_sub() @@ -415,10 +435,12 @@ def test_reference_previous_access(fortran_reader): b = routine.children[1].lhs a_2 = routine.children[2].lhs b_2 = routine.children[3].lhs - assert a.previous_access() is None - assert b.previous_access() is None - assert a_2.previous_access() is a - assert b_2.previous_access() is b + assert len(a.previous_accesses()) == 0 + assert len(b.previous_accesses()) == 0 + assert len(a_2.previous_accesses()) == 1 + assert len(b_2.previous_accesses()) == 1 + assert a_2.previous_accesses()[0] is a + assert b_2.previous_accesses()[0] is b # Check the function for array access code = '''subroutine my_sub() @@ -430,8 +452,9 @@ def test_reference_previous_access(fortran_reader): routine = psyir.children[0] a = routine.children[0].lhs a_2 = routine.children[1].lhs - assert a.previous_access() is None - assert a_2.previous_access() is a + assert len(a.previous_accesses()) == 0 + assert len(a_2.previous_accesses()) == 1 + assert a_2.previous_accesses()[0] is a # Check if statements code = '''subroutine my_sub() @@ -448,9 +471,12 @@ def test_reference_previous_access(fortran_reader): a = routine.children[0].lhs a_2 = routine.children[1].if_body.children[0].lhs a_3 = routine.children[2].lhs - assert a.previous_access() is None - assert a_2.previous_access() is a - assert a_3.previous_access() is a_2 + assert len(a.previous_accesses()) == 0 + assert len(a_2.previous_accesses()) == 1 + assert a_2.previous_accesses()[0] is a + assert len(a_3.previous_accesses()) == 2 + assert a_3.previous_accesses()[0] is a_2 + assert a_3.previous_accesses()[1] is a # Check else block behaviour code = '''subroutine my_sub() @@ -470,10 +496,15 @@ def test_reference_previous_access(fortran_reader): a_2 = routine.children[1].if_body.children[0].lhs a_3 = routine.children[1].else_body.children[0].lhs a_4 = routine.children[2].lhs - assert a.previous_access() is None - assert a_2.previous_access() is a - assert a_3.previous_access() is a_2 - assert a_4.previous_access() is a_3 + assert len(a.previous_accesses()) == 0 + assert len(a_2.previous_accesses()) == 1 + assert a_2.previous_accesses()[0] is a + assert len(a_2.previous_accesses()) == 1 + assert a_3.previous_accesses()[0] is a + assert len(a_4.previous_accesses()) == 3 + assert a_4.previous_accesses()[0] is a_3 + assert a_4.previous_accesses()[1] is a_2 + assert a_4.previous_accesses()[2] is a def test_reference_accesses_initialisation_statement(fortran_reader): @@ -492,20 +523,20 @@ def test_reference_accesses_initialisation_statement(fortran_reader): psyir = fortran_reader.psyir_from_source(code) routine = psyir.children[0].children[0] a = routine.children[0].lhs - assert a.previous_access() is None + assert len(a.previous_accesses()) == 0 sym_tab = routine.symbol_table symbols = sym_tab.get_symbols() b_sym = symbols['b'] refs = b_sym.initial_value.walk(Reference) assert refs[0].next_accesses()[0] == refs[1] - assert refs[1].previous_access() == refs[0] - assert refs[0].previous_access() is None + assert refs[1].previous_accesses()[0] == refs[0] + assert len(refs[0].previous_accesses()) == 0 assert len(refs[1].next_accesses()) == 0 -def test_reference_previous_access_with_codeblock(fortran_reader): - ''' Test when te previous_access is a Codeblock. ''' +def test_reference_previous_accesses_with_codeblock(fortran_reader): + ''' Test when te previous_accesses is a Codeblock. ''' code = '''subroutine my_sub() character, dimension(100) :: a write(a, "A") "mytest" @@ -516,7 +547,7 @@ def test_reference_previous_access_with_codeblock(fortran_reader): routine = psyir.children[0] a = routine.children[1].lhs codeblock = routine.children[1] - if a.previous_access() is not codeblock: + if a.previous_accesses() is not codeblock: pytest.xfail("#2271 Codeblocks don't currently support " "reference_accesses") From 2e71fd6ef30f9a32b17059c92fc1a12a356569a6 Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Tue, 3 Dec 2024 14:50:00 +0000 Subject: [PATCH 07/12] Fixed linting errors --- src/psyclone/psyir/nodes/reference.py | 2 +- src/psyclone/tests/psyir/nodes/reference_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/psyclone/psyir/nodes/reference.py b/src/psyclone/psyir/nodes/reference.py index fe1f3404e8..7b4b721328 100644 --- a/src/psyclone/psyir/nodes/reference.py +++ b/src/psyclone/psyir/nodes/reference.py @@ -40,7 +40,7 @@ ''' This module contains the implementation of the Reference node.''' -from psyclone.core import AccessType, Signature, VariablesAccessInfo +from psyclone.core import AccessType, Signature # We cannot import from 'nodes' directly due to circular import from psyclone.psyir.nodes.datanode import DataNode from psyclone.psyir.symbols import Symbol diff --git a/src/psyclone/tests/psyir/nodes/reference_test.py b/src/psyclone/tests/psyir/nodes/reference_test.py index d01cef56fb..f9e91974b6 100644 --- a/src/psyclone/tests/psyir/nodes/reference_test.py +++ b/src/psyclone/tests/psyir/nodes/reference_test.py @@ -396,7 +396,7 @@ def test_reference_previous_accesses(fortran_reader): assert len(b.previous_accesses()) == 0 assert a_2.previous_accesses()[0] is a assert b_2.previous_accesses()[0] is b - + code = '''subroutine my_sub() integer :: a integer :: b @@ -411,7 +411,7 @@ def test_reference_previous_accesses(fortran_reader): loop = routine.children[1] b_a = loop.loop_body.children[0].lhs a_2 = loop.loop_body.children[0].rhs - assert len(a.previous_accesses()) == 0 + assert len(a.previous_accesses()) == 0 assert len(b_a.previous_accesses()) == 1 assert b_a.previous_accesses()[0] is b_a assert len(a_2.previous_accesses()) == 1 From dcc6cb12671583cbcfc1ac4a13754e0778895066 Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Tue, 3 Dec 2024 15:14:43 +0000 Subject: [PATCH 08/12] Readded an incorrect piece of removed arithmetic --- src/psyclone/psyir/tools/definition_use_chains.py | 2 +- .../tools/definition_use_chains_backward_dependence_test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index 6405dab2a3..389524fc30 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -867,7 +867,7 @@ def find_backward_accesses(self): body = ancestor.loop_body.children[:] # Find the stop point - this needs to be the last node # in the ancestor loop - sub_stop_point = ancestor.walk(Node)[-1].abs_position + sub_stop_point = ancestor.walk(Node)[-1].abs_position + 1 # We make a copy of the reference to have a detached # node to avoid handling the special cases based on # the parents of the reference. 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 d8c4f0b7ae..3240775768 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 @@ -323,7 +323,7 @@ def test_definition_use_chain_find_backward_accesses_while_loop_example( assert reaches[3] is routine.children[1].lhs -def test_definition_use_chain_foward_accesses_nested_loop_example( +def test_definition_use_chain_backward_accesses_nested_loop_example( fortran_reader, ): """Functionality test for the find_backward_accesses routine. This From 7d814615df65492c487a2afbe67a90557b688b99 Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:15:30 +0000 Subject: [PATCH 09/12] Changes for review --- .../psyir/tools/definition_use_chains.py | 24 ++++++++----------- .../tests/psyir/nodes/reference_test.py | 14 +++++------ ...tion_use_chains_forward_dependence_test.py | 4 ++-- 3 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/psyclone/psyir/tools/definition_use_chains.py b/src/psyclone/psyir/tools/definition_use_chains.py index 389524fc30..260c5847c7 100644 --- a/src/psyclone/psyir/tools/definition_use_chains.py +++ b/src/psyclone/psyir/tools/definition_use_chains.py @@ -91,7 +91,7 @@ def __init__( ): if not isinstance(reference, Reference): raise TypeError( - f"The reference passed into a DefinitionUseChain " + f"The 'reference' argument passed into a DefinitionUseChain " f"must be a Reference but found " f"'{type(reference).__name__}'." ) @@ -331,8 +331,8 @@ def find_forward_accesses(self): self._stop_point = save_stop_position return self._reaches else: - # We assume that the control flow here could not be taken, - # i.e. that this doesn't kill the chain. + # We assume that the control flow here could be 'not + # taken', i.e. that this doesn't kill the chain. # TODO #2760: In theory we could analyse loop structures # or if block structures to see if we're guaranteed to # write to the symbol. @@ -455,9 +455,8 @@ def _compute_forward_uses(self, basic_block_list): # positives for structure accesses inside CodeBlocks. if isinstance(reference._fp2_nodes[0], Goto_Stmt): raise NotImplementedError( - "DefinitionUseChains can't " - "handle code containing GOTO" - " statements." + "DefinitionUseChains can't handle code containing" + " GOTO statements." ) # If we find an Exit or Cycle statement, we can't # reach further in this code region so we can return. @@ -622,9 +621,9 @@ def _find_basic_blocks(self, nodelist): refs = node.else_body.walk(Reference) for ref in refs: if ref is self._reference: + # If its in the else_body we don't add the if_body in_else_body = True break - # If its in the else_body we don't add the if_body if not in_else_body: control_flow_nodes.append(node) basic_blocks.append(node.if_body.children[:]) @@ -706,9 +705,8 @@ def _compute_backward_uses(self, basic_block_list): # positives for structure accesses inside CodeBlocks. if isinstance(reference._fp2_nodes[0], Goto_Stmt): raise NotImplementedError( - "DefinitionUseChains can't " - "handle code containing GOTO" - " statements." + "DefinitionUseChains can't handle code containing" + " GOTO statements." ) if ( self._reference.symbol.name @@ -740,7 +738,6 @@ def _compute_backward_uses(self, basic_block_list): elif reference.get_signature_and_indices()[0] == sig: # Work out if its read only or not. assign = reference.ancestor(Assignment) - # TODO Need to invert how we think about assignments. # RHS reads occur "before" LHS writes, so if we # hit the LHS or an assignment then we won't have # a dependency to the value used from the LHS. @@ -951,8 +948,8 @@ def find_backward_accesses(self): self._stop_point = save_stop_position return self._reaches else: - # We assume that the control flow here could not be taken, - # i.e. that this doesn't kill the chain. + # We assume that the control flow here could be 'not + # taken', i.e. that this doesn't kill the chain. # TODO #2760: In theory we could analyse loop structures # or if block structures to see if we're guaranteed to # write to the symbol. @@ -984,7 +981,6 @@ def find_backward_accesses(self): # Check if there is an ancestor Assignment. ancestor = self._reference.ancestor(Assignment) if ancestor is not None: - # TODO Need to fix this - not sure what is going on here. # If we get here to check the start part of a loop we need # to handle this differently. # If the reference is the lhs then we can ignore the RHS. diff --git a/src/psyclone/tests/psyir/nodes/reference_test.py b/src/psyclone/tests/psyir/nodes/reference_test.py index f9e91974b6..64fa550b1d 100644 --- a/src/psyclone/tests/psyir/nodes/reference_test.py +++ b/src/psyclone/tests/psyir/nodes/reference_test.py @@ -247,12 +247,12 @@ def test_reference_next_accesses(fortran_reader): end subroutine''' psyir = fortran_reader.psyir_from_source(code) routine = psyir.children[0] - a = routine.children[0].lhs + a_before_loop = routine.children[0].lhs loop = routine.children[1] b = loop.loop_body.children[0].lhs - a_2 = loop.loop_body.children[0].rhs - assert len(a.next_accesses()) == 1 - assert a.next_accesses()[0] is loop + a_in_loop = loop.loop_body.children[0].rhs + assert len(a_before_loop.next_accesses()) == 1 + assert a_before_loop.next_accesses()[0] is loop assert len(b.next_accesses()) == 1 assert b.next_accesses()[0] == b @@ -268,11 +268,11 @@ def test_reference_next_accesses(fortran_reader): end subroutine''' psyir = fortran_reader.psyir_from_source(code) routine = psyir.children[0] - a = routine.children[1].lhs + a_after_loop = routine.children[1].lhs loop = routine.children[0] b = loop.loop_body.children[0].lhs - a_2 = loop.loop_body.children[0].rhs - assert len(a.next_accesses()) == 0 + a_in_loop = loop.loop_body.children[0].rhs + assert len(a_after_loop.next_accesses()) == 0 # Check the function for basic structures code = '''subroutine my_sub() 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 0047bf9baa..010652fcab 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 @@ -115,8 +115,8 @@ def test_definition_use_chain_init_and_properties(fortran_reader): # Test remaining TypeErrors with pytest.raises(TypeError) as excinfo: duc = DefinitionUseChain("123") - assert ("The reference passed into a DefinitionUseChain must be a " - "Reference but found 'str'." in str(excinfo.value)) + assert ("The 'reference' argument passed into a DefinitionUseChain must " + "be a Reference but found 'str'." in str(excinfo.value)) with pytest.raises(TypeError) as excinfo: duc = DefinitionUseChain(r1, start_point="123") assert ("The start_point passed into a DefinitionUseChain must be an " From f8cb8cedb984c4391b0471568de7b75434ca8f9d Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Wed, 11 Dec 2024 15:42:33 +0000 Subject: [PATCH 10/12] Changes for review --- src/psyclone/tests/psyir/nodes/reference_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/psyclone/tests/psyir/nodes/reference_test.py b/src/psyclone/tests/psyir/nodes/reference_test.py index 64fa550b1d..7b7e3d9cfe 100644 --- a/src/psyclone/tests/psyir/nodes/reference_test.py +++ b/src/psyclone/tests/psyir/nodes/reference_test.py @@ -271,7 +271,6 @@ def test_reference_next_accesses(fortran_reader): a_after_loop = routine.children[1].lhs loop = routine.children[0] b = loop.loop_body.children[0].lhs - a_in_loop = loop.loop_body.children[0].rhs assert len(a_after_loop.next_accesses()) == 0 # Check the function for basic structures From eb93169d35790c706f1ca19d12c97e2d565874a6 Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Thu, 2 Jan 2025 11:08:10 +0000 Subject: [PATCH 11/12] Fixed flaking error --- src/psyclone/tests/psyir/nodes/reference_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/psyclone/tests/psyir/nodes/reference_test.py b/src/psyclone/tests/psyir/nodes/reference_test.py index 7b7e3d9cfe..ea8f5f4f77 100644 --- a/src/psyclone/tests/psyir/nodes/reference_test.py +++ b/src/psyclone/tests/psyir/nodes/reference_test.py @@ -250,7 +250,6 @@ def test_reference_next_accesses(fortran_reader): a_before_loop = routine.children[0].lhs loop = routine.children[1] b = loop.loop_body.children[0].lhs - a_in_loop = loop.loop_body.children[0].rhs assert len(a_before_loop.next_accesses()) == 1 assert a_before_loop.next_accesses()[0] is loop assert len(b.next_accesses()) == 1 From e8273f0e0d210522a449b384b9c01d16d9d9155f Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Mon, 13 Jan 2025 10:27:30 +0000 Subject: [PATCH 12/12] #2704 Update changelog and documentation --- changelog | 3 +++ doc/developer_guide/dependency.rst | 6 +++--- .../tools/definition_use_chains_backward_dependence_test.py | 4 ---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/changelog b/changelog index 8fb6369f5e..fe5ec70972 100644 --- a/changelog +++ b/changelog @@ -29,6 +29,9 @@ 10) PR #2810. Adds caching of the fparser2 parse tree to FileInfo. Is disabled by default. + 11) PR #2814 for #2704. Adds backward accesses capabilities to the + DefinitionUseChain tool. + release 3.0.0 6th of December 2024 1) PR #2477 for #2463. Add support for Fortran Namelist statements. diff --git a/doc/developer_guide/dependency.rst b/doc/developer_guide/dependency.rst index e110aef8d3..b4974f6a9d 100644 --- a/doc/developer_guide/dependency.rst +++ b/doc/developer_guide/dependency.rst @@ -666,12 +666,12 @@ can be parallelised: DefinitionUseChain ================== PSyclone also provides a DefinitionUseChain class, which can search for forward -dependencies (backward NYI) for a given Reference inside a region of code. This +and backward dependencies for a given Reference inside a region of code. This implementation differs from the DependencyTools as it is control-flow aware, so can find many dependencies for a single Reference in a given Routine or scope. -This is primarily used to implement the `References.next_accesses` function, but can be -used directly as follows: +This is primarily used to implement the `Reference.next_accesses` and +`Reference.previous_accessess` functions, but can be used directly as follows: .. code:: 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 3240775768..bafc2c92c8 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 @@ -253,9 +253,6 @@ def test_definition_use_chain_find_backward_accesses_loop_example( # Second a = A + i # Third (?) is A = a + i # Last is A = 1 - print(reaches[0].parent.debug_string()) - print(reaches[1].parent.debug_string()) - print(reaches[2].parent.debug_string()) assert len(reaches) == 4 assert ( reaches[0] is routine.children[1].loop_body.children[1].rhs.children[0] @@ -460,7 +457,6 @@ def test_definition_use_chain_find_backward_accesses_codeblock_and_call_cflow( chains = DefinitionUseChain(routine.children[2].rhs.children[0]) reaches = chains.find_backward_accesses() assert len(reaches) == 2 - print([x.parent.debug_string() for x in reaches]) assert reaches[0] is routine.children[1].if_body.children[1].children[1] assert reaches[1] is routine.children[0]