diff --git a/changelog b/changelog index 9fd88419a5..0811c1773e 100644 --- a/changelog +++ b/changelog @@ -44,6 +44,9 @@ 15) PR #2855 for #2853. Allows the Fortran standard used by fparser to be set in the config file and ensure it is used consistently. + 16) PR #2857 for 2854. Excludes character assignments from ACC KERNELS + regions to avoid issues in NEMO. + release 3.0.0 6th of December 2024 1) PR #2477 for #2463. Add support for Fortran Namelist statements. diff --git a/psyclone.pdf b/psyclone.pdf index 5abfb3ba63..f3c9b723a1 100644 Binary files a/psyclone.pdf and b/psyclone.pdf differ diff --git a/src/psyclone/psyir/nodes/structure_reference.py b/src/psyclone/psyir/nodes/structure_reference.py index ac04a2e1e9..2479aba66a 100644 --- a/src/psyclone/psyir/nodes/structure_reference.py +++ b/src/psyclone/psyir/nodes/structure_reference.py @@ -336,7 +336,8 @@ def _get_cursor_shape(cursor, cursor_type): if not isinstance(cursor_type, (UnresolvedType, UnsupportedType)): # Once we've hit an Unresolved/UnsupportedType the cursor_type # will remain set to that as we can't do any better. - cursor_type = cursor_type.components[cursor.name].datatype + cursor_type = cursor_type.components[ + cursor.name.lower()].datatype try: cursor_shape = _get_cursor_shape(cursor, cursor_type) except NotImplementedError: diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index e37fe94422..7685f0db13 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -1051,23 +1051,25 @@ def add(self, name, datatype, visibility, initial_value, f"be a 'str' but got " f"'{type(inline_comment).__name__}'") - self._components[name] = self.ComponentType(name, datatype, visibility, - initial_value) + key_name = name.lower() + self._components[key_name] = self.ComponentType(name, datatype, + visibility, + initial_value) # Use object.__setattr__ due to the frozen nature of ComponentType - object.__setattr__(self._components[name], + object.__setattr__(self._components[key_name], "_preceding_comment", preceding_comment) - object.__setattr__(self._components[name], + object.__setattr__(self._components[key_name], "_inline_comment", inline_comment) def lookup(self, name): ''' - :returns: the ComponentType tuple describing the named member of this \ + :returns: the ComponentType tuple describing the named member of this StructureType. :rtype: :py:class:`psyclone.psyir.symbols.StructureType.ComponentType` ''' - return self._components[name] + return self._components[name.lower()] def __eq__(self, other): ''' @@ -1112,7 +1114,8 @@ def replace_symbols_using(self, table): if component.initial_value: component.initial_value.replace_symbols_using(table) # Construct the new ComponentType - new_components[component.name] = StructureType.ComponentType( + key_name = component.name.lower() + new_components[key_name] = StructureType.ComponentType( component.name, new_type, component.visibility, component.initial_value) self._components = new_components diff --git a/src/psyclone/psyir/transformations/acc_kernels_trans.py b/src/psyclone/psyir/transformations/acc_kernels_trans.py index 285ac7db26..2ec6a23880 100644 --- a/src/psyclone/psyir/transformations/acc_kernels_trans.py +++ b/src/psyclone/psyir/transformations/acc_kernels_trans.py @@ -39,12 +39,15 @@ ''' This module provides the ACCKernelsTrans transformation. ''' import re +from typing import List, Union from psyclone import psyGen from psyclone.psyir.nodes import ( - ACCKernelsDirective, Assignment, Call, CodeBlock, Loop, PSyDataNode, - Reference, Return, Routine, Statement, WhileLoop) + ACCKernelsDirective, Assignment, Call, CodeBlock, Loop, + Node, PSyDataNode, Reference, Return, Routine, Statement, WhileLoop) from psyclone.psyir.symbols import UnsupportedFortranType +from psyclone.psyir.transformations.arrayassignment2loops_trans import ( + ArrayAssignment2LoopsTrans) from psyclone.psyir.transformations.region_trans import RegionTrans from psyclone.psyir.transformations.transformation_error import ( TransformationError) @@ -74,12 +77,12 @@ class ACCKernelsTrans(RegionTrans): excluded_node_types = (CodeBlock, Return, PSyDataNode, psyGen.HaloExchange, WhileLoop) - def apply(self, node, options=None): + def apply(self, node: Union[Node, List[Node]], options: dict = None): ''' Enclose the supplied list of PSyIR nodes within an OpenACC Kernels region. - :param node: a node or list of nodes in the PSyIR to enclose. + :param node: the node(s) in the PSyIR to enclose. :type node: :py:class:`psyclone.psyir.nodes.Node` | list[:py:class:`psyclone.psyir.nodes.Node`] :param options: a dictionary with options for transformations. @@ -88,6 +91,11 @@ def apply(self, node, options=None): region should have the 'default present' attribute (indicating that data is already on the accelerator). When using managed memory this option should be False. + :param bool options["allow_string"]: whether to allow the + transformation on assignments involving character types. Defaults + to False. + :param bool options["verbose"]: log the reason the validation failed, + at the moment with a comment in the provided PSyIR node. ''' # Ensure we are always working with a list of nodes, even if only @@ -110,7 +118,8 @@ def apply(self, node, options=None): parent.children.insert(start_index, directive) - def validate(self, nodes, options=None): + def validate(self, nodes: Union[Node, List[Node]], + options: dict = None) -> None: # pylint: disable=signature-differs ''' Check that we can safely enclose the supplied node or list of nodes @@ -124,6 +133,11 @@ def validate(self, nodes, options=None): :param bool options["disable_loop_check"]: whether to disable the check that the supplied region contains 1 or more loops. Default is False (i.e. the check is enabled). + :param bool options["allow_string"]: whether to allow the + transformation on assignments involving character types. Defaults + to False. + :param bool options["verbose"]: log the reason the validation failed, + at the moment with a comment in the provided PSyIR node. :raises NotImplementedError: if the supplied Nodes belong to a GOInvokeSchedule. @@ -133,8 +147,13 @@ def validate(self, nodes, options=None): a routine that is not available on the accelerator. :raises TransformationError: if there are no Loops within the proposed region and options["disable_loop_check"] is not True. + :raises TransformationError: if any assignments in the region contain a + character type child and options["allow_string"] is not True. ''' + if not options: + options = {} + # Ensure we are always working with a list of nodes, even if only # one was supplied via the `nodes` argument. node_list = self.get_node_list(nodes) @@ -180,6 +199,17 @@ def validate(self, nodes, options=None): f"Assumed-size character variables cannot be enclosed " f"in an OpenACC region but found " f"'{stmt.debug_string()}'") + # Check there are no character assignments in the region as these + # cause various problems with (at least) NVHPC <= 24.5 + if not options.get("allow_string", False): + message = ( + f"{self.name} does not permit assignments involving " + f"character variables by default (use the 'allow_string' " + f"option to include them)") + for assign in node.walk(Assignment): + ArrayAssignment2LoopsTrans.validate_no_char( + assign, message, options) + # Check that any called routines are supported on the device. for icall in node.walk(Call): if not icall.is_available_on_device(): diff --git a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py index ada451e0ae..150c97bbda 100644 --- a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py +++ b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py @@ -45,7 +45,7 @@ from psyclone.errors import LazyString from psyclone.psyGen import Transformation from psyclone.psyir.nodes import ( - Assignment, Call, IntrinsicCall, Loop, Literal, Range, Reference, + Assignment, Call, IntrinsicCall, Loop, Literal, Node, Range, Reference, CodeBlock, Routine, BinaryOperation) from psyclone.psyir.nodes.array_mixin import ArrayMixin from psyclone.psyir.symbols import ( @@ -285,23 +285,10 @@ def validate(self, node, options=None): # If we allow string arrays then we can skip the check. if not options.get("allow_string", False): - for child in node.walk((Literal, Reference)): - try: - forbidden = ScalarType.Intrinsic.CHARACTER - if (child.is_character(unknown_as=False) or - (child.symbol.datatype.intrinsic == forbidden)): - message = (f"{self.name} does not expand ranges " - f"on character arrays by default (use the" - f"'allow_string' option to expand them)") - if verbose: - node.append_preceding_comment(message) - # pylint: disable=cell-var-from-loop - raise TransformationError(LazyString( - lambda: f"{message}, but found:" - f"\n{node.debug_string()}")) - except (NotImplementedError, AttributeError): - # We cannot always get the datatype, we ignore this for now - pass + message = (f"{self.name} does not expand ranges " + f"on character arrays by default (use the" + f"'allow_string' option to expand them)") + self.validate_no_char(node, message, options) # We don't accept calls that are not guaranteed to be elemental for call in node.rhs.walk(Call): @@ -372,6 +359,41 @@ def validate(self, node, options=None): raise TransformationError(LazyString( lambda: f"{message} In:\n{node.debug_string()}")) + @staticmethod + def validate_no_char(node: Node, message: str, options: dict) -> None: + ''' + Check that there is no character variable accessed in the sub-tree with + the supplied node at its root. + + :param node: the root node to check for character assignments. + :param message: the message to use if a character assignment is found. + :param options: any options that apply to this check. + :param bool options["verbose"]: log the reason the validation failed, + at the moment with a comment in the provided PSyIR node. + + :raises TransformationError: if the supplied node contains a + child of character type. + + ''' + # Whether or not to log the resason for raising an error. At the moment + # "logging" means adding a comment in the output code. + verbose = options.get("verbose", False) + + for child in node.walk((Literal, Reference)): + try: + forbidden = ScalarType.Intrinsic.CHARACTER + if (child.is_character(unknown_as=False) or + (child.symbol.datatype.intrinsic == forbidden)): + if verbose: + node.append_preceding_comment(message) + # pylint: disable=cell-var-from-loop + raise TransformationError(LazyString( + lambda: f"{message}, but found:" + f"\n{node.debug_string()}")) + except (NotImplementedError, AttributeError): + # We cannot always get the datatype, we ignore this for now + pass + __all__ = [ 'ArrayAssignment2LoopsTrans'] diff --git a/src/psyclone/tests/psyir/nodes/structure_reference_test.py b/src/psyclone/tests/psyir/nodes/structure_reference_test.py index 7a0ec8e921..c40a78b6b4 100644 --- a/src/psyclone/tests/psyir/nodes/structure_reference_test.py +++ b/src/psyclone/tests/psyir/nodes/structure_reference_test.py @@ -232,7 +232,7 @@ def test_struc_ref_datatype(): '''Test the datatype() method of StructureReference.''' atype = symbols.ArrayType(symbols.REAL_TYPE, [10, 8]) rtype = symbols.StructureType.create([ - ("gibber", symbols.BOOLEAN_TYPE, symbols.Symbol.Visibility.PUBLIC, + ("Gibber", symbols.BOOLEAN_TYPE, symbols.Symbol.Visibility.PUBLIC, None)]) # TODO #1031. Currently cannot create an array of StructureTypes # directly - have to have a DataTypeSymbol. @@ -241,10 +241,10 @@ def test_struc_ref_datatype(): grid_type = symbols.StructureType.create([ ("nx", symbols.INTEGER_TYPE, symbols.Symbol.Visibility.PUBLIC, None), ("data", atype, symbols.Symbol.Visibility.PRIVATE, None), - # A single member of structure type. - ("roger", rtype, symbols.Symbol.Visibility.PUBLIC, None), + # A single member of structure type with a mixed-case name. + ("roGEr", rtype, symbols.Symbol.Visibility.PUBLIC, None), # An array of structure type. - ("titty", artype, symbols.Symbol.Visibility.PUBLIC, None)]) + ("lucy", artype, symbols.Symbol.Visibility.PUBLIC, None)]) # Symbol with type defined by StructureType ssym0 = symbols.DataSymbol("grid", grid_type) # Reference to scalar member of structure @@ -266,34 +266,34 @@ def test_struc_ref_datatype(): gref = nodes.StructureReference.create(ssym, ["roger", "gibber"]) assert gref.datatype == symbols.BOOLEAN_TYPE - # Reference to structure member of structure - rref = nodes.StructureReference.create(ssym, ["roger"]) + # Reference to structure member of structure using different capitalisation + rref = nodes.StructureReference.create(ssym, ["rogeR"]) assert rref.datatype == rtype # Reference to single element of array of structures within a structure singleref = nodes.StructureReference.create( - ssym, [("titty", [one.copy(), two.copy()])]) + ssym, [("lucy", [one.copy(), two.copy()])]) assert singleref.datatype == rtype_sym - # Reference to grid%titty(1,2)%gibber + # Reference to grid%lucy(1,2)%gibber sref = nodes.StructureReference.create( - ssym, [("titty", [one.copy(), two.copy()]), "gibber"]) + ssym, [("lucy", [one.copy(), two.copy()]), "gibber"]) assert sref.datatype == symbols.BOOLEAN_TYPE - # Reference to grid%titty(my_func(1), 2) where my_func is unresolved. + # Reference to grid%lucy(my_func(1), 2) where my_func is unresolved. func_sym = symbols.DataSymbol("my_func", datatype=symbols.UnresolvedType(), interface=symbols.UnresolvedInterface()) fref = nodes.ArrayReference.create(func_sym, [one.copy()]) sref2 = nodes.StructureReference.create( - ssym, [("titty", [fref, two.copy()])]) + ssym, [("lucy", [fref, two.copy()])]) assert isinstance(sref2.datatype, symbols.UnresolvedType) # Reference to sub-array of structure members of structure myrange = nodes.Range.create(two.copy(), nodes.Literal("4", symbols.INTEGER_TYPE)) arref = nodes.StructureReference.create( - ssym, [("titty", [nodes.Literal("3", symbols.INTEGER_TYPE), myrange])]) + ssym, [("lucy", [nodes.Literal("3", symbols.INTEGER_TYPE), myrange])]) dtype = arref.datatype assert isinstance(dtype, symbols.ArrayType) assert dtype.intrinsic == rtype_sym @@ -302,7 +302,7 @@ def test_struc_ref_datatype(): assert isinstance(dtype.shape[0].upper, nodes.BinaryOperation) # Reference to whole array of structures that are a member of a structure - fullref = nodes.StructureReference.create(ssym, ["titty"]) + fullref = nodes.StructureReference.create(ssym, ["lucy"]) dtype = fullref.datatype assert dtype == artype diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 27382f3cfa..09df5046a1 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -894,8 +894,11 @@ def test_structure_type(): stype = StructureType() assert str(stype) == "StructureType<>" assert not stype.components - stype.add("flag", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None) + stype.add("flaG", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None) + # Lookup is not case sensitive flag = stype.lookup("flag") + # But we retain information on the original capitalisation + assert flag.name == "flaG" assert not flag.initial_value assert isinstance(flag, StructureType.ComponentType) stype.add("flag2", INTEGER_TYPE, Symbol.Visibility.PUBLIC, diff --git a/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py b/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py index 8216017652..e569d01b39 100644 --- a/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py @@ -438,19 +438,33 @@ def test_no_assumed_size_char_in_kernels(fortran_reader): sub = psyir.walk(Routine)[0] acc_trans = ACCKernelsTrans() with pytest.raises(TransformationError) as err: - acc_trans.validate(sub.children[0], options={}) + acc_trans.validate(sub.children[0]) assert ("Assumed-size character variables cannot be enclosed in an " "OpenACC region but found 'if (assumed_size_char == 'literal')" in str(err.value)) with pytest.raises(TransformationError) as err: - acc_trans.validate(sub.children[1], options={}) + acc_trans.validate(sub.children[1], options={"allow_string": True}) assert ("Assumed-size character variables cannot be enclosed in an OpenACC" " region but found 'assumed_size_char(:LEN(explicit_size_char)) = " in str(err.value)) with pytest.raises(TransformationError) as err: - acc_trans.validate(sub.children[2], options={}) + acc_trans.validate(sub.children[2], options={"allow_string": True}) assert ("Cannot include 'ACHAR(9)' in an OpenACC region because " "it is not available on GPU" in str(err.value)) + # Check that the character assignment is excluded by default. + with pytest.raises(TransformationError) as err: + acc_trans.validate(sub.children[2]) + assert ("ACCKernelsTrans does not permit assignments involving character " + "variables by default (use the 'allow_string' option to include " + "them), but found:" in str(err.value)) + # Check the verbose option. + with pytest.raises(TransformationError) as err: + acc_trans.validate(sub.children[2], options={"verbose": True}) + assert (sub.children[2].preceding_comment == + "ACCKernelsTrans does not permit assignments involving character " + "variables by default (use the 'allow_string' option to include " + "them)") + # String with explicit length is fine. acc_trans.validate(sub.children[3], options={}) # CHARACTER*(*) notation is also rejected.