(#Towards 2302) intrinsic argument names

changelog

@@ -1,3 +1,6 @@
+   5) PR #3150 towards #2302. Adds functionality to map arguments to
+   Fortran intrinsics to the corresponding named arguments.   
+
    4) PR #3178 for #3196. Improves fparser2 reader directive handling.
 
    3) PR #3210 for #3203. Removes outstanding references to the old,

doc/developer_guide/psyir.rst

@@ -594,6 +594,18 @@ available PSyIR `IntrinsicCall` match those of the `Fortran 2018 standard
 In addition to Fortran Intrinsics, special Fortran statements such as:
 `ALLOCATE`, `DEALLOCATE` and `NULLIFY` are also PSyIR IntrinsicCalls.
 
+``IntrinsicCall`` nodes have a ``compute_argument_names`` function, that is used
+within PSyclone during their creation (via the ``IntrinsicCall.create``
+function). This attempts to match the Intrinsic and input arguments to
+one of the interfaces for the intrinsic (as some intrinsics have multiple
+possible argument interfaces). If successful, PSyclone
+will convert all of the arguments to be named arguments. If argument name computation
+fails, then PSyclone will create a ``CodeBlock``. This computation is
+required to guarantee correct behaviour when computing reference_accesses
+or the return type of an Intrinsic. The computation of argument names may change
+the output of the intrinsic - optional arguments will always have their argument
+names displayed in the Fortran output, whilst required argument names are
+not generated by default.
 
 IntrinsicCalls, like Calls, have properties to inform if the call is to a
 pure, elemental, inquiry (does not touch the first argument data) function

doc/developer_guide/transformations.rst

@@ -557,7 +557,7 @@ the ``ParallelLoopTrans`` class for reference):
    built upon the ``apply`` definition (e.g. ``LoopTrans`` has
    validation used for subclasses, but performs no actions in its newly added
    ``apply`` method).
-3. The ``validate`` method should call the ``validate_options`` method on each of
+3. The ``validate`` method should call the ``validate_options`` method on
    the keyword arguments and ``**kwargs``. This method should not be called on
    the ``options`` dictionary. The ``options`` input should overrule the keyword
    arguments when determining options to the apply and validate method.

doc/user_guide/psyclone_command.rst

@@ -50,16 +50,18 @@ by the command:
 
 .. parsed-literal::
 
+
   > psyclone -h
     usage: psyclone [-h] [-v] [-c CONFIG] [-s SCRIPT] [--enable-cache] [-l {off,all,output}]
                     [-p {invokes,routines,kernels}]
-                    [--backend {disable-validation,disable-indentation}] [-o OUTPUT_FILE]
-                    [-api DSL] [-oalg OUTPUT_ALGORITHM_FILE] [-opsy OUTPUT_PSY_FILE]
+                    [-o OUTPUT_FILE] [-api DSL] [-oalg OUTPUT_ALGORITHM_FILE] [-opsy OUTPUT_PSY_FILE]
                     [-okern OUTPUT_KERNEL_PATH] [-dm] [-nodm]
                     [--kernel-renaming {multiple,single}]
                     [--log-level {OFF,DEBUG,INFO,WARNING,ERROR,CRITICAL}] [--log-file LOG_FILE]
                     [--keep-comments] [--keep-directives] [-I INCLUDE] [-d DIRECTORY]
                     [--modman-file-ignore IGNORE_PATTERN] [--free-form | --fixed-form]
+                    [--backend-disable-validation] [--backend-disable-indentation]
+                    [--backend-add-all-intrinsic-arg-names]
                     filename
 
     Transform a file using the PSyclone source-to-source Fortran compiler
@@ -83,11 +85,6 @@ by the command:
                             to apply line-length limit to output Fortran only.
       -p {invokes,routines,kernels}, --profile {invokes,routines,kernels}
                             add profiling hooks for 'kernels', 'invokes' or 'routines'
-      --backend {disable-validation,disable-indentation}
-                            options to control the PSyIR backend used for code generation. Use
-                            'disable-validation' to disable the validation checks that are
-                            performed by default. Use 'disable-indentation' to turn off all
-                            indentation in the generated code.
       -o OUTPUT_FILE        (code-transformation mode) output file
       -api DSL, --psykal-dsl DSL
                             whether to use a PSyKAl DSL (one of ['lfric', 'gocean'])
@@ -123,6 +120,18 @@ by the command:
       --modman-file-ignore IGNORE_PATTERN
                             Ignore files that contain the specified pattern.
 
+    Fortran backend control options.:
+        These settings control how PSyclone outputs Fortran.
+
+        --backend-disable-validation
+                        Disables validation checks that PSyclone backends perform by default.
+        --backend-disable-indentation
+                        Disables all indentation in the generated output code.
+        --backend-add-all-intrinsic-arg-names
+                        By default, the backend outputs the names of only optional arguments to intrinsic calls.
+                        This option enables all argument names on intrinsic
+                        calls, i.e. SUM(array=arr, mask=maskarr) instead of SUM(arr, mask=maskarr).
+
 Basic Use
 ---------
 
@@ -317,6 +326,16 @@ The default behaviour may be changed by adding the
 :ref:`configuration file <config-default-section>`. Note that any
 command-line setting always takes precedence.
 
+Overriding Fortran Intrinsics
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+PSyclone internally computes the argument names for each argument provided to
+a Fortran IntrinsicCall PSyIR node. This can
+cause problems with code that overrides Fortran intrinsics. To ensure correct
+behaviour of the output, the ``--backend-add-all-intrinsic-arg-names`` option
+must **not** be passed to PSyclone, else the resultant code may not compile or
+run correctly.
+
 Automatic Profiling Instrumentation
 -----------------------------------
 

src/psyclone/configuration.py

@@ -235,6 +235,10 @@ def __init__(self):
         # The Fortran standard that fparser should use
         self._fortran_standard = None
 
+        # By default, the PSyIR backends don't output argument names on (most)
+        # IntrinsicCalls. This option controls that behaviour.
+        self._backend_intrinsic_named_kwargs = False
+
     # -------------------------------------------------------------------------
     def load(self, config_file=None):
         '''Loads a configuration file.
@@ -771,6 +775,28 @@ def get_constants(self):
         '''
         return self.api_conf().get_constants()
 
+    @property
+    def backend_intrinsic_named_kwargs(self) -> bool:
+        '''
+        :returns: whether the output of intrinsic named arguments is
+                  enabled for required intrinsic arguments.
+        '''
+        return self._backend_intrinsic_named_kwargs
+
+    @backend_intrinsic_named_kwargs.setter
+    def backend_intrinsic_named_kwargs(self, output_kwargs: bool) -> None:
+        '''
+        Setter for whether the backend should output required argument names
+        on IntrinsicCalls.
+
+        :param output_kwargs: whether to output required argument names.
+        '''
+        if not isinstance(output_kwargs, bool):
+            raise TypeError(f"backend_intrinsic_named_kwargs must be a bool "
+                            f"but found '{type(output_kwargs).__name__}'.")
+
+        self._backend_intrinsic_named_kwargs = output_kwargs
+
 
 # =============================================================================
 class BaseConfig:

src/psyclone/generator.py

@@ -493,13 +493,6 @@ def main(arguments):
     parser.add_argument(
         '-p', '--profile', action="append", choices=Profiler.SUPPORTED_OPTIONS,
         help="add profiling hooks for 'kernels', 'invokes' or 'routines'")
-    parser.add_argument(
-        '--backend', dest='backend', action="append",
-        choices=['disable-validation', 'disable-indentation'],
-        help=("options to control the PSyIR backend used for code generation. "
-              "Use 'disable-validation' to disable the validation checks that "
-              "are performed by default. Use 'disable-indentation' to turn off"
-              " all indentation in the generated code."))
 
     # Code-transformation mode flags
     parser.add_argument('-o', metavar='OUTPUT_FILE',
@@ -580,6 +573,31 @@ def main(arguments):
              "(default is to look at the input file extension)."
     )
 
+    backend_group = parser.add_argument_group(
+            "Fortran backend control options.",
+            "These settings control how PSyclone outputs Fortran. "
+    )
+    backend_group.add_argument(
+        "--backend-disable-validation", default=argparse.SUPPRESS,
+        action="store_true",
+        help=("Disables validation checks that PSyclone backends perform by "
+              "default.")
+    )
+    backend_group.add_argument(
+        "--backend-disable-indentation", default=argparse.SUPPRESS,
+        action="store_true",
+        help="Disables all indentation in the generated output code."
+    )
+    backend_group.add_argument(
+        "--backend-add-all-intrinsic-arg-names",
+        default=argparse.SUPPRESS,
+        action="store_true",
+        help="By default, the backend outputs the names of only optional "
+             "arguments to intrinsic calls. This option enables all argument "
+             "names on intrinsic calls, i.e. SUM(array=arr, mask=maskarr) "
+             "instead of SUM(arr, mask=maskarr)."
+    )
+
     args = parser.parse_args(arguments)
 
     # Set the logging system up.
@@ -631,20 +649,25 @@ def main(arguments):
         api = args.psykal_dsl
     Config.get().api = api
 
+    # Record any intrinsic output format settings.
+    if "backend_add_all_intrinsic_arg_names" in args:
+        # Tells the backend to attempt to add names to required
+        # arguments to Fortran intrinsics.
+        Config.get().backend_intrinsic_named_kwargs = True
+    # A command-line flag overrides the setting in the Config file (if
+    # any).
+    if "backend_disable_validation" in args:
+        Config.get().backend_checks_enabled = False
+    if "backend_disable_indentation" in args:
+        Config.get().backend_indentation_disabled = True
+
     # Record any profiling options.
     if args.profile:
         try:
             Profiler.set_options(args.profile, api)
         except ValueError as err:
             print(f"Invalid profiling option: {err}", file=sys.stderr)
             sys.exit(1)
-    if args.backend:
-        # A command-line flag overrides the setting in the Config file (if
-        # any).
-        if "disable-validation" in args.backend:
-            Config.get().backend_checks_enabled = False
-        if "disable-indentation" in args.backend:
-            Config.get().backend_indentation_disabled = True
 
     # The Configuration manager checks that the supplied path(s) is/are
     # valid so protect with a try

src/psyclone/psyir/backend/fortran.py

@@ -40,6 +40,7 @@
 from a PSyIR tree. '''
 
 # pylint: disable=too-many-lines
+from psyclone.configuration import Config
 from psyclone.errors import InternalError
 from psyclone.psyir.backend.language_writer import LanguageWriter
 from psyclone.psyir.backend.visitor import VisitorError
@@ -1739,24 +1740,77 @@ def _gen_arguments(self, node):
                 result_list.append(self._visit(child))
         return ", ".join(result_list)
 
-    def call_node(self, node) -> str:
-        '''Translate the PSyIR call node to Fortran.
+    def intrinsiccall_node(self, node: IntrinsicCall) -> str:
+        '''Translate the PSyIR IntrinsicCall node to Fortran.
 
-        :param node: a Call PSyIR node.
-        :type node: :py:class:`psyclone.psyir.nodes.Call`
+        :param node: an IntrinsicCall PSyIR node.
 
         :returns: the equivalent Fortran code.
 
         '''
-        args = self._gen_arguments(node)
-        if isinstance(node, IntrinsicCall) and node.routine.name not in [
+        # Check the config to determine if we're outputting all argument
+        # names.
+        if not Config.get().backend_intrinsic_named_kwargs:
+            # Config says to avoid outputting argument names where
+            # possible.
+            try:
+                # Argument name computation handles any error checking we
+                # might otherwise want to try. Most IntrinsicCalls should
+                # already have argument names added, but we do it here to
+                # ensure that argument name computation is possible.
+                node.compute_argument_names()
+                intrinsic_interface = node._find_matching_interface()
+                args = []
+                correct_names = True
+                for idx, arg_name in enumerate(node.argument_names):
+                    if idx < len(intrinsic_interface) and correct_names:
+                        # This is a potential required argument.
+                        if arg_name == intrinsic_interface[idx]:
+                            args.append(self._visit(node.arguments[idx]))
+                            continue
+                        # Otherwise it didn't match, so we can't remove any
+                        # more argument names, and fall back to the default
+                        # behaviour from here.
+                        correct_names = False
+                    # Otherwise, use the default behaviour.
+                    if node.argument_names[idx]:
+                        args.append(
+                             f"{node.argument_names[idx]}="
+                             f"{self._visit(node.arguments[idx])}"
+                        )
+                    else:
+                        args.append(f"{self._visit(node.arguments[idx])}")
+                args = ", ".join(args)
+            except NotImplementedError:
+                # If the Intrinsic fails to have argument names added, or to
+                # match to an interface, then use the default behaviour.
+                args = self._gen_arguments(node)
+        else:
+            args = self._gen_arguments(node)
+
+        if node.routine.name not in [
                 "DATE_AND_TIME", "SYSTEM_CLOCK", "MVBITS", "RANDOM_NUMBER",
                 "RANDOM_SEED"]:
             # Most intrinsics are functions and so don't have 'call'.
             if not node.parent or isinstance(node.parent, Schedule):
                 return f"{self._nindent}{node.routine.name}({args})\n"
             return f"{node.routine.name}({args})"
 
+        # Otherwise we have one of the intrinsics that requires "Call X"
+        # syntax.
+        return f"{self._nindent}call {self._visit(node.routine)}({args})\n"
+
+    def call_node(self, node: Call) -> str:
+        '''Translate the PSyIR call node to Fortran.
+
+        :param node: a Call PSyIR node.
+        :type node: :py:class:`psyclone.psyir.nodes.Call`
+
+        :returns: the equivalent Fortran code.
+
+        '''
+        args = self._gen_arguments(node)
+
         if not node.parent or isinstance(node.parent, Schedule):
             return f"{self._nindent}call {self._visit(node.routine)}({args})\n"
 

src/psyclone/psyir/backend/sympy_writer.py

@@ -46,13 +46,14 @@
 
 from psyclone.core import (Signature, AccessSequence,
                            VariablesAccessMap)
+from psyclone.errors import GenerationError
 from psyclone.psyir.backend.fortran import FortranWriter
 from psyclone.psyir.backend.visitor import VisitorError
 from psyclone.psyir.frontend.sympy_reader import SymPyReader
 from psyclone.psyir.nodes import (
     ArrayOfStructuresReference, ArrayReference, BinaryOperation, Call,
     DataNode, IntrinsicCall, Literal, Node,
-    Range, Reference, StructureReference)
+    Range, Reference, StructureReference, Schedule)
 from psyclone.psyir.symbols import (
     ArrayType, DataSymbol, RoutineSymbol, ScalarType, Symbol,
     SymbolError, SymbolTable, UnresolvedType)
@@ -759,20 +760,26 @@ def intrinsiccall_node(self, node: IntrinsicCall) -> str:
         :returns: the SymPy representation for the Intrinsic.
 
         '''
+        # Add argument names to the intrinsic
+        try:
+            node.compute_argument_names()
+        except (GenerationError, NotImplementedError) as err:
+            raise VisitorError(
+                f"Sympy handler can't handle an IntrinsicCall that "
+                f"can't have argument names automatically added. Use "
+                f"explicit argument names instead. "
+                f"Failing node was "
+                f"'{node.debug_string()}'.") from err
+
         # Sympy does not support argument names, remove them for now
         if any(node.argument_names):
-            # TODO #2302: This is not totally right without canonical intrinsic
-            # positions for arguments. One alternative is to refuse it with:
-            # raise VisitorError(
-            #     f"Named arguments are not supported by SymPy but found: "
-            #     f"'{node.debug_string()}'.")
-            # but this leaves sympy comparisons almost always giving false when
-            # out of order arguments are rare, so instead we ignore it for now.
-
             # It makes a copy (of the parent because if matters to the call
             # visitor) because we don't want to delete the original arg names
-            parent = node.parent.copy()
-            node = parent.children[node.position]
+            if node.parent:
+                parent = node.parent.copy()
+                node = parent.children[node.position]
+            else:
+                node = node.copy()
             for idx in range(len(node.argument_names)):
                 # pylint: disable=protected-access
                 node._argument_names[idx] = (node._argument_names[idx][0],
@@ -782,7 +789,21 @@ def intrinsiccall_node(self, node: IntrinsicCall) -> str:
             args = self._gen_arguments(node)
             return f"{self._nindent}{name}({args})"
         except KeyError:
-            return super().call_node(node)
+            # This section is copied from FortranWriter IntrinsicCall,
+            # but doesn't attempt to match argument names and so avoids
+            # re-adding optional argument names back in.
+            args = self._gen_arguments(node)
+            # These routines require `call` syntax in Fortran.
+            if node.routine.name not in [
+                    "DATE_AND_TIME", "SYSTEM_CLOCK", "MVBITS",
+                    "RANDOM_NUMBER", "RANDOM_SEED"]:
+                # Most intrinsics are functions and so don't have 'call'.
+                if not node.parent or isinstance(node.parent, Schedule):
+                    return f"{self._nindent}{node.routine.name}({args})\n"
+                return f"{node.routine.name}({args})"
+            # Otherwise we have an intrinsic that has call syntax.
+            return (f"{self._nindent}call "
+                    f"{self._visit(node.routine)}({args})\n")
 
     # -------------------------------------------------------------------------
     def reference_node(self, node: Reference) -> str: