diff --git a/changelog b/changelog index fe5ec70972..0811c1773e 100644 --- a/changelog +++ b/changelog @@ -32,6 +32,21 @@ 11) PR #2814 for #2704. Adds backward accesses capabilities to the DefinitionUseChain tool. + 12) PR #2509 for #2394. Adds support for logical and character types + to the PSyData tooling. + + 13) PR #2862 for #2858. Change to PSyIR frontend and backend to ensure + existing parentheses in expressions are preserved. + + 14) PR #2825 for #2278 and #2849. Signatures AccessType now indentifies + INQUIRY and TYPE_INFO, for inquiry intrinsic accesses and precision symbols. + + 15) PR #2855 for #2853. Allows the Fortran standard used by fparser to + be set in the config file and ensure it is used consistently. + + 16) PR #2857 for 2854. Excludes character assignments from ACC KERNELS + regions to avoid issues in NEMO. + release 3.0.0 6th of December 2024 1) PR #2477 for #2463. Add support for Fortran Namelist statements. diff --git a/config/psyclone.cfg b/config/psyclone.cfg index 72394bbf53..45c2090912 100644 --- a/config/psyclone.cfg +++ b/config/psyclone.cfg @@ -56,6 +56,10 @@ OCL_DEVICES_PER_NODE = 1 # and will not produce a warning message if they cannot be found. IGNORE_MODULES = netcdf, mpi +# The Fortran standard to use when parsing files. This value is passed +# to fparser. Valid values are f2003 and f2008 +FORTRAN_STANDARD = f2008 + # Settings specific to the LFRic API # ================================== [lfric] diff --git a/doc/developer_guide/dependency.rst b/doc/developer_guide/dependency.rst index b4974f6a9d..eec7dcd676 100644 --- a/doc/developer_guide/dependency.rst +++ b/doc/developer_guide/dependency.rst @@ -263,16 +263,17 @@ DataAccess class i.e. the `_field_write_arguments()` and Variable Accesses ================= -Especially in the NEMO API, it is not possible to rely on pre-defined -kernel information to determine dependencies between loops. So an additional, -somewhat lower-level API has been implemented that can be used to determine -variable accesses (READ, WRITE etc.), which is based on the PSyIR information. -The only exception to this is if a kernel is called, in which case the -metadata for the kernel declaration will be used to determine the variable -accesses for the call statement. The information about all variable usage -of a PSyIR node or a list of nodes can be gathered by creating an object of -type `psyclone.core.VariablesAccessInfo`. -This class uses a `Signature` object to keep track of the variables used. +When using PSyclone with generic Fortran code, it is not possible to +rely on pre-defined kernel information to determine dependencies +between loops. So an additional, somewhat lower-level API has been +implemented that can be used to determine variable accesses (READ, +WRITE etc.), which is based on the PSyIR information. The only +exception to this is if a kernel is called, in which case the metadata +for the kernel declaration will be used to determine the variable +accesses for the call statement. The information about all variable +usage of a PSyIR node or a list of nodes can be gathered by creating +an object of type `psyclone.core.VariablesAccessInfo`. This class +uses a `Signature` object to keep track of the variables used. Signature --------- @@ -289,6 +290,14 @@ a single component. :members: :special-members: __hash__, __eq__, __lt__ +AccessType +---------- + +An individual access to a ``Signature`` is described by an instance of the +``AccessType`` enumeration: + +.. autoclass:: psyclone.core.access_type.AccessType + :members: VariablesAccessInfo ------------------- @@ -319,22 +328,7 @@ instance is holding information about. VariablesAccessInfo Options +++++++++++++++++++++++++++ -By default, `VariablesAccessInfo` will not report the first argument of -the PSyIR operators `lbound`, `ubound`, or `size` as read accesses, -since these functions do not actually access the content of the array, -they only query the size. If these accesses are required (e.g. in kernel -extraction this could be important if an array is only used in these -intrinsic - a driver would still need these arrays in order to query -the size), the optional `options` parameter of the `VariablesAccessInfo` -constructor can be used: add the key -`COLLECT-ARRAY-SHAPE-READS` and set it to true:: - - vai = VariablesAccessInfo(options={'COLLECT-ARRAY-SHAPE-READS': True}) - -In this case all arrays specified as first parameter to one of the -PSyIR operators above will be reported as read access. - -Fortran also allows to rename a symbol locally when it is being imported, +Fortran allows an imported symbol to be renamed locally (`use some_mod, only: renamed => original_name`). Depending on use case, it might be useful to get the non-local, original name. By default, `VariablesAccessInfo` will report the local name (i.e. the renamed name), diff --git a/doc/user_guide/configuration.rst b/doc/user_guide/configuration.rst index d4b3cc92be..396ebf5bea 100644 --- a/doc/user_guide/configuration.rst +++ b/doc/user_guide/configuration.rst @@ -98,6 +98,7 @@ section e.g.: REPROD_PAD_SIZE = 8 PSYIR_ROOT_NAME = psyir_tmp VALID_PSY_DATA_PREFIXES = profile, extract + FORTRAN_STANDARD = f2008 and an optional API specific section, for example for the ``lfric`` section: @@ -176,6 +177,9 @@ BACKEND_CHECKS_ENABLED Optional (defaults to True). Whether or not the PSyIR backend should validate the tree that it is passed. Can be overridden by the ``--backend`` command-line flag (see :ref:`backend-options`). +FORTRAN_STANDARD Optional (defaults to f2008). The Fortran standard str + that should be used by fparser. Valid values are + f2003 and f2008. ======================= ======================================================= =========== Common Sections diff --git a/doc/user_guide/libraries.rst b/doc/user_guide/libraries.rst index ea6ad8decc..fc0af07317 100644 --- a/doc/user_guide/libraries.rst +++ b/doc/user_guide/libraries.rst @@ -99,8 +99,9 @@ Read-only libraries check that a field declared as read-only is not modified during a kernel call. More information can be found in the :ref:`Read-Only Verification ` section. -The libraries for :ref:`LFRic ` and -:ref:`GOcean ` APIs are included with PSyclone in +The libraries for :ref:`LFRic `, +:ref:`GOcean ` APIs and generic Fortran code +are included with PSyclone in the ``lib/read_only`` `directory `__. For detailed instructions on how to build and use these libraries diff --git a/doc/user_guide/psy_data.rst b/doc/user_guide/psy_data.rst index eaf4b496b6..eb87e55f5f 100644 --- a/doc/user_guide/psy_data.rst +++ b/doc/user_guide/psy_data.rst @@ -120,6 +120,7 @@ be printed at runtime, e.g.:: The transformation that adds read-only-verification to an application can be applied for both the :ref:`LFRic ` and :ref:`GOcean API ` - no API-specific transformations are required. +It can also be used to verify existing Fortran code. Below is an example that searches for each loop in a PSyKAl invoke code (which will always surround kernel calls) and applies the transformation to each one. This code has been successfully used as a global transformation with the LFRic @@ -136,10 +137,10 @@ Gravity Wave application (the executable is named ``gravity_wave``) read_only_verify.apply(loop) Besides the transformation, a library is required to do the actual -verification at runtime. There are two implementations of the +verification at runtime. There are three implementations of the read-only-verification library included in PSyclone: one for LFRic, -and one for GOcean. -Both libraries support the environment variable ``PSYDATA_VERBOSE``. +one for GOcean, and one for generic Fortran code. +These libraries support the environment variable ``PSYDATA_VERBOSE``. This can be used to control how much output is generated by the read-only-verification library at runtime. If the variable is not specified or has the value '0', warnings will only @@ -230,6 +231,25 @@ An executable example for using the GOcean read-only-verification library is included in ``examples/gocean/eg5/readonly``, see :ref:`gocean_example_readonly`. + +Read-Only-Verification Library for Generic Fortran +++++++++++++++++++++++++++++++++++++++++++++++++++ + +This library is contained in the ``lib/read_only/generic`` directory and +it must be compiled before linking any existing Fortran code that uses +read-only verification. + +Compilation of the library is done by invoking ``make`` and setting +the required variables: + +.. code-block:: shell + + make F90=ifort F90FLAGS="--some-flag" + +This will create a library called ``lib_read_only.a``. +An executable example for using the generic read-only-verification +library is included in ``examples/nemo/eg6/``. + .. _psydata_value_range_check: Value Range Check diff --git a/doc/user_guide/tutorials_and_examples.rst b/doc/user_guide/tutorials_and_examples.rst index c7f836e32f..6b6ff19b4e 100644 --- a/doc/user_guide/tutorials_and_examples.rst +++ b/doc/user_guide/tutorials_and_examples.rst @@ -783,6 +783,14 @@ compilation instructions are in the ``README.md`` file, including how to switch from using the stand-alone extraction library to the NetCDF-based one (see :ref:`extraction_libraries` for details). +Example 6: Read-only Verification +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This example shows the use of read-only verification with PSyclone for +generic Fortran code. It instruments each kernel in a small Fortran +program with the PSyData-based read-only verification code. Detailed +compilation instructions are in the ``README.md`` file. + Scripts ^^^^^^^ diff --git a/examples/nemo/README.md b/examples/nemo/README.md index 6f4717af5c..eaef163053 100644 --- a/examples/nemo/README.md +++ b/examples/nemo/README.md @@ -140,3 +140,9 @@ benchmark. Extraction of kernel data. Using the tra_adv benchmark, this example shows the extraction of kernel input- and output-data. + +## Example 6 + +A simple stand-alone example that shows verification that read-only data +is not modified, e.g. by out-of-bounds accesses to other variables. +This uses the PSyData interface to instrument generic Fortran code. diff --git a/examples/nemo/eg5/Makefile b/examples/nemo/eg5/Makefile index 422a8e197f..c62c147a99 100644 --- a/examples/nemo/eg5/Makefile +++ b/examples/nemo/eg5/Makefile @@ -95,7 +95,7 @@ kernels: extract_kernels.py $(PSYCLONE) -l all -s ./extract_kernels.py -o psy.f90 ../code/tra_adv.F90 $(EXTRACT_DIR)/$(LIB_NAME): - make -C $(EXTRACT_DIR) + ${MAKE} -C $(EXTRACT_DIR) # Compilation uses the 'kernels' transformed code psy.f90: kernels @@ -103,3 +103,6 @@ psy.o: $(EXTRACT_DIR)/$(LIB_NAME) %.o: %.f90 $(F90) $(F90FLAGS) -c $< + +allclean: clean + ${MAKE} -C ${EXTRACT_DIR} allclean diff --git a/examples/nemo/eg5/extract_kernels.py b/examples/nemo/eg5/extract_kernels.py index 7639104f01..142452a5c6 100755 --- a/examples/nemo/eg5/extract_kernels.py +++ b/examples/nemo/eg5/extract_kernels.py @@ -88,16 +88,7 @@ def trans(psyir): if not isinstance(kern, Loop): continue try: - # TODO #2080: once this is fixed, the option can be removed - # The example contains array expressions, e.g.: - # zwx(:,:,jpk) = 0.e0 - # PSyclone represents this internally using Range with LBOUND - # and UBOUND intrinsics and currently this results in several - # occurrences of zws on the left hand side, which will trigger - # an exception in the dependency analysis. Therefore, disable - # the collection of read accesses for the shape of an array. - extract.apply(kern, - options={"COLLECT-ARRAY-SHAPE-READS": False}) + extract.apply(kern) except TransformationError as err: # Typically that's caused by a kernel having a CodeBlock # inside. diff --git a/examples/nemo/eg6/Makefile b/examples/nemo/eg6/Makefile new file mode 100644 index 0000000000..7468ec56bd --- /dev/null +++ b/examples/nemo/eg6/Makefile @@ -0,0 +1,86 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2023-2025, 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: A. R. Porter, STFC Daresbury Lab +# Modified J. Henrichs, Bureau of Meteorology + +# This example uses PSyclone (which must be installed) to generate Fortran for +# the tracer-advection benchmark. The script provided here instruments code to +# check that read-only variables are not modified. + +# The compiler to use may be specified via the F90 environment variable, +# it defaults to: +# export F90=gfortran +# export F90FLAGS="-g -O0" + +include ../../common.mk + +TYPE ?= standalone + +GENERATED_FILES += psy.f90 psy.o dummy + +PSYROOT=../../.. +READ_ONLY_CHECK_DIR ?= $(PSYROOT)/lib/read_only/generic + +F90FLAGS += -I$(READ_ONLY_CHECK_DIR) +LIB_NAME = lib_read_only.a + +.PHONY: allclean + +compile: dummy + +run: compile + ./dummy + +dummy: psy.o $(READ_ONLY_CHECK_DIR)/$(LIB_NAME) + $(F90) $(F90FLAGS) psy.o -o dummy $(READ_ONLY_CHECK_DIR)/$(LIB_NAME) $(LDFLAGS) + +transform: kernels + +# Need `-l all` to ensure line-lengths in generated code are less than the +# standard-mandated 132 chars. +kernels: read_only_check.py + $(PSYCLONE) -l all -s ./read_only_check.py -o psy.f90 dummy.f90 + +$(READ_ONLY_CHECK_DIR)/$(LIB_NAME): + $(MAKE) -C $(READ_ONLY_CHECK_DIR) + +# Compilation uses the 'kernels' transformed code +psy.f90: dummy.f90 read_only_check.py kernels +psy.o: $(READ_ONLY_CHECK_DIR)/$(LIB_NAME) + +%.o: %.f90 + $(F90) $(F90FLAGS) -c $< + +allclean: clean + ${MAKE} -C $(READ_ONLY_CHECK_DIR) allclean diff --git a/examples/nemo/eg6/README.md b/examples/nemo/eg6/README.md new file mode 100644 index 0000000000..a0a72e1def --- /dev/null +++ b/examples/nemo/eg6/README.md @@ -0,0 +1,100 @@ +# PSyclone NEMO Example 6 - Read-only Verification + +**Author:** J. Henrichs, Bureau of Meteorology + +This example demonstrates the use of PSyclone to add code that checks +if variables that are only read are actually modified (e.g. because +of memory overwrite). + +Once you have installed PSyclone, you can transform the file using: + +```sh +psyclone -s ./read_only_check.py dummy.f90 +``` + +Executing this will output the transformed Fortran code with the +read-only-verification code added. + +The generic read-only verification library in +``../../../lib/read_only/generic`` is used, and will also be +automatically compiled if required. + +## Compiling and Execution + +To create and compile the example, type ``make compile``. +This example can be compiled and executed. It will report nothing, +since no read-only variable is overwritten. But you can verify that +the variables are checked by setting ``PSYDATA_VERBOSE=2``: + +```sh +$ PSYDATA_VERBOSE=2 ./dummy + PSyData: PreStart dummy r0 + PSyData: DeclareScalarChar: dummy r0: char_var + PSyData: DeclareScalarLogical: dummy r0: logical_var + PSyData: DeclareScalarChar: dummy r0: char_var + PSyData: DeclareScalarLogical: dummy r0: logical_var + PSyData: checked variable char_var + PSyData: checked variable logical_var + PSyData: PostEnd dummy r0 + 3.00000000 F +``` + +If you copy the lines 68 and 69 from ``dummy.f90`` into ``psy.f90``, +the code will modify ``logical_var`` (by using out-of-bound array accesses. +Or you could just manually set ``logical_var = .true.``). If you then +compile again (using `make compile`, otherwise the original file would +get processed again, overwriting your changes), an error will be produced. + +```sh +$ ./dummy + ------------- PSyData ------------------------- + Logical(kind=4) variable logical_var has been modified in dummy : r0 + Original value: F + New value: T + ------------- PSyData ------------------------- + 3.00000000 T +``` + +Note that adding the assignment to ``logical_var`` as above to the original +``dummy.f90`` file would mean that ``logical_var`` is not a read-only variable +anymore, so no test and therefore no error will be produced for this variable. +The code commented out can also not be processed by PSyclone (missing support +for ``loc`` and ``sizeof``, which are non-standard Fortran extensions). + +## Licence + +----------------------------------------------------------------------------- + +BSD 3-Clause License + +Copyright (c) 2025, 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. + +----------------------------------------------------------------------------- diff --git a/examples/nemo/eg6/dummy.f90 b/examples/nemo/eg6/dummy.f90 new file mode 100644 index 0000000000..76c936acfa --- /dev/null +++ b/examples/nemo/eg6/dummy.f90 @@ -0,0 +1,74 @@ +! ----------------------------------------------------------------------------- +! BSD 3-Clause License +! +! Copyright (c) 2025, 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: J. Henrichs, Bureau of Meteorology + +! This is a very simple program that is used to show how PSyclone can +! detect if a read-only variable is modified (typically because of a +! memory overwrite elsewhere). In order to actually trigger the warning, +! you need to modify the code after processing it with PSyclone, see +! details in lines 68-69. + +program dummy + real, dimension(10, 10) :: umask + character(len=10) :: char_var + integer :: ji, jj + logical :: logical_var + integer(kind=8) :: offset + integer, intrinsic :: loc, sizeof + + logical_var = .false. + char_var = "test" + + do jj = 1, 10 + do ji = 1, 10 + if (char_var .eq. "abc" .and. logical_var) then + umask(ji,jj) = 1 + else + umask(ji,jj) = 3 + endif + end do + + ! VERY NAUGHTY CODE AHEAD - there be dragon!! + ! We modify the values of some read-only fields by using offsets + ! in a written array. This will abort if the code is compiled + ! with array bound check of course. + ! TODO: Unfortunately, PSyclone does not support loc or sizeof (which + ! are non-standard extensions), so the code below does not work. But + ! you can create the psy-layer, and then copy the following lines into + ! psy.f90, and recompile the program (use `make compile`). This will then + ! issue a warning about `logical_var` being overwritten. + ! offset = ( loc(logical_var) - loc(umask(1,1)) ) / sizeof(logical_var) + ! umask(1+offset, 1) = 123.0 + end do + + print *,umask(1,1), logical_var +end program dummy diff --git a/examples/nemo/eg6/read_only_check.py b/examples/nemo/eg6/read_only_check.py new file mode 100644 index 0000000000..6c209b42f1 --- /dev/null +++ b/examples/nemo/eg6/read_only_check.py @@ -0,0 +1,62 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2025, 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: J. Henrichs, Bureau of Meteorology + +'''A transformation script that applies read-only-verification +to a small Fortran program. You can use + $ psyclone --config ../../../psyclone.cfg \ + -s ./read_only_check.py -opsy psy.f90 dummy.f90 + +''' + +from psyclone.psyir.transformations import ReadOnlyVerifyTrans +from psyclone.psyir.nodes import Loop, Routine + + +def trans(psyir): + '''Applies the read-only verification transformation to every + subroutine in the file. + + :param psyir: the PSyIR of the provided file. + :type psyir: :py:class:`psyclone.psyir.nodes.FileContainer` + ''' + + rov = ReadOnlyVerifyTrans() + + for subroutine in psyir.walk(Routine): + print(f"Transforming subroutine: {subroutine.name}") + for kern in subroutine.children: + if not isinstance(kern, Loop): + continue + rov.apply(kern) diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index e47c6f2891..a9e87e29f4 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -89,21 +89,13 @@ CONTAINS_STMT_FUNCTIONS = ["sbc_dcy"] # These files change the results from baseline when psyclone processes them -PASSTHROUGH_ISSUES = [ - "ldfslp.f90", # It has a '!dir$ NOVECTOR' that gets deleted by fparser -] +PASSTHROUGH_ISSUES = [] # These files change the results from the baseline when psyclone adds # parallelisation dirctives PARALLELISATION_ISSUES = [ "ldfc1d_c2d.f90", "tramle.f90", - # These files get the same results when parallelised by: "nvfortran -O1 - # -Kieee -nofma -Mnovect" but had to be excluded by other compiler/flags - # TODO #2787: May solve these issues. - "icedyn_rhg_evp.f90", - "domqco.f90", - "dynspg_ts.f90", ] diff --git a/examples/psyir/create.py b/examples/psyir/create.py index bb6a9e31f3..5b42fafeb0 100644 --- a/examples/psyir/create.py +++ b/examples/psyir/create.py @@ -208,9 +208,8 @@ def tmp2(): result = writer(psyir_tree) print(result) - # Write out the code as C. At the moment NaryOperator, Routine - # and Container are not supported in the C backend so the full example - # can't be output. + # Write out the code as C. At the moment Routine and Container are not + # supported in the C backend so the full example can't be output. writer = CWriter() result = writer(psyir_tree.children[0].children[0].children[3]) print(result) diff --git a/examples/xdsl/Makefile b/examples/xdsl/Makefile index 414f586b0c..1e2a1a3c5c 100644 --- a/examples/xdsl/Makefile +++ b/examples/xdsl/Makefile @@ -39,10 +39,10 @@ EXAMPLES=$(sort $(wildcard eg*)) include ../top_level.mk transform: - @echo "No transformation supported for xdsl" + @echo "No transformation supported for xdsl" compile: transform - @echo "No compilation supported for xdsl" + @echo "No compilation supported for xdsl" run: compile - @echo "No run targets for xdsl" \ No newline at end of file + @echo "No run targets for xdsl" diff --git a/lib/Makefile b/lib/Makefile index e1b203c03f..520e6aa5ba 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -61,6 +61,12 @@ all: $(MAKE) -C read_only all $(MAKE) -C value_range_check all +allclean: + $(MAKE) -C extract clean + $(MAKE) -C profiling clean + $(MAKE) -C read_only clean + $(MAKE) -C value_range_check clean + %.f90: %.jinja process.py $$($(PSYDATA_LIB_DIR)/get_python.sh) $(PSYDATA_LIB_DIR)/process.py $< > $*.f90 diff --git a/lib/extract/compare_variables_mod.jinja b/lib/extract/compare_variables_mod.jinja index c47a13ac82..6329b43545 100644 --- a/lib/extract/compare_variables_mod.jinja +++ b/lib/extract/compare_variables_mod.jinja @@ -23,18 +23,14 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined -%} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Character", "character(*)", 8), - ("Long", "real(kind=int64)", 64), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Character", "character(*)" ), + ("Long", "real(kind=int64)" ), + ("Int", "integer(kind=int32)") ] %} {% endif -%} @@ -99,7 +95,7 @@ module compare_variables_mod {# ------------------------------------------------------------------------- #} interface compare {% set all_compares=[] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} module procedure compare_scalar_{{name}} {% for dim in ALL_DIMS %} module procedure compare_array_{{dim}}d{{name}} @@ -158,7 +154,7 @@ contains end subroutine compare_summary ! ------------------------------------------------------------------------- -{% for name, type, bits in ALL_TYPES %} +{% for name, type in ALL_TYPES %} {# Logical needs EQV for its tests #} {% if name == "Logical" %} {% set EQUAL = ".EQV." -%} @@ -311,6 +307,6 @@ contains end subroutine Compare_array_{{dim}}d{{name}} {% endfor -%} {# for dim in ALL_DIMS #} -{%- endfor -%} {# for name, type, bits in ALL_TYPES #} +{%- endfor -%} {# for name, type in ALL_TYPES #} end module compare_variables_mod diff --git a/lib/extract/netcdf/dl_esm_inf/README.md b/lib/extract/netcdf/dl_esm_inf/README.md index 28a5615e40..3d18357a2e 100644 --- a/lib/extract/netcdf/dl_esm_inf/README.md +++ b/lib/extract/netcdf/dl_esm_inf/README.md @@ -86,7 +86,7 @@ The compilation process will create the wrapper library previously selected compiler flags. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. To also remove the compiled infrastructure library it is necessary to run ``make allclean`` (this is especially important if changing compilers diff --git a/lib/extract/netcdf/extract_netcdf_base.jinja b/lib/extract/netcdf/extract_netcdf_base.jinja index e112e0b6d3..d175142e7b 100644 --- a/lib/extract/netcdf/extract_netcdf_base.jinja +++ b/lib/extract/netcdf/extract_netcdf_base.jinja @@ -26,18 +26,14 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined -%} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Character", "character(*)", 32), - ("Long", "real(kind=int64)", 64), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Character", "character(*)" ), + ("Long", "real(kind=int64)" ), + ("Int", "integer(kind=int32)") ] %} {% endif -%} @@ -119,7 +115,7 @@ module extract_netcdf_base_mod {# ------------------------------------------------------------------------- -#} {% set all_declares=[] -%} {% set all_provides=[] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} procedure :: DeclareScalar{{name}} procedure :: WriteScalar{{name}} {{- all_declares.append("DeclareScalar"~name) or "" -}} @@ -318,7 +314,7 @@ contains "Int": "NF90_INT"} -%} {% set indent=" " %} -{% for name, type, bits in ALL_TYPES %} +{% for name, type in ALL_TYPES %} {% set NETCDF_TYPE = NCDF_TYPE_MAPPING[name] %} ! ------------------------------------------------------------------------- !> @brief This subroutine declares a scalar {{type}} value. @@ -506,6 +502,6 @@ contains {% endfor -%} {# for dim in ALL_DIMS #} -{%- endfor -%} {# for name, type, bits in ALL_TYPES #} +{%- endfor -%} {# for name, type in ALL_TYPES #} end module extract_netcdf_base_mod diff --git a/lib/extract/netcdf/generic/README.md b/lib/extract/netcdf/generic/README.md index ad3b0cd8c5..af858bfd53 100644 --- a/lib/extract/netcdf/generic/README.md +++ b/lib/extract/netcdf/generic/README.md @@ -58,7 +58,7 @@ The compilation process will create the wrapper library ``lib_extract.a``. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. ### Linking the wrapper library diff --git a/lib/extract/netcdf/lfric/README.md b/lib/extract/netcdf/lfric/README.md index 2920c09e98..373222d7af 100644 --- a/lib/extract/netcdf/lfric/README.md +++ b/lib/extract/netcdf/lfric/README.md @@ -90,7 +90,7 @@ infrastructure library, ``liblfric_netcdf.a``, if required, with the previously selected compiler flags. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. To also remove the compiled infrastructure library it is necessary to run ``make allclean`` (this is especially important if changing compilers diff --git a/lib/extract/netcdf/read_kernel_data_mod.jinja b/lib/extract/netcdf/read_kernel_data_mod.jinja index 6cfc390c2c..1e04f3ae31 100644 --- a/lib/extract/netcdf/read_kernel_data_mod.jinja +++ b/lib/extract/netcdf/read_kernel_data_mod.jinja @@ -19,18 +19,14 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined -%} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Char", "character(*)", 8), - ("Logical","integer(kind=real32)",32), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Char", "character(*)" ), + ("Logical","integer(kind=real32)"), + ("Int", "integer(kind=int32)" ) ] %} {% endif -%} @@ -99,7 +95,7 @@ module read_kernel_data_mod {# Collect and declare the various procedures for the same generic interface -#} {# ------------------------------------------------------------------------- -#} {% set all_reads=[] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} procedure :: ReadScalar{{name}} {{- all_reads.append("ReadScalar"~name) or "" }} {% for dim in ALL_DIMS %} @@ -197,7 +193,7 @@ contains "Logical":"NF90_INT", "Int": "NF90_INT"} -%} -{% for name, type, bits in ALL_TYPES %} +{% for name, type in ALL_TYPES %} {% set NETCDF_TYPE = NCDF_TYPE_MAPPING[name] %} ! ------------------------------------------------------------------------- @@ -329,6 +325,6 @@ contains {% endfor -%} {# for dim in ALL_DIMS #} -{%- endfor -%} {# for name, type, bits in ALL_TYPES #} +{%- endfor -%} {# for name, type in ALL_TYPES #} end module read_kernel_data_mod diff --git a/lib/extract/standalone/dl_esm_inf/README.md b/lib/extract/standalone/dl_esm_inf/README.md index 143a3bdcd8..be85e14ea8 100644 --- a/lib/extract/standalone/dl_esm_inf/README.md +++ b/lib/extract/standalone/dl_esm_inf/README.md @@ -77,7 +77,7 @@ The compilation process will create the wrapper library previously selected compiler flags. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. To also remove the compiled infrastructure library it is necessary to run ``make allclean`` (this is especially important if changing compilers diff --git a/lib/extract/standalone/extract_standalone_base.jinja b/lib/extract/standalone/extract_standalone_base.jinja index 38d807e2fd..710ec7a495 100644 --- a/lib/extract/standalone/extract_standalone_base.jinja +++ b/lib/extract/standalone/extract_standalone_base.jinja @@ -26,18 +26,14 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined -%} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Character", "character(*)", 8), - ("Long", "real(kind=int64)", 64), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Character", "character(*)" ), + ("Long", "real(kind=int64)" ), + ("Int", "integer(kind=int32)") ] %} {% endif -%} @@ -112,7 +108,7 @@ module extract_standalone_base_mod {# ------------------------------------------------------------------------- -#} {% set all_declares=[] -%} {% set all_provides=[] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} procedure :: WriteScalar{{name}} {{- all_provides.append("WriteScalar"~name) or "" }} {% for dim in ALL_DIMS %} @@ -215,7 +211,7 @@ contains end subroutine PostEnd -{% for name, type, bits in ALL_TYPES %} +{% for name, type in ALL_TYPES %} ! ------------------------------------------------------------------------- !> @brief This subroutine writes the value of a scalar {{type}} !! variable to the file. It takes the variable id from the @@ -275,6 +271,6 @@ contains {% endfor -%} {# for dim in ALL_DIMS #} -{%- endfor -%} {# for name, type, bits in ALL_TYPES #} +{%- endfor -%} {# for name, type in ALL_TYPES #} end module extract_standalone_base_mod diff --git a/lib/extract/standalone/generic/README.md b/lib/extract/standalone/generic/README.md index e5d5f681dc..e48d9e2038 100644 --- a/lib/extract/standalone/generic/README.md +++ b/lib/extract/standalone/generic/README.md @@ -47,7 +47,7 @@ The compilation process will create the wrapper library ``lib_extract.a``. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. ### Linking the wrapper library diff --git a/lib/extract/standalone/lfric/README.md b/lib/extract/standalone/lfric/README.md index e975a8b914..192d06219d 100644 --- a/lib/extract/standalone/lfric/README.md +++ b/lib/extract/standalone/lfric/README.md @@ -81,7 +81,7 @@ infrastructure library, ``liblfric.a``, if required, with the previously selected compiler flags. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. To also remove the compiled infrastructure library it is necessary to run ``make allclean`` (this is especially important if changing compilers diff --git a/lib/extract/standalone/read_kernel_data_mod.jinja b/lib/extract/standalone/read_kernel_data_mod.jinja index 8a0faac460..338f74e055 100644 --- a/lib/extract/standalone/read_kernel_data_mod.jinja +++ b/lib/extract/standalone/read_kernel_data_mod.jinja @@ -19,18 +19,14 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined -%} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Char", "character(*)", 8), - ("Logical","integer(kind=real32)",32), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Char", "character(*)" ), + ("Logical","integer(kind=real32)"), + ("Int", "integer(kind=int32)" ) ] %} {% endif -%} @@ -102,7 +98,7 @@ module read_kernel_data_mod {# Collect and declare the various procedures for the same generic interface -#} {# ------------------------------------------------------------------------- -#} {% set all_reads=[] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} procedure :: ReadScalar{{name}} {{- all_reads.append("ReadScalar"~name) or "" }} {% for dim in ALL_DIMS %} @@ -168,7 +164,7 @@ contains end subroutine OpenReadFileName -{% for name, type, bits in ALL_TYPES %} +{% for name, type in ALL_TYPES %} ! ------------------------------------------------------------------------- !> @brief This subroutine reads the value of a scalar {{type}} @@ -264,6 +260,6 @@ contains {% endfor -%} {# for dim in ALL_DIMS #} -{%- endfor -%} {# for name, type, bits in ALL_TYPES #} +{%- endfor -%} {# for name, type in ALL_TYPES #} end module read_kernel_data_mod diff --git a/lib/process.py b/lib/process.py index 3d21c891a0..14609d30cb 100755 --- a/lib/process.py +++ b/lib/process.py @@ -102,12 +102,12 @@ # --------------------------------------------------------- # This is a mapping from the command line option name # to the tuple that is required for the jinja templates: -TYPE_DATA = {"real": ("Real", "real(kind=real32)", 32), - "double": ("Double", "real(kind=real64)", 64), - "int": ("Int", "integer(kind=int32)", 32), - "char": ("Char", "character(*)", 8), - "logical": ("Logical", "Logical(kind=4)", 4), - "long": ("Long", "integer(kind=int64)", 64)} +TYPE_DATA = {"real": ("Real", "real(kind=real32)"), + "double": ("Double", "real(kind=real64)"), + "int": ("Int", "integer(kind=int32)"), + "char": ("Char", "character(*)"), + "logical": ("Logical", "Logical(kind=4)"), + "long": ("Long", "integer(kind=int64)")} # --------------------------------------------------------- # Check type information: diff --git a/lib/psy_data_base.jinja b/lib/psy_data_base.jinja index f71a4e0df4..3b6d095f10 100644 --- a/lib/psy_data_base.jinja +++ b/lib/psy_data_base.jinja @@ -12,16 +12,12 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined %} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Int", "integer(kind=int32)") ] %} {% endif %} ! ----------------------------------------------------------------------------- @@ -110,7 +106,7 @@ module psy_data_base_mod {# ------------------------------------------------------------- -#} {% set all_declares=[] -%} {% set all_provides=[] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} procedure :: DeclareScalar{{name}} procedure :: ProvideScalar{{name}} {{- all_declares.append("DeclareScalar"~name) or "" -}} @@ -316,7 +312,7 @@ contains ! ========================================================================= ! Jinja created code: ! ========================================================================= - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} ! ------------------------------------------------------------------------- !> @brief This subroutine declares a scalar {{type}} value. This !! implementation only increases the next index, and prints diff --git a/lib/read_only/Makefile b/lib/read_only/Makefile index ef3d023a7b..2ea4103429 100644 --- a/lib/read_only/Makefile +++ b/lib/read_only/Makefile @@ -48,10 +48,10 @@ F90FLAGS ?= PSYDATA_LIB_DIR ?= ./.. # ----------------------------------------------------------------------------- -# The read-only verification library is implemented for int, real and -# double scalars and 2-dimension arrays -PROCESS_ARGS = -prefix=read_only_verify_ -types=int,real,double \ - -dims=2 +# The read-only verification library is implemented for int, logical, +# characters, real, and double precision scalars and 1 to 4 dimensional arrays +PROCESS_ARGS = -prefix=read_only_verify_ -types=char,int,logical,real,double \ + -dims=1,2,3,4 PROCESS = $$($(PSYDATA_LIB_DIR)/get_python.sh) $(PSYDATA_LIB_DIR)/process.py default: read_only_base.o psy_data_base.o @@ -63,6 +63,7 @@ process: read_only_base.f90 all: $(MAKE) -C dl_esm_inf $(MAKE) -C lfric + $(MAKE) -C generic %.f90: %.jinja $(PROCESS) $(PROCESS_ARGS) $< > $*.f90 @@ -78,6 +79,7 @@ read_only_base.o: psy_data_base.o clean: rm -f read_only_base.f90 psy_data_base.f90 *.o *.mod -allclean: +allclean: clean $(MAKE) -C dl_esm_inf allclean $(MAKE) -C lfric allclean + $(MAKE) -C generic allclean diff --git a/lib/read_only/README.md b/lib/read_only/README.md index 259ecba724..ecb85f83af 100644 --- a/lib/read_only/README.md +++ b/lib/read_only/README.md @@ -38,6 +38,10 @@ Contains the read-only, PSyData-API-based, wrapper library for the [LFRic (Dynamo 0.3) API]( https://psyclone.readthedocs.io/en/stable/dynamo0p3.html). +## [``generic``](./generic) directory + +Contains the generic read-only wrapper library. + diff --git a/lib/read_only/generic/read_only.f90 b/lib/read_only/generic/read_only.f90 new file mode 100644 index 0000000000..89e10a3a3a --- /dev/null +++ b/lib/read_only/generic/read_only.f90 @@ -0,0 +1,63 @@ +! ----------------------------------------------------------------------------- +! BSD 3-Clause License +! +! Copyright (c) 2025, 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 J. Henrichs, Bureau of Meteorology + +!> This module implements a verification that read-only variables in generic +!! Fortran code are indeed not modified (e.g. because of memory overwrite) +!! This very slim module is implemented since PSyclone expects +!! certain module and type names, which the base class ReadOnlyBaseType +!! does not provide. + +module read_only_verify_psy_data_mod + + use, intrinsic :: iso_fortran_env, only : int64, int32, & + real32, real64, & + stderr => Error_Unit + + use read_only_base_mod, only : ReadOnlyBaseType, read_only_verify_PSyDataInit, & + read_only_verify_PSyDataShutdown, is_enabled, & + read_only_verify_PSyDataStart, read_only_verify_PSyDataStop + + implicit none + + !> This is the data type that stores a checksum for each read-only + !! variable. A static instance of this type is created for each + !! instrumented region with PSyclone. It is empty, this class is + !! only here to get the right name for PSyclone in case of generic + !! transformation usage. + type, extends(ReadOnlyBaseType), public :: read_only_verify_PSyDataType + + end type read_only_verify_PSyDataType + +end module read_only_verify_psy_data_mod diff --git a/lib/read_only/lfric/README.md b/lib/read_only/lfric/README.md index 32877f30cc..069b19da2f 100644 --- a/lib/read_only/lfric/README.md +++ b/lib/read_only/lfric/README.md @@ -63,7 +63,7 @@ The ``Makefile`` will compile the LFRic infrastructure library, ``liblfric.a``, if required, with the previously selected compiler flags. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. To also remove the compiled infrastructure library it is necessary to run ``make allclean`` (this is especially important if changing compilers diff --git a/lib/read_only/read_only_base.jinja b/lib/read_only/read_only_base.jinja index 64b094f541..6cfec22da4 100644 --- a/lib/read_only/read_only_base.jinja +++ b/lib/read_only/read_only_base.jinja @@ -29,16 +29,12 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined %} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Int", "integer(kind=int32)") ] %} {% endif %} ! ----------------------------------------------------------------------------- @@ -117,7 +113,7 @@ module read_only_base_mod {# Collect the various procedures for the same generic interface -#} {# ------------------------------------------------------------- -#} {% set all_provides=[] -%} - {%- for name, type, bits in ALL_TYPES %} + {%- for name, type in ALL_TYPES %} procedure :: ProvideScalar{{name}} {{- all_provides.append("ProvideScalar"~name) or "" }} {% for dim in ALL_DIMS %} @@ -145,7 +141,7 @@ module read_only_base_mod {# Create a generic interface for all ComputeChecksum functions #} {# ------------------------------------------------------------ #} {% set all_compute = [] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} {% for dim in ALL_DIMS %} {{- all_compute.append("ComputeChecksum"~dim~"d"~name) or "" -}} {% endfor %} @@ -265,7 +261,21 @@ contains LFRic which uses ComputeChecksum1dDouble) #} -{% for name, type, bits in ALL_TYPES %} +{% for name, type in ALL_TYPES %} + {# We need the number of bits since there is slightly different code + required for 32 and 64 bit values (due to the fact that the + Fortran transfer(value, mold) function leaves undefined bits + when mold is larger than value. #} + {% if name in ["Real", "Int", "Logical"] %} + {% set bits = 32 %} + {% elif name in ["Double", "Long"] %} + {% set bits = 64 %} + {% elif name in ["Char"] %} + {% set bits = 8 %} + {% else %} + ERROR - UNSUPPORTED BIT SIZE for {{name}}! + {% endif %} + ! ========================================================================= ! Implementation for all {{type}} types @@ -284,16 +294,26 @@ contains character(*), intent(in) :: name {{type}}, intent(in) :: value - {{type}} :: orig_value - integer(kind=int64) :: checksum, int_64 + {% if name == "Char" %} + character, allocatable :: orig_value + integer :: i + {% else %} + {{type}} :: orig_value + {% endif %} + integer(kind=int64) :: checksum, int_64 {% if bits==32 %} - integer(kind=int32) :: int_32 + integer(kind=int32) :: int_32 {% endif %} if (.not. is_enabled) return - {% if bits == 32 %} + {% if name == "Char" %} + checksum = 0 + do i=1, len(value) + checksum = checksum + ichar(value(i:i)) + enddo + {% elif bits == 32 %} ! `transfer` leaves undefined bits in a 64-bit value ! so assign to 32-bit, then assign to 64-bit to have all bits defined int_32 = transfer(value, int_32) @@ -315,20 +335,22 @@ contains write(stderr,*) "------------- PSyData -------------------------" write(stderr,*) "{{type}} variable ", name, " has been modified in ", & trim(this%module_name)," : ", trim(this%region_name) - ! We can recreate the original value which is stored as - ! 64-bit integer as the checksum: - {% if bits == 32 %} + {% if name != "Char" %} + ! We can recreate the original scalar value which is stored as + ! 64-bit integer in the checksum: + {% if bits == 32 %} int_32 = this%checksums(this%next_var_index) orig_value = transfer(int_32, orig_value) - {% elif bits == 64 %} + {% elif bits == 64 %} orig_value = transfer(this%checksums(this%next_var_index), orig_value) - {% elif name == "Logical" %} + {% elif name == "Logical" %} orig_value = this%checksums(this%next_var_index) .eq. 1 - {% else %} + {% else %} ERROR - UNSUPPORTED BIT SIZE {{bits}}! - {% endif %} + {% endif %} write(stderr,*) "Original value: ", orig_value write(stderr,*) "New value: ", value + {% endif %} write(stderr,*) "------------- PSyData -------------------------" else if(this%verbosity>1) then write(stderr,*) "PSyData: checked variable ", trim(name) @@ -400,7 +422,9 @@ contains {{type}}, dimension({{DIMENSION}}) :: field integer :: {{vars}} - {% if bits == 32 %} + {% if name == "Char" %} + integer :: i + {% elif bits == 32 %} integer(kind=int32) :: int_32 {% endif %} integer(kind=int64) :: checksum, int_64 @@ -410,7 +434,12 @@ contains {% for j in range(dim, 0, -1) %} {{" "*3*(dim-j)}}do i{{j}}=1, size(field, {{j}}) {% endfor %} - {% if bits == 32 %} + {% if name == "Char" %} + {{indent}}int_64 = 0 + {{indent}}do i=1, len(field({{vars}})) + {{indent}} int_64 = int_64 + ichar(field({{vars}})(i:i)) + {{indent}}enddo + {% elif bits == 32 %} {{indent}}! transfer leaves undefined bits in a 64-bit target {{indent}}! so we transfer to 32-bits and then assign to 64-bit {{indent}}int_32 = transfer(field({{vars}}), int_32) diff --git a/lib/value_range_check/Makefile b/lib/value_range_check/Makefile index b137bcbc3e..9427778a42 100644 --- a/lib/value_range_check/Makefile +++ b/lib/value_range_check/Makefile @@ -48,10 +48,10 @@ F90FLAGS ?= PSYDATA_LIB_DIR ?= ./.. # ----------------------------------------------------------------------------- -# The nan-test library is implemented for int, real and -# double scalars and 2-dimension arrays -PROCESS_ARGS = -prefix=value_range_check_ -types=int,real,double \ - -dims=2 +# The value-range check library is implemented for int, logical, characters, +# real, and double precision scalars and 1 to 4 dimensional arrays +PROCESS_ARGS = -prefix=value_range_check_ -types=char,int,logical,real,double \ + -dims=1,2,3,4 PROCESS = $$($(PSYDATA_LIB_DIR)/get_python.sh) $(PSYDATA_LIB_DIR)/process.py diff --git a/lib/value_range_check/dl_esm_inf/README.md b/lib/value_range_check/dl_esm_inf/README.md index 3fa1eea327..bfe8e05758 100644 --- a/lib/value_range_check/dl_esm_inf/README.md +++ b/lib/value_range_check/dl_esm_inf/README.md @@ -57,7 +57,7 @@ The ``Makefile`` will compile the ``dl_esm_inf`` infrastructure library, ``lib_fd.a``, if required, with the previously selected compiler flags. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. To also remove the compiled infrastructure library it is necessary to run ``make allclean`` (this is especially important if changing compilers diff --git a/lib/value_range_check/lfric/README.md b/lib/value_range_check/lfric/README.md index 52fbc98d1e..e29ef50b70 100644 --- a/lib/value_range_check/lfric/README.md +++ b/lib/value_range_check/lfric/README.md @@ -64,7 +64,7 @@ The ``Makefile`` will compile the LFRic infrastructure library, ``liblfric.a``, if required, with the previously selected compiler flags. Similar to compilation of the [examples]( -https://psyclone.readthedocs.io/en/latest/examples.html#compilation), the +https://psyclone.readthedocs.io/en/latest/tutorials_and_examples.html#compilation), the compiled wrapper library can be removed by running ``make clean``. To also remove the compiled infrastructure library it is necessary to run ``make allclean`` (this is especially important if changing compilers diff --git a/lib/value_range_check/value_range_check_base.jinja b/lib/value_range_check/value_range_check_base.jinja index e3a1c51f75..2f486669fc 100644 --- a/lib/value_range_check/value_range_check_base.jinja +++ b/lib/value_range_check/value_range_check_base.jinja @@ -26,16 +26,14 @@ {# The types that are supported. The first entry of each tuple is the name used when naming subroutines and in user messages. - The second entry is the Fortran declaration. The third entry - is the number of bits. There is slightly different code - required for 32 and 64 bit values (due to the fact that the - Fortran transfer(value, mould) function leaves undefined bits - when mould is larger than value.) #} + The second entry is the Fortran declaration. #} {% if ALL_TYPES is not defined %} - {% set ALL_TYPES = [ ("Double", "real(kind=real64)", 64), - ("Real", "real(kind=real32)", 32), - ("Int", "integer(kind=int32)", 32) ] %} + {% set ALL_TYPES = [ ("Double", "real(kind=real64)" ), + ("Real", "real(kind=real32)" ), + ("Char", "character", ), + ("Logical", "logical", ), + ("Int", "integer(kind=int32)") ] %} {% endif %} ! ----------------------------------------------------------------------------- @@ -126,7 +124,7 @@ module value_range_check_base_mod {# ------------------------------------------------------------------------- -#} {% set all_declares=[] -%} {% set all_provides=[] -%} - {% for name, type, bits in ALL_TYPES %} + {% for name, type in ALL_TYPES %} procedure :: ProvideScalar{{name}} {{- all_provides.append("ProvideScalar"~name) or "" }} {% for dim in ALL_DIMS %} @@ -315,7 +313,7 @@ contains ! Jinja created code. ! ========================================================================= -{% for name, type, bits in ALL_TYPES %} +{% for name, type in ALL_TYPES %} ! ========================================================================= ! Implementation for all {{type}} types @@ -338,6 +336,11 @@ contains character(*), intent(in) :: name {{type}}, intent(in) :: value + {% if name in ["Logical", "Char"] %} + ! We can't compare {{type}} values with a real, nor can + ! we check for non-finite values. So do nothing. + {% else %} + {# Now it's a floating point or integer value #} if (.not. is_enabled) return if (.not. this%env_var_have_been_read) then @@ -346,11 +349,9 @@ contains this%value_ranges(this%next_var_index,:)) endif - {% if name !="Logical" %} - if (this%has_checks(this%next_var_index) == has_min) then {# The spaces take care of proper indentation #} - {{indent}}if (value< this%value_ranges(this%next_var_index,1)) then + {{indent}}if (value < this%value_ranges(this%next_var_index,1)) then {{indent}} write(stderr, '(11G0)') "PSyData: Variable '", & {{indent}} name,"' has the value ", & {{indent}} value, " in module '", trim(this%module_name), & @@ -381,9 +382,11 @@ contains {{indent}} this%value_ranges(this%next_var_index,2),"'." {{indent}}endif endif - {% endif %} - {% if name not in ["Int", "Logical"] %} + {% if name in ["Int", "Long"] %} + ! Variables of type {{type}} do not have NANs, and cannot usefully be + ! checked for non-finite values. So nothing to do here. + {% else -%} {# floating point #} if (IEEE_SUPPORT_DATATYPE(value)) then if (.not. IEEE_IS_FINITE(value)) then write(stderr, '(8G0)') "PSyData: Variable '", name,"' has invalid value ", & @@ -391,10 +394,8 @@ contains "', region '", trim(this%region_name),"'." endif endif - {% else %} - ! Variables of type {{type}} do not have NANs, and cannot usefully be - ! checked for range. So nothing to do here. - {% endif %} + {% endif %} {# floating point value #} + {% endif %} {# floating point or integer #} call this%PSyDataBaseType%ProvideScalar{{name}}(name, value) @@ -430,11 +431,15 @@ contains character(*), intent(in) :: name {{type}}, dimension({{DIMENSION}}), intent(in) :: value - {# IEEE_SUPPORT_DATATYPE does not even compile for int data types #} - {% if name not in ["Int", "Logical"] %} integer :: {{vars}} + {% if name in ["Logical", "Char"] %} + ! We can't compare {{type}} values with a real, nor can + ! we check for non-finite values. So do nothing. + {% else %} + {# Now it's a floating point or integer value #} if (.not. is_enabled) return + if (.not. this%env_var_have_been_read) then call this%get_min_max(this%module_name, this%region_name, & name, this%has_checks(this%next_var_index), & @@ -496,6 +501,10 @@ contains {% endfor %} endif + {% if name in ["Int", "Long"] %} + ! Variables of type {{type}} do not have NANs, and cannot usefully be + ! checked for non-finite values. So nothing to do here. + {% else -%} {# floating point #} if (IEEE_SUPPORT_DATATYPE(value)) then {# The spaces take care of proper indentation #} {% for j in range(dim, 0, -1) %} @@ -512,10 +521,8 @@ contains {{" "*3*(j-1)}}enddo {% endfor %} endif - {% else %} - ! Variables of type {{type}} do not have NANs - ! So nothing to do here. - {% endif %} + {% endif %} {# floating point value #} + {% endif %} {# floating point or integer #} call this%PSyDataBaseType%ProvideArray{{dim}}d{{name}}(name, value) diff --git a/psyclone.pdf b/psyclone.pdf index 56b8d4e313..f3c9b723a1 100644 Binary files a/psyclone.pdf and b/psyclone.pdf differ diff --git a/src/psyclone/configuration.py b/src/psyclone/configuration.py index 5cf728a286..e66456d012 100644 --- a/src/psyclone/configuration.py +++ b/src/psyclone/configuration.py @@ -226,6 +226,9 @@ def __init__(self): # checks which can be useful in the case of unimplemented features. self._backend_checks_enabled = True + # The Fortran standard that fparser should use + self._fortran_standard = None + # ------------------------------------------------------------------------- def load(self, config_file=None): '''Loads a configuration file. @@ -369,6 +372,16 @@ def load(self, config_file=None): for module_name in ignore_modules: mod_manager.add_ignore_module(module_name) + # Get the Fortran standard to use: + self._fortran_standard = \ + self._config['DEFAULT'].get("FORTRAN_STANDARD", "f2008").lower() + valid_standard = ["f2003", "f2008"] + if self._fortran_standard not in valid_standard: + raise ConfigurationError(f"Invalid Fortran standard " + f"'{self._fortran_standard}' specified " + f"in config file. Must be one of" + f"{valid_standard}") + # Set the flag that the config file has been loaded now. Config._HAS_CONFIG_BEEN_INITIALISED = True @@ -698,6 +711,12 @@ def ocl_devices_per_node(self): :rtype: int''' return self._ocl_devices_per_node + @property + def fortran_standard(self) -> str: + ''':returns: The Fortran standard to be used by fparser. + ''' + return self._fortran_standard + def get_default_keys(self): '''Returns all keys from the default section. :returns list: List of all keys of the default section as strings. diff --git a/src/psyclone/core/access_type.py b/src/psyclone/core/access_type.py index e8fc51f7bc..f772173a8c 100644 --- a/src/psyclone/core/access_type.py +++ b/src/psyclone/core/access_type.py @@ -37,7 +37,6 @@ '''This module implements the AccessType used throughout PSyclone.''' -from __future__ import print_function, absolute_import from enum import Enum from psyclone.configuration import Config @@ -45,46 +44,66 @@ class AccessType(Enum): '''A simple enum-class for the various valid access types. ''' - + #: Data associated with the symbol is read. READ = 1 + #: Data associated with the symbols is written. WRITE = 2 + #: Data associated with the symbol is both read and written (e.g. is passed + #: to a routine with intent(inout)). READWRITE = 3 + #: Incremented from more than one cell column (see the LFRic API section + #: of the User Guide). INC = 4 + #: Read before incrementing. Requires that the outermost halo be clean (see + #: the LFRic API section of the User Guide). READINC = 5 + #: Is the output of a SUM reduction. SUM = 6 - # This is used internally to indicate unknown access type of - # a variable, e.g. when a variable is passed to a subroutine - # and the access type of this variable in the subroutine - # is unknown + #: This is used internally to indicate unknown access type of + #: a variable, e.g. when a variable is passed to a subroutine + #: and the access type of this variable in the subroutine + #: is unknown. + #: TODO #2863 - VariablesAccessInfo does not currently consider + #: UNKNOWN accesses and it should! UNKNOWN = 7 + #: A symbol representing a routine is called. + CALL = 8 + #: The property/ies of a symbol is/are queried but the data it + #: represents is not accessed (e.g. 'var' in SIZE(var, dim=1)). + INQUIRY = 9 + #: The symbol is used to access its type information (available at compile + #: time) - e.g. precision values such as 'wp' in 1.0_wp. + TYPE_INFO = 10 - def __str__(self): + def __str__(self) -> str: '''Convert to a string representation, returning just the - enum (e.g. 'WRITE').. - :return: API name for this string. - :rtype: str + enum (e.g. 'WRITE'). ''' # pylint complains without str() that the return type is not a str return str(self.name) - def api_specific_name(self): + def api_specific_name(self) -> str: '''This convenience function returns the name of the type in the - current API. E.g. in a lfric API, WRITE --> "gh_write" + current API. E.g. in the lfric API, WRITE --> "gh_write". If no + mapping is available then the generic name is returned. + :returns: The API specific name. - :rtype: str ''' api_config = Config.get().api_conf() rev_access_mapping = api_config.get_reverse_access_mapping() - return rev_access_mapping[self] + return rev_access_mapping.get(self, str(self).lower()) @staticmethod - def from_string(access_string): + def from_string(access_string: str): '''Convert a string (e.g. "read") into the corresponding AccessType enum value (AccessType.READ). - :param str access_string: Access type as string. + :param access_string: Access type as a string. + :returns: Corresponding AccessType enum. - :Raises: ValueError if access_string is not a valid access type. + :rtype: :py:class:`psyclone.core.access_type.AccessType` + + :raises ValueError: if access_string is not a valid access type. ''' for access in AccessType: if access.name == access_string.upper(): @@ -128,6 +147,15 @@ def get_valid_reduction_names(): return [access.api_specific_name() for access in AccessType.get_valid_reduction_modes()] + @staticmethod + def non_data_accesses(): + ''' + :returns: all access types that do not touch any data associated with + a symbol. + :rtype: list[:py:class:`psyclone.core.AccessType`] + ''' + return [AccessType.CALL, AccessType.TYPE_INFO, AccessType.INQUIRY] + # ---------- Documentation utils -------------------------------------------- # # The list of module members that we wish AutoAPI to generate diff --git a/src/psyclone/core/single_variable_access_info.py b/src/psyclone/core/single_variable_access_info.py index 4d4e2d0fa8..8adf93875a 100644 --- a/src/psyclone/core/single_variable_access_info.py +++ b/src/psyclone/core/single_variable_access_info.py @@ -152,6 +152,14 @@ def access_type(self): :rtype: :py:class:`psyclone.core.access_type.AccessType`''' return self._access_type + @property + def is_data_access(self) -> bool: + ''' + :returns: whether or not this access is to the data associated with + a signature (i.e. is not just an inquiry-type access). + ''' + return self._access_type not in AccessType.non_data_accesses() + @property def location(self): ''':returns: the location information for this access.\ @@ -208,39 +216,51 @@ def var_name(self): ''' return str(self._signature) - def is_written(self): - ''':returns: True if this variable is written (at least once). - :rtype: bool + def is_called(self) -> bool: + ''' + :returns: whether or not any accesses of this variable + represent a call. + ''' + return any(info.access_type == AccessType.CALL for + info in self._accesses) + + def is_written(self) -> bool: + ''' + :returns: True if this variable is written (at least once). ''' return any(access_info.access_type in AccessType.all_write_accesses() for access_info in self._accesses) - def is_written_first(self): - ''':returns: True if this variable is written in the first access \ - (which indicates that this variable is not an input variable \ + def is_written_first(self) -> bool: + ''' + :returns: True if this variable is written in the first data access + (which indicates that this variable is not an input variable for a kernel). - - :rtype: bool ''' - return len(self._accesses) > 0 and \ - (self._accesses[0].access_type == AccessType.WRITE) + for acc in self._accesses: + if acc.access_type in AccessType.non_data_accesses(): + continue + if acc.access_type != AccessType.WRITE: + return False + return True + return False - def is_read_only(self): - '''Checks if this variable is always read, and never - written. + def is_read_only(self) -> bool: + '''Checks if this variable is always read, and never written. :returns: True if this variable is read only. - :rtype: bool ''' - return all(access_info.access_type == AccessType.READ + access_types = AccessType.non_data_accesses() + [AccessType.READ] + return all(access_info.access_type in access_types for access_info in self._accesses) - def is_read(self): - ''':returns: True if this variable is read (at least once). - :rtype: bool + def is_read(self) -> bool: + ''' + :returns: True if this variable is read (at least once). ''' - return any(access_info.access_type in AccessType.all_read_accesses() + read_accesses = AccessType.all_read_accesses() + return any(access_info.access_type in read_accesses for access_info in self._accesses) def has_read_write(self): @@ -252,6 +272,16 @@ def has_read_write(self): return any(access_info.access_type == AccessType.READWRITE for access_info in self._accesses) + def has_data_access(self) -> bool: + ''' + :returns: True if there is an access of the data associated with this + signature (as opposed to a call or an inquiry), False otherwise. + ''' + for info in self._accesses: + if info.access_type not in AccessType.non_data_accesses(): + return True + return False + def __getitem__(self, index): ''':return: the access information for the specified index. :rtype: py:class:`psyclone.core.AccessInfo` @@ -310,19 +340,33 @@ def change_read_to_write(self): This function is then called to change the assigned-to variable on the LHS to from READ to WRITE. Since the LHS is stored in a separate SingleVariableAccessInfo class, it is guaranteed that there is only - one entry for the variable. - ''' - if len(self._accesses) != 1: - raise InternalError(f"Variable '{self._signature}' had " - f"{len(self._accesses)} accesses listed, " - "not one in change_read_to_write.") + one READ entry for the variable (although there maybe INQUIRY accesses + for array bounds). - if self._accesses[0].access_type != AccessType.READ: - raise InternalError(f"Trying to change variable " - f"'{self._signature}' to 'WRITE' " - "which does not have 'READ' access.") - - self._accesses[0].change_read_to_write() + :raises InternalError: if there is an access that is not READ or + INQUIRY or there is > 1 READ access. + ''' + read_access = None + for acc in self._accesses: + + if acc.access_type == AccessType.READ: + if read_access: + raise InternalError( + f"Trying to change variable '{self._signature}' to " + f"'WRITE' but it has more than one 'READ' access.") + read_access = acc + + elif acc.access_type not in AccessType.non_data_accesses(): + raise InternalError( + f"Variable '{self._signature}' has a '{acc.access_type}' " + f"access. change_read_to_write() expects only inquiry " + f"accesses and a single 'READ' access.") + + if not read_access: + raise InternalError( + f"Trying to change variable '{self._signature}' to 'WRITE' but" + f" it does not have a 'READ' access.") + read_access.change_read_to_write() def is_array(self, index_variable=None): '''Checks if the variable is used as an array, i.e. if it has diff --git a/src/psyclone/core/variables_access_info.py b/src/psyclone/core/variables_access_info.py index 12e741afc3..0fb8ec8459 100644 --- a/src/psyclone/core/variables_access_info.py +++ b/src/psyclone/core/variables_access_info.py @@ -40,6 +40,8 @@ '''This module provides management of variable access information.''' +from typing import List + from psyclone.core.component_indices import ComponentIndices from psyclone.core.signature import Signature from psyclone.core.single_variable_access_info import SingleVariableAccessInfo @@ -60,10 +62,6 @@ class VariablesAccessInfo(dict): :param options: a dictionary with options to influence which variable accesses are to be collected. :type options: Dict[str, Any] - :param Any options["COLLECT-ARRAY-SHAPE-READS"]: if this option is set - to a True value, arrays used as first parameter to the PSyIR query - operators lbound, ubound, or size will be reported as 'read'. - Otherwise, these accesses will be ignored. :param Any options["USE-ORIGINAL-NAMES"]: if this option is set to a True value, an imported symbol that is renamed (``use mod, a=>b``) will be reported using the original name (``b`` in the example). @@ -81,14 +79,10 @@ class VariablesAccessInfo(dict): # List of valid options and their default values. Note that only the # options method checks this, since it is convenient to pass in options # from the DependencyTools that might contain options for these tools. - # COLLECT-ARRAY-SHAPE-READS: controls if access to the shape of an array - # (e.g. ``ubound(a)`` are reported as read or not at all. Defaults - # to True. # USE-ORIGINAL-NAMES: if set this will report the original names of any # symbol that is being renamed (``use mod, renamed_a=>a``). Defaults # to False. - _DEFAULT_OPTIONS = {"COLLECT-ARRAY-SHAPE-READS": False, - "USE-ORIGINAL-NAMES": False} + _DEFAULT_OPTIONS = {"USE-ORIGINAL-NAMES": False} def __init__(self, nodes=None, options=None): # This dictionary stores the mapping of signatures to the @@ -159,6 +153,9 @@ def __str__(self): mode = "READ" elif self.is_written(signature): mode = "WRITE" + else: + # The data associated with this signature is not accessed. + mode = "NO_DATA_ACCESS" output_list.append(f"{signature}: {mode}") return ", ".join(output_list) @@ -277,12 +274,24 @@ def add_access(self, signature, access_type, node, component_indices=None): def all_signatures(self): ''':returns: all signatures contained in this instance, sorted (in \ order to make test results reproducible). - :rtype: List[:py:class:`psyclone.core.signature`] + :rtype: List[:py:class:`psyclone.core.Signature`] ''' list_of_vars = list(self.keys()) list_of_vars.sort() return list_of_vars + @property + def all_data_accesses(self) -> List[Signature]: + ''' + :returns: all Signatures in this instance that have a data access (i.e. + the data associated with them is read or written). + ''' + result = [] + for sig in self.all_signatures: + if self[sig].has_data_access(): + result.append(sig) + return result + def merge(self, other_access_info): '''Merges data from a VariablesAccessInfo instance to the information in this instance. @@ -319,6 +328,14 @@ def merge(self, other_access_info): # locations just merged in self._location = self._location + max_new_location + def is_called(self, signature: Signature) -> bool: + ''' + :param signature: signature of the variable. + + :returns: True if the specified variable is called at least once. + ''' + return self[signature].is_called() + def is_written(self, signature): '''Checks if the specified variable signature is at least written once. @@ -336,15 +353,14 @@ def is_written(self, signature): var_access_info = self[signature] return var_access_info.is_written() - def is_read(self, signature): + def is_read(self, signature) -> bool: '''Checks if the specified variable signature is at least read once. :param signature: signature of the variable :type signature: :py:class:`psyclone.core.Signature` - :returns: True if the specified variable name is read (at least \ + :returns: True if the specified variable name is read (at least once). - :rtype: bool :raises: KeyError if the signature cannot be found.''' diff --git a/src/psyclone/domain/common/transformations/kernel_module_inline_trans.py b/src/psyclone/domain/common/transformations/kernel_module_inline_trans.py index 84efcf2bc5..bc0c81ce98 100644 --- a/src/psyclone/domain/common/transformations/kernel_module_inline_trans.py +++ b/src/psyclone/domain/common/transformations/kernel_module_inline_trans.py @@ -43,13 +43,13 @@ ''' - +from psyclone.core import VariablesAccessInfo from psyclone.errors import InternalError from psyclone.psyGen import Transformation, CodedKern from psyclone.psyir.transformations import TransformationError from psyclone.psyir.symbols import ( ContainerSymbol, DataSymbol, DataTypeSymbol, DefaultModuleInterface, - IntrinsicSymbol, RoutineSymbol, Symbol) + RoutineSymbol, Symbol) from psyclone.psyir.nodes import ( Container, Reference, Routine, ScopingNode, Literal, CodeBlock, Call, IntrinsicCall) @@ -154,43 +154,26 @@ def validate(self, node, options=None): ) from error # We do not support kernels that use symbols representing data - # declared in their own parent module (we would need to new imports - # from this module for those, and we don't do this yet). - # These can only be found in References and CodeBlocks. - for var in kernel_schedule.walk(Reference): - symbol = var.symbol - if isinstance(symbol, IntrinsicSymbol): - continue - if not symbol.is_import: - if not var.scope.symbol_table.lookup( - symbol.name, scope_limit=kernel_schedule, - otherwise=False): - raise TransformationError( - f"{kern_or_call} '{kname}' contains accesses to " - f"'{symbol.name}' which is declared in the same " - f"module scope. Cannot inline such a {kern_or_call}.") - for block in kernel_schedule.walk(CodeBlock): - for name in block.get_symbol_names(): - # Is this quantity declared within the kernel? - sym = block.scope.symbol_table.lookup( - name, scope_limit=kernel_schedule, otherwise=None) - if not sym: - # It isn't declared in the kernel. - # Can we find the corresponding symbol at all? - sym = block.scope.symbol_table.lookup(name, otherwise=None) - if not sym: - raise TransformationError( - f"{kern_or_call} '{kname}' contains accesses to " - f"'{name}' in a CodeBlock but the origin of this " - f"symbol is unknown.") - # We found it in an outer scope - is it from an import or a - # declaration? - if not sym.is_import: - raise TransformationError( - f"{kern_or_call} '{kname}' contains accesses to " - f"'{name}' in a CodeBlock that is declared in the " - f"same module scope. Cannot inline such a " - f"{kern_or_call}.") + # declared in their own parent module (we would need to add new imports + # from this module at the call site, and we don't do this yet). + # TODO #2424 - this suffers from the limitation that + # VariablesAccessInfo does not work with nested scopes. (e.g. 2 + # different symbols with the same name but declared in different, + # nested scopes will be assumed to be the same symbol). + vai = VariablesAccessInfo(kernel_schedule) + table = kernel_schedule.symbol_table + for sig in vai.all_signatures: + symbol = table.lookup(sig.var_name, otherwise=None) + if not symbol: + raise TransformationError( + f"{kern_or_call} '{kname}' contains accesses to " + f"'{sig.var_name}' but the origin of this signature is " + f"unknown.") + if not symbol.is_import and symbol.name not in table: + raise TransformationError( + f"{kern_or_call} '{kname}' contains accesses to " + f"'{symbol.name}' which is declared in the callee " + f"module scope. Cannot inline such a {kern_or_call}.") # We can't transform subroutines that shadow top-level symbol module # names, because we won't be able to bring this into the subroutine diff --git a/src/psyclone/domain/gocean/kernel/psyir.py b/src/psyclone/domain/gocean/kernel/psyir.py index 9681547406..a21837ae96 100644 --- a/src/psyclone/domain/gocean/kernel/psyir.py +++ b/src/psyclone/domain/gocean/kernel/psyir.py @@ -33,6 +33,7 @@ # ----------------------------------------------------------------------------- # Author: R. W. Ford, STFC Daresbury Lab # Modified: A. R. Porter and S. Siso, STFC Daresbury Lab +# J. Henrichs, Bureau of Meteorology '''This module contains PSyclone Kernel-layer-specific PSyIR classes for the GOcean API. @@ -251,8 +252,9 @@ def create_from_fortran_string(fortran_string): ''' kernel_metadata = GOceanKernelMetadata() - # Ensure the Fortran2003 parser is initialised. - _ = ParserFactory().create(std="f2003") + # Ensure the Fortran parser is initialised. + std = Config.get().fortran_standard + _ = ParserFactory().create(std=std) reader = FortranStringReader(fortran_string) try: spec_part = Fortran2003.Derived_Type_Def(reader) diff --git a/src/psyclone/domain/gocean/transformations/gocean_extract_trans.py b/src/psyclone/domain/gocean/transformations/gocean_extract_trans.py index bbe62df156..20225ffb0b 100644 --- a/src/psyclone/domain/gocean/transformations/gocean_extract_trans.py +++ b/src/psyclone/domain/gocean/transformations/gocean_extract_trans.py @@ -152,29 +152,30 @@ def apply(self, nodes, options=None): is required (and is supported by the runtime library). ''' - - my_options = self.merge_in_default_options(options) + if options is None: + options = {} ctu = CallTreeUtils() nodes = self.get_node_list(nodes) - region_name = self.get_unique_region_name(nodes, my_options) - my_options["region_name"] = region_name - my_options["prefix"] = my_options.get("prefix", "extract") + region_name = self.get_unique_region_name(nodes, options) + options["region_name"] = region_name + options["prefix"] = options.get("prefix", "extract") - read_write_info = ctu.get_in_out_parameters(nodes, options=my_options) + read_write_info = ctu.get_in_out_parameters( + nodes, include_non_data_accesses=True) # Determine a unique postfix to be used for output variables # that avoid any name clashes postfix = ExtractTrans.determine_postfix(read_write_info, postfix="_post") - my_options["post_var_postfix"] = postfix + options["post_var_postfix"] = postfix - if my_options.get("create_driver", False): + if options.get("create_driver", False): # We need to create the driver before inserting the ExtractNode # (since some of the visitors used in driver creation do not # handle an ExtractNode in the tree) self._driver_creator.write_driver(nodes, read_write_info, postfix=postfix, - prefix=my_options["prefix"], + prefix=options["prefix"], region_name=region_name) - super().apply(nodes, my_options) + super().apply(nodes, options) diff --git a/src/psyclone/domain/lfric/arg_ordering.py b/src/psyclone/domain/lfric/arg_ordering.py index a935a79c8e..ce00697750 100644 --- a/src/psyclone/domain/lfric/arg_ordering.py +++ b/src/psyclone/domain/lfric/arg_ordering.py @@ -761,6 +761,9 @@ def scalar(self, scalar_arg, var_accesses=None): # information. We do this by providing None as var access. self.append(scalar_arg.name, None, mode=scalar_arg.access, metadata_posn=scalar_arg.metadata_index) + if scalar_arg.precision and var_accesses is not None: + var_accesses.add_access(Signature(scalar_arg.precision), + AccessType.TYPE_INFO, self._kern) else: self.append(scalar_arg.name, var_accesses, mode=scalar_arg.access, metadata_posn=scalar_arg.metadata_index) diff --git a/src/psyclone/domain/lfric/kernel/common_metadata.py b/src/psyclone/domain/lfric/kernel/common_metadata.py index 0c4f8b9455..b74c1424bc 100644 --- a/src/psyclone/domain/lfric/kernel/common_metadata.py +++ b/src/psyclone/domain/lfric/kernel/common_metadata.py @@ -43,6 +43,8 @@ from fparser.two.parser import ParserFactory from fparser.two.utils import NoMatchError, FortranSyntaxError +from psyclone.configuration import Config + # TODO issue #1886. This class and its subclasses may have # commonalities with the GOcean metadata processing. @@ -120,7 +122,8 @@ def create_fparser2(fortran_string, encoding): expected form. ''' - _ = ParserFactory().create(std="f2003") + std = Config.get().fortran_standard + _ = ParserFactory().create(std=std) reader = FortranStringReader(fortran_string) match = True try: diff --git a/src/psyclone/domain/lfric/lfric_extract_driver_creator.py b/src/psyclone/domain/lfric/lfric_extract_driver_creator.py index b3aff39cbf..395602903b 100644 --- a/src/psyclone/domain/lfric/lfric_extract_driver_creator.py +++ b/src/psyclone/domain/lfric/lfric_extract_driver_creator.py @@ -583,6 +583,7 @@ def _sym_is_field(sym): # variables have References, and will already have been declared # in the symbol table (in _add_all_kernel_symbols). sig_str = self._flatten_signature(signature) + if module_name: mod_info = mod_man.get_module_info(module_name) orig_sym = mod_info.get_symbol(signature[0]) diff --git a/src/psyclone/gocean1p0.py b/src/psyclone/gocean1p0.py index 483be1653c..75116cbe46 100644 --- a/src/psyclone/gocean1p0.py +++ b/src/psyclone/gocean1p0.py @@ -1221,13 +1221,6 @@ def reference_accesses(self, var_accesses): super().reference_accesses(var_accesses) var_accesses.next_location() - def local_vars(self): - '''Return a list of the variable (names) that are local to this loop - (and must therefore be e.g. threadprivate if doing OpenMP) - - ''' - return [] - @property def index_offset(self): ''' The grid index-offset convention that this kernel expects ''' diff --git a/src/psyclone/parse/file_info.py b/src/psyclone/parse/file_info.py index 5cfd5c81ac..06b7f25149 100644 --- a/src/psyclone/parse/file_info.py +++ b/src/psyclone/parse/file_info.py @@ -412,10 +412,11 @@ def get_fparser_tree( return self._fparser_tree try: + config = Config.get() reader = FortranStringReader( - source_code, include_dirs=Config.get().include_paths + source_code, include_dirs=config.include_paths ) - parser = ParserFactory().create(std="f2008") + parser = ParserFactory().create(std=config.fortran_standard) self._fparser_tree = parser(reader) except Exception as err: diff --git a/src/psyclone/parse/kernel.py b/src/psyclone/parse/kernel.py index eb987b3735..201c031881 100644 --- a/src/psyclone/parse/kernel.py +++ b/src/psyclone/parse/kernel.py @@ -58,7 +58,7 @@ import psyclone.expression as expr from psyclone.errors import InternalError -from psyclone.configuration import LFRIC_API_NAMES, GOCEAN_API_NAMES +from psyclone.configuration import Config, LFRIC_API_NAMES, GOCEAN_API_NAMES from psyclone.parse.utils import check_api, check_line_length, ParseError @@ -912,8 +912,8 @@ def get_integer_variable(self, name): :raises ParseError: if the RHS of the assignment is not a Name. ''' - # Ensure the Fortran2008 parser is initialised - _ = ParserFactory().create(std="f2008") + # Ensure the Fortran parser is initialised + _ = ParserFactory().create(std=Config.get().fortran_standard) # Fortran is not case sensitive so nor is our matching lower_name = name.lower() @@ -955,8 +955,8 @@ def get_integer_array(self, name): does not match the extent of the array. ''' - # Ensure the classes are setup for the Fortran2008 parser - _ = ParserFactory().create(std="f2008") + # Ensure the classes are setup for the Fortran parser + _ = ParserFactory().create(std=Config.get().fortran_standard) # Fortran is not case sensitive so nor is our matching lower_name = name.lower() diff --git a/src/psyclone/parse/utils.py b/src/psyclone/parse/utils.py index 8f5b111057..474c78a2c5 100644 --- a/src/psyclone/parse/utils.py +++ b/src/psyclone/parse/utils.py @@ -114,7 +114,6 @@ def parse_fp2(filename): :raises ParseError: if the file could not be parsed. ''' - parser = ParserFactory().create(std="f2008") # We get the directories to search for any Fortran include files from # our configuration object. config = Config.get() @@ -124,6 +123,7 @@ def parse_fp2(filename): raise ParseError( f"algorithm.py:parse_fp2: Failed to parse file '{filename}'. " f"Error returned was ' {error} '.") from error + parser = ParserFactory().create(std=config.fortran_standard) try: parse_tree = parser(reader) except FortranSyntaxError as msg: diff --git a/src/psyclone/psyGen.py b/src/psyclone/psyGen.py index 8730288e4a..a2fb3438f5 100644 --- a/src/psyclone/psyGen.py +++ b/src/psyclone/psyGen.py @@ -1323,9 +1323,6 @@ def is_coloured(self): def iterates_over(self): return self._iterates_over - def local_vars(self): - raise NotImplementedError("Kern.local_vars should be implemented") - def gen_code(self, parent): raise NotImplementedError("Kern.gen_code should be implemented") @@ -1576,8 +1573,9 @@ def ast(self): return self._fp2_ast # Use the fparser1 AST to generate Fortran source fortran = self._module_code.tofortran() - # Create an fparser2 Fortran2008 parser - my_parser = parser.ParserFactory().create(std="f2008") + # Create an fparser2 Fortran parser + std = Config.get().fortran_standard + my_parser = parser.ParserFactory().create(std=std) # Parse that Fortran using our parser reader = FortranStringReader(fortran) self._fp2_ast = my_parser(reader) @@ -1815,14 +1813,6 @@ def _validate_child(position, child): ''' return position == 0 and isinstance(child, Schedule) - @abc.abstractmethod - def local_vars(self): - ''' - :returns: list of the variable (names) that are local to this kernel \ - (and must therefore be e.g. threadprivate if doing OpenMP) - :rtype: list of str - ''' - def node_str(self, colour=True): ''' Returns the name of this node with (optional) control codes to generate coloured output in a terminal that supports it. @@ -1867,12 +1857,6 @@ def load(self, call, arguments, parent=None): name = call.ktype.name super(BuiltIn, self).__init__(parent, call, name, arguments) - def local_vars(self): - '''Variables that are local to this built-in and therefore need to be - made private when parallelising using OpenMP or similar. By default - builtin's do not have any local variables so set to nothing''' - return [] - class Arguments(): ''' diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index 3617d4fe73..f87c1141ec 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -337,14 +337,13 @@ def _reverse_map(reverse_dict, op_map): mapping. Any key that does already exist in `reverse_dict` is not overwritten, only new keys are added. - :param reverse_dict: the dictionary to which the new mapping of \ + :param reverse_dict: the dictionary to which the new mapping of operator to string is added. - :type reverse_dict: dict from \ - :py:class:`psyclone.psyir.nodes.BinaryOperation`, \ - :py:class:`psyclone.psyir.nodes.NaryOperation` or \ - :py:class:`psyclone.psyir.nodes.UnaryOperation` to str + :type reverse_dict: dict[ + :py:class:`psyclone.psyir.nodes.Operation`, str + ] - :param op_map: mapping from string representation of operator to \ + :param op_map: mapping from string representation of operator to enumerated type. :type op_map: :py:class:`collections.OrderedDict` @@ -1282,14 +1281,16 @@ def binaryoperation_node(self, node): return f"({lhs} {fort_oper} {rhs})" if precedence(fort_oper) == precedence(parent_fort_oper): # We still may need to enforce precedence - if (isinstance(parent, UnaryOperation) or - (isinstance(parent, BinaryOperation) and - parent.children[1] == node)): - # We need brackets to enforce precedence - # as a) a unary operator is performed - # before a binary operator and b) floating - # point operations are not actually - # associative due to rounding errors. + if ( + # If parent is a UnaryOperation + isinstance(parent, UnaryOperation) or + # Or it is a BinaryOperation ... + (isinstance(parent, BinaryOperation) and + # ... with right-to-left precedence + (parent.children[1] == node) or + # ... or originally had explicit parenthesis + node.has_explicit_grouping) + ): return f"({lhs} {fort_oper} {rhs})" return f"{lhs} {fort_oper} {rhs}" except KeyError as error: diff --git a/src/psyclone/psyir/frontend/fortran.py b/src/psyclone/psyir/frontend/fortran.py index f3205a35dd..2a920181d0 100644 --- a/src/psyclone/psyir/frontend/fortran.py +++ b/src/psyclone/psyir/frontend/fortran.py @@ -32,6 +32,7 @@ # ----------------------------------------------------------------------------- # Author: S. Siso, STFC Daresbury Lab # Modifications: A. R. Porter, N. Nobre and R. W. Ford, STFC Daresbury Lab +# J. Henrichs, Bureau of Meteorology # ----------------------------------------------------------------------------- ''' This module provides the PSyIR Fortran front-end.''' @@ -84,7 +85,8 @@ def __init__(self, free_form: bool = True, ignore_comments: bool = True, last_comments_as_codeblocks: bool = False, resolve_modules: Union[bool, List[str]] = False): if not self._parser: - self._parser = ParserFactory().create(std="f2008") + std = Config.get().fortran_standard + self._parser = ParserFactory().create(std=std) self._free_form = free_form if ignore_comments and not ignore_directives: raise ValueError( diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index af41068e57..7b84dd2027 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -3445,7 +3445,7 @@ def _add_target_attribute(var_name, table): f"subroutine dummy()\n" f" {datatype.declaration}\n" f"end subroutine\n") - parser = ParserFactory().create(std="f2008") + parser = ParserFactory().create(std=Config.get().fortran_standard) reader = FortranStringReader(dummy_code) fp2_ast = parser(reader) type_decl_stmt = fp2_ast.children[0].children[1].children[0] @@ -3695,7 +3695,8 @@ def _create_select_type( # Parse the the created Fortran text to an fparser2 tree and # store the resulting tree in a PSyIR CodeBlock. - parser = ParserFactory().create(std="f2008") + std = Config.get().fortran_standard + parser = ParserFactory().create(std=std) reader = FortranStringReader(code) fp2_program = parser(reader) # Ignore the program part of the fparser2 tree @@ -4895,7 +4896,17 @@ def _parenthesis_handler(self, node, parent): # Use the items[1] content of the node as it contains the required # information (items[0] and items[2] just contain the left and right # brackets as strings so can be disregarded. - return self._create_child(node.items[1], parent) + new_node = self._create_child(node.items[1], parent) + + # Explicit parenthesis on BinaryOperations are sometimes needed for + # reproducibility (because a Fortran compiler may evaluate any + # mathematically-equivalent expression, provided that the integrity + # of parentheses is not violated - Fortran2008 section 7.1.5.2.4), + # so we store the fact that they are here. + if isinstance(new_node, BinaryOperation): + new_node.has_explicit_grouping = True + + return new_node def _part_ref_handler(self, node, parent): ''' diff --git a/src/psyclone/psyir/nodes/call.py b/src/psyclone/psyir/nodes/call.py index f49ce59577..9abe9d71f1 100644 --- a/src/psyclone/psyir/nodes/call.py +++ b/src/psyclone/psyir/nodes/call.py @@ -315,16 +315,20 @@ def reference_accesses(self, var_accesses): # We conservatively default to READWRITE otherwise (TODO #446). default_access = AccessType.READWRITE - # TODO #2271: This may skip references in inner expressions of - # structure calls, but to implement properly we new a new kind of - # AccessType that represents being called (USED but not READ, maybe - # the same that we need for INQUIRY type attributes?) + # The RoutineSymbol has a CALL access. + sig, indices_list = self.routine.get_signature_and_indices() + var_accesses.add_access(sig, AccessType.CALL, self.routine) + # Continue processing references in any index expressions. + for indices in indices_list: + for idx in indices: + idx.reference_accesses(var_accesses) + for arg in self.arguments: if isinstance(arg, Reference): # This argument is pass-by-reference. sig, indices_list = arg.get_signature_and_indices() var_accesses.add_access(sig, default_access, arg) - # Any symbols referenced in any index expressions are READ. + # Continue processing references in any index expressions. for indices in indices_list: for idx in indices: idx.reference_accesses(var_accesses) @@ -373,12 +377,13 @@ def is_elemental(self): @property def is_pure(self): ''' - :returns: whether the routine being called is pure (guaranteed to \ - return the same result when provided with the same argument \ + :returns: whether the routine being called is pure (guaranteed to + return the same result when provided with the same argument values). If this information is not known then it returns None. :rtype: NoneType | bool ''' - if self.routine and self.routine.symbol: + if (self.routine and self.routine.symbol and + isinstance(self.routine.symbol, RoutineSymbol)): return self.routine.symbol.is_pure return None diff --git a/src/psyclone/psyir/nodes/codeblock.py b/src/psyclone/psyir/nodes/codeblock.py index 0244084284..072f934d67 100644 --- a/src/psyclone/psyir/nodes/codeblock.py +++ b/src/psyclone/psyir/nodes/codeblock.py @@ -39,8 +39,11 @@ ''' This module contains the CodeBlock node implementation.''' from enum import Enum +from typing import List + from fparser.two import Fortran2003 from fparser.two.utils import walk +from psyclone.core import AccessType, Signature, VariablesAccessInfo from psyclone.psyir.nodes.statement import Statement from psyclone.psyir.nodes.datanode import DataNode @@ -150,10 +153,25 @@ def node_str(self, colour=True): return (f"{self.coloured_name(colour)}[" f"{list(map(type, self._fp2_nodes))}]") - def get_symbol_names(self): + def get_symbol_names(self) -> List[str]: ''' - :returns: the list of symbol names used inside the CodeBock. - :rtype: list[str] + Analyses the fparser2 parse tree associated with this CodeBlock and + returns the names of all symbols accessed within it. Since, by + definition, we do not understand the contents of a CodeBlock, we do not + attempt to analyse how these symbols are accessed - they are all marked + as being READWRITE (this includes the names of any routines that might + be called). + + Note that the names of any Fortran intrinsics are *not* included in the + result. If the original code has unwisely overridden a Fortran + intrinsic then fparser *may* incorrectly identify the use of such a + variable/routine as still being an intrinsic call and, as such, it will + be omitted from the names returned by this method. + + TODO #2863 - these limitations (blanket use of READWRITE and the + ignoring of Fortran intrinsics) need to be re-visited. + + :returns: the symbol names used inside the CodeBock. ''' parse_tree = self.get_ast_nodes result = [] @@ -174,8 +192,45 @@ def get_symbol_names(self): Fortran2003.End_If_Stmt)): # We don't want labels associated with loop or branch control. result.append(node.string) - + # Precision on literals requires special attention since they are just + # stored in the tree as str (fparser/#456). + for node in walk(parse_tree, (Fortran2003.Int_Literal_Constant, + Fortran2003.Real_Literal_Constant, + Fortran2003.Logical_Literal_Constant, + Fortran2003.Char_Literal_Constant)): + if node.items[1]: + result.append(node.items[1]) + # Complex literals require even more special attention. + for node in walk(parse_tree, Fortran2003.Complex_Literal_Constant): + # A complex literal constant has a real part and an imaginary part. + # Each of these can have a kind. + for part in node.items: + if part.items[1]: + result.append(part.items[1]) return result + def reference_accesses(self, var_accesses: VariablesAccessInfo): + ''' + Get all variable access information. Since this is a CodeBlock we + only know the names of symbols accessed within it but not how they + are accessed. Therefore we err on the side of caution and mark + them all as READWRITE, unfortunately, this will include the names of + any routines that are called. + + TODO #2863 - it would be better to use AccessType.UNKNOWN here but + currently VariablesAccessInfo does not consider that type of access. + + This method makes use of + :py:meth:`~psyclone.psyir.nodes.CodeBlock.get_symbol_names` and is + therefore subject to the same limitations as that method. + + :param var_accesses: VariablesAccessInfo instance that stores the + information about variable accesses. + + ''' + for name in self.get_symbol_names(): + var_accesses.add_access(Signature(name), AccessType.READWRITE, + self) + def __str__(self): return f"CodeBlock[{len(self._fp2_nodes)} nodes]" diff --git a/src/psyclone/psyir/nodes/directive.py b/src/psyclone/psyir/nodes/directive.py index fffb221651..32ce89b27d 100644 --- a/src/psyclone/psyir/nodes/directive.py +++ b/src/psyclone/psyir/nodes/directive.py @@ -105,6 +105,10 @@ def create_data_movement_deep_copy_refs(self): node = vinfo.all_accesses[0].node sym = table.lookup(sig.var_name) + if not vinfo.has_data_access(): + # Ignore references that don't correspond to data accesses. + continue + if isinstance(sym.datatype, ScalarType): # We ignore scalars as these are typically copied by value. continue diff --git a/src/psyclone/psyir/nodes/extract_node.py b/src/psyclone/psyir/nodes/extract_node.py index 8184f6afb3..26c6c59b1f 100644 --- a/src/psyclone/psyir/nodes/extract_node.py +++ b/src/psyclone/psyir/nodes/extract_node.py @@ -171,8 +171,7 @@ def gen_code(self, parent): from psyclone.psyir.tools.call_tree_utils import CallTreeUtils # Determine the variables to write: ctu = CallTreeUtils() - self._read_write_info = \ - ctu.get_in_out_parameters(self, options=self.options) + self._read_write_info = ctu.get_in_out_parameters(self) options = {'pre_var_list': self._read_write_info.read_list, 'post_var_list': self._read_write_info.write_list, @@ -209,8 +208,8 @@ def lower_to_language_level(self): from psyclone.psyir.tools.call_tree_utils import CallTreeUtils # Determine the variables to write: ctu = CallTreeUtils() - self._read_write_info = \ - ctu.get_in_out_parameters(self, options=self.options) + self._read_write_info = ctu.get_in_out_parameters( + self, include_non_data_accesses=True) options = {'pre_var_list': self._read_write_info.read_list, 'post_var_list': self._read_write_info.write_list, diff --git a/src/psyclone/psyir/nodes/intrinsic_call.py b/src/psyclone/psyir/nodes/intrinsic_call.py index 5ce6ee2937..3895cac10a 100644 --- a/src/psyclone/psyir/nodes/intrinsic_call.py +++ b/src/psyclone/psyir/nodes/intrinsic_call.py @@ -42,6 +42,7 @@ from collections.abc import Iterable from enum import Enum +from psyclone.core import AccessType from psyclone.psyir.nodes.call import Call from psyclone.psyir.nodes.datanode import DataNode from psyclone.psyir.nodes.literal import Literal @@ -905,25 +906,30 @@ def create(cls, intrinsic, arguments=()): def reference_accesses(self, var_accesses): '''Get all reference access information from this node. - If the 'COLLECT-ARRAY-SHAPE-READS' options is set, it will report array - accesses used as first parameter in 'inquiry intrinsics' like - `lbound`, `ubound`, or `size` as 'read' accesses. + + Any variables used as the first argument to 'inquiry intrinsics' like + `lbound`, `ubound`, or `size` are marked as having INQUIRY accesses. :param var_accesses: VariablesAccessInfo instance that stores the information about variable accesses. :type var_accesses: :py:class:`psyclone.core.VariablesAccessInfo` ''' - if (self.intrinsic.is_inquiry and not - var_accesses.options("COLLECT-ARRAY-SHAPE-READS")): + if self.intrinsic.is_inquiry and isinstance(self.arguments[0], + Reference): # If this is an inquiry access (which doesn't actually access the - # value) and we haven't explicitly requested them, ignore the - # inquired variables, which are always the first argument. - for child in self.arguments[1:]: - child.reference_accesses(var_accesses) - else: - for child in self.arguments: - child.reference_accesses(var_accesses) + # value) then make sure we use the correct access type for the + # inquired variable, which is always the first argument. + sig, indices = self.arguments[0].get_signature_and_indices() + var_accesses.add_access(sig, AccessType.INQUIRY, self.arguments[0]) + for idx_list in indices: + for idx in idx_list: + idx.reference_accesses(var_accesses) + elif self.arguments: + self.arguments[0].reference_accesses(var_accesses) + + for child in self.arguments[1:]: + child.reference_accesses(var_accesses) # TODO #2102: Maybe the three properties below can be removed if intrinsic # is a symbol, as they would act as the super() implementation. diff --git a/src/psyclone/psyir/nodes/literal.py b/src/psyclone/psyir/nodes/literal.py index 7b05bd9f40..d710df5596 100644 --- a/src/psyclone/psyir/nodes/literal.py +++ b/src/psyclone/psyir/nodes/literal.py @@ -41,8 +41,9 @@ import re +from psyclone.core import AccessType, Signature, VariablesAccessInfo from psyclone.psyir.nodes.datanode import DataNode -from psyclone.psyir.symbols import ScalarType, ArrayType +from psyclone.psyir.symbols import ScalarType, Symbol, ArrayType class Literal(DataNode): @@ -170,6 +171,19 @@ def node_str(self, colour=True): return (f"{self.coloured_name(colour)}" f"[value:'{self._value}', {self.datatype}]") + def reference_accesses(self, access_info: VariablesAccessInfo): + ''' + Get all variable access information. Since this is a Literal, the + only possible place a symbol might be present is in the precision. + + :param var_accesses: VariablesAccessInfo instance that stores the + information about variable accesses. + + ''' + if isinstance(self.datatype.precision, Symbol): + access_info.add_access(Signature(self.datatype.precision.name), + AccessType.TYPE_INFO, self) + def replace_symbols_using(self, table): ''' Replace any Symbols referred to by this object with those in the diff --git a/src/psyclone/psyir/nodes/omp_directives.py b/src/psyclone/psyir/nodes/omp_directives.py index 61ba4e03a1..af0ae7a541 100644 --- a/src/psyclone/psyir/nodes/omp_directives.py +++ b/src/psyclone/psyir/nodes/omp_directives.py @@ -1596,6 +1596,8 @@ def infer_sharing_attributes(self): var_accesses = VariablesAccessInfo() self.reference_accesses(var_accesses) for signature in var_accesses.all_signatures: + if not var_accesses[signature].has_data_access(): + continue accesses = var_accesses[signature].all_accesses # TODO #2094: var_name only captures the top-level # component in the derived type accessor. If the attributes @@ -1619,7 +1621,7 @@ def infer_sharing_attributes(self): continue # All arrays not explicitly marked as threadprivate are shared - if accesses[0].is_array(): + if any(accs.is_array() for accs in accesses): continue # If a variable is only accessed once, it is either an error diff --git a/src/psyclone/psyir/nodes/operation.py b/src/psyclone/psyir/nodes/operation.py index 37e8490df4..f276f277fd 100644 --- a/src/psyclone/psyir/nodes/operation.py +++ b/src/psyclone/psyir/nodes/operation.py @@ -54,13 +54,13 @@ class Operation(DataNode, metaclass=ABCMeta): Abstract base class for PSyIR nodes representing operators. :param operator: the operator used in the operation. - :type operator: :py:class:`psyclone.psyir.nodes.UnaryOperation.Operator` \ - or :py:class:`psyclone.psyir.nodes.BinaryOperation.Operator` or \ - :py:class:`psyclone.psyir.nodes.NaryOperation.Operator` + :type operator: Union[ + :py:class:`psyclone.psyir.nodes.UnaryOperation.Operator`, + :py:class:`psyclone.psyir.nodes.BinaryOperation.Operator`] :param parent: the parent node of this Operation in the PSyIR. - :type parent: :py:class:`psyclone.psyir.nodes.Node` + :type parent: Optional[:py:class:`psyclone.psyir.nodes.Node`] - :raises TypeError: if the supplied operator is not an instance of \ + :raises TypeError: if the supplied operator is not an instance of self.Operator. ''' @@ -101,9 +101,9 @@ def operator(self): Return the operator. :returns: Enumerated type capturing the operator. - :rtype: :py:class:`psyclone.psyir.nodes.UnaryOperation.Operator` or \ - :py:class:`psyclone.psyir.nodes.BinaryOperation.Operator` or \ - :py:class:`psyclone.psyir.nodes.NaryOperation.Operator` + :rtype: Union[ + :py:class:`psyclone.psyir.nodes.UnaryOperation.Operator` + :py:class:`psyclone.psyir.nodes.BinaryOperation.Operator`] ''' return self._operator @@ -173,7 +173,7 @@ def create(operator, operand): :py:class:`psyclone.psyir.nodes.UnaryOperation.Operator` :param operand: the PSyIR node that oper operates on, or a tuple containing the name of the argument and the PSyIR node. - :type operand: Union[:py:class:`psyclone.psyir.nodes.Node` | + :type operand: Union[:py:class:`psyclone.psyir.nodes.Node`, Tuple[str, :py:class:`psyclone.psyir.nodes.Node`]] :returns: a UnaryOperation instance. @@ -209,6 +209,14 @@ class BinaryOperation(Operation): Node representing a BinaryOperation expression. As such it has two operands as children 0 and 1, and an attribute with the operator type. + :param operator: the operator used in the operation. + :type operator: :py:class:`psyclone.psyir.nodes.BinaryOperation.Operator` + :param bool has_explicit_grouping: Whether this operation should be + surrounded by explicit grouping syntax (e.g. parenthesis) regardless of + not breaking any other precedence rules. Defaults to False. + :param parent: the parent node of this Operation in the PSyIR. + :type parent: Optional[:py:class:`psyclone.psyir.nodes.Node`] + ''' #: The Operators that a BinaryOperation can represent. Operator = Enum('Operator', [ @@ -225,6 +233,10 @@ class BinaryOperation(Operation): # Textual description of the node. _children_valid_format = "DataNode, DataNode" + def __init__(self, operator, has_explicit_grouping=False, parent=None): + super().__init__(operator, parent=parent) + self.has_explicit_grouping = has_explicit_grouping + @staticmethod def _validate_child(position, child): ''' @@ -239,7 +251,7 @@ def _validate_child(position, child): return position in (0, 1) and isinstance(child, DataNode) @staticmethod - def create(operator, lhs, rhs): + def create(operator, lhs, rhs, has_explicit_grouping=False): '''Create a BinaryOperator instance given an operator and lhs and rhs child instances with optional names. @@ -256,6 +268,10 @@ def create(operator, lhs, rhs): argument and the PSyIR node. :type rhs: Union[:py:class:`psyclone.psyir.nodes.Node`, Tuple[str, :py:class:`psyclone.psyir.nodes.Node`]] + :param bool has_explicit_grouping: Whether this operation should be + surrounded by explicit grouping syntax (e.g. parenthesis) + regardless of not breaking any other precedence rules. Defaults to + False. :returns: a BinaryOperator instance. :rtype: :py:class:`psyclone.psyir.nodes.BinaryOperation` @@ -270,11 +286,35 @@ def create(operator, lhs, rhs): f"operator argument in create method of BinaryOperation class " f"should be a PSyIR BinaryOperation Operator but found " f"'{type(operator).__name__}'.") - binary_op = BinaryOperation(operator) + binary_op = BinaryOperation(operator, has_explicit_grouping) binary_op.addchild(lhs) binary_op.addchild(rhs) return binary_op + @property + def has_explicit_grouping(self) -> bool: + ''' + :returns: Whether this operation should be surrounded by explicit + grouping syntax (e.g. parenthesis) regardless of not breaking any + other precedence rules. + ''' + return self._has_explicit_grouping + + @has_explicit_grouping.setter + def has_explicit_grouping(self, value: bool): + ''' + :param value: Whether this operation should be surrounded by explicit + grouping syntax (e.g. parenthesis) regardless of not breaking any + other precedence rules. + + :raises TypeError: if the provided value is not a boolean. + + ''' + if not isinstance(value, bool): + raise TypeError(f"BinaryOperation.has_explicit_grouping must be " + f"boolean, but found '{type(value).__name__}'.") + self._has_explicit_grouping = value + def _get_result_precision(self, precisions): ''' Compares the two precisions to determine the precision of the result diff --git a/src/psyclone/psyir/nodes/psy_data_node.py b/src/psyclone/psyir/nodes/psy_data_node.py index 262dd2e8b2..d82b2b0c3c 100644 --- a/src/psyclone/psyir/nodes/psy_data_node.py +++ b/src/psyclone/psyir/nodes/psy_data_node.py @@ -774,7 +774,7 @@ def gen_type_bound_call(typename, methodname, argument_list=None, argument_str += ",".join([str(arg) for arg in argument_list]) argument_str += ")" - ParserFactory().create(std="f2008") + ParserFactory().create(std=Config.get().fortran_standard) reader = FortranStringReader( f"CALL {typename}%{methodname}{argument_str}") # Tell the reader that the source is free format diff --git a/src/psyclone/psyir/nodes/read_only_verify_node.py b/src/psyclone/psyir/nodes/read_only_verify_node.py index cd9b610f6a..669ba786a8 100644 --- a/src/psyclone/psyir/nodes/read_only_verify_node.py +++ b/src/psyclone/psyir/nodes/read_only_verify_node.py @@ -89,7 +89,8 @@ def lower_to_language_level(self): from psyclone.psyir.tools import ReadWriteInfo ctu = CallTreeUtils() read_write_info = ReadWriteInfo() - ctu.get_input_parameters(read_write_info, self, options=self.options) + ctu.get_input_parameters(read_write_info, self, + include_non_data_accesses=True) options = {'pre_var_list': read_write_info.read_list, 'post_var_list': read_write_info.read_list} diff --git a/src/psyclone/psyir/nodes/scoping_node.py b/src/psyclone/psyir/nodes/scoping_node.py index eeb3d855ac..440166dfbd 100644 --- a/src/psyclone/psyir/nodes/scoping_node.py +++ b/src/psyclone/psyir/nodes/scoping_node.py @@ -36,8 +36,11 @@ ''' This module contains the ScopingNode implementation.''' +from psyclone.core import AccessType, Signature, VariablesAccessInfo from psyclone.psyir.nodes.node import Node -from psyclone.psyir.symbols import RoutineSymbol, SymbolError, SymbolTable +from psyclone.psyir.symbols import ( + ArrayType, DataType, DataTypeSymbol, RoutineSymbol, StructureType, Symbol, + SymbolError, SymbolTable, UnsupportedFortranType) class ScopingNode(Node): @@ -155,10 +158,10 @@ def _refine_copy(self, other): pass super(ScopingNode, self)._refine_copy(other) - # pylint: disable=protected-access # Add any routine tags back for tag in removed_tags.keys(): + # pylint: disable-next=protected-access self._symbol_table._tags[tag] = self._symbol_table.lookup( removed_tags[tag].name) @@ -170,6 +173,68 @@ def _refine_copy(self, other): # call to _refine_copy and only do this call when that depth is zero. self.replace_symbols_using(self._symbol_table) + def reference_accesses(self, access_info: VariablesAccessInfo): + ''' + Get all variable access information. This specialisation is required + to query the SymbolTable associated with a Scoping node and ensure + that any Symbols appearing in precision specifications, array shapes or + initialisation expressions are captured. + + :param var_accesses: VariablesAccessInfo instance that stores the + information about variable accesses. + + ''' + def _get_accesses(dtype: DataType, info: VariablesAccessInfo): + ''' + Store information on any symbols referenced within the supplied + datatype. + + :param dtype: the datatype to query. + :param info: the VariablesAccessInfo instance in which to store + information. + ''' + if (hasattr(dtype, "precision") and isinstance(dtype.precision, + Symbol)): + # The use of a Symbol to specify precision does not constitute + # a read (since it is resolved at compile time). + access_info.add_access( + Signature(dtype.precision.name), + AccessType.TYPE_INFO, self) + + if isinstance(dtype, DataTypeSymbol): + # The use of a DataTypeSymbol in a declaration is a compile- + # time access. + info.add_access(Signature(dtype.name), + AccessType.TYPE_INFO, self) + elif isinstance(dtype, StructureType): + for cmpt in sym.datatype.components.values(): + # Recurse for members of a StructureType + _get_accesses(cmpt.datatype, info) + if cmpt.initial_value: + cmpt.initial_value.reference_accesses(info) + elif isinstance(dtype, ArrayType): + for dim in dtype.shape: + if isinstance(dim, ArrayType.ArrayBounds): + dim.lower.reference_accesses(access_info) + dim.upper.reference_accesses(access_info) + elif (isinstance(dtype, UnsupportedFortranType) and + dtype.partial_datatype): + # Recurse to examine partial datatype information. + _get_accesses(dtype.partial_datatype, info) + + # Examine the datatypes and initial values of all DataSymbols. + for sym in self._symbol_table.datasymbols: + _get_accesses(sym.datatype, access_info) + + if sym.initial_value: + sym.initial_value.reference_accesses(access_info) + + # Examine the definition of each DataTypeSymbol. + for sym in self._symbol_table.datatypesymbols: + _get_accesses(sym.datatype, access_info) + + super().reference_accesses(access_info) + def replace_symbols_using(self, table): ''' Update any Symbols referenced by this Node (and its descendants) with diff --git a/src/psyclone/psyir/nodes/structure_reference.py b/src/psyclone/psyir/nodes/structure_reference.py index ac04a2e1e9..2479aba66a 100644 --- a/src/psyclone/psyir/nodes/structure_reference.py +++ b/src/psyclone/psyir/nodes/structure_reference.py @@ -336,7 +336,8 @@ def _get_cursor_shape(cursor, cursor_type): if not isinstance(cursor_type, (UnresolvedType, UnsupportedType)): # Once we've hit an Unresolved/UnsupportedType the cursor_type # will remain set to that as we can't do any better. - cursor_type = cursor_type.components[cursor.name].datatype + cursor_type = cursor_type.components[ + cursor.name.lower()].datatype try: cursor_shape = _get_cursor_shape(cursor, cursor_type) except NotImplementedError: diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index 02e16f4861..7685f0db13 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -44,6 +44,7 @@ from enum import Enum from typing import Any, Union +from psyclone.configuration import Config from psyclone.errors import InternalError from psyclone.psyir.commentable_mixin import CommentableMixin from psyclone.psyir.symbols.data_type_symbol import DataTypeSymbol @@ -219,7 +220,7 @@ def type_text(self): string_reader = FortranStringReader(self._declaration) # Set reader to free format. string_reader.set_format(FortranFormat(True, False)) - ParserFactory().create(std="f2008") + ParserFactory().create(std=Config.get().fortran_standard) try: ptree = Fortran2003.Specification_Part( string_reader) @@ -641,8 +642,8 @@ def _validate_shape(self, extents): :type extents: List[ :py:class:`psyclone.psyir.symbols.ArrayType.Extent` | int | :py:class:`psyclone.psyir.nodes.DataNode` | - Tuple[int | :py:class:`psyclone.psyir.nodes.DataNode | - :py:class:`psyclone.psyir.symbols.ArrayType.Extent]] + Tuple[int | :py:class:`psyclone.psyir.nodes.DataNode` | + :py:class:`psyclone.psyir.symbols.ArrayType.Extent`]] :raises TypeError: if extents is not a list. :raises TypeError: if one or more of the supplied extents is a @@ -1050,23 +1051,25 @@ def add(self, name, datatype, visibility, initial_value, f"be a 'str' but got " f"'{type(inline_comment).__name__}'") - self._components[name] = self.ComponentType(name, datatype, visibility, - initial_value) + key_name = name.lower() + self._components[key_name] = self.ComponentType(name, datatype, + visibility, + initial_value) # Use object.__setattr__ due to the frozen nature of ComponentType - object.__setattr__(self._components[name], + object.__setattr__(self._components[key_name], "_preceding_comment", preceding_comment) - object.__setattr__(self._components[name], + object.__setattr__(self._components[key_name], "_inline_comment", inline_comment) def lookup(self, name): ''' - :returns: the ComponentType tuple describing the named member of this \ + :returns: the ComponentType tuple describing the named member of this StructureType. :rtype: :py:class:`psyclone.psyir.symbols.StructureType.ComponentType` ''' - return self._components[name] + return self._components[name.lower()] def __eq__(self, other): ''' @@ -1111,7 +1114,8 @@ def replace_symbols_using(self, table): if component.initial_value: component.initial_value.replace_symbols_using(table) # Construct the new ComponentType - new_components[component.name] = StructureType.ComponentType( + key_name = component.name.lower() + new_components[key_name] = StructureType.ComponentType( component.name, new_type, component.visibility, component.initial_value) self._components = new_components diff --git a/src/psyclone/psyir/tools/call_tree_utils.py b/src/psyclone/psyir/tools/call_tree_utils.py index 3d11f7020c..7773477394 100644 --- a/src/psyclone/psyir/tools/call_tree_utils.py +++ b/src/psyclone/psyir/tools/call_tree_utils.py @@ -160,7 +160,8 @@ def _compute_all_non_locals(self, routine): # ------------------------------------------------------------------------- def get_input_parameters(self, read_write_info, node_list, - variables_info=None, options=None): + variables_info=None, + include_non_data_accesses=False): '''Adds all variables that are input parameters (i.e. are read before potentially being written) to the read_write_info object. @@ -173,31 +174,29 @@ def get_input_parameters(self, read_write_info, node_list, can be used to avoid repeatedly collecting this information. :type variables_info: :py:class:`psyclone.core.variables_info.VariablesAccessInfo` - :param options: a dictionary with options for the CallTreeUtils - which will also be used when creating the VariablesAccessInfo - instance if required. - :type param: Optional[dict[str, Any]] - :param Any options["COLLECT-ARRAY-SHAPE-READS"]: if this option is - set to a True value, arrays used as first parameter to the - PSyIR operators lbound, ubound, or size will be reported as - 'read'. Otherwise, these accesses will be ignored. ''' # Collect the information about all variables used: if not variables_info: - variables_info = VariablesAccessInfo(node_list, options=options) + variables_info = VariablesAccessInfo(node_list) - for signature in variables_info.all_signatures: + if include_non_data_accesses: + all_accesses = variables_info.all_signatures + else: + all_accesses = variables_info.all_data_accesses + + for signature in all_accesses: # If the first access is a write, the variable is not an input # parameter and does not need to be saved. Note that loop variables # have a WRITE before a READ access, so they will be ignored # automatically. - if not variables_info[signature].is_written_first(): + if (variables_info[signature].is_read_only() or + not variables_info[signature].is_written_first()): read_write_info.add_read(signature) # ------------------------------------------------------------------------- def get_output_parameters(self, read_write_info, node_list, - variables_info=None, options=None): + variables_info=None): '''Adds all variables that are output parameters (i.e. are written) to the read_write_info object. @@ -210,19 +209,11 @@ def get_output_parameters(self, read_write_info, node_list, can be used to avoid repeatedly collecting this information. :type variables_info: \ Optional[:py:class:`psyclone.core.variables_info.VariablesAccessInfo`] - :param options: a dictionary with options for the CallTreeUtils - which will also be used when creating the VariablesAccessInfo - instance if required. - :type param: Optional[dict[str, Any]] - :param Any options["COLLECT-ARRAY-SHAPE-READS"]: if this option is - set to a True value, arrays used as first parameter to the - PSyIR operators lbound, ubound, or size will be reported as - 'read'. Otherwise, these accesses will be ignored. ''' # Collect the information about all variables used: if not variables_info: - variables_info = VariablesAccessInfo(node_list, options=options) + variables_info = VariablesAccessInfo(node_list) for signature in variables_info.all_signatures: if variables_info.is_written(signature): @@ -230,7 +221,7 @@ def get_output_parameters(self, read_write_info, node_list, # ------------------------------------------------------------------------- def get_in_out_parameters(self, node_list, collect_non_local_symbols=False, - options=None): + include_non_data_accesses=False): '''Returns a ReadWriteInfo object that contains all variables that are input and output parameters to the specified node list. This function calls `get_input_parameter` and `get_output_parameter`, but avoids the @@ -250,23 +241,17 @@ def get_in_out_parameters(self, node_list, collect_non_local_symbols=False, :param bool collect_non_local_symbols: whether non-local symbols (i.e. symbols used in other modules either directly or indirectly) should be included in the in/out information. - :param options: a dictionary with options for the CallTreeUtils - which will also be used when creating the VariablesAccessInfo - instance if required. - :type options: Optional[dict[str, Any]] - :param Any options["COLLECT-ARRAY-SHAPE-READS"]: if this option is - set to a True value, arrays used as first parameter to the - PSyIR operators lbound, ubound, or size will be reported as - 'read'. Otherwise, these accesses will be ignored. :returns: a ReadWriteInfo object with the information about input- and output parameters. :rtype: :py:class:`psyclone.psyir.tools.ReadWriteInfo` ''' - variables_info = VariablesAccessInfo(node_list, options=options) + variables_info = VariablesAccessInfo(node_list) read_write_info = ReadWriteInfo() - self.get_input_parameters(read_write_info, node_list, variables_info) + self.get_input_parameters( + read_write_info, node_list, variables_info, + include_non_data_accesses=include_non_data_accesses) self.get_output_parameters(read_write_info, node_list, variables_info) if collect_non_local_symbols: self.get_non_local_read_write_info(node_list, read_write_info) diff --git a/src/psyclone/psyir/tools/dependency_tools.py b/src/psyclone/psyir/tools/dependency_tools.py index 3c0e902a7e..e29e4b87a0 100644 --- a/src/psyclone/psyir/tools/dependency_tools.py +++ b/src/psyclone/psyir/tools/dependency_tools.py @@ -610,6 +610,9 @@ def _array_access_parallelisable(self, loop_variables, var_info): # including itself (to detect write-write race conditions: # a((i-2)**2) = b(i): i=1 and i=3 would write to a(1)) for other_access in var_info: + if not other_access.is_data_access: + # Not a data access so can ignore. + continue if not self._is_loop_carried_dependency(loop_variables, write_access, other_access): diff --git a/src/psyclone/psyir/transformations/acc_kernels_trans.py b/src/psyclone/psyir/transformations/acc_kernels_trans.py index 285ac7db26..2ec6a23880 100644 --- a/src/psyclone/psyir/transformations/acc_kernels_trans.py +++ b/src/psyclone/psyir/transformations/acc_kernels_trans.py @@ -39,12 +39,15 @@ ''' This module provides the ACCKernelsTrans transformation. ''' import re +from typing import List, Union from psyclone import psyGen from psyclone.psyir.nodes import ( - ACCKernelsDirective, Assignment, Call, CodeBlock, Loop, PSyDataNode, - Reference, Return, Routine, Statement, WhileLoop) + ACCKernelsDirective, Assignment, Call, CodeBlock, Loop, + Node, PSyDataNode, Reference, Return, Routine, Statement, WhileLoop) from psyclone.psyir.symbols import UnsupportedFortranType +from psyclone.psyir.transformations.arrayassignment2loops_trans import ( + ArrayAssignment2LoopsTrans) from psyclone.psyir.transformations.region_trans import RegionTrans from psyclone.psyir.transformations.transformation_error import ( TransformationError) @@ -74,12 +77,12 @@ class ACCKernelsTrans(RegionTrans): excluded_node_types = (CodeBlock, Return, PSyDataNode, psyGen.HaloExchange, WhileLoop) - def apply(self, node, options=None): + def apply(self, node: Union[Node, List[Node]], options: dict = None): ''' Enclose the supplied list of PSyIR nodes within an OpenACC Kernels region. - :param node: a node or list of nodes in the PSyIR to enclose. + :param node: the node(s) in the PSyIR to enclose. :type node: :py:class:`psyclone.psyir.nodes.Node` | list[:py:class:`psyclone.psyir.nodes.Node`] :param options: a dictionary with options for transformations. @@ -88,6 +91,11 @@ def apply(self, node, options=None): region should have the 'default present' attribute (indicating that data is already on the accelerator). When using managed memory this option should be False. + :param bool options["allow_string"]: whether to allow the + transformation on assignments involving character types. Defaults + to False. + :param bool options["verbose"]: log the reason the validation failed, + at the moment with a comment in the provided PSyIR node. ''' # Ensure we are always working with a list of nodes, even if only @@ -110,7 +118,8 @@ def apply(self, node, options=None): parent.children.insert(start_index, directive) - def validate(self, nodes, options=None): + def validate(self, nodes: Union[Node, List[Node]], + options: dict = None) -> None: # pylint: disable=signature-differs ''' Check that we can safely enclose the supplied node or list of nodes @@ -124,6 +133,11 @@ def validate(self, nodes, options=None): :param bool options["disable_loop_check"]: whether to disable the check that the supplied region contains 1 or more loops. Default is False (i.e. the check is enabled). + :param bool options["allow_string"]: whether to allow the + transformation on assignments involving character types. Defaults + to False. + :param bool options["verbose"]: log the reason the validation failed, + at the moment with a comment in the provided PSyIR node. :raises NotImplementedError: if the supplied Nodes belong to a GOInvokeSchedule. @@ -133,8 +147,13 @@ def validate(self, nodes, options=None): a routine that is not available on the accelerator. :raises TransformationError: if there are no Loops within the proposed region and options["disable_loop_check"] is not True. + :raises TransformationError: if any assignments in the region contain a + character type child and options["allow_string"] is not True. ''' + if not options: + options = {} + # Ensure we are always working with a list of nodes, even if only # one was supplied via the `nodes` argument. node_list = self.get_node_list(nodes) @@ -180,6 +199,17 @@ def validate(self, nodes, options=None): f"Assumed-size character variables cannot be enclosed " f"in an OpenACC region but found " f"'{stmt.debug_string()}'") + # Check there are no character assignments in the region as these + # cause various problems with (at least) NVHPC <= 24.5 + if not options.get("allow_string", False): + message = ( + f"{self.name} does not permit assignments involving " + f"character variables by default (use the 'allow_string' " + f"option to include them)") + for assign in node.walk(Assignment): + ArrayAssignment2LoopsTrans.validate_no_char( + assign, message, options) + # Check that any called routines are supported on the device. for icall in node.walk(Call): if not icall.is_available_on_device(): diff --git a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py index ada451e0ae..150c97bbda 100644 --- a/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py +++ b/src/psyclone/psyir/transformations/arrayassignment2loops_trans.py @@ -45,7 +45,7 @@ from psyclone.errors import LazyString from psyclone.psyGen import Transformation from psyclone.psyir.nodes import ( - Assignment, Call, IntrinsicCall, Loop, Literal, Range, Reference, + Assignment, Call, IntrinsicCall, Loop, Literal, Node, Range, Reference, CodeBlock, Routine, BinaryOperation) from psyclone.psyir.nodes.array_mixin import ArrayMixin from psyclone.psyir.symbols import ( @@ -285,23 +285,10 @@ def validate(self, node, options=None): # If we allow string arrays then we can skip the check. if not options.get("allow_string", False): - for child in node.walk((Literal, Reference)): - try: - forbidden = ScalarType.Intrinsic.CHARACTER - if (child.is_character(unknown_as=False) or - (child.symbol.datatype.intrinsic == forbidden)): - message = (f"{self.name} does not expand ranges " - f"on character arrays by default (use the" - f"'allow_string' option to expand them)") - if verbose: - node.append_preceding_comment(message) - # pylint: disable=cell-var-from-loop - raise TransformationError(LazyString( - lambda: f"{message}, but found:" - f"\n{node.debug_string()}")) - except (NotImplementedError, AttributeError): - # We cannot always get the datatype, we ignore this for now - pass + message = (f"{self.name} does not expand ranges " + f"on character arrays by default (use the" + f"'allow_string' option to expand them)") + self.validate_no_char(node, message, options) # We don't accept calls that are not guaranteed to be elemental for call in node.rhs.walk(Call): @@ -372,6 +359,41 @@ def validate(self, node, options=None): raise TransformationError(LazyString( lambda: f"{message} In:\n{node.debug_string()}")) + @staticmethod + def validate_no_char(node: Node, message: str, options: dict) -> None: + ''' + Check that there is no character variable accessed in the sub-tree with + the supplied node at its root. + + :param node: the root node to check for character assignments. + :param message: the message to use if a character assignment is found. + :param options: any options that apply to this check. + :param bool options["verbose"]: log the reason the validation failed, + at the moment with a comment in the provided PSyIR node. + + :raises TransformationError: if the supplied node contains a + child of character type. + + ''' + # Whether or not to log the resason for raising an error. At the moment + # "logging" means adding a comment in the output code. + verbose = options.get("verbose", False) + + for child in node.walk((Literal, Reference)): + try: + forbidden = ScalarType.Intrinsic.CHARACTER + if (child.is_character(unknown_as=False) or + (child.symbol.datatype.intrinsic == forbidden)): + if verbose: + node.append_preceding_comment(message) + # pylint: disable=cell-var-from-loop + raise TransformationError(LazyString( + lambda: f"{message}, but found:" + f"\n{node.debug_string()}")) + except (NotImplementedError, AttributeError): + # We cannot always get the datatype, we ignore this for now + pass + __all__ = [ 'ArrayAssignment2LoopsTrans'] diff --git a/src/psyclone/psyir/transformations/extract_trans.py b/src/psyclone/psyir/transformations/extract_trans.py index b2d3acfd2e..ffc5116d97 100644 --- a/src/psyclone/psyir/transformations/extract_trans.py +++ b/src/psyclone/psyir/transformations/extract_trans.py @@ -79,21 +79,6 @@ def __init__(self, node_class=ExtractNode): # node class. super().__init__(node_class=node_class) - # ------------------------------------------------------------------------- - def get_default_options(self): - '''Returns a new dictionary with additional options, specific to the - transformation, that will be added to the user option. Any values - specified by the user will take precedence. For the extract - transformation, by default we want VariablesAccessInformation to - report array arguments to lbound, ubound and size as read accesses, - so we are certain these arrays will be included in the extraction. - - :returns: a dictionary with additional options. - :rtype: Dict[str, Any] - ''' - - return {"COLLECT-ARRAY-SHAPE-READS": True} - # ------------------------------------------------------------------------- @staticmethod def determine_postfix(read_write_info, postfix="_post"): diff --git a/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py b/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py index d232971384..417af518e3 100644 --- a/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py +++ b/src/psyclone/psyir/transformations/intrinsics/minormax2code_trans.py @@ -82,9 +82,8 @@ def __init__(self): def apply(self, node, options=None): '''Apply this utility transformation to the specified node. This node - must be a MIN or MAX BinaryOperation or NaryOperation. The - operation is converted to equivalent inline code. This is - implemented as a PSyIR transform from: + must be a MIN or MAX IntrinsicCall. The intrinsic is converted to + equivalent inline code. This is implemented as a PSyIR transform from: .. code-block:: python @@ -108,13 +107,12 @@ def apply(self, node, options=None): PSyIR expressions and the ``...`` before and after ``[MIN or MAX](A, B, C ...)`` can be arbitrary PSyIR code. - This transformation requires the operation node to be a + This transformation requires the IntrinsicCall node to be a descendent of an assignment and will raise an exception if this is not the case. - :param node: a MIN or MAX Binary- or Nary-Operation node. - :type node: :py:class:`psyclone.psyir.nodes.BinaryOperation` or \ - :py:class:`psyclone.psyir.nodes.NaryOperation` + :param node: a MIN or MAX intrinsic. + :type node: :py:class:`psyclone.psyir.nodes.IntrinsicCall` :param options: a dictionary with options for transformations. :type options: Optional[Dict[str, Any]] @@ -141,7 +139,7 @@ def apply(self, node, options=None): f"tmp_{self._intrinsic.name.lower()}", symbol_type=DataSymbol, datatype=REAL_TYPE) - # Replace operation with a temporary (res_var). + # Replace intrinsic with a temporary (res_var). node.replace_with(Reference(res_var_symbol)) # res_var=A (child[0] of node) diff --git a/src/psyclone/psyir/transformations/psy_data_trans.py b/src/psyclone/psyir/transformations/psy_data_trans.py index 04f6a80229..2672aee190 100644 --- a/src/psyclone/psyir/transformations/psy_data_trans.py +++ b/src/psyclone/psyir/transformations/psy_data_trans.py @@ -111,39 +111,6 @@ class name as a string, which means that the same function can return self.__class__.__name__ - # ------------------------------------------------------------------------- - def get_default_options(self): - '''Returns a new dictionary with additional options, specific to the - transformation, that will be added to the user option. Any values - specified by the user will take precedence. - - :returns: a dictionary with additional options. - :rtype: Dict[str, Any] - ''' - - return {} - - # ------------------------------------------------------------------------- - def merge_in_default_options(self, options): - '''This function returns a new dictionary which contains the default - options for this transformation plus al user-specified options. - Any user-specified option will take precedence over the default - values. - - :param options: a dictionary with options for transformations. - :type options: Dict[str, Any] - :returns: a new dictionary which merges the default options with \ - the user-specified options. - :rtype: Dict[str:Any] - - ''' - new_options = self.get_default_options() - if options: - # Update will overwrite any existing setting with the ones - # specified by the user: - new_options.update(options) - return new_options - # ------------------------------------------------------------------------ def get_unique_region_name(self, nodes, options): '''This function returns the region and module name. If they are @@ -254,9 +221,10 @@ def validate(self, nodes, options=None): raise TransformationError("A PSyData node cannot be inserted " "inside an OpenACC region.") - my_options = self.merge_in_default_options(options) - if "region_name" in my_options: - name = my_options["region_name"] + if options is None: + options = {} + if "region_name" in options: + name = options["region_name"] # pylint: disable=too-many-boolean-expressions if not isinstance(name, tuple) or not len(name) == 2 or \ not name[0] or not isinstance(name[0], str) or \ @@ -265,9 +233,9 @@ def validate(self, nodes, options=None): f"Error in {self.name}. User-supplied region name " f"must be a tuple containing two non-empty strings.") # pylint: enable=too-many-boolean-expressions - prefix = my_options.get("prefix", None) - if "prefix" in my_options: - prefix = my_options.get("prefix", None) + prefix = options.get("prefix", None) + if "prefix" in options: + prefix = options.get("prefix", None) if prefix not in Config.get().valid_psy_data_prefixes: raise TransformationError( f"Error in 'prefix' parameter: found '{prefix}', while" @@ -276,7 +244,7 @@ def validate(self, nodes, options=None): # We have to create an instance of the node that will be inserted in # order to find out what module name it will use. - pdata_node = self._node_class(options=my_options) + pdata_node = self._node_class(options=options) table = node_list[0].scope.symbol_table for name in ([sym.name for sym in pdata_node.imported_symbols] + [pdata_node.fortran_module]): @@ -295,7 +263,7 @@ def validate(self, nodes, options=None): except KeyError: pass - super().validate(node_list, my_options) + super().validate(node_list, options) # ------------------------------------------------------------------------ def apply(self, nodes, options=None): @@ -322,10 +290,8 @@ def apply(self, nodes, options=None): ''' node_list = self.get_node_list(nodes) - # Add any transformation-specific settings that are required: - my_options = self.merge_in_default_options(options) # Perform validation checks - self.validate(node_list, my_options) + self.validate(node_list, options) # Get useful references parent = node_list[0].parent @@ -345,7 +311,7 @@ def apply(self, nodes, options=None): node.detach() psy_data_node = self._node_class.create( - node_list, symbol_table=table, options=my_options) + node_list, symbol_table=table, options=options) parent.addchild(psy_data_node, position) diff --git a/src/psyclone/psyir/transformations/read_only_verify_trans.py b/src/psyclone/psyir/transformations/read_only_verify_trans.py index 20092a7971..c249d7393e 100644 --- a/src/psyclone/psyir/transformations/read_only_verify_trans.py +++ b/src/psyclone/psyir/transformations/read_only_verify_trans.py @@ -73,21 +73,6 @@ class ReadOnlyVerifyTrans(PSyDataTrans): def __init__(self, node_class=ReadOnlyVerifyNode): super().__init__(node_class=node_class) - # ------------------------------------------------------------------------- - def get_default_options(self): - '''Returns a new dictionary with additional options, specific to the - transformation, that will be added to the user option. Any values - specified by the user will take precedence. For the read-only verify - transformation, by default we want VariablesAccessInformation to - report array arguments to lbound, ubound and size as read accesses, - since these arguments should also not be overwritten. - - :returns: a dictionary with additional options. - :rtype: Dict[str, Any] - ''' - - return {"COLLECT-ARRAY-SHAPE-READS": True} - # ------------------------------------------------------------------------- def validate(self, node_list, options=None): # pylint: disable=arguments-renamed diff --git a/src/psyclone/tests/config_test.py b/src/psyclone/tests/configuration_test.py similarity index 95% rename from src/psyclone/tests/config_test.py rename to src/psyclone/tests/configuration_test.py index 433dcb6a53..4a42689f1d 100644 --- a/src/psyclone/tests/config_test.py +++ b/src/psyclone/tests/configuration_test.py @@ -33,7 +33,7 @@ # ----------------------------------------------------------------------------- # Author: A. R. Porter, STFC Daresbury Lab # Modified: I. Kavcic and O. Brunt, Met Office, -# R. W. Ford, STFC Daresbury Lab +# R. W. Ford, STFC Daresbury Lab # J. Henrichs, Bureau of Meteorology # N. Nobre, STFC Daresbury Lab # S. Siso, STFC Daresbury Lab @@ -74,6 +74,7 @@ OCL_DEVICES_PER_NODE = 1 IGNORE_MODULES = netcdf, mpi BACKEND_CHECKS_ENABLED = false +FORTRAN_STANDARD = f2003 [lfric] access_mapping = gh_read: read, gh_write: write, gh_readwrite: readwrite, gh_inc: inc, gh_sum: sum @@ -766,3 +767,35 @@ def test_aliased_api_names(tmpdir): assert "lfric" in config._api_conf assert "dynamo0.3" not in config._api_conf assert config._api_conf['lfric'].num_any_space == 13 + + +def test_fortran_standard(tmpdir): + '''Test the handling of the Fortran standard. The dummy + config content here specifies f2003 in the config file, + so we can check that we get the expected default of f2008 + if it is removed. + ''' + # Check that we read the expected value in the config file: + config_file = tmpdir.join("config") + content = _CONFIG_CONTENT + config = get_config(config_file, content) + assert config.fortran_standard == "f2003" + # Remove the Fortran_standard from the config file, in + # which case we must get the default of f2008: + content = re.sub(r"^FORTRAN_STANDARD = f2003$", "", + content, flags=re.MULTILINE) + config = get_config(config_file, content) + assert config.fortran_standard == "f2008" + + # Check that an invalid Fortran standard raises an exception. + content = _CONFIG_CONTENT + # Set an invalid Fortran_standard + content = re.sub(r"^FORTRAN_STANDARD = f2003", + "FORTRAN_STANDARD = invalid", + content, flags=re.MULTILINE) + with pytest.raises(ConfigurationError) as err: + get_config(config_file, content) + + assert ("PSyclone configuration error: Invalid Fortran standard 'invalid' " + "specified in config file. Must be one of['f2003', 'f2008']" + in str(err.value)) diff --git a/src/psyclone/tests/core/access_type_test.py b/src/psyclone/tests/core/access_type_test.py index 5d2f5223e8..32fb15b904 100644 --- a/src/psyclone/tests/core/access_type_test.py +++ b/src/psyclone/tests/core/access_type_test.py @@ -32,11 +32,10 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author: Joerg Henrichs, Bureau of Meteorology -# Modified by R. W. Ford and N. Nobre, STFC Daresbury Lab +# Modified by R. W. Ford, N. Nobre and A. R. Porter, STFC Daresbury Lab '''This module tests AccessType.''' -from __future__ import absolute_import import pytest from psyclone.configuration import Config from psyclone.core.access_type import AccessType @@ -51,6 +50,10 @@ def test_str(): assert str(AccessType.INC) == "INC" assert str(AccessType.READINC) == "READINC" assert str(AccessType.SUM) == "SUM" + assert str(AccessType.CALL) == "CALL" + assert str(AccessType.INQUIRY) == "INQUIRY" + assert str(AccessType.TYPE_INFO) == "TYPE_INFO" + assert str(AccessType.UNKNOWN) == "UNKNOWN" def test_api_specific_name(): @@ -65,6 +68,10 @@ def test_api_specific_name(): assert AccessType.INC.api_specific_name() == "gh_inc" assert AccessType.READINC.api_specific_name() == "gh_readinc" assert AccessType.SUM.api_specific_name() == "gh_sum" + assert AccessType.CALL.api_specific_name() == "call" + assert AccessType.INQUIRY.api_specific_name() == "inquiry" + assert AccessType.TYPE_INFO.api_specific_name() == "type_info" + assert AccessType.UNKNOWN.api_specific_name() == "unknown" assert AccessType.get_valid_reduction_modes() == [AccessType.SUM] assert AccessType.get_valid_reduction_names() == ["gh_sum"] # Use set to make this independent of the order: @@ -91,6 +98,7 @@ def test_from_string(): assert AccessType.from_string("readinc") == AccessType.READINC assert AccessType.from_string("sum") == AccessType.SUM assert AccessType.from_string("unknown") == AccessType.UNKNOWN + assert AccessType.from_string("type_info") == AccessType.TYPE_INFO with pytest.raises(ValueError) as err: AccessType.from_string("invalid") @@ -117,7 +125,23 @@ def test_all_read_accesses(): all_read_accesses = AccessType.all_read_accesses() assert isinstance(all_read_accesses, list) assert len(all_read_accesses) == 4 + # No duplications. assert (len(all_read_accesses) == len(set(all_read_accesses))) assert all(isinstance(read_access, AccessType) for read_access in all_read_accesses) + + +def test_non_data_accesses(): + '''Test the non_data_accesses() method.''' + accesses = AccessType.non_data_accesses() + assert isinstance(accesses, list) + # No duplications + assert (len(accesses) == len(set(accesses))) + assert all(isinstance(acc, AccessType) for acc in accesses) + all_read_accesses = AccessType.all_read_accesses() + all_write_accesses = AccessType.all_write_accesses() + all_reductions = AccessType.get_valid_reduction_modes() + all_data_accesses = all_read_accesses + all_write_accesses + all_reductions + for acc in accesses: + assert acc not in all_data_accesses diff --git a/src/psyclone/tests/core/single_variable_access_info_test.py b/src/psyclone/tests/core/single_variable_access_info_test.py index f09dc47bae..fa0d2ba556 100644 --- a/src/psyclone/tests/core/single_variable_access_info_test.py +++ b/src/psyclone/tests/core/single_variable_access_info_test.py @@ -132,12 +132,14 @@ def test_variable_access_info(): assert vai.all_write_accesses == [] assert vai.signature == Signature("var_name") + vai.add_access_with_location(AccessType.INQUIRY, 3, Node(), + component_indices=None) vai.add_access_with_location(AccessType.READ, 2, Node(), component_indices=None) - assert str(vai) == "var_name:READ(2)" + assert str(vai) == "var_name:INQUIRY(3),READ(2)" assert vai.is_read() assert vai.is_read_only() - assert vai.all_read_accesses == [vai[0]] + assert vai.all_read_accesses == [vai[1]] assert vai.all_write_accesses == [] assert not vai.is_written() assert not vai.is_written_first() @@ -147,34 +149,40 @@ def test_variable_access_info(): assert vai.is_written_first() assert not vai.is_read_only() assert vai.all_read_accesses == [] - assert vai.all_write_accesses == [vai[0]] + assert vai.all_write_accesses == [vai[1]] # Now we have one write access, which we should not be able to # change to write again: with pytest.raises(InternalError) as err: vai.change_read_to_write() - assert "Trying to change variable 'var_name' to 'WRITE' which "\ - "does not have 'READ' access." in str(err.value) + assert ("Variable 'var_name' has a 'WRITE' access. change_read_to_write() " + "expects only inquiry accesses and a single 'READ' access." + in str(err.value)) assert vai.all_accesses[0] == vai[0] with pytest.raises(IndexError) as err: - _ = vai[1] + _ = vai[2] - # Add a READ access - now we should not be able to - # change read to write anymore: + # Add a READ access - we should not be able to + # change this read to write as there's already a WRITE access. vai.add_access_with_location(AccessType.READ, 1, Node(), component_indices=None) with pytest.raises(InternalError) as err: vai.change_read_to_write() - assert "Variable 'var_name' had 2 accesses listed, "\ - "not one in change_read_to_write." in str(err.value) - + assert ("Variable 'var_name' has a 'WRITE' access. change_read_to_write() " + "expects only inquiry accesses and a single 'READ' access." + in str(err.value)) # And make sure the variable is not read_only if a write is added vai.add_access_with_location(AccessType.WRITE, 3, Node(), component_indices=None) assert vai.is_read_only() is False - assert vai.all_read_accesses == [vai[1]] - assert vai.all_write_accesses == [vai[0], vai[2]] + assert vai.all_read_accesses == [vai[2]] + assert vai.all_write_accesses == [vai[1], vai[3]] + # Check that we catch a case where there are no accesses at all. + vai = SingleVariableAccessInfo(Signature("var_name")) + with pytest.raises(InternalError) as err: + vai.change_read_to_write() + assert "but it does not have a 'READ' access" in str(err.value) # ----------------------------------------------------------------------------- diff --git a/src/psyclone/tests/core/variables_access_info_test.py b/src/psyclone/tests/core/variables_access_info_test.py index 293863a9a6..33665f9a54 100644 --- a/src/psyclone/tests/core/variables_access_info_test.py +++ b/src/psyclone/tests/core/variables_access_info_test.py @@ -388,33 +388,24 @@ def test_variables_access_info_options(): '''Test handling of options for VariablesAccessInfo. ''' vai = VariablesAccessInfo() - assert vai.options("COLLECT-ARRAY-SHAPE-READS") is False assert vai.options("USE-ORIGINAL-NAMES") is False - vai = VariablesAccessInfo(options={'COLLECT-ARRAY-SHAPE-READS': True}) - assert vai.options("COLLECT-ARRAY-SHAPE-READS") is True - assert vai.options("USE-ORIGINAL-NAMES") is False - assert vai.options() == {"COLLECT-ARRAY-SHAPE-READS": True, - "USE-ORIGINAL-NAMES": False} - vai = VariablesAccessInfo(options={'USE-ORIGINAL-NAMES': True}) - assert vai.options("COLLECT-ARRAY-SHAPE-READS") is False assert vai.options("USE-ORIGINAL-NAMES") is True - assert vai.options() == {"COLLECT-ARRAY-SHAPE-READS": False, - "USE-ORIGINAL-NAMES": True} + assert vai.options() == {"USE-ORIGINAL-NAMES": True} with pytest.raises(InternalError) as err: vai.options("invalid") assert ("Option key 'invalid' is invalid, it must be one of " - "['COLLECT-ARRAY-SHAPE-READS', 'USE-ORIGINAL-NAMES']." - in str(err.value)) + "['USE-ORIGINAL-NAMES']." in str(err.value)) # ----------------------------------------------------------------------------- @pytest.mark.parametrize("function", ["size", "lbound", "ubound"]) def test_variables_access_info_shape_bounds(fortran_reader, function): - '''Test that access to an array using shape, or lbound/ubound can be - disables using options + '''Test that access to an array using shape, or lbound/ubound is marked + as 'inquiry'. + ''' code = f'''module test contains @@ -427,19 +418,9 @@ def test_variables_access_info_shape_bounds(fortran_reader, function): psyir = fortran_reader.psyir_from_source(code) node1 = psyir.walk(Assignment)[0] - # By default, array shape accesses are not reads. + # Array-shape accesses are 'inquiry' vai = VariablesAccessInfo(node1) - assert str(vai) == "n: WRITE" - - # Check that explicitly disabling array shape reads works: - vai = VariablesAccessInfo(node1, - options={"COLLECT-ARRAY-SHAPE-READS": False}) - assert str(vai) == "n: WRITE" - - # Check that we can enable collection of array shape reads: - vai = VariablesAccessInfo(node1, - options={"COLLECT-ARRAY-SHAPE-READS": True}) - assert str(vai) == "a: READ, n: WRITE" + assert str(vai) == "a: NO_DATA_ACCESS, n: WRITE" # ----------------------------------------------------------------------------- @@ -450,10 +431,12 @@ def test_variables_access_info_domain_loop(): ''' _, invoke = get_invoke("25.1_kern_two_domain.f90", "lfric", idx=0) vai = VariablesAccessInfo(invoke.schedule) - assert str(vai) == ("a: READ, b: READ, f1_data: READWRITE, f2_data: " - "READWRITE, map_w3: READ, ncell_2d_no_halos: READ, " - "ndf_w3: READ, nlayers_f1: READ, nlayers_f2: READ, " - "undf_w3: READ") + assert str(vai) == ( + "a: READ, b: READ, f1_data: READWRITE, f2_data: " + "READWRITE, field_type: NO_DATA_ACCESS, i_def: NO_DATA_ACCESS, " + "map_w3: READ, mesh_type: NO_DATA_ACCESS, ncell_2d_no_halos: " + "READ, ndf_w3: READ, nlayers_f1: READ, nlayers_f2: READ, " + "r_def: NO_DATA_ACCESS, undf_w3: READ") # ----------------------------------------------------------------------------- @@ -467,13 +450,15 @@ def test_lfric_access_info(): schedule = psy.invokes.invoke_list[0].schedule vai = VariablesAccessInfo(schedule) - # Make sure a literal (1.0_r_def in this example) is not reported as a - # variable in the access list: - assert ("basis_w1_qr: READ, basis_w3_qr: READ, cell: READ+WRITE, " - "diff_basis_w2_qr: READ, diff_basis_w3_qr: READ, f1_data: " - "READ+WRITE, f2_data: READ, loop0_start: READ, loop0_stop: READ, " - "m1_data: READ, m2_data: READ, map_w1: READ, map_w2: READ, map_w3:" - " READ, ndf_w1: READ, ndf_w2: READ, ndf_w3: READ, nlayers_f1: " - "READ, np_xy_qr: READ, np_z_qr: READ, undf_w1: READ, undf_w2: " - "READ, undf_w3: READ, weights_xy_qr: READ, weights_z_qr: READ" - == str(vai)) + # Make sure literals (e.g. 1_i_def or 2.0_r_def in this example) are not + # reported as variables in the access list (but that the associated + # precisions are): + assert ( + "basis_w1_qr: READ, basis_w3_qr: READ, cell: READ+WRITE, " + "diff_basis_w2_qr: READ, diff_basis_w3_qr: READ, f1_data: " + "READ+WRITE, f2_data: READ, field_type: NO_DATA_ACCESS, i_def: " + "NO_DATA_ACCESS, loop0_start: READ, loop0_stop: READ, m1_data: READ, " + "m2_data: READ, map_w1: READ, map_w2: READ, map_w3: READ, ndf_w1: " + "READ, ndf_w2: READ, ndf_w3: READ, nlayers_f1: READ, np_xy_qr: READ, " + "np_z_qr: READ, r_def: NO_DATA_ACCESS, undf_w1: READ, undf_w2: READ, " + "undf_w3: READ, weights_xy_qr: READ, weights_z_qr: READ" == str(vai)) diff --git a/src/psyclone/tests/dependency_test.py b/src/psyclone/tests/dependency_test.py index b19e604da6..9a5bd179fd 100644 --- a/src/psyclone/tests/dependency_test.py +++ b/src/psyclone/tests/dependency_test.py @@ -291,10 +291,11 @@ def test_lfric(): var_accesses = VariablesAccessInfo(schedule) assert str(var_accesses) == ( "a: READ, cell: READ+WRITE, f1_data: READ+WRITE, f2_data: READ, " - "loop0_start: READ, loop0_stop: READ, m1_data: READ, " - "m2_data: READ, map_w1: READ, map_w2: READ, " - "map_w3: READ, ndf_w1: READ, ndf_w2: READ, ndf_w3: READ, " - "nlayers_f1: READ, undf_w1: READ, undf_w2: READ, undf_w3: READ") + "field_type: NO_DATA_ACCESS, i_def: NO_DATA_ACCESS, loop0_start: " + "READ, loop0_stop: READ, m1_data: READ, m2_data: READ, map_w1: READ, " + "map_w2: READ, map_w3: READ, ndf_w1: READ, ndf_w2: READ, ndf_w3: READ," + " nlayers_f1: READ, r_def: NO_DATA_ACCESS, undf_w1: READ, undf_w2: " + "READ, undf_w3: READ") def test_lfric_kern_cma_args(): diff --git a/src/psyclone/tests/domain/common/transformations/kernel_module_inline_trans_test.py b/src/psyclone/tests/domain/common/transformations/kernel_module_inline_trans_test.py index 2e95b464ba..d6d36a7fad 100644 --- a/src/psyclone/tests/domain/common/transformations/kernel_module_inline_trans_test.py +++ b/src/psyclone/tests/domain/common/transformations/kernel_module_inline_trans_test.py @@ -150,8 +150,8 @@ def test_validate_no_inline_global_var(parser): with pytest.raises(TransformationError) as err: inline_trans.validate(kernels[0]) assert ("Kernel 'kernel_with_global_code' contains accesses to 'alpha' " - "which is declared in the same module scope. Cannot inline such a " - "Kernel." in str(err.value)) + "which is declared in the callee module scope. Cannot inline such " + "a Kernel." in str(err.value)) # Check that the issue is also reported if the symbol is inside a # Codeblock @@ -166,8 +166,8 @@ def test_validate_no_inline_global_var(parser): with pytest.raises(TransformationError) as err: inline_trans.validate(kernels[0]) - assert ("'kernel_with_global_code' contains accesses to 'alpha' in a " - "CodeBlock that is declared in the same module scope. Cannot " + assert ("'kernel_with_global_code' contains accesses to 'alpha' which is " + "declared in the callee module scope. Cannot " "inline such a Kernel." in str(err.value)) # Check that a symbol of unknown origin within a CodeBlock is caught. @@ -179,10 +179,15 @@ def test_validate_no_inline_global_var(parser): block = CodeBlock([stmt], CodeBlock.Structure.STATEMENT) kernels[0].get_kernel_schedule().pop_all_children() kernels[0].get_kernel_schedule().addchild(block) + table = kernels[0].get_kernel_schedule().symbol_table + # Remove symbols that refer to 'go_wp' in outer scope. + table._symbols.pop("field_old") + table._symbols.pop("field_new") + table._symbols.pop("field") with pytest.raises(TransformationError) as err: inline_trans.validate(kernels[0]) assert ("Kernel 'kernel_with_global_code' contains accesses to 'unknown' " - "in a CodeBlock but the origin of this symbol is unknown" in + "but the origin of this signature is unknown" in str(err.value)) # But make sure that an IntrinsicCall routine name is not considered @@ -368,6 +373,54 @@ def test_validate_fail_to_get_psyir(fortran_reader): in str(err.value)) +def test_validate_nested_scopes(fortran_reader, monkeypatch): + ''' + Test that validate() works correctly when two symbols in nested scopes + have a name clash. + + TODO #2424 - this xfails at the moment because VariablesAccessInfo does not + support nested scopes. + + ''' + _, invoke = get_invoke("single_invoke_three_kernels.f90", "gocean", + idx=0, dist_mem=False) + schedule = invoke.schedule + kern_call = schedule.children[1].loop_body[0].loop_body[0] + + # Manually set the kernel to the desired problematic code + psyir = fortran_reader.psyir_from_source(''' + module my_mod + use external_mod + contains + subroutine compute_cv_code() + integer :: i + do i = 1, 10 + a(i) = i + end do + end subroutine compute_cv_code + end module my_mod + ''') + routine = psyir.walk(Routine)[0] + # Move the definition of 'a' into the table associated with the loop. + sym = routine.symbol_table.lookup("a") + routine[0].loop_body.symbol_table.add(sym) + del routine.symbol_table._symbols["a"] + # Put a new, different symbol (with the same name) into the table of the + # parent Container. + routine.parent.scope.symbol_table.add(DataSymbol("a", REAL_TYPE)) + monkeypatch.setattr(kern_call, "_kern_schedule", routine) + + # The transformation should succeed (because the symbol named 'a' is + # actually local to the routine. However, the dependence analysis thinks it + # clashes with the symbol of the same name in the module scope. + intrans = KernelModuleInlineTrans() + try: + intrans.validate(kern_call) + except TransformationError: + pytest.xfail(reason="TODO #2424 - VariablesAccessInfo does not support" + " nested scopes") + + def test_module_inline_apply_transformation(tmpdir, fortran_writer): ''' Test that we can succesfully inline a basic kernel subroutine routine into the PSy layer module using a transformation ''' diff --git a/src/psyclone/tests/domain/gocean/kernel/gokern_test.py b/src/psyclone/tests/domain/gocean/kernel/gokern_test.py index a7a163a247..b6e99176b8 100644 --- a/src/psyclone/tests/domain/gocean/kernel/gokern_test.py +++ b/src/psyclone/tests/domain/gocean/kernel/gokern_test.py @@ -202,17 +202,3 @@ def test_gok_access_info_scalar_and_property(): assert isinstance(comp_ind[2][1], Reference) assert comp_ind[2][0].symbol.name == "i" assert comp_ind[2][1].symbol.name == "j" - - -# ----------------------------------------------------------------------------- -def test_gok_local_vars(): - '''Tests the local_var function - - ''' - _, invoke = get_invoke("test00.1_invoke_kernel_using_const_scalar.f90", - "gocean", idx=0) - schedule = invoke.schedule - - # Get the first kernel - kern1 = schedule.walk(GOKern)[0] - assert kern1.local_vars() == [] diff --git a/src/psyclone/tests/domain/lfric/lfric_builtins_test.py b/src/psyclone/tests/domain/lfric/lfric_builtins_test.py index 6ce8c3cd1b..9c6bb0f87b 100644 --- a/src/psyclone/tests/domain/lfric/lfric_builtins_test.py +++ b/src/psyclone/tests/domain/lfric/lfric_builtins_test.py @@ -2141,5 +2141,7 @@ def test_field_access_info_for_arrays_in_builtins(): assert Signature("f2_data") in vai - assert ("a: READ, df: READ+WRITE, f1_data: READ, f2_data: WRITE, " - "loop0_start: READ, loop0_stop: READ" == str(vai)) + assert ( + "a: READ, df: READ+WRITE, f1_data: READ, f2_data: WRITE, " + "field_type: NO_DATA_ACCESS, i_def: NO_DATA_ACCESS, loop0_start: " + "READ, loop0_stop: READ, r_def: NO_DATA_ACCESS" == str(vai)) diff --git a/src/psyclone/tests/nemo/test_files/imperfect_nest.f90 b/src/psyclone/tests/nemo/test_files/imperfect_nest.f90 index 77983bf092..0a51d29a5d 100644 --- a/src/psyclone/tests/nemo/test_files/imperfect_nest.f90 +++ b/src/psyclone/tests/nemo/test_files/imperfect_nest.f90 @@ -73,7 +73,7 @@ program imperfect_nest DO jj = 1, jpjm1 DO ji = 1, jpim1 zabe1 = pahu(ji, jj, jk) * e2_e1u(ji, jj) * e3u_n(ji, jj, jk) - zmsku = 1. / MAX(wmask(ji + 1, jj, jk) + wmask(ji, jj, jk + 1) + wmask(ji + 1, jj, jk + 1) + wmask(ji, jj, jk), 1.) + zmsku = 1._wp / MAX(wmask(ji + 1, jj, jk) + wmask(ji, jj, jk + 1) + wmask(ji + 1, jj, jk + 1) + wmask(ji, jj, jk), 1.) zcof1 = - pahu(ji, jj, jk) * e2u(ji, jj) * uslp(ji, jj, jk) * zmsku zftu(ji, jj, jk) = (zabe1 * zdit(ji, jj, jk) + zcof1 * (zdkt(ji + 1, jj) + zdk1t(ji, jj) + zdk1t(ji + 1, jj) + zdkt(ji, jj))) * umask(ji, jj, jk) END DO diff --git a/src/psyclone/tests/nemo/transformations/openacc/data_directive_test.py b/src/psyclone/tests/nemo/transformations/openacc/data_directive_test.py index 7f031bcfef..5c477fed32 100644 --- a/src/psyclone/tests/nemo/transformations/openacc/data_directive_test.py +++ b/src/psyclone/tests/nemo/transformations/openacc/data_directive_test.py @@ -55,6 +55,7 @@ # Test code with explicit NEMO-style do loop EXPLICIT_DO = ("program explicit_do\n" + " use kinds_mod, only: r_def\n" " REAL :: r\n" " INTEGER :: ji, jj, jk\n" " INTEGER, PARAMETER :: jpi=3, jpj=5, jpk=7\n" @@ -62,7 +63,7 @@ " DO jk = 1, jpk\n" " DO jj = 1, jpj\n" " DO ji = 1, jpi\n" - " umask(ji,jj,jk) = ji*jj*jk/r\n" + " umask(ji,jj,jk) = 3.0_r_def - ji*jj*jk/r\n" " END DO\n" " END DO\n" " END DO\n" diff --git a/src/psyclone/tests/psyGen_test.py b/src/psyclone/tests/psyGen_test.py index 1a5680e124..1cf1d6caa5 100644 --- a/src/psyclone/tests/psyGen_test.py +++ b/src/psyclone/tests/psyGen_test.py @@ -747,9 +747,6 @@ def __init__(self, call, parent_call, check): dummy_call = DummyClass(my_ktype) my_call = Kern(None, dummy_call, "dummy", DummyArguments) - with pytest.raises(NotImplementedError) as excinfo: - my_call.local_vars() - assert "Kern.local_vars should be implemented" in str(excinfo.value) with pytest.raises(NotImplementedError) as excinfo: my_call.gen_code(None) diff --git a/src/psyclone/tests/psyad/transformations/test_assignment_trans.py b/src/psyclone/tests/psyad/transformations/test_assignment_trans.py index c43b33916d..4fd8acdb03 100644 --- a/src/psyclone/tests/psyad/transformations/test_assignment_trans.py +++ b/src/psyclone/tests/psyad/transformations/test_assignment_trans.py @@ -350,7 +350,7 @@ def test_multi_add(tmpdir): " real, dimension(10) :: a\n real, dimension(10) :: b\n" " real, dimension(10) :: c\n real, dimension(10) :: d\n" " integer :: i\n integer :: j\n integer :: n\n\n" - " b(j) = b(j) + 3 / n * a(i + 2)\n" + " b(j) = b(j) + (3 / n) * a(i + 2)\n" " c(1) = c(1) + a(i + 2) / (2 * n)\n" " d(n) = d(n) + a(i + 2)\n" " a(i + 2) = 0.0\n\n") diff --git a/src/psyclone/tests/psyir/backend/fortran_test.py b/src/psyclone/tests/psyir/backend/fortran_test.py index fea5b63625..ab06d947ae 100644 --- a/src/psyclone/tests/psyir/backend/fortran_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_test.py @@ -1106,14 +1106,15 @@ def test_fw_binaryoperator_precedence(fortran_reader, fortran_writer, tmpdir): " a = b * (c + d)\n" " a = b * c + d\n" " a = (b * c) + d\n" - " a = b * c * d * a\n" - " a = (((b * c) * d) * a)\n" - " a = (b * (c * (d * a)))\n" + " a = b * c * d * a\n" # Left-to-right + " a = (((b * c) * d) * a)\n" # Left-to-right with explicit par. + " a = (b * (c * (d * a)))\n" # Right-to-left " a = -(a + b)\n" " e = .not.(e .and. (f .or. g))\n" " e = (((.not.e) .and. f) .or. g)\n" " e = (e .and. (f .eqv. g))\n" " e = (e .and. f .neqv. g)\n" + " a = ((a + b) + (c + d)) / a * b\n" "end subroutine tmp\n" "end module test") schedule = fortran_reader.psyir_from_source(code) @@ -1124,13 +1125,14 @@ def test_fw_binaryoperator_precedence(fortran_reader, fortran_writer, tmpdir): " a = b * c + d\n" " a = b * c + d\n" " a = b * c * d * a\n" - " a = b * c * d * a\n" + " a = ((b * c) * d) * a\n" " a = b * (c * (d * a))\n" " a = -(a + b)\n" " e = .NOT.(e .AND. (f .OR. g))\n" " e = .NOT.e .AND. f .OR. g\n" " e = e .AND. (f .EQV. g)\n" - " e = e .AND. f .NEQV. g\n") + " e = e .AND. f .NEQV. g\n" + " a = ((a + b) + (c + d)) / a * b\n") assert expected in result assert Compile(tmpdir).string_compiles(result) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py index e6b5d195e9..53c5cfdd12 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py @@ -1938,6 +1938,29 @@ def test_handling_parenthesis(): assert isinstance(new_node, BinaryOperation) +@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") +def test_handling_parenthesis_over_binary_op(): + ''' Test that fparser2 parenthesis are recorded in the appropriate + attribute when they are explicit over BinaryOperations. + ''' + reader = FortranStringReader("a = (a + b) + (c + d)") + fparser2parenthesis = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2parenthesis]) + + bop1 = fake_parent[0].rhs + bop2 = bop1.children[0] + bop3 = bop1.children[1] + + # The parent addition does not have explicit parenthesis + assert not bop1.has_explicit_grouping + # But the two inner ones have explict parenthesis syntax + assert bop2.has_explicit_grouping + assert bop3.has_explicit_grouping + + @pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") def test_handling_array_product(): ''' Check that we correctly handle array products. diff --git a/src/psyclone/tests/psyir/nodes/call_test.py b/src/psyclone/tests/psyir/nodes/call_test.py index 667a8ee682..a52c2f7a27 100644 --- a/src/psyclone/tests/psyir/nodes/call_test.py +++ b/src/psyclone/tests/psyir/nodes/call_test.py @@ -379,12 +379,15 @@ def test_call_reference_accesses(): call1.reference_accesses(var_info) # Check that the current location number is increased after the call: assert var_info._location == 1 - assert not var_info.all_signatures + # The Routine symbol is not considered 'read'. + assert not var_info.is_read(Signature("trillian")) + assert var_info.is_called(Signature("trillian")) dsym = DataSymbol("beta", INTEGER_TYPE) # Simple argument passed by reference. call2 = Call.create(rsym, [Reference(dsym)]) call2.reference_accesses(var_info) assert var_info.has_read_write(Signature("beta")) + assert not var_info.is_called(Signature("beta")) # Array access argument. The array should be READWRITE, any variable in # the index expression should be READ. idx_sym = DataSymbol("ji", INTEGER_TYPE) @@ -418,6 +421,55 @@ def test_call_reference_accesses(): assert not var_info.is_written(Signature("beta")) +def test_type_bound_call_reference_accesses(fortran_reader): + '''Test the reference_accesses() method for a call to a type-bound + procedure. + + TODO #2823 - we currently make dangerous assumptions about accesses + to variables if whether or not they are being used as function + arguments is ambiguous. + + ''' + code = '''\ + module my_mod + contains + pure function get_start(idx) result(start) + integer, intent(in) :: idx + integer :: start + start = idx - 1 + end function get_start + subroutine my_sub + use some_mod, only: my_grid, domain + integer :: i, j, k, f + ! We know that get_start() is a call to a pure function so 'f' is + ! read only. + call my_grid(k)%update(get_start(f), domain%get_start(i),j) + end subroutine my_sub + end module my_mod + ''' + psyir = fortran_reader.psyir_from_source(code) + call = psyir.walk(Call)[0] + vai = VariablesAccessInfo() + call.reference_accesses(vai) + # The type-bound procedure is called. + assert vai.is_called(Signature("my_grid%update")) + # As is the function defined in the same module. + assert vai.is_called(Signature("get_start")) + # All of the indices are marked as read. + for var in ["i", "j", "k", "f"]: + assert vai.is_read(Signature(var)) + # Only the arguments to the calls are marked as read-write. + assert vai.has_read_write(Signature("j")) + assert not vai.has_read_write(Signature("k")) + assert not vai.has_read_write(Signature("f")) + # We can't tell whether 'domain%get_start(i)' is an array access + # or a function call. We currently, dangerously, assume it is the former. + if not vai.has_read_write(Signature("i")): + pytest.xfail(reason="TODO #2823 - potential array accesses/function " + "calls are always assumed to be array accesses. This is " + "unsafe.") + + def test_call_argumentnames_after_removearg(): '''Test the argument_names property makes things consistent if a child argument is removed. This is used transparently by the class to diff --git a/src/psyclone/tests/psyir/nodes/codeblock_test.py b/src/psyclone/tests/psyir/nodes/codeblock_test.py index 73639a5b92..c6f952fc3a 100644 --- a/src/psyclone/tests/psyir/nodes/codeblock_test.py +++ b/src/psyclone/tests/psyir/nodes/codeblock_test.py @@ -40,6 +40,7 @@ import pytest from fparser.common.readfortran import FortranStringReader +from psyclone.core import VariablesAccessInfo from psyclone.psyir.nodes import CodeBlock from psyclone.psyir.nodes.node import colored from psyclone.errors import GenerationError @@ -133,6 +134,52 @@ def test_codeblock_get_symbol_names(parser): assert "that_is_true" in sym_names +def test_codeblock_ref_accesses(parser): + '''Test that the reference_accesses() method works as expected. + + TODO #2863 - accesses within a CodeBlock should really be marked as + AccessType.UNKNOWN but are currently always READWRITE. Also, calls to + Fortran intrinsics are not captured. + + ''' + vai = VariablesAccessInfo() + reader = FortranStringReader(''' + subroutine mytest + that_is_true = .TRUE._bool_kind + hello_str = char_kind_"hello" + my_cmplx = (1.0_c_def, 1.0_b_def) + myloop: DO i = 1, 10_i_def + a = b + sqrt(c) + 1.0_r_def + myifblock: IF(this_is_true)THEN + EXIT myloop + ELSE IF(that_is_true)THEN myifblock + call my_routine() + write(*,*) "Bye" + ELSE myifblock + write(*,*) "hello" + END IF myifblock + END DO myloop + end subroutine mytest''') + prog = parser(reader) + block = CodeBlock(prog.children, CodeBlock.Structure.STATEMENT) + block.reference_accesses(vai) + all_sigs = vai.all_signatures + all_names = [sig.var_name for sig in all_sigs] + assert "a" in all_names + assert "i" in all_names + # Check that the various precision symbols are included. + assert "i_def" in all_names + assert "r_def" in all_names + assert "bool_kind" in all_names + assert "char_kind" in all_names + assert "c_def" in all_names + assert "b_def" in all_names + # The target of a CALL is included. + assert "my_routine" in all_names + # All signatures should be marked as READWRITE access. + assert all(vai.has_read_write(sig) for sig in all_sigs) + + def test_codeblock_equality(parser): '''Test the __eq__ method of the Codeblock class.''' reader = FortranStringReader(''' diff --git a/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py b/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py index 333096719d..4d1639902a 100644 --- a/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py +++ b/src/psyclone/tests/psyir/nodes/intrinsic_call_test.py @@ -418,7 +418,7 @@ def test_reference_accesses_bounds(operator, fortran_reader): '''Test that the reference_accesses method behaves as expected when the reference is the first argument to either the lbound or ubound intrinsic as that is simply looking up the array bounds (therefore - var_access_info should be empty) and when the reference is the + the access is an enquiry) and when the reference is the second argument of either the lbound or ubound intrinsic (in which case the access should be a read). @@ -434,15 +434,10 @@ def test_reference_accesses_bounds(operator, fortran_reader): psyir = fortran_reader.psyir_from_source(code) schedule = psyir.walk(Assignment)[0] - # By default, the access to 'a' should not be reported as read, - # but the access to b must be reported: + # The access to 'a' should be reported as 'NO_DATA_ACCESS' as its + # actual data is not accessed. vai = VariablesAccessInfo(schedule) - assert str(vai) == "b: READ, n: WRITE" - - # When explicitly requested, the access to 'a' should be reported: - vai = VariablesAccessInfo(schedule, - options={"COLLECT-ARRAY-SHAPE-READS": True}) - assert str(vai) == "a: READ, b: READ, n: WRITE" + assert str(vai) == "a: NO_DATA_ACCESS, b: READ, n: WRITE" def test_enumerator_name_matches_name_field(): diff --git a/src/psyclone/tests/psyir/nodes/omp_directives_test.py b/src/psyclone/tests/psyir/nodes/omp_directives_test.py index eabe3d632a..1acdb7800e 100644 --- a/src/psyclone/tests/psyir/nodes/omp_directives_test.py +++ b/src/psyclone/tests/psyir/nodes/omp_directives_test.py @@ -694,15 +694,17 @@ def test_directiveinfer_sharing_attributes_lfric(): def test_infer_sharing_attributes_with_explicitly_private_symbols( fortran_reader): ''' Tests the infer_sharing_attributes() method when some of the loops have - explictly declared private symbols. + explictly declared private symbols. Also test that non-data accesses to + array symbols are ignored. ''' psyir = fortran_reader.psyir_from_source(''' subroutine my_subroutine() integer :: i, j, scalar1, scalar2 - real, dimension(10) :: array + real, dimension(10) :: array, array2 do j = 1, 10 - do i = 1, 10 - array(i) = scalar2 + do i = 1, size(array, 1) + ! Access to array2 is type information rather than data + array(i) = scalar2 * size(array2, 1) enddo enddo end subroutine''') diff --git a/src/psyclone/tests/psyir/nodes/operation_test.py b/src/psyclone/tests/psyir/nodes/operation_test.py index 1102d1c688..2ce5d5375a 100644 --- a/src/psyclone/tests/psyir/nodes/operation_test.py +++ b/src/psyclone/tests/psyir/nodes/operation_test.py @@ -67,6 +67,11 @@ def test_binaryoperation_initialization(): "BinaryOperation.Operator but found" in str(err.value) bop = BinaryOperation(BinaryOperation.Operator.ADD) assert bop._operator is BinaryOperation.Operator.ADD + assert not bop._has_explicit_grouping # Defaults to False + + bop = BinaryOperation(BinaryOperation.Operator.ADD, + has_explicit_grouping=True) + assert bop._has_explicit_grouping # But can be set to True def test_binaryoperation_operator(): @@ -78,6 +83,23 @@ def test_binaryoperation_operator(): assert binary_operation.operator == BinaryOperation.Operator.ADD +def test_binaryoperation_has_explicit_grouping(): + ''' Check the setter/getter methods of the has_explicit_grouping' of + BinaryOperation.''' + + bop = BinaryOperation(BinaryOperation.Operator.ADD) + assert not bop.has_explicit_grouping + + with pytest.raises(TypeError) as err: + bop.has_explicit_grouping = "3" + assert ("BinaryOperation.has_explicit_grouping must be boolean, but " + "found 'str'.") in str(err.value) + + # Try valid setter/getter + bop.has_explicit_grouping = True + assert bop.has_explicit_grouping + + def test_binaryoperation_node_str(): ''' Check the node_str method of the Binary Operation class.''' binary_operation = BinaryOperation(BinaryOperation.Operator.ADD) @@ -114,10 +136,16 @@ def test_binaryoperation_create(): rhs = Reference(DataSymbol("tmp2", REAL_SINGLE_TYPE)) oper = BinaryOperation.Operator.ADD binaryoperation = BinaryOperation.create(oper, lhs, rhs) + assert not binaryoperation._has_explicit_grouping check_links(binaryoperation, [lhs, rhs]) result = FortranWriter().binaryoperation_node(binaryoperation) assert result == "tmp1 + tmp2" + # Check with the optional has_explicit_grouping parameter + binaryoperation = BinaryOperation.create( + oper, lhs.copy(), rhs.copy(), has_explicit_grouping=True) + assert binaryoperation.has_explicit_grouping + def test_binaryoperation_create_invalid(): '''Test that the create method in a BinaryOperation class raises the diff --git a/src/psyclone/tests/psyir/nodes/scoping_node_test.py b/src/psyclone/tests/psyir/nodes/scoping_node_test.py index 9689cc4834..71e2a94c50 100644 --- a/src/psyclone/tests/psyir/nodes/scoping_node_test.py +++ b/src/psyclone/tests/psyir/nodes/scoping_node_test.py @@ -37,10 +37,14 @@ ''' Performs py.test tests on the ScopingNode PSyIR node. ''' import pytest -from psyclone.psyir.nodes import (Schedule, Assignment, Reference, Container, - Loop, Literal, Routine, ArrayReference) -from psyclone.psyir.symbols import (DataSymbol, ArrayType, INTEGER_TYPE, - ArgumentInterface, SymbolTable, REAL_TYPE) +from psyclone.core import Signature, VariablesAccessInfo +from psyclone.psyir.nodes import ( + Schedule, Assignment, Reference, Container, Loop, Literal, + Routine, ArrayReference) +from psyclone.psyir.symbols import ( + ArrayType, ArgumentInterface, DataSymbol, DataTypeSymbol, INTEGER_TYPE, + REAL_TYPE, ScalarType, StructureType, Symbol, SymbolTable, + UnsupportedFortranType) from psyclone.tests.utilities import Compile @@ -275,3 +279,116 @@ def test_scoping_node_equality(): assert sched1 == sched2 assert sched1 != sched3 + + +def test_scoping_node_reference_accesses(): + '''Test the reference_accesses() method of ScopingNode.''' + vai = VariablesAccessInfo() + sched = Schedule() + table = sched.symbol_table + # First test with an empty symbol table. + sched.reference_accesses(vai) + assert not vai.all_signatures + # Just adding a Symbol to the table does not affect anything. + prsym = table.new_symbol("r_def", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + sched.reference_accesses(vai) + assert not vai.all_signatures + # Add another Symbol that references the first one in its precision. + new_type = ScalarType(ScalarType.Intrinsic.REAL, prsym) + _ = table.new_symbol("var1", symbol_type=DataSymbol, datatype=new_type) + sched.reference_accesses(vai) + assert vai.all_signatures == [Signature("r_def")] + assert not vai[Signature("r_def")].has_data_access() + # Add a Symbol with initialisation. + idef = table.new_symbol("i_def", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + int_type = ScalarType(ScalarType.Intrinsic.INTEGER, idef) + _ = table.new_symbol("var2", symbol_type=DataSymbol, + datatype=INTEGER_TYPE, + is_constant=True, + initial_value=Literal("100", int_type)) + vai2 = VariablesAccessInfo() + sched.reference_accesses(vai2) + assert len(vai2.all_signatures) == 2 + assert Signature("i_def") in vai2.all_signatures + assert not vai2[Signature("i_def")].has_data_access() + + +def test_reference_accesses_struct(): + '''Test reference_accesses() when the associated SymbolTable contains + a StructureType. + + ''' + sched = Schedule() + table = sched.symbol_table + idef = table.new_symbol("i_def", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + rdef = table.new_symbol("r_def", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + int_type = ScalarType(ScalarType.Intrinsic.INTEGER, idef) + real_type = ScalarType(ScalarType.Intrinsic.INTEGER, rdef) + stype = StructureType.create([ + ("iflag", int_type, Symbol.Visibility.PRIVATE, None), + ("rmask", real_type, Symbol.Visibility.PUBLIC, + Literal("100", real_type))]) + ssym = DataTypeSymbol("my_type", stype) + table.add(ssym) + vai3 = VariablesAccessInfo() + sched.reference_accesses(vai3) + assert len(vai3.all_signatures) == 2 + assert Signature("i_def") in vai3.all_signatures + assert Signature("r_def") in vai3.all_signatures + table.new_symbol("var4", symbol_type=DataSymbol, datatype=ssym) + vai4 = VariablesAccessInfo() + sched.reference_accesses(vai4) + + +def test_reference_accesses_array(): + '''Test reference_accesses() when the associated SymbolTable contains + an array with dimensions that make reference to another Symbol. + + ''' + sched = Schedule() + table = sched.symbol_table + idef = table.new_symbol("i_def", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + rdef = table.new_symbol("r_def", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + int_type = ScalarType(ScalarType.Intrinsic.INTEGER, idef) + real_type = ScalarType(ScalarType.Intrinsic.REAL, rdef) + var2 = table.new_symbol("var2", symbol_type=DataSymbol, + datatype=INTEGER_TYPE, is_constant=True, + initial_value=Literal("100", int_type)) + atype = ArrayType(real_type, [Reference(var2)]) + _ = table.new_symbol("var3", symbol_type=DataSymbol, datatype=atype) + vai = VariablesAccessInfo() + sched.reference_accesses(vai) + assert Signature("i_def") in vai.all_signatures + assert Signature("r_def") in vai.all_signatures + assert Signature("var2") in vai.all_signatures + + +def test_reference_accesses_unknown_type(): + '''Test reference_accesses() when the symbol table contains a symbol + of UnsupportedFortranType but with partial type information. + + ''' + sched = Schedule() + table = sched.symbol_table + # Create partial type information - an array of specified precision + # with an extent specified by another symbol. + rdef = table.new_symbol("r_def", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + big_sym = table.new_symbol("big", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + real_type = ScalarType(ScalarType.Intrinsic.REAL, rdef) + ptype = ArrayType(real_type, [Reference(big_sym)]) + utype = UnsupportedFortranType( + "real(r_def), dimension(big), target :: array", + partial_datatype=ptype) + table.new_symbol("array", symbol_type=DataSymbol, datatype=utype) + vai = VariablesAccessInfo() + sched.reference_accesses(vai) + assert Signature("r_def") in vai.all_signatures + assert Signature("big") in vai.all_signatures diff --git a/src/psyclone/tests/psyir/nodes/structure_reference_test.py b/src/psyclone/tests/psyir/nodes/structure_reference_test.py index 7a0ec8e921..c40a78b6b4 100644 --- a/src/psyclone/tests/psyir/nodes/structure_reference_test.py +++ b/src/psyclone/tests/psyir/nodes/structure_reference_test.py @@ -232,7 +232,7 @@ def test_struc_ref_datatype(): '''Test the datatype() method of StructureReference.''' atype = symbols.ArrayType(symbols.REAL_TYPE, [10, 8]) rtype = symbols.StructureType.create([ - ("gibber", symbols.BOOLEAN_TYPE, symbols.Symbol.Visibility.PUBLIC, + ("Gibber", symbols.BOOLEAN_TYPE, symbols.Symbol.Visibility.PUBLIC, None)]) # TODO #1031. Currently cannot create an array of StructureTypes # directly - have to have a DataTypeSymbol. @@ -241,10 +241,10 @@ def test_struc_ref_datatype(): grid_type = symbols.StructureType.create([ ("nx", symbols.INTEGER_TYPE, symbols.Symbol.Visibility.PUBLIC, None), ("data", atype, symbols.Symbol.Visibility.PRIVATE, None), - # A single member of structure type. - ("roger", rtype, symbols.Symbol.Visibility.PUBLIC, None), + # A single member of structure type with a mixed-case name. + ("roGEr", rtype, symbols.Symbol.Visibility.PUBLIC, None), # An array of structure type. - ("titty", artype, symbols.Symbol.Visibility.PUBLIC, None)]) + ("lucy", artype, symbols.Symbol.Visibility.PUBLIC, None)]) # Symbol with type defined by StructureType ssym0 = symbols.DataSymbol("grid", grid_type) # Reference to scalar member of structure @@ -266,34 +266,34 @@ def test_struc_ref_datatype(): gref = nodes.StructureReference.create(ssym, ["roger", "gibber"]) assert gref.datatype == symbols.BOOLEAN_TYPE - # Reference to structure member of structure - rref = nodes.StructureReference.create(ssym, ["roger"]) + # Reference to structure member of structure using different capitalisation + rref = nodes.StructureReference.create(ssym, ["rogeR"]) assert rref.datatype == rtype # Reference to single element of array of structures within a structure singleref = nodes.StructureReference.create( - ssym, [("titty", [one.copy(), two.copy()])]) + ssym, [("lucy", [one.copy(), two.copy()])]) assert singleref.datatype == rtype_sym - # Reference to grid%titty(1,2)%gibber + # Reference to grid%lucy(1,2)%gibber sref = nodes.StructureReference.create( - ssym, [("titty", [one.copy(), two.copy()]), "gibber"]) + ssym, [("lucy", [one.copy(), two.copy()]), "gibber"]) assert sref.datatype == symbols.BOOLEAN_TYPE - # Reference to grid%titty(my_func(1), 2) where my_func is unresolved. + # Reference to grid%lucy(my_func(1), 2) where my_func is unresolved. func_sym = symbols.DataSymbol("my_func", datatype=symbols.UnresolvedType(), interface=symbols.UnresolvedInterface()) fref = nodes.ArrayReference.create(func_sym, [one.copy()]) sref2 = nodes.StructureReference.create( - ssym, [("titty", [fref, two.copy()])]) + ssym, [("lucy", [fref, two.copy()])]) assert isinstance(sref2.datatype, symbols.UnresolvedType) # Reference to sub-array of structure members of structure myrange = nodes.Range.create(two.copy(), nodes.Literal("4", symbols.INTEGER_TYPE)) arref = nodes.StructureReference.create( - ssym, [("titty", [nodes.Literal("3", symbols.INTEGER_TYPE), myrange])]) + ssym, [("lucy", [nodes.Literal("3", symbols.INTEGER_TYPE), myrange])]) dtype = arref.datatype assert isinstance(dtype, symbols.ArrayType) assert dtype.intrinsic == rtype_sym @@ -302,7 +302,7 @@ def test_struc_ref_datatype(): assert isinstance(dtype.shape[0].upper, nodes.BinaryOperation) # Reference to whole array of structures that are a member of a structure - fullref = nodes.StructureReference.create(ssym, ["titty"]) + fullref = nodes.StructureReference.create(ssym, ["lucy"]) dtype = fullref.datatype assert dtype == artype diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 27382f3cfa..09df5046a1 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -894,8 +894,11 @@ def test_structure_type(): stype = StructureType() assert str(stype) == "StructureType<>" assert not stype.components - stype.add("flag", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None) + stype.add("flaG", INTEGER_TYPE, Symbol.Visibility.PUBLIC, None) + # Lookup is not case sensitive flag = stype.lookup("flag") + # But we retain information on the original capitalisation + assert flag.name == "flaG" assert not flag.initial_value assert isinstance(flag, StructureType.ComponentType) stype.add("flag2", INTEGER_TYPE, Symbol.Visibility.PUBLIC, diff --git a/src/psyclone/tests/psyir/tools/call_tree_utils_test.py b/src/psyclone/tests/psyir/tools/call_tree_utils_test.py index 397b198a0c..dcc4127d7b 100644 --- a/src/psyclone/tests/psyir/tools/call_tree_utils_test.py +++ b/src/psyclone/tests/psyir/tools/call_tree_utils_test.py @@ -569,14 +569,13 @@ def test_call_tree_utils_inout_parameters_generic(fortran_reader): # inside the ubound/lbound function calls. read_write_info = ReadWriteInfo() ctu.get_input_parameters(read_write_info, loops, - options={'COLLECT-ARRAY-SHAPE-READS': True}) + include_non_data_accesses=True) input_set = set(sig for _, sig in read_write_info.set_of_all_used_vars) assert input_set == set([Signature("b"), Signature("c"), Signature("jpj"), Signature("dummy")]) - read_write_info = ctu.\ - get_in_out_parameters(loops, - options={'COLLECT-ARRAY-SHAPE-READS': True}) + read_write_info = ctu.get_in_out_parameters(loops, + include_non_data_accesses=True) output_set = set(read_write_info.signatures_read) assert output_set == set([Signature("b"), Signature("c"), Signature("jpj"), Signature("dummy")]) diff --git a/src/psyclone/tests/psyir/tools/dependency_tools_test.py b/src/psyclone/tests/psyir/tools/dependency_tools_test.py index fee1cd4289..8c4e28995f 100644 --- a/src/psyclone/tests/psyir/tools/dependency_tools_test.py +++ b/src/psyclone/tests/psyir/tools/dependency_tools_test.py @@ -39,7 +39,7 @@ import pytest from psyclone.configuration import Config -from psyclone.core import Signature, VariablesAccessInfo +from psyclone.core import AccessType, Signature, VariablesAccessInfo from psyclone.errors import InternalError from psyclone.psyir.nodes import Loop from psyclone.psyir.tools import DependencyTools, DTCode @@ -356,9 +356,18 @@ def test_array_access_pairs_1_var(lhs, rhs, distance, fortran_reader): assign = psyir.children[0].children[0] sig = Signature("a1") - # Get all access info for the expression to 'a1' - access_info_lhs = VariablesAccessInfo(assign.lhs)[sig][0] - access_info_rhs = VariablesAccessInfo(assign.rhs)[sig][0] + # Get the READ access to 'a1' for expression (this is complicated by the + # presence of 'inquiry' accesses for the array bounds in some cases). + a1vinfo = VariablesAccessInfo(assign.lhs)[sig] + for access in a1vinfo.all_accesses: + if access.access_type == AccessType.READ: + access_info_lhs = access + break + a1vinfo_rh = VariablesAccessInfo(assign.rhs)[sig] + for access in a1vinfo_rh.all_accesses: + if access.access_type == AccessType.READ: + access_info_rhs = access + break subscript_lhs = access_info_lhs.component_indices[(0, 0)] subscript_rhs = access_info_rhs.component_indices[(0, 0)] diff --git a/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py b/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py index 8216017652..e569d01b39 100644 --- a/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/acc_kernels_trans_test.py @@ -438,19 +438,33 @@ def test_no_assumed_size_char_in_kernels(fortran_reader): sub = psyir.walk(Routine)[0] acc_trans = ACCKernelsTrans() with pytest.raises(TransformationError) as err: - acc_trans.validate(sub.children[0], options={}) + acc_trans.validate(sub.children[0]) assert ("Assumed-size character variables cannot be enclosed in an " "OpenACC region but found 'if (assumed_size_char == 'literal')" in str(err.value)) with pytest.raises(TransformationError) as err: - acc_trans.validate(sub.children[1], options={}) + acc_trans.validate(sub.children[1], options={"allow_string": True}) assert ("Assumed-size character variables cannot be enclosed in an OpenACC" " region but found 'assumed_size_char(:LEN(explicit_size_char)) = " in str(err.value)) with pytest.raises(TransformationError) as err: - acc_trans.validate(sub.children[2], options={}) + acc_trans.validate(sub.children[2], options={"allow_string": True}) assert ("Cannot include 'ACHAR(9)' in an OpenACC region because " "it is not available on GPU" in str(err.value)) + # Check that the character assignment is excluded by default. + with pytest.raises(TransformationError) as err: + acc_trans.validate(sub.children[2]) + assert ("ACCKernelsTrans does not permit assignments involving character " + "variables by default (use the 'allow_string' option to include " + "them), but found:" in str(err.value)) + # Check the verbose option. + with pytest.raises(TransformationError) as err: + acc_trans.validate(sub.children[2], options={"verbose": True}) + assert (sub.children[2].preceding_comment == + "ACCKernelsTrans does not permit assignments involving character " + "variables by default (use the 'allow_string' option to include " + "them)") + # String with explicit length is fine. acc_trans.validate(sub.children[3], options={}) # CHARACTER*(*) notation is also rejected. diff --git a/src/psyclone/tests/psyir/transformations/extract_trans_test.py b/src/psyclone/tests/psyir/transformations/extract_trans_test.py index 2889e3ffd8..dab1a80d96 100644 --- a/src/psyclone/tests/psyir/transformations/extract_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/extract_trans_test.py @@ -118,14 +118,6 @@ def test_determine_postfix(): assert postfix == "_post1" -# --------------------------------------------------------------------------- # -def test_get_default_options(): - '''Check the default options.''' - - etrans = ExtractTrans() - assert etrans.get_default_options() == {"COLLECT-ARRAY-SHAPE-READS": True} - - # ----------------------------------------------------------------------------- def test_extract_validate(): '''Test that the validate function can successfully finish.''' diff --git a/src/psyclone/tests/psyir/transformations/hoist_trans_test.py b/src/psyclone/tests/psyir/transformations/hoist_trans_test.py index de105065ee..9a69054d87 100644 --- a/src/psyclone/tests/psyir/transformations/hoist_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/hoist_trans_test.py @@ -482,6 +482,41 @@ def test_apply_remove_emtpy_loops(fortran_reader, fortran_writer): assert expected in fortran_writer(psyir) +def test_validate_array_of_struct(fortran_reader): + ''' + Test that validate() spots the use of an array variable within + an intrinsic call. + + ''' + psyir = fortran_reader.psyir_from_source( + ''' + module lbcnfd + use lib_mpp + contains + SUBROUTINE lbc_nfd_dp( ptab, cd_nat, psgn, kfld ) + TYPE(PTR_4d_dp), DIMENSION(:), INTENT(inout) :: ptab + CHARACTER(len=1), DIMENSION(:), INTENT(in ) :: cd_nat + REAL(dp), DIMENSION(:), INTENT(in ) :: psgn + INTEGER , INTENT(in ) :: kfld + integer :: jf + integer :: ipi + integer :: ij2 + + do jf = 1, kfld, 1 + ipi = SIZE(ptab(jf)%pt4d, 1) + end do + end subroutine lbc_nfd_dp + end module lbcnfd + ''' + ) + loop = psyir.walk(Loop)[0] + hoist_trans = HoistTrans() + with pytest.raises(TransformationError) as err: + hoist_trans.validate(loop.loop_body[0]) + assert ("The statement 'ipi = SIZE(ptab(jf)%pt4d, 1)' can't be hoisted as " + "it reads variable 'jf'" in str(err.value)) + + def test_str(): '''Test the hoist transformation's str method return the expected results. diff --git a/src/psyclone/tests/psyir/transformations/psy_data_trans_test.py b/src/psyclone/tests/psyir/transformations/psy_data_trans_test.py index 622ee62af5..3ca7f38597 100644 --- a/src/psyclone/tests/psyir/transformations/psy_data_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/psy_data_trans_test.py @@ -200,21 +200,7 @@ def test_trans_with_shape_function(monkeypatch, fortran_reader, config = Config.get() monkeypatch.setattr(config, "distributed_memory", False) - psyir_copy = psyir.copy() transformation.apply(loop) out = fortran_writer(psyir) assert 'PreDeclareVariable("dummy", dummy)' in out assert 'ProvideVariable("dummy", dummy)' in out - - # Now check that the user can overwrite 'COLLECT-ARRAY-SHAPE-READS' - # ================================================================= - # Since the original tree was modified, we now use the copy: - - loop = psyir_copy.children[0].children[0] - # Disable array-shape-reads, which means 'dummy' should then not - # be in the list of input parameters anymore, and therefore - transformation.apply(loop, options={"COLLECT-ARRAY-SHAPE-READS": False}) - out = fortran_writer(psyir_copy) - # No reference to 'dummy' should be in the created code: - assert 'PreDeclareVariable("dummy", dummy)' not in out - assert 'ProvideVariable("dummy", dummy)' not in out