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. 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..6ebe7ffbdd 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, verbose=True) + except TransformationError as err: + print(err.value) if convert_range_loops: # Convert all array implicit loops to explicit loops diff --git a/src/psyclone/psyir/nodes/intrinsic_call.py b/src/psyclone/psyir/nodes/intrinsic_call.py index 9bfadbc9b5..cea33cea06 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 = 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 # 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..ff03ae6860 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,16 +102,16 @@ 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 + 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. @@ -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 4ad327d6de..05be7ea9d4 100644 --- a/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/array_reduction_base_trans.py @@ -45,13 +45,15 @@ 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, 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 @@ -64,31 +66,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.") @@ -117,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) @@ -133,14 +111,28 @@ 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. 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()}' " + f"because it is of '{node.datatype}'.") + # There should be at least one arrayreference or reference to # an array in the expression # pylint: disable=unidiomatic-typecheck @@ -175,21 +167,8 @@ 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): + 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. @@ -203,83 +182,61 @@ 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) - - 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() + self.validate(node, options, verbose=verbose, **kwargs) - expr, _, mask_ref = self._get_args(node) + # Get nodes of interest + orig_assignment = node.ancestor(Assignment) + array_arg = node.argument_by_name("array") + mask_arg = node.argument_by_name("mask") + symtab = node.scope.symbol_table - # 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: 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 # '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) @@ -287,18 +244,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: @@ -308,21 +258,27 @@ def apply(self, node, options=None, **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 " 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 - orig_assignment = assignment_rhs.children[0].copy() - indexed_mask_ref = assignment_rhs.children[1].copy() - assignment_rhs.replace_with(orig_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 + 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 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) # do idx = LBOUND(a,1), UBOUND(a,1) @@ -337,13 +293,22 @@ def apply(self, node, options=None, **kwargs): # end if # enddo # enddo + + # Create a temporary symbol to accumulate the reduction value (make + # sure to do that after ArrayAssignment2LoopsTrans, so it does not + # have to be cleaned 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( - new_lhs.copy(), self._loop_body( - new_lhs.copy(), assignment.rhs.copy())) - if mask_ref: + tmp_ref.copy(), self._loop_body( + tmp_ref.copy(), assignment.rhs.copy())) + 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) @@ -370,23 +335,24 @@ 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) + if verbose: + 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) - 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 (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, + 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 c5f36ab0c8..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 @@ -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 @@ -40,11 +41,10 @@ ''' 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) + TransformationError, Maxval2LoopTrans, ArrayAssignment2LoopsTrans) from psyclone.psyir.transformations.intrinsics.array_reduction_base_trans \ import ArrayReductionBaseTrans from psyclone.tests.utilities import Compile @@ -65,24 +65,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 " @@ -244,19 +226,64 @@ 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)) + + +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 + 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) + 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) + + with pytest.raises(TransformationError) as info: + 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, but + # it has a verbose comment if requested + code = fortran_writer(psyir) + assert "reduction_var" not in code + 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)) -# apply @pytest.mark.parametrize("idim1,idim2,rdim11,rdim12,rdim21,rdim22", [("10", "20", "1", "10", "1", "20"), @@ -272,7 +299,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 = ( @@ -283,17 +311,19 @@ 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" ! 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" - 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") + f" enddo\n" + f" result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) # 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) @@ -303,8 +333,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 = ( @@ -316,13 +346,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 @@ -338,8 +368,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 = ( @@ -352,15 +382,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 = ( @@ -370,14 +401,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] @@ -407,12 +439,13 @@ 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") + " enddo\n" + " result = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -423,42 +456,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" - " result = -HUGE(result)\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" - " enddo\n" - " enddo\n" - " enddo\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 @@ -474,15 +471,16 @@ 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") + " enddo\n" + " zmax(1) = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -503,14 +501,15 @@ 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") + " enddo\n" + " zmax(1) = reduction_var") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -533,12 +532,13 @@ 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") + " enddo\n" + " x = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() # FileContainer/Routine/Assignment/IntrinsicCall @@ -565,11 +565,13 @@ def test_expression_1d(fortran_reader, fortran_writer, tmpdir): " real, dimension(10) :: a\n" " real, dimension(10) :: b\n" " real :: x\n" - " integer :: idx\n\n" - " x = -HUGE(x)\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" - " 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() @@ -598,16 +600,19 @@ def test_expression_3d(fortran_reader, fortran_writer, tmpdir): " real :: x\n" " integer :: idx\n" " integer :: idx_1\n" - " integer :: idx_2\n\n" - " x = -HUGE(x)\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" " 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() @@ -631,11 +636,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 @@ -658,11 +663,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] @@ -684,13 +689,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] @@ -713,10 +718,11 @@ 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" - " enddo\n") + " reduction_var = MAX(reduction_var, a(idx))\n" + " enddo\n" + " mystruct%x(3) = reduction_var\n") psyir = fortran_reader.psyir_from_source(code) trans = Maxval2LoopTrans() node = psyir.walk(IntrinsicCall)[0] @@ -743,9 +749,47 @@ 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))' because it is" + " of 'UnresolvedType'." in str(info.value)) # 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 an + expression that has implicit or explicit casts. + ''' + code = ( + "subroutine maxval_test(array,n,m)\n" + " integer, parameter :: nk = 4, orig = 8\n" + " integer :: n, m\n" + " real(kind=orig) :: 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) + + # 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 + + # 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..3a8391e628 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 @@ -124,13 +125,15 @@ def test_apply(fortran_reader, fortran_writer, tmpdir): " real, dimension(10,20) :: array\n" " real :: result\n" " integer :: idx\n" - " integer :: idx_1\n\n" - " result = -HUGE(result)\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" - " 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..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.''' @@ -116,12 +117,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..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.''' @@ -126,12 +127,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..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.''' @@ -125,12 +126,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]