From 01f0f53d64e6aa8cfa8f930d3f014a30d5f69097 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Tue, 3 Mar 2026 09:57:11 +0000 Subject: [PATCH 01/10] #3022 Remove array_reduction_base _get_args method and add a test --- .../intrinsics/array_reduction_base_trans.py | 39 +++++--------- .../array_reduction_base_trans_test.py | 52 +++++++++++-------- 2 files changed, 43 insertions(+), 48 deletions(-) diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index 4ad327d6de..14e05f5e88 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -64,31 +64,6 @@ class ArrayReductionBaseTrans(Transformation, ABC): _INTRINSIC_NAME = None _INTRINSIC_TYPE = None - @staticmethod - def _get_args(node): - '''Utility method that returns the array-reduction intrinsic arguments - (array reference, dimension and mask). - - :param node: an array-reduction intrinsic. - :type node: :py:class:`psyclone.psyir.nodes.IntrinsicCall` - - returns: a tuple containing the 3 arguments. - rtype: Tuple[py:class:`psyclone.psyir.nodes.reference.Reference`, - py:class:`psyclone.psyir.nodes.Literal` | - :py:class:`psyclone.psyir.nodes.Reference`, - Optional[:py:class:`psyclone.psyir.nodes.Node`]] - - ''' - # Determine the arguments to the intrinsic - args = [None, None, None] - arg_names_map = {"array": 0, "dim": 1, "mask": 2} - # Add argument names to the intrinsic. - node.compute_argument_names() - for idx, child in enumerate(node.arguments): - name = node.argument_names[idx].lower() - args[arg_names_map[name]] = child - return tuple(args) - def __str__(self): return (f"Convert the PSyIR {self._INTRINSIC_NAME} intrinsic " "to equivalent PSyIR code.") @@ -133,7 +108,16 @@ def validate(self, node, options=None, **kwargs): f"argument is not a {self._INTRINSIC_NAME.lower()} " f"intrinsic, found '{node.routine.name}'.") - array_ref, dim_ref, _ = self._get_args(node) + try: + node.compute_argument_names() + except (ValueError, NotImplementedError) as err: + raise TransformationError( + f"Error in {self.name} transformation. Psyclone cannot " + f"disambiguate the arguments names in:\n{node.debug_string()}" + ) from err + + array_ref = node.argument_by_name("array") + dim_ref = node.argument_by_name("dim") # dim_ref is not yet supported by this transformation. if dim_ref: @@ -226,7 +210,8 @@ def apply(self, node, options=None, **kwargs): else: new_lhs = orig_lhs.copy() - expr, _, mask_ref = self._get_args(node) + expr = node.argument_by_name("array") + mask_ref = node.argument_by_name("mask") # Step 1: replace all references to arrays within the # intrinsic expressions and mask argument (if it exists) to diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py index c5f36ab0c8..5cafa1cb3f 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py @@ -40,9 +40,8 @@ ''' import pytest -from psyclone.psyir.nodes import IntrinsicCall, Reference, Literal -from psyclone.psyir.symbols import ( - Symbol, BOOLEAN_TYPE, INTEGER_TYPE, DataSymbol, REAL_TYPE) +from psyclone.psyir.nodes import IntrinsicCall, Reference +from psyclone.psyir.symbols import DataSymbol, REAL_TYPE from psyclone.psyir.transformations import ( TransformationError, Maxval2LoopTrans) from psyclone.psyir.transformations.intrinsics.array_reduction_base_trans \ @@ -65,24 +64,6 @@ def test_init_exception(): assert "_loop_body" in str(info.value) -def test_get_args(): - '''Check the _get_args static method works as expected.''' - # array - array_reference = Reference(Symbol("array")) - node = IntrinsicCall.create(IntrinsicCall.Intrinsic.SUM, [array_reference]) - result = ArrayReductionBaseTrans._get_args(node) - assert result == (array_reference, None, None) - - # array, mask, dim - mask_reference = Literal("true", BOOLEAN_TYPE) - dim_reference = Literal("1", INTEGER_TYPE) - node = IntrinsicCall.create(IntrinsicCall.Intrinsic.SUM, [ - array_reference.copy(), ("mask", mask_reference), - ("dim", dim_reference)]) - result = ArrayReductionBaseTrans._get_args(node) - assert result == (array_reference, dim_reference, mask_reference) - - def test_str(): ''' Check that the __str__ method behaves as expected. ''' assert str(Maxval2LoopTrans()) == ("Convert the PSyIR MAXVAL intrinsic to " @@ -749,3 +730,32 @@ def test_range2loop_fails(fortran_reader, fortran_writer): # Check that the failed transformation does not modify the code code_after = fortran_writer(psyir) assert code_before == code_after + + +def test_replace_reduction_with_type_changes(tmpdir, fortran_reader, + fortran_writer): + ''' Check that the temporary created to aggregate the partial results + is of the correct type even when the the reduction operation is inside and + expression that has implicit or explicit casts. + ''' + code = ( + "subroutine maxval_test(array,n,m)\n" + " integer, parameter :: nk = 4\n" + " integer :: n, m\n" + " real :: array(10,20)\n" + " integer :: result1\n" + " integer(kind=nk) :: result2\n" + " logical :: result3\n" + " result1 = INT(maxval(array))\n" + " result2 = REAL(maxval(array), nk)\n" + " result3 = maxval(array) > 4\n" + "end subroutine\n") + psyir = fortran_reader.psyir_from_source(code) + trans = Maxval2LoopTrans() + for intr_node in psyir.walk(IntrinsicCall): + if intr_node.intrinsic == IntrinsicCall.Intrinsic.MAXVAL: + trans.apply(intr_node) + output = fortran_writer(psyir) + print(output) + assert False + assert Compile(tmpdir).string_compiles(output) From f93e65990255a2082c84d0c51ed62e8c288d9b64 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Tue, 3 Mar 2026 11:28:37 +0000 Subject: [PATCH 02/10] #3022 Always use a reduction temporary variables when converting reduction intrinsics to code --- .../intrinsics/array_reduction_base_trans.py | 69 +++++------- .../array_reduction_base_trans_test.py | 103 ++++++++++-------- .../intrinsics/maxval2loop_trans_test.py | 8 +- .../intrinsics/minval2loop_trans_test.py | 7 +- .../intrinsics/product2loop_trans_test.py | 7 +- .../intrinsics/sum2loop_trans_test.py | 7 +- 6 files changed, 105 insertions(+), 96 deletions(-) diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index 14e05f5e88..43e44ce9e2 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -45,7 +45,8 @@ from psyclone.psyir.nodes import ( Assignment, Reference, ArrayReference, IfBlock, IntrinsicCall, Node, UnaryOperation, BinaryOperation) -from psyclone.psyir.symbols import ArrayType, DataSymbol, ScalarType +from psyclone.psyir.symbols import ( + ArrayType, DataSymbol, ScalarType, UnresolvedType, UnsupportedType) from psyclone.psyGen import Transformation from psyclone.psyir.transformations.reference2arrayrange_trans import \ Reference2ArrayRangeTrans @@ -112,19 +113,23 @@ def validate(self, node, options=None, **kwargs): node.compute_argument_names() except (ValueError, NotImplementedError) as err: raise TransformationError( - f"Error in {self.name} transformation. Psyclone cannot " - f"disambiguate the arguments names in:\n{node.debug_string()}" + f"Error in {self.name} transformation. Cannot " + f"disambiguate the arguments names in '{node.debug_string()}'" ) from err array_ref = node.argument_by_name("array") dim_ref = node.argument_by_name("dim") - # dim_ref is not yet supported by this transformation. if dim_ref: raise TransformationError( f"The dimension argument to {self._INTRINSIC_NAME} is not " f"yet supported.") + if isinstance(node.datatype, (UnresolvedType, UnsupportedType)): + raise TransformationError( + f"Error in {self.name} transformation. Cannot create " + f"a temporary variable for '{node.debug_string()}'") + # There should be at least one arrayreference or reference to # an array in the expression # pylint: disable=unidiomatic-typecheck @@ -190,25 +195,13 @@ def apply(self, node, options=None, **kwargs): self.validate(node, options, **kwargs) - orig_lhs = node.ancestor(Assignment).lhs.copy() orig_rhs = node.ancestor(Assignment).rhs.copy() - # Determine whether the assignment is an increment (as we have - # to use a temporary if so) e.g. x = x + MAXVAL(a) and store a - # reference to the appropriate variable in new_lhs for future - # use. - lhs_symbol = orig_lhs.symbol - increment = False - for rhs_reference in orig_rhs.walk(Reference): - if rhs_reference.symbol is lhs_symbol: - increment = True - if increment: - new_lhs_symbol = node.scope.symbol_table.new_symbol( - root_name="tmp_var", symbol_type=DataSymbol, - datatype=orig_lhs.datatype) - new_lhs = Reference(new_lhs_symbol) - else: - new_lhs = orig_lhs.copy() + # Create a temporary symbol to accomulate the reduction value + tmp_symbol = node.scope.symbol_table.new_symbol( + root_name="reduction_var", symbol_type=DataSymbol, + datatype=node.datatype) + tmp_ref = Reference(tmp_symbol) expr = node.argument_by_name("array") mask_ref = node.argument_by_name("mask") @@ -301,13 +294,13 @@ def apply(self, node, options=None, **kwargs): outer_loop = assignment_parent.children[assignment_position] if mask_ref: # remove mask from the rhs of the assignment - orig_assignment = assignment_rhs.children[0].copy() + new_assignment = assignment_rhs.children[0].copy() indexed_mask_ref = assignment_rhs.children[1].copy() - assignment_rhs.replace_with(orig_assignment) + assignment_rhs.replace_with(new_assignment) # Step 4 convert the original assignment (now within a loop # and indexed) to its intrinsic form by replacing the - # assignment with new_lhs=INTRINSIC(orig_lhs,). Also + # assignment with tmp=INTRINSIC(orig_lhs,). Also # add in the mask if one has been specified. For example: # do idx2 = LBOUND(a,2), UBOUND(a,2) # do idx = LBOUND(a,1), UBOUND(a,1) @@ -323,8 +316,8 @@ def apply(self, node, options=None, **kwargs): # enddo # enddo new_assignment = Assignment.create( - new_lhs.copy(), self._loop_body( - new_lhs.copy(), assignment.rhs.copy())) + tmp_ref.copy(), self._loop_body( + tmp_ref.copy(), assignment.rhs.copy())) if mask_ref: # Place the indexed mask around the statement. new_assignment = IfBlock.create( @@ -355,23 +348,19 @@ def apply(self, node, options=None, **kwargs): # enddo # enddo # x = value1 + x * value2 - lhs = new_lhs.copy() + # Initialisation statement: + lhs = tmp_ref.copy() rhs = self._init_var(lhs) assignment = Assignment.create(lhs, rhs) outer_loop.parent.children.insert(outer_loop.position, assignment) - if not (isinstance(orig_rhs, IntrinsicCall) and - orig_rhs.intrinsic is self._INTRINSIC_TYPE): - # The intrinsic call is not the only thing on the rhs of - # the expression, so we need to deal with the additional - # computation. - rhs = orig_rhs.copy() - for child in rhs.walk(IntrinsicCall): - if child.intrinsic is self._INTRINSIC_TYPE: - child.replace_with(new_lhs.copy()) - break - assignment = Assignment.create(orig_lhs.copy(), rhs) - outer_loop.parent.children.insert( - outer_loop.position+1, assignment) + # Update original assignment with the reduced value + rhs = orig_rhs.copy() + for child in orig_assignment.walk(Node): + if child is node: + child.replace_with(tmp_ref.copy()) + break + outer_loop.parent.children.insert(outer_loop.position+1, + orig_assignment) @abstractmethod def _loop_body(self, lhs, rhs): diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py index 5cafa1cb3f..e2297ea52c 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py @@ -264,10 +264,10 @@ def test_apply(idim1, idim2, rdim11, rdim12, rdim21, rdim22, f" result = maxval(array)\n" f"end subroutine\n") expected = ( - f" result = -HUGE(result)\n" + f" reduction_var = -HUGE(reduction_var)\n" f" do idx = {rdim21}, {rdim22}, 1\n" f" do idx_1 = {rdim11}, {rdim12}, 1\n" - f" result = MAX(result, array(idx_1,idx))\n" + f" reduction_var = MAX(reduction_var, array(idx_1,idx))\n" f" enddo\n" f" enddo\n") psyir = fortran_reader.psyir_from_source(code) @@ -297,13 +297,13 @@ def test_apply_multi(fortran_reader, fortran_writer, tmpdir): " result = value1 + maxval(array) * value2\n" "end subroutine\n") expected = ( - " result = -HUGE(result)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, m, 1\n" " do idx_1 = 1, n, 1\n" - " result = MAX(result, array(idx_1,idx))\n" + " reduction_var = MAX(reduction_var, array(idx_1,idx))\n" " enddo\n" " enddo\n" - " result = value1 + result * value2\n") + " result = value1 + reduction_var * value2\n") psyir = fortran_reader.psyir_from_source(code) # FileContainer/Routine/Assignment/BinaryOperation(ADD)/ # BinaryOperation(MUL)/IntrinsicCall @@ -351,14 +351,15 @@ def test_mask(fortran_reader, fortran_writer, tmpdir): " result = maxval(array, mask=MOD(array, 2.0)==1)\n" "end program\n") expected = ( - " result = -HUGE(result)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 10, 1\n" " do idx_1 = 1, 10, 1\n" " if (MOD(array(idx_1,idx), 2.0) == 1) then\n" - " result = MAX(result, array(idx_1,idx))\n" + " reduction_var = MAX(reduction_var, array(idx_1,idx))\n" " end if\n" " enddo\n" - " enddo\n") + " enddo\n" + " result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) # FileContainer/Routine/Assignment/IntrinsicCall node = psyir.walk(IntrinsicCall)[0] @@ -388,10 +389,10 @@ def test_mask_array_indexed(fortran_reader, fortran_writer, tmpdir): " result = maxval(a, mask=b(1)>a)\n" "end program\n") expected = ( - " result = -HUGE(result)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 4, 1\n" " if (b(1) > a(idx)) then\n" - " result = MAX(result, a(idx))\n" + " reduction_var = MAX(reduction_var, a(idx))\n" " end if\n" " enddo\n") psyir = fortran_reader.psyir_from_source(code) @@ -421,14 +422,15 @@ def test_allocate(fortran_reader, fortran_writer, tmpdir): "end program\n") expected = ( " ALLOCATE(a(1:4,1:4,1:4))\n" - " result = -HUGE(result)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(a, dim=3), UBOUND(a, dim=3), 1\n" " do idx_1 = LBOUND(a, dim=2), UBOUND(a, dim=2), 1\n" " do idx_2 = LBOUND(a, dim=1), UBOUND(a, dim=1), 1\n" - " result = MAX(result, a(idx_2,idx_1,idx))\n" + " reduction_var = MAX(reduction_var, a(idx_2,idx_1,idx))\n" " enddo\n" " enddo\n" " enddo\n" + " result = reduction_var\n" " DEALLOCATE(a)\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() @@ -455,12 +457,12 @@ def test_references(fortran_reader, fortran_writer, tmpdir): "zmax(1) = MAXVAL(ABS(sshn + ssh_ref * tmask), mask=tmask==1.0)\n" "end subroutine\n") expected = ( - " zmax(1) = -HUGE(zmax(1))\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 10, 1\n" " do idx_1 = 1, 10, 1\n" " if (tmask(idx_1,idx) == 1.0) then\n" - " zmax(1) = MAX(zmax(1), ABS(sshn(idx_1,idx) + ssh_ref * " - "tmask(idx_1,idx)))\n" + " reduction_var = MAX(reduction_var, ABS(sshn(idx_1,idx) + " + "ssh_ref * tmask(idx_1,idx)))\n" " end if\n" " enddo\n" " enddo\n") @@ -484,12 +486,12 @@ def test_nemo_example(fortran_reader, fortran_writer, tmpdir): "zmax(1) = MAXVAL(ABS(sshn(:,:) + ssh_ref * tmask(:,:,1)))\n" "end subroutine\n") expected = ( - " zmax(1) = -HUGE(zmax(1))\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(sshn, dim=2), UBOUND(sshn, dim=2), 1\n" " do idx_1 = LBOUND(sshn, dim=1), " "UBOUND(sshn, dim=1), 1\n" - " zmax(1) = MAX(zmax(1), ABS(sshn(idx_1,idx) + ssh_ref * " - "tmask(idx_1,idx,1)))\n" + " reduction_var = MAX(reduction_var, ABS(sshn(idx_1,idx) + " + "ssh_ref * tmask(idx_1,idx,1)))\n" " enddo\n" " enddo\n") psyir = fortran_reader.psyir_from_source(code) @@ -514,10 +516,10 @@ def test_constant_dims(fortran_reader, fortran_writer, tmpdir): "x = maxval(a(:,1)+b(10,:), mask=c(:)==1.0)\n" "end subroutine\n") expected = ( - " x = -HUGE(x)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(a, dim=1), UBOUND(a, dim=1), 1\n" " if (c(idx) == 1.0) then\n" - " x = MAX(x, a(idx,1) + b(10,idx))\n" + " reduction_var = MAX(reduction_var, a(idx,1) + b(10,idx))\n" " end if\n" " enddo\n") psyir = fortran_reader.psyir_from_source(code) @@ -546,11 +548,13 @@ def test_expression_1d(fortran_reader, fortran_writer, tmpdir): " real, dimension(10) :: a\n" " real, dimension(10) :: b\n" " real :: x\n" + " real :: reduction_var\n" " integer :: idx\n\n" - " x = -HUGE(x)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(a, dim=1), UBOUND(a, dim=1), 1\n" - " x = MAX(x, a(idx) + b(idx))\n" - " enddo\n\n" + " reduction_var = MAX(reduction_var, a(idx) + b(idx))\n" + " enddo\n" + " x = reduction_var\n\n" "end subroutine test\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() @@ -577,18 +581,21 @@ def test_expression_3d(fortran_reader, fortran_writer, tmpdir): "subroutine test()\n" " real, dimension(10,10,10) :: a\n" " real :: x\n" + " real :: reduction_var\n" " integer :: idx\n" " integer :: idx_1\n" " integer :: idx_2\n\n" - " x = -HUGE(x)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(a, dim=3), UBOUND(a, dim=3), 1\n" " do idx_1 = LBOUND(a, dim=2), UBOUND(a, dim=2), 1\n" " do idx_2 = LBOUND(a, dim=1), " "UBOUND(a, dim=1), 1\n" - " x = MAX(x, -a(idx_2,idx_1,idx) + 10.0)\n" + " reduction_var = MAX(reduction_var, -a(idx_2,idx_1,idx) " + "+ 10.0)\n" " enddo\n" " enddo\n" - " enddo\n\n" + " enddo\n" + " x = reduction_var\n\n" "end subroutine test\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() @@ -612,11 +619,11 @@ def test_multi_intrinsics(fortran_reader, fortran_writer, tmpdir): "x = maxval(a(:)) + maxval(b(:))\n" "end subroutine\n") expected = ( - " x = -HUGE(x)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(a, dim=1), UBOUND(a, dim=1), 1\n" - " x = MAX(x, a(idx))\n" + " reduction_var = MAX(reduction_var, a(idx))\n" " enddo\n" - " x = x + MAXVAL(b(:))\n") + " x = reduction_var + MAXVAL(b(:))\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/BinaryOp/IntrinsicCall @@ -639,11 +646,11 @@ def test_increment(fortran_reader, fortran_writer, tmpdir): "x = x + maxval(a)\n" "end subroutine\n") expected = ( - " tmp_var = -HUGE(tmp_var)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 10, 1\n" - " tmp_var = MAX(tmp_var, a(idx))\n" + " reduction_var = MAX(reduction_var, a(idx))\n" " enddo\n" - " x = x + tmp_var\n") + " x = x + reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() node = psyir.walk(IntrinsicCall)[0] @@ -665,13 +672,13 @@ def test_increment_with_accessor(fortran_reader, fortran_writer, tmpdir): "real, dimension(1) :: x\n" "x(1) = x(1) + maxval(a)\n" "end subroutine\n") - expected_decl = "real :: tmp_var" + expected_decl = "real :: reduction_var" expected = ( - " tmp_var = -HUGE(tmp_var)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 10, 1\n" - " tmp_var = MAX(tmp_var, a(idx))\n" + " reduction_var = MAX(reduction_var, a(idx))\n" " enddo\n" - " x(1) = x(1) + tmp_var\n") + " x(1) = x(1) + reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() node = psyir.walk(IntrinsicCall)[0] @@ -694,9 +701,9 @@ def test_reduce_to_struct_and_array_accessors(fortran_reader, fortran_writer): "mystruct%x(3) = maxval(a)\n" "end subroutine\n") expected = ( - " mystruct%x(3) = -HUGE(mystruct%x(3))\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 10, 1\n" - " mystruct%x(3) = MAX(mystruct%x(3), a(idx))\n" + " reduction_var = MAX(reduction_var, a(idx))\n" " enddo\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() @@ -724,8 +731,8 @@ def test_range2loop_fails(fortran_reader, fortran_writer): code_before = fortran_writer(psyir) with pytest.raises(TransformationError) as info: trans.apply(node) - assert ("ArrayAssignment2LoopsTrans does not accept calls which " - "are not guaranteed to return a scalar or be elemental" + assert ("Error in Maxval2LoopTrans transformation. Cannot create a " + "temporary variable for 'MAXVAL(a(:) + b(3))'" in str(info.value)) # Check that the failed transformation does not modify the code code_after = fortran_writer(psyir) @@ -740,9 +747,9 @@ def test_replace_reduction_with_type_changes(tmpdir, fortran_reader, ''' code = ( "subroutine maxval_test(array,n,m)\n" - " integer, parameter :: nk = 4\n" + " integer, parameter :: nk = 4, orig = 8\n" " integer :: n, m\n" - " real :: array(10,20)\n" + " real(kind=orig) :: array(10,20)\n" " integer :: result1\n" " integer(kind=nk) :: result2\n" " logical :: result3\n" @@ -756,6 +763,14 @@ def test_replace_reduction_with_type_changes(tmpdir, fortran_reader, if intr_node.intrinsic == IntrinsicCall.Intrinsic.MAXVAL: trans.apply(intr_node) output = fortran_writer(psyir) - print(output) - assert False + + # The aggregation value will be of the same time as the array + assert "real(kind=orig) :: reduction_var\n" in output + assert "real(kind=orig) :: reduction_var_1\n" in output + assert "real(kind=orig) :: reduction_var_2\n" in output + + # The final assignment will have the original cast operation + assert "result1 = INT(reduction_var)" in output + assert "result2 = REAL(reduction_var_1, kind=nk)" in output + assert "result3 = reduction_var_2 > 4" in output assert Compile(tmpdir).string_compiles(output) diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py index f0941d2305..33452590ee 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py @@ -123,14 +123,16 @@ def test_apply(fortran_reader, fortran_writer, tmpdir): " integer :: m\n" " real, dimension(10,20) :: array\n" " real :: result\n" + " real :: reduction_var\n" " integer :: idx\n" " integer :: idx_1\n\n" - " result = -HUGE(result)\n" + " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 20, 1\n" " do idx_1 = 1, 10, 1\n" - " result = MAX(result, array(idx_1,idx))\n" + " reduction_var = MAX(reduction_var, array(idx_1,idx))\n" " enddo\n" - " enddo\n\n" + " enddo\n" + " result = reduction_var\n\n" "end subroutine maxval_test\n") psyir = fortran_reader.psyir_from_source(code) # FileContainer/Routine/Assignment/IntrinsicCall diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py index 5528ed6067..986923a4c0 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py @@ -116,12 +116,13 @@ def test_apply(fortran_reader, fortran_writer, tmpdir): " result = minval(array)\n" "end subroutine\n") expected = ( - " result = HUGE(result)\n" + " reduction_var = HUGE(reduction_var)\n" " do idx = 1, 20, 1\n" " do idx_1 = 1, 10, 1\n" - " result = MIN(result, array(idx_1,idx))\n" + " reduction_var = MIN(reduction_var, array(idx_1,idx))\n" " enddo\n" - " enddo\n") + " enddo\n" + " result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) # FileContainer/Routine/Assignment/IntrinsicCall intrinsic_node = psyir.children[0].children[0].children[1] diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py index 5519d73a52..dc2c2ebf3c 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py @@ -126,12 +126,13 @@ def test_apply(fortran_reader, fortran_writer, tmpdir): " result = product(array)\n" "end subroutine\n") expected = ( - " result = 1.0\n" + " reduction_var = 1.0\n" " do idx = 1, 20, 1\n" " do idx_1 = 1, 10, 1\n" - " result = result * array(idx_1,idx)\n" + " reduction_var = reduction_var * array(idx_1,idx)\n" " enddo\n" - " enddo\n") + " enddo\n" + " result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) # FileContainer/Routine/Assignment/IntrinsicCall intrinsic_node = psyir.children[0].children[0].children[1] diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py index 0896f36906..9dc775f386 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py @@ -125,12 +125,13 @@ def test_apply(fortran_reader, fortran_writer, tmpdir): " result = sum(array)\n" "end subroutine\n") expected = ( - " result = 0.0\n" + " reduction_var = 0.0\n" " do idx = 1, 20, 1\n" " do idx_1 = 1, 10, 1\n" - " result = result + array(idx_1,idx)\n" + " reduction_var = reduction_var + array(idx_1,idx)\n" " enddo\n" - " enddo\n") + " enddo\n" + " result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) # FileContainer/Routine/Assignment/IntrinsicCall intrinsic_node = psyir.children[0].children[0].children[1] From c8395bb939d9dc0f76355e3a0a986017f27e79fa Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Tue, 3 Mar 2026 11:33:14 +0000 Subject: [PATCH 03/10] #3022 Remove TODOs for this issue --- examples/nemo/scripts/omp_cpu_trans.py | 3 +-- examples/nemo/scripts/omp_gpu_trans.py | 3 +-- examples/nemo/scripts/utils.py | 15 ++++++--------- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/examples/nemo/scripts/omp_cpu_trans.py b/examples/nemo/scripts/omp_cpu_trans.py index 7c5713b404..7bf3cb96ec 100755 --- a/examples/nemo/scripts/omp_cpu_trans.py +++ b/examples/nemo/scripts/omp_cpu_trans.py @@ -114,8 +114,7 @@ def trans(psyir): subroutine, hoist_local_arrays=False, convert_array_notation=True, - # See issue #3022 - loopify_array_intrinsics=psyir.name != "getincom.f90", + loopify_array_intrinsics=True, convert_range_loops=True, hoist_expressions=False, scalarise_loops=False diff --git a/examples/nemo/scripts/omp_gpu_trans.py b/examples/nemo/scripts/omp_gpu_trans.py index e0579f4c7c..1e3bc3a9af 100755 --- a/examples/nemo/scripts/omp_gpu_trans.py +++ b/examples/nemo/scripts/omp_gpu_trans.py @@ -203,8 +203,7 @@ def trans(psyir): subroutine, hoist_local_arrays=False, convert_array_notation=True, - # See issue #3022 - loopify_array_intrinsics=psyir.name != "getincom.f90", + loopify_array_intrinsics=True, convert_range_loops=True, increase_array_ranks=not NEMOV4, hoist_expressions=True diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index 43a8dd9ffc..db6da92987 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -218,15 +218,12 @@ def normalise_loops( pass if loopify_array_intrinsics: - filename = schedule.root.name - # TODO #3022: Some files have a bug in Maxval2LoopTrans - if filename not in ("flincom.f90", "histcom.f90", "restcom.f90"): - for intr in schedule.walk(IntrinsicCall): - if intr.intrinsic.name == "MAXVAL": - try: - Maxval2LoopTrans().apply(intr) - except TransformationError as err: - print(err.value) + for intr in schedule.walk(IntrinsicCall): + if intr.intrinsic.name == "MAXVAL": + try: + Maxval2LoopTrans().apply(intr) + except TransformationError as err: + print(err.value) if convert_range_loops: # Convert all array implicit loops to explicit loops From 8bbc724d6146930bd2e955a0312ac96bd7c68e34 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Tue, 3 Mar 2026 13:59:13 +0000 Subject: [PATCH 04/10] #3022 Clean up array_reduction_2_code implementation and tests --- .../intrinsics/array_reduction_base_trans.py | 129 +++++++----------- .../array_reduction_base_trans_test.py | 83 ++++------- .../intrinsics/maxval2loop_trans_test.py | 3 +- .../intrinsics/minval2loop_trans_test.py | 1 + .../intrinsics/product2loop_trans_test.py | 1 + .../intrinsics/sum2loop_trans_test.py | 1 + 6 files changed, 85 insertions(+), 133 deletions(-) diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index 43e44ce9e2..f27d1b4bc2 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -46,13 +46,14 @@ Assignment, Reference, ArrayReference, IfBlock, IntrinsicCall, Node, UnaryOperation, BinaryOperation) from psyclone.psyir.symbols import ( - ArrayType, DataSymbol, ScalarType, UnresolvedType, UnsupportedType) + ArrayType, DataSymbol, UnresolvedType, UnsupportedType) from psyclone.psyGen import Transformation from psyclone.psyir.transformations.reference2arrayrange_trans import \ Reference2ArrayRangeTrans from psyclone.psyir.transformations.transformation_error import \ TransformationError from psyclone.utils import transformation_documentation_wrapper +from psyclone.psyir.transformations import ArrayAssignment2LoopsTrans @transformation_documentation_wrapper @@ -148,6 +149,7 @@ def validate(self, node, options=None, **kwargs): f"{self.name} only works when the intrinsic is part " f"of an Assignment.") + # FIXME: are they now? assignment = array_ref.ancestor(Assignment) for this_node in assignment.lhs.walk(Node): if this_node == array_ref: @@ -164,19 +166,6 @@ def validate(self, node, options=None, **kwargs): f"Unexpected shape for array. Expecting one of " f"Deferred, Attribute or Bounds but found '{shape}'.") - # If the lhs symbol is used anywhere on the assignment rhs, we need - # to create a temporary, and for this we need to resolve its datatype - for rhs_reference in assignment.rhs.walk(Reference): - if rhs_reference.symbol is assignment.lhs.symbol: - if not (isinstance(assignment.lhs.symbol, DataSymbol) and - isinstance(assignment.lhs.datatype, ScalarType)): - line = assignment.debug_string().strip('\n') - raise TransformationError( - f"To loopify '{line}'" - f" we need a temporary variable, but the type of " - f"'{assignment.lhs.debug_string()}' can not be " - f"resolved or is unsupported.") - # pylint: disable=too-many-locals def apply(self, node, options=None, **kwargs): '''Apply the array-reduction intrinsic conversion transformation to @@ -195,7 +184,10 @@ def apply(self, node, options=None, **kwargs): self.validate(node, options, **kwargs) - orig_rhs = node.ancestor(Assignment).rhs.copy() + # Get nodes of interest + orig_assignment = node.ancestor(Assignment) + array_arg = node.argument_by_name("array") + mask_arg = node.argument_by_name("mask") # Create a temporary symbol to accomulate the reduction value tmp_symbol = node.scope.symbol_table.new_symbol( @@ -203,61 +195,51 @@ def apply(self, node, options=None, **kwargs): datatype=node.datatype) tmp_ref = Reference(tmp_symbol) - expr = node.argument_by_name("array") - mask_ref = node.argument_by_name("mask") - - # Step 1: replace all references to arrays within the - # intrinsic expressions and mask argument (if it exists) to - # array ranges. For example, 'maxval(a+b, mask=mod(c,2.0)==1)' + # Step 1: Replace all references to arrays within the intrinsic array + # and mask arguments (if it exists) to array ranges. + # For example, 'maxval(a+b, mask=mod(c,2.0)==1)' # becomes 'maxval(a(:,:)+b(:,:), mask=mod(c(:,:),2.0)==1)' if # 'a', 'b' and 'c' are 2 dimensional arrays. - rhs = expr.copy() - _ = UnaryOperation.create(UnaryOperation.Operator.PLUS, rhs) - reference2arrayrange = Reference2ArrayRangeTrans() - # The reference to rhs becomes invalid in the following - # transformation so we keep a copy of the parent here and - # reset rhs to rhs_parent.children[0] after the - # transformation. - rhs_parent = rhs.parent - for reference in rhs.walk(Reference): - reference2arrayrange.apply(reference) - # Reset rhs from its parent as the previous transformation - # makes the value of rhs become invalid. We know there is only - # one child so can safely use children[0]. - rhs = rhs_parent.children[0] - if mask_ref: - mask_ref_parent = mask_ref.parent - mask_ref_index = mask_ref.position - for reference in mask_ref.walk(Reference): - reference2arrayrange.apply(reference) - mask_ref = mask_ref_parent.children[mask_ref_index] - - # Step 2: Put the intrinsic's extracted expression (stored in - # the 'rhs' variable) on the rhs of an argument with one of - # the arrays within the expression being added to the lhs of - # the argument. For example if: + # We need to do this in a detached copy because Reference2ArrayRange + # does not normally expand arguments of non-elemental functions. + dummy = UnaryOperation.create(UnaryOperation.Operator.PLUS, + array_arg.copy()) + for reference in dummy.walk(Reference): + Reference2ArrayRangeTrans().apply(reference) + new_array_expr = dummy.children[0] + if mask_arg: + dummy = UnaryOperation.create(UnaryOperation.Operator.PLUS, + mask_arg.copy()) + for reference in dummy.walk(Reference): + Reference2ArrayRangeTrans().apply(reference) + new_mask_expr = dummy.children[0] + + # Step 2: Put the intrinsic's extracted expression on the rhs of an + # assignment with one of the arrays within the expression being added + # to the lhs of the assignment (because ArrayAssignment2Loops uses it + # to obtain the loop bounds). For example if: # x = maxval(a(:,:)+b(:,:)) # then # rhs = a(:,:)+b(:,:) # resulting in the following code being created: # a(:,:) = a(:,:)+b(:,:) - array_refs = rhs.walk(ArrayReference) - # The lhs of the created expression needs to be an array - # reference from the expression itself because the - # ArrayAssignment2Loops transformation uses it to obtain the loop - # bounds. - lhs = array_refs[0].copy() - - assignment = Assignment.create(lhs, rhs.detach()) - # Replace existing code so the new code gets access to symbol - # tables etc. - orig_assignment = node.ancestor(Assignment) + array_refs = new_array_expr.walk(ArrayReference) + assignment = Assignment.create(array_refs[0].copy(), + new_array_expr.detach()) + + # If there is a mask, also add its expression to the assignment + if mask_arg: + combined_expression = BinaryOperation.create( + BinaryOperation.Operator.AND, assignment.rhs.detach(), + new_mask_expr.detach()) + assignment.addchild(combined_expression) + + # Replace the existing expression so the new code gets replaces by + # loops in-place and the new iteration symbols have access to the scope orig_assignment.replace_with(assignment) - # Step 3 call ArrayAssignment2Loops to create loop bounds - # and array indexing from the array ranges created in step 2 - # (keeping track of where the new loop nest is created). Also - # extract the mask if it exists. For example: + # Step 3: call ArrayAssignment2Loops to create loop and array indexing + # to the array ranges created in step 2. For example: # a(:,:) = a(:,:)+b(:,:) # becomes # do idx2 = LBOUND(a,2), UBOUND(a,2) @@ -265,18 +247,11 @@ def apply(self, node, options=None, **kwargs): # a(idx,idx2) = a(idx,idx2) + b(idx,idx2) # enddo # enddo - if mask_ref: - # add mask to the rhs of the assignment - assignment_rhs = BinaryOperation.create( - BinaryOperation.Operator.AND, assignment.rhs.copy(), - mask_ref.copy()) - assignment.rhs.replace_with(assignment_rhs) assignment_parent = assignment.parent assignment_position = assignment.position # Must be placed here to avoid circular imports # pylint: disable=import-outside-toplevel - from psyclone.psyir.transformations import ArrayAssignment2LoopsTrans try: ArrayAssignment2LoopsTrans().apply(assignment) except TransformationError as err: @@ -292,14 +267,15 @@ def apply(self, node, options=None, **kwargs): f"expression:\n{assignment.debug_string()}\n into a loop " f"because:\n{err.value}") outer_loop = assignment_parent.children[assignment_position] - if mask_ref: + if mask_arg: # remove mask from the rhs of the assignment - new_assignment = assignment_rhs.children[0].copy() - indexed_mask_ref = assignment_rhs.children[1].copy() - assignment_rhs.replace_with(new_assignment) + indexed_mask_expr = combined_expression.children[1].detach() + assignment.rhs.replace_with( + combined_expression.children[0].detach() + ) - # Step 4 convert the original assignment (now within a loop - # and indexed) to its intrinsic form by replacing the + # Step 4: convert the original assignment (now within a loop + # and indices) to its intrinsic form by replacing the # assignment with tmp=INTRINSIC(orig_lhs,). Also # add in the mask if one has been specified. For example: # do idx2 = LBOUND(a,2), UBOUND(a,2) @@ -318,10 +294,10 @@ def apply(self, node, options=None, **kwargs): new_assignment = Assignment.create( tmp_ref.copy(), self._loop_body( tmp_ref.copy(), assignment.rhs.copy())) - if mask_ref: + if mask_arg: # Place the indexed mask around the statement. new_assignment = IfBlock.create( - indexed_mask_ref.copy(), [new_assignment]) + indexed_mask_expr.copy(), [new_assignment]) assignment.replace_with(new_assignment) @@ -354,7 +330,6 @@ def apply(self, node, options=None, **kwargs): assignment = Assignment.create(lhs, rhs) outer_loop.parent.children.insert(outer_loop.position, assignment) # Update original assignment with the reduced value - rhs = orig_rhs.copy() for child in orig_assignment.walk(Node): if child is node: child.replace_with(tmp_ref.copy()) diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py index e2297ea52c..06e58b43f7 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author: R. W. Ford, STFC Daresbury Laboratory +# Modified: S. Siso, STFC Daresbury Lab '''Module containing tests for the array_reduction_base_trans which is an abstract parent class for the array-reduction intrinsic @@ -225,16 +226,17 @@ def test_validate_increment_with_unsupported_type(fortran_reader): "subroutine test()\n" "use othermod\n" "real :: a(10)\n" - "x(1) = x(1) + maxval(a)\n" + "x(1) = x(1) + maxval(a, mask=b)\n" "end subroutine\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() node = psyir.walk(IntrinsicCall)[0] with pytest.raises(TransformationError) as info: trans.apply(node) - assert ("To loopify 'x(1) = x(1) + MAXVAL(a)' we need a temporary " - "variable, but the type of 'x(1)' can not be resolved or is " - "unsupported." in str(info.value)) + assert ("The supplied node should be a Reference to a DataSymbol but " + "found 'b: Symbol'. Consider adding the name of the " + "file containing the declaration of this quantity to " + "RESOLVE_IMPORTS." in str(info.value)) # apply @@ -253,7 +255,8 @@ def test_apply(idim1, idim2, rdim11, rdim12, rdim21, rdim22, assignment with a single array argument gets transformed as expected. Test with known and unknown array sizes. What we care about here are the initialisation of the result variable, the - generated intrinsic (MAX) and the loop bounds. + generated intrinsic (MAX), the loop bounds, and the assignment + with the replaced value. ''' code = ( @@ -269,7 +272,8 @@ def test_apply(idim1, idim2, rdim11, rdim12, rdim21, rdim22, f" do idx_1 = {rdim11}, {rdim12}, 1\n" f" reduction_var = MAX(reduction_var, array(idx_1,idx))\n" f" enddo\n" - f" enddo\n") + f" enddo\n" + f" result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) # FileContainer/Routine/Assignment/IntrinsicCall node = psyir.walk(IntrinsicCall)[0] @@ -284,8 +288,8 @@ def test_apply_multi(fortran_reader, fortran_writer, tmpdir): '''Test that a MAXVAL intrinsic as part of multiple term on the rhs of an assignment with a single array argument gets transformed as expected. What we care about here are the initialisation of the - result variable, the generated intrinsic (MAX) and the loop - bounds. + result variable, the generated intrinsic (MAX), the loop bounds, and the + assignment with the replaced value. ''' code = ( @@ -319,8 +323,8 @@ def test_apply_dimension_1d(fortran_reader): '''Test that the apply method works as expected when a dimension argument is specified and the array is one dimensional. This should be the same as if dimension were not specified at all. - What we care about here are the initialisation of the result - variable, the generated intrinsic (MAX) and the loop bounds. + + TODO #1658: reductions with dim argument are not supported. ''' code = ( @@ -333,15 +337,16 @@ def test_apply_dimension_1d(fortran_reader): node = psyir.walk(IntrinsicCall)[0] trans = Maxval2LoopTrans() with pytest.raises(TransformationError) as info: - trans.validate(node) + trans.apply(node) assert ("The dimension argument to MAXVAL is not yet supported." in str(info.value)) def test_mask(fortran_reader, fortran_writer, tmpdir): '''Test that the transformation works when there is a mask specified. - What we care about here are the initialisation of the result - variable, the generated intrinsic (MAX) and the loop bounds. + What we care about here are the initialisation of the result variable, + the generated intrinsic (MAX), the loop bounds, and the assignment with + the replaced value. ''' code = ( @@ -394,7 +399,8 @@ def test_mask_array_indexed(fortran_reader, fortran_writer, tmpdir): " if (b(1) > a(idx)) then\n" " reduction_var = MAX(reduction_var, a(idx))\n" " end if\n" - " enddo\n") + " enddo\n" + " result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -405,43 +411,6 @@ def test_mask_array_indexed(fortran_reader, fortran_writer, tmpdir): assert Compile(tmpdir).string_compiles(result) -def test_allocate(fortran_reader, fortran_writer, tmpdir): - '''Test that a newly created array is allocated after the original - array is allocated (if the original array is allocated). Use the - Maxval2LoopTrans transformations (a subclass of - ArrayReductionBaseTrans), as it is easier to test. - - ''' - code = ( - "program sum_test\n" - " real, allocatable :: a(:,:,:)\n" - " real :: result(4,4)\n" - " allocate(a(4,4,4))\n" - " result = maxval(a)\n" - " deallocate(a)\n" - "end program\n") - expected = ( - " ALLOCATE(a(1:4,1:4,1:4))\n" - " reduction_var = -HUGE(reduction_var)\n" - " do idx = LBOUND(a, dim=3), UBOUND(a, dim=3), 1\n" - " do idx_1 = LBOUND(a, dim=2), UBOUND(a, dim=2), 1\n" - " do idx_2 = LBOUND(a, dim=1), UBOUND(a, dim=1), 1\n" - " reduction_var = MAX(reduction_var, a(idx_2,idx_1,idx))\n" - " enddo\n" - " enddo\n" - " enddo\n" - " result = reduction_var\n" - " DEALLOCATE(a)\n") - psyir = fortran_reader.psyir_from_source(code) - trans = Maxval2LoopTrans() - # FileContainer/Routine/Assignment/IntrinsicCall - node = psyir.walk(IntrinsicCall)[1] - trans.apply(node) - result = fortran_writer(psyir) - assert expected in result, result - assert Compile(tmpdir).string_compiles(result) - - def test_references(fortran_reader, fortran_writer, tmpdir): '''Test that the expected result is obtained when the arrays in the expression and mask have References. These are internally changed @@ -465,7 +434,8 @@ def test_references(fortran_reader, fortran_writer, tmpdir): "ssh_ref * tmask(idx_1,idx)))\n" " end if\n" " enddo\n" - " enddo\n") + " enddo\n" + " zmax(1) = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -493,7 +463,8 @@ def test_nemo_example(fortran_reader, fortran_writer, tmpdir): " reduction_var = MAX(reduction_var, ABS(sshn(idx_1,idx) + " "ssh_ref * tmask(idx_1,idx,1)))\n" " enddo\n" - " enddo\n") + " enddo\n" + " zmax(1) = reduction_var") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -521,7 +492,8 @@ def test_constant_dims(fortran_reader, fortran_writer, tmpdir): " if (c(idx) == 1.0) then\n" " reduction_var = MAX(reduction_var, a(idx,1) + b(10,idx))\n" " end if\n" - " enddo\n") + " enddo\n" + " x = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -704,7 +676,8 @@ def test_reduce_to_struct_and_array_accessors(fortran_reader, fortran_writer): " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 10, 1\n" " reduction_var = MAX(reduction_var, a(idx))\n" - " enddo\n") + " enddo\n" + " mystruct%x(3) = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() node = psyir.walk(IntrinsicCall)[0] diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py index 33452590ee..b806f82486 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py @@ -33,11 +33,12 @@ # ----------------------------------------------------------------------------- # Author: R. W. Ford, STFC Daresbury Laboratory # Modified: A. B. G. Chalk, STFC Daresbury Lab +# Modified: S. Siso, STFC Daresbury Lab '''Module containing tests for the maxval2loop transformation.''' -import pytest import warnings +import pytest from psyclone.psyir.nodes import Reference, Literal from psyclone.psyir.symbols import REAL_TYPE, DataSymbol diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py index 986923a4c0..8ead97ff00 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/minval2loop_trans_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author: R. W. Ford, STFC Daresbury Laboratory +# Modified: S. Siso, STFC Daresbury Lab '''Module containing tests for the minval2loop transformation.''' diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py index dc2c2ebf3c..dce30d24b8 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/product2loop_trans_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author: R. W. Ford, STFC Daresbury Laboratory +# Modified: S. Siso, STFC Daresbury Lab '''Module containing tests for the product2loop transformation.''' diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py index 9dc775f386..b3cf7fa303 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/sum2loop_trans_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author: R. W. Ford, STFC Daresbury Laboratory +# Modified: S. Siso, STFC Daresbury Lab '''Module containing tests for the sum2loop transformation.''' From e5092088c01e5b8c824798a6bf75342a3e7135cc Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Thu, 5 Mar 2026 15:09:42 +0000 Subject: [PATCH 05/10] #3022 Fix datatype issue and add verbose option to ArrayReductionBaseTrans --- examples/nemo/scripts/utils.py | 2 +- src/psyclone/psyir/nodes/intrinsic_call.py | 5 ++--- src/psyclone/psyir/tools/type_info_computation.py | 12 +++++++++--- .../intrinsics/array_reduction_base_trans.py | 7 ++++++- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index db6da92987..6ebe7ffbdd 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -221,7 +221,7 @@ def normalise_loops( for intr in schedule.walk(IntrinsicCall): if intr.intrinsic.name == "MAXVAL": try: - Maxval2LoopTrans().apply(intr) + Maxval2LoopTrans().apply(intr, verbose=True) except TransformationError as err: print(err.value) diff --git a/src/psyclone/psyir/nodes/intrinsic_call.py b/src/psyclone/psyir/nodes/intrinsic_call.py index 9bfadbc9b5..4b61c54917 100644 --- a/src/psyclone/psyir/nodes/intrinsic_call.py +++ b/src/psyclone/psyir/nodes/intrinsic_call.py @@ -564,13 +564,12 @@ def _maxval_return_type(node: IntrinsicCall) -> DataType: :returns: the computed datatype for the IntrinsicCall. """ - dtype = ScalarType(node.argument_by_name("array").datatype.intrinsic, - node.argument_by_name("array").datatype.precision) + arg = node.argument_by_name("array") + dtype = node.argument_by_name("array").datatype.elemental_type if "dim" not in node.argument_names: return dtype # We have a dimension specified. We don't know the resultant shape # in any detail as its dependent on the value of dim - arg = node.argument_by_name("array") return _type_of_arg_with_rank_minus_one(arg, dtype) diff --git a/src/psyclone/psyir/tools/type_info_computation.py b/src/psyclone/psyir/tools/type_info_computation.py index d1dbe3e8d2..b7a51ecca4 100644 --- a/src/psyclone/psyir/tools/type_info_computation.py +++ b/src/psyclone/psyir/tools/type_info_computation.py @@ -44,7 +44,7 @@ from psyclone.errors import InternalError from psyclone.psyir.nodes import Reference from psyclone.psyir.symbols.datatypes import ( - ScalarType, UnresolvedType, DataType + ScalarType, UnresolvedType, DataType, ArrayType ) @@ -102,7 +102,7 @@ def compute_precision( def compute_scalar_type( argtypes: list[DataType] -) -> ScalarType.Intrinsic: +) -> ScalarType: ''' Examines the argtypes to determine the base type of the result of a numerical operation with them as operands. Usesthe rules in Section 7.2 @@ -130,7 +130,9 @@ def compute_scalar_type( return UnresolvedType() # If all the datatypes are the same then we can return the first. - if (argtypes[0] == argtypes[1]): + if argtypes[0] == argtypes[1]: + if isinstance(argtypes[0], ArrayType): + return argtypes[0].elemental_type return argtypes[0] # TODO 1590 - ensure support for complex numbers here in the future. @@ -145,8 +147,12 @@ def compute_scalar_type( # If either has REAL intrinsic type, the result is a REAL. if argtypes[0].intrinsic == ScalarType.Intrinsic.REAL: + if isinstance(argtypes[0], ArrayType): + return argtypes[0].elemental_type return argtypes[0] if argtypes[1].intrinsic == ScalarType.Intrinsic.REAL: + if isinstance(argtypes[1], ArrayType): + return argtypes[1].elemental_type return argtypes[1] # Otherwise, the type of the result is not consistent with diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index f27d1b4bc2..ab149b442c 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -167,7 +167,7 @@ def validate(self, node, options=None, **kwargs): f"Deferred, Attribute or Bounds but found '{shape}'.") # pylint: disable=too-many-locals - def apply(self, node, options=None, **kwargs): + def apply(self, node, options=None, verbose: bool = False, **kwargs): '''Apply the array-reduction intrinsic conversion transformation to the specified node. This node must be one of these intrinsic operations which is converted to an equivalent loop structure. @@ -181,6 +181,7 @@ def apply(self, node, options=None, **kwargs): # TODO 2668: options are now deprecated: if options: warnings.warn(self._deprecation_warning, DeprecationWarning, 2) + verbose = options.get("verbose", False) self.validate(node, options, **kwargs) @@ -328,6 +329,10 @@ def apply(self, node, options=None, **kwargs): lhs = tmp_ref.copy() rhs = self._init_var(lhs) assignment = Assignment.create(lhs, rhs) + if verbose: + assignment.preceding_comment = ( + f"{self.name} expansion of: {orig_assignment.debug_string()}" + ) outer_loop.parent.children.insert(outer_loop.position, assignment) # Update original assignment with the reduced value for child in orig_assignment.walk(Node): From 5bbe11b046a74e4e0c689c3175315f3792b0685c Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Thu, 5 Mar 2026 16:45:27 +0000 Subject: [PATCH 06/10] #3022 Improve coverage --- .../intrinsics/array_reduction_base_trans.py | 16 +++++--- .../array_reduction_base_trans_test.py | 41 ++++++++++++++++--- .../intrinsics/maxval2loop_trans_test.py | 4 +- 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index ab149b442c..2885c0b887 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -189,12 +189,7 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): orig_assignment = node.ancestor(Assignment) array_arg = node.argument_by_name("array") mask_arg = node.argument_by_name("mask") - - # Create a temporary symbol to accomulate the reduction value - tmp_symbol = node.scope.symbol_table.new_symbol( - root_name="reduction_var", symbol_type=DataSymbol, - datatype=node.datatype) - tmp_ref = Reference(tmp_symbol) + symtab = node.scope.symbol_table # Step 1: Replace all references to arrays within the intrinsic array # and mask arguments (if it exists) to array ranges. @@ -292,6 +287,15 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): # end if # enddo # enddo + + # Create a temporary symbol to accomulate the reduction value (make + # sure to do that after ArrayAssignment2LoopsTrans, so it does not + # have to be clean up if that transformation fails) + tmp_symbol = symtab.new_symbol( + root_name="reduction_var", symbol_type=DataSymbol, + datatype=node.datatype) + tmp_ref = Reference(tmp_symbol) + new_assignment = Assignment.create( tmp_ref.copy(), self._loop_body( tmp_ref.copy(), assignment.rhs.copy())) diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py index 06e58b43f7..99035088e8 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py @@ -44,7 +44,7 @@ from psyclone.psyir.nodes import IntrinsicCall, Reference from psyclone.psyir.symbols import DataSymbol, REAL_TYPE from psyclone.psyir.transformations import ( - TransformationError, Maxval2LoopTrans) + TransformationError, Maxval2LoopTrans, ArrayAssignment2LoopsTrans) from psyclone.psyir.transformations.intrinsics.array_reduction_base_trans \ import ArrayReductionBaseTrans from psyclone.tests.utilities import Compile @@ -239,7 +239,36 @@ def test_validate_increment_with_unsupported_type(fortran_reader): "RESOLVE_IMPORTS." in str(info.value)) -# apply +def test_apply_unsupported_arrayassingment2loop( + fortran_reader, fortran_writer, monkeypatch): + ''' Check that if an error is produced in the internal call to + ArrayAssignment2LoopsTrans, the proper message is given and the output + code does not have leftover temporaries. + + ''' + code = ( + "subroutine test()\n" + "integer, dimension(10,10) :: a, b\n" + "integer :: c\n" + "c = maxval(a(:,:) + b(:,:))\n" + "end subroutine\n") + psyir = fortran_reader.psyir_from_source(code) + + def mock_validate(*args, **kwargs): + raise TransformationError("myerror") + monkeypatch.setattr(ArrayAssignment2LoopsTrans, "validate", mock_validate) + + trans = Maxval2LoopTrans() + node = psyir.walk(IntrinsicCall)[0] + with pytest.raises(TransformationError) as info: + trans.apply(node) + assert ("ArrayAssignment2LoopsTrans could not convert the expression:\n" + "a(:,:) = a(:,:) + b(:,:)\n\n into a loop because:\n" + "Transformation Error: myerror" in str(info.value)) + + # Also check that the output does not have any leftover temporary var + assert "reduction_var" not in fortran_writer(psyir) + @pytest.mark.parametrize("idim1,idim2,rdim11,rdim12,rdim21,rdim22", [("10", "20", "1", "10", "1", "20"), @@ -520,8 +549,8 @@ def test_expression_1d(fortran_reader, fortran_writer, tmpdir): " real, dimension(10) :: a\n" " real, dimension(10) :: b\n" " real :: x\n" - " real :: reduction_var\n" - " integer :: idx\n\n" + " integer :: idx\n" + " real :: reduction_var\n\n" " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(a, dim=1), UBOUND(a, dim=1), 1\n" " reduction_var = MAX(reduction_var, a(idx) + b(idx))\n" @@ -553,10 +582,10 @@ def test_expression_3d(fortran_reader, fortran_writer, tmpdir): "subroutine test()\n" " real, dimension(10,10,10) :: a\n" " real :: x\n" - " real :: reduction_var\n" " integer :: idx\n" " integer :: idx_1\n" - " integer :: idx_2\n\n" + " integer :: idx_2\n" + " real :: reduction_var\n\n" " reduction_var = -HUGE(reduction_var)\n" " do idx = LBOUND(a, dim=3), UBOUND(a, dim=3), 1\n" " do idx_1 = LBOUND(a, dim=2), UBOUND(a, dim=2), 1\n" diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py index b806f82486..3a8391e628 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/maxval2loop_trans_test.py @@ -124,9 +124,9 @@ def test_apply(fortran_reader, fortran_writer, tmpdir): " integer :: m\n" " real, dimension(10,20) :: array\n" " real :: result\n" - " real :: reduction_var\n" " integer :: idx\n" - " integer :: idx_1\n\n" + " integer :: idx_1\n" + " real :: reduction_var\n\n" " reduction_var = -HUGE(reduction_var)\n" " do idx = 1, 20, 1\n" " do idx_1 = 1, 10, 1\n" From e11f1c880215fe9db655ce07b18ca57f95c83e6e Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Fri, 6 Mar 2026 14:33:13 +0000 Subject: [PATCH 07/10] #3022 Remove a fixme --- .../transformations/intrinsics/array_reduction_base_trans.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index 2885c0b887..aa543bcc23 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -149,7 +149,6 @@ def validate(self, node, options=None, **kwargs): f"{self.name} only works when the intrinsic is part " f"of an Assignment.") - # FIXME: are they now? assignment = array_ref.ancestor(Assignment) for this_node in assignment.lhs.walk(Node): if this_node == array_ref: From 3038b9b6839ec27155a226c1d54b99999654f462 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Fri, 6 Mar 2026 16:00:24 +0000 Subject: [PATCH 08/10] #3022 Add tests for missing coverage --- .../intrinsics/array_reduction_base_trans.py | 14 +++++--- .../array_reduction_base_trans_test.py | 34 ++++++++++++++----- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index aa543bcc23..7da9514aa2 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -94,6 +94,7 @@ def validate(self, node, options=None, **kwargs): an assignment. ''' + super().validate(node, options=options, **kwargs) if not options: self.validate_options(**kwargs) @@ -182,7 +183,7 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): warnings.warn(self._deprecation_warning, DeprecationWarning, 2) verbose = options.get("verbose", False) - self.validate(node, options, **kwargs) + self.validate(node, options, verbose=verbose, **kwargs) # Get nodes of interest orig_assignment = node.ancestor(Assignment) @@ -256,6 +257,11 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): # to the original statement (with maybe some leftover tmp variable) # and produce the error here. assignment.replace_with(orig_assignment) + if verbose: + orig_assignment.append_preceding_comment( + f"{self.name} failed because ArrayAssignment2LoopsTrans: " + f"{err.value}" + ) # pylint: disable=raise-missing-from raise TransformationError( f"ArrayAssignment2LoopsTrans could not convert the " @@ -333,9 +339,9 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): rhs = self._init_var(lhs) assignment = Assignment.create(lhs, rhs) if verbose: - assignment.preceding_comment = ( - f"{self.name} expansion of: {orig_assignment.debug_string()}" - ) + code = orig_assignment.debug_string().strip() + assignment.append_preceding_comment( + f"{self.name} expansion of: {code}") outer_loop.parent.children.insert(outer_loop.position, assignment) # Update original assignment with the reduced value for child in orig_assignment.walk(Node): diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py index 99035088e8..38abd70632 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py @@ -239,11 +239,11 @@ def test_validate_increment_with_unsupported_type(fortran_reader): "RESOLVE_IMPORTS." in str(info.value)) -def test_apply_unsupported_arrayassingment2loop( +def test_apply_unsupported_arrayassingment2loop_and_compute_arg_names( fortran_reader, fortran_writer, monkeypatch): ''' Check that if an error is produced in the internal call to - ArrayAssignment2LoopsTrans, the proper message is given and the output - code does not have leftover temporaries. + ArrayAssignment2LoopsTrans or compute_argument_names, the proper message + is given and the output code does not have leftover temporaries. ''' code = ( @@ -253,21 +253,36 @@ def test_apply_unsupported_arrayassingment2loop( "c = maxval(a(:,:) + b(:,:))\n" "end subroutine\n") psyir = fortran_reader.psyir_from_source(code) + trans = Maxval2LoopTrans() + node = psyir.walk(IntrinsicCall)[0] + # Create a faulty ArrayAssignment2LoopsTrans def mock_validate(*args, **kwargs): raise TransformationError("myerror") monkeypatch.setattr(ArrayAssignment2LoopsTrans, "validate", mock_validate) - trans = Maxval2LoopTrans() - node = psyir.walk(IntrinsicCall)[0] with pytest.raises(TransformationError) as info: - trans.apply(node) + trans.apply(node, verbose=True) assert ("ArrayAssignment2LoopsTrans could not convert the expression:\n" "a(:,:) = a(:,:) + b(:,:)\n\n into a loop because:\n" "Transformation Error: myerror" in str(info.value)) - - # Also check that the output does not have any leftover temporary var + # Also check that the output does not have any leftover temporary var, but + # it has a verbose comment if requested + code = fortran_writer(psyir) assert "reduction_var" not in fortran_writer(psyir) + assert ("! Maxval2LoopTrans failed because ArrayAssignment2LoopsTrans: " + "Transformation Error: myerror" in code) + + # Create a faulty compute_argument_names + def mock_compute(*args, **kwargs): + raise NotImplementedError("myerror") + monkeypatch.setattr(IntrinsicCall, "compute_argument_names", mock_compute) + + with pytest.raises(TransformationError) as info: + trans.apply(node) + assert ("Error in Maxval2LoopTrans transformation. Cannot disambiguate " + "the arguments names in 'MAXVAL(array=a(:,:) + b(:,:))'" + in str(info.value)) @pytest.mark.parametrize("idim1,idim2,rdim11,rdim12,rdim21,rdim22", @@ -296,6 +311,7 @@ def test_apply(idim1, idim2, rdim11, rdim12, rdim21, rdim22, f" result = maxval(array)\n" f"end subroutine\n") expected = ( + f" ! Maxval2LoopTrans expansion of: result = MAXVAL(array)\n" f" reduction_var = -HUGE(reduction_var)\n" f" do idx = {rdim21}, {rdim22}, 1\n" f" do idx_1 = {rdim11}, {rdim12}, 1\n" @@ -307,7 +323,7 @@ def test_apply(idim1, idim2, rdim11, rdim12, rdim21, rdim22, # FileContainer/Routine/Assignment/IntrinsicCall node = psyir.walk(IntrinsicCall)[0] trans = Maxval2LoopTrans() - trans.apply(node) + trans.apply(node, verbose=True) result = fortran_writer(psyir) assert expected in result assert Compile(tmpdir).string_compiles(result) From f9b4d529bddedefa62600745f3c8d7f243ba21e5 Mon Sep 17 00:00:00 2001 From: Sergi Siso Date: Tue, 17 Mar 2026 13:11:33 +0000 Subject: [PATCH 09/10] #3022 Improve implementation of ArrayReductionBaseTrans --- src/psyclone/psyir/nodes/intrinsic_call.py | 2 +- .../psyir/tools/type_info_computation.py | 4 ++-- .../intrinsics/array_reduction_base_trans.py | 17 ++++++++++------- .../array_reduction_base_trans_test.py | 11 ++++++----- 4 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/psyclone/psyir/nodes/intrinsic_call.py b/src/psyclone/psyir/nodes/intrinsic_call.py index 4b61c54917..cea33cea06 100644 --- a/src/psyclone/psyir/nodes/intrinsic_call.py +++ b/src/psyclone/psyir/nodes/intrinsic_call.py @@ -565,7 +565,7 @@ def _maxval_return_type(node: IntrinsicCall) -> DataType: :returns: the computed datatype for the IntrinsicCall. """ arg = node.argument_by_name("array") - dtype = node.argument_by_name("array").datatype.elemental_type + dtype = arg.datatype.elemental_type if "dim" not in node.argument_names: return dtype # We have a dimension specified. We don't know the resultant shape diff --git a/src/psyclone/psyir/tools/type_info_computation.py b/src/psyclone/psyir/tools/type_info_computation.py index b7a51ecca4..ff03ae6860 100644 --- a/src/psyclone/psyir/tools/type_info_computation.py +++ b/src/psyclone/psyir/tools/type_info_computation.py @@ -105,13 +105,13 @@ def compute_scalar_type( ) -> ScalarType: ''' Examines the argtypes to determine the base type of the result of a - numerical operation with them as operands. Usesthe rules in Section 7.2 + numerical operation with them as operands. Uses the rules in Section 7.2 of the Fortran2008 standard. If the type cannot be determined then an instance of `UnresolvedType` is returned. :param argtypes: the types of the arguments. - :returns: the base (scalar) type of the result of the input arguments. + :returns: the elemental type of the result of the input arguments. :raises InternalError: If more than two argument types are provided. :raises TypeError: If the types differ and any are not a numeric datatype. diff --git a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py index 7da9514aa2..05be7ea9d4 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -130,7 +130,8 @@ def validate(self, node, options=None, **kwargs): if isinstance(node.datatype, (UnresolvedType, UnsupportedType)): raise TransformationError( f"Error in {self.name} transformation. Cannot create " - f"a temporary variable for '{node.debug_string()}'") + f"a temporary variable for '{node.debug_string()}' " + f"because it is of '{node.datatype}'.") # There should be at least one arrayreference or reference to # an array in the expression @@ -191,7 +192,7 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): mask_arg = node.argument_by_name("mask") symtab = node.scope.symbol_table - # Step 1: Replace all references to arrays within the intrinsic array + # Step 1: Convert all references to arrays within the intrinsic array # and mask arguments (if it exists) to array ranges. # For example, 'maxval(a+b, mask=mod(c,2.0)==1)' # becomes 'maxval(a(:,:)+b(:,:), mask=mod(c(:,:),2.0)==1)' if @@ -293,9 +294,9 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): # enddo # enddo - # Create a temporary symbol to accomulate the reduction value (make + # Create a temporary symbol to accumulate the reduction value (make # sure to do that after ArrayAssignment2LoopsTrans, so it does not - # have to be clean up if that transformation fails) + # have to be cleaned up if that transformation fails) tmp_symbol = symtab.new_symbol( root_name="reduction_var", symbol_type=DataSymbol, datatype=node.datatype) @@ -343,13 +344,15 @@ def apply(self, node, options=None, verbose: bool = False, **kwargs): assignment.append_preceding_comment( f"{self.name} expansion of: {code}") outer_loop.parent.children.insert(outer_loop.position, assignment) - # Update original assignment with the reduced value - for child in orig_assignment.walk(Node): + # Update original assignment with the reduced value (after that + # the orig_assignment will not be usable) + replacement_assignment = orig_assignment + for child in replacement_assignment.walk(Node): if child is node: child.replace_with(tmp_ref.copy()) break outer_loop.parent.children.insert(outer_loop.position+1, - orig_assignment) + replacement_assignment) @abstractmethod def _loop_body(self, lhs, rhs): diff --git a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py index 38abd70632..5bbdacc4fd 100644 --- a/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/intrinsics/array_reduction_base_trans_test.py @@ -239,7 +239,7 @@ def test_validate_increment_with_unsupported_type(fortran_reader): "RESOLVE_IMPORTS." in str(info.value)) -def test_apply_unsupported_arrayassingment2loop_and_compute_arg_names( +def test_apply_unsupported_arrayassignment2loop_and_compute_arg_names( fortran_reader, fortran_writer, monkeypatch): ''' Check that if an error is produced in the internal call to ArrayAssignment2LoopsTrans or compute_argument_names, the proper message @@ -269,7 +269,7 @@ def mock_validate(*args, **kwargs): # Also check that the output does not have any leftover temporary var, but # it has a verbose comment if requested code = fortran_writer(psyir) - assert "reduction_var" not in fortran_writer(psyir) + assert "reduction_var" not in code assert ("! Maxval2LoopTrans failed because ArrayAssignment2LoopsTrans: " "Transformation Error: myerror" in code) @@ -750,7 +750,8 @@ def test_range2loop_fails(fortran_reader, fortran_writer): with pytest.raises(TransformationError) as info: trans.apply(node) assert ("Error in Maxval2LoopTrans transformation. Cannot create a " - "temporary variable for 'MAXVAL(a(:) + b(3))'" + "temporary variable for 'MAXVAL(a(:) + b(3))' because it is" + " of 'UnresolvedType'." in str(info.value)) # Check that the failed transformation does not modify the code code_after = fortran_writer(psyir) @@ -760,7 +761,7 @@ def test_range2loop_fails(fortran_reader, fortran_writer): def test_replace_reduction_with_type_changes(tmpdir, fortran_reader, fortran_writer): ''' Check that the temporary created to aggregate the partial results - is of the correct type even when the the reduction operation is inside and + is of the correct type even when the the reduction operation is inside an expression that has implicit or explicit casts. ''' code = ( @@ -782,7 +783,7 @@ def test_replace_reduction_with_type_changes(tmpdir, fortran_reader, trans.apply(intr_node) output = fortran_writer(psyir) - # The aggregation value will be of the same time as the array + # The aggregation value will be of the same type as the array assert "real(kind=orig) :: reduction_var\n" in output assert "real(kind=orig) :: reduction_var_1\n" in output assert "real(kind=orig) :: reduction_var_2\n" in output From 8b63598341b46064055f64fc0d64a0f528b3e68a Mon Sep 17 00:00:00 2001 From: Andrew Porter Date: Wed, 18 Mar 2026 11:01:19 +0000 Subject: [PATCH 10/10] #3359 update changelog --- changelog | 3 +++ 1 file changed, 3 insertions(+) diff --git a/changelog b/changelog index a32e9e3802..fe14e0bdba 100644 --- a/changelog +++ b/changelog @@ -1,3 +1,6 @@ + 66) PR #3359 for #3022. Uses the improved datatype functionality to + correctly transform reduction intrinsics into native code. + 65) PR #3365 towards #2668. Updates more transformations to accept keyword arguments instead of options.