Skip to content

Commit

Permalink
Merge pull request #2465 from stfc/2004_move_same_range
Browse files Browse the repository at this point in the history 
(towards 2004) Move same_range functionality to ArrayMixin and other improvements
  • Loading branch information
@hiker
hiker authored Feb 1, 2024
2 parents a8cd7b5 + 1bbf44b commit 699ceb2
Show file tree
Hide file tree
Showing 7 changed files with 283 additions and 253 deletions.
3 changes: 3 additions & 0 deletions changelog
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@

54) PR #2493 for #2492. Fix bug in NemoOuterArrayRange2LoopTrans

55) PR #2465 towards #2004. Move same_range functionality to
ArrayMixin and other improvements.

release 2.4.0 29th of September 2023

1) PR #1758 for #1741. Splits the PSyData read functionality into a
Expand Down
31 changes: 18 additions & 13 deletions src/psyclone/psyir/frontend/fortran.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

''' This module provides the PSyIR Fortran front-end.'''

from typing import Optional
from fparser.common.readfortran import FortranStringReader, FortranFileReader
from fparser.common.sourceinfo import FortranFormat
from fparser.two import Fortran2003, pattern_tools
Expand Down Expand Up @@ -63,12 +64,12 @@ def __init__(self):
SYMBOL_TABLES.clear()

@staticmethod
def validate_name(name):
def validate_name(name: str):
'''
Utility method that checks that the supplied name is a valid
Fortran name.
:param str name: the name to check.
:param name: the name to check.
:raises TypeError: if the name is not a string.
:raises ValueError: if this is not a valid name.
Expand All @@ -82,11 +83,11 @@ def validate_name(name):
raise ValueError(
f"Invalid Fortran name '{name}' found.")

def psyir_from_source(self, source_code, free_form=True):
def psyir_from_source(self, source_code: str, free_form: bool = True):
''' Generate the PSyIR tree representing the given Fortran source code.
:param str source_code: text representation of the code to be parsed.
:param bool free_form: If parsing free-form code or not (default True).
:param source_code: text representation of the code to be parsed.
:param free_form: If parsing free-form code or not (default True).
:returns: PSyIR representing the provided Fortran source code.
:rtype: :py:class:`psyclone.psyir.nodes.Node`
Expand All @@ -100,15 +101,15 @@ def psyir_from_source(self, source_code, free_form=True):
psyir = self._processor.generate_psyir(parse_tree)
return psyir

def psyir_from_expression(self, source_code, symbol_table):
def psyir_from_expression(self, source_code: str,
symbol_table: Optional[SymbolTable] = None):
'''Generate the PSyIR tree for the supplied Fortran statement. The
symbol table is expected to provide all symbols found in the
expression.
:param str source_code: text of the expression to be parsed.
:param source_code: text of the expression to be parsed.
:param symbol_table: the SymbolTable in which to search for any
symbols that are encountered.
:type symbol_table: :py:class:`psyclone.psyir.symbols.SymbolTable`
:returns: PSyIR representing the provided Fortran expression.
:rtype: :py:class:`psyclone.psyir.nodes.Node`
Expand All @@ -118,7 +119,9 @@ def psyir_from_expression(self, source_code, symbol_table):
Fortran expression.
'''
if not isinstance(symbol_table, SymbolTable):
if symbol_table is None:
symbol_table = SymbolTable()
elif not isinstance(symbol_table, SymbolTable):
raise TypeError(f"Must be supplied with a valid SymbolTable but "
f"got '{type(symbol_table).__name__}'")

Expand All @@ -142,15 +145,15 @@ def psyir_from_expression(self, source_code, symbol_table):
self._processor.process_nodes(fake_parent[0], [parse_tree])
return fake_parent[0].children[0].detach()

def psyir_from_statement(self, source_code, symbol_table):
def psyir_from_statement(self, source_code: str,
symbol_table: Optional[SymbolTable] = None):
'''Generate the PSyIR tree for the supplied Fortran statement. The
symbolt table is expected to provide all symbols found in the
statement.
:param str source_code: text of the statement to be parsed.
:param source_code: text of the statement to be parsed.
:param symbol_table: the SymbolTable in which to search for any
symbols that are encountered.
:type symbol_table: :py:class:`psyclone.psyir.symbols.SymbolTable`
:returns: PSyIR representing the provided Fortran statement.
:rtype: :py:class:`psyclone.psyir.nodes.Node`
Expand All @@ -160,7 +163,9 @@ def psyir_from_statement(self, source_code, symbol_table):
Fortran statement.
'''
if not isinstance(symbol_table, SymbolTable):
if symbol_table is None:
symbol_table = SymbolTable()
elif not isinstance(symbol_table, SymbolTable):
raise TypeError(f"Must be supplied with a valid SymbolTable but "
f"got '{type(symbol_table).__name__}'")
string_reader = FortranStringReader(source_code)
Expand Down
124 changes: 122 additions & 2 deletions src/psyclone/psyir/nodes/array_mixin.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,8 @@ def _is_bound_op(self, expr, bound_operator, index):
:type expr: :py:class:`psyclone.psyir.nodes.Node`
:param bound_operator: the particular bound operation.
:type bound_operator:
:py:class:`psyclone.psyir.nodes.IntrinsicCall.Intrinsic.LBOUND |
:py:class:`psyclone.psyir.nodes.IntrinsicCall.Intrinsic.UBOUND
:py:class:`psyclone.psyir.nodes.IntrinsicCall.Intrinsic.LBOUND` |
:py:class:`psyclone.psyir.nodes.IntrinsicCall.Intrinsic.UBOUND`
:param int index: the bounds index.
:returns: True if the expr is in the expected form and False otherwise.
Expand Down Expand Up @@ -598,6 +598,126 @@ def get_outer_range_index(self):
return child.position
raise IndexError

def same_range(self, index: int, array2, index2: int) -> bool:
''' This method compares the range of this array node at a given index
with the range of a second array at a second index. This is useful to
verify if array operations are valid, e.g.: A(3,:,5) + B(:,2,2).
Note that this check supports symbolic comparisons, e.g.:
A(3:4) has the same range as B(2+1:5-1),
and will consider compile-time unknown dimensions as equal, e.g.:
A(:) has the same range as B(:).
TODO #2485. This method has false negatives: cases when the range
is the same but it can not be proved, so we return False.
TODO #2004. This method currently compares exact ranges, not just the
length of them, which could be done with "(upper-lower)/step" symbolic
comparisons. This is because arrayrange2loop does not account for
arrays declared with different lbounds, but this could be improved.
:param index: the index indicating the location of a range node in
this array.
:param array2: the array accessor that we want to compare it to.
:param index2: the index indicating the location of a range node in
array2.
:returns: True if the ranges are the same and False if they are not
the same, or if it is not possible to determine.
:raises: TypeError if any of the arguments are of the wrong type.
'''
# pylint: disable=too-many-branches
if not isinstance(index, int):
raise TypeError(
f"The 'index' argument of the same_range() method should be an"
f" int but found '{type(index).__name__}'.")
if not isinstance(array2, ArrayMixin):
raise TypeError(
f"The 'array2' argument of the same_range() method should be "
f"an ArrayMixin but found '{type(array2).__name__}'.")
if not isinstance(index2, int):
raise TypeError(
f"The 'index2' argument of the same_range() method should be "
f"an int but found '{type(index2).__name__}'.")
if not index < len(self.children):
raise IndexError(
f"The value of the 'index' argument of the same_range() method"
f" is '{index}', but it should be less than the number of "
f"dimensions in the associated array, which is "
f"'{len(self.children)}'.")
if not index2 < len(array2.children):
raise IndexError(
f"The value of the 'index2' argument of the same_range() "
f"method is '{index2}', but it should be less than the number"
f" of dimensions in the associated array 'array2', which is "
f"'{len(array2.children)}'.")
if not isinstance(self.children[index], Range):
raise TypeError(
f"The child of the first array argument at the specified index"
f" '{index}' should be a Range node, but found "
f"'{type(self.children[index]).__name__}'.")
if not isinstance(array2.children[index2], Range):
raise TypeError(
f"The child of the second array argument at the specified "
f"index '{index2}' should be a Range node, but found "
f"'{type(array2.children[index2]).__name__}'.")

range1 = self.children[index]
range2 = array2.children[index2]

sym_maths = SymbolicMaths.get()
# compare lower bounds
if self.is_lower_bound(index) and array2.is_lower_bound(index2):
# Both self and array2 use the lbound() intrinsic to
# specify the lower bound of the array dimension. We may
# not be able to determine what the lower bounds of these
# arrays are statically but at runtime the code will fail
# if the ranges do not match so we assume that the lower
# bounds are consistent.
pass
elif self.is_lower_bound(index) or array2.is_lower_bound(index2):
# One and only one of self and array2 use the lbound()
# intrinsic to specify the lower bound of the array
# dimension. In this case assume that the ranges are
# different (although they could potentially be the same).
return False
elif not sym_maths.equal(range1.start, range2.start):
# Neither self nor array2 use the lbound() intrinsic to
# specify the lower bound of the array dimension. Try to
# determine if they are the same by matching the
# text. Use symbolic maths to do the comparison.
return False

# compare upper bounds
if self.is_upper_bound(index) and array2.is_upper_bound(index2):
# Both self and array2 use the ubound() intrinsic to
# specify the upper bound of the array dimension. We may
# not be able to determine what the upper bounds of these
# arrays are statically but at runtime the code will fail
# if the ranges do not match so we assume that the upper
# bounds are consistent.
pass
elif self.is_upper_bound(index) or array2.is_upper_bound(index2):
# One and only one of self and array2 use the ubound()
# intrinsic to specify the upper bound of the array
# dimension. In this case assume that the ranges are
# different (although they could potentially be the same).
return False
elif not sym_maths.equal(range1.stop, range2.stop):
# Neither self nor array2 use the ubound() intrinsic to
# specify the upper bound of the array dimension. Use
# symbolic maths to check if they are equal.
return False

# compare steps
if not sym_maths.equal(range1.step, range2.step):
return False

# Everything matches.
return True


# For AutoAPI documentation generation
__all__ = ['ArrayMixin']
125 changes: 1 addition & 124 deletions src/psyclone/psyir/transformations/arrayrange2loop_trans.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
'''

from psyclone.core import SymbolicMaths
from psyclone.psyGen import Transformation
from psyclone.psyir.nodes import Loop, Range, Reference, ArrayReference, \
Assignment, Call, IntrinsicCall
Expand Down Expand Up @@ -78,127 +77,6 @@ class ArrayRange2LoopTrans(Transformation):
'''

@staticmethod
def same_range(array1, idx1, array2, idx2):
'''This method compares the range node at position 'idx1' in array
access 'array1' with the range node at position 'idx2' in
array access 'array2'.
The natural place to test the equivalence of two ranges is in
the Range class. However, the test required here is slightly
different as it is valid to assume that two array slices are
the same even if their actual sizes are not known (as the code
would raise a runtime exception if this were not the case).
:param array1: an array node containing a range node at index idx1.
:type array1: py:class:`psyclone.psyir.node.ArrayReference`
:param int idx1: an index indicating the location of a range \
node in array1 (in its children list).
:param array2: an array node containing a range node at index idx2.
:type array2: py:class:`psyclone.psyir.node.ArrayReference`
:param int idx2: an index indicating the location of a range \
node in array2 (in its children list).
:returns: True if the ranges are the same and False if they \
are not the same, or if it is not possible to determine.
:rtype: bool
:raises: TypeError if one or more of the arguments are of the \
wrong type.
'''
# pylint: disable=too-many-branches
if not isinstance(array1, ArrayReference):
raise TypeError(
f"The first argument to the same_range() method should be an "
f"ArrayReference but found '{type(array1).__name__}'.")
if not isinstance(idx1, int):
raise TypeError(
f"The second argument to the same_range() method should be an "
f"int but found '{type(idx1).__name__}'.")
if not isinstance(array2, ArrayReference):
raise TypeError(
f"The third argument to the same_range() method should be an "
f"ArrayReference but found '{type(array2).__name__}'.")
if not isinstance(idx2, int):
raise TypeError(
f"The fourth argument to the same_range() method should be an "
f"int but found '{type(idx2).__name__}'.")
if not idx1 < len(array1.children):
raise IndexError(
f"The value of the second argument to the same_range() method "
f"'{idx1}' should be less than the number of dimensions "
f"'{len(array1.children)}' in the associated array 'array1'.")
if not idx2 < len(array2.children):
raise IndexError(
f"The value of the fourth argument to the same_range() method "
f"'{idx2}' should be less than the number of dimensions "
f"'{len(array2.children)}' in the associated array 'array2'.")
if not isinstance(array1.children[idx1], Range):
raise TypeError(
f"The child of the first array argument at the specified index"
f" ({idx1}) should be a Range node, but found "
f"'{type(array1.children[idx1]).__name__}'.")
if not isinstance(array2.children[idx2], Range):
raise TypeError(
f"The child of the second array argument at the specified "
f"index ({idx2}) should be a Range node, but found "
f"'{type(array2.children[idx2]).__name__}'.")

range1 = array1.children[idx1]
range2 = array2.children[idx2]

sym_maths = SymbolicMaths.get()
# compare lower bounds
if array1.is_lower_bound(idx1) and array2.is_lower_bound(idx2):
# Both array1 and array2 use the lbound() intrinsic to
# specify the lower bound of the array dimension. We may
# not be able to determine what the lower bounds of these
# arrays are statically but at runtime the code will fail
# if the ranges do not match so we assume that the lower
# bounds are consistent.
pass
elif array1.is_lower_bound(idx1) or array2.is_lower_bound(idx2):
# One and only one of array1 and array2 use the lbound()
# intrinsic to specify the lower bound of the array
# dimension. In this case assume that the ranges are
# different (although they could potentially be the same).
return False
elif not sym_maths.equal(range1.start, range2.start):
# Neither array1 nor array2 use the lbound() intrinsic to
# specify the lower bound of the array dimension. Try to
# determine if they are the same by matching the
# text. Use symbolic maths to do the comparison.
return False

# compare upper bounds
if array1.is_upper_bound(idx1) and array2.is_upper_bound(idx2):
# Both array1 and array2 use the ubound() intrinsic to
# specify the upper bound of the array dimension. We may
# not be able to determine what the upper bounds of these
# arrays are statically but at runtime the code will fail
# if the ranges do not match so we assume that the upper
# bounds are consistent.
pass
elif array1.is_upper_bound(idx1) or array2.is_upper_bound(idx2):
# One and only one of array1 and array2 use the ubound()
# intrinsic to specify the upper bound of the array
# dimension. In this case assume that the ranges are
# different (although they could potentially be the same).
return False
elif not sym_maths.equal(range1.stop, range2.stop):
# Neither array1 nor array2 use the ubound() intrinsic to
# specify the upper bound of the array dimension. Use
# symbolic maths to check if they are equal.
return False

# compare steps
if not sym_maths.equal(range1.step, range2.step):
return False

# Everything matches.
return True

def apply(self, node, options=None):
'''Apply the ArrayRange2Loop transformation to the specified node. The
node must be an assignment. The rightmost range node in each array
Expand Down Expand Up @@ -322,8 +200,7 @@ def validate(self, node, options=None):
# loop variables where the ranges are
# different, or occur in different index
# locations.
if not ArrayRange2LoopTrans.same_range(
node.lhs, lhs_index, array, idx):
if not node.lhs.same_range(lhs_index, array, idx):
# Ranges are, or may be, different so we
# can't safely replace this range with a
# loop iterator.
Expand Down
Loading

0 comments on commit 699ceb2

Please sign in to comment.