Merge pull request #2857 from stfc/2854_acckernels_no_char

(Closes #2854) exclude char assignments from ACC KERNELS regions

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.

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:

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

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():

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']

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
 

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,

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.