diff --git a/.github/workflows/compilation.yml b/.github/workflows/compilation.yml index a1e032a797..d90b08417c 100644 --- a/.github/workflows/compilation.yml +++ b/.github/workflows/compilation.yml @@ -51,14 +51,14 @@ on: push env: - CUDA_VERSION: 12.6.0 + CUDA_VERSION: 12.6.2 GFORTRAN_VERSION: 14.2.0 - HDF5_VERSION: 1.14.4.3 + HDF5_VERSION: 1.14.5 NETCDF_C_VERSION: 4.9.2 NETCDF_FORTRAN_VERSION: 4.6.1 - NVFORTRAN_VERSION: 24.7 + NVFORTRAN_VERSION: 24.9 OPENMPI_VERSION: 5.0.5 - PYTHON_VERSION: 3.12.5 + PYTHON_VERSION: 3.13.0 jobs: run_if_on_mirror: diff --git a/.github/workflows/lfric_test.yml b/.github/workflows/lfric_test.yml index 23d668452e..8d336e2ead 100644 --- a/.github/workflows/lfric_test.yml +++ b/.github/workflows/lfric_test.yml @@ -47,7 +47,7 @@ jobs: runs-on: self-hosted env: LFRIC_APPS_REV: 3269 - PYTHON_VERSION: 3.12.5 + PYTHON_VERSION: 3.13.0 steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/nemo_tests.yml b/.github/workflows/nemo_tests.yml index bbcd2bf468..7dcf064239 100644 --- a/.github/workflows/nemo_tests.yml +++ b/.github/workflows/nemo_tests.yml @@ -46,13 +46,13 @@ jobs: if: ${{ github.repository == 'stfc/PSyclone-mirror' }} runs-on: self-hosted env: - HDF5_VERSION: 1.14.4.3 + HDF5_VERSION: 1.14.5 NETCDF_C_VERSION: 4.9.2 NETCDF_FORTRAN_VERSION: 4.6.1 NVFORTRAN_VERSION: 23.7 ONEAPI_VERSION: 2024.2.1 PERL_VERSION: 5.40.0 - PYTHON_VERSION: 3.12.5 + PYTHON_VERSION: 3.13.0 steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/python-package.yml b/.github/workflows/python-package.yml index eec02a9151..f4e85492b9 100644 --- a/.github/workflows/python-package.yml +++ b/.github/workflows/python-package.yml @@ -62,7 +62,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: - python-version: '3.12' + python-version: '3.13' - run: sudo apt-get install -y graphviz doxygen - run: python -m pip install --upgrade pip - run: pip install .[doc] @@ -83,7 +83,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: - python-version: '3.12' + python-version: '3.13' - run: python -m pip install --upgrade pip - run: pip install .[doc] # Sphinx since version 7.2 (7.2.0/1/2) aborts with @@ -99,7 +99,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - python-version: [3.7, 3.8, 3.12] + python-version: [3.7, 3.8, 3.13] steps: - uses: actions/checkout@v4 with: diff --git a/changelog b/changelog index 4348afb53e..335e026d04 100644 --- a/changelog +++ b/changelog @@ -243,6 +243,22 @@ 83) PR #2743 for #2742. Ensure generated code for function spaces in LFRic is always in a consistent order. + 84) PR #2746. Update minor versions of CUDA, HDF5, nvfortran and + Python used in the CI. + + 85) PR #2748. Revert Python used on RTD to 3.12 as 3.13 unsupported. + + 86) PR #2707 for #257. Fixes bugs in the loop-fusion transformation. + + 87) PR #2752 towards #2717. Adds Assignment.is_literal_assignment property. + + 88) PR #2753 towards #2717. Adds DataNode.is_character method. + + 89) PR #2712 for #2711. Adds support for user-supplied Kernels that + operate on dofs (in the LFRic API). + + 99) PR #2685 for #2027. Improves datatype and shape inference. + release 2.5.0 14th of February 2024 1) PR #2199 for #2189. Fix bugs with missing maps in enter data diff --git a/doc/developer_guide/APIs.rst b/doc/developer_guide/APIs.rst index f74cf6f7f8..ea3f1ca774 100644 --- a/doc/developer_guide/APIs.rst +++ b/doc/developer_guide/APIs.rst @@ -48,29 +48,29 @@ TBD .. Generating API-specific code .. ============================ -.. +.. .. This section explains how to create a new API in PSyclone. PSyclone .. currently supports the following APIs: lfric and gocean. -.. +.. .. config.py .. --------- -.. +.. .. The names of the supported APIs and the default API are specified in .. ``configuration.py``. When adding a new API you must add the name you would like .. to use to the ``_supported_api_list``. -.. +.. .. parse.py .. -------- -.. +.. .. The parser reads the algorithm code and associated kernel metadata. -.. +.. .. The parser currently assumes that all APIs will use the ``invoke()`` .. API for the algorithm-to-psy layer but that the content and structure .. of the metadata in the kernel code may differ. If the algorithm API .. differs, then the parser will need to be refactored. This is beyond .. the scope of this document and is currently not considered in the .. PSyclone software architecture. -.. +.. .. The kernel metadata however, will be different from one API to .. another. To parse this kernel-API-specific metadata a .. ``KernelTypeFactory`` is provided which should return the appropriate @@ -79,7 +79,7 @@ TBD .. in the ``KernelTypeFactory`` class. If the kernel metadata happens to be .. the same as another existing API then the existing ``KernelType`` .. subclass can be used for the new API. -.. +.. .. The ``KernelType`` subclass needs to specialise the class constructor .. and initialise the ``KernelType`` base class with the .. supplied arguments. The role of the ``KernelType`` subclass is to create @@ -88,95 +88,95 @@ TBD .. this is appends the kernel-metadata-specific subclass instance is .. appended to the ``_arg_descriptors`` list provided by the ``KernelType`` .. base class. -.. +.. .. TBC -.. +.. .. This information -.. +.. .. KernelType base class assumes kernel metadata stored as a type. Searches for that type. .. Checks whether the metadata is public (it should be ?) .. Assumes iterates_over variable. .. Binding to a procedure - assumes one of two styles. .. Assumes a meta_args type .. *What about our func_args type???* -.. +.. .. type x .. meta_args= .. *meta_func=* .. iterates_over= .. code => or code = .. end type x -.. +.. .. The descriptor class ... -.. +.. .. psyGen.py .. --------- -.. +.. .. factory .. +++++++ -.. +.. .. A new file needs to be created and the following classes found in .. psyGen.py need to be subclassed. -.. +.. .. PSy, Invokes, Invoke, InvokeSchedule, Loop, Kern, Arguments, Argument .. You may also choose to subclass the Inf class if required. -.. +.. .. The subclass of the PSy class then needs to be added as an option to .. the create method in the PSyFactory class. -.. +.. .. Initialisation .. ++++++++++++++ -.. +.. .. The parser information passed to the PSy layer is used to create an .. invokes object which in turn creates a list of invoke objects. Each .. invoke object contains an InvokeSchedule which consists of loops and .. calls. Finally, a call contains an arguments object which itself .. contains a list of argument objects. -.. +.. .. To make sure the subclass versions of the above objects are created .. the __init__ methods of the subclasses must make sure they create .. the appropriate objects. -.. +.. .. Some of the baseclass constructors (__init__ methods) support the .. classname being provided. This allow them to instantiate the .. appropriate objects without knowing what they are. -.. +.. .. gen_code() .. ++++++++++ -.. +.. .. All of the above classes (with the exception of PSy which supports a .. gen() method) have the gen_code() method. This method passes the .. parent of the generation tree and expect the object to add the code .. associated with the object as a child of the parent. The object is .. then expected to call any children. This approach is powerful as it .. lets each object concentrate on the code that it is responsible for. -.. +.. .. Adding code in gen_code() .. +++++++++++++++++++++++++ -.. +.. .. The f2pygen classes have been developed to help create appropriate .. fortran code in the gen_code() method. -.. +.. .. When writing a gen_code() method for a particular object and API it is .. natural to add code as a child of the parent provided by the callee of .. the method. However, in some cases we do not want code to appear at .. the current position in the hierarchy. -.. +.. .. The add() method .. ++++++++++++++++ -.. +.. .. PSyclone supports this via the add() method -.. +.. .. explicitly place at the appropriate place in the hierarchy. For example, .. parent.parent.add(...) -.. +.. .. optional argument. default is auto. This attempts to place code in the .. expected place. For example, specify a declaration. auto finds a .. correct place to put this code. -.. +.. .. Specify position explicitly .. "before", "after", "first", "last" -.. +.. .. Sometimes don't know exactly where to place. On example that is .. supported is when you want to add something before or after a loop .. nest. start_parent_loop(). This method recurses up until the parent is @@ -333,7 +333,7 @@ grey) then we get: .. image:: dofs_cont_halos.png :width: 230 - + An example for a depth-1 halo implementation with the earlier mesh split into 2 partitions is given below, with the halo cells drawn in grey and halo dofs coloured red. An example local indexing scheme is @@ -403,9 +403,9 @@ Loop iterators -------------- In the current implementation of the LFRic API it is possible to -iterate (loop) either over cells or dofs. At the moment all coded -kernels are written to iterate over cells and all Built-in kernels are -written to iterate over dofs, but that does not have to be the case. +iterate (loop) either over cells or dofs. At the moment coded +kernels can be written to iterate over cells or dofs and all Built-in kernels +are written to iterate over dofs, but that does not have to be the case. The loop iteration information is specified in the kernel metadata. In the case of Built-ins there is kernel metadata but it is part of @@ -994,7 +994,7 @@ If an application is being built in parallel then it is possible that different invocations of PSyclone will happen simultaneously and therefore we must take care to avoid race conditions when querying the filesystem. For this reason we use ``os.open``:: - + fd = os.open(, os.O_CREAT | os.O_WRONLY | os.O_EXCL) The ``os.O_CREATE`` and ``os.O_EXCL`` flags in combination mean that @@ -1020,7 +1020,7 @@ of a given colour may be safely updated in parallel Example of the colouring of the horizontal cells used to ensure the thread-safe update of shared dofs (black circles). (Courtesy of S. Mullerworth, Met Office.) - + The loop over colours must then be performed sequentially but the loop over cells of a given colour may be done in parallel. A loop that requires colouring may be transformed using the ``Dynamo0p3ColourTrans`` @@ -1206,13 +1206,13 @@ TBD .. OpenMP Support .. -------------- -.. +.. .. Loop directives are treated as first class entities in the psyGen .. package. Therefore they can be added to psyGen's high level .. representation of the fortran code structure in the same way as calls .. and loops. Obviously it is only valid to add a loop directive outside .. of a loop. -.. +.. .. When adding a call inside a loop the placement of any additional calls .. or declarations must be specified correctly to ensure that they are .. placed at the correct location in the hierarchy. To avoid accidentally @@ -1222,7 +1222,7 @@ TBD .. f2pygen*. This method returns the location at the top of any loop .. hierarchy and before any comments immediately before the top level .. loop. -.. +.. .. The OpenMPLoopDirective object needs to know which variables are .. shared and which are private. In the current implementation default .. shared is used and private variables are listed. To determine the @@ -1233,13 +1233,13 @@ TBD .. the directive and adds each calls list of private variables, returned .. with the local_vars() method. Therefore the OpenMPLoopDirective object .. relies on calls specifying which variables they require being local. -.. +.. .. Next ... -.. +.. .. Update transformation for colours -.. -.. OpenMPLoop transformation in transformations.py. -.. +.. +.. OpenMPLoop transformation in transformations.py. +.. .. Create third transformation which goes over all loops in a schedule and .. applies the OpenMP loop transformation. diff --git a/doc/developer_guide/working_practises.rst b/doc/developer_guide/working_practises.rst index 12403d2fc0..a7d1b78134 100644 --- a/doc/developer_guide/working_practises.rst +++ b/doc/developer_guide/working_practises.rst @@ -448,13 +448,13 @@ computational cost (so that we 'fail fast'): 3. All links within the Sphinx documentation (rst files) are checked (see note below); - 4. All of the examples are tested (for Python versions 3.7, 3.8 and 3.12) + 4. All of the examples are tested (for Python versions 3.7, 3.8 and 3.13) using the ``Makefile`` in the ``examples`` directory. No compilation is performed; only the ``transform`` (performs the PSyclone transformations) and ``notebook`` (runs the various Jupyter notebooks) targets are used. The ``transform`` target is run 2-way parallel (``-j 2``). - 5. The full test suite is run for Python versions 3.7, 3.8 and 3.12 but + 5. The full test suite is run for Python versions 3.7, 3.8 and 3.13 but without the compilation checks. ``pytest`` is passed the ``-n auto`` flag so that it will run the tests in parallel on as many cores as are available (currently 2 on GHA instances). diff --git a/doc/user_guide/dynamo0p3.rst b/doc/user_guide/dynamo0p3.rst index 29f00f6ad1..84e9f5d0df 100644 --- a/doc/user_guide/dynamo0p3.rst +++ b/doc/user_guide/dynamo0p3.rst @@ -796,9 +796,6 @@ support for i-first kernels point the looping (and associated parallelisation) will be put back into the PSy layer. -.. note:: Support for DoF kernels have not yet been implemented in PSyclone - (see PSyclone issue #1351 for progress). - .. _lfric-user-kernel-rules: Rules for all User-Supplied Kernels that Operate on Cell-Columns @@ -981,9 +978,6 @@ on a ``CELL_COLUMN`` without CMA Operators. Specifically: Rules for all User-Supplied Kernels that Operate on DoFs (DoF Kernels) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ -.. note:: Support for DoF kernels have not yet been implemented in PSyclone - (see PSyclone issue #1351 for progress). - Kernels that have ``operates_on = DOF`` and :ref:`LFRic Built-ins` overlap significantly in their scope, and the conventions that DoF Kernels must follow are influenced @@ -1015,8 +1009,9 @@ The list of rules for DoF Kernels is as follows: to do this.) Any scalar arguments must therefore be declared in the metadata as `GH_READ` - see :ref:`below` -6) Kernels must be written to operate on a single DoF, such that single DoFs - can be provided to the Kernel within a loop over the DoFs of a field. +6) Kernels must be written to operate on a single DoF, such that field values + at the same dof location/index can be provided to the Kernel within a loop + over the DoFs of the function space of the field that is being updated. .. _lfric-api-kernel-metadata: @@ -1889,16 +1884,14 @@ operates_on The fourth type of metadata provided is ``OPERATES_ON``. This specifies that the Kernel has been written with the assumption that it is supplied with the specified data for each field/operator argument. -For user-supplied kernels this is currently only permitted to be -``CELL_COLUMN`` or ``DOMAIN``. The possible values for ``OPERATES_ON`` -and their interpretation are summarised in the following table: +The possible values for ``OPERATES_ON`` and their interpretation are +summarised in the following table: =========== ========================================================= operates_on Data passed for each field/operator argument =========== ========================================================= cell_column Single column of cells -dof Single DoF (currently :ref:`built-ins` only, but see PSyclone - issue #1351) +dof Single DoF domain All columns of cells =========== ========================================================= @@ -2477,9 +2470,6 @@ as the second argument to the kernel (after ``nlayers``). Rules for DoF Kernels ##################### -.. note:: Support for DoF kernels have not yet been implemented in PSyclone - (see PSyclone issue #1351 for progress). - The rules for kernels that have ``operates_on = DOF`` are similar to those for general-purpose kernels but, due to the restriction that only fields and scalars can be passed to them, are much fewer. The full set of rules, along @@ -2504,12 +2494,6 @@ with PSyclone's naming conventions, are: passed in separately. Again, the intent is determined from the metadata (see :ref:`meta_args `). - 3) Include the unique number of degrees of freedom for the function space. - This is an ``integer`` of kind ``i_def`` with intent ``in``. The name of - this argument is simply ``undf`` without a function space suffix (as for - general purpose kernels) since all fields will be on the same function - space. - .. _lfric-kernel-arg-intents: Argument Intents diff --git a/doc/user_guide/getting_going.rst b/doc/user_guide/getting_going.rst index 2729f989b6..5e8e37ffa2 100644 --- a/doc/user_guide/getting_going.rst +++ b/doc/user_guide/getting_going.rst @@ -212,7 +212,7 @@ Dependencies ------------ PSyclone is written in Python so needs Python 3 to be installed on the -target machine. PSyclone is regularly tested with Python 3.7, 3.8 and 3.12 +target machine. PSyclone is regularly tested with Python 3.7, 3.8 and 3.13 but should work with any version >= 3.6. (The last PSyclone release to support Python 2.7 was version 2.1.0.) diff --git a/examples/gocean/eg5/extract/Makefile b/examples/gocean/eg5/extract/Makefile index fe6a5d5976..8685dc3290 100644 --- a/examples/gocean/eg5/extract/Makefile +++ b/examples/gocean/eg5/extract/Makefile @@ -122,13 +122,13 @@ $(NAME): $(INF_LIB) $(EXTRACT_DIR)/$(LIB_NAME) $(KERNELS) alg.o psy.o #TODO #1757: $(INF_LIB) is required because of the meta-data in the # kernel - once this is fixed, $(INF_LIB) can be removed. $(DRIVER_INIT).$(TYPE): $(KERNELS) $(DRIVER_INIT).o - $(F90) $(KERNELS) $(DRIVER_INIT).o -o $(DRIVER_INIT).$(TYPE) \ + $(F90) $(F90FLAGS) $(KERNELS) $(DRIVER_INIT).o -o $(DRIVER_INIT).$(TYPE) \ $(INF_LIB) $(EXTRACT_DIR)/$(LIB_NAME) $(LDFLAGS) #TODO #1757: $(INF_LIB) is required because of the meta-data in the # kernel - once this is fixed, $(INF_LIB) can be removed. $(DRIVER_UPDATE).$(TYPE): $(KERNELS) $(DRIVER_UPDATE).o - $(F90) $(KERNELS) $(DRIVER_UPDATE).o -o $(DRIVER_UPDATE).$(TYPE) \ + $(F90) $(F90FLAGS) $(KERNELS) $(DRIVER_UPDATE).o -o $(DRIVER_UPDATE).$(TYPE) \ $(INF_LIB) $(EXTRACT_DIR)/$(LIB_NAME) $(LDFLAGS) # The dl_esm_inf library diff --git a/examples/nemo/eg5/Makefile b/examples/nemo/eg5/Makefile index 4f2b991761..29029b34aa 100644 --- a/examples/nemo/eg5/Makefile +++ b/examples/nemo/eg5/Makefile @@ -85,7 +85,7 @@ run: compile IT=2 JPI=10 JPJ=10 JPK=5 ./traadv-$(TYPE).exe traadv-$(TYPE).exe: psy.o $(EXTRACT_DIR)/$(LIB_NAME) - $(F90) psy.o -o traadv-$(TYPE).exe $(EXTRACT_DIR)/$(LIB_NAME) $(LDFLAGS) + $(F90) $(F90FLAGS) psy.o -o traadv-$(TYPE).exe $(EXTRACT_DIR)/$(LIB_NAME) $(LDFLAGS) transform: kernels diff --git a/examples/nemo/scripts/omp_cpu_trans.py b/examples/nemo/scripts/omp_cpu_trans.py index aa4258ba69..ceba2eadf6 100755 --- a/examples/nemo/scripts/omp_cpu_trans.py +++ b/examples/nemo/scripts/omp_cpu_trans.py @@ -47,7 +47,6 @@ # List of all files that psyclone will skip processing FILES_TO_SKIP = NOT_PERFORMANT + [ - "lbclnk.f90", # TODO #2685: effective shape bug "asminc.f90", "trosk.f90", "vremap.f90", diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index af1b5be162..39c98d9128 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -37,8 +37,8 @@ from psyclone.domain.common.transformations import KernelModuleInlineTrans from psyclone.psyir.nodes import ( - Loop, Assignment, Directive, Container, Reference, CodeBlock, Call, - Return, IfBlock, Routine, IntrinsicCall) + Assignment, Loop, Directive, Container, Reference, CodeBlock, + Call, Return, IfBlock, Routine, IntrinsicCall) from psyclone.psyir.symbols import ( DataSymbol, INTEGER_TYPE, REAL_TYPE, ArrayType, ScalarType, RoutineSymbol, ImportInterface) @@ -248,7 +248,7 @@ def normalise_loops( if convert_array_notation: # Make sure all array dimensions are explicit - for reference in schedule.walk(Reference, stop_type=Reference): + for reference in schedule.walk(Reference): part_of_the_call = reference.ancestor(Call) if part_of_the_call: if not part_of_the_call.is_elemental: diff --git a/psyclone.pdf b/psyclone.pdf index 5cd0c34142..b20d5a0685 100644 Binary files a/psyclone.pdf and b/psyclone.pdf differ diff --git a/src/psyclone/core/symbolic_maths.py b/src/psyclone/core/symbolic_maths.py index 42408e0fb2..5b227ac675 100644 --- a/src/psyclone/core/symbolic_maths.py +++ b/src/psyclone/core/symbolic_maths.py @@ -82,13 +82,20 @@ def get(): # ------------------------------------------------------------------------- @staticmethod - def equal(exp1, exp2): + def equal(exp1, exp2, identical_variables=None): '''Test if the two PSyIR expressions are symbolically equivalent. + The optional identical_variables dictionary can contain information + about variables which are known to be the same. For example, if + `identical_variables={'i': 'j'}`, then 'i+1' and 'j+1' will be + considered equal. :param exp1: the first expression to be compared. :type exp1: :py:class:`psyclone.psyir.nodes.Node` :param exp2: the second expression to be compared. :type exp2: :py:class:`psyclone.psyir.nodes.Node` + :param identical_variables: which variable names are known to be + identical + :type identical_variables: Optional[dict[str, str]] :returns: whether the two expressions are mathematically \ identical. @@ -99,7 +106,8 @@ def equal(exp1, exp2): if exp1 is None or exp2 is None: return exp1 == exp2 - diff = SymbolicMaths._subtract(exp1, exp2) + diff = SymbolicMaths._subtract(exp1, exp2, + identical_variables=identical_variables) # For ranges all values (start, stop, step) must be equal, meaning # each index of the difference must evaluate to 0: if isinstance(diff, list): @@ -164,17 +172,25 @@ def never_equal(exp1, exp2): # ------------------------------------------------------------------------- @staticmethod - def _subtract(exp1, exp2): + def _subtract(exp1, exp2, identical_variables=None): '''Subtracts two PSyIR expressions and returns the simplified result of this operation. An expression might result in multiple SymPy expressions - for example, a `Range` node becomes a 3-tuple (start, stop, step). In this case, each of the components will be handled individually, and a list will be returned. + The optional identical_variables dictionary can contain information + about variables which are known to be the same. For example, if + `identical_variables={'i': 'j'}`, then 'i+1' and 'j+1' will be + considered equal. + :param exp1: the first expression to be compared. :type exp1: Optional[:py:class:`psyclone.psyir.nodes.Node`] :param exp2: the second expression to be compared. :type exp2: Optional[:py:class:`psyclone.psyir.nodes.Node`] + :param identical_variables: which variable names are known to be + identical + :type identical_variables: Optional[dict[str, str]] :returns: the sympy expression resulting from subtracting exp2 \ from exp1. @@ -188,7 +204,10 @@ def _subtract(exp1, exp2): # Use the SymPyWriter to convert the two expressions to # SymPy expressions: - sympy_expressions = SymPyWriter(exp1, exp2) + writer = SymPyWriter() + sympy_expressions = writer([exp1, exp2], + identical_variables=identical_variables) + # If an expression is a range node, then the corresponding SymPy # expression will be a tuple: if isinstance(sympy_expressions[0], tuple) and \ diff --git a/src/psyclone/domain/lfric/kern_call_acc_arg_list.py b/src/psyclone/domain/lfric/kern_call_acc_arg_list.py index 564e1b70e6..e6a5663911 100644 --- a/src/psyclone/domain/lfric/kern_call_acc_arg_list.py +++ b/src/psyclone/domain/lfric/kern_call_acc_arg_list.py @@ -43,7 +43,7 @@ ''' from psyclone import psyGen -from psyclone.domain.lfric import KernCallArgList +from psyclone.domain.lfric import KernCallArgList, LFRicConstants from psyclone.errors import InternalError @@ -93,6 +93,27 @@ def cell_position(self, var_accesses=None): _, ref = self.cell_ref_name(var_accesses) self.append(ref.symbol.name) + def field(self, arg, var_accesses=None): + '''Add the field array associated with the argument 'arg' to the + argument list. If supplied it also stores this access in var_accesses. + + :param arg: the field to be added. + :type arg: :py:class:`psyclone.dynamo0p3.DynKernelArgument` + :param var_accesses: optional VariablesAccessInfo instance to store + the information about variable accesses. + :type var_accesses: :py:class:`psyclone.core.VariablesAccessInfo` + + ''' + const = LFRicConstants() + suffix = const.ARG_TYPE_SUFFIX_MAPPING[arg.argument_type] + # Look-up the name of the variable that stores the reference to + # the data in this field. + sym = self._symtab.lookup_with_tag(f"{arg.name}:{suffix}") + + # Add the field data array as being read. + self.append(sym.name, var_accesses, var_access_name=sym.name, + mode=arg.access, metadata_posn=arg.metadata_index) + def stencil(self, arg, var_accesses=None): '''Add general stencil information associated with the argument 'arg' to the argument list. OpenACC requires the full dofmap to be diff --git a/src/psyclone/domain/lfric/kern_call_arg_list.py b/src/psyclone/domain/lfric/kern_call_arg_list.py index 40e4efa658..a9039602fd 100644 --- a/src/psyclone/domain/lfric/kern_call_arg_list.py +++ b/src/psyclone/domain/lfric/kern_call_arg_list.py @@ -219,8 +219,7 @@ def mesh_height(self, var_accesses=None): :param var_accesses: optional VariablesAccessInfo instance to store the information about variable accesses. - :type var_accesses: - :py:class:`psyclone.core.VariablesAccessInfo` + :type var_accesses: :py:class:`psyclone.core.VariablesAccessInfo` ''' if self._kern.iterates_over not in ["cell_column", "domain"]: @@ -340,10 +339,9 @@ def field_vector(self, argvect, var_accesses=None): :param argvect: the field vector to add. :type argvect: :py:class:`psyclone.dynamo0p3.DynKernelArgument` - :param var_accesses: optional VariablesAccessInfo instance to store \ + :param var_accesses: optional VariablesAccessInfo instance to store the information about variable accesses. - :type var_accesses: \ - :py:class:`psyclone.core.VariablesAccessInfo` + :type var_accesses: :py:class:`psyclone.core.VariablesAccessInfo` ''' suffix = LFRicConstants().ARG_TYPE_SUFFIX_MAPPING[ @@ -379,11 +377,22 @@ def field(self, arg, var_accesses=None): # Look-up the name of the variable that stores the reference to # the data in this field. sym = self._symtab.lookup_with_tag(f"{arg.name}:{suffix}") - # Add the field data array as being read. - self.append(sym.name, var_accesses, var_access_name=sym.name, - mode=arg.access, metadata_posn=arg.metadata_index) - self.psyir_append(Reference(sym)) + if self._kern.iterates_over == "dof": + # If dof kernel, add access to the field by dof ref + dof_sym = self._symtab.find_or_create_integer_symbol( + "df", tag="dof_loop_idx") + self.append_array_reference(sym.name, [Reference(dof_sym)], + ScalarType.Intrinsic.INTEGER, + symbol=sym) + # Then append our symbol + name = f"{sym.name}({dof_sym.name})" + self.append(name, var_accesses, var_access_name=sym.name) + else: + # Add the field data array as being read. + self.append(sym.name, var_accesses, var_access_name=sym.name, + mode=arg.access, metadata_posn=arg.metadata_index) + self.psyir_append(Reference(sym)) def stencil_unknown_extent(self, arg, var_accesses=None): '''Add stencil information to the argument list associated with the @@ -597,15 +606,19 @@ def fs_compulsory_field(self, function_space, var_accesses=None): '''Add compulsory arguments associated with this function space to the list. If supplied it also stores this access in var_accesses. - :param function_space: the function space for which the compulsory \ + :param function_space: the function space for which the compulsory arguments are added. :type function_space: :py:class:`psyclone.domain.lfric.FunctionSpace` - :param var_accesses: optional VariablesAccessInfo instance to store \ + :param var_accesses: optional VariablesAccessInfo instance to store the information about variable accesses. - :type var_accesses: \ + :type var_accesses: :py:class:`psyclone.core.VariablesAccessInfo` ''' + if self._kern.iterates_over == "dof": + # Dofmaps and `undf` are not required for DoF kernels + return + sym = self.append_integer_reference(function_space.undf_name) self.append(sym.name, var_accesses) diff --git a/src/psyclone/domain/lfric/lfric_kern.py b/src/psyclone/domain/lfric/lfric_kern.py index 74c24dd248..4e16bfe194 100644 --- a/src/psyclone/domain/lfric/lfric_kern.py +++ b/src/psyclone/domain/lfric/lfric_kern.py @@ -574,6 +574,19 @@ def base_name(self): ''' return self._base_name + @property + def undf_name(self): + ''' + Dynamically looks up the name of the 'undf' variable for the + space that this kernel updates. + + :returns: the name of the undf variable. + :rtype: str + + ''' + field = self._arguments.iteration_space_arg() + return field.function_space.undf_name + @property def argument_kinds(self): ''' @@ -599,7 +612,7 @@ def gen_stub(self): const = LFRicConstants() supported_operates_on = const.USER_KERNEL_ITERATION_SPACES[:] # TODO #925 Add support for 'domain' kernels - # TODO #1351 Add suport for 'dof' kernels + # TODO #1351 Add support for 'dof' kernels supported_operates_on.remove("domain") supported_operates_on.remove("dof") diff --git a/src/psyclone/domain/lfric/lfric_kern_call_factory.py b/src/psyclone/domain/lfric/lfric_kern_call_factory.py index 1dfc481593..eec261d546 100644 --- a/src/psyclone/domain/lfric/lfric_kern_call_factory.py +++ b/src/psyclone/domain/lfric/lfric_kern_call_factory.py @@ -70,6 +70,9 @@ def create(call, parent=None): # We still need a loop object though as that is where the logic # for handling halo exchanges is currently implemented. loop_type = "null" + elif call.ktype.iterates_over == "dof": + # Loop over dofs within a field. + loop_type = "dof" else: # Loop over cells, indicated by an empty string. loop_type = "" diff --git a/src/psyclone/domain/lfric/lfric_loop.py b/src/psyclone/domain/lfric/lfric_loop.py index b62fea48d4..793b2df6c8 100644 --- a/src/psyclone/domain/lfric/lfric_loop.py +++ b/src/psyclone/domain/lfric/lfric_loop.py @@ -44,7 +44,6 @@ from psyclone.core import AccessType from psyclone.domain.common.psylayer import PSyLoop from psyclone.domain.lfric import LFRicConstants, LFRicKern -from psyclone.domain.lfric.lfric_builtins import LFRicBuiltIn from psyclone.domain.lfric.lfric_types import LFRicTypes from psyclone.errors import GenerationError, InternalError from psyclone.psyGen import InvokeSchedule, HaloExchange @@ -255,9 +254,8 @@ def load(self, kern): # Loop bounds self.set_lower_bound("start") const = LFRicConstants() - if isinstance(kern, LFRicBuiltIn): - # If the kernel is a built-in/pointwise operation - # then this loop must be over DoFs + if kern.iterates_over == "dof": + # This loop must be over DoFs if Config.get().api_conf("lfric").compute_annexed_dofs \ and Config.get().distributed_memory \ and not kern.is_reduction: diff --git a/src/psyclone/psyir/backend/sympy_writer.py b/src/psyclone/psyir/backend/sympy_writer.py index 03f8fa9fa8..25a6d5aef0 100644 --- a/src/psyclone/psyir/backend/sympy_writer.py +++ b/src/psyclone/psyir/backend/sympy_writer.py @@ -246,7 +246,7 @@ def _create_sympy_array_function(self, name, sig=None, num_dims=None): return new_func # ------------------------------------------------------------------------- - def _create_type_map(self, list_of_expressions): + def _create_type_map(self, list_of_expressions, identical_variables=None): '''This function creates a dictionary mapping each Reference in any of the expressions to either a SymPy Function (if the reference is an array reference) or a Symbol (if the reference is not an @@ -269,6 +269,15 @@ def _create_type_map(self, list_of_expressions): ``lambda_1``). The SymPyWriter (e.g. in ``reference_node``) will use the renamed value when creating the string representation. + The optional identical_variables dictionary can contain information + about variables which are known to be the same. For example, if + `identical_variables={'i': 'j'}`, then 'i+1' and 'j+1' will be + considered equal. + + :param identical_variables: which variable names are known to be + identical + :type identical_variables: Optional[dict[str, str]] + :param list_of_expressions: the list of expressions from which all references are taken and added to a symbol table to avoid renaming any symbols (so that only member names will be renamed). @@ -320,6 +329,16 @@ def _create_type_map(self, list_of_expressions): self._sympy_type_map[unique_sym.name] = \ self._create_sympy_array_function(name) + if not identical_variables: + identical_variables = {} + # For all variables that are the same, set the symbols to be + # identical. This means if e.g. identical_variables={'i': 'j'}, + # the expression i-j becomes j-j = 0 + + for var1, var2 in identical_variables.items(): + if var1 in self._sympy_type_map and var2 in self._sympy_type_map: + self._sympy_type_map[var1] = self._sympy_type_map[var2] + # Now all symbols have been added to the symbol table, create # unique names for the lower- and upper-bounds using special tags: self._lower_bound = \ @@ -358,11 +377,19 @@ def type_map(self): return self._sympy_type_map # ------------------------------------------------------------------------- - def _to_str(self, list_of_expressions): + def _to_str(self, list_of_expressions, identical_variables=None): '''Converts PSyIR expressions to strings. It will replace Fortran- specific expressions with code that can be parsed by SymPy. The argument can either be a single element (in which case a single string is returned) or a list/tuple, in which case a list is returned. + The optional identical_variables dictionary can contain information + about variables which are known to be the same. For example, if + `identical_variables={'i': 'j'}`, then 'i+1' and 'j+1' will be + considered equal. + + :param identical_variables: which variable names are known to be + identical + :type identical_variables: Optional[dict[str, str]] :param list_of_expressions: the list of expressions which are to be converted into SymPy-parsable strings. @@ -379,7 +406,8 @@ def _to_str(self, list_of_expressions): # Create the type map in `self._sympy_type_map`, which is required # when converting these strings to SymPy expressions - self._create_type_map(list_of_expressions) + self._create_type_map(list_of_expressions, + identical_variables=identical_variables) expression_str_list = [] for expr in list_of_expressions: @@ -392,7 +420,7 @@ def _to_str(self, list_of_expressions): return expression_str_list # ------------------------------------------------------------------------- - def __call__(self, list_of_expressions): + def __call__(self, list_of_expressions, identical_variables=None): ''' This function takes a list of PSyIR expressions, and converts them all into Sympy expressions using the SymPy parser. @@ -400,11 +428,18 @@ def __call__(self, list_of_expressions): constants with kind specification, ...), including the renaming of member accesses, as described in https://psyclone-dev.readthedocs.io/en/latest/sympy.html#sympy + The optional identical_variables dictionary can contain information + about variables which are known to be the same. For example, if + `identical_variables={'i': 'j'}`, then 'i+1' and 'j+1' will be + considered equal. :param list_of_expressions: the list of expressions which are to be converted into SymPy-parsable strings. :type list_of_expressions: list of :py:class:`psyclone.psyir.nodes.Node` + :param identical_variables: which variable names are known to be + identical + :type identical_variables: Optional[dict[str, str]] :returns: a 2-tuple consisting of the the converted PSyIR expressions, followed by a dictionary mapping the symbol names @@ -413,12 +448,25 @@ def __call__(self, list_of_expressions): List[:py:class:`sympy.core.basic.Basic`]] :raises VisitorError: if an invalid SymPy expression is found. + :raises TypeError: if the identical_variables parameter is not + a dict, or does contain a key or value that is not a string. ''' + if identical_variables: + if not isinstance(identical_variables, dict): + raise TypeError(f"Expected identical_variables to be " + f"a dictionary, but got " + f"{type(identical_variables)}.") + if any(not isinstance(key, str) or not isinstance(value, str) + for key, value in identical_variables.items()): + raise TypeError("Dictionary identical_variables " + "contains a non-string key or value.") + is_list = isinstance(list_of_expressions, (tuple, list)) if not is_list: list_of_expressions = [list_of_expressions] - expression_str_list = self._to_str(list_of_expressions) + expression_str_list = self._to_str( + list_of_expressions, identical_variables=identical_variables) result = [] for expr in expression_str_list: diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index af421886d7..88fc6cc512 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -1164,6 +1164,7 @@ def _parse_dimensions(self, dimensions, symbol_table): shape = [] # Traverse shape specs in Depth-first-search order for dim in walk(dimensions, (Fortran2003.Assumed_Shape_Spec, + Fortran2003.Deferred_Shape_Spec, Fortran2003.Explicit_Shape_Spec, Fortran2003.Assumed_Size_Spec)): @@ -1194,6 +1195,11 @@ def _parse_dimensions(self, dimensions, symbol_table): else: shape.append(None) + elif isinstance(dim, Fortran2003.Deferred_Shape_Spec): + # Deferred_Shape_Spec has no children (R520). For our purposes + # it is equivalent to Assumed_Shape_Spec(None, None). + shape.append(None) + elif isinstance(dim, Fortran2003.Explicit_Shape_Spec): upper = self._process_array_bound(dim.items[1], symbol_table) @@ -1680,7 +1686,8 @@ def _process_decln(self, scope, symbol_table, decl, visibility_map=None, has_save_attr = False if attr_specs: for attr in attr_specs.items: - if isinstance(attr, Fortran2003.Attr_Spec): + if isinstance(attr, (Fortran2003.Attr_Spec, + Fortran2003.Component_Attr_Spec)): normalized_string = str(attr).lower().replace(' ', '') if normalized_string == "save": if interface is not None: diff --git a/src/psyclone/psyir/nodes/array_mixin.py b/src/psyclone/psyir/nodes/array_mixin.py index 5c992f0638..7acb40380f 100644 --- a/src/psyclone/psyir/nodes/array_mixin.py +++ b/src/psyclone/psyir/nodes/array_mixin.py @@ -610,7 +610,8 @@ def _get_effective_shape(self): :rtype: list[:py:class:`psyclone.psyir.nodes.DataNode`] :raises NotImplementedError: if any of the array-indices involve a - function call or an expression. + function call or an expression or are + of unknown type. ''' shape = [] for idx, idx_expr in enumerate(self.indices): @@ -621,7 +622,7 @@ def _get_effective_shape(self): dtype = idx_expr.datatype if isinstance(dtype, ArrayType): # An array slice can be defined by a 1D slice of another - # array, e.g. `a(b(1:4))`. + # array, e.g. `a(b(1:4))` or `a(b)`. indirect_array_shape = dtype.shape if len(indirect_array_shape) > 1: raise NotImplementedError( @@ -630,8 +631,22 @@ def _get_effective_shape(self): f"used to index into '{self.name}' has " f"{len(indirect_array_shape)} dimensions.") # pylint: disable=protected-access - shape.append(idx_expr._extent(idx)) - + if isinstance(idx_expr, ArrayMixin): + shape.append(idx_expr._extent(idx)) + else: + # We have a Reference (to an array) with no explicit + # indexing. The extent of this is then the SIZE of + # that array. + sizeop = IntrinsicCall.create( + IntrinsicCall.Intrinsic.SIZE, [idx_expr.copy()]) + shape.append(sizeop) + elif isinstance(dtype, (UnsupportedType, UnresolvedType)): + raise NotImplementedError( + f"The array index expression " + f"'{idx_expr.debug_string()}' in access " + f"'{self.debug_string()}' is of '{dtype}' type and " + f"therefore whether it is an array slice (i.e. an " + f"indirect access) cannot be determined.") elif isinstance(idx_expr, (Call, Operation, CodeBlock)): # We can't yet straightforwardly query the type of a function # call or Operation - TODO #1799. diff --git a/src/psyclone/psyir/nodes/assignment.py b/src/psyclone/psyir/nodes/assignment.py index 3aca28e0b7..8095204e0d 100644 --- a/src/psyclone/psyir/nodes/assignment.py +++ b/src/psyclone/psyir/nodes/assignment.py @@ -34,6 +34,7 @@ # Authors R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab # I. Kavcic, Met Office # J. Henrichs, Bureau of Meteorology +# J. G. Wallwork, University of Cambridge # ----------------------------------------------------------------------------- ''' This module contains the Assignment node implementation.''' @@ -41,6 +42,7 @@ from psyclone.core import VariablesAccessInfo from psyclone.errors import InternalError from psyclone.f2pygen import PSyIRGen +from psyclone.psyir.nodes.literal import Literal from psyclone.psyir.nodes.array_reference import ArrayReference from psyclone.psyir.nodes.datanode import DataNode from psyclone.psyir.nodes.intrinsic_call import ( @@ -211,9 +213,9 @@ def reference_accesses(self, var_accesses): @property def is_array_assignment(self): ''' - returns: True if the lhs of the assignment is an array access with at \ + :returns: True if the lhs of the assignment is an array access with at least one of its dimensions being a range and False otherwise. - rtype: bool + :rtype: bool ''' # It's not sufficient simply to check for a Range node as that may be @@ -244,6 +246,16 @@ def is_array_assignment(self): return True return False + @property + def is_literal_assignment(self): + ''' + :returns: True if the rhs of the assignment is a literal value and + False otherwise. + :rtype: bool + + ''' + return isinstance(self.rhs, Literal) + def gen_code(self, parent): '''F2pygen code generation of an Assignment. diff --git a/src/psyclone/psyir/nodes/datanode.py b/src/psyclone/psyir/nodes/datanode.py index 153baade74..083a8e3c73 100644 --- a/src/psyclone/psyir/nodes/datanode.py +++ b/src/psyclone/psyir/nodes/datanode.py @@ -32,12 +32,13 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Authors: A. R. Porter and S. Siso, STFC Daresbury Lab +# Modified J. G. Wallwork, University of Cambridge # ----------------------------------------------------------------------------- ''' This module contains the DataNode abstract node implementation.''' from psyclone.psyir.nodes.node import Node -from psyclone.psyir.symbols.datatypes import UnresolvedType +from psyclone.psyir.symbols.datatypes import ScalarType, UnresolvedType class DataNode(Node): @@ -55,3 +56,27 @@ def datatype(self): :rtype: :py:class:`psyclone.psyir.symbols.UnresolvedType` ''' return UnresolvedType() + + def is_character(self, unknown_as=None): + ''' + :param unknown_as: Determines behaviour in the case where it cannot be + determined whether the DataNode is a character. Defaults to None, + in which case an exception is raised. + :type unknown_as: Optional[bool] + + :returns: True if this DataNode is a character, otherwise False. + :rtype: bool + + :raises ValueError: if the intrinsic type cannot be determined. + + ''' + if not hasattr(self.datatype, "intrinsic"): + if unknown_as is None: + raise ValueError( + "is_character could not resolve whether the expression" + f" '{self.debug_string()}' operates on characters." + ) + return unknown_as + return ( + self.datatype.intrinsic == ScalarType.Intrinsic.CHARACTER + ) diff --git a/src/psyclone/psyir/nodes/structure_reference.py b/src/psyclone/psyir/nodes/structure_reference.py index 89ab37e1a0..a3183445fc 100644 --- a/src/psyclone/psyir/nodes/structure_reference.py +++ b/src/psyclone/psyir/nodes/structure_reference.py @@ -274,10 +274,34 @@ def datatype(self): :returns: the datatype of this reference. :rtype: :py:class:`psyclone.psyir.symbols.DataType` - :raises NotImplementedError: if the structure reference represents \ + :raises NotImplementedError: if the structure reference represents an array of arrays. ''' + def _get_cursor_shape(cursor, cursor_type): + ''' + Utility that returns the shape of the supplied node. + + :param cursor: the node to get the shape of. + :type cursor: :py:class:`psyclone.psyir.nodes.Node` + :param cursor_type: the type of the node. + :type cursor_type: :py:class:`psyclone.psyir.symbols.DataType` + + :returns: the shape of the node or None if it is not an array. + :rtype: Optional[list[:py:class:`psyclone.psyir.nodes.Node`]] + + ''' + if not (isinstance(cursor, ArrayMixin) or + isinstance(cursor_type, ArrayType)): + return None + if isinstance(cursor, ArrayMixin): + # It is an ArrayMixin and has indices so could be a single + # element or a slice. + # pylint: disable=protected-access + return cursor._get_effective_shape() + # No indices so it is an access to a whole array. + return cursor_type.shape + # pylint: disable=too-many-return-statements, too-many-branches if self._overwrite_datatype: return self._overwrite_datatype @@ -301,14 +325,10 @@ def datatype(self): # If the reference has explicit array syntax, we need to consider it # to calculate the resulting shape. - if isinstance(cursor, ArrayMixin): - try: - # pylint: disable=protected-access - shape = cursor._get_effective_shape() - except NotImplementedError: - return UnresolvedType() - else: - shape = [] + try: + shape = _get_cursor_shape(cursor, cursor_type) + except NotImplementedError: + return UnresolvedType() # Walk down the structure, collecting information on any array slices # as we go. @@ -322,46 +342,24 @@ def datatype(self): # 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 - if isinstance(cursor, ArrayMixin): - # pylint: disable=protected-access - try: - shape.extend(cursor._get_effective_shape()) - except NotImplementedError: - return UnresolvedType() - - # We've reached the ultimate member of the structure access. - if shape: - if isinstance(cursor_type, ArrayType): - # It's of array type but does it represent a single element, - # a slice or a whole array? (We use `children` rather than - # `indices` so as to avoid having to check that `cursor` is - # an `ArrayMember`.) - if cursor.children: - # It has indices so could be a single element or a slice. - # pylint: disable=protected-access - cursor_shape = cursor._get_effective_shape() - else: - # No indices so it is an access to a whole array. - cursor_shape = cursor_type.shape - if cursor_shape and len(shape) != len(cursor_shape): - # This ultimate access is an array but we've already - # encountered one or more slices earlier in the access - # expression. + try: + cursor_shape = _get_cursor_shape(cursor, cursor_type) + except NotImplementedError: + return UnresolvedType() + if cursor_shape: + if shape: raise NotImplementedError( f"Array of arrays not supported: the ultimate member " f"'{cursor.name}' of the StructureAccess represents " f"an array but other array notation is present in the " f"full access expression: '{self.debug_string()}'") + shape = cursor_shape + if shape: return ArrayType(cursor_type, shape) - # We don't have an explicit array access (because `shape` is Falsey) - # but is the ultimate member itself an array? + # We must have a scalar. if isinstance(cursor_type, ArrayType): - if not cursor.children: - # It is and there are no index expressions so we return the - # ArrayType. - return cursor_type # We have an access to a single element of the array. # Currently arrays of scalars are handled in a # different way to all other types of array. Issue #1857 will diff --git a/src/psyclone/psyir/tools/dependency_tools.py b/src/psyclone/psyir/tools/dependency_tools.py index 99bbaec812..5bc9002485 100644 --- a/src/psyclone/psyir/tools/dependency_tools.py +++ b/src/psyclone/psyir/tools/dependency_tools.py @@ -43,7 +43,7 @@ import sympy from psyclone.configuration import Config -from psyclone.core import (AccessType, SymbolicMaths, +from psyclone.core import (AccessType, Signature, SymbolicMaths, VariablesAccessInfo) from psyclone.errors import InternalError, LazyString from psyclone.psyir.backend.sympy_writer import SymPyWriter @@ -70,6 +70,11 @@ class DTCode(IntEnum): ERROR_MIN = 200 ERROR_WRITE_WRITE_RACE = 201 ERROR_DEPENDENCY = 202 + + ERROR_LOOP_VAR_USED_IN_OTHER_LOOP = 203 + ERROR_SCALAR_WRITTEN_AND_READ = 204 + ERROR_DIFFERENT_INDEX_LOCATIONS = 205 + ERROR_READ_WRITE_NO_LOOP_VAR = 206 ERROR_MAX = 299 @@ -806,3 +811,207 @@ def can_loop_be_parallelised(self, loop, result = False return result + +# ------------------------------------------------------------------------- + def can_loops_be_fused(self, loop1, loop2): + '''Function that verifies if two loops can be fused. + + :param loop1: the first loop. + :type loop1: :py:class:`psyclone.psyir.nodes.Loop` + :param loop2: the second loop. + :type loop2: :py:class:`psyclone.psyir.nodes.Loop` + + :return: whether the loops can be fused or not. + :rtype: bool + + ''' + # pylint: disable=too-many-locals + # This should only be called from loop_fuse_trans, which + # has done tests for loop boundaries (depending on domain) + + self._clear_messages() + vars1 = VariablesAccessInfo(loop1) + vars2 = VariablesAccessInfo(loop2) + + # Check if the loops have the same loop variable + loop_var1 = loop1.variable + loop_var2 = loop2.variable + if loop_var1 != loop_var2: + # If they don't have the same variable, find out if the one + # loop accesses the other loops variable symbol. + # If so, then for now we disallow this merge (though we could + # in theory allow using the unused one unless both use each + # others) + if Signature(loop_var2.name) in vars1: + self._add_message(f"First loop contains accesses to the " + f"second loop's variable: " + f"{loop_var2.name}.", + DTCode.ERROR_LOOP_VAR_USED_IN_OTHER_LOOP, + [loop_var2.name]) + return False + if Signature(loop_var1.name) in vars2: + self._add_message(f"Second loop contains accesses to the " + f"first loop's variable: " + f"{loop_var1.name}.", + DTCode.ERROR_LOOP_VAR_USED_IN_OTHER_LOOP, + [loop_var1.name]) + return False + + # Get all variables that occur in both loops. A variable + # that is only in one loop is not affected by fusion. + all_vars = set(vars1).intersection(vars2) + symbol_table = loop1.scope.symbol_table + + for signature in all_vars: + var_name = str(signature) + # Ignore the loop variable + if var_name == loop_var1.name: + continue + var_info1 = vars1[signature] + var_info2 = vars2[signature] + + # Variables that are only read in both loops can always be + # fused + if var_info1.is_read_only() and var_info2.is_read_only(): + continue + + symbol = symbol_table.lookup(signature.var_name) + # TODO #1270 - the is_array_access function might be moved + is_array = symbol.is_array_access(access_info=var_info1) + if not is_array: + result = self._fuse_validate_written_scalar(var_info1, + var_info2) + else: + result = self._fuse_validate_written_array(var_info1, + var_info2, + loop_var1, + loop_var2) + if not result: + return False + + return True + + # ------------------------------------------------------------------------- + def _fuse_validate_written_scalar(self, var_info1, var_info2): + '''Validates if the accesses to a scalar that is at least written once + allows loop fusion. The accesses of the variable is contained in the + two parameters (which also include the name of the variable). If the + access does not allow loop fusion, a message is added to the + dependency tools: + - a scalar variable is written in one loop, but only read in + the other. + + :param var_info1: access information for variable in the first loop. + :type var_info1: :py:class:`psyclone.core.var_info.VariableInfo` + :param var_info2: access information for variable in the second loop. + :type var_info2: :py:class:`psyclone.core.var_info.VariableInfo` + + :returns: whether the scalar is accessed in a way that allows + loop fusion. + :rtype: bool + + ''' + # If a scalar variable is first written in both loops, that pattern + # is typically ok. Example: + # - inner loops (loop variable is written then read), + # - a=sqrt(j); b(j)=sin(a)*cos(a) - a scalar variable as 'constant' + # TODO #641: atm the variable access information has no details + # about a conditional access, so the test below could result in + # incorrectly allowing fusion. But the test is essential for many + # much more typical use cases (especially inner loops). + if var_info1[0].access_type == AccessType.WRITE and \ + var_info2[0].access_type == AccessType.WRITE: + return True + + self._add_message(f"Scalar variable '{var_info1.var_name}' is " + f"written in one loop, but only read in the " + f"other loop.", DTCode.ERROR_SCALAR_WRITTEN_AND_READ) + return False + + # ------------------------------------------------------------------------- + def _fuse_validate_written_array(self, var_info1, var_info2, + loop_variable1, loop_variable2): + '''Validates if the accesses to an array, which is at least written + once, allows loop fusion. The access pattern to this array is + specified in the two parameters `var_info1` and `var_info2`. If + loop fusion is not possible, a message is added to the dependency + tools: + - an array that is written to uses inconsistent indices, e.g. + a(i,j) and a(j,i). + + :param var_info1: access information for variable in the first loop. + :type var_info1: \ + :py:class:`psyclone.core.var_info.SingleVariableAccessInfo` + :param var_info2: access information for variable in the second loop. + :type var_info2: \ + :py:class:`psyclone.core.var_info.SingleVariableAccessInfo` + :param loop_variable1: symbol of the variable associated with the \ + first loop being fused. + :type loop_variable: :py:class:`psyclone.psyir.symbols.DataSymbol` + :param loop_variable2: symbol of the variable associated with the \ + second loops being fused. + :type loop_variable: :py:class:`psyclone.psyir.symbols.DataSymbol` + + :returns: whether the scalar is accessed in a way that allows + loop fusion. + :rtype: bool + + ''' + # pylint: disable=too-many-locals + all_accesses = var_info1.all_accesses + var_info2.all_accesses + loop_var_name1 = loop_variable1.name + # Compare all accesses with the first one. If the loop variable + # is used in a different subscript, raise an error. We test this + # by computing the partition of the indices: + comp_1 = all_accesses[0].component_indices + # Note that we compare an access with itself, this will + # help us detecting if an array is accessed without using + # the loop variable (which would indicate a kind of reduction): + for other_access in all_accesses: + comp_other = other_access.component_indices + partitions = self._partition(comp_1, comp_other, + [loop_var_name1]) + for (set_of_vars, index) in partitions: + # Find the partition that contains the loop variable: + if loop_var_name1 in set_of_vars: + break + else: + error = (f"Variable '{var_info1.signature[0]}' does not " + f"depend on loop variable '{loop_var_name1}', but is " + f"read and written") + self._add_message(error, DTCode.ERROR_READ_WRITE_NO_LOOP_VAR, + [var_info1.signature[0]]) + return False + + # If the loop variable is used in different dimensions or + # members of a derived type. E.g. a(i,j) and a(j,i), or + # a(i)%b(j) and a(j)%b(i) it is used inconsistent: + if len(index) > 1: + # Add the appropriate error message: + access1 = all_accesses[0].node.debug_string() + access2 = other_access.node.debug_string() + error = (f"Variable '{var_info1.signature[0]}' is written to " + f"and the loop variable '{loop_var_name1}' is used " + f"in different index locations: {access1} and " + f"{access2}.") + self._add_message(error, + DTCode.ERROR_DIFFERENT_INDEX_LOCATIONS, + [var_info1.signature[0]]) + return False + first_index = all_accesses[0].component_indices[index[0]] + other_index = other_access.component_indices[index[0]] + if not SymbolicMaths.equal( + first_index, other_index, + identical_variables={loop_var_name1: loop_variable2.name}): + # If we have one accesses for the loop variable that is + # different from others (e.g. a(i) and a(i+1)), for now + # don't allow loop fusion. + access1 = all_accesses[0].node.debug_string() + access2 = other_access.node.debug_string() + error = (f"Variable '{var_info1.signature[0]}' is used with " + f"different indices: '{access1}' and '{access2}'.") + self._add_message(error, + DTCode.ERROR_DEPENDENCY, + [var_info1.signature[0]]) + return False + return True diff --git a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py index 0fc0238a5d..59a65b99ff 100644 --- a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py +++ b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py @@ -49,10 +49,10 @@ CodeBlock, Routine, BinaryOperation) from psyclone.psyir.nodes.array_mixin import ArrayMixin from psyclone.psyir.symbols import ( - DataSymbol, INTEGER_TYPE, ScalarType, UnresolvedType, SymbolError, - ArrayType) -from psyclone.psyir.transformations.transformation_error \ - import TransformationError + ArrayType, DataSymbol, INTEGER_TYPE, ScalarType, SymbolError, + UnresolvedType) +from psyclone.psyir.transformations.transformation_error import ( + TransformationError) from psyclone.psyir.transformations.reference2arrayrange_trans import ( Reference2ArrayRangeTrans) @@ -241,17 +241,16 @@ def validate(self, node, options=None): f"Range, but none were found in '{node.debug_string()}'.")) # All the ArrayMixins must have the same number of Ranges to expand - found = None + num_of_ranges = None for accessor in node.walk(ArrayMixin): - num_of_ranges = len([x for x in accessor.indices - if isinstance(x, Range)]) - if num_of_ranges > 0: - if not found: + count = len([x for x in accessor.indices if isinstance(x, Range)]) + if count > 0: + if not num_of_ranges: # If it's the first value we find, we store it - found = num_of_ranges + num_of_ranges = count else: # Otherwise we compare it against the previous found value - if found != num_of_ranges: + if count != num_of_ranges: raise TransformationError(LazyString( lambda: f"{self.name} does not support statements" f" containing array accesses that have varying " @@ -322,8 +321,8 @@ def validate(self, node, options=None): lambda: f"{message} in:\n{node.debug_string()}.")) # For each top-level reference (because we don't support nesting), the - # apply will have to be able to decide if its an Array (and access it - # with the index) or an Scalar (and leave it as it is). We can not + # apply() will have to determine whether it's an Array (and access it + # with the index) or a Scalar (and leave it as it is). We cannot # transform references where this is unclear. for reference in node.walk(Reference, stop_type=Reference): if isinstance(reference.parent, Call): @@ -351,12 +350,12 @@ def validate(self, node, options=None): # scalar. if not isinstance(reference.datatype, (ScalarType, ArrayType)): if isinstance(reference.symbol, DataSymbol): - typestr = f"an {reference.symbol.datatype}" + typestr = f"an {reference.datatype}" else: typestr = "not a DataSymbol" message = ( f"{self.name} cannot expand expression because it " - f"contains the variable '{reference.symbol.name}' " + f"contains the access '{reference.debug_string()}' " f"which is {typestr} and therefore cannot be guaranteed" f" to be ScalarType.") if not isinstance(reference.symbol, DataSymbol) or \ diff --git a/src/psyclone/psyir/transformations/inline_trans.py b/src/psyclone/psyir/transformations/inline_trans.py index 47da70ff55..d44df08d74 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -530,7 +530,10 @@ def _replace_formal_struc_arg(self, actual_arg, ref, call_node, break cursor = cursor.member - if not actual_arg.walk(Range) and local_indices: + # TODO #1858 - once we support converting structure accesses into + # explicit array accesses, we can put back the testing in + # inline_trans_test.py that covers this code and remove the pragma: + if not actual_arg.walk(Range) and local_indices: # pragma: no cover # There are no Ranges in the actual argument but the local # reference is an array access. # Create updated index expressions for that access. diff --git a/src/psyclone/psyir/transformations/loop_fuse_trans.py b/src/psyclone/psyir/transformations/loop_fuse_trans.py index 3a442888c6..4e6cb5ac7e 100644 --- a/src/psyclone/psyir/transformations/loop_fuse_trans.py +++ b/src/psyclone/psyir/transformations/loop_fuse_trans.py @@ -40,8 +40,7 @@ class for all API-specific loop fusion transformations. ''' -from psyclone.core import AccessType, SymbolicMaths, VariablesAccessInfo, \ - Signature +from psyclone.core import SymbolicMaths from psyclone.domain.common.psylayer import PSyLoop from psyclone.psyGen import InvokeSchedule from psyclone.psyir.nodes import Reference, Routine @@ -119,6 +118,13 @@ def validate(self, node1, node2, options=None): raise TransformationError( f"Error in {self.name} transformation. Nodes are not siblings " f"who are next to each other.") + + # Check node1 comes before node2: + if node2.position-node1.position != 1: + raise TransformationError( + f"Error in {self.name} transformation. The second loop does " + f"not immediately follow the first loop.") + # Check that the iteration space is the same if isinstance(node1, PSyLoop) and isinstance(node2, PSyLoop): # TODO 1731: For some PSyLoops the iteration space is encoded just @@ -143,155 +149,15 @@ def validate(self, node1, node2, options=None): f"{node1.debug_string()}\n" f"{node2.debug_string()}")) - vars1 = VariablesAccessInfo(node1) - vars2 = VariablesAccessInfo(node2) - - # Check if the loops have the same loop variable - loop_var1 = node1.variable - loop_var2 = node2.variable - if loop_var1 != loop_var2: - # If they don't have the same variable, find out if the one - # loop accesses the other loops variable symbol. - # If so, then for now we disallow this merge (though we could - # in theory allow using the unused one unless both use each - # others) - if Signature(loop_var2.name) in vars1: - raise TransformationError( - f"Error in {self.name} transformation. First " - f"loop contains accesses to the second loop's " - f"variable: {loop_var2.name}.") - if Signature(loop_var1.name) in vars2: - raise TransformationError( - f"Error in {self.name} transformation. Second " - f"loop contains accesses to the first loop's " - f"variable: {loop_var1.name}.") - - # Get all variables that occur in both loops. A variable - # that is only in one loop is not affected by fusion. - all_vars = set(vars1).intersection(vars2) - symbol_table = node1.scope.symbol_table - - for signature in all_vars: - var_name = str(signature) - # Ignore the loop variable - if var_name == loop_var1.name: - continue - var_info1 = vars1[signature] - var_info2 = vars2[signature] - - # Variables that are only read in both loops can always be - # fused - if var_info1.is_read_only() and var_info2.is_read_only(): - continue + if not ignore_dep_analysis: + dep_tools = DependencyTools() + can_be_fused = dep_tools.can_loops_be_fused(node1, node2) - symbol = symbol_table.lookup(signature.var_name) - # TODO #1270 - the is_array_access function might be moved - is_array = symbol.is_array_access(access_info=var_info1) - if not ignore_dep_analysis: - if not is_array: - self._validate_written_scalar(var_info1, var_info2) - else: - self._validate_written_array(var_info1, var_info2, - loop_var1) + if not can_be_fused: + messages = dep_tools.get_all_messages() + raise TransformationError(f"{self.name}. {messages[0]}") # ------------------------------------------------------------------------- - @staticmethod - def _validate_written_scalar(var_info1, var_info2): - '''Validates if the accesses to a scalar that is at least written once - allows loop fusion. The accesses of the variable is contained in the - two parameters (which also include the name of the variable). - - :param var_info1: access information for variable in the first loop. - :type var_info1: :py:class:`psyclone.core.var_info.VariableInfo` - :param var_info2: access information for variable in the second loop. - :type var_info2: :py:class:`psyclone.core.var_info.VariableInfo` - - :raises TransformationError: a scalar variable is written in one \ - loop, but only read in the other. - - ''' - # If a scalar variable is first written in both loops, that pattern - # is typically ok. Example: - # - inner loops (loop variable is written then read), - # - a=sqrt(j); b(j)=sin(a)*cos(a) - a scalar variable as 'constant' - # TODO #641: atm the variable access information has no details - # about a conditional access, so the test below could result in - # incorrectly allowing fusion. But the test is essential for many - # much more typical use cases (especially inner loops). - if var_info1[0].access_type == AccessType.WRITE and \ - var_info2[0].access_type == AccessType.WRITE: - return - - raise TransformationError( - f"Scalar variable '{var_info1.var_name}' is written in one loop, " - f"but only read in the other loop.") - - # ------------------------------------------------------------------------- - @staticmethod - def _validate_written_array(var_info1, var_info2, loop_variable): - '''Validates if the accesses to an array, which is at least written - once, allows loop fusion. The access pattern to this array is - specified in the two parameters `var_info1` and `var_info2`. - - :param var_info1: access information for variable in the first loop. - :type var_info1: \ - :py:class:`psyclone.core.var_info.SingleVariableAccessInfo` - :param var_info2: access information for variable in the second loop. - :type var_info2: \ - :py:class:`psyclone.core.var_info.SingleVariableAccessInfo` - :param loop_variable: symbol of the variable associated with the \ - loops being fused. - :type loop_variable: :py:class:`psyclone.psyir.symbols.DataSymbol` - - :raises TransformationError: an array that is written to uses \ - inconsistent indices, e.g. a(i,j) and a(j,i). - - ''' - # TODO #1075: Loop fusion should be verified using a method in - # DependencyTools. - # pylint: disable=too-many-locals - dep_tools = DependencyTools() - all_accesses = var_info1.all_accesses + var_info2.all_accesses - loop_var_name = loop_variable.name - # Compare all accesses with the first one. If the loop variable - # is used in a different subscript, raise an error. We test this - # by computing the partition of the indices:s - comp_1 = all_accesses[0].component_indices - # Note that we compare an access with itself, this will - # help us detecting if an array is accessed without using - # the loop variable (which would indicate a kind of reduction): - for other_access in all_accesses: - comp_other = other_access.component_indices - # TODO #1075: when this functionality is moved into the - # DependencyTools, the access to partition is not an - # access to a protected member anymore. - # pylint: disable=protected-access - partitions = dep_tools._partition(comp_1, comp_other, - [loop_var_name]) - var_found = False - for (set_of_vars, index) in partitions: - # Find the partition that contains the loop variable: - if loop_var_name not in set_of_vars: - continue - var_found = True - # If the loop variable contains more than one index, it is - # used inconsistent: - if len(index) <= 1: - continue - # Raise the appropriate error message: - access1 = all_accesses[0].node.debug_string() - access2 = other_access.node.debug_string() - error = (f"Variable '{var_info1.signature[0]}' is written to " - f"and the loop variable '{loop_var_name}' is used " - f"in different index locations: {access1} and " - f"{access2}.") - raise TransformationError(error) - if not var_found: - error = (f"Variable '{var_info1.signature[0]}' does not " - f"depend on loop variable '{loop_var_name}', but is " - f"read and written") - raise TransformationError(error) - def apply(self, node1, node2, options=None): # pylint: disable=arguments-differ ''' Fuses two loops represented by `psyclone.psyir.nodes.Node` objects diff --git a/src/psyclone/tests/domain/lfric/dofkern_test.py b/src/psyclone/tests/domain/lfric/dofkern_test.py index 1f0dd9fda5..7102462ac6 100644 --- a/src/psyclone/tests/domain/lfric/dofkern_test.py +++ b/src/psyclone/tests/domain/lfric/dofkern_test.py @@ -31,20 +31,25 @@ # ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- -# Authors O. Brunt, Met Office +# Author O. Brunt, Met Office ''' -This module tests the metadata validation in LFRicKernMetadata of +This module tests metadata validation and code generation of user-supplied kernels operating on degrees of freedom (dofs) ''' - +import os import pytest from fparser import api as fpapi from psyclone.configuration import Config -from psyclone.domain.lfric import LFRicKernMetadata +from psyclone.domain.lfric import LFRicKernMetadata, LFRicLoop +from psyclone.dynamo0p3 import LFRicHaloExchange +from psyclone.parse.algorithm import parse from psyclone.parse.utils import ParseError +from psyclone.psyGen import PSyFactory +from psyclone.tests.lfric_build import LFRicBuild +from psyclone.transformations import Dynamo0p3RedundantComputationTrans @pytest.fixture(scope="module", autouse=True) @@ -55,6 +60,10 @@ def setup(): Config._instance = None +BASE_PATH = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname( + os.path.abspath(__file__)))), "test_files", "dynamo0p3") +TEST_API = "lfric" + CODE = ''' module testkern_dofs_mod type, extends(kernel_type) :: testkern_dofs_type @@ -77,7 +86,8 @@ def setup(): def test_dof_kernel_mixed_function_spaces(): - ''' Check that we raise an exception if we encounter a dof kernel + ''' + Check that we raise an exception if we encounter a dof kernel call with arguments of different function spaces. ''' @@ -93,7 +103,8 @@ def test_dof_kernel_mixed_function_spaces(): def test_dof_kernel_invalid_arg(): - ''' Check that we raise an exception if we find metadata for a dof kernel + ''' + Check that we raise an exception if we find metadata for a dof kernel which specifies arguments that are not fields or scalars. ''' @@ -120,7 +131,8 @@ def test_dof_kernel_invalid_arg(): def test_dof_kernel_invalid_field_vector(): - ''' Check that we raise an exception if we encounter metadata + ''' + Check that we raise an exception if we encounter metadata for a dof kernel with a field vector. ''' @@ -142,3 +154,232 @@ def test_dof_kernel_invalid_field_vector(): assert ("Kernel 'testkern_dofs_type' operates on 'dof' but has a vector " "argument 'gh_field*3'. This is not permitted in the LFRic API." in str(excinfo.value)) + + +def test_upper_bounds(monkeypatch, annexed, dist_mem, tmpdir): + ''' + Checks that the correct upper bound is generated for a dof-kernel for all + permutations of the `DISTRIBUTED_MEMORY` and `COMPUTE_ANNEXED_DOFS` + configuration settings. + + ''' + # Set up annexed dofs + config = Config.get() + lfric_config = config.api_conf("lfric") + monkeypatch.setattr(lfric_config, "_compute_annexed_dofs", annexed) + + _, invoke_info = parse(os.path.join(BASE_PATH, + "1.14_single_invoke_dofs.f90"), + api=TEST_API) + psy = PSyFactory(TEST_API, distributed_memory=dist_mem).create(invoke_info) + code = str(psy.gen) + + # Distributed memory + if annexed and dist_mem: + expected = (" loop0_start = 1\n" + " loop0_stop = f1_proxy%vspace%get_last_dof_annexed()" + ) + elif not annexed and dist_mem: + expected = (" loop0_start = 1\n" + " loop0_stop = f1_proxy%vspace%get_last_dof_owned()" + ) + + # Shared memory + elif not dist_mem: + expected = (" loop0_start = 1\n" + " loop0_stop = undf_w1" + ) + + assert expected in code + # Check compilation + assert LFRicBuild(tmpdir).code_compiles(psy) + + +def test_indexed_field_args(tmpdir): + ''' + Checks that the correct array references are generated for all field + arguments in a dof kernel. The index should be the same as the loop + index - 'df'. + + ''' + _, invoke_info = parse(os.path.join(BASE_PATH, + "1.14_single_invoke_dofs.f90"), + api=TEST_API) + psy = PSyFactory(TEST_API, distributed_memory=False).create(invoke_info) + code = str(psy.gen) + + expected = ("call testkern_dofs_code(f1_data(df), f2_data(df), " + "f3_data(df), f4_data(df), scalar_arg)") + + assert expected in code + # Check compilation + assert LFRicBuild(tmpdir).code_compiles(psy) + + +def test_redundant_comp_trans(tmpdir, monkeypatch): + ''' + Check that the correct halo exchanges are added if redundant + computation is enabled for a dof kernel called before a + user-supplied kernel. + + ''' + api_config = Config.get().api_conf(TEST_API) + monkeypatch.setattr(api_config, "_compute_annexed_dofs", True) + _, invoke_info = parse(os.path.join(BASE_PATH, + "1.14_single_invoke_dofs.f90"), + api=TEST_API) + psy = PSyFactory(TEST_API, distributed_memory=True).create(invoke_info) + + first_invoke = psy.invokes.invoke_list[0] + + # Since (redundant) computation over annexed dofs is enabled, there + # should be no halo exchange before the first kernel call + assert isinstance(first_invoke.schedule[0], LFRicLoop) + + # Now transform the first loop to perform redundant computation out to + # the level-3 halo + rtrans = Dynamo0p3RedundantComputationTrans() + rtrans.apply(first_invoke.schedule[0], options={"depth": 3}) + + # There should now be a halo exchange for f2 + assert isinstance(first_invoke.schedule[0], LFRicHaloExchange) + assert first_invoke.schedule[0].field.name == "f2" + # Check the correct depth is set + assert "halo_exchange(depth=3)" in str(psy.gen) + assert "halo_exchange(depth=1)" not in str(psy.gen) + + # There should be one halo exchange for each field that is not f1 + # (f2, f3, f4) + assert len([node for node in first_invoke.schedule.walk(LFRicHaloExchange) + if node.field.name == "f2"]) == 1 + assert len([node for node in first_invoke.schedule.walk(LFRicHaloExchange) + if node.field.name == "f3"]) == 1 + assert len([node for node in first_invoke.schedule.walk(LFRicHaloExchange) + if node.field.name == "f4"]) == 1 + # Check compiles + assert LFRicBuild(tmpdir).code_compiles(psy) + + +def test_multi_invoke_cell_dof_builtin(tmpdir, monkeypatch, annexed, dist_mem): + ''' + Check that the expected code is generated from a multi-kernel invoke with + kernels operating on different domains. + + ''' + # Set up annexed dofs + config = Config.get() + lfric_config = config.api_conf("lfric") + monkeypatch.setattr(lfric_config, "_compute_annexed_dofs", annexed) + + _, invoke_info = parse(os.path.join( + BASE_PATH, "4.17_multikernel_invokes_cell_dof_builtin.f90"), + api=TEST_API) + psy = PSyFactory(TEST_API, distributed_memory=dist_mem).create(invoke_info) + code = str(psy.gen) + + # Work through the generated code and compare what is expected to what is + # generated + + # Use statements + assert " use testkern_mod, only : testkern_code\n" in code + assert " use testkern_dofs_mod, only : testkern_dofs_code\n" in code + if dist_mem: + # Check mesh_mod is added to use statements + assert " use mesh_mod, only : mesh_type\n" in code + + # Consistent declarations + assert """ + type(field_type), intent(in) :: f1 + type(field_type), intent(in) :: f2 + type(field_type), intent(in) :: f3 + type(field_type), intent(in) :: f4 + real(kind=r_def), intent(in) :: scalar_arg + real(kind=r_def), intent(in) :: a + type(field_type), intent(in) :: m1 + type(field_type), intent(in) :: m2 + """ in code + assert """ + real(kind=r_def), pointer, dimension(:) :: f1_data => null() + real(kind=r_def), pointer, dimension(:) :: f2_data => null() + real(kind=r_def), pointer, dimension(:) :: f3_data => null() + real(kind=r_def), pointer, dimension(:) :: f4_data => null() + real(kind=r_def), pointer, dimension(:) :: m1_data => null() + real(kind=r_def), pointer, dimension(:) :: m2_data => null() + """ in code + + # Check that dof kernel is called correctly + output = ( + " do df = loop0_start, loop0_stop, 1\n" + " call testkern_dofs_code(f1_data(df), f2_data(df), " + "f3_data(df), f4_data(df), scalar_arg)\n" + " enddo\n" + ) + assert output in code + + # Now check that correct halo exchanges and set clean/dirty are + # performed after the dof kernel call and before the next kernel (cell) + if dist_mem: + if not annexed: + # Check f1 field has halo exchange performed when annexed == False + output = ( + " do df = loop0_start, loop0_stop, 1\n" + " call testkern_dofs_code(f1_data(df), f2_data(df), " + "f3_data(df), f4_data(df), scalar_arg)\n" + " enddo\n" + "\n" + " ! Set halos dirty/clean for fields modified in the " + "above loop(s)\n" + " call f1_proxy%set_dirty()\n" + " call f1_proxy%halo_exchange(depth=1)\n" + ) + else: + # Check f1 field is set dirty but no halo exchange is performed + output = ( + " do df = loop0_start, loop0_stop, 1\n" + " call testkern_dofs_code(f1_data(df), f2_data(df), " + "f3_data(df), f4_data(df), scalar_arg)\n" + " enddo\n" + "\n" + " ! Set halos dirty/clean for fields modified in the " + "above loop(s)\n" + " call f1_proxy%set_dirty()\n" + ) + # This should be present in all distributed memory cases: + # Check halos are set dirty/clean for modified fields in dof + # kernel (above) and happen before the next kernel (cell_column) + common_halo_exchange_code = ( + " if (f2_proxy%is_dirty(depth=1)) then\n" + " call f2_proxy%halo_exchange(depth=1)\n" + " end if\n" + " if (m1_proxy%is_dirty(depth=1)) then\n" + " call m1_proxy%halo_exchange(depth=1)\n" + " end if\n" + " if (m2_proxy%is_dirty(depth=1)) then\n" + " call m2_proxy%halo_exchange(depth=1)\n" + " end if\n" + " do cell = loop1_start, loop1_stop, 1\n" + " call testkern_code" + ) + output += common_halo_exchange_code # Append common + assert output in code + + # Check cell-column kern is called correctly + output = ( + " do cell = loop1_start, loop1_stop, 1\n" + " call testkern_code(nlayers_f1, a, f1_data, f2_data, m1_data, " + "m2_data, ndf_w1, undf_w1, map_w1(:,cell), ndf_w2, undf_w2, " + "map_w2(:,cell), ndf_w3, undf_w3, map_w3(:,cell))\n" + " enddo\n" + ) + assert output in code + + # Check built-in is called correctly + output = ( + " do df = loop2_start, loop2_stop, 1\n" + " ! Built-in: inc_aX_plus_Y (real-valued fields)\n" + " f1_data(df) = 0.5_r_def * f1_data(df) + f2_data(df)\n" + " enddo\n" + ) + assert output in code + + assert LFRicBuild(tmpdir).code_compiles(psy) diff --git a/src/psyclone/tests/domain/lfric/kern_call_acc_arg_list_test.py b/src/psyclone/tests/domain/lfric/kern_call_acc_arg_list_test.py index 26f1f339bf..6e6d4a9f06 100644 --- a/src/psyclone/tests/domain/lfric/kern_call_acc_arg_list_test.py +++ b/src/psyclone/tests/domain/lfric/kern_call_acc_arg_list_test.py @@ -212,3 +212,31 @@ def test_lfric_stencil(): assert "f1: READ+WRITE" in var_info assert "f2: READ" in var_info assert "f2_stencil_dofmap: READ" in var_info + + +def test_lfric_field(): + '''Check that the method to generate a field argument returns the + field data varaible name and the correct variable access info. + + ''' + # Use the OpenACC transforms to create the required kernels + acc_par_trans = ACCParallelTrans() + acc_enter_trans = ACCEnterDataTrans() + _, invoke = get_invoke("15.1.1_builtin_and_normal_kernel_invoke_2.f90", + "lfric", + idx=0, dist_mem=False) + sched = invoke.schedule + acc_par_trans.apply(sched.children) + acc_enter_trans.apply(sched) + + # Find the first kernel: + kern = invoke.schedule.walk(psyGen.CodedKern)[0] + create_acc_arg_list = KernCallAccArgList(kern) + var_accesses = VariablesAccessInfo() + create_acc_arg_list.generate(var_accesses=var_accesses) + var_info = str(var_accesses) + # Check fields + assert "f1_data: READ+WRITE" in var_info # Written to in Built-in + assert "f2_data: READ" in var_info + assert "m1_data: READ" in var_info + assert "m2_data: READ" in var_info diff --git a/src/psyclone/tests/domain/lfric/lfric_kern_test.py b/src/psyclone/tests/domain/lfric/lfric_kern_test.py index c8ef438fe1..989a3e3e6e 100644 --- a/src/psyclone/tests/domain/lfric/lfric_kern_test.py +++ b/src/psyclone/tests/domain/lfric/lfric_kern_test.py @@ -456,3 +456,17 @@ def test_kern_not_coloured_inc(monkeypatch): assert ("Kernel 'testkern_code' has an argument with INC access and " "therefore must be coloured in order to be parallelised with " "OpenMP." in str(err.value)) + + +def test_undf_name(): + '''Tests that the LFRicKern.undf_name property returns the correct + result when called. + + ''' + _, invoke_info = parse(os.path.join(BASE_PATH, "1_single_invoke.f90"), + api=TEST_API) + psy = PSyFactory(TEST_API, distributed_memory=True).create(invoke_info) + sched = psy.invokes.invoke_list[0].schedule + kern = sched.walk(LFRicKern)[0] + + assert kern.undf_name == "undf_w1" diff --git a/src/psyclone/tests/psyad/main_test.py b/src/psyclone/tests/psyad/main_test.py index 63ad4d9c97..70918e38fd 100644 --- a/src/psyclone/tests/psyad/main_test.py +++ b/src/psyclone/tests/psyad/main_test.py @@ -126,8 +126,9 @@ def test_main_h_option(capsys): assert str(info.value) == "0" output, error = capsys.readouterr() assert error == "" - # The name of the executable is replaced with either pytest or -c - # when using pytest, therefore we split this test into sections. + # Python usage messages have seen slight tweaks over the years, e.g., + # Python >= 3.13 tweaks the usage message to avoid repeating the args + # to an option between aliases, therefore we split this test into sections. assert "usage: " in output expected2 = ( "[-h] [-oad OAD] [-v] [-t] [-api API] [-coord-arg COORD_ARG] " @@ -138,9 +139,12 @@ def test_main_h_option(capsys): "positional arguments:\n" " filename tangent-linear kernel source\n\n") assert expected2 in output + assert (" -h, --help show this help message and exit\n" + in output) + assert (" -a ACTIVE [ACTIVE ...], --active ACTIVE [ACTIVE ...]\n" + in output or + " -a, --active ACTIVE [ACTIVE ...]\n" in output) expected3 = ( - " -h, --help show this help message and exit\n" - " -a ACTIVE [ACTIVE ...], --active ACTIVE [ACTIVE ...]\n" " names of active variables\n" " -v, --verbose increase the verbosity of the output\n" " -t, --gen-test generate a standalone unit test for the " @@ -161,9 +165,6 @@ def test_main_h_option(capsys): " -otest TEST_FILENAME filename for the unit test (implies -t)\n" " -oad OAD filename for the transformed code\n") assert expected3 in output - assert ("-otest TEST_FILENAME filename for the unit test (implies -t)" - in output) - assert "-oad OAD filename for the transformed code" in output # no args diff --git a/src/psyclone/tests/psyir/backend/sympy_writer_test.py b/src/psyclone/tests/psyir/backend/sympy_writer_test.py index 1f8b8149c7..e1e1d38bb2 100644 --- a/src/psyclone/tests/psyir/backend/sympy_writer_test.py +++ b/src/psyclone/tests/psyir/backend/sympy_writer_test.py @@ -46,7 +46,7 @@ from psyclone.psyir.frontend.sympy_reader import SymPyReader from psyclone.psyir.backend.sympy_writer import SymPyWriter from psyclone.psyir.backend.visitor import VisitorError -from psyclone.psyir.nodes import Literal +from psyclone.psyir.nodes import Assignment, Literal, Node from psyclone.psyir.symbols import (ArrayType, BOOLEAN_TYPE, CHARACTER_TYPE, INTEGER_TYPE) @@ -251,7 +251,7 @@ def test_sym_writer_rename_members(fortran_reader, expressions): @pytest.mark.parametrize("expr, sym_map", [("a%x", {"a": Symbol("a"), - "a_x": Symbol("a_x")}), + "a_x": Symbol("a%x")}), ("a%x(i)", {"a": Symbol("a"), "a_x": Function("a_x"), "i": Symbol("i")}), @@ -285,7 +285,7 @@ def test_sym_writer_symbol_types(fortran_reader, expr, sym_map): expr = psyir.children[0].children[0].rhs sympy_writer = SymPyWriter() _ = sympy_writer(expr) - assert sympy_writer.type_map.keys() == sym_map.keys() + assert sympy_writer.type_map.items() == sym_map.items() @pytest.mark.parametrize("expr, sym_map", [("i", {'i': Symbol('i')}), @@ -554,3 +554,58 @@ def test_sym_writer_reserved_names(fortran_reader, expression): sympy_exp = sympy_writer(psyir_expr) assert str(sympy_exp) == expression + + +@pytest.mark.parametrize("expressions", [("a+b", "2*b"), + ("a-b", "0"), + ("a-a", "0"), + ("b-b", "0"), + ("b", "b"), + # We can't just use 'a', since then + # there would be no variable 'b' + # defined. So add 0*b: + ("a-0*b", "b"), + ("a+b+c", "2*b + c"), + ]) +def test_sym_writer_identical_variables(fortran_reader, expressions): + '''Test that we can indicate that certain variables are identical, + in which case the sympy expression will replace one variable with + the other. For example, if a=b --> a-b = b-b = 0 + ''' + # A dummy program to easily create the PSyIR for the + # expressions we need. We just take the RHS of the assignments + source = f'''program test_prog + use some_mod + integer :: a, b, c + x = {expressions[0]} + end program test_prog ''' + psyir = fortran_reader.psyir_from_source(source) + # Take the right-hand-side of the assignment: + expr = psyir.walk(Assignment)[0].rhs + + sympy_writer = SymPyWriter() + identical_variables = {'a': 'b'} + assert (str(sympy_writer(expr, identical_variables=identical_variables)) + == expressions[1]) + + +def test_sym_writer_identical_variables_errors(): + '''Test that we raise appropriate errors if identical_variables is or + contains unexpected types. + ''' + + sympy_writer = SymPyWriter() + with pytest.raises(TypeError) as err: + sympy_writer(Node(), identical_variables=1) + assert ("Expected identical_variables to be a dictionary, but got " + "" in str(err.value)) + + with pytest.raises(TypeError) as err: + sympy_writer(Node(), identical_variables={1: 1}) + assert ("Dictionary identical_variables contains a non-string key or " + "value" in str(err.value)) + + with pytest.raises(TypeError) as err: + sympy_writer(Node(), identical_variables={"var": 1}) + assert ("Dictionary identical_variables contains a non-string key or " + "value" in str(err.value)) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py index 3135b37a58..1e6a8c8553 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py @@ -850,6 +850,18 @@ def test_unsupported_decln(fortran_reader): assert ("Invalid variable declaration found in _process_decln for " "'problem'" in str(err.value)) + reader = FortranStringReader("integer, intent(in) :: l1") + fparser2spec = Specification_Part(reader).content[0] + # Break the attribute-spec list for this declaration. + attr_specs = fparser2spec.items[1] + attr_specs.items = (attr_specs.items[0], "rubbish") + fake_parent = KernelSchedule.create("dummy_schedule") + symtab = fake_parent.symbol_table + processor = Fparser2Reader() + with pytest.raises(NotImplementedError) as error: + processor._process_decln(fake_parent, symtab, fparser2spec) + assert "Unrecognised attribute type 'str'" in str(error.value) + def test_unsupported_decln_structure_type(fortran_reader): ''' diff --git a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py index 3b51510dcb..b87f0666a2 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_where_handler_test.py @@ -692,16 +692,24 @@ def test_where_stmt_validity(): @pytest.mark.usefixtures("parser") -def test_where_stmt(): +def test_where_stmt(fortran_reader): ''' Basic check that we handle a WHERE statement correctly. ''' - fake_parent, _ = process_where( - "WHERE( at_i(:,:) > rn_amax_2d(:,:) ) " - "a_i(:,:,jl) = a_i(:,:,jl) * rn_amax_2d(:,:) / at_i(:,:)", - Fortran2003.Where_Stmt, ["at_i", "rn_amax_2d", "jl", "a_i"]) - assert len(fake_parent.children) == 1 - assert isinstance(fake_parent[0], Loop) + code = '''\ + program where_test + implicit none + integer :: jl + real, dimension(:,:), allocatable :: at_i, rn_amax_2d + real, dimension(:,:,:), allocatable :: a_i + WHERE( at_i(:,:) > rn_amax_2d(:,:) ) & + a_i(:,:,jl) = a_i(:,:,jl) * rn_amax_2d(:,:) / at_i(:,:) + end program where_test + ''' + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + assert len(routine.children) == 1 + assert isinstance(routine[0], Loop) def test_where_stmt_no_reduction(fortran_reader, fortran_writer): @@ -765,11 +773,11 @@ def test_where_ordering(parser): ("where (my_type%var(:) > epsi20)\n" "my_type2(jl)%array2(:,jl) = 3.0\n", "my_type%var"), ("where (my_type%block(jl)%var(:) > epsi20)\n" - "my_type%block%array(:,jl) = 3.0\n", "my_type%block(jl)%var"), + "my_type%block(jl)%array(:,jl) = 3.0\n", "my_type%block(jl)%var"), ("where (my_type%block(jl)%var(:) > epsi20)\n" "my_type%block(jl)%array(:,jl) = 3.0\n", "my_type%block(jl)%var"), - ("where (my_type2(:)%var > epsi20)\n" - "my_type2(:)%var = 3.0\n", "my_type2")]) + ("where (my_type2(:)%var(jl) > epsi20)\n" + "my_type2(:)%var(jl) = 3.0\n", "my_type2")]) def test_where_derived_type(fortran_reader, code, size_arg): ''' Test that we handle the case where array members of a derived type are accessed within a WHERE. ''' @@ -805,10 +813,12 @@ def test_where_derived_type(fortran_reader, code, size_arg): assert isinstance(loops[1].loop_body[0], IfBlock) # All Range nodes should have been replaced assert not loops[0].walk(Range) - # All ArrayMember accesses should now use the `widx1` loop variable + # All ArrayMember accesses other than 'var' should now use the `widx1` + # loop variable array_members = loops[0].walk(ArrayMember) for member in array_members: - assert "+ widx1 - 1" in member.indices[0].debug_string() + if member.name != "var": + assert "+ widx1 - 1" in member.indices[0].debug_string() @pytest.mark.parametrize( diff --git a/src/psyclone/tests/psyir/nodes/array_mixin_test.py b/src/psyclone/tests/psyir/nodes/array_mixin_test.py index f9290e82b3..7c4630fed1 100644 --- a/src/psyclone/tests/psyir/nodes/array_mixin_test.py +++ b/src/psyclone/tests/psyir/nodes/array_mixin_test.py @@ -608,6 +608,7 @@ def test_get_effective_shape(fortran_reader): code = ( "subroutine test()\n" " use some_mod\n" + " integer :: idx = 2\n" " integer :: indices(8,3)\n" " real a(10), b(10,10)\n" " a(1:10) = 0.0\n" @@ -621,6 +622,8 @@ def test_get_effective_shape(fortran_reader): " b(indices(2:3,1:2), 2:5) = 2.0\n" " a(f()) = 2.0\n" " a(2+3) = 1.0\n" + " b(idx, a) = -1.0\n" + " b(scalarval, arrayval) = 1\n" "end subroutine\n") psyir = fortran_reader.psyir_from_source(code) routine = psyir.walk(Routine)[0] @@ -691,6 +694,49 @@ def test_get_effective_shape(fortran_reader): with pytest.raises(NotImplementedError) as err: _ = routine.children[child_idx].lhs._get_effective_shape() assert "include a function call or expression" in str(err.value) + # Array access with indices given by another array that is not explicitly + # indexed. + child_idx += 1 + shape = routine.children[child_idx].lhs._get_effective_shape() + assert len(shape) == 1 + assert "SIZE(a)" in shape[0].debug_string() + # Array-index expressions are symbols of unknown type so we don't know + # whether we have an array slice or just a scalar. + child_idx += 1 + with pytest.raises(NotImplementedError) as err: + _ = routine.children[child_idx].lhs._get_effective_shape() + assert ("index expression 'scalarval' in access 'b(scalarval,arrayval)' is" + " of 'UnresolvedType' type and therefore whether it is an array " + "slice (i.e. an indirect access) cannot be determined." + in str(err.value)) + + +def test_struct_get_effective_shape(fortran_reader): + '''Tests for the _get_effective_shape() method for ArrayMember and + ArrayOfStructuresMixin (since they inherit it from ArrayMixin).''' + code = ( + "subroutine test()\n" + " type :: my_type\n" + " real, dimension(21,12) :: data\n" + " end type my_type\n" + " type(my_type) :: grid\n" + " type(my_type), dimension(5) :: grid_list\n" + " grid%data(:,:) = 0.0\n" + " grid_list(:)%data(1) = 0.0\n" + "end subroutine\n") + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Routine)[0] + # Slice of ArrayMember + child_idx = 0 + shape = routine.children[child_idx].lhs.member._get_effective_shape() + assert len(shape) == 2 + assert "SIZE(grid%data, dim=1)" in shape[0].debug_string() + assert "SIZE(grid%data, dim=2)" in shape[1].debug_string() + # Slice of ArrayOfStructuresMixin + child_idx += 1 + shape = routine.children[child_idx].lhs._get_effective_shape() + assert len(shape) == 1 + assert isinstance(shape[0], IntrinsicCall) # get_outer_range_index diff --git a/src/psyclone/tests/psyir/nodes/array_reference_test.py b/src/psyclone/tests/psyir/nodes/array_reference_test.py index 8073840657..027e812ac8 100644 --- a/src/psyclone/tests/psyir/nodes/array_reference_test.py +++ b/src/psyclone/tests/psyir/nodes/array_reference_test.py @@ -580,6 +580,13 @@ def test_array_datatype(): aref6._symbol = test_struc_sym assert isinstance(aref6.datatype, UnresolvedType) + # TODO #2448 - we don't handle an array access to something that we + # don't know is an array. + aref7 = ArrayReference(generic_sym) + aref7.addchild(one.copy()) + aref7._symbol = DataSymbol("int_test", INTEGER_TYPE) + assert isinstance(aref7.datatype, UnresolvedType) + def test_array_create_colon(fortran_writer): '''Test that the create method accepts ":" as shortcut to automatically diff --git a/src/psyclone/tests/psyir/nodes/assignment_test.py b/src/psyclone/tests/psyir/nodes/assignment_test.py index 3e2d13e4d5..b610fd3b43 100644 --- a/src/psyclone/tests/psyir/nodes/assignment_test.py +++ b/src/psyclone/tests/psyir/nodes/assignment_test.py @@ -34,6 +34,7 @@ # Authors R. W. Ford, A. R. Porter and S. Siso, STFC Daresbury Lab # I. Kavcic, Met Office # J. Henrichs, Bureau of Meteorology +# J. G. Wallwork, University of Cambridge # ----------------------------------------------------------------------------- ''' Performs py.test tests on the Assignment PSyIR node. ''' @@ -100,6 +101,7 @@ def test_assignment_create(): lhs = Reference(DataSymbol("tmp", REAL_SINGLE_TYPE)) rhs = Literal("0.0", REAL_SINGLE_TYPE) assignment = Assignment.create(lhs, rhs) + assert assignment.is_literal_assignment check_links(assignment, [lhs, rhs]) result = FortranWriter().assignment_node(assignment) assert result == "tmp = 0.0\n" @@ -150,6 +152,7 @@ def test_is_array_assignment(): array_ref = ArrayReference.create(symbol, [x_range, int_one.copy()]) assignment = Assignment.create(array_ref, one.copy()) assert assignment.is_array_assignment is True + assert assignment.is_literal_assignment # Check when lhs consists of various forms of structure access grid_type = StructureType.create([ @@ -349,8 +352,11 @@ def test_pointer_assignment(): assignment1 = Assignment(is_pointer=True) assignment1.addchild(lhs.copy()) assignment1.addchild(rhs.copy()) + assert not assignment1.is_literal_assignment assignment2 = Assignment.create(lhs, rhs, is_pointer=True) + assert not assignment2.is_literal_assignment not_pointer = Assignment.create(lhs.copy(), rhs.copy()) + assert not not_pointer.is_literal_assignment # Getters, equality and copy assert assignment1.is_pointer diff --git a/src/psyclone/tests/psyir/nodes/datanode_test.py b/src/psyclone/tests/psyir/nodes/datanode_test.py index 421cac772c..cde45d53eb 100644 --- a/src/psyclone/tests/psyir/nodes/datanode_test.py +++ b/src/psyclone/tests/psyir/nodes/datanode_test.py @@ -32,13 +32,17 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author: A. R. Porter, STFC Daresbury Lab +# Modified J. G. Wallwork, University of Cambridge # ----------------------------------------------------------------------------- '''Performs pytest tests on the PSyIR DataNode. ''' -from psyclone.psyir.nodes import DataNode -from psyclone.psyir.symbols import UnresolvedType +import pytest + +from psyclone.psyir.nodes import DataNode, Reference, BinaryOperation +from psyclone.psyir.symbols import (CHARACTER_TYPE, DataSymbol, UnresolvedType, + INTEGER_SINGLE_TYPE, REAL_TYPE) def test_datanode_datatype(): @@ -48,3 +52,30 @@ def test_datanode_datatype(): ''' dnode = DataNode() assert isinstance(dnode.datatype, UnresolvedType) + + +def test_datanode_is_character(): + '''Test that character expressions are marked correctly. + ''' + reference = Reference(DataSymbol("char", CHARACTER_TYPE)) + assert reference.is_character() + + reference = Reference(DataSymbol("int", INTEGER_SINGLE_TYPE)) + reference2 = Reference(DataSymbol("int2", INTEGER_SINGLE_TYPE)) + bop = BinaryOperation.create(BinaryOperation.Operator.MUL, + reference, reference2) + assert not reference.is_character() + assert not reference2.is_character() + assert not bop.is_character() + + reference = Reference(DataSymbol("real", REAL_TYPE)) + assert not reference.is_character() + + reference = Reference(DataSymbol("unknown", UnresolvedType())) + with pytest.raises(ValueError) as excinfo: + _ = reference.is_character() + assert ("is_character could not resolve whether the expression 'unknown'" + " operates on characters." in str(excinfo.value)) + reference = Reference(DataSymbol("unknown", UnresolvedType())) + assert not reference.is_character(unknown_as=False) + assert reference.is_character(unknown_as=True) diff --git a/src/psyclone/tests/psyir/nodes/structure_reference_test.py b/src/psyclone/tests/psyir/nodes/structure_reference_test.py index 3531d7478f..0af67c5dc6 100644 --- a/src/psyclone/tests/psyir/nodes/structure_reference_test.py +++ b/src/psyclone/tests/psyir/nodes/structure_reference_test.py @@ -241,7 +241,9 @@ 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), + # An array of structure type. ("titty", artype, symbols.Symbol.Visibility.PUBLIC, None)]) # Symbol with type defined by StructureType ssym0 = symbols.DataSymbol("grid", grid_type) @@ -273,6 +275,20 @@ def test_struc_ref_datatype(): ssym, [("titty", [one.copy(), two.copy()])]) assert singleref.datatype == rtype_sym + # Reference to grid%titty(1,2)%gibber + sref = nodes.StructureReference.create( + ssym, [("titty", [one.copy(), two.copy()]), "gibber"]) + assert sref.datatype == symbols.BOOLEAN_TYPE + + # Reference to grid%titty(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()])]) + 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)) diff --git a/src/psyclone/tests/psyir/tools/dependency_tools_test.py b/src/psyclone/tests/psyir/tools/dependency_tools_test.py index 85a9beee34..49a85611fb 100644 --- a/src/psyclone/tests/psyir/tools/dependency_tools_test.py +++ b/src/psyclone/tests/psyir/tools/dependency_tools_test.py @@ -41,10 +41,17 @@ from psyclone.configuration import Config from psyclone.core import Signature, VariablesAccessInfo from psyclone.errors import InternalError +from psyclone.psyir.nodes import Loop from psyclone.psyir.tools import DependencyTools, DTCode from psyclone.tests.utilities import get_invoke +@pytest.fixture(scope="function", autouse=True) +def clear_config_instance(): + '''The tests in this file all assume that no DSL API is used.''' + Config.get().api = "" + + # ----------------------------------------------------------------------------- def test_messages(): '''Tests the messaging system of the dependency tools.''' @@ -86,7 +93,7 @@ def test_messages(): def test_dep_tool_constructor_errors(): '''Test that invalid loop types raise an error in the constructor. ''' - # Test that a a change to the API works as expected, i.e. does + # Test that a change to the API works as expected, i.e. does # not raise an exception with a valid loop type, but still raises # one with an invalid loop type Config.get().api = "lfric" @@ -729,3 +736,303 @@ def test_gocean_parallel(): "'u_fld(i,j - 1)' in '< kern call: stencil_not_parallel_code >' " "are dependent and cannot be parallelised. Variable: 'u_fld'." in str(dep_tools.get_all_messages()[0])) + + +def test_fuse_different_variables_with_access(fortran_reader): + '''Test that fusing loops with different variables is disallowed when + either loop uses the other loop's variable for any reason.''' + code = '''subroutine sub() + integer :: ji, jj, n, jk + integer, dimension(10, 10) :: s, t + do jj = 1, n + do ji = 1, 10 + s(ji, jj) = t(ji, jj) + 1 + end do + do jk = 1, 10 + ji = jk + s(jk, jj) = t(jk, jj) + ji + end do + end do + end subroutine sub''' + psyir = fortran_reader.psyir_from_source(code) + loops = psyir.children[0].walk(Loop) + dep_tools = DependencyTools() + + assert not dep_tools.can_loops_be_fused(loops[1], loops[2]) + assert len(dep_tools.get_all_messages()) == 1 + msg = dep_tools.get_all_messages()[0] + assert ("Second loop contains accesses to the first loop's variable: ji" + in str(msg)) + assert msg.var_names[0] == "ji" + + # We provide the loops in the reverse order. The loop fusion + # transformation would reject this, but the dependency tools only + # check the dependencies, so this issue would not be raised here. + assert not dep_tools.can_loops_be_fused(loops[2], loops[1]) + msg = dep_tools.get_all_messages()[0] + assert ("First loop contains accesses to the second loop's variable: ji" + in str(msg)) + assert msg.var_names[0] == "ji" + + +# ---------------------------------------------------------------------------- +def test_fuse_inconsistent_array_indexing(fortran_reader): + '''Test that accessing an array with inconsistent index usage (e.g. s(i,j) + and s(j,i)) is detected. + ''' + + code = '''subroutine sub() + integer :: ji, jj, n + integer, dimension(10,10) :: s, t + do jj=1, n + do ji=1, 10 + s(ji, jj)=t(ji, jj)+1 + enddo + enddo + do jj=1, n + do ji=1, 10 + t(ji, jj) = s(ji, jj+1) + t(ji, jj) + enddo + enddo + end subroutine sub''' + + dep_tools = DependencyTools() + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + + assert not dep_tools.can_loops_be_fused(loop1, loop2) + msg = dep_tools.get_all_messages()[0] + assert ("Variable 's' is used with different indices: 's(ji,jj)' and " + "'s(ji,jj + 1)" in str(msg)) + + +# ---------------------------------------------------------------------------- +def test_fuse_loop_independent_array_access(fortran_reader): + '''Test that using arrays which are not dependent on the loop variable + are handled correctly. Example: + do j ... a(1) = b(j) * c(j) + do j ... d(j) = a(1) + ''' + # In this example s(1,1) is used as a scalar (i.e. not dependent + # on the loop variable), and fusing is invalid + code = '''subroutine sub() + integer :: ji, jj, n + integer, dimension(10,10) :: s, t + do jj=1, n + do ji=1, 10 + s(1, 1)=t(ji, jj)+1 + enddo + enddo + do jj=1, n + do ji=1, 10 + t(ji, jj) = s(1, 1) + t(ji, jj) + enddo + enddo + end subroutine sub''' + + dep_tools = DependencyTools() + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + assert not dep_tools.can_loops_be_fused(loop1, loop2) + msg = dep_tools.get_all_messages()[0] + assert ("Variable 's' does not depend on loop variable 'jj', but is " + "read and written" in str(msg)) + + +# ---------------------------------------------------------------------------- +def test_fuse_scalars(fortran_reader): + '''Test that using scalars work as expected in all combinations of + being read/written in both loops. + ''' + + # First test: read/read of scalar variable + code = '''subroutine sub() + integer :: ji, jj, n + real, dimension(10,10) :: s, t + real :: a + do jj=1, n + do ji=1, 10 + s(ji, jj) = t(ji, jj) + a + enddo + enddo + do jj=1, n + do ji=1, 10 + t(ji, jj) = t(ji, jj) - a + enddo + enddo + end subroutine sub''' + dep_tools = DependencyTools() + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + assert dep_tools.can_loops_be_fused(loop1, loop2) + + # Second test: read/write of scalar variable + code = '''subroutine sub() + integer :: ji, jj, n + real, dimension(10,10) :: s, t + real :: a + do jj=1, n + do ji=1, 10 + s(ji, jj)=t(ji, jj)+a + enddo + enddo + do jj=1, n + do ji=1, 10 + a = t(ji, jj) - 2 + s(ji, jj)=t(ji, jj)+a + enddo + enddo + end subroutine sub''' + + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + assert not dep_tools.can_loops_be_fused(loop1, loop2) + msg = dep_tools.get_all_messages()[0] + assert ("Scalar variable 'a' is written in one loop, but only read in " + "the other loop." in str(msg)) + + # Third test: write/read of scalar variable + code = '''subroutine sub() + integer :: ji, jj, n + real, dimension(10,10) :: s, t + real :: b + do jj=1, n + do ji=1, 10 + b = t(ji, jj) - 2 + s(ji, jj )=t(ji, jj)+b + enddo + enddo + do jj=1, n + do ji=1, 10 + s(ji, jj)=t(ji, jj)+b + enddo + enddo + end subroutine sub''' + + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + assert not dep_tools.can_loops_be_fused(loop1, loop2) + msg = dep_tools.get_all_messages()[0] + assert ("Scalar variable 'b' is written in one loop, but only read in " + "the other loop." in str(msg)) + + # Fourth test: write/write of scalar variable - this is ok + code = '''subroutine sub() + integer :: ji, jj, n + real, dimension(10,10) :: s, t + real :: b + do jj=1, n + do ji=1, 10 + b = t(ji, jj) - 2 + s(ji, jj )=t(ji, jj)+b + enddo + enddo + do jj=1, n + do ji=1, 10 + b = sqrt(t(ji, jj)) + s(ji, jj)=t(ji, jj)+b + enddo + enddo + end subroutine sub''' + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + assert dep_tools.can_loops_be_fused(loop1, loop2) + + +# ---------------------------------------------------------------------------- +def test_fuse_dimension_change(fortran_reader): + '''Test that inconsistent use of dimensions are detected, e.g.: + loop1: a(i,j) + loop2: a(j,i) + when at least one operation is a write + ''' + + # This cannot be fused, since 's' is written in the first iteration + # and read in the second with inconsistent indices + code = '''subroutine sub() + integer :: ji, jj, n + integer, dimension(10,10) :: s, t, u + do jj=1, n+1 + do ji=1, 10 + s(ji, jj)=t(ji, jj)+1 + enddo + enddo + do jj=1, n+1 + do ji=1, 10 + u(ji, jj)=s(jj, ji)+1 + enddo + enddo + end subroutine sub''' + + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + dep_tools = DependencyTools() + assert not dep_tools.can_loops_be_fused(loop1, loop2) + msg = dep_tools.get_all_messages()[0] + assert ("Variable 's' is written to and the " + "loop variable 'jj' is used in different index locations: " + "s(ji,jj) and s(jj,ji)." + in str(msg)) + + # This cannot be fused, since 's' is read in the + # first iteration and written in the second with + # different indices. + code = '''subroutine sub() + integer :: ji, jj, n + integer, dimension(10,10) :: s, t, u + do jj=1, n+1 + do ji=1, 10 + u(ji, jj)=s(jj, ji)+1 + enddo + enddo + do jj=1, n+1 + do ji=1, 10 + s(ji, jj)=t(ji, jj)+1 + enddo + enddo + end subroutine sub''' + + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + dep_tools = DependencyTools() + assert not dep_tools.can_loops_be_fused(loop1, loop2) + msg = dep_tools.get_all_messages()[0] + assert ("Variable 's' is written to and the loop variable 'jj' is " + "used in different index locations: s(jj,ji) and s(ji,jj)." + in str(msg)) + + # Same test using a structure type: + code = '''subroutine sub() + use my_module + integer :: ji, jj, n + type(my_type) :: s, t, u + do jj=1, n+1 + do ji=1, 10 + u%comp1(ji)%comp2(jj)=s%comp1(jj)%comp2(ji)+1 + enddo + enddo + do jj=1, n+1 + do ji=1, 10 + s%comp1(ji)%comp2(jj)=t%comp1(ji)%comp2(jj)+1 + enddo + enddo + end subroutine sub''' + + psyir = fortran_reader.psyir_from_source(code) + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + dep_tools = DependencyTools() + assert not dep_tools.can_loops_be_fused(loop1, loop2) + msg = dep_tools.get_all_messages()[0] + assert ("Variable 's' is written to and the loop variable 'jj' is used " + "in different index locations: s%comp1(jj)%comp2(ji) and " + "s%comp1(ji)%comp2(jj)." + in str(msg)) diff --git a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py index 2348ac0938..f2bf3bb7aa 100644 --- a/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/arrayassignment2loops_trans_test.py @@ -303,6 +303,40 @@ def test_apply_to_arrays_with_different_bounds(fortran_reader, fortran_writer): in output_test4) +def test_apply_indirect_indexing(fortran_reader, fortran_writer): + ''' + Check the application of the transformation when the array is indexed + indirectly. + + ''' + psyir = fortran_reader.psyir_from_source(''' + program test + use dom_oce + INTEGER, DIMENSION(4) :: iwewe + INTEGER, DIMENSION(8,kfld) :: ishtSi + integer :: jf + do jf = 1, kfld + ishtSi(5:8,jf) = ishtSi(iwewe, jf) + ishtSi(1:4,jf) = grid(3)%var(iwewe, jf) + end do + end program test + ''') + assignments = psyir.walk(Assignment) + trans = ArrayAssignment2LoopsTrans() + trans.apply(assignments[0]) + trans.apply(assignments[1]) + result = fortran_writer(psyir) + assert (''' + do idx = 5, 8, 1 + ishtsi(idx,jf) = ishtsi(iwewe(idx + (1 - 5)),jf) + enddo''' in result) + assert (''' + do idx_1 = 1, 4, 1 + ishtsi(idx_1,jf) = grid(3)%var(iwewe(idx_1),jf) + enddo + enddo''' in result) + + def test_apply_outside_routine(fortran_reader, fortran_writer): ''' Check that the transformation still succeeds and generates valid code when found in a scope detached from a Routine. ''' @@ -568,15 +602,18 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): ''' psyir = fortran_reader.psyir_from_source(''' subroutine test(unsupported) - use my_variables, only: unresolved + use my_variables, only: unresolved, map integer :: scalar = 1 integer, dimension(:) :: array = 1, x integer, dimension(:), optional :: unsupported - + integer, dimension(4) :: ishtsi x(:) = scalar x(:) = array x(:) = unresolved x(:) = unsupported + ! An indirectly-addressed RHS but we can't tell whether or not map is + ! an array reference + x(:) = ishtsi(map,scalar) end subroutine test ''') @@ -594,7 +631,7 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): with pytest.raises(TransformationError) as info: trans.apply(assign_with_unresolved, opts) assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unresolved' which is not a DataSymbol " + "contains the access 'unresolved' which is not a DataSymbol " "and therefore cannot be guaranteed to be ScalarType. Resolving " "the import that brings this variable into scope may help." in str(info.value)) @@ -604,18 +641,25 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): with pytest.raises(TransformationError) as info: trans.apply(assign_with_unresolved) assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unresolved' which is an UnresolvedType " + "contains the access 'unresolved' which is an UnresolvedType " "and therefore cannot be guaranteed to be ScalarType. Resolving " "the import that brings this variable into scope may help." in str(info.value)) with pytest.raises(TransformationError) as info: trans.apply(psyir.walk(Assignment)[3], opts) assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unsupported' which is an Unsupported" + "contains the access 'unsupported' which is an Unsupported" "FortranType('INTEGER, DIMENSION(:), OPTIONAL :: unsupported') " "and therefore cannot be guaranteed to be ScalarType." in str(info.value)) + with pytest.raises(TransformationError) as info: + trans.apply(psyir.walk(Assignment)[4], opts) + assert ("ArrayAssignment2LoopsTrans cannot expand expression because it " + "contains the access 'ishtsi(map,scalar)' which is an " + "UnresolvedType and therefore cannot be guaranteed to be " + "ScalarType" in str(info.value)) + # The end result should look like: assert ( " do idx = LBOUND(x, dim=1), UBOUND(x, dim=1), 1\n" @@ -625,15 +669,19 @@ def test_validate_rhs_plain_references(fortran_reader, fortran_writer): " x(idx_1) = array(idx_1)\n" " enddo\n\n" " ! ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unresolved' which is not a DataSymbol and " + "contains the access 'unresolved' which is not a DataSymbol and " "therefore cannot be guaranteed to be ScalarType. Resolving the import" " that brings this variable into scope may help.\n" " x(:) = unresolved\n\n" " ! ArrayAssignment2LoopsTrans cannot expand expression because it " - "contains the variable 'unsupported' which is an UnsupportedFortran" + "contains the access 'unsupported' which is an UnsupportedFortran" "Type('INTEGER, DIMENSION(:), OPTIONAL :: unsupported') and therefore " "cannot be guaranteed to be ScalarType.\n" - " x(:) = unsupported\n" + " x(:) = unsupported\n\n" + " ! ArrayAssignment2LoopsTrans cannot expand expression because it " + "contains the access 'ishtsi(map,scalar)' which is an UnresolvedType " + "and therefore cannot be guaranteed to be ScalarType.\n" + " x(:) = ishtsi(map,scalar)\n" ) in fortran_writer(psyir) @@ -685,7 +733,7 @@ def test_validate_non_elemental_functions(fortran_reader): with pytest.raises(TransformationError) as err: trans.validate(assignment3, options={"verbose": True}) errormsg = ("ArrayAssignment2LoopsTrans cannot expand expression because " - "it contains the variable 'myfunc' which is not a DataSymbol " + "it contains the access 'myfunc(y)' which is not a DataSymbol " "and therefore cannot be guaranteed to be ScalarType. " "Resolving the import that brings this variable into scope " "may help.") @@ -695,9 +743,95 @@ def test_validate_non_elemental_functions(fortran_reader): with pytest.raises(TransformationError) as err: trans.validate(assignment4, options={"verbose": True}) errormsg = ("ArrayAssignment2LoopsTrans cannot expand expression because " - "it contains the variable 'var' which is an UnresolvedType " - "and therefore cannot be guaranteed to be ScalarType. " - "Resolving the import that brings this variable into scope " - "may help.") + "it contains the access 'var%myfunc(y)' which is an " + "UnresolvedType and therefore cannot be guaranteed to be " + "ScalarType. Resolving the import that brings this variable " + "into scope may help.") assert errormsg in assignment4.preceding_comment assert errormsg in str(err.value) + + +def test_validate_indirect_indexing(fortran_reader): + ''' + Check the validation of the transformation when the array is indexed + indirectly in unsupported ways. + + ''' + psyir = fortran_reader.psyir_from_source(''' + program test + use dom_oce + INTEGER, DIMENSION(4) :: iwewe + INTEGER, DIMENSION(8,kfld) :: ishtSi + ! Assignment with CodeBlock on RHS. + iwewe(:) = (/ jpwe,jpea,jpwe,jpea /) + ! Assignment with CodeBlock in array-index expression. + iwewe(:) = ishtSi((/ jpwe,jpea,jpwe,jpea /), 1) + ! Index expression does evaluate to an array but we don't currently + ! handle getting a type in this case (TODO #1799). + ishtSi(5:8,jf) = ishtSi(iwewe+1, jf) + ! Index expression contains a call to an unknown function. + ishtSi(5:8,jf) = ishtSi(my_func(1), jf) + end program test + ''') + assignments = psyir.walk(Assignment) + assert isinstance(assignments[0].rhs, CodeBlock) + trans = ArrayAssignment2LoopsTrans() + with pytest.raises(TransformationError) as err: + trans.validate(assignments[0]) + assert ("ArrayAssignment2LoopsTrans does not support array assignments " + "that contain a CodeBlock anywhere in the expression" + in str(err.value)) + with pytest.raises(TransformationError) as err: + trans.validate(assignments[1]) + assert ("ArrayAssignment2LoopsTrans does not support array assignments " + "that contain a CodeBlock anywhere in the expression" + in str(err.value)) + # TODO #1799 - this requires that we extended ArrayMixin.datatype to + # support array accesses with index expressions that are Operations. + with pytest.raises(TransformationError) as err: + trans.validate(assignments[2]) + assert ("cannot expand expression because it contains the access " + "'ishtsi(iwewe + 1,jf)' which is an UnresolvedType and therefore " + "cannot be guaranteed" in str(err.value)) + with pytest.raises(TransformationError) as err: + trans.validate(assignments[3]) + assert ("cannot expand expression because it contains the access " + "'ishtsi(my_func(1),jf)' which is an UnresolvedType and therefore " + "cannot be guaranteed to be ScalarType." in str(err.value)) + + +def test_validate_structure(fortran_reader): + ''' + Check the validation of the transformation when it is a structure access + on the RHS. + + ''' + psyir = fortran_reader.psyir_from_source(''' + program test + integer, parameter :: ngrids = 4, kfld=5 + integer, dimension(8,kfld) :: ishtSi + type :: sub_grid_type + integer, dimension(kfld,kfld) :: map + end type sub_grid_type + type :: grid_type + real, dimension(:,:), allocatable :: data + type(sub_grid_type), dimension(ngrids) :: subgrid + end type grid_type + type(grid_type) :: grid + integer :: jf + ! Cannot tell whether or not the access on the RHS is an array. + ishtSi(5:8,jf) = grid%data(my_func(1), jf) + ! The array access to subgrid is not yet supported. + ishtSi(5:8,jf) = grid%subgrid%map(1,1) + end program test + ''') + assignments = psyir.walk(Assignment) + trans = ArrayAssignment2LoopsTrans() + with pytest.raises(TransformationError) as err: + trans.validate(assignments[0]) + assert ("contains the access 'grid%data(my_func(1),jf)' which is an " + "UnresolvedType" in str(err.value)) + # TODO #1858 - once we've extended Reference2ArrayRangeTrans to support + # StructureMembers we can use it as part of this transformation and this + # example will be supported. + trans.validate(assignments[1]) diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index 35748414aa..e7efd1c172 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -833,10 +833,10 @@ def test_apply_struct_array_slice_arg(fortran_reader, fortran_writer, tmpdir): f" grid%data2d(:,:) = 1.0\n" f" do i=1,10\n" f" a(i) = 1.0\n" - f" call sub(grid%local%data)\n" f" call sub(micah%grids(3)%region%data(:))\n" f" call sub(grid%data2d(:,i))\n" f" call sub(grid%data2d(1:5,i))\n" + f" call sub(grid%local%data)\n" f" end do\n" f" end subroutine run_it\n" f" subroutine sub(x)\n" @@ -854,35 +854,42 @@ def test_apply_struct_array_slice_arg(fortran_reader, fortran_writer, tmpdir): inline_trans = InlineTrans() for call in psyir.walk(Call): if not isinstance(call, IntrinsicCall): + if call.arguments[0].debug_string() == "grid%local%data": + # TODO #1858: this if construct can be removed once we + # support getting the type of `grid%local%data`. + continue inline_trans.apply(call) output = fortran_writer(psyir) assert (" do i = 1, 10, 1\n" " a(i) = 1.0\n" " do ji = 1, 5, 1\n" - " grid%local%data(ji) = 2.0 * grid%local%data(ji)\n" - " enddo\n" - " grid%local%data(1:2) = 0.0\n" - " grid%local%data(:) = 3.0\n" - " grid%local%data = 5.0\n" - " do ji_1 = 1, 5, 1\n" - " micah%grids(3)%region%data(ji_1) = 2.0 * " - "micah%grids(3)%region%data(ji_1)\n" + " micah%grids(3)%region%data(ji) = 2.0 * " + "micah%grids(3)%region%data(ji)\n" " enddo\n" " micah%grids(3)%region%data(1:2) = 0.0\n" " micah%grids(3)%region%data(:) = 3.0\n" " micah%grids(3)%region%data(:) = 5.0\n" - " do ji_2 = 1, 5, 1\n" - " grid%data2d(ji_2,i) = 2.0 * grid%data2d(ji_2,i)\n" + " do ji_1 = 1, 5, 1\n" + " grid%data2d(ji_1,i) = 2.0 * grid%data2d(ji_1,i)\n" " enddo\n" " grid%data2d(1:2,i) = 0.0\n" " grid%data2d(:,i) = 3.0\n" " grid%data2d(:,i) = 5.0\n" - " do ji_3 = 1, 5, 1\n" - " grid%data2d(ji_3,i) = 2.0 * grid%data2d(ji_3,i)\n" + " do ji_2 = 1, 5, 1\n" + " grid%data2d(ji_2,i) = 2.0 * grid%data2d(ji_2,i)\n" " enddo\n" " grid%data2d(1:2,i) = 0.0\n" " grid%data2d(1:5,i) = 3.0\n" " grid%data2d(1:5,i) = 5.0\n" + # TODO #1858: replace the following line with the commented-out + # lines below. + " call sub(grid%local%data)\n" + # " do ji_3 = 1, 5, 1\n" + # " grid%local%data(ji_3) = 2.0 * grid%local%data(ji_3)\n" + # " enddo\n" + # " grid%local%data(1:2) = 0.0\n" + # " grid%local%data(:) = 3.0\n" + # " grid%local%data = 5.0\n" " enddo\n" in output) assert Compile(tmpdir).string_compiles(output) diff --git a/src/psyclone/tests/psyir/transformations/loop_fusion_test.py b/src/psyclone/tests/psyir/transformations/loop_fusion_test.py index d139bdfe95..cc73abbdb8 100644 --- a/src/psyclone/tests/psyir/transformations/loop_fusion_test.py +++ b/src/psyclone/tests/psyir/transformations/loop_fusion_test.py @@ -39,6 +39,7 @@ '''This module tests the loop fusion transformation. ''' +from unittest import mock import pytest from psyclone.psyir.nodes import Literal, Loop, Schedule, Return @@ -144,6 +145,36 @@ def fuse_loops(fortran_code, fortran_reader, fortran_writer): return fortran_writer(psyir), psyir +# ---------------------------------------------------------------------------- +def test_fuse_dependency_tools_called(fortran_reader): + '''Make sure that `DependencyTools.can_loops_be_fused` is indeed called + from the loop fuse transformation with the right parameters. + ''' + + # First test: read/read of scalar variable + code = '''subroutine sub() + integer :: jj, n + real, dimension(10) :: s, t + real :: a + do jj=1, n + s(jj) = t(jj) + a + enddo + do jj=1, n + t(jj) = t(jj) - a + enddo + end subroutine sub''' + psyir = fortran_reader.psyir_from_source(code) + # Get the PSyIR of the original loops + loop1 = psyir.children[0].children[0] + loop2 = psyir.children[0].children[1] + + fuse = LoopFuseTrans() + with mock.patch("psyclone.psyir.tools.dependency_tools.DependencyTools." + "can_loops_be_fused", result=True) as dep_fuse_check: + fuse.apply(loop1, loop2) + dep_fuse_check.assert_called_once_with(loop1, loop2) + + # ---------------------------------------------------------------------------- def test_fuse_ok(tmpdir, fortran_reader, fortran_writer): '''This tests verifies that loop fusion can be successfully applied to @@ -329,14 +360,15 @@ def test_fuse_correct_bounds(tmpdir, fortran_reader, fortran_writer): # ---------------------------------------------------------------------------- -def test_fuse_dimension_change(tmpdir, fortran_reader, fortran_writer): - '''Test that inconsistent use of dimemsions are detected, e.g.: - loop1: a(i,j) - loop2: a(j,i) - when at least one operation is a write +def test_fuse_dimension_change_for_read(tmpdir, fortran_reader, + fortran_writer): + '''Test that inconsistent use of dimensions for variables that are read + only are accepted. The failure cases are all tested in the + DependencyTools, the success is here in order to easily allow compilation + tests, and also check again that the output of loop fuse is as expected. ''' - # The first example can be merged, since 't' is read-only, + # This example can be merged, since 't' is read-only, # so it doesn't matter that it is accessed differently code = '''subroutine sub() integer :: ji, jj, n @@ -366,114 +398,13 @@ def test_fuse_dimension_change(tmpdir, fortran_reader, fortran_writer): assert correct in out assert Compile(tmpdir).string_compiles(out) - # This cannot be fused, since 's' is written in the - # first iteration and read in the second. - code = '''subroutine sub() - integer :: ji, jj, n - integer, dimension(10,10) :: s, t, u - do jj=1, n+1 - do ji=1, 10 - s(ji, jj)=t(ji, jj)+1 - enddo - enddo - do jj=1, n+1 - do ji=1, 10 - u(ji, jj)=s(jj, ji)+1 - enddo - enddo - end subroutine sub''' - - with pytest.raises(TransformationError) as err: - fuse_loops(code, fortran_reader, fortran_writer) - assert ("Variable 's' is written to and the " - "loop variable 'jj' is used in different index locations: " - "s(ji,jj) and s(jj,ji)." - in str(err.value)) - - # This cannot be fused, since 's' is read in the - # first iteration and written in the second with - # different indices. - code = '''subroutine sub() - integer :: ji, jj, n - integer, dimension(10,10) :: s, t, u - do jj=1, n+1 - do ji=1, 10 - u(ji, jj)=s(jj, ji)+1 - enddo - enddo - do jj=1, n+1 - do ji=1, 10 - s(ji, jj)=t(ji, jj)+1 - enddo - enddo - end subroutine sub''' - - with pytest.raises(TransformationError) as err: - fuse_loops(code, fortran_reader, fortran_writer) - assert ("Variable 's' is written to and the loop variable 'jj' is " - "used in different index locations: s(jj,ji) and s(ji,jj)." - in str(err.value)) - - # Same test using a structure type: - code = '''subroutine sub() - use my_module - integer :: ji, jj, n - type(my_type) :: s, t, u - do jj=1, n+1 - do ji=1, 10 - u%comp1(ji)%comp2(jj)=s%comp1(jj)%comp2(ji)+1 - enddo - enddo - do jj=1, n+1 - do ji=1, 10 - s%comp1(ji)%comp2(jj)=t%comp1(ji)%comp2(jj)+1 - enddo - enddo - end subroutine sub''' - - with pytest.raises(TransformationError) as err: - fuse_loops(code, fortran_reader, fortran_writer) - assert ("Variable 's' is written to and the loop variable 'jj' is used " - "in different index locations: s%comp1(jj)%comp2(ji) and " - "s%comp1(ji)%comp2(jj)." - in str(err.value)) - - -# ---------------------------------------------------------------------------- -def test_fuse_independent_array(fortran_reader, fortran_writer): - '''Test that using arrays which are not dependent on the loop variable - are handled correctly. Example: - do j ... a(1) = b(j) * c(j) - do j ... d(j) = a(1) - ''' - - # The first example can be merged, since 's' does not - # depend on the loop variable, and it is written and read. - code = '''subroutine sub() - integer :: ji, jj, n - integer, dimension(10,10) :: s, t - do jj=1, n - do ji=1, 10 - s(1, 1)=t(ji, jj)+1 - enddo - enddo - do jj=1, n - do ji=1, 10 - t(ji, jj) = s(1, 1) + t(ji, jj) - enddo - enddo - end subroutine sub''' - - with pytest.raises(TransformationError) as err: - fuse_loops(code, fortran_reader, fortran_writer) - assert ("Variable 's' does not depend on loop variable 'jj', but is " - "read and written" in str(err.value)) - # ---------------------------------------------------------------------------- def test_fuse_scalars(tmpdir, fortran_reader, fortran_writer): '''Test that using scalars work as expected in all combinations of - being read/written in both loops. + being read/written in both loops. Note that the dependency tools check + more cases, this test is only here to check compilation of the fused + loops. ''' # First test: read/read of scalar variable @@ -495,53 +426,7 @@ def test_fuse_scalars(tmpdir, fortran_reader, fortran_writer): out, _ = fuse_loops(code, fortran_reader, fortran_writer) assert Compile(tmpdir).string_compiles(out) - # Second test: read/write of scalar variable - code = '''subroutine sub() - integer :: ji, jj, n - real, dimension(10,10) :: s, t - real :: a - do jj=1, n - do ji=1, 10 - s(ji, jj)=t(ji, jj)+a - enddo - enddo - do jj=1, n - do ji=1, 10 - a = t(ji, jj) - 2 - s(ji, jj)=t(ji, jj)+a - enddo - enddo - end subroutine sub''' - - with pytest.raises(TransformationError) as err: - fuse_loops(code, fortran_reader, fortran_writer) - assert ("Scalar variable 'a' is written in one loop, but only read in " - "the other loop." in str(err.value)) - - # Third test: write/read of scalar variable - code = '''subroutine sub() - integer :: ji, jj, n - real, dimension(10,10) :: s, t - real :: b - do jj=1, n - do ji=1, 10 - b = t(ji, jj) - 2 - s(ji, jj )=t(ji, jj)+b - enddo - enddo - do jj=1, n - do ji=1, 10 - s(ji, jj)=t(ji, jj)+b - enddo - enddo - end subroutine sub''' - - with pytest.raises(TransformationError) as err: - fuse_loops(code, fortran_reader, fortran_writer) - assert "Scalar variable 'b' is written in one loop, but only read in " \ - "the other loop." in str(err.value) - - # Fourth test: write/write of scalar variable - this is ok + # Second test: write/write of scalar variable - this is ok code = '''subroutine sub() integer :: ji, jj, n real, dimension(10,10) :: s, t @@ -744,7 +629,7 @@ def test_loop_fuse_different_variables(fortran_reader, fortran_writer): s(ji, jj) = t(ji, jj) + 1 end do do jk = 1, 10 - s(jk, jj) = t(jk, jj) + 1 + s(jk, jj) = t(jk, jj) - 1 end do end do end subroutine sub''' @@ -764,13 +649,23 @@ def test_loop_fuse_different_variables(fortran_reader, fortran_writer): do jj = 1, n, 1 do ji = 1, 10, 1 s(ji,jj) = t(ji,jj) + 1 - s(ji,jj) = t(ji,jj) + 1 + s(ji,jj) = t(ji,jj) - 1 enddo enddo end subroutine sub''' assert correct in out + # Now try to provide the loops in the wrong order. Recreate the PSyIR + # by re-parsing the original code + psyir = fortran_reader.psyir_from_source(code) + loops = psyir.children[0].walk(Loop) + fuse = LoopFuseTrans() + with pytest.raises(TransformationError) as err: + fuse.apply(loops[2], loops[1]) + assert ("Error in LoopFuseTrans transformation. The second loop does not " + "immediately follow the first loop" in str(err.value)) + def test_loop_fuse_different_variables_with_access(fortran_reader): '''Test that fusing loops with different variables is disallowed when @@ -793,7 +688,7 @@ def test_loop_fuse_different_variables_with_access(fortran_reader): fuse = LoopFuseTrans() with pytest.raises(TransformationError) as excinfo: fuse.apply(loops[1], loops[2]) - assert ("Error in LoopFuseTrans transformation. Second loop contains " + assert ("LoopFuseTrans. Error: Second loop contains " "accesses to the first loop's variable: ji." in str(excinfo.value)) code = '''subroutine sub() @@ -814,6 +709,6 @@ def test_loop_fuse_different_variables_with_access(fortran_reader): fuse = LoopFuseTrans() with pytest.raises(TransformationError) as excinfo: fuse.apply(loops[1], loops[2]) - assert ("Error in LoopFuseTrans transformation. First loop contains " + assert ("LoopFuseTrans. Error: First loop contains " "accesses to the second loop's variable: jk." in str(excinfo.value)) diff --git a/src/psyclone/tests/test_files/dynamo0p3/1.14_single_invoke_dofs.f90 b/src/psyclone/tests/test_files/dynamo0p3/1.14_single_invoke_dofs.f90 index 4a8174e6b8..5df8f8cf13 100755 --- a/src/psyclone/tests/test_files/dynamo0p3/1.14_single_invoke_dofs.f90 +++ b/src/psyclone/tests/test_files/dynamo0p3/1.14_single_invoke_dofs.f90 @@ -32,20 +32,23 @@ ! POSSIBILITY OF SUCH DAMAGE. ! ----------------------------------------------------------------------------- ! Author I. Kavcic, Met Office +! Modified O. Brunt, Met Office program single_invoke_dofs ! Description: single user-defined kernel specified in an invoke call that - ! iterates over DoFs (currently not supported) + ! iterates over DoFs + use constants_mod, only: r_def use field_mod, only: field_type use testkern_dofs_mod, only: testkern_dofs_type implicit none type(field_type) :: f1, f2, f3, f4 + real(kind=r_def) :: scalar_arg call invoke( & - testkern_dofs_type(f1, f2, f3, f4) & + testkern_dofs_type(f1, f2, f3, f4, scalar_arg) & ) end program single_invoke_dofs diff --git a/src/psyclone/tests/test_files/dynamo0p3/4.17_multikernel_invokes_cell_dof_builtin.f90 b/src/psyclone/tests/test_files/dynamo0p3/4.17_multikernel_invokes_cell_dof_builtin.f90 new file mode 100644 index 0000000000..389a0101b8 --- /dev/null +++ b/src/psyclone/tests/test_files/dynamo0p3/4.17_multikernel_invokes_cell_dof_builtin.f90 @@ -0,0 +1,57 @@ +! ----------------------------------------------------------------------------- +! BSD 3-Clause License +! +! Copyright (c) 2024, Science and Technology Facilities Council +! All rights reserved. +! +! Redistribution and use in source and binary forms, with or without +! modification, are permitted provided that the following conditions are met: +! +! * Redistributions of source code must retain the above copyright notice, this +! list of conditions and the following disclaimer. +! +! * Redistributions in binary form must reproduce the above copyright notice, +! this list of conditions and the following disclaimer in the documentation +! and/or other materials provided with the distribution. +! +! * Neither the name of the copyright holder nor the names of its +! contributors may be used to endorse or promote products derived from +! this software without specific prior written permission. +! +! THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +! "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +! LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +! FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +! COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +! INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +! BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +! LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +! CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +! LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN +! ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE +! POSSIBILITY OF SUCH DAMAGE. +! ----------------------------------------------------------------------------- +! Author O. Brunt, Met Office + +program multikernel_invokes_cell_dof_builtin + + ! Description: Two user-defined kernels operating over different domains + ! ("cell-column" and "dof"), followed by an LFRic built-in kernel in the + ! same invoke. + use constants_mod, only: r_def + use field_mod, only: field_type + use testkern_dofs_mod, only: testkern_dofs_type + use testkern_mod, only: testkern_type + + implicit none + + type(field_type) :: f1, f2, f3, f4, m1, m2 + real(kind=r_def) :: a, scalar_arg + + call invoke( & + testkern_dofs_type(f1, f2, f3, f4, scalar_arg), & + testkern_type(a, f1, f2, m1, m2), & + inc_aX_plus_Y(0.5_r_def, f1, f2) & + ) + +end program multikernel_invokes_cell_dof_builtin \ No newline at end of file diff --git a/src/psyclone/tests/test_files/dynamo0p3/testkern_dofs_mod.f90 b/src/psyclone/tests/test_files/dynamo0p3/testkern_dofs_mod.f90 index 0a4e34ecc8..ac1488838c 100644 --- a/src/psyclone/tests/test_files/dynamo0p3/testkern_dofs_mod.f90 +++ b/src/psyclone/tests/test_files/dynamo0p3/testkern_dofs_mod.f90 @@ -30,7 +30,7 @@ ! OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. ! ----------------------------------------------------------------------------- ! Author R. W. Ford, STFC Daresbury Lab -! Modified I. Kavcic, Met Office +! Modified I. Kavcic, O. Brunt, Met Office module testkern_dofs_mod @@ -41,13 +41,13 @@ module testkern_dofs_mod implicit none - ! User-defined single kernel that operates on DoFs (currently not supported) type, extends(kernel_type) :: testkern_dofs_type - type(arg_type), dimension(4) :: meta_args = & + type(arg_type), dimension(5) :: meta_args = & (/ arg_type(gh_field, gh_real, gh_write, w1), & arg_type(gh_field, gh_real, gh_read, w1), & arg_type(gh_field, gh_real, gh_read, w1), & - arg_type(gh_field, gh_real, gh_read, w1) & + arg_type(gh_field, gh_real, gh_read, w1), & + arg_type(gh_scalar, gh_real, gh_read) & /) integer :: operates_on = DOF contains @@ -56,7 +56,13 @@ module testkern_dofs_mod contains - subroutine testkern_dofs_code(a, b, c, d) + subroutine testkern_dofs_code(a, b, c, d, scalar_arg) + implicit none + + real(kind=r_def), intent(inout) :: a + real(kind=r_def), intent(in) :: b, c, d + real(kind=r_def), intent(in) :: scalar_arg + end subroutine testkern_dofs_code end module testkern_dofs_mod diff --git a/tutorial/practicals/nemo/2_nemo_profiling/Makefile b/tutorial/practicals/nemo/2_nemo_profiling/Makefile index ff957684ee..a8a59854e5 100644 --- a/tutorial/practicals/nemo/2_nemo_profiling/Makefile +++ b/tutorial/practicals/nemo/2_nemo_profiling/Makefile @@ -108,7 +108,7 @@ transform: -o output_3.f90 -l output tra_adv_mod.F90 compile: transform $(KERNELS) output.o solutions/runner.o - $(F90) $(KERNELS) output.o solutions/runner.o -o $(NAME) \ + $(F90) $(F90FLAGS) $(KERNELS) output.o solutions/runner.o -o $(NAME) \ $(PROFILE_WRAPPER_LINK) $(PROFILE_LINK) # Only used for the compile CI target to compile the solution file