From 40cc9554a47b053762b64d27aa7d654e749eb106 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 7 Aug 2024 11:30:22 +0100 Subject: [PATCH 01/15] #2027 fix bug in ArrayMixin._get_effective_shape() --- src/psyclone/psyir/nodes/array_mixin.py | 12 ++++++++++-- src/psyclone/tests/psyir/nodes/array_mixin_test.py | 8 ++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/psyclone/psyir/nodes/array_mixin.py b/src/psyclone/psyir/nodes/array_mixin.py index 5c992f0638..ab2b35eff4 100644 --- a/src/psyclone/psyir/nodes/array_mixin.py +++ b/src/psyclone/psyir/nodes/array_mixin.py @@ -621,7 +621,7 @@ def _get_effective_shape(self): dtype = idx_expr.datatype if isinstance(dtype, ArrayType): # An array slice can be defined by a 1D slice of another - # array, e.g. `a(b(1:4))`. + # array, e.g. `a(b(1:4))` or `a(b)`. indirect_array_shape = dtype.shape if len(indirect_array_shape) > 1: raise NotImplementedError( @@ -630,7 +630,15 @@ def _get_effective_shape(self): f"used to index into '{self.name}' has " f"{len(indirect_array_shape)} dimensions.") # pylint: disable=protected-access - shape.append(idx_expr._extent(idx)) + if isinstance(idx_expr, ArrayMixin): + shape.append(idx_expr._extent(idx)) + else: + # We have a Reference (to an array) with no explicit + # indexing. The extent of this is then the SIZE of + # that array. + sizeop = IntrinsicCall.create( + IntrinsicCall.Intrinsic.SIZE, [idx_expr.copy()]) + shape.append(sizeop) elif isinstance(idx_expr, (Call, Operation, CodeBlock)): # We can't yet straightforwardly query the type of a function diff --git a/src/psyclone/tests/psyir/nodes/array_mixin_test.py b/src/psyclone/tests/psyir/nodes/array_mixin_test.py index f9290e82b3..0ad384422e 100644 --- a/src/psyclone/tests/psyir/nodes/array_mixin_test.py +++ b/src/psyclone/tests/psyir/nodes/array_mixin_test.py @@ -608,6 +608,7 @@ def test_get_effective_shape(fortran_reader): code = ( "subroutine test()\n" " use some_mod\n" + " integer :: idx = 2\n" " integer :: indices(8,3)\n" " real a(10), b(10,10)\n" " a(1:10) = 0.0\n" @@ -621,6 +622,7 @@ def test_get_effective_shape(fortran_reader): " b(indices(2:3,1:2), 2:5) = 2.0\n" " a(f()) = 2.0\n" " a(2+3) = 1.0\n" + " b(idx, a) = -1.0\n" "end subroutine\n") psyir = fortran_reader.psyir_from_source(code) routine = psyir.walk(Routine)[0] @@ -691,6 +693,12 @@ def test_get_effective_shape(fortran_reader): with pytest.raises(NotImplementedError) as err: _ = routine.children[child_idx].lhs._get_effective_shape() assert "include a function call or expression" in str(err.value) + # Array access with indices given by another array that is not explicitly + # indexed. + child_idx += 1 + shape = routine.children[child_idx].lhs._get_effective_shape() + assert len(shape) == 1 + assert "SIZE(a)" in shape[0].debug_string() # get_outer_range_index From 03510ff848bda787b4278d9e3753159d30af9134 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 7 Aug 2024 11:34:09 +0100 Subject: [PATCH 02/15] #2027 fix bug in ArrayAssignment2LoopsTrans.validate --- .../arrayassignment2loops_trans.py | 49 ++++++++++++++---- .../arrayassignment2loops_trans_test.py | 50 ++++++++++++++++++- 2 files changed, 86 insertions(+), 13 deletions(-) diff --git a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py index 0fc0238a5d..f10804c7e5 100644 --- a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py +++ b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py @@ -49,10 +49,10 @@ CodeBlock, Routine, BinaryOperation) from psyclone.psyir.nodes.array_mixin import ArrayMixin from psyclone.psyir.symbols import ( - DataSymbol, INTEGER_TYPE, ScalarType, UnresolvedType, SymbolError, - ArrayType) -from psyclone.psyir.transformations.transformation_error \ - import TransformationError + ArrayType, DataSymbol, INTEGER_TYPE, ScalarType, SymbolError, + UnresolvedType, UnsupportedType) +from psyclone.psyir.transformations.transformation_error import ( + TransformationError) from psyclone.psyir.transformations.reference2arrayrange_trans import ( Reference2ArrayRangeTrans) @@ -241,17 +241,16 @@ def validate(self, node, options=None): f"Range, but none were found in '{node.debug_string()}'.")) # All the ArrayMixins must have the same number of Ranges to expand - found = None + num_of_ranges = None for accessor in node.walk(ArrayMixin): - num_of_ranges = len([x for x in accessor.indices - if isinstance(x, Range)]) - if num_of_ranges > 0: - if not found: + count = len([x for x in accessor.indices if isinstance(x, Range)]) + if count: + if not num_of_ranges: # If it's the first value we find, we store it - found = num_of_ranges + num_of_ranges = count else: # Otherwise we compare it against the previous found value - if found != num_of_ranges: + if count != num_of_ranges: raise TransformationError(LazyString( lambda: f"{self.name} does not support statements" f" containing array accesses that have varying " @@ -368,6 +367,34 @@ def validate(self, node, options=None): # pylint: disable=cell-var-from-loop raise TransformationError(LazyString( lambda: f"{message} In:\n{node.debug_string()}")) + # We must understand any array accesses on the RHS. This means all + # array indices must be known (because otherwise they themselves + # might be arrays, e.g. + # a(:) = b(c) where `c` can be a scalar or an array. + if isinstance(reference, ArrayMixin): + # Check the index expressions + range_count = len([x for x in accessor.indices + if isinstance(x, Range)]) + if range_count != num_of_ranges: + # If the number of ranges in this access is not the same as + # on the LHS then we may or may not have a scalar. + for exprn in reference.indices: + if not isinstance(exprn, Reference): + continue + if not isinstance(exprn.datatype, (UnsupportedType, + UnresolvedType)): + continue + message = ( + f"{self.name} cannot expand expression because it " + f"contains the access '{reference.debug_string()}'" + f" and whether or not the index " + f"expression '{exprn.debug_string()}' is " + f"itself an array is unknown.") + if verbose: + node.append_preceding_comment(message) + # pylint: disable=cell-var-from-loop + raise TransformationError(LazyString( + lambda: f"{message} In:\n{node.debug_string()}")) __all__ = [ diff --git a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py index e9ffea258f..0a2cf1a045 100644 --- a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py @@ -303,6 +303,38 @@ def test_apply_to_arrays_with_different_bounds(fortran_reader, fortran_writer): in output_test4) +def test_apply_indirect_indexing(fortran_reader, fortran_writer): + ''' + Check the application of the transformation when the array is indexed + indirectly. + + ''' + psyir = fortran_reader.psyir_from_source(''' + program test + use dom_oce + INTEGER, DIMENSION(4) :: iwewe + INTEGER, DIMENSION(8,kfld) :: ishtSi + integer :: jf + iwewe(:) = (/ jpwe,jpea,jpwe,jpea /) + do jf = 1, kfld + ishtSi(5:8,jf) = ishtSi( iwewe,jf ) + end do + end program test + ''') + assignments = psyir.walk(Assignment) + # Only transform the second assignment as the first contains a + # CodeBlock. (We check this here so that this test can be extended + # if/when the CodeBlock is removed.) + assert isinstance(assignments[0].rhs, CodeBlock) + trans = ArrayAssignment2LoopsTrans() + trans.apply(assignments[1]) + result = fortran_writer(psyir) + assert (''' + do idx = 5, 8, 1 + ishtsi(idx,jf) = ishtsi(iwewe(idx + (1 - 5)),jf) + enddo''' in result) + + def test_apply_outside_routine(fortran_reader, fortran_writer): ''' Check that the transformation still succeeds and generates valid code when found in a scope detached from a Routine. ''' @@ -568,15 +600,18 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): ''' psyir = fortran_reader.psyir_from_source(''' subroutine test(unsupported) - use my_variables, only: unresolved + use my_variables, only: unresolved, map integer :: scalar = 1 integer, dimension(:) :: array = 1, x integer, dimension(:), optional :: unsupported - + integer, dimension(4) :: ishtsi x(:) = scalar x(:) = array x(:) = unresolved x(:) = unsupported + ! An indirectly-addressed RHS but we can't tell whether or not map is + ! an array reference + x(:) = ishtsi(map,scalar) end subroutine test ''') @@ -616,6 +651,13 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): "and therefore cannot be guaranteed to be ScalarType." in str(info.value)) + with pytest.raises(TransformationError) as info: + trans.apply(psyir.walk(Assignment)[4], opts) + assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " + "contains the access 'ishtsi(map,scalar)' and whether or not the " + "index expression 'map' is itself an array is unknown" in + str(info.value)) + # The end result should look like: assert ( " do idx = LBOUND(x, dim=1), UBOUND(x, dim=1), 1\n" @@ -634,6 +676,10 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): "Type('INTEGER, DIMENSION(:), OPTIONAL :: unsupported') and therefore " "cannot be guaranteed to be ScalarType.\n" " x(:) = unsupported\n" + " ! ArrayAssignment2LoopsTrans cannot expand expression because it " + "contains the access 'ishtsi(map,scalar)' and whether or not the " + "index expression 'map' is itself an array is unknown.\n" + " x(:) = ishtsi(map,scalar)\n" ) in fortran_writer(psyir) From 65e7ff98b08d3fb55d84a8ed93a7a7ef5a6f8b03 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 7 Aug 2024 11:51:02 +0100 Subject: [PATCH 03/15] #2027 extend utils.py to convert array refs used as array indices --- examples/nemo/scripts/utils.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index d00da0e639..80474f23f2 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -36,8 +36,8 @@ ''' Utilities file to parallelise Nemo code. ''' from psyclone.psyir.nodes import ( - Loop, Assignment, Directive, Container, Reference, CodeBlock, Call, - Return, IfBlock, Routine, IntrinsicCall) + ArrayMixin, Assignment, Loop, Directive, Container, Reference, CodeBlock, + Call, Return, IfBlock, Routine, IntrinsicCall) from psyclone.psyir.symbols import ( DataSymbol, INTEGER_TYPE, REAL_TYPE, ArrayType, ScalarType, RoutineSymbol, ImportInterface) @@ -194,6 +194,15 @@ def normalise_loops( Reference2ArrayRangeTrans().apply(reference) except TransformationError: pass + if isinstance(reference, ArrayMixin): + # Look at array-index expressions too. + for exprn in reference.indices: + if (isinstance(exprn, Reference) and + isinstance(exprn.symbol, DataSymbol)): + try: + Reference2ArrayRangeTrans().apply(exprn) + except TransformationError: + pass if loopify_array_intrinsics: for intr in schedule.walk(IntrinsicCall): From 33abf82229e9a68f075a5785d44a547f69815ca1 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 7 Aug 2024 12:13:21 +0100 Subject: [PATCH 04/15] #2027 fix error in utils.py --- examples/nemo/scripts/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index 80474f23f2..60b32d2266 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -36,7 +36,7 @@ ''' Utilities file to parallelise Nemo code. ''' from psyclone.psyir.nodes import ( - ArrayMixin, Assignment, Loop, Directive, Container, Reference, CodeBlock, + Assignment, Loop, Directive, Container, Reference, CodeBlock, Call, Return, IfBlock, Routine, IntrinsicCall) from psyclone.psyir.symbols import ( DataSymbol, INTEGER_TYPE, REAL_TYPE, ArrayType, ScalarType, @@ -194,7 +194,7 @@ def normalise_loops( Reference2ArrayRangeTrans().apply(reference) except TransformationError: pass - if isinstance(reference, ArrayMixin): + if hasattr(reference, "indices"): # Look at array-index expressions too. for exprn in reference.indices: if (isinstance(exprn, Reference) and From 2ec7260a1c1be6c75a1b5cc140770b40b16ad86f Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 28 Aug 2024 12:26:20 +0100 Subject: [PATCH 05/15] #2685 improve testing of ArrayAssignment2LoopsTrans --- .../arrayassignment2loops_trans.py | 19 +++-- .../arrayassignment2loops_trans_test.py | 83 +++++++++++++++---- 2 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py index f10804c7e5..4998eda19c 100644 --- a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py +++ b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py @@ -244,7 +244,7 @@ def validate(self, node, options=None): num_of_ranges = None for accessor in node.walk(ArrayMixin): count = len([x for x in accessor.indices if isinstance(x, Range)]) - if count: + if count > 0: if not num_of_ranges: # If it's the first value we find, we store it num_of_ranges = count @@ -321,8 +321,8 @@ def validate(self, node, options=None): lambda: f"{message} in:\n{node.debug_string()}.")) # For each top-level reference (because we don't support nesting), the - # apply will have to be able to decide if its an Array (and access it - # with the index) or an Scalar (and leave it as it is). We can not + # apply() will have to determine whether it's an Array (and access it + # with the index) or a Scalar (and leave it as it is). We cannot # transform references where this is unclear. for reference in node.walk(Reference, stop_type=Reference): if isinstance(reference.parent, Call): @@ -350,12 +350,12 @@ def validate(self, node, options=None): # scalar. if not isinstance(reference.datatype, (ScalarType, ArrayType)): if isinstance(reference.symbol, DataSymbol): - typestr = f"an {reference.symbol.datatype}" + typestr = f"an {reference.datatype}" else: typestr = "not a DataSymbol" message = ( f"{self.name} cannot expand expression because it " - f"contains the variable '{reference.symbol.name}' " + f"contains the access '{reference.debug_string()}' " f"which is {typestr} and therefore cannot be guaranteed" f" to be ScalarType.") if not isinstance(reference.symbol, DataSymbol) or \ @@ -370,16 +370,21 @@ def validate(self, node, options=None): # We must understand any array accesses on the RHS. This means all # array indices must be known (because otherwise they themselves # might be arrays, e.g. - # a(:) = b(c) where `c` can be a scalar or an array. + # a(:) = b(c) where `c` can be a scalar or an array + # or a(:) = grid%b(c) if isinstance(reference, ArrayMixin): # Check the index expressions range_count = len([x for x in accessor.indices if isinstance(x, Range)]) if range_count != num_of_ranges: # If the number of ranges in this access is not the same as - # on the LHS then we may or may not have a scalar. + # on the LHS then we may or may not have a scalar. To + # determine this we must know the type of every one of the + # index expressions. for exprn in reference.indices: if not isinstance(exprn, Reference): + # We don't have a Reference so it cannot be of + # array type (we rejected CodeBlocks earlier). continue if not isinstance(exprn.datatype, (UnsupportedType, UnresolvedType)): diff --git a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py index 0a2cf1a045..566ede719a 100644 --- a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py @@ -315,24 +315,26 @@ def test_apply_indirect_indexing(fortran_reader, fortran_writer): INTEGER, DIMENSION(4) :: iwewe INTEGER, DIMENSION(8,kfld) :: ishtSi integer :: jf - iwewe(:) = (/ jpwe,jpea,jpwe,jpea /) do jf = 1, kfld - ishtSi(5:8,jf) = ishtSi( iwewe,jf ) + ishtSi(5:8,jf) = ishtSi(iwewe, jf) + ishtSi(1:4,jf) = grid(3)%var(iwewe, jf) end do end program test ''') assignments = psyir.walk(Assignment) - # Only transform the second assignment as the first contains a - # CodeBlock. (We check this here so that this test can be extended - # if/when the CodeBlock is removed.) - assert isinstance(assignments[0].rhs, CodeBlock) trans = ArrayAssignment2LoopsTrans() + trans.apply(assignments[0]) trans.apply(assignments[1]) result = fortran_writer(psyir) assert (''' do idx = 5, 8, 1 ishtsi(idx,jf) = ishtsi(iwewe(idx + (1 - 5)),jf) enddo''' in result) + assert (''' + do idx_1 = 1, 4, 1 + ishtsi(idx_1,jf) = grid(3)%var(iwewe(idx_1),jf) + enddo + enddo''' in result) def test_apply_outside_routine(fortran_reader, fortran_writer): @@ -629,7 +631,7 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): with pytest.raises(TransformationError) as info: trans.apply(assign_with_unresolved, opts) assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unresolved' which is not a DataSymbol " + "contains the access 'unresolved' which is not a DataSymbol " "and therefore cannot be guaranteed to be ScalarType. Resolving " "the import that brings this variable into scope may help." in str(info.value)) @@ -639,14 +641,14 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): with pytest.raises(TransformationError) as info: trans.apply(assign_with_unresolved) assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unresolved' which is an UnresolvedType " + "contains the access 'unresolved' which is an UnresolvedType " "and therefore cannot be guaranteed to be ScalarType. Resolving " "the import that brings this variable into scope may help." in str(info.value)) with pytest.raises(TransformationError) as info: trans.apply(psyir.walk(Assignment)[3], opts) assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unsupported' which is an Unsupported" + "contains the access 'unsupported' which is an Unsupported" "FortranType('INTEGER, DIMENSION(:), OPTIONAL :: unsupported') " "and therefore cannot be guaranteed to be ScalarType." in str(info.value)) @@ -667,12 +669,12 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): " x(idx_1) = array(idx_1)\n" " enddo\n" " ! ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unresolved' which is not a DataSymbol and " + "contains the access 'unresolved' which is not a DataSymbol and " "therefore cannot be guaranteed to be ScalarType. Resolving the import" " that brings this variable into scope may help.\n" " x(:) = unresolved\n" " ! ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unsupported' which is an UnsupportedFortran" + "contains the access 'unsupported' which is an UnsupportedFortran" "Type('INTEGER, DIMENSION(:), OPTIONAL :: unsupported') and therefore " "cannot be guaranteed to be ScalarType.\n" " x(:) = unsupported\n" @@ -731,7 +733,7 @@ def test_validate_non_elemental_functions(fortran_reader): with pytest.raises(TransformationError) as err: trans.validate(assignment3, options={"verbose": True}) errormsg = ("ArrayAssignment2LoopsTrans cannot expand expression because " - "it contains the variable 'myfunc' which is not a DataSymbol " + "it contains the access 'myfunc(y)' which is not a DataSymbol " "and therefore cannot be guaranteed to be ScalarType. " "Resolving the import that brings this variable into scope " "may help.") @@ -741,9 +743,58 @@ def test_validate_non_elemental_functions(fortran_reader): with pytest.raises(TransformationError) as err: trans.validate(assignment4, options={"verbose": True}) errormsg = ("ArrayAssignment2LoopsTrans cannot expand expression because " - "it contains the variable 'var' which is an UnresolvedType " - "and therefore cannot be guaranteed to be ScalarType. " - "Resolving the import that brings this variable into scope " - "may help.") + "it contains the access 'var%myfunc(y)' which is an " + "UnresolvedType and therefore cannot be guaranteed to be " + "ScalarType. Resolving the import that brings this variable " + "into scope may help.") assert errormsg in assignment4.preceding_comment assert errormsg in str(err.value) + + +def test_validate_indirect_indexing(fortran_reader): + ''' + Check the validation of the transformation when the array is indexed + indirectly in unsupported ways. + + ''' + psyir = fortran_reader.psyir_from_source(''' + program test + use dom_oce + INTEGER, DIMENSION(4) :: iwewe + INTEGER, DIMENSION(8,kfld) :: ishtSi + ! Assignment with CodeBlock on RHS. + iwewe(:) = (/ jpwe,jpea,jpwe,jpea /) + ! Assignment with CodeBlock in array-index expression. + iwewe(:) = ishtSi((/ jpwe,jpea,jpwe,jpea /), 1) + ! Index expression does evaluate to an array but we don't currently + ! handle getting a type in this case (TODO #1799). + ishtSi(5:8,jf) = ishtSi(iwewe+1, jf) + ! Index expression contains a call to an unknown function. + ishtSi(5:8,jf) = ishtSi(my_func(1), jf) + end program test + ''') + assignments = psyir.walk(Assignment) + assert isinstance(assignments[0].rhs, CodeBlock) + trans = ArrayAssignment2LoopsTrans() + with pytest.raises(TransformationError) as err: + trans.validate(assignments[0]) + assert ("ArrayAssignment2LoopsTrans does not support array assignments " + "that contain a CodeBlock anywhere in the expression" + in str(err.value)) + with pytest.raises(TransformationError) as err: + trans.validate(assignments[1]) + assert ("ArrayAssignment2LoopsTrans does not support array assignments " + "that contain a CodeBlock anywhere in the expression" + in str(err.value)) + # TODO #1799 - this requires that we extended ArrayMixin.datatype to + # support array accesses with index expressions that are Operations. + with pytest.raises(TransformationError) as err: + trans.validate(assignments[2]) + assert ("cannot expand expression because it contains the access " + "'ishtsi(iwewe + 1,jf)' which is an UnresolvedType and therefore " + "cannot be guaranteed" in str(err.value)) + with pytest.raises(TransformationError) as err: + trans.validate(assignments[3]) + assert ("cannot expand expression because it contains the access " + "'ishtsi(my_func(1),jf)' and whether or not the index expression " + "'my_func(1)' is itself an array is unknown." in str(err.value)) From 32308edca9b0db39b9c0a41a066b530d300b38e9 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Fri, 30 Aug 2024 17:28:37 +0100 Subject: [PATCH 06/15] #2685 tighten-up check on type of index expressions [skip ci] --- src/psyclone/psyir/nodes/array_mixin.py | 11 +++++- .../tests/psyir/nodes/array_mixin_test.py | 38 +++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/psyclone/psyir/nodes/array_mixin.py b/src/psyclone/psyir/nodes/array_mixin.py index ab2b35eff4..7acb40380f 100644 --- a/src/psyclone/psyir/nodes/array_mixin.py +++ b/src/psyclone/psyir/nodes/array_mixin.py @@ -610,7 +610,8 @@ def _get_effective_shape(self): :rtype: list[:py:class:`psyclone.psyir.nodes.DataNode`] :raises NotImplementedError: if any of the array-indices involve a - function call or an expression. + function call or an expression or are + of unknown type. ''' shape = [] for idx, idx_expr in enumerate(self.indices): @@ -639,7 +640,13 @@ def _get_effective_shape(self): sizeop = IntrinsicCall.create( IntrinsicCall.Intrinsic.SIZE, [idx_expr.copy()]) shape.append(sizeop) - + elif isinstance(dtype, (UnsupportedType, UnresolvedType)): + raise NotImplementedError( + f"The array index expression " + f"'{idx_expr.debug_string()}' in access " + f"'{self.debug_string()}' is of '{dtype}' type and " + f"therefore whether it is an array slice (i.e. an " + f"indirect access) cannot be determined.") elif isinstance(idx_expr, (Call, Operation, CodeBlock)): # We can't yet straightforwardly query the type of a function # call or Operation - TODO #1799. diff --git a/src/psyclone/tests/psyir/nodes/array_mixin_test.py b/src/psyclone/tests/psyir/nodes/array_mixin_test.py index 0ad384422e..7c4630fed1 100644 --- a/src/psyclone/tests/psyir/nodes/array_mixin_test.py +++ b/src/psyclone/tests/psyir/nodes/array_mixin_test.py @@ -623,6 +623,7 @@ def test_get_effective_shape(fortran_reader): " a(f()) = 2.0\n" " a(2+3) = 1.0\n" " b(idx, a) = -1.0\n" + " b(scalarval, arrayval) = 1\n" "end subroutine\n") psyir = fortran_reader.psyir_from_source(code) routine = psyir.walk(Routine)[0] @@ -699,6 +700,43 @@ def test_get_effective_shape(fortran_reader): shape = routine.children[child_idx].lhs._get_effective_shape() assert len(shape) == 1 assert "SIZE(a)" in shape[0].debug_string() + # Array-index expressions are symbols of unknown type so we don't know + # whether we have an array slice or just a scalar. + child_idx += 1 + with pytest.raises(NotImplementedError) as err: + _ = routine.children[child_idx].lhs._get_effective_shape() + assert ("index expression 'scalarval' in access 'b(scalarval,arrayval)' is" + " of 'UnresolvedType' type and therefore whether it is an array " + "slice (i.e. an indirect access) cannot be determined." + in str(err.value)) + + +def test_struct_get_effective_shape(fortran_reader): + '''Tests for the _get_effective_shape() method for ArrayMember and + ArrayOfStructuresMixin (since they inherit it from ArrayMixin).''' + code = ( + "subroutine test()\n" + " type :: my_type\n" + " real, dimension(21,12) :: data\n" + " end type my_type\n" + " type(my_type) :: grid\n" + " type(my_type), dimension(5) :: grid_list\n" + " grid%data(:,:) = 0.0\n" + " grid_list(:)%data(1) = 0.0\n" + "end subroutine\n") + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Slice of ArrayMember + child_idx = 0 + shape = routine.children[child_idx].lhs.member._get_effective_shape() + assert len(shape) == 2 + assert "SIZE(grid%data, dim=1)" in shape[0].debug_string() + assert "SIZE(grid%data, dim=2)" in shape[1].debug_string() + # Slice of ArrayOfStructuresMixin + child_idx += 1 + shape = routine.children[child_idx].lhs._get_effective_shape() + assert len(shape) == 1 + assert isinstance(shape[0], IntrinsicCall) # get_outer_range_index From d8f3800abc36faaea98c7358e67f9414f702f228 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Tue, 1 Oct 2024 11:11:21 +0100 Subject: [PATCH 07/15] #2027 fix failing tests due to change in err msg --- .../frontend/fparser2_where_handler_test.py | 22 +++++++++++++------ .../arrayassignment2loops_trans_test.py | 14 ++++++------ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py index 3b51510dcb..9cc2742465 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py @@ -692,16 +692,24 @@ def test_where_stmt_validity(): @pytest.mark.usefixtures("parser") -def test_where_stmt(): +def test_where_stmt(fortran_reader): ''' Basic check that we handle a WHERE statement correctly. ''' - fake_parent, _ = process_where( - "WHERE( at_i(:,:) > rn_amax_2d(:,:) ) " - "a_i(:,:,jl) = a_i(:,:,jl) * rn_amax_2d(:,:) / at_i(:,:)", - Fortran2003.Where_Stmt, ["at_i", "rn_amax_2d", "jl", "a_i"]) - assert len(fake_parent.children) == 1 - assert isinstance(fake_parent[0], Loop) + code = '''\ + program where_test + implicit none + integer :: jl + real, dimension(:,:), allocatable :: at_i, rn_amax_2d + real, dimension(:,:,:), allocatable :: a_i + WHERE( at_i(:,:) > rn_amax_2d(:,:) ) & + a_i(:,:,jl) = a_i(:,:,jl) * rn_amax_2d(:,:) / at_i(:,:) + end program where_test + ''' + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + assert len(routine.children) == 1 + assert isinstance(routine[0], Loop) def test_where_stmt_no_reduction(fortran_reader, fortran_writer): diff --git a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py index 566ede719a..4d592ac23e 100644 --- a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py @@ -656,9 +656,9 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): with pytest.raises(TransformationError) as info: trans.apply(psyir.walk(Assignment)[4], opts) assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the access 'ishtsi(map,scalar)' and whether or not the " - "index expression 'map' is itself an array is unknown" in - str(info.value)) + "contains the access 'ishtsi(map,scalar)' which is an " + "UnresolvedType and therefore cannot be guaranteed to be " + "ScalarType" in str(info.value)) # The end result should look like: assert ( @@ -679,8 +679,8 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): "cannot be guaranteed to be ScalarType.\n" " x(:) = unsupported\n" " ! ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the access 'ishtsi(map,scalar)' and whether or not the " - "index expression 'map' is itself an array is unknown.\n" + "contains the access 'ishtsi(map,scalar)' which is an UnresolvedType " + "and therefore cannot be guaranteed to be ScalarType.\n" " x(:) = ishtsi(map,scalar)\n" ) in fortran_writer(psyir) @@ -796,5 +796,5 @@ def test_validate_indirect_indexing(fortran_reader): with pytest.raises(TransformationError) as err: trans.validate(assignments[3]) assert ("cannot expand expression because it contains the access " - "'ishtsi(my_func(1),jf)' and whether or not the index expression " - "'my_func(1)' is itself an array is unknown." in str(err.value)) + "'ishtsi(my_func(1),jf)' which is an UnresolvedType and therefore " + "cannot be guaranteed to be ScalarType." in str(err.value)) From 1d356842d08494ab8419bda4ebd711e55ec02716 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Tue, 1 Oct 2024 11:15:47 +0100 Subject: [PATCH 08/15] #2027 put back processing of lbclnk in omp_cpu_trans.py --- examples/nemo/scripts/omp_cpu_trans.py | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/nemo/scripts/omp_cpu_trans.py b/examples/nemo/scripts/omp_cpu_trans.py index 904b44d110..35449494b6 100755 --- a/examples/nemo/scripts/omp_cpu_trans.py +++ b/examples/nemo/scripts/omp_cpu_trans.py @@ -47,7 +47,6 @@ # List of all files that psyclone will skip processing FILES_TO_SKIP = NOT_PERFORMANT + NOT_WORKING + [ - "lbclnk.f90", # TODO #2685: effective shape bug "asminc.f90", "trosk.f90", "vremap.f90", From 4cb2a6fcdc5e66298c3879ac9a96c0a6e4ef0b3d Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 2 Oct 2024 10:28:55 +0100 Subject: [PATCH 09/15] #2027 bug fixes for array refs within struct accesses [skip ci] --- src/psyclone/psyir/frontend/fparser2.py | 9 ++- .../psyir/nodes/structure_reference.py | 58 +++++++++++-------- .../arrayassignment2loops_trans.py | 33 ----------- .../frontend/fparser2_where_handler_test.py | 12 ++-- .../arrayassignment2loops_trans_test.py | 37 ++++++++++++ .../transformations/inline_trans_test.py | 33 ++++++----- 6 files changed, 106 insertions(+), 76 deletions(-) diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 0fdcb32a63..d9de790548 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -1352,6 +1352,7 @@ def _parse_dimensions(self, dimensions, symbol_table): shape = [] # Traverse shape specs in Depth-first-search order for dim in walk(dimensions, (Fortran2003.Assumed_Shape_Spec, + Fortran2003.Deferred_Shape_Spec, Fortran2003.Explicit_Shape_Spec, Fortran2003.Assumed_Size_Spec)): @@ -1382,6 +1383,11 @@ def _parse_dimensions(self, dimensions, symbol_table): else: shape.append(None) + elif isinstance(dim, Fortran2003.Deferred_Shape_Spec): + # Deferred_Shape_Spec has no children (R520). For our purposes + # it is equivalent to Assumed_Shape_Spec(None, None). + shape.append(None) + elif isinstance(dim, Fortran2003.Explicit_Shape_Spec): upper = self._process_array_bound(dim.items[1], symbol_table) @@ -1868,7 +1874,8 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, has_save_attr = False if attr_specs: for attr in attr_specs.items: - if isinstance(attr, Fortran2003.Attr_Spec): + if isinstance(attr, (Fortran2003.Attr_Spec, + Fortran2003.Component_Attr_Spec)): normalized_string = str(attr).lower().replace(' ', '') if normalized_string == "save": if interface is not None: diff --git a/src/psyclone/psyir/nodes/structure_reference.py b/src/psyclone/psyir/nodes/structure_reference.py index fd7c4e8842..99e490f548 100644 --- a/src/psyclone/psyir/nodes/structure_reference.py +++ b/src/psyclone/psyir/nodes/structure_reference.py @@ -269,10 +269,26 @@ def datatype(self): :returns: the datatype of this reference. :rtype: :py:class:`psyclone.psyir.symbols.DataType` - :raises NotImplementedError: if the structure reference represents \ + :raises NotImplementedError: if the structure reference represents an array of arrays. ''' + def _cursor_shape(cursor, cursor_type): + ''' + ''' + if not (isinstance(cursor, ArrayMixin) or + isinstance(cursor_type, ArrayType)): + return None + if isinstance(cursor, ArrayMixin): + # It is an ArrayMixin and has indices so could be a single + # element or a slice. + # pylint: disable=protected-access + cursor_shape = cursor._get_effective_shape() + else: + # No indices so it is an access to a whole array. + cursor_shape = cursor_type.shape + return cursor_shape + # pylint: disable=too-many-return-statements, too-many-branches if self._overwrite_datatype: return self._overwrite_datatype @@ -307,6 +323,8 @@ def datatype(self): # Walk down the structure, collecting information on any array slices # as we go. + #if cursor.name == "grid": + # import pdb; pdb.set_trace() while isinstance(cursor, (StructureMember, StructureReference)): cursor = cursor.member if isinstance(cursor_type, ArrayType): @@ -317,36 +335,28 @@ def datatype(self): # Once we've hit an Unresolved/UnsupportedType the cursor_type # will remain set to that as we can't do any better. cursor_type = cursor_type.components[cursor.name].datatype - if isinstance(cursor, ArrayMixin): + if isinstance(cursor, ArrayMixin) or isinstance(cursor_type, + ArrayType): # pylint: disable=protected-access try: - shape.extend(cursor._get_effective_shape()) + cursor_shape = _cursor_shape(cursor, cursor_type) + if cursor_shape: + shape.extend(cursor_shape) except NotImplementedError: return UnresolvedType() # We've reached the ultimate member of the structure access. if shape: - if isinstance(cursor_type, ArrayType): - # It's of array type but does it represent a single element, - # a slice or a whole array? (We use `children` rather than - # `indices` so as to avoid having to check that `cursor` is - # an `ArrayMember`.) - if cursor.children: - # It has indices so could be a single element or a slice. - # pylint: disable=protected-access - cursor_shape = cursor._get_effective_shape() - else: - # No indices so it is an access to a whole array. - cursor_shape = cursor_type.shape - if cursor_shape and len(shape) != len(cursor_shape): - # This ultimate access is an array but we've already - # encountered one or more slices earlier in the access - # expression. - raise NotImplementedError( - f"Array of arrays not supported: the ultimate member " - f"'{cursor.name}' of the StructureAccess represents " - f"an array but other array notation is present in the " - f"full access expression: '{self.debug_string()}'") + cursor_shape = _cursor_shape(cursor, cursor_type) + if cursor_shape and len(shape) != len(cursor_shape): + # This ultimate access is an array but we've already + # encountered one or more slices earlier in the access + # expression. + raise NotImplementedError( + f"Array of arrays not supported: the ultimate member " + f"'{cursor.name}' of the StructureAccess represents " + f"an array but other array notation is present in the " + f"full access expression: '{self.debug_string()}'") return ArrayType(cursor_type, shape) diff --git a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py index 4998eda19c..09590938ef 100644 --- a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py +++ b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py @@ -367,39 +367,6 @@ def validate(self, node, options=None): # pylint: disable=cell-var-from-loop raise TransformationError(LazyString( lambda: f"{message} In:\n{node.debug_string()}")) - # We must understand any array accesses on the RHS. This means all - # array indices must be known (because otherwise they themselves - # might be arrays, e.g. - # a(:) = b(c) where `c` can be a scalar or an array - # or a(:) = grid%b(c) - if isinstance(reference, ArrayMixin): - # Check the index expressions - range_count = len([x for x in accessor.indices - if isinstance(x, Range)]) - if range_count != num_of_ranges: - # If the number of ranges in this access is not the same as - # on the LHS then we may or may not have a scalar. To - # determine this we must know the type of every one of the - # index expressions. - for exprn in reference.indices: - if not isinstance(exprn, Reference): - # We don't have a Reference so it cannot be of - # array type (we rejected CodeBlocks earlier). - continue - if not isinstance(exprn.datatype, (UnsupportedType, - UnresolvedType)): - continue - message = ( - f"{self.name} cannot expand expression because it " - f"contains the access '{reference.debug_string()}'" - f" and whether or not the index " - f"expression '{exprn.debug_string()}' is " - f"itself an array is unknown.") - if verbose: - node.append_preceding_comment(message) - # pylint: disable=cell-var-from-loop - raise TransformationError(LazyString( - lambda: f"{message} In:\n{node.debug_string()}")) __all__ = [ diff --git a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py index 9cc2742465..b87f0666a2 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py @@ -773,11 +773,11 @@ def test_where_ordering(parser): ("where (my_type%var(:) > epsi20)\n" "my_type2(jl)%array2(:,jl) = 3.0\n", "my_type%var"), ("where (my_type%block(jl)%var(:) > epsi20)\n" - "my_type%block%array(:,jl) = 3.0\n", "my_type%block(jl)%var"), + "my_type%block(jl)%array(:,jl) = 3.0\n", "my_type%block(jl)%var"), ("where (my_type%block(jl)%var(:) > epsi20)\n" "my_type%block(jl)%array(:,jl) = 3.0\n", "my_type%block(jl)%var"), - ("where (my_type2(:)%var > epsi20)\n" - "my_type2(:)%var = 3.0\n", "my_type2")]) + ("where (my_type2(:)%var(jl) > epsi20)\n" + "my_type2(:)%var(jl) = 3.0\n", "my_type2")]) def test_where_derived_type(fortran_reader, code, size_arg): ''' Test that we handle the case where array members of a derived type are accessed within a WHERE. ''' @@ -813,10 +813,12 @@ def test_where_derived_type(fortran_reader, code, size_arg): assert isinstance(loops[1].loop_body[0], IfBlock) # All Range nodes should have been replaced assert not loops[0].walk(Range) - # All ArrayMember accesses should now use the `widx1` loop variable + # All ArrayMember accesses other than 'var' should now use the `widx1` + # loop variable array_members = loops[0].walk(ArrayMember) for member in array_members: - assert "+ widx1 - 1" in member.indices[0].debug_string() + if member.name != "var": + assert "+ widx1 - 1" in member.indices[0].debug_string() @pytest.mark.parametrize( diff --git a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py index 4d592ac23e..95cb0be342 100644 --- a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py @@ -798,3 +798,40 @@ def test_validate_indirect_indexing(fortran_reader): assert ("cannot expand expression because it contains the access " "'ishtsi(my_func(1),jf)' which is an UnresolvedType and therefore " "cannot be guaranteed to be ScalarType." in str(err.value)) + + +def test_validate_structure(fortran_reader): + ''' + Check the validation of the transformation when it is a structure access + on the RHS. + + ''' + psyir = fortran_reader.psyir_from_source(''' + program test + integer, parameter :: ngrids = 4, kfld=5 + integer, dimension(8,kfld) :: ishtSi + type :: sub_grid_type + integer, dimension(kfld,kfld) :: map + end type sub_grid_type + type :: grid_type + real, dimension(:,:), allocatable :: data + type(sub_grid_type), dimension(ngrids) :: subgrid + end type grid_type + type(grid_type) :: grid + integer :: jf + ! Cannot tell whether or not the access on the RHS is an array. + ishtSi(5:8,jf) = grid%data(my_func(1), jf) + ! The array access to subgrid is not yet supported. + ishtSi(5:8,jf) = grid%subgrid%map(1,1) + end program test + ''') + assignments = psyir.walk(Assignment) + trans = ArrayAssignment2LoopsTrans() + with pytest.raises(TransformationError) as err: + trans.validate(assignments[0]) + assert ("contains the access 'grid%data(my_func(1),jf)' which is an " + "UnresolvedType" in str(err.value)) + # TODO #1858 - once we've extended Reference2ArrayRangeTrans to support + # StructureMembers we can use it as part of this transformation and this + # example will be supported. + trans.validate(assignments[1]) diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index 740ee5e270..295ef72223 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -834,10 +834,10 @@ def test_apply_struct_array_slice_arg(fortran_reader, fortran_writer, tmpdir): f" grid%data2d(:,:) = 1.0\n" f" do i=1,10\n" f" a(i) = 1.0\n" - f" call sub(grid%local%data)\n" f" call sub(micah%grids(3)%region%data(:))\n" f" call sub(grid%data2d(:,i))\n" f" call sub(grid%data2d(1:5,i))\n" + f" call sub(grid%local%data)\n" f" end do\n" f" end subroutine run_it\n" f" subroutine sub(x)\n" @@ -855,35 +855,42 @@ def test_apply_struct_array_slice_arg(fortran_reader, fortran_writer, tmpdir): inline_trans = InlineTrans() for call in psyir.walk(Call): if not isinstance(call, IntrinsicCall): + if call.arguments[0].debug_string() == "grid%local%data": + # TODO #1858: this if construct can be removed once we + # support getting the type of `grid%local%data`. + continue inline_trans.apply(call) output = fortran_writer(psyir) assert (" do i = 1, 10, 1\n" " a(i) = 1.0\n" " do ji = 1, 5, 1\n" - " grid%local%data(ji) = 2.0 * grid%local%data(ji)\n" - " enddo\n" - " grid%local%data(1:2) = 0.0\n" - " grid%local%data(:) = 3.0\n" - " grid%local%data = 5.0\n" - " do ji_1 = 1, 5, 1\n" - " micah%grids(3)%region%data(ji_1) = 2.0 * " - "micah%grids(3)%region%data(ji_1)\n" + " micah%grids(3)%region%data(ji) = 2.0 * " + "micah%grids(3)%region%data(ji)\n" " enddo\n" " micah%grids(3)%region%data(1:2) = 0.0\n" " micah%grids(3)%region%data(:) = 3.0\n" " micah%grids(3)%region%data(:) = 5.0\n" - " do ji_2 = 1, 5, 1\n" - " grid%data2d(ji_2,i) = 2.0 * grid%data2d(ji_2,i)\n" + " do ji_1 = 1, 5, 1\n" + " grid%data2d(ji_1,i) = 2.0 * grid%data2d(ji_1,i)\n" " enddo\n" " grid%data2d(1:2,i) = 0.0\n" " grid%data2d(:,i) = 3.0\n" " grid%data2d(:,i) = 5.0\n" - " do ji_3 = 1, 5, 1\n" - " grid%data2d(ji_3,i) = 2.0 * grid%data2d(ji_3,i)\n" + " do ji_2 = 1, 5, 1\n" + " grid%data2d(ji_2,i) = 2.0 * grid%data2d(ji_2,i)\n" " enddo\n" " grid%data2d(1:2,i) = 0.0\n" " grid%data2d(1:5,i) = 3.0\n" " grid%data2d(1:5,i) = 5.0\n" + # TODO #1858: replace the following line with the commented-out + # lines below. + " call sub(grid%local%data)\n" + # " do ji_3 = 1, 5, 1\n" + # " grid%local%data(ji_3) = 2.0 * grid%local%data(ji_3)\n" + # " enddo\n" + # " grid%local%data(1:2) = 0.0\n" + # " grid%local%data(:) = 3.0\n" + # " grid%local%data = 5.0\n" " enddo\n" in output) assert Compile(tmpdir).string_compiles(output) From 60d6f7750488b90d77486da8915df1af1b8e9808 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 2 Oct 2024 13:18:24 +0100 Subject: [PATCH 10/15] #2027 further tidying of StructureReference.datatype method --- .../psyir/nodes/structure_reference.py | 69 +++++++++---------- 1 file changed, 31 insertions(+), 38 deletions(-) diff --git a/src/psyclone/psyir/nodes/structure_reference.py b/src/psyclone/psyir/nodes/structure_reference.py index 99e490f548..625abbd08d 100644 --- a/src/psyclone/psyir/nodes/structure_reference.py +++ b/src/psyclone/psyir/nodes/structure_reference.py @@ -273,8 +273,18 @@ def datatype(self): an array of arrays. ''' - def _cursor_shape(cursor, cursor_type): + def _get_cursor_shape(cursor, cursor_type): ''' + Utility that returns the shape of the supplied node. + + :param cursor: the node to get the shape of. + :type cursor: :py:class:`psyclone.psyir.nodes.Node` + :param cursor_type: the type of the node. + :type cursor_type: :py:class:`psyclone.psyir.symbols.DataType` + + :returns: the shape of the node or None if it is not an array. + :rtype: Optional[list[:py:class:`psyclone.psyir.nodes.Node`]] + ''' if not (isinstance(cursor, ArrayMixin) or isinstance(cursor_type, ArrayType)): @@ -283,11 +293,9 @@ def _cursor_shape(cursor, cursor_type): # It is an ArrayMixin and has indices so could be a single # element or a slice. # pylint: disable=protected-access - cursor_shape = cursor._get_effective_shape() - else: - # No indices so it is an access to a whole array. - cursor_shape = cursor_type.shape - return cursor_shape + return cursor._get_effective_shape() + # No indices so it is an access to a whole array. + return cursor_type.shape # pylint: disable=too-many-return-statements, too-many-branches if self._overwrite_datatype: @@ -312,19 +320,13 @@ def _cursor_shape(cursor, cursor_type): # If the reference has explicit array syntax, we need to consider it # to calculate the resulting shape. - if isinstance(cursor, ArrayMixin): - try: - # pylint: disable=protected-access - shape = cursor._get_effective_shape() - except NotImplementedError: - return UnresolvedType() - else: - shape = [] + try: + shape = _get_cursor_shape(cursor, cursor_type) + except NotImplementedError: + return UnresolvedType() # Walk down the structure, collecting information on any array slices # as we go. - #if cursor.name == "grid": - # import pdb; pdb.set_trace() while isinstance(cursor, (StructureMember, StructureReference)): cursor = cursor.member if isinstance(cursor_type, ArrayType): @@ -335,29 +337,20 @@ def _cursor_shape(cursor, cursor_type): # Once we've hit an Unresolved/UnsupportedType the cursor_type # will remain set to that as we can't do any better. cursor_type = cursor_type.components[cursor.name].datatype - if isinstance(cursor, ArrayMixin) or isinstance(cursor_type, - ArrayType): - # pylint: disable=protected-access - try: - cursor_shape = _cursor_shape(cursor, cursor_type) - if cursor_shape: - shape.extend(cursor_shape) - except NotImplementedError: - return UnresolvedType() - - # We've reached the ultimate member of the structure access. - if shape: - cursor_shape = _cursor_shape(cursor, cursor_type) - if cursor_shape and len(shape) != len(cursor_shape): - # This ultimate access is an array but we've already - # encountered one or more slices earlier in the access - # expression. - raise NotImplementedError( - f"Array of arrays not supported: the ultimate member " - f"'{cursor.name}' of the StructureAccess represents " - f"an array but other array notation is present in the " - f"full access expression: '{self.debug_string()}'") + try: + cursor_shape = _get_cursor_shape(cursor, cursor_type) + except NotImplementedError: + return UnresolvedType() + if cursor_shape: + if shape: + raise NotImplementedError( + f"Array of arrays not supported: the ultimate member " + f"'{cursor.name}' of the StructureAccess represents " + f"an array but other array notation is present in the " + f"full access expression: '{self.debug_string()}'") + shape = cursor_shape + if shape: return ArrayType(cursor_type, shape) # We don't have an explicit array access (because `shape` is Falsey) From beab4b6f8e73e6a085bccdb0a08b3ae68a12df58 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 2 Oct 2024 13:19:57 +0100 Subject: [PATCH 11/15] #2027 rm unused import --- .../psyir/transformations/arrayassignment2loops_trans.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py index 09590938ef..59a65b99ff 100644 --- a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py +++ b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py @@ -50,7 +50,7 @@ from psyclone.psyir.nodes.array_mixin import ArrayMixin from psyclone.psyir.symbols import ( ArrayType, DataSymbol, INTEGER_TYPE, ScalarType, SymbolError, - UnresolvedType, UnsupportedType) + UnresolvedType) from psyclone.psyir.transformations.transformation_error import ( TransformationError) from psyclone.psyir.transformations.reference2arrayrange_trans import ( From 8622d0e46aef3a38a4a878f5f8c827017bdaff4c Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Tue, 29 Oct 2024 13:23:40 +0000 Subject: [PATCH 12/15] #2027 fix indirect coverage changes --- src/psyclone/psyir/nodes/structure_reference.py | 7 +------ src/psyclone/psyir/transformations/inline_trans.py | 5 ++++- src/psyclone/tests/psyir/frontend/fparser2_test.py | 12 ++++++++++++ .../tests/psyir/nodes/array_reference_test.py | 7 +++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/psyclone/psyir/nodes/structure_reference.py b/src/psyclone/psyir/nodes/structure_reference.py index 625abbd08d..f4be0a6f65 100644 --- a/src/psyclone/psyir/nodes/structure_reference.py +++ b/src/psyclone/psyir/nodes/structure_reference.py @@ -353,13 +353,8 @@ def _get_cursor_shape(cursor, cursor_type): if shape: return ArrayType(cursor_type, shape) - # We don't have an explicit array access (because `shape` is Falsey) - # but is the ultimate member itself an array? + # We must have a scalar. if isinstance(cursor_type, ArrayType): - if not cursor.children: - # It is and there are no index expressions so we return the - # ArrayType. - return cursor_type # We have an access to a single element of the array. # Currently arrays of scalars are handled in a # different way to all other types of array. Issue #1857 will diff --git a/src/psyclone/psyir/transformations/inline_trans.py b/src/psyclone/psyir/transformations/inline_trans.py index 47da70ff55..d44df08d74 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -530,7 +530,10 @@ def _replace_formal_struc_arg(self, actual_arg, ref, call_node, break cursor = cursor.member - if not actual_arg.walk(Range) and local_indices: + # TODO #1858 - once we support converting structure accesses into + # explicit array accesses, we can put back the testing in + # inline_trans_test.py that covers this code and remove the pragma: + if not actual_arg.walk(Range) and local_indices: # pragma: no cover # There are no Ranges in the actual argument but the local # reference is an array access. # Create updated index expressions for that access. diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py index 3135b37a58..1e6a8c8553 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py @@ -850,6 +850,18 @@ def test_unsupported_decln(fortran_reader): assert ("Invalid variable declaration found in _process_decln for " "'problem'" in str(err.value)) + reader = FortranStringReader("integer, intent(in) :: l1") + fparser2spec = Specification_Part(reader).content[0] + # Break the attribute-spec list for this declaration. + attr_specs = fparser2spec.items[1] + attr_specs.items = (attr_specs.items[0], "rubbish") + fake_parent = KernelSchedule.create("dummy_schedule") + symtab = fake_parent.symbol_table + processor = Fparser2Reader() + with pytest.raises(NotImplementedError) as error: + processor._process_decln(fake_parent, symtab, fparser2spec) + assert "Unrecognised attribute type 'str'" in str(error.value) + def test_unsupported_decln_structure_type(fortran_reader): ''' diff --git a/src/psyclone/tests/psyir/nodes/array_reference_test.py b/src/psyclone/tests/psyir/nodes/array_reference_test.py index 8073840657..027e812ac8 100644 --- a/src/psyclone/tests/psyir/nodes/array_reference_test.py +++ b/src/psyclone/tests/psyir/nodes/array_reference_test.py @@ -580,6 +580,13 @@ def test_array_datatype(): aref6._symbol = test_struc_sym assert isinstance(aref6.datatype, UnresolvedType) + # TODO #2448 - we don't handle an array access to something that we + # don't know is an array. + aref7 = ArrayReference(generic_sym) + aref7.addchild(one.copy()) + aref7._symbol = DataSymbol("int_test", INTEGER_TYPE) + assert isinstance(aref7.datatype, UnresolvedType) + def test_array_create_colon(fortran_writer): '''Test that the create method accepts ":" as shortcut to automatically From 3b6911bef4119b2030c900e029414e8a9304b567 Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Tue, 29 Oct 2024 21:25:45 +0000 Subject: [PATCH 13/15] #2027 fix coverage of structure_reference.py --- .../psyir/nodes/structure_reference_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/psyclone/tests/psyir/nodes/structure_reference_test.py b/src/psyclone/tests/psyir/nodes/structure_reference_test.py index a8cf02a4ab..55881ccdd7 100644 --- a/src/psyclone/tests/psyir/nodes/structure_reference_test.py +++ b/src/psyclone/tests/psyir/nodes/structure_reference_test.py @@ -241,7 +241,9 @@ def test_struc_ref_datatype(): grid_type = symbols.StructureType.create([ ("nx", symbols.INTEGER_TYPE, symbols.Symbol.Visibility.PUBLIC, None), ("data", atype, symbols.Symbol.Visibility.PRIVATE, None), + # A single member of structure type. ("roger", rtype, symbols.Symbol.Visibility.PUBLIC, None), + # An array of structure type. ("titty", artype, symbols.Symbol.Visibility.PUBLIC, None)]) # Symbol with type defined by StructureType ssym0 = symbols.DataSymbol("grid", grid_type) @@ -273,6 +275,20 @@ def test_struc_ref_datatype(): ssym, [("titty", [one.copy(), two.copy()])]) assert singleref.datatype == rtype_sym + # Reference to grid%titty(1,2)%gibber + sref = nodes.StructureReference.create( + ssym, [("titty", [one.copy(), two.copy()]), "gibber"]) + assert sref.datatype == symbols.BOOLEAN_TYPE + + # Reference to grid%titty(my_func(1), 2) where my_func is unresolved. + func_sym = symbols.DataSymbol("my_func", + datatype=symbols.UnresolvedType(), + interface=symbols.UnresolvedInterface()) + fref = nodes.ArrayReference.create(func_sym, [one.copy()]) + sref2 = nodes.StructureReference.create( + ssym, [("titty", [fref, two.copy()])]) + assert isinstance(sref2.datatype, symbols.UnresolvedType) + # Reference to sub-array of structure members of structure myrange = nodes.Range.create(two.copy(), nodes.Literal("4", symbols.INTEGER_TYPE)) From b26e6b46784a2f291772d6cb261c99a43dc6b11a Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Tue, 29 Oct 2024 21:43:01 +0000 Subject: [PATCH 14/15] #2027 simplify search for References to apply Reference2ArrayRangeTrans to --- examples/nemo/scripts/utils.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index 202c7ca59a..39c98d9128 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -248,7 +248,7 @@ def normalise_loops( if convert_array_notation: # Make sure all array dimensions are explicit - for reference in schedule.walk(Reference, stop_type=Reference): + for reference in schedule.walk(Reference): part_of_the_call = reference.ancestor(Call) if part_of_the_call: if not part_of_the_call.is_elemental: @@ -258,15 +258,6 @@ def normalise_loops( Reference2ArrayRangeTrans().apply(reference) except TransformationError: pass - if hasattr(reference, "indices"): - # Look at array-index expressions too. - for exprn in reference.indices: - if (isinstance(exprn, Reference) and - isinstance(exprn.symbol, DataSymbol)): - try: - Reference2ArrayRangeTrans().apply(exprn) - except TransformationError: - pass if loopify_array_intrinsics: for intr in schedule.walk(IntrinsicCall): From 9a3cf25a2837fbc2c6c3b53b42937ee3b2547e34 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Fri, 1 Nov 2024 09:16:26 +0000 Subject: [PATCH 15/15] #2027: Update changelog --- changelog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/changelog b/changelog index 895ca4ebf6..335e026d04 100644 --- a/changelog +++ b/changelog @@ -256,6 +256,8 @@ 89) PR #2712 for #2711. Adds support for user-supplied Kernels that operate on dofs (in the LFRic API). + + 99) PR #2685 for #2027. Improves datatype and shape inference. release 2.5.0 14th of February 2024