diff --git a/.github/workflows/nemo_tests.yml b/.github/workflows/nemo_tests.yml new file mode 100644 index 00000000000..478d76651bb --- /dev/null +++ b/.github/workflows/nemo_tests.yml @@ -0,0 +1,132 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2023, 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 S. Siso, STFC Daresbury Lab + +# This workflow will use a self-hosted runner to perform the more expensive +# integrations tests that are not run on GHA systems. + +name: NEMO Integration Tests + +on: + push + +jobs: + run_if_on_mirror: + if: ${{ github.repository == 'stfc/PSyclone-mirror' }} + runs-on: self-hosted + env: + PERL5LIB: /home/aporter/perl5/lib/perl5 + PERL_LOCAL_LIB_ROOT: /home/aporter/perl5 + PERL_MB_OPT: "--install_base \"/home/aporter/perl5\"" + PERL_MM_OPT: "INSTALL_BASE=/home/aporter/perl5" + + steps: + - uses: actions/checkout@v3 + with: + submodules: recursive + # This is required to get the commit history for merge commits for + # the ci-skip check below. + fetch-depth: '0' + - name: Check for [skip ci] in commit message + uses: mstachniuk/ci-skip@v1 + with: + # This setting causes the tests to 'fail' if [skip ci] is specified + fail-fast: true + commit-filter: '[skip ci]' + - name: Install dependencies + run: | + python -m venv .runner_venv + . .runner_venv/bin/activate + python -m pip install --upgrade pip + # If you wish to install the version of fparser pointed to by the + # submodule instead of the released version (from PyPI) then + # uncomment the following line: + pip install external/fparser + pip install .[test] + # Add Perl to the PATH + echo "/home/aporter/perl5/bin" >> $GITHUB_PATH + # Compile nvidia profiling tools + module load nvidia-hpcsdk + cd lib/profiling/nvidia/ + F90=nvfortran make + + # PSyclone, compile and run MetOffice NEMO with OpenMP for GPUs + - name: NEMO MetOffice OpenMP for GPU + run: | + . .runner_venv/bin/activate + export PSYCLONE_NEMO_DIR=${GITHUB_WORKSPACE}/examples/nemo/scripts + export NEMO_DIR=${HOME}/NEMO + cd examples/nemo/scripts + make -j 4 openmp_gpu + module load nvidia-hpcsdk netcdf_fortran + make -j 4 compile-openmp_gpu + export NV_ACC_POOL_THRESHOLD=75 + make run-openmp_gpu | tee output.txt + # Check the output is as expected for the first 6 digits + tail -n 1 output.txt | grep -q " it : 10" + tail -n 1 output.txt | grep -q "|ssh|_max: 0.259483" + tail -n 1 output.txt | grep -q "|U|_max: 0.458515" + tail -n 1 output.txt | grep -q "S_min: 0.482686" + tail -n 1 output.txt | grep -q "S_max: 0.407622" + grep -A 1 "Elapsed Time" output.txt + echo $GITHUB_REF_NAME $GITHUB_SHA $(grep -A 1 "Elapsed Time" output.txt | head -n 2 | tail -n 1) >> ${NEMO_DIR}/performance_history + + # PSyclone, compile and run ECMWF NEMO with OpenMP for CPUs + - name: NEMO ECMWF OpenMP for CPU + run: | + . .runner_venv/bin/activate + export PSYCLONE_NEMO_DIR=${GITHUB_WORKSPACE}/examples/nemo/scripts + export NEMO_DIR=${HOME}/NEMOGCM_V40 + export COMPILER_ARCH=linux_intel + export ADD_KEYS="IEEE_IS_NAN=ieee_is_nan key_nosignedzero" + export DEL_KEYS="key_iomput" + export MODEL_DIR=/archive/ssiso/ecmwf_eORCA1_GO8/ + export NAMELISTS_DIR=${NEMO_DIR}/testscripts_V40/output/openmp_outer_V40_eORCA1_GO8_Z75_20170906_cray_dp_1x1/ + cd examples/nemo/scripts + make -j 4 openmp_cpu + module load intel/oneapi compiler mpi + export LD_LIBRARY_PATH=/home/ssiso/ecmwf_nemo/ESiWACE2/scripts/dev/netcdf-c-4.9.0/lib/:$LD_LIBRARY_PATH + export LD_LIBRARY_PATH=/home/ssiso/ecmwf_nemo/ESiWACE2/scripts/dev/netcdf-fortran-4.5.4/lib:$LD_LIBRARY_PATH + export LD_LIBRARY_PATH=/home/ssiso/ecmwf_nemo/ESiWACE2/scripts/dev/hdf5-1.12.2/lib/:$LD_LIBRARY_PATH + make -j 4 compile-openmp_cpu + export OMP_NUM_THREADS=4 + make run-openmp_cpu | tee output.txt + # Check the output is as expected for the first 6 digits + tail -n 1 output.txt | grep -q " it : 10" + tail -n 1 output.txt | grep -q "|ssh|_max: 0.199714" + tail -n 1 output.txt | grep -q "|U|_max: 0.148409" + tail -n 1 output.txt | grep -q "S_min: 0.108530" + tail -n 1 output.txt | grep -q "S_max: 0.404045" + grep -A 1 "Elapsed Time" output.txt + echo $GITHUB_REF_NAME $GITHUB_SHA $(grep -A 1 "Elapsed Time" output.txt | head -n 2 | tail -n 1) >> ${NEMO_DIR}/performance_history diff --git a/.gitignore b/.gitignore index f401dd74e57..64ddd59297b 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,5 @@ src/*.egg-info *.sublime-project *.sublime-workspace .idea +.rtx.toml +.venv diff --git a/changelog b/changelog index a991e339f99..b52c920e188 100644 --- a/changelog +++ b/changelog @@ -470,6 +470,42 @@ 159) PR #2181 for #2175. Bug fixes for processing of Fortran declarations and integer literals. + 160) PR #2107 for #2106. Improvements to NEMO processing scripts and + inclusion in Integration Testing. + + 161) PR #2198 for #2194. Fixes handling of a function where the + precision of the result is only defined within the function. + + 162) PR #2187 towards #1618. Remove builtin use statements when + lowering. + + 163) PR #2191 for #2185. Fix element order used in PSyAD test + harness. + + 164) PR #2208 towards #2205. Ensures that functions or subroutines + that CONTAIN other functions/subroutines are put into a CodeBlock. + + 165) PR #2124 fir #2101. Refactors OpenMP inference of private/first- + private variables and those requiring synchronisation. + + 166) PR #2210 for #2206. Ensure symbol renaming included when deep- + copying a symbol table. + + 167) PR #2209 for #2204. Add support for EQV and NEQV and fix select + case bug with logical comparisons. + + 168) PR #2211 for #2183. Change PSyAD test-harness generation to use + the same variable names as the TL kernel being tested. + + 169) PR #2212 for #2203. Fix bug in PSyIR fparser2 frontend when + DO WHILE contains normal DO loops. + + 170) PR #2220 for #2219. Adds 'shared_with' argument to the + node.ancestor() method. + + 171) PR #2174 for #2171. Add support for obtaining the partial datatype + of an UnknownFortranType. + release 2.3.1 17th of June 2022 1) PR #1747 for #1720. Adds support for If blocks to PSyAD. diff --git a/doc/developer_guide/working_practises.rst b/doc/developer_guide/working_practises.rst index 2a8e1e3d66a..43dd2a33ddf 100644 --- a/doc/developer_guide/working_practises.rst +++ b/doc/developer_guide/working_practises.rst @@ -545,10 +545,10 @@ As mentioned above, running the test suite and/or examples with compilation enabled significantly increases the required compute time. However, there is a need to test PSyclone with full builds of the LFRic and NEMO applications. Therefore, in addition to the principal action described -above, there are two others which are configured in ``repo-sync.yml`` -and ``compilation.yml``. +above, there are the following workflow files that manage multiple +Integration tests: -The ``repo-sync`` action must be triggered +The ``repo-sync`` action, which must be triggered manually (on GitHub) and pushes a copy of the current branch to a private repository. (This action uses the ``integration`` environment and can therefore only be triggered by GitHub users who have ``review`` permissions @@ -556,15 +556,19 @@ in that environment.) That private repository has a GitHub self-hosted runner setup which then enables tests to be run on a machine at the Hartree Centre. Access to the private repository is handled using ssh with a key saved as a 'secret' in the GitHub PSyclone repository. - -It is the work performed by the self-hosted runner that is configured in the -``compilation.yml`` file. Currently this is limited to simply running the test -suite with compilation enabled (using ``gfortran``) but we plan to extend this -to perform integration tests with whole LFRic and NEMO applications, including -GPU execution. Since the self-hosted runner is only available in the private -repository, this action is configured such that it only runs if the name +The work performed by the self-hosted runner is configured in the ``yml`` files +below. Since the self-hosted runner is only available in the private +repository, these action are configured such that they only run if the name of the repository is that of the private one. +The ``compilation.yml`` action, runs the test suite and examples with +compilation enabled (using ``gfortran``). + +The ``nemo.yml`` action, processes the NEMO source code (available in +self-hosted runner) with the PSyclone scripts in examples/nemo/scripts. +Then it compiles the generated code, runs it, and validates that the +output produced matches with the expected results. + Performance =========== diff --git a/examples/nemo/scripts/.gitignore b/examples/nemo/scripts/.gitignore new file mode 100644 index 00000000000..5fad75984f3 --- /dev/null +++ b/examples/nemo/scripts/.gitignore @@ -0,0 +1 @@ +psycloned-* diff --git a/examples/nemo/scripts/Makefile b/examples/nemo/scripts/Makefile new file mode 100644 index 00000000000..59c142467f4 --- /dev/null +++ b/examples/nemo/scripts/Makefile @@ -0,0 +1,108 @@ +# Makefile to Generate PSyclone versions of NEMO, Compile, and Run them. +# It supports the targets: serial (only compile), openmp-cpu, openmp-gpu, openacc-kernels +# - To generate the psycloned versions use: `make ` +# - To compile that version use: `make compile-` +# - To run that version use: `make run-` + +# ---- Start of the configurable part of the Makefile ---- + +# - Specify NEMO directory +# NEMO_DIR ?= +# - Specify location of PSyclone transformation scripts +PSYCLONE_NEMO_DIR ?= ${WORKSPACE}/PSyclone/examples/nemo/scripts +# - Specify location of preprocessed files - you need to call 'make compile-serial' once first) +# ROOT_SRC ?= +# - Specify NEMO test case +TEST_CASE ?= SPITZ12 +# - Specify compilation options +# COMPILER_ARCH ?= +# ADD_KEYS ?= +# DEL_KEYS ?= +# - Specify input files +# MODEL_DIR ?= +# NAMELISTS_DIR ?= + +# Example Config for MetOffice - eORCA1_GO8 - nvidia +NEMO_DIR ?= ${WORKSPACE}/NEMO +ROOT_SRC ?= ${NEMO_DIR}/cfgs/SPITZ12_serial/BLD/ppsrc/nemo/ +COMPILER_ARCH ?= linux_nvidia_omp_gpu +ADD_KEYS ?= "IEEE_IS_NAN=ieee_is_nan key_nosignedzero" +DEL_KEYS ?= "key_iomput key_mpp_mpi key_si3" +MODEL_DIR ?= /home/aporter/NEMO/orca1_inputs/ +NAMELISTS_DIR ?= ${NEMO_DIR}/ + +# Example Config for ECMWF - eORCA1_GO8_Z75 (with MPI and SI3) - intel compiler +# NEMO_DIR ?= ${WORKSPACE}/ecmwf_nemo/nemo/NEMOGCM_V40/ +# ROOT_SRC ?= ${NEMO_DIR}/cfgs/SPITZ12_serial/BLD/ppsrc/nemo/ +# COMPILER_ARCH ?= linux_intel +# ADD_KEYS ?= "IEEE_IS_NAN=ieee_is_nan key_nosignedzero" +# DEL_KEYS ?= "key_iomput" +# MODEL_DIR ?= /archive/ssiso/ecmwf_eORCA1_GO8/ +# NAMELISTS_DIR ?= ${WORKSPACE}/ecmwf_nemo/nemo/testscripts_V40/output/openmp_outer_V40_eORCA1_GO8_Z75_20170906_cray_dp_1x1/ + +# ---- End of configuration section - do not edit below this point ---- + +TARGETS := openmp_cpu openmp_gpu openacc_kernels openacc_loops +SRC_FILES := $(wildcard ${ROOT_SRC}/*.f90) +OUTPUT_FOLDERS := $(addprefix psycloned-, ${TARGETS}) + +.PHONY: clean + +# Generate PSycloned folders +$(OUTPUT_FOLDERS): + @echo "Creating folder $@" + mkdir $@ + +# PSyclone targets (process all the f90 files in ${ROOT_SRC}) +$(TARGETS): % : $(addprefix psycloned-%/, $(notdir $(SRC_FILES))) + @echo "Finished generating $@" + +# PSyclone instructions for each target type +psycloned-openmp_cpu/%.f90: ${ROOT_SRC}%.f90 psycloned-openmp_cpu + ${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/omp_cpu_trans.py -I ${ROOT_SRC} -o psycloned-openmp_cpu $< + +psycloned-openmp_gpu/%.f90: ${ROOT_SRC}%.f90 psycloned-openmp_gpu + ${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/omp_gpu_trans.py -I ${ROOT_SRC} -o psycloned-openmp_gpu $< + +psycloned-openacc_kernels/%.f90: ${ROOT_SRC}%.f90 psycloned-openacc_kernels + ${PSYCLONE_NEMO_DIR}/process_nemo.py -s ${PSYCLONE_NEMO_DIR}/kernels_trans.py -I ${ROOT_SRC} -o psycloned-openacc_kernels $< + + +# Get the number of Makefile parallel jobs to pass it to the makenemo +MAKE_PID := $(shell echo $$PPID) +JOBS := $(shell ps T | sed -n 's/.*$(MAKE_PID).*$(MAKE).* \(-j\|--jobs=\) *\([0-9][0-9]*\).*/\2/p') +JOBS := $(if ${JOBS}, ${JOBS}, 4) # If none were given, default to 4 + +# Compile NEMO +compile-%: + @test -s psycloned-$(lastword $(subst -, ,$@)) || { \ + echo "The psycloned-$(lastword $(subst -, ,$@)) folder does not exist!"; \ + echo "You may need to execute 'make $(lastword $(subst -, ,$@))' first."; \ + echo "Exiting..."; exit 1; } + cd $(NEMO_DIR) ; \ + ./makenemo -n ${TEST_CASE}_$(lastword $(subst -, ,$@)) -r ${TEST_CASE} \ + -e ${PWD}/psycloned-$(lastword $(subst -, ,$@)) -m ${COMPILER_ARCH} -j ${JOBS} \ + add_key ${ADD_KEYS} del_key ${DEL_KEYS} + +# The compile-serial is a special case +compile-serial: + cd $(NEMO_DIR) ; \ + ./makenemo -n ${TEST_CASE}_serial -r ${TEST_CASE} \ + -m ${COMPILER_ARCH} -j ${JOBS} add_key ${ADD_KEYS} del_key ${DEL_KEYS} + +# Run NEMO +run-%: + ln -sf ${MODEL_DIR}/*.nc ${NEMO_DIR}/cfgs/${TEST_CASE}_$(lastword $(subst -, ,$@))/EXP00/. + cp ${NAMELISTS_DIR}namelist_* ${NEMO_DIR}/cfgs/${TEST_CASE}_$(lastword $(subst -, ,$@))/EXP00/. + cd ${NEMO_DIR}/cfgs/${TEST_CASE}_$(lastword $(subst -, ,$@))/EXP00; ./nemo + cd ${NEMO_DIR}/cfgs/${TEST_CASE}_$(lastword $(subst -, ,$@))/EXP00; cat timing.output; cat run.stat + +# Run NEMO with NVPROF +nvprof-%: + ln -sf ${MODEL_DIR}/*.nc ${NEMO_DIR}/cfgs/${TEST_CASE}_$(lastword $(subst -, ,$@))/EXP00/. + cp ${NAMELISTS_DIR}namelist_* ${NEMO_DIR}/cfgs/${TEST_CASE}_$(lastword $(subst -, ,$@))/EXP00/. + cd ${NEMO_DIR}/cfgs/${TEST_CASE}_$(lastword $(subst -, ,$@))/EXP00; nsys profile ./nemo + +# Clean (only psycloned- folders) +clean: + rm -rf $(OUTPUT_FOLDERS) diff --git a/examples/nemo/scripts/omp_cpu_trans.py b/examples/nemo/scripts/omp_cpu_trans.py index d4b1a26aac3..4489691257f 100755 --- a/examples/nemo/scripts/omp_cpu_trans.py +++ b/examples/nemo/scripts/omp_cpu_trans.py @@ -37,10 +37,9 @@ ''' PSyclone transformation script to insert OpenMP for CPU directives into Nemo code. Tested with ECMWF Nemo 4.0 code. ''' +from psyclone.transformations import OMPLoopTrans from utils import insert_explicit_loop_parallelism, normalise_loops, \ enhance_tree_information, add_profiling -from psyclone.psyGen import TransInfo -from psyclone.transformations import OMPParallelTrans PROFILING_ENABLED = False @@ -56,8 +55,9 @@ def trans(psy): :rtype: :py:class:`psyclone.psyGen.PSy` ''' - omp_parallel_trans = OMPParallelTrans() - omp_loop_trans = TransInfo().get_trans_name('OMPLoopTrans') + omp_parallel_trans = None + omp_loop_trans = OMPLoopTrans(omp_schedule="static") + omp_loop_trans.omp_directive = "paralleldo" print(f"Invokes found in {psy.name}:") for invoke in psy.invokes.invoke_list: @@ -66,33 +66,30 @@ def trans(psy): if PROFILING_ENABLED: add_profiling(invoke.schedule.children) - # TODO #1841: These subroutines have a bug in the array-range-to-loop - # transformation. - if invoke.name in ( - "blk_oce", # NVFORTRAN-S-0083-Vector expression used where - # scalar expression + enhance_tree_information(invoke.schedule) + + if invoke.name in ("eos_rprof"): + # TODO #1959: This subroutines make the ECMWF compilation fail + # because it moves a statement function outside of the + # specification part. + print("Skipping normalisation for ", invoke.name) + + elif invoke.name in ( "trc_oce_rgb", # Produces incorrect results "removepoints" # Compiler error: The shapes of the array # expressions do not conform ): - print("Skipping", invoke.name) - continue - - # TODO #1959: This subroutines make the ECMWF compilation fail because - # it moves a statement function outside of the specification part. - if invoke.name in ("eos_rprof"): - print("Skipping", invoke.name) - continue - - enhance_tree_information(invoke.schedule) - - normalise_loops( - invoke.schedule, - hoist_local_arrays=True, - convert_array_notation=True, - convert_range_loops=True, - hoist_expressions=False - ) + # TODO #1841: These subroutines have a bug in the + # array-range-to-loop transformation. + print("Skipping normalisation for ", invoke.name) + else: + normalise_loops( + invoke.schedule, + hoist_local_arrays=False, + convert_array_notation=True, + convert_range_loops=True, + hoist_expressions=False + ) insert_explicit_loop_parallelism( invoke.schedule, @@ -100,11 +97,6 @@ def trans(psy): loop_directive_trans=omp_loop_trans, # Collapse may be useful in some architecture/compiler collapse=False, - # Currently if there is a call we don't parallelise because we - # expect the subroutine to already be parallelised. lib_fortran - # is the only exception because we know it only has calls to - # functions without internal loops. - exclude_calls=psy.name != "psy_lib_fortran_psy", ) return psy diff --git a/examples/nemo/scripts/omp_gpu_trans.py b/examples/nemo/scripts/omp_gpu_trans.py index c1c44d17f0c..f69b2200b07 100755 --- a/examples/nemo/scripts/omp_gpu_trans.py +++ b/examples/nemo/scripts/omp_gpu_trans.py @@ -37,12 +37,12 @@ ''' PSyclone transformation script showing the introduction of OpenMP for GPU directives into Nemo code. ''' -from utils import insert_explicit_loop_parallelism, normalise_loops, \ - enhance_tree_information, add_profiling from psyclone.psyGen import TransInfo from psyclone.psyir.nodes import Call, Loop from psyclone.psyir.transformations import OMPTargetTrans from psyclone.transformations import OMPDeclareTargetTrans +from utils import insert_explicit_loop_parallelism, normalise_loops, \ + enhance_tree_information, add_profiling PROFILING_ENABLED = True @@ -74,16 +74,24 @@ def trans(psy): print("Skipping", invoke.name) continue - # TODO 1837: Has a TRIM intrinsic that can not be offloaded - if invoke.name in ("cpl_oasis3_cpl_freq"): - print("Skipping", invoke.name) - continue - # TODO #1841: These files have a bug in the array-range-to-loop # transformation. One leads to the following compiler error # NVFORTRAN-S-0083-Vector expression used where scalar expression # required, the other to an incorrect result. - if invoke.name in ("blk_oce", "trc_oce_rgb"): + if invoke.name in ("trc_oce_rgb", ): + print("Skipping", invoke.name) + continue + + # This are functions with scalar bodies, we don't want to parallelise + # them, but we could: + # - Inine them + # - Annotate them with 'omp declare target' and allow to call from gpus + + # TODO 2019: DDPDD in additon has a wp precision symbol that PSyclone + # wrongly considers undeclared + if invoke.name in ("q_sat", "sbc_dcy", "gamma_moist", "cd_neutral_10m", + "psi_h", "psi_m", "DDPDD"): + print("Skipping", invoke.name) continue @@ -108,11 +116,7 @@ def trans(psy): region_directive_trans=omp_target_trans, loop_directive_trans=omp_loop_trans, # Collapse is necessary to give GPUs enough parallel items - collapse=True, - # We exclude loops with calls when parallelising, with the - # exception being lib_fortran where we have marked the - # called functions as GPU-enabled - exclude_calls=psy.name != "psy_lib_fortran_psy", + collapse=True ) return psy diff --git a/examples/nemo/scripts/process_nemo.py b/examples/nemo/scripts/process_nemo.py index 618828fd4b8..3c48a50443e 100755 --- a/examples/nemo/scripts/process_nemo.py +++ b/examples/nemo/scripts/process_nemo.py @@ -84,11 +84,6 @@ "sbccpl.f90", # TODO #1902: Excluded to avoid HoistLocalArraysTrans bug "mpp_ini.f90", - - # TODO # 1142: The following issue only affects the ECMWF Nemo code. - # Currently we have to manually uncomment the line below to process ECMWF - # NEMO but this can be improved resolving the referenced issue. - # "lbclnk.f90", # has mpif.h in the ECMWF version ] @@ -112,6 +107,7 @@ help="Add profiling instrumentation to the " "PROFILE_ONLY list of files. Script-processed " "files are not affected by this argument.") + PARSER.add_argument('-I', dest='include_path') ARGS = PARSER.parse_args() # Check whether the PSyclone command has been specified in an environment @@ -146,13 +142,16 @@ print(f"Processing {file_name}...") extra_args = [] if ARGS.script_file: - extra_args = ["-s", ARGS.script_file] + extra_args += ["-s", ARGS.script_file] + if ARGS.include_path: + extra_args += ["-I", ARGS.include_path] extra_args += ["-oalg", "/dev/null", "-opsy", out_file, ffile] # Since we're in Python we could call psyclone.generator.main() # directly but PSyclone is not designed to be called repeatedly # in that way and doesn't clear up state between invocations. tstart = perf_counter() + print("Executing:" + " ".join(args + extra_args)) rtype = os.system(" ".join(args + extra_args)) tstop = perf_counter() diff --git a/examples/nemo/scripts/utils.py b/examples/nemo/scripts/utils.py index bfb3b5abd82..d60a38e3c00 100755 --- a/examples/nemo/scripts/utils.py +++ b/examples/nemo/scripts/utils.py @@ -36,9 +36,10 @@ ''' Utilities file to parallelise Nemo code. ''' from psyclone.psyir.nodes import Loop, Assignment, Directive, Container, \ - Reference, CodeBlock, Call, Return, IfBlock, Routine + Reference, CodeBlock, Call, Return, IfBlock, Routine, BinaryOperation, \ + IntrinsicCall from psyclone.psyir.symbols import DataSymbol, INTEGER_TYPE, REAL_TYPE, \ - ArrayType + ArrayType, ScalarType, RoutineSymbol, ImportInterface from psyclone.psyir.transformations import HoistLoopBoundExprTrans, \ HoistTrans, ProfileTrans, HoistLocalArraysTrans, Reference2ArrayRangeTrans from psyclone.domain.nemo.transformations import NemoAllArrayRange2LoopTrans @@ -55,9 +56,31 @@ "interp1", "interp2", "interp3", "integ_spline", "sbc_dcy", "sum", "sign_", "ddpdd"] +# From: https://docs.nvidia.com/hpc-sdk/compilers/hpc-compilers-user-guide/ +# index.html#acc-fort-intrin-sum +NVIDIA_GPU_SUPPORTED_INTRINSICS = [ + IntrinsicCall.Intrinsic.SUM, +] + VERBOSE = False +def _it_should_be(symbol, of_type, instance): + ''' Make sure that symbol has the datatype as provided. + + :param symbol: the symbol to check. + :type symbol: :py:class:`psyclone.psyir.symbol.Symbol` + :param type of_type: the datatype type that it must be. + :param instance: the instance of Datatype to assign as the symbol datatype. + :type instance: :py:class:`psyclone.psyir.symbol.DataType` + + ''' + if not isinstance(symbol, DataSymbol): + symbol.specialise(DataSymbol, datatype=instance) + elif not isinstance(symbol.datatype, of_type): + symbol.datatype = instance + + def enhance_tree_information(schedule): ''' Resolve imports in order to populate relevant datatype on the tree symbol tables. @@ -68,44 +91,45 @@ def enhance_tree_information(schedule): mod_sym_tab = schedule.ancestor(Container).symbol_table - if "oce" in mod_sym_tab: - oce_symbol = mod_sym_tab.lookup("oce") - mod_sym_tab.resolve_imports(container_symbols=[oce_symbol]) - - if "par_oce" in mod_sym_tab: - par_oce_symbol = mod_sym_tab.lookup("par_oce") - mod_sym_tab.resolve_imports(container_symbols=[par_oce_symbol]) + modules_to_import = ("oce", "par_oce", "dom_oce", "phycst", "ice", + "obs_fbm", "flo_oce", "sbc_ice", "wet_dry") - if "dom_oce" in mod_sym_tab: - dom_oce_symbol = mod_sym_tab.lookup("dom_oce") - mod_sym_tab.resolve_imports(container_symbols=[dom_oce_symbol]) + for module_name in modules_to_import: + if module_name in mod_sym_tab: + mod_symbol = mod_sym_tab.lookup(module_name) + mod_sym_tab.resolve_imports(container_symbols=[mod_symbol]) - if "phycst" in mod_sym_tab: - phycst_symbol = mod_sym_tab.lookup("phycst") - mod_sym_tab.resolve_imports(container_symbols=[phycst_symbol]) - - if "ice" in mod_sym_tab: - ice_symbol = mod_sym_tab.lookup("ice") - mod_sym_tab.resolve_imports(container_symbols=[ice_symbol]) + are_integers = ('jpi', 'jpim1', 'jpj', 'jpjm1', 'jp_tem', 'jp_sal', + 'jpkm1', 'jpiglo', 'jpni', 'jpk', 'jpiglo_crs', + 'jpmxl_atf', 'jpmxl_ldf', 'jpmxl_zdf', 'jpnij', + 'jpts', 'jpvor_bev', 'nleapy', 'nn_ctls', 'jpmxl_npc', + 'jpmxl_zdfp', 'npti') # Manually set the datatype of some integer and real variables that are # important for performance for reference in schedule.walk(Reference): - if reference.symbol.name in ('jpi', 'jpim1', 'jpj', 'jpjm1', 'jp_tem' - 'jp_sal', 'jpkm1'): - if not isinstance(reference.symbol, DataSymbol): - reference.symbol.specialise(DataSymbol, datatype=INTEGER_TYPE) - elif reference.symbol.name in ('rn_avt_rnf', 'rau0'): - if not isinstance(reference.symbol, DataSymbol): - reference.symbol.specialise(DataSymbol, datatype=REAL_TYPE) - elif reference.symbol.name in ('tmask'): - if not isinstance(reference.symbol, DataSymbol): - reference.symbol.specialise( - DataSymbol, - datatype=ArrayType(REAL_TYPE, [ + if reference.symbol.name in are_integers: + _it_should_be(reference.symbol, ScalarType, INTEGER_TYPE) + elif reference.symbol.name in ('rn_avt_rnf', ): + _it_should_be(reference.symbol, ScalarType, REAL_TYPE) + elif isinstance(reference.symbol.interface, ImportInterface) and \ + reference.symbol.interface.container_symbol.name == "phycst": + # Everything imported from phycst is a REAL + _it_should_be(reference.symbol, ScalarType, REAL_TYPE) + elif reference.symbol.name == 'tmask': + if reference.ancestor(Container).name == "dom_oce": + continue # Do not update the original declaration + _it_should_be(reference.symbol, ArrayType, ArrayType(REAL_TYPE, [ ArrayType.Extent.ATTRIBUTE, ArrayType.Extent.ATTRIBUTE, ArrayType.Extent.ATTRIBUTE])) + elif reference.symbol.name == "sbc_dcy": + # The parser gets this wrong, it is a Call not an Array access + reference.symbol.specialise(RoutineSymbol) + call = Call(reference.symbol) + for child in reference.children: + call.addchild(child.detach()) + reference.replace_with(call) def normalise_loops( @@ -176,7 +200,6 @@ def insert_explicit_loop_parallelism( region_directive_trans=None, loop_directive_trans=None, collapse: bool = True, - exclude_calls: bool = True ): ''' For each loop in the schedule that doesn't already have a Directive as an ancestor, attempt to insert the given region and loop directives. @@ -203,14 +226,71 @@ def insert_explicit_loop_parallelism( if loop.ancestor(Directive): continue # Skip if an outer loop is already parallelised - if exclude_calls and loop.walk((Call, CodeBlock)): + opts = {} + + routine_name = loop.ancestor(Routine).invoke.name + + if ('dyn_spg' in routine_name and len(loop.walk(Loop)) > 2): + print("Loop not parallelised because its in 'dyn_spg' and " + "its not the inner loop") continue + # Skip if it is an array operation loop on an ice routine if along the + # third dim or higher or if the loop nests a loop over ice points + # (npti) or if the loop and array dims do not match. + # In addition, they often nest ice linearised loops (npti) + # which we'd rather parallelise + if ('ice' in routine_name + and isinstance(loop.stop_expr, BinaryOperation) + and (loop.stop_expr.operator == BinaryOperation.Operator.UBOUND or + loop.stop_expr.operator == BinaryOperation.Operator.SIZE) + and (len(loop.walk(Loop)) > 2 + or any([ref.symbol.name in ('npti',) + for lp in loop.loop_body.walk(Loop) + for ref in lp.stop_expr.walk(Reference)]) + or (str(len(loop.walk(Loop))) != + loop.stop_expr.children[1].value))): + print("ICE Loop not parallelised for performance reasons") + continue + # Skip if looping over ice categories, ice or snow layers + # as these have only 5, 4, and 1 iterations, respectively + if (any([ref.symbol.name in ('jpl', 'nlay_i', 'nlay_s') + for ref in loop.stop_expr.walk(Reference)])): + print("Loop not parallelised because stops at 'jpl', 'nlay_i' " + "or 'nlay_s'.") + continue + + def skip_for_correctness(): + for call in loop.walk(Call): + if not isinstance(call, IntrinsicCall): + print(f"Loop not parallelised because it has a call to " + f"{call.routine.name}") + return True + if call.intrinsic not in NVIDIA_GPU_SUPPORTED_INTRINSICS: + print(f"Loop not parallelised because it has a " + f"{call.intrinsic.name}") + return True + if loop.walk(CodeBlock): + print(f"Loop not parallelised because it has a CodeBlock") + return True + return False + + # If we see one such ice linearised loop, we assume + # calls/codeblocks are not a problem (they are not) + if not any([ref.symbol.name in ('npti',) + for ref in loop.stop_expr.walk(Reference)]): + if skip_for_correctness(): + continue + + # pnd_lev requires manual privatisation of ztmp + if any([name in routine_name for name in ('tab_', 'pnd_')]): + opts = {"force": True} try: - loop_directive_trans.apply(loop) + loop_directive_trans.apply(loop, options=opts) # Only add the region directive if the loop was successfully # parallelised. - region_directive_trans.apply(loop.parent.parent) + if region_directive_trans: + region_directive_trans.apply(loop.parent.parent) except TransformationError as err: # This loop can not be transformed, proceed to next loop print("Loop not parallelised because:", str(err)) diff --git a/psyclone.pdf b/psyclone.pdf index b7a6e1898c2..154d485079b 100644 Binary files a/psyclone.pdf and b/psyclone.pdf differ diff --git a/src/psyclone/domain/lfric/algorithm/lfric_alg.py b/src/psyclone/domain/lfric/algorithm/lfric_alg.py index 4cca91f5eb3..f29a5bf1750 100644 --- a/src/psyclone/domain/lfric/algorithm/lfric_alg.py +++ b/src/psyclone/domain/lfric/algorithm/lfric_alg.py @@ -61,11 +61,6 @@ class LFRicAlg: layer from Kernel metadata. ''' - - #: The order of the finite-element scheme that will be used by any - #: generated algorithm layer. - _ELEMENT_ORDER = "1" - def create_from_kernel(self, name, kernel_path): ''' Generates LFRic algorithm PSyIR that calls the supplied kernel through @@ -240,9 +235,10 @@ def create_alg_routine(name): def _create_function_spaces(self, prog, fspaces): ''' - Adds PSyIR to the supplied Routine that declares and intialises the - specified function spaces. The order of these spaces is set by the - `_ELEMENT_ORDER` class constant. + Adds PSyIR to the supplied Routine that declares and intialises + the specified function spaces. The order of these spaces is + set by the element_order variable which is provided by the + LFRic finite_element_config_mod module. :param prog: the routine to which to add declarations and \ initialisation. @@ -252,20 +248,19 @@ def _create_function_spaces(self, prog, fspaces): :raises InternalError: if a function space is supplied that is not a \ recognised LFRic function space. + ''' table = prog.symbol_table reader = FortranReader() # The order of the finite-element scheme. - table.add_lfric_precision_symbol("i_def") - data_type_class = LFRicTypes("LFRicIntegerScalarDataType") - order = table.new_symbol("element_order", tag="element_order", - symbol_type=DataSymbol, - datatype=data_type_class(), - constant_value=Literal( - self._ELEMENT_ORDER, - data_type_class())) + fe_config_mod = table.new_symbol( + "finite_element_config_mod", symbol_type=ContainerSymbol) + order = table.new_symbol( + "element_order", tag="element_order", + symbol_type=DataSymbol, datatype=DeferredType(), + interface=ImportInterface(fe_config_mod)) fs_cont_mod = table.new_symbol("fs_continuity_mod", symbol_type=ContainerSymbol) diff --git a/src/psyclone/domain/lfric/arg_index_to_metadata_index.py b/src/psyclone/domain/lfric/arg_index_to_metadata_index.py index d9d03dc525c..5ab598d0f8f 100644 --- a/src/psyclone/domain/lfric/arg_index_to_metadata_index.py +++ b/src/psyclone/domain/lfric/arg_index_to_metadata_index.py @@ -114,6 +114,7 @@ def _operator(cls, meta_arg): :py:class:`psyclone.domain.lfric.kernel.OperatorArgMetadata` ''' + cls._index += 1 # ncell_3d cls._add_arg(meta_arg) @classmethod @@ -127,6 +128,10 @@ def _cma_operator(cls, meta_arg): ''' cls._add_arg(meta_arg) + cls._index += 1 # nrow + if meta_arg.function_space_to != meta_arg.function_space_from: + cls._index += 1 # ncol + cls._index += 5 # bandwidth, alpha, beta, gamma_m, gamma_p @classmethod def _add_arg(cls, meta_arg): diff --git a/src/psyclone/domain/nemo/transformations/nemo_allarrayrange2loop_trans.py b/src/psyclone/domain/nemo/transformations/nemo_allarrayrange2loop_trans.py index 51d2172e107..bcab120f9e8 100644 --- a/src/psyclone/domain/nemo/transformations/nemo_allarrayrange2loop_trans.py +++ b/src/psyclone/domain/nemo/transformations/nemo_allarrayrange2loop_trans.py @@ -98,10 +98,14 @@ def apply(self, node, options=None): :param node: an Assignment node. :type node: :py:class:`psyclone.psyir.nodes.Assignment` - :param options: a dictionary with options for \ - transformations. No options are used in this \ - transformation. This is an optional argument that defaults \ - to None. + :param options: a dictionary with options for transformations. No + options are used in this transformation. This is an optional + argument that defaults to None. + :param bool options["verbose"]: whether to print out the reason + why the inner transformation was not applied. This is useful + because this transfomation succeeds even if one of the inner + transformations fails, and therefor the reason why the inner + transformation failed is not propagated. :type options: Optional[Dict[str, Any]] ''' @@ -111,8 +115,17 @@ def apply(self, node, options=None): try: while True: trans.apply(node) - except TransformationError: - pass + except TransformationError as err: + # TODO #11: Instead we could use proper logging + if options and options.get("verbose", False): + errmsg = str(err) + # Skip the errors that are obious and are generated for any + # statement that is not an array expression + if "assignment node should be an expression with an array " \ + "that has a Range node" not in errmsg and \ + "be a Reference that contains an array access somewhere" \ + not in errmsg: + print(errmsg) def __str__(self): return ("Convert all array ranges in a PSyIR assignment into " diff --git a/src/psyclone/generator.py b/src/psyclone/generator.py index 0bfd4c6e8ac..c3d2aafc39d 100644 --- a/src/psyclone/generator.py +++ b/src/psyclone/generator.py @@ -73,6 +73,7 @@ from psyclone.psyGen import PSyFactory from psyclone.psyir.backend.fortran import FortranWriter from psyclone.psyir.frontend.fortran import FortranReader +from psyclone.psyir.frontend.fparser2 import Fparser2Reader from psyclone.psyir.nodes import Loop, Container, Routine from psyclone.psyir.transformations import TransformationError from psyclone.version import __VERSION__ @@ -275,12 +276,15 @@ def generate(filename, api="", kernel_paths=None, script_name=None, # Create language-level PSyIR from the Algorithm file reader = FortranReader() if api == "dynamo0.3": - # avoid undeclared builtin errors in PSyIR by adding "use - # builtins". TODO issue #1618. This symbol needs to be - # removed when lowering. + # avoid undeclared builtin errors in PSyIR by adding a + # wildcard use statement. fp2_tree = parse_fp2(filename) - add_builtins_use(fp2_tree) - psyir = reader.psyir_from_source(str(fp2_tree)) + # Choose a module name that is invalid Fortran so that it + # does not clash with any existing names in the algorithm + # layer. + builtins_module_name = "_psyclone_builtins" + add_builtins_use(fp2_tree, builtins_module_name) + psyir = Fparser2Reader().generate_psyir(fp2_tree) # Check that there is only one module/program per file. check_psyir(psyir, filename) else: @@ -349,6 +353,15 @@ def generate(filename, api="", kernel_paths=None, script_name=None, for invoke in psyir.walk(AlgorithmInvokeCall): invoke_trans.apply( invoke, options={"kernels": kernels[id(invoke)]}) + if api == "dynamo0.3": + # Remove any use statements that were temporarily added to + # avoid the PSyIR complaining about undeclared builtin + # names. + for node in psyir.walk((Routine, Container)): + symbol_table = node.symbol_table + if builtins_module_name in symbol_table: + symbol = symbol_table.lookup(builtins_module_name) + symbol_table.remove(symbol) # Create Fortran from Algorithm PSyIR writer = FortranWriter() @@ -581,23 +594,36 @@ def check_psyir(psyir, filename): f"found '{type(psyir.children[0]).__name__}'.") -def add_builtins_use(fp2_tree): - '''Modify the fparser2 tree adding a 'use builtins' so that builtin kernel +def add_builtins_use(fp2_tree, name): + '''Modify the fparser2 tree adding a 'use ' so that builtin kernel functors do not appear to be undeclared. :param fp2_tree: the fparser2 tree to modify. :type fp2_tree: py:class:`fparser.two.Program` + :param str name: the name of the module imported by the use + statement. ''' for node in fp2_tree.children: if isinstance(node, (Fortran2003.Module, Fortran2003.Main_Program)): - # add "use builtins" to the module or program + # add "use " to the module or program if not isinstance( node.children[1], Fortran2003.Specification_Part): - fp2_reader = get_reader("use builtins") - node.children.insert( - 1, Fortran2003.Specification_Part(fp2_reader)) + # Create a valid use statement then modify its name as + # the supplied name may be invalid Fortran to avoid + # clashes with existing Fortran names. + fp2_reader = get_reader("use dummy") + spec_part = Fortran2003.Specification_Part(fp2_reader) + use_stmt = spec_part.children[0] + use_name = use_stmt.children[2] + use_name.string = name + node.children.insert(1, spec_part) else: spec_part = node.children[1] - spec_part.children.insert( - 0, Fortran2003.Use_Stmt("use builtins")) + # Create a valid use statement then modify its name as + # the supplied name may be invalid Fortran to avoid + # clashes with existing Fortran names. + use_stmt = Fortran2003.Use_Stmt("use dummy") + use_name = use_stmt.children[2] + use_name.string = name + spec_part.children.insert(0, use_stmt) diff --git a/src/psyclone/psyGen.py b/src/psyclone/psyGen.py index 62a96d797b2..5017ab58966 100644 --- a/src/psyclone/psyGen.py +++ b/src/psyclone/psyGen.py @@ -51,12 +51,10 @@ DeclGen, DeallocateGen, DoGen, UseGen) from psyclone.parse.algorithm import BuiltInCall from psyclone.psyir.backend.fortran import FortranWriter -from psyclone.psyir.backend.visitor import PSyIRVisitor from psyclone.psyir.nodes import (Node, Schedule, Loop, Statement, Container, Routine, Call, OMPDoDirective) from psyclone.psyir.symbols import (ArrayType, DataSymbol, RoutineSymbol, Symbol, ContainerSymbol, ImportInterface, - ContainerSymbol, ImportInterface, ArgumentInterface, DeferredType) from psyclone.psyir.symbols.datatypes import UnknownFortranType @@ -1741,9 +1739,13 @@ def _rename_psyir(self, suffix): container_table = container.symbol_table for sym in container_table.datatypesymbols: if isinstance(sym.datatype, UnknownFortranType): - orig_declaration = sym.datatype.declaration - sym.datatype.declaration = orig_declaration.replace( + new_declaration = sym.datatype.declaration.replace( orig_kern_name, new_kern_name) + # pylint: disable=protected-access + sym._datatype = UnknownFortranType( + new_declaration, + partial_datatype=sym.datatype.partial_datatype) + # pylint: enable=protected-access @property def modified(self): diff --git a/src/psyclone/psyad/domain/lfric/lfric_adjoint_harness.py b/src/psyclone/psyad/domain/lfric/lfric_adjoint_harness.py index 08bcc65e4cc..fff398c5f2d 100644 --- a/src/psyclone/psyad/domain/lfric/lfric_adjoint_harness.py +++ b/src/psyclone/psyad/domain/lfric/lfric_adjoint_harness.py @@ -37,12 +37,14 @@ ''' Provides LFRic-specific PSyclone adjoint test-harness functionality. ''' from fparser import api as fpapi -from psyclone.core import AccessType -from psyclone.domain.lfric import LFRicConstants, LFRicTypes +from psyclone.core import AccessType +from psyclone.domain.lfric import ( + LFRicConstants, LFRicTypes, ArgIndexToMetadataIndex) +from psyclone.domain.lfric.algorithm.lfric_alg import LFRicAlg from psyclone.domain.lfric.algorithm.psyir import ( LFRicAlgorithmInvokeCall, LFRicBuiltinFunctorFactory, LFRicKernelFunctor) -from psyclone.domain.lfric.algorithm.lfric_alg import LFRicAlg +from psyclone.domain.lfric.transformations import RaisePSyIR2LFRicKernTrans from psyclone.errors import InternalError, GenerationError from psyclone.psyad.domain.common.adjoint_utils import (create_adjoint_name, create_real_comparison, @@ -467,6 +469,10 @@ def generate_lfric_adjoint_harness(tl_psyir, coord_arg_idx=None, routine = container.walk(Routine)[0] table = routine.symbol_table + tl_subroutine = tl_container.children[0] + tl_subroutine_table = tl_subroutine.symbol_table + tl_argument_list = tl_subroutine_table.argument_list + # Parse the kernel metadata. This still uses fparser1 as that's what # the meta-data handling is currently based upon. We therefore have to # convert back from PSyIR to Fortran for the moment. @@ -509,6 +515,33 @@ def generate_lfric_adjoint_harness(tl_psyir, coord_arg_idx=None, # TODO #1806 - once we have the new PSyIR-based metadata handling then # we can pass PSyIR to this routine rather than an fparser1 parse tree. kern = lfalg.kernel_from_metadata(parse_tree, kernel_name) + + # Replace generic names for fields. operators etc generated in + # DynKern with the scientific names used by the tangent-linear + # kernel. This makes the harness code more readable. Changing the + # names in-place within DynKern is the neatest solution given that + # this is a legacy structure. + + # First raise the tangent-linear kernel PSyIR to LFRic PSyIR. This + # gives us access to the kernel metadata. + kern_trans = RaisePSyIR2LFRicKernTrans() + kern_trans.apply(tl_psyir, options={"metadata_name": kernel_name}) + metadata = tl_psyir.children[0].metadata + # Use the metadata to determine the mapping from a metadata + # meta_arg index to the kernel argument index. Note, the meta_arg + # index corresponds to the order of the arguments stored in + # DynKern. + index_map = ArgIndexToMetadataIndex.mapping(metadata) + inv_index_map = {value: key for key, value in index_map.items()} + + # For each kernel argument, replace the generic name with the + # scientific name used in the tangent-linear code. + for idx, arg in enumerate(kern.arguments.args): + tl_arg_idx = inv_index_map[idx] + # pylint: disable=protected-access + arg._name = tl_argument_list[tl_arg_idx].name + # pylint: enable=protected-access + kern_args = lfalg.construct_kernel_args(routine, kern) # Validate the index values for the coordinate and face_id fields if @@ -565,8 +598,9 @@ def generate_lfric_adjoint_harness(tl_psyir, coord_arg_idx=None, # This kernel argument is not modified by the test harness so we # don't need to keep a copy of it. continue - input_sym = table.new_symbol(sym.name+"_input", symbol_type=DataSymbol, - datatype=DeferredType()) + input_sym = table.new_symbol( + f"{sym.name}_input", symbol_type=DataSymbol, + datatype=DeferredType()) input_sym.copy_properties(sym) input_symbols[sym.name] = input_sym @@ -598,6 +632,7 @@ def generate_lfric_adjoint_harness(tl_psyir, coord_arg_idx=None, # Fields. kernel_list = _init_fields_random(kernel_input_arg_list, input_symbols, table) + # Operators. kernel_list.extend(_init_operators_random( [sym for sym, _, _ in kern_args.operators], table)) diff --git a/src/psyclone/psyir/backend/fortran.py b/src/psyclone/psyir/backend/fortran.py index 3c068dda02f..3947ec3e2d2 100644 --- a/src/psyclone/psyir/backend/fortran.py +++ b/src/psyclone/psyir/backend/fortran.py @@ -48,7 +48,8 @@ from psyclone.psyir.frontend.fparser2 import Fparser2Reader, \ TYPE_MAP_FROM_FORTRAN from psyclone.psyir.nodes import BinaryOperation, Call, CodeBlock, DataNode, \ - IntrinsicCall, Literal, Operation, Range, Routine, Schedule, UnaryOperation + IntrinsicCall, Literal, Operation, Range, Routine, Schedule, \ + UnaryOperation from psyclone.psyir.symbols import ( ArgumentInterface, ArrayType, ContainerSymbol, DataSymbol, DataTypeSymbol, DeferredType, RoutineSymbol, ScalarType, Symbol, IntrinsicSymbol, @@ -215,7 +216,7 @@ def precedence(fortran_operator): # then it should be respected. These issues are dealt with in the # binaryoperation handler. fortran_precedence = [ - ['.EQV.', 'NEQV'], + ['.EQV.', '.NEQV.'], ['.OR.'], ['.AND.'], ['.NOT.'], @@ -371,7 +372,7 @@ def __init__(self, skip_nodes=False, indent_string=" ", # also uses this Fortran backend. # pylint: disable=import-outside-toplevel from psyclone.psyir.tools import DependencyTools - self._dep_tools = DependencyTools(language_writer=self) + self._dep_tools = DependencyTools() @staticmethod def _reverse_map(reverse_dict, op_map): diff --git a/src/psyclone/psyir/frontend/fparser2.py b/src/psyclone/psyir/frontend/fparser2.py index 7cf5c8eae6d..ad668cc289a 100644 --- a/src/psyclone/psyir/frontend/fparser2.py +++ b/src/psyclone/psyir/frontend/fparser2.py @@ -33,6 +33,7 @@ # Authors: R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab # J. Henrichs, Bureau of Meteorology # I. Kavcic, Met Office +# Modified: A. B. G. Chalk, STFC Daresbury Lab # ----------------------------------------------------------------------------- ''' This module provides the fparser2 to PSyIR front-end, it follows a @@ -303,7 +304,6 @@ def _find_or_create_imported_symbol(location, name, scope_limit=None, if hasattr(test_node, 'symbol_table'): # This Node does have a SymbolTable. symbol_table = test_node.symbol_table - try: # If the name matches a Symbol in this SymbolTable then # return the Symbol (after specialising it, if necessary). @@ -1012,8 +1012,10 @@ class Fparser2Reader(): ('**', BinaryOperation.Operator.POW), ('==', BinaryOperation.Operator.EQ), ('.eq.', BinaryOperation.Operator.EQ), + ('.eqv.', BinaryOperation.Operator.EQV), ('/=', BinaryOperation.Operator.NE), ('.ne.', BinaryOperation.Operator.NE), + ('.neqv.', BinaryOperation.Operator.NEQV), ('<=', BinaryOperation.Operator.LE), ('.le.', BinaryOperation.Operator.LE), ('<', BinaryOperation.Operator.LT), @@ -2090,6 +2092,78 @@ def _process_derived_type_decln(self, parent, decl, visibility_map): tsymbol.datatype = UnknownFortranType(str(decl)) tsymbol.interface = UnknownInterface() + def _get_partial_datatype(self, node, scope, visibility_map): + '''Try to obtain partial datatype information from node by removing + any unsupported properties in the declaration. + + :param node: fparser2 node containing the declaration statement. + :type node: :py:class:`fparser.two.Fortran2008.Type_Declaration_Stmt` + or :py:class:`fparser.two.Fortran2003.Type_Declaration_Stmt` + :param scope: PSyIR node in which to insert the symbols found. + :type scope: :py:class:`psyclone.psyir.nodes.ScopingNode` + :param visibility_map: mapping of symbol names to explicit + visibilities. + :type visibility_map: dict with str keys and values of type + :py:class:`psyclone.psyir.symbols.Symbol.Visibility` + + :returns: a PSyIR datatype, or datatype symbol, containing + partial datatype information for the declaration statement + in the cases where it is possible to extract this + information and None otherwise. + :rtype: Optional[:py:class:`psyclone.psyir.symbols.DataType` or + :py:class:`psyclone.psyir.symbols.DataTypeSymbol`] + + ''' + # 1: Remove any initialisation and additional variables. TODO: + # This won't be needed when #1419 is implemented (assuming the + # implementation supports both assignments and pointer + # assignments). + entity_decl_list = node.children[2] + orig_entity_decl_list = list(entity_decl_list.children[:]) + entity_decl_list.items = tuple(entity_decl_list.children[0:1]) + entity_decl = entity_decl_list.children[0] + orig_entity_decl_children = list(entity_decl.children[:]) + if isinstance(entity_decl.children[3], Fortran2003.Initialization): + entity_decl.items = ( + entity_decl.items[0], entity_decl.items[1], + entity_decl.items[2], None) + + # 2: Remove any unsupported attributes + unsupported_attribute_names = ["pointer", "target"] + attr_spec_list = node.children[1] + orig_node_children = list(node.children[:]) + orig_attr_spec_list_children = (list(node.children[1].children[:]) + if attr_spec_list else None) + if attr_spec_list: + entry_list = [] + for attr_spec in attr_spec_list.children: + if str(attr_spec).lower() not in unsupported_attribute_names: + entry_list.append(attr_spec) + if not entry_list: + node.items = (node.items[0], None, node.items[2]) + else: + node.items[1].items = tuple(entry_list) + + # Try to parse the modified node. + symbol_table = SymbolTable() + try: + self._process_decln(scope, symbol_table, node, + visibility_map) + symbol_name = node.children[2].children[0].children[0].string + symbol_name = symbol_name.lower() + datatype = symbol_table.lookup(symbol_name).datatype + except NotImplementedError: + datatype = None + + # Restore the fparser2 parse tree + node.items = tuple(orig_node_children) + if node.children[1]: + node.children[1].items = tuple(orig_attr_spec_list_children) + node.children[2].items = tuple(orig_entity_decl_list) + node.children[2].children[0].items = tuple(orig_entity_decl_children) + + return datatype + def process_declarations(self, parent, nodes, arg_list, visibility_map=None): ''' @@ -2107,19 +2181,19 @@ def process_declarations(self, parent, nodes, arg_list, :param arg_list: fparser2 AST node containing the argument list. :type arg_list: :py:class:`fparser.Fortran2003.Dummy_Arg_List` :param visibility_map: mapping of symbol names to explicit - visibilities. - :type visibility_map: dict with str keys and values of type \ - :py:class:`psyclone.psyir.symbols.Symbol.Visibility` + visibilities. + :type visibility_map: dict with str keys and values of type + :py:class:`psyclone.psyir.symbols.Symbol.Visibility` :raises GenerationError: if an INCLUDE statement is encountered. - :raises NotImplementedError: the provided declarations contain \ - attributes which are not supported yet. - :raises GenerationError: if the parse tree for a USE statement does \ - not have the expected structure. - :raises SymbolError: if a declaration is found for a Symbol that is \ - already in the symbol table with a defined interface. - :raises InternalError: if the provided declaration is an unexpected \ - or invalid fparser or Fortran expression. + :raises NotImplementedError: the provided declarations contain + attributes which are not supported yet. + :raises GenerationError: if the parse tree for a USE statement does + not have the expected structure. + :raises SymbolError: if a declaration is found for a Symbol that is + already in the symbol table with a defined interface. + :raises InternalError: if the provided declaration is an unexpected + or invalid fparser or Fortran expression. ''' if visibility_map is None: @@ -2239,16 +2313,23 @@ def process_declarations(self, parent, nodes, arg_list, except KeyError: pass + # Try to extract partial datatype information. + datatype = self._get_partial_datatype( + node, parent, visibility_map) + # If a declaration declares multiple entities, it's # possible that some may have already been processed # successfully and thus be in the symbol table. try: parent.symbol_table.add( - DataSymbol(symbol_name, - UnknownFortranType(str(node)), - interface=UnknownInterface(), - visibility=vis), + DataSymbol( + symbol_name, UnknownFortranType( + str(node), + partial_datatype=datatype), + interface=UnknownInterface(), + visibility=vis), tag=tag) + except KeyError as err: if len(orig_children) == 1: raise SymbolError( @@ -2529,7 +2610,6 @@ def process_nodes(self, parent, nodes): ''' code_block_nodes = [] for child in nodes: - try: psy_child = self._create_child(child, parent) except NotImplementedError: @@ -2827,7 +2907,7 @@ def _do_construct_handler(self, node, parent): if construct_name in [name.string for name in names]: raise NotImplementedError() - ctrl = walk(node.content, Fortran2003.Loop_Control) + ctrl = walk(nonlabel_do, Fortran2003.Loop_Control) # do loops with no condition and do while loops if not ctrl or ctrl[0].items[0]: annotation = ['was_unconditional'] if not ctrl else None @@ -3202,6 +3282,10 @@ def _process_case_value(self, selector, node, parent): :param parent: parent node in the PSyIR. :type parent: :py:class:`psyclone.psyir.nodes.Node` + :raises NotImplementedError: If the operator for an equality cannot be + determined (i.e. the statement cannot be + determined to be a logical comparison + or not) ''' if isinstance(node, Fortran2003.Case_Value_Range): # The case value is a range (e.g. lim1:lim2) @@ -3230,12 +3314,54 @@ def _process_case_value(self, selector, node, parent): self.process_nodes(parent=leop, nodes=[node.items[1]]) new_parent.addchild(leop) else: - # The case value is some scalar initialisation expression + # The case value is some scalar expression bop = BinaryOperation(BinaryOperation.Operator.EQ, parent=parent) - parent.addchild(bop) self.process_nodes(parent=bop, nodes=[selector]) self.process_nodes(parent=bop, nodes=[node]) + # TODO #1799 when generic child.datatype is supported we can + # remove the conditional inside the loop and support full + # expressions + + # Keep track of whether we know if the operator should be EQ/EQV + operator_known = False + for child in bop.children: + if (isinstance(child, Literal) and + isinstance(child.datatype, ScalarType)): + # We know the operator for all literals + operator_known = True + if (child.datatype.intrinsic == + ScalarType.Intrinsic.BOOLEAN): + rhs = bop.children[1].detach() + lhs = bop.children[0].detach() + bop = BinaryOperation(BinaryOperation.Operator.EQV, + parent=parent) + bop.addchild(lhs) + bop.addchild(rhs) + elif (isinstance(child, Reference) and + isinstance(child.symbol, DataSymbol) and + not isinstance(child.symbol.datatype, + UnknownFortranType)): + # We know the operator for all known reference types. + operator_known = True + if (child.symbol.datatype.intrinsic == + ScalarType.Intrinsic.BOOLEAN): + rhs = bop.children[1].detach() + lhs = bop.children[0].detach() + bop = BinaryOperation(BinaryOperation.Operator.EQV, + parent=parent) + bop.addchild(lhs) + bop.addchild(rhs) + + if operator_known: + parent.addchild(bop) + else: + raise NotImplementedError(f"PSyclone can't determine if this " + f"case should be '==' or '.EQV.' " + f"because it can't figure out if " + f"{bop.children[0].debug_string} " + f"or {bop.children[1].debug_string}" + f" are logical expressions.") @staticmethod def _array_notation_rank(node): @@ -4364,10 +4490,22 @@ def _subroutine_handler(self, node, parent): :returns: PSyIR representation of node. :rtype: :py:class:`psyclone.psyir.nodes.Routine` + + :raises NotImplementedError: if the node contains a Contains clause. :raises NotImplementedError: if an unsupported prefix is found or no \ explicit type information is available for a Function. ''' + try: + _first_type_match(node.children, + Fortran2003.Internal_Subprogram_Part) + has_contains = True + except ValueError: + has_contains = False + if has_contains: + raise NotImplementedError("PSyclone doesn't yet support 'Contains'" + " inside a Subroutine or Function") + name = node.children[0].children[1].string routine = Routine(name, parent=parent) @@ -4407,7 +4545,7 @@ def _subroutine_handler(self, node, parent): raise NotImplementedError( f"Routine has unsupported prefix: {child.string}") else: - base_type, _ = self._process_type_spec(parent, child) + base_type, _ = self._process_type_spec(routine, child) if isinstance(node, Fortran2003.Function_Subprogram): # Check whether this function-stmt has a suffix containing diff --git a/src/psyclone/psyir/nodes/node.py b/src/psyclone/psyir/nodes/node.py index 7084373ef0d..b138f87e4cd 100644 --- a/src/psyclone/psyir/nodes/node.py +++ b/src/psyclone/psyir/nodes/node.py @@ -1059,7 +1059,7 @@ class or a tuple of classes) is found. This can be used to avoid return local_list def ancestor(self, my_type, excluding=None, include_self=False, - limit=None): + limit=None, shared_with=None): ''' Search back up the tree and check whether this node has an ancestor that is an instance of the supplied type. If it does then we return @@ -1068,6 +1068,9 @@ def ancestor(self, my_type, excluding=None, include_self=False, `include_self` is True then the current node is included in the search. If `limit` is provided then the search ceases if/when the supplied node is encountered. + If `shared_with` is provided, then the ancestor search will find an + ancestor of both this node and the node provided as `shared_with` if + such an ancestor exists. :param my_type: class(es) to search for. :type my_type: type | Tuple[type, ...] @@ -1077,6 +1080,9 @@ def ancestor(self, my_type, excluding=None, include_self=False, search. :param limit: an optional node at which to stop the search. :type limit: Optional[:py:class:`psyclone.psyir.nodes.Node`] + :param shared_with: an optional node which must also have the + found node as an ancestor. + :type shared_with: Optional[:py:class:`psyclone.psyir.nodes.Node`] :returns: First ancestor Node that is an instance of any of the \ requested classes or None if not found. @@ -1107,15 +1113,55 @@ def ancestor(self, my_type, excluding=None, include_self=False, f"The 'limit' argument to ancestor() must be an instance of " f"Node but got '{type(limit).__name__}'") + # If we need to find a shared ancestor, then find a starting ancestor + # for the sharing node. + shared_ancestor = None + if shared_with is not None: + shared_ancestor = shared_with.ancestor( + my_type, excluding=excluding, + include_self=include_self, limit=limit) + while myparent is not None: if isinstance(myparent, my_type): if not (excluding and isinstance(myparent, excludes)): + # If this is a valid instance but not the same as for + # the shared_with node, we do logic afterwards to continue + # searching upwards, as we could be either higher or + # lower than the shared_ancestor found previously. + if shared_ancestor and shared_ancestor is not myparent: + break # This parent node is not an instance of an excluded # sub-class so return it return myparent if myparent is limit: break myparent = myparent.parent + + # We search up the tree until we find an ancestor of the requested + # type(s) shared by the shared_with node. + while (myparent is not shared_ancestor and myparent and + shared_ancestor): + if myparent is limit: + break + if myparent.depth > shared_ancestor.depth: + # If myparent is deeper in the tree than the current + # potential shared ancestor, search upwards to find + # the next valid ancestor of this node. + myparent = myparent.ancestor( + my_type, excluding=excluding, + include_self=False, limit=limit) + else: + # shared_ancestor is equal or deeper in the tree than + # myparent, so search upwards for the next valid ancestor + # of shared_ancestor. + shared_ancestor = shared_ancestor.ancestor( + my_type, excluding=excluding, include_self=False, + limit=limit) + # If myparent is shared ancestor then return myparent. + if myparent is shared_ancestor: + return myparent + + # Otherwise we didn't find an ancestor that was valid. return None def kernels(self): diff --git a/src/psyclone/psyir/nodes/omp_directives.py b/src/psyclone/psyir/nodes/omp_directives.py index 0ed2ff1bb76..377c36a9fdc 100644 --- a/src/psyclone/psyir/nodes/omp_directives.py +++ b/src/psyclone/psyir/nodes/omp_directives.py @@ -494,7 +494,16 @@ def private_clause(self): def gen_code(self, parent): '''Generate the fortran OMP Parallel Directive and any associated - code''' + code. + + :param parent: the node in the generated AST to which to add content. + :type parent: :py:class:`psyclone.f2pygen.BaseGen` + + :raises GenerationError: if the OpenMP directive needs some + synchronisation mechanism to create valid code. These are not + implemented yet. + + ''' # pylint: disable=import-outside-toplevel from psyclone.psyGen import zero_reduction_variables @@ -507,8 +516,17 @@ def gen_code(self, parent): # this almost certainly indicates a user error. self._encloses_omp_directive() - # Update/generate the private clause if the code has changed. - private_clause, fprivate_clause = self._get_private_clauses() + # Generate the private and firstprivate clauses + private, fprivate, need_sync = self._infer_sharing_attributes() + private_clause = OMPPrivateClause.create( + sorted(private, key=lambda x: x.name)) + fprivate_clause = OMPFirstprivateClause.create( + sorted(fprivate, key=lambda x: x.name)) + if need_sync: + raise GenerationError( + f"OMPParallelDirective.gen_code() does not support symbols " + f"that need synchronisation, but found: " + f"{[x.name for x in need_sync]}") reprod_red_call_list = self.reductions(reprod=True) if reprod_red_call_list: @@ -591,6 +609,9 @@ def lower_to_language_level(self): :returns: the lowered version of this node. :rtype: :py:class:`psyclone.psyir.node.Node` + :raises GenerationError: if the OpenMP directive needs some + synchronisation mechanism to create valid code. These are not + implemented yet. ''' # Keep the first two children and compute the rest using the current # state of the node/tree (lowering it first in case new symbols are @@ -598,7 +619,20 @@ def lower_to_language_level(self): self._children = self._children[:2] for child in self.children: child.lower_to_language_level() - private_clause, fprivate_clause = self._get_private_clauses() + + # Create data sharing clauses (order alphabetically to make generation + # reproducible) + private, fprivate, need_sync = self._infer_sharing_attributes() + private_clause = OMPPrivateClause.create( + sorted(private, key=lambda x: x.name)) + fprivate_clause = OMPFirstprivateClause.create( + sorted(fprivate, key=lambda x: x.name)) + if need_sync: + raise GenerationError( + f"Lowering {type(self).__name__} does not support symbols that" + f" need synchronisation, but found: " + f"{[x.name for x in need_sync]}") + self.addchild(private_clause) self.addchild(fprivate_clause) return self @@ -630,21 +664,40 @@ def end_string(self): ''' return "omp end parallel" - def _get_private_clauses(self): - ''' - Returns the OpenMP clauses with the symbols of the variable names that - should be marked as PRIVATE and FIRSTPRIVATE in this parallel region. - - :returns: a PRIVATE and a FIRSTPRIVATE clause containing the \ - variables that need to be marked private for this directive. - :rtype: Tuple[ \ - :py:class:`psyclone.psyir.nodes.omp_clauses.OMPPrivateClause`, - :py:class:`psyclone.psyir.nodes.omp_clauses.OMPFirstprivateClause`] + def _infer_sharing_attributes(self): + ''' + The PSyIR does not specify if each symbol inside an OpenMP region is + private, firstprivate, shared or shared but needs synchronisation, + the attributes are infered looking at the usage of each symbol inside + the parallel region. + + This method analyses the directive body and automatically classifies + each symbol using the following rules: + - All arrays are shared. + - Scalars that are accessed only once are shared. + - Scalars that are read-only or written outside a loop are shared. + - Scalars written in multiple iterations of a loop are private, unless: + * there is a write-after-read dependency in a loop iteration, + in this case they are shared but need synchronisation; + * they are read before in the same parallel region (but not inside + the same loop iteration), in this case they are firstprivate. + * they are only conditionally written in some iterations; + in this case they are firstprivate. + + This method returns the sets of private, firstprivate, and shared but + needing synchronisation symbols, all symbols not in these sets are + assumed shared. How to synchronise the symbols in the third set is + up to the caller of this method. + + :returns: three set of symbols that classify each of the symbols in \ + the directive body as PRIVATE, FIRSTPRIVATE or SHARED NEEDING \ + SYNCHRONISATION. + :rtype: Tuple[Set(:py:class:`psyclone.psyir.symbols.symbol), \ + Set(:py:class:`psyclone.psyir.symbols.symbol), \ + Set(:py:class:`psyclone.psyir.symbols.symbol)] :raises GenerationError: if the DefaultClauseType associated with \ this OMPParallelDirective is not shared. - :raises InternalError: if a Kernel has local variable(s) but they \ - aren't named. ''' if (self.default_clause.clause_type != @@ -652,12 +705,26 @@ def _get_private_clauses(self): raise GenerationError("OMPParallelClause cannot correctly generate" " the private clause when its default " "data sharing attribute in its default " - "clause is not shared.") + "clause is not 'shared'.") + + # TODO #598: Improve the handling of scalar variables, there are + # remaining issues when we have accesses after the parallel region + # of variables that we currently declare as private. This should be + # lastprivate. + # e.g: + # !$omp parallel do <- will set private(ji, my_index) + # do ji = 1, jpk + # my_index = ji+1 + # array(my_index) = 2 + # enddo + # #end do + # call func(my_index) <- my_index has not been updated private = set() fprivate = set() + need_sync = set() - # Now determine scalar variables that must be private: + # Determine variables that must be private, firstprivate or need_sync var_accesses = VariablesAccessInfo() self.reference_accesses(var_accesses) for signature in var_accesses.all_signatures: @@ -671,71 +738,81 @@ def _get_private_clauses(self): if len(accesses) == 1: continue - # We have at least two accesses. We consider private variables the - # ones that are written in every iteration of a loop. If one such - # scalar is read before it is written, it will be considered - # firstprivate. + # TODO #598: If we only have writes, it must be need_sync: + # do ji = 1, jpk + # if ji=3: + # found = .true. + # Or lastprivate in order to maintain the serial semantics + # do ji = 1, jpk + # found = ji + + # We consider private variables as being the ones that are written + # in every iteration of a loop. + # If one such scalar is potentially read before it is written, it + # will be considered firstprivate. has_been_read = False + last_read_position = 0 for access in accesses: if access.access_type == AccessType.READ: has_been_read = True + last_read_position = access.node.abs_position if access.access_type == AccessType.WRITE: - # Check if the write access is inside a loop. If the write - # is outside of a loop, it is an assignment to a shared - # variable. Example where jpk is likely used outside of the - # parallel section later, so it must be declared as shared - # in order keep its value: + + # Check if the write access is outside a loop. In this case + # it will be marked as shared. This is done because it is + # likely to be re-used later. e.g: # !$omp parallel # jpk = 100 # !omp do # do ji = 1, jpk - - # TODO #598: Improve the handling of scalar variables, - # there are remaining issues with references after the - # parallel region of variables that we currently declare - # as private. - - # Check if it is inside a loop loop_ancestor = access.node.ancestor( (Loop, WhileLoop), limit=self, include_self=True) - - if loop_ancestor: - # The assignment to the variable is inside a loop, so - # declare it to be private - name = str(signature).lower() - symbol = access.node.scope.symbol_table.lookup(name) - - # Is the write conditional? - conditional_write = access.node.ancestor( - IfBlock, - limit=loop_ancestor, - include_self=True) - - # If a previous value might be needed we mark it as - # firstprivate - if has_been_read or conditional_write: + if not loop_ancestor: + # If we find it at least once outside a loop we keep it + # as shared + break + + # Otherwise, the assignment to this variable is inside a + # loop (and it will be repeated for each iteration), so + # we declare it as private or need_synch + name = signature.var_name + # TODO #2094: var_name only captures the top-level + # component in the derived type accessor. If the attributes + # only apply to a sub-component, this won't be captured + # appropriately. + symbol = access.node.scope.symbol_table.lookup(name) + + # If it has been read before we have to check if ... + if has_been_read: + loop_pos = loop_ancestor.loop_body.abs_position + if last_read_position < loop_pos: + # .. it was before the loop, so it is fprivate fprivate.add(symbol) else: - private.add(symbol) + # or inside the loop, in which case it needs sync + need_sync.add(symbol) + break + + # If the write is not guaranteed, we make it firstprivate + # so that in the case that the write doesn't happen we keep + # the original value + conditional_write = access.node.ancestor( + IfBlock, + limit=loop_ancestor, + include_self=True) + if conditional_write: + fprivate.add(symbol) + break # Already found the first write and decided if it is # shared, private or firstprivate. We can stop looking. + private.add(symbol) break - # Convert the sets into a list and sort them, so that we get - # reproducible results - list_private = list(private) - list_private.sort(key=lambda x: x.name) - list_fprivate = list(fprivate) - list_fprivate.sort(key=lambda x: x.name) - - # Create the OMPPrivateClause and OMPFirstpirvateClause - priv_clause = OMPPrivateClause.create(list_private) - fpriv_clause = OMPFirstprivateClause.create(list_fprivate) - return priv_clause, fpriv_clause + return private, fprivate, need_sync def validate_global_constraints(self): ''' @@ -1313,15 +1390,25 @@ def gen_code(self, parent): # pylint: disable=protected-access default_str = self.children[1]._clause_string # pylint: enable=protected-access - private, fprivate = self._get_private_clauses() + private, fprivate, need_sync = self._infer_sharing_attributes() + private_clause = OMPPrivateClause.create( + sorted(private, key=lambda x: x.name)) + fprivate_clause = OMPFirstprivateClause.create( + sorted(fprivate, key=lambda x: x.name)) + if need_sync: + raise GenerationError( + f"OMPParallelDoDirective.gen_code() does not support symbols " + f"that need synchronisation, but found: " + f"{[x.name for x in need_sync]}") - private_list = [child.symbol.name for child in private.children] - private_str = "private(" + ",".join(private_list) + ")" - fp_list = [child.symbol.name for child in fprivate.children] + private_str = "" + fprivate_str = "" + private_list = [child.symbol.name for child in private_clause.children] + if private_list: + private_str = "private(" + ",".join(private_list) + ")" + fp_list = [child.symbol.name for child in fprivate_clause.children] if fp_list: fprivate_str = "firstprivate(" + ",".join(fp_list) + ")" - else: - fprivate_str = "" # Set schedule clause if self._omp_schedule != "none": diff --git a/src/psyclone/psyir/nodes/operation.py b/src/psyclone/psyir/nodes/operation.py index f2aafb8046d..43bc06d4804 100644 --- a/src/psyclone/psyir/nodes/operation.py +++ b/src/psyclone/psyir/nodes/operation.py @@ -399,7 +399,7 @@ class BinaryOperation(Operation): # Relational Operators 'EQ', 'NE', 'GT', 'LT', 'GE', 'LE', # Logical Operators - 'AND', 'OR', + 'AND', 'OR', 'EQV', 'NEQV', # Other Maths Operators 'SIGN', 'MIN', 'MAX', # Casting operators diff --git a/src/psyclone/psyir/symbols/datatypes.py b/src/psyclone/psyir/symbols/datatypes.py index fcb5f93071a..3672ff9e791 100644 --- a/src/psyclone/psyir/symbols/datatypes.py +++ b/src/psyclone/psyir/symbols/datatypes.py @@ -96,8 +96,8 @@ class UnknownType(DataType, metaclass=abc.ABCMeta): This class is abstract and must be subclassed for each language supported by the PSyIR frontends. - :param str declaration_txt: string containing the original variable \ - declaration. + :param str declaration_txt: the original textual declaration of + the symbol. :raises TypeError: if the supplied declaration_txt is not a str. @@ -108,13 +108,16 @@ def __init__(self, declaration_txt): f"UnknownType constructor expects the original variable " f"declaration as a string but got an argument of type " f"'{type(declaration_txt).__name__}'") - self._declaration = None - self.declaration = declaration_txt + self._declaration = declaration_txt @abc.abstractmethod def __str__(self): ''' Abstract method that must be implemented in subclass. ''' + # Note, an UnknownType is immutable so a declaration setter is not + # allowed. This is to allow subclasses to extract and provide + # parts of the declaration without worrying about their values + # becoming invalid due to a change in the original declaration. @property def declaration(self): ''' @@ -124,52 +127,45 @@ def declaration(self): ''' return self._declaration - @declaration.setter - def declaration(self, value): - ''' - Sets the original declaration that this instance represents. - - :param str value: the original declaration. - - ''' - self._declaration = value[:] - class UnknownFortranType(UnknownType): - ''' - Indicates that a Fortran declaration is not supported by the PSyIR. + '''Indicates that a Fortran declaration is not supported by the PSyIR. + + :param str declaration_txt: string containing the original variable + declaration. + :param partial_datatype: a subset of the unknown type if the + subset can form a valid PSyIR datatype. + :type partial_datatype: Optional[ + :py:class:`psyclone.psyir.symbols.DataType` or + :py:class:`psyclone.psyir.symbols.DataTypeSymbol`] - :param str declaration_txt: string containing the original variable \ - declaration. ''' - def __init__(self, declaration_txt): + def __init__(self, declaration_txt, partial_datatype=None): super().__init__(declaration_txt) # This will hold the Fortran type specification (as opposed to # the whole declaration). self._type_text = "" + if not isinstance( + partial_datatype, (type(None), DataType, DataTypeSymbol)): + raise TypeError( + f"partial_datatype argument in UnknownFortranType " + f"initialisation should be a DataType, DataTypeSymbol, or " + f"NoneType, but found '{type(partial_datatype).__name__}'.") + # This holds a subset of the type in a datatype if it is + # possible to determine enough information to create one. + self._partial_datatype = partial_datatype def __str__(self): return f"UnknownFortranType('{self._declaration}')" @property - def declaration(self): - ''' - This useless routine is required so that we can override the associated - setter method below. - ''' - return super().declaration - - @declaration.setter - def declaration(self, value): + def partial_datatype(self): ''' - Sets the original declaration that this instance represents and - removes any cached type text. - - :param str value: the original declaration. - + :returns: partial datatype information if it can be determined, \ + else None. + :rtype: Optional[:py:class:`psyclone.symbols.DataType`] ''' - self._declaration = value[:] - self._type_text = "" + return self._partial_datatype @property def type_text(self): @@ -178,7 +174,7 @@ def type_text(self): parse tree to extract the type information. This is returned in text form and also cached. - TODO #1419 - alter Unknown(Fortran)Type so that it is only the + TODO #2137 - alter Unknown(Fortran)Type so that it is only the type information that is stored as a string. i.e. remove the name of the variable being declared. Once that is done this method won't be required. diff --git a/src/psyclone/psyir/symbols/symbol_table.py b/src/psyclone/psyir/symbols/symbol_table.py index b72a9b50409..c482d9ad2c2 100644 --- a/src/psyclone/psyir/symbols/symbol_table.py +++ b/src/psyclone/psyir/symbols/symbol_table.py @@ -34,6 +34,7 @@ # Authors R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab # I. Kavcic, Met Office # J. Henrichs, Bureau of Meteorology +# Modified A. B. G. Chalk, STFC Daresbury Lab # ----------------------------------------------------------------------------- ''' This module contains the SymbolTable implementation. ''' @@ -282,8 +283,10 @@ def deep_copy(self): # Fix the container links for imported symbols for symbol in new_st.imported_symbols: name = symbol.interface.container_symbol.name + orig_name = symbol.interface.orig_name new_container = new_st.lookup(name) - symbol.interface = ImportInterface(new_container) + symbol.interface = ImportInterface(new_container, + orig_name=orig_name) # Set the default visibility new_st._default_visibility = self.default_visibility diff --git a/src/psyclone/psyir/tools/dependency_tools.py b/src/psyclone/psyir/tools/dependency_tools.py index 83e5987e48b..e35fb726566 100644 --- a/src/psyclone/psyir/tools/dependency_tools.py +++ b/src/psyclone/psyir/tools/dependency_tools.py @@ -47,7 +47,6 @@ VariablesAccessInfo) from psyclone.errors import InternalError, LazyString from psyclone.psyir.nodes import Loop -from psyclone.psyir.backend.fortran import FortranWriter from psyclone.psyir.backend.sympy_writer import SymPyWriter from psyclone.psyir.backend.visitor import VisitorError from psyclone.psyir.tools.read_write_info import ReadWriteInfo @@ -136,17 +135,11 @@ class DependencyTools(): specified in the PSyclone config file. This can be used to\ exclude for example 1-dimensional loops. :type loop_types_to_parallelise: Optional[List[str]] - :param language_writer: a backend visitor to convert PSyIR expressions \ - to a representation in the selected language. This is used for \ - creating error and warning messages. - :type language_writer: \ - Optional[:py:class:`psyclone.psyir.backend.visitor.PSyIRVisitor`] :raises TypeError: if an invalid loop type is specified. ''' - def __init__(self, loop_types_to_parallelise=None, - language_writer=None): + def __init__(self, loop_types_to_parallelise=None): if loop_types_to_parallelise: # Verify that all loop types specified are valid: config = Config.get() @@ -162,10 +155,6 @@ def __init__(self, loop_types_to_parallelise=None, self._loop_types_to_parallelise = loop_types_to_parallelise[:] else: self._loop_types_to_parallelise = [] - if not language_writer: - self._language_writer = FortranWriter() - else: - self._language_writer = language_writer self._clear_messages() # ------------------------------------------------------------------------- @@ -650,26 +639,26 @@ def _array_access_parallelisable(self, loop_variables, var_info): self._add_message(LazyString( lambda node=write_access.node: (f"The write access to '" - f"{self._language_writer(node)}' causes " + f"{node.debug_string()}' causes " f"a write-write race condition.")), DTCode.ERROR_WRITE_WRITE_RACE, [LazyString(lambda node=node: - f"{self._language_writer(node)}")]) + f"{node.debug_string()}")]) else: self._add_message(LazyString( lambda wnode=write_access.node, onode=other_access.node: (f"The write access to " - f"'{self._language_writer(wnode)}' " - f"and to '{self._language_writer(onode)}" + f"'{wnode.debug_string()}' " + f"and to '{onode.debug_string()}" f"' are dependent and cannot be " f"parallelised.")), DTCode.ERROR_DEPENDENCY, [LazyString(lambda wnode=write_access.node: - f"{self._language_writer(wnode)}" + f"{wnode.debug_string()}" ), LazyString(lambda onode=other_access.node: - f"{self._language_writer(onode)}")]) + f"{onode.debug_string()}")]) return False return True diff --git a/src/psyclone/tests/domain/lfric/algorithm/lfric_alg_test.py b/src/psyclone/tests/domain/lfric/algorithm/lfric_alg_test.py index 8aa1aa826c7..dbc5f56cfa1 100644 --- a/src/psyclone/tests/domain/lfric/algorithm/lfric_alg_test.py +++ b/src/psyclone/tests/domain/lfric/algorithm/lfric_alg_test.py @@ -126,6 +126,9 @@ def test_create_function_spaces_no_spaces(lfric_alg, prog): ''' Check that a Routine is populated as expected, even when there are no actual function spaces. ''' lfric_alg._create_function_spaces(prog, []) + fe_config_mod = prog.symbol_table.lookup("finite_element_config_mod") + element_order = prog.symbol_table.lookup("element_order") + assert element_order.interface.container_symbol == fe_config_mod assert prog.symbol_table.lookup("element_order") assert isinstance(prog.symbol_table.lookup("fs_continuity_mod"), ContainerSymbol) @@ -144,6 +147,9 @@ def test_create_function_spaces(lfric_alg, prog, fortran_writer): ''' Check that a Routine is populated correctly when valid function-space names are supplied. ''' lfric_alg._create_function_spaces(prog, ["w3", "w1"]) + fe_config_mod = prog.symbol_table.lookup("finite_element_config_mod") + element_order = prog.symbol_table.lookup("element_order") + assert element_order.interface.container_symbol == fe_config_mod fs_mod_sym = prog.symbol_table.lookup("fs_continuity_mod") gen = fortran_writer(prog) for space in ["w1", "w3"]: diff --git a/src/psyclone/tests/domain/lfric/arg_index_to_metadata_index_test.py b/src/psyclone/tests/domain/lfric/arg_index_to_metadata_index_test.py index 72974d93b45..1eef4ae33c5 100644 --- a/src/psyclone/tests/domain/lfric/arg_index_to_metadata_index_test.py +++ b/src/psyclone/tests/domain/lfric/arg_index_to_metadata_index_test.py @@ -93,6 +93,7 @@ def test_scalar(): assert len(cls._info) == 1 # pylint: disable=unsubscriptable-object assert cls._info[0] == 1 + assert cls._index == 1 def test_field(): @@ -104,6 +105,7 @@ def test_field(): assert len(cls._info) == 1 # pylint: disable=unsubscriptable-object assert cls._info[0] == 0 + assert cls._index == 1 def test_field_vector(): @@ -117,6 +119,7 @@ def test_field_vector(): assert cls._info[0] == 0 assert cls._info[1] == 0 assert cls._info[2] == 0 + assert cls._index == 3 def test_operator(): @@ -127,7 +130,8 @@ def test_operator(): cls = call_method("_operator", meta_arg, metadata) assert len(cls._info) == 1 # pylint: disable=unsubscriptable-object - assert cls._info[0] == 0 + assert cls._info[1] == 0 + assert cls._index == 2 def test_cma_operator(): @@ -139,6 +143,7 @@ def test_cma_operator(): assert len(cls._info) == 1 # pylint: disable=unsubscriptable-object assert cls._info[0] == 0 + assert cls._index == 8 def test_add_arg(): diff --git a/src/psyclone/tests/domain/nemo/transformations/nemo_allarrayrange2loop_trans_test.py b/src/psyclone/tests/domain/nemo/transformations/nemo_allarrayrange2loop_trans_test.py index 92ce8622e6d..6cd854906e0 100644 --- a/src/psyclone/tests/domain/nemo/transformations/nemo_allarrayrange2loop_trans_test.py +++ b/src/psyclone/tests/domain/nemo/transformations/nemo_allarrayrange2loop_trans_test.py @@ -1,7 +1,7 @@ # ----------------------------------------------------------------------------- # BSD 3-Clause License # -# Copyright (c) 2020-2022, Science and Technology Facilities Council. +# Copyright (c) 2020-2023, Science and Technology Facilities Council. # All rights reserved. # # Redistribution and use in source and binary forms, with or without @@ -158,6 +158,44 @@ def test_apply_with_structures(fortran_reader, fortran_writer): "ptab(jf)%pt2d(jpim1,idx_1,idx)") in result +def test_apply_option_verbose(fortran_reader, capsys): + '''Check that the transformation with the verbose option provides more + information about why the last attempt to convert the array range into + a loop was not successful. + + ''' + trans = NemoAllArrayRange2LoopTrans() + + psyir = fortran_reader.psyir_from_source(''' + subroutine test + use my_variables + integer, parameter :: constant = 3 + integer :: jk, a, b + integer, dimension(3) :: array + a = b + array(:) = my_func() + end subroutine test + ''') + + # Basic cases like assignment 1 are skipped + assignment = psyir.walk(Assignment)[0] + trans.apply(assignment, options={'verbose': True}) + out, _ = capsys.readouterr() + assert out == "" + + # Other cases like assignment 2 are printed to stdout because they are due + # to PSyclone limitations, in this case my_func is a codeblock because + # currently the Fortran reader only generate functions if it can see its + # declaration. + assignment = psyir.walk(Assignment)[1] + trans.apply(assignment, options={'verbose': True}) + out, _ = capsys.readouterr() + assert ("Error in NemoArrayRange2LoopTrans transformation. This " + "transformation does not support array assignments that contain " + "a CodeBlock anywhere in the expression, but found:\n" + "array(:) = my_func()" in out) + + def test_apply_calls_validate(): '''Check that the apply() method calls the validate method.''' trans = NemoAllArrayRange2LoopTrans() diff --git a/src/psyclone/tests/generator_test.py b/src/psyclone/tests/generator_test.py index ad69f137a9c..52933a486d2 100644 --- a/src/psyclone/tests/generator_test.py +++ b/src/psyclone/tests/generator_test.py @@ -759,7 +759,7 @@ def test_generate_trans_error(tmpdir, capsys, monkeypatch): "module setval_c_mod\n" "contains\n" "subroutine setval_c()\n" - " use builtins\n" + " use psyclone_builtins\n" " use constants_mod, only: r_def\n" " use field_mod, only : field_type\n" " type(field_type) :: field\n" @@ -786,6 +786,30 @@ def test_generate_trans_error(tmpdir, capsys, monkeypatch): assert expected_output2 in output +def test_generate_no_builtin_container(tmpdir, monkeypatch): + '''Test that a builtin use statement is removed if it has been added + to a Container (a module). Also tests that everything works OK if + no use statement is found in a symbol table (as FileContainer does + not contain one). + + ''' + monkeypatch.setattr(generator, "LFRIC_TESTING", True) + code = ( + "module test_mod\n" + " contains\n" + " subroutine test()\n" + " use field_mod, only : field_type\n" + " type(field_type) :: field\n" + " call invoke(setval_c(field, 0.0))\n" + " end subroutine test\n" + "end module\n") + filename = str(tmpdir.join("alg.f90")) + with open(filename, "w", encoding='utf-8') as my_file: + my_file.write(code) + alg, _ = generate(filename, api="dynamo0.3") + assert "use _psyclone_builtins" not in alg + + def test_main_unexpected_fatal_error(capsys, monkeypatch): '''Tests that we get the expected output and the code exits with an error when an unexpected fatal error is returned from the generate @@ -1129,8 +1153,8 @@ def test_add_builtins_use(): parser = ParserFactory().create(std="f2008") reader = FortranStringReader(code) fp2_tree = parser(reader) - add_builtins_use(fp2_tree) - assert "USE builtins" in str(fp2_tree) + add_builtins_use(fp2_tree, "my_name") + assert "USE my_name" in str(fp2_tree) # spec_part code = ( "program test_prog\n" @@ -1138,8 +1162,8 @@ def test_add_builtins_use(): "end program\n") reader = FortranStringReader(code) fp2_tree = parser(reader) - add_builtins_use(fp2_tree) - assert "USE builtins" in str(fp2_tree) + add_builtins_use(fp2_tree, "ANOTHER_NAME") + assert "USE ANOTHER_NAME" in str(fp2_tree) # multiple modules/programs code = ( "program test_prog\n" @@ -1150,7 +1174,7 @@ def test_add_builtins_use(): "end module\n") reader = FortranStringReader(code) fp2_tree = parser(reader) - add_builtins_use(fp2_tree) + add_builtins_use(fp2_tree, "builtins") assert str(fp2_tree) == ( "PROGRAM test_prog\n USE builtins\nEND PROGRAM\n" "MODULE test_mod1\n USE builtins\nEND MODULE\n" @@ -1176,9 +1200,8 @@ def test_no_script_lfric_new(monkeypatch): assert " testkern_type" not in alg # module symbol is removed assert "testkern_mod" not in alg - # TODO issue #1618. The builtins statement should be removed from - # the processed source code. - assert "use builtins" in alg + # _psyclone_builtins symbol (that was added by PSyclone) is removed + assert "use _psyclone_builtins" not in alg def test_script_lfric_new(monkeypatch): @@ -1202,9 +1225,8 @@ def test_script_lfric_new(monkeypatch): assert " testkern_type" not in alg # module symbol is removed assert "testkern_mod" not in alg - # TODO issue #1618. The builtins statement should be removed from - # the processed source code. - assert "use builtins" in alg + # _psyclone_builtins symbol (that was added by PSyclone) is removed + assert "use _psyclone_builtins" not in alg def test_builtins_lfric_new(monkeypatch): @@ -1232,9 +1254,8 @@ def test_builtins_lfric_new(monkeypatch): assert " testkern_mod" not in alg assert " testkern_wtheta_mod" not in alg assert " testkern_w2_only_mod" not in alg - # TODO issue #1618. The builtins statement should be removed from - # the processed source code. - assert "use builtins" in alg + # _psyclone_builtins symbol (that was added by PSyclone) is removed + assert "use _psyclone_builtins" not in alg def test_no_invokes_lfric_new(monkeypatch): diff --git a/src/psyclone/tests/psyad/domain/lfric/test_lfric_adjoint_harness.py b/src/psyclone/tests/psyad/domain/lfric/test_lfric_adjoint_harness.py index d9ec943d9af..ada76ba72fe 100644 --- a/src/psyclone/tests/psyad/domain/lfric/test_lfric_adjoint_harness.py +++ b/src/psyclone/tests/psyad/domain/lfric/test_lfric_adjoint_harness.py @@ -356,10 +356,11 @@ def test_init_scalar_value(monkeypatch): # property of the datatype. sym4 = DataSymbol("my_var", LFRicTypes("LFRicRealScalarDataType")()) - class broken_type: + class BrokenType: + '''Utility class to provide an unsupported type.''' def __init__(self): self.name = "wrong" - monkeypatch.setattr(sym4.datatype, "intrinsic", broken_type()) + monkeypatch.setattr(sym4.datatype, "intrinsic", BrokenType()) with pytest.raises(InternalError) as err: _init_scalar_value(sym4, routine, {}) assert ("scalars of REAL, INTEGER or BOOLEAN type are supported but got " @@ -452,8 +453,8 @@ def test_generate_lfric_adjoint_harness_invalid_code(fortran_reader): "(Container) but the supplied PSyIR does not have a Container " "node:\nFileContainer[]\n Routine[name:'oops']" in str(err.value)) - psyir = fortran_reader.psyir_from_source("module wrong\n" - "end module wrong\n") + psyir = fortran_reader.psyir_from_source( + TL_CODE.replace("testkern_mod", "wrong")) with pytest.raises(ValueError) as err: _ = generate_lfric_adjoint_harness(psyir) assert ("The supplied LFRic TL kernel is contained within a module named " @@ -495,46 +496,47 @@ def test_generate_lfric_adjoint_harness(fortran_reader, fortran_writer): tl_psyir = fortran_reader.psyir_from_source(TL_CODE) psyir = generate_lfric_adjoint_harness(tl_psyir) gen = fortran_writer(psyir).lower() + assert "use finite_element_config_mod, only : element_order" in gen assert "module adjoint_test_mod" in gen assert "subroutine adjoint_test(mesh, chi, panel_id)" in gen # We should have a field, a copy of that field and an inner-product value # for that field. - assert (" real(kind=r_def) :: rscalar_1\n" - " type(field_type) :: field_2\n" - " real(kind=r_def) :: rscalar_1_input\n" - " type(field_type) :: field_2_input\n" - " real(kind=r_def) :: field_2_inner_prod\n" in gen) + assert (" real(kind=r_def) :: ascalar\n" + " type(field_type) :: field\n" + " real(kind=r_def) :: ascalar_input\n" + " type(field_type) :: field_input\n" + " real(kind=r_def) :: field_inner_prod\n" in gen) # The field and its copy must be initialised. - assert ("call field_2 % initialise(vector_space=vector_space_w3_ptr, " - "name='field_2')" in gen) - assert ("call field_2_input % initialise(vector_space=vector_space_w3_ptr," - " name='field_2_input')" in gen) + assert ("call field % initialise(vector_space=vector_space_w3_ptr, " + "name='field')" in gen) + assert ("call field_input % initialise(vector_space=vector_space_w3_ptr," + " name='field_input')" in gen) # So too must the scalar argument. - assert (" call random_number(rscalar_1)\n" - " rscalar_1_input = rscalar_1\n" in gen) + assert (" call random_number(ascalar)\n" + " ascalar_input = ascalar\n" in gen) # The field must be given random values and those copied into the copy. # The TL kernel must then be called and the inner-product of the result # computed. - assert "field_2_inner_prod = 0.0_r_def" in gen + assert "field_inner_prod = 0.0_r_def" in gen assert (" ! initialise arguments and call the tangent-linear kernel.\n" - " call invoke(setval_random(field_2), setval_x(field_2_input, " - "field_2), testkern_type(rscalar_1, field_2), x_innerproduct_x(" - "field_2_inner_prod, field_2))\n" in gen) + " call invoke(setval_random(field), setval_x(field_input, " + "field), testkern_type(ascalar, field), x_innerproduct_x(" + "field_inner_prod, field))\n" in gen) # Compute and store the sum of all inner products. assert (" inner1 = 0.0_r_def\n" - " inner1 = inner1 + rscalar_1 * rscalar_1\n" - " inner1 = inner1 + field_2_inner_prod\n" - " field_2_field_2_input_inner_prod = 0.0_r_def\n" in gen) + " inner1 = inner1 + ascalar * ascalar\n" + " inner1 = inner1 + field_inner_prod\n" + " field_field_input_inner_prod = 0.0_r_def\n" in gen) # Run the adjoint of the kernel and compute the inner products of its # outputs with the inputs to the TL kernel. - assert ("call invoke(adj_testkern_type(rscalar_1, field_2), " - "x_innerproduct_y(field_2_field_2_input_inner_prod, field_2, " - "field_2_input))" + assert ("call invoke(adj_testkern_type(ascalar, field), " + "x_innerproduct_y(field_field_input_inner_prod, field, " + "field_input))" in gen) assert (" inner2 = 0.0_r_def\n" - " inner2 = inner2 + rscalar_1 * rscalar_1_input\n" - " inner2 = inner2 + field_2_field_2_input_inner_prod\n" in gen) + " inner2 = inner2 + ascalar * ascalar_input\n" + " inner2 = inner2 + field_field_input_inner_prod\n" in gen) def test_generate_lfric_adj_test_quadrature(fortran_reader): @@ -547,6 +549,19 @@ def test_generate_lfric_adj_test_quadrature(fortran_reader): " /)\n" " integer :: operates_on = cell_column\n" " integer :: gh_shape = gh_quadrature_xyoz\n") + new_code = new_code.replace( + "field, ndf_w3, undf_w3, map_w3)\n", + "field, ndf_w3, undf_w3, map_w3, basis_w3_qr_xyoz, np_xy_qr_xyoz, " + "np_z_qr_xyoz, weights_xy_qr_xyoz, weights_z_qr_xyoz)\n" + " INTEGER(KIND=i_def), intent(in) :: np_xy_qr_xyoz, " + "np_z_qr_xyoz\n" + " REAL(KIND=r_def), intent(in), dimension(1,ndf_w3," + "np_xy_qr_xyoz,np_z_qr_xyoz) :: basis_w3_qr_xyoz\n" + " REAL(KIND=r_def), intent(in), dimension(np_xy_qr_xyoz) :: " + "weights_xy_qr_xyoz\n" + " REAL(KIND=r_def), intent(in), dimension(np_z_qr_xyoz) :: " + "weights_z_qr_xyoz\n") + tl_psyir = fortran_reader.psyir_from_source(new_code) psyir = generate_lfric_adjoint_harness(tl_psyir) routine = psyir.walk(nodes.Routine)[0] @@ -555,7 +570,7 @@ def test_generate_lfric_adj_test_quadrature(fortran_reader): # are never active). if sym.name.endswith("_input"): assert (sym.name.startswith("field") or - sym.name.startswith("rscalar")) + sym.name.startswith("ascalar")) def test_generate_lfric_adjoint_harness_operator(fortran_reader, @@ -570,22 +585,35 @@ def test_generate_lfric_adjoint_harness_operator(fortran_reader, "arg_type(gh_field, gh_real, gh_write, w3), &\n" "arg_type(gh_operator,gh_real,gh_read,w3,w0) &") code = code.replace("dimension(2)", "dimension(3)") + code = code.replace("nlayers, ascalar", "cell, nlayers, ascalar") + code = code.replace( + "field, ndf_w3, undf_w3, map_w3", + "field, op_ncell_3d, op, ndf_w3, undf_w3, map_w3, ndf_w0") + code = code.replace( + " field = ascalar\n", + " INTEGER(KIND=i_def), intent(in) :: ndf_w0\n" + " INTEGER(KIND=i_def), intent(in) :: cell\n" + " INTEGER(KIND=i_def), intent(in) :: op_ncell_3d\n" + " REAL(KIND=r_def), intent(in), dimension(ndf_w3,ndf_w0," + "op_ncell_3d) :: op\n" + " field = ascalar\n") + tl_psyir = fortran_reader.psyir_from_source(code) psyir = generate_lfric_adjoint_harness(tl_psyir) gen = fortran_writer(psyir) - assert "type(operator_type) :: op_3\n" in gen + assert "type(operator_type) :: op\n" in gen assert ("vector_space_w0_ptr => function_space_collection % get_fs(mesh, " "element_order, w0)\n" in gen) assert ("vector_space_w3_ptr => function_space_collection % get_fs(mesh, " "element_order, w3)\n" in gen) # Initialise takes the *to* and *from* spaces as arguments in that order. - assert ("call op_3 % initialise(vector_space_w3_ptr, vector_space_w0_ptr)" + assert ("call op % initialise(vector_space_w3_ptr, vector_space_w0_ptr)" in gen) # Operator is given random values and passed to the TL kernel. - assert ("setop_random_kernel_type(op_3), " - "testkern_type(rscalar_1, field_2, op_3)" in gen) + assert ("setop_random_kernel_type(op), " + "testkern_type(ascalar, field, op)" in gen) # Operator is passed to the Adjoint kernel too. - assert "call invoke(adj_testkern_type(rscalar_1, field_2, op_3)" in gen + assert "call invoke(adj_testkern_type(ascalar, field, op)" in gen def test_gen_lfric_adjoint_harness_written_operator(fortran_reader,): @@ -598,10 +626,23 @@ def test_gen_lfric_adjoint_harness_written_operator(fortran_reader,): "arg_type(gh_field, gh_real, gh_write, w3), &\n" "arg_type(gh_operator,gh_real,gh_write,w3,w0) &") code = code.replace("dimension(2)", "dimension(3)") + + code = code.replace("nlayers, ascalar", "cell, nlayers, ascalar") + code = code.replace( + "field, ndf_w3, undf_w3, map_w3", + "field, op_ncell_3d, op, ndf_w3, undf_w3, map_w3, ndf_w0") + code = code.replace( + " field = ascalar\n", + " INTEGER(KIND=i_def), intent(in) :: ndf_w0\n" + " INTEGER(KIND=i_def), intent(in) :: cell\n" + " INTEGER(KIND=i_def), intent(in) :: op_ncell_3d\n" + " REAL(KIND=r_def), intent(out), dimension(ndf_w3,ndf_w0," + "op_ncell_3d) :: op\n" + " field = ascalar\n") tl_psyir = fortran_reader.psyir_from_source(code) with pytest.raises(GenerationError) as err: generate_lfric_adjoint_harness(tl_psyir) - assert ("Operator argument 'op_3' to TL kernel 'testkern_type' is written " + assert ("Operator argument 'op' to TL kernel 'testkern_type' is written " "to. This is not supported." in str(err.value)) @@ -614,6 +655,8 @@ def test_generate_lfric_adjoint_harness_invalid_geom_arg(fortran_reader): _ = generate_lfric_adjoint_harness(tl_psyir, coord_arg_idx=1) assert ("The 'coordinate' argument is expected to be a field but argument " "1 to kernel 'testkern_code' is a 'gh_scalar'" in str(err.value)) + # Recreate the tl_psyir as it gets raised by this routine. + tl_psyir = fortran_reader.psyir_from_source(TL_CODE) with pytest.raises(ValueError) as err: _ = generate_lfric_adjoint_harness(tl_psyir, panel_id_arg_idx=1) assert ("The 'panel-id' argument is expected to be a field but argument 1 " @@ -678,8 +721,8 @@ def test_generate_lfric_adjoint_harness_chi_arg(fortran_reader, assert "setval_random(chi(1))" not in gen # chi should be passed as the second argument to the TL and adjoint # kernels. - assert "testkern_type(rscalar_1, chi, field_3, field_4)" in gen - assert "invoke(adj_testkern_type(rscalar_1, chi, field_3, field_4)" in gen + assert "testkern_type(ascalar, chi, field, pids)" in gen + assert "invoke(adj_testkern_type(ascalar, chi, field, pids)" in gen def test_generate_lfric_adjoint_harness_panel_id_arg(fortran_reader, @@ -699,8 +742,8 @@ def test_generate_lfric_adjoint_harness_panel_id_arg(fortran_reader, assert "setval_random(panel_id)" not in gen # panel id should be passed as the 4th argument to both the TL and adjoint # kernels. - assert "testkern_type(rscalar_1, field_2, field_3, panel_id)" in gen - assert ("invoke(adj_testkern_type(rscalar_1, field_2, field_3, panel_id)" + assert "testkern_type(ascalar, cfield3, field, panel_id)" in gen + assert ("invoke(adj_testkern_type(ascalar, cfield3, field, panel_id)" in gen) @@ -715,8 +758,8 @@ def test_generate_lfric_adjoint_harness_geom_args(fortran_reader, psyir = generate_lfric_adjoint_harness(tl_psyir, panel_id_arg_idx=4, coord_arg_idx=2) gen = fortran_writer(psyir) - assert "testkern_type(rscalar_1, chi, field_3, panel_id)" in gen - assert ("invoke(adj_testkern_type(rscalar_1, chi, field_3, panel_id)" + assert "testkern_type(ascalar, chi, field, panel_id)" in gen + assert ("invoke(adj_testkern_type(ascalar, chi, field, panel_id)" in gen) diff --git a/src/psyclone/tests/psyir/backend/fortran_test.py b/src/psyclone/tests/psyir/backend/fortran_test.py index 4167930e2ae..1e0e8939fcb 100644 --- a/src/psyclone/tests/psyir/backend/fortran_test.py +++ b/src/psyclone/tests/psyir/backend/fortran_test.py @@ -530,6 +530,7 @@ def test_precedence(): assert precedence('*') < precedence('**') assert precedence('.EQ.') == precedence('==') assert precedence('*') == precedence('/') + assert precedence('.EQV.') == precedence('.NEQV.') def test_precedence_error(): @@ -1174,6 +1175,8 @@ def test_fw_binaryoperator_precedence(fortran_reader, fortran_writer, tmpdir): " 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" "end subroutine tmp\n" "end module test") schedule = fortran_reader.psyir_from_source(code) @@ -1188,7 +1191,9 @@ def test_fw_binaryoperator_precedence(fortran_reader, fortran_writer, tmpdir): " 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 = .NOT.e .AND. f .OR. g\n" + " e = e .AND. (f .EQV. g)\n" + " e = e .AND. f .NEQV. g\n") assert expected in result assert Compile(tmpdir).string_compiles(result) @@ -2155,3 +2160,23 @@ def test_fw_operand_clause(fortran_writer): ref_a = Reference(symbol_a) op_clause.addchild(ref_a) assert "depend(inout: a)" in fortran_writer(op_clause) + + +def test_fw_keeps_symbol_renaming(fortran_writer, fortran_reader): + '''Test that the FortranWriter correctly keeps => in Use statements to + ensure variable renaming is handle correctly.''' + code = ''' + module a + integer, parameter :: a_mod_name = 2 + end module a + module b + use a, only: b_mod_name => a_mod_name + contains + subroutine X() + print *, b_mod_name + end subroutine X + end module b + ''' + psyir = fortran_reader.psyir_from_source(code) + output = fortran_writer(psyir) + assert "b_mod_name=>a_mod_name" in output diff --git a/src/psyclone/tests/psyir/frontend/fparser2_do_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_do_handler_test.py index 08928514784..d9f12376ef6 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_do_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_do_handler_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Authors: A. R. Porter and N. Nobre, STFC Daresbury Lab +# Modified A. B. G. Chalk, STFC Datesbury Lab ''' Module containing pytest tests for the handling of the DO construct in the PSyIR fparser2 frontend. ''' @@ -43,6 +44,7 @@ from psyclone.errors import InternalError from psyclone.psyir.nodes import Assignment, BinaryOperation, CodeBlock, \ Literal, Loop, Routine, Schedule, WhileLoop +from psyclone.tests.utilities import Compile def test_handling_end_do_stmt(fortran_reader): @@ -181,3 +183,68 @@ def test_undeclared_loop_var(fortran_reader): _ = fortran_reader.psyir_from_source(code) assert ("Loop-variable name 'i' is not declared and there are no " "unqualified use statements" in str(err.value)) + + +def test_do_inside_while(fortran_reader, fortran_writer, tmpdir): + '''Check that the loop handler correctly identifies a while loop + containing a do loop as a separate while loop.''' + code = '''subroutine test_subroutine + integer :: j, iu_stdout, range_bands, i + + i = 0 + DO + WRITE(iu_stdout, '(A)') & + 'Enter units followed by lower and upper limits and increment:' + DO + EXIT + END DO + range_bands = 3 + if (range_bands + i > 3 .and. range_bands + i < 15) then + CYCLE + end if + do j = 1, range_bands + i = i + 1 + end do + if (i > 15) then + EXIT + end if + end do + + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + loops = psyir.walk((WhileLoop, Loop)) + assert isinstance(loops[0], WhileLoop) + assert isinstance(loops[1], WhileLoop) + assert isinstance(loops[2], Loop) + + output = fortran_writer(psyir) + correct = '''subroutine test_subroutine() + integer :: j + integer :: iu_stdout + integer :: range_bands + integer :: i + + i = 0 + do while (.true.) + WRITE(iu_stdout, '(A)') 'Enter units followed by lower and upper \ +limits and increment:' + do while (.true.) + EXIT + end do + range_bands = 3 + if (range_bands + i > 3 .AND. range_bands + i < 15) then + CYCLE + end if + do j = 1, range_bands, 1 + i = i + 1 + enddo + if (i > 15) then + EXIT + end if + end do + +end subroutine test_subroutine +''' + assert correct == output + assert Compile(tmpdir).string_compiles(output) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_fortran_use_test.py b/src/psyclone/tests/psyir/frontend/fparser2_fortran_use_test.py index faa6cb08087..11ac70766fb 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_fortran_use_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_fortran_use_test.py @@ -34,6 +34,7 @@ # Author: A. R. Porter, STFC Daresbury Lab # Modified: R. W. Ford, STFC Daresbury Lab # Modified: S. Siso, STFC Daresbury Lab +# Modified: A. B. G. Chalk, STFC Daresbury Lab ''' Performs py.test tests on the support for use statements in the fparser2 PSyIR front-end ''' @@ -46,7 +47,68 @@ from psyclone.psyir.frontend.fparser2 import Fparser2Reader from psyclone.psyir.nodes import KernelSchedule, Container from psyclone.psyir.symbols import ContainerSymbol, SymbolError, Symbol, \ - DataSymbol, AutomaticInterface, INTEGER_SINGLE_TYPE + DataSymbol, AutomaticInterface, INTEGER_SINGLE_TYPE, ScalarType, \ + RoutineSymbol + + +def test_use_return(fortran_reader): + ''' Check the the Fparser frontend correctly handles when a function uses + a return variable with a kind defined inside the function.''' + code = '''real(rkind) function x() + use my_mod, only: rkind + x = 1.0_rkind + end function x''' + psyir = fortran_reader.psyir_from_source(code) + sym = psyir.children[0].symbol_table.lookup("x") + assert isinstance(sym, DataSymbol) + assert isinstance(sym.datatype, ScalarType) + assert sym.datatype.intrinsic == ScalarType.Intrinsic.REAL + assert isinstance(sym.datatype.precision, DataSymbol) + assert sym.datatype.precision.name == "rkind" + + +def test_use_return2(fortran_reader): + ''' Check the the Fparser frontend correctly handles when a function uses + a return variable with a kind defined inside the parent module.''' + code = '''module mymod + use my_mod, only: rkind + contains + real(rkind) function x() + x = 1.0_rkind + end function x + end module mymod''' + psyir = fortran_reader.psyir_from_source(code) + sym = psyir.children[0].symbol_table.lookup("x") + assert isinstance(sym, RoutineSymbol) + assert isinstance(sym.datatype, ScalarType) + assert sym.datatype.intrinsic == ScalarType.Intrinsic.REAL + sym = psyir.children[0].children[0].symbol_table.lookup("x") + assert isinstance(sym, DataSymbol) + assert isinstance(sym.datatype, ScalarType) + assert sym.datatype.intrinsic == ScalarType.Intrinsic.REAL + assert isinstance(sym.datatype.precision, DataSymbol) + assert sym.datatype.precision.name == "rkind" + + code = '''module mymod + use my_mod, only: rkind + private + contains + real(rkind) function x() + x = 1.0_rkind + end function x + end module mymod''' + psyir = fortran_reader.psyir_from_source(code) + sym = psyir.children[0].symbol_table.lookup("x") + assert isinstance(sym, RoutineSymbol) + assert sym.visibility == Symbol.Visibility.PRIVATE + assert isinstance(sym.datatype, ScalarType) + assert sym.datatype.intrinsic == ScalarType.Intrinsic.REAL + sym = psyir.children[0].children[0].symbol_table.lookup("x") + assert isinstance(sym, DataSymbol) + assert isinstance(sym.datatype, ScalarType) + assert sym.datatype.intrinsic == ScalarType.Intrinsic.REAL + assert isinstance(sym.datatype.precision, DataSymbol) + assert sym.datatype.precision.name == "rkind" @pytest.mark.usefixtures("f2008_parser") diff --git a/src/psyclone/tests/psyir/frontend/fparser2_select_case_test.py b/src/psyclone/tests/psyir/frontend/fparser2_select_case_test.py new file mode 100644 index 00000000000..9b66cebaf68 --- /dev/null +++ b/src/psyclone/tests/psyir/frontend/fparser2_select_case_test.py @@ -0,0 +1,488 @@ +# ----------------------------------------------------------------------------- +# BSD 3-Clause License +# +# Copyright (c) 2019-2020, 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. Porter and A. B. G. Chalk, STFC Daresbury Laboratory + +''' Module containing pytest tests for the handling of some select case +construction for the Fparser->PSyIR frontend.''' + +from __future__ import absolute_import +import pytest + +from fparser.common.readfortran import FortranStringReader +from fparser.two.Fortran2003 import ( + Assignment_Stmt, Execution_Part, Name) + +from psyclone.errors import InternalError +from psyclone.psyir.frontend.fparser2 import Fparser2Reader +from psyclone.psyir.nodes import ( + Schedule, CodeBlock, Assignment, BinaryOperation, IfBlock) +from psyclone.psyir.symbols import ( + DataSymbol, INTEGER_TYPE, Symbol) + + +@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") +def test_handling_case_construct(): + ''' Test that fparser2 Case_Construct is converted to the expected PSyIR + tree structure. + + TODO #754 fix test so that 'disable_declaration_check' fixture is not + required. + ''' + reader = FortranStringReader( + '''SELECT CASE (selector) + CASE (label1) + branch1 = 1 + CASE (label2) + branch2 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2case_construct]) + + # Check a new node was properly generated and connected to parent + assert len(fake_parent.children) == 1 + ifnode = fake_parent.children[0] + assert isinstance(ifnode, IfBlock) + assert ifnode.if_body.ast is fparser2case_construct.content[2] + assert ifnode.if_body.ast_end is fparser2case_construct.content[2] + assert 'was_case' in ifnode.annotations + assert ifnode.condition.children[0].name == 'selector' + assert ifnode.condition.children[1].name == 'label1' + assert ifnode.if_body[0].children[0].name == 'branch1' + assert isinstance(ifnode.else_body[0], IfBlock) + assert ifnode.else_body[0].condition.children[1].name == 'label2' + assert ifnode.else_body[0].if_body[0].children[0].name == 'branch2' + assert ifnode.else_body[0].ast is \ + fparser2case_construct.content[4] + assert ifnode.else_body[0].children[1].ast is \ + fparser2case_construct.content[4] + assert ifnode.else_body[0].children[1].ast_end is \ + fparser2case_construct.content[4] + assert len(ifnode.else_body[0].children) == 2 # SELECT CASE ends here + + +@pytest.mark.usefixtures("f2008_parser") +def test_case_default(): + ''' Check that the fparser2Reader handles SELECT blocks with + a default clause. + + ''' + case_clauses = ["CASE default\nbranch3 = 1\nbranch3 = branch3 * 2\n", + "CASE (label1)\nbranch1 = 1\n", + "CASE (label2)\nbranch2 = 1\n"] + + # Create the symbols that the frontend will expect to already be + # present in the symbol table. + symbols = [] + for idx in [1, 2, 3]: + symbols.append(DataSymbol(f"branch{idx}", INTEGER_TYPE)) + for var_name in ["selector", "label1", "label2"]: + symbols.append(DataSymbol(var_name, INTEGER_TYPE)) + + # Loop over the 3 possible locations for the 'default' clause + for idx1, idx2, idx3 in [(0, 1, 2), (1, 0, 2), (1, 2, 0)]: + fortran_text = ( + f"SELECT CASE (selector)\n" + f"{case_clauses[idx1]}{case_clauses[idx2]}{case_clauses[idx3]}" + f"END SELECT\n") + reader = FortranStringReader(fortran_text) + fparser2case_construct = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + # Ensure we have the necessary symbols in the symbol table. + for sym in symbols: + fake_parent.symbol_table.add(sym) + + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2case_construct]) + assigns = fake_parent.walk(Assignment) + # Check that the assignment to 'branch 3' (in the default clause) is + # the deepest in the tree + assert "branch3" in str(assigns[2]) + assert isinstance(assigns[2].ast, Assignment_Stmt) + assert isinstance(assigns[2].parent, Schedule) + assert isinstance(assigns[2].parent.ast, Assignment_Stmt) + assert "branch3 * 2" in str(assigns[2].parent.ast_end) + assert isinstance(assigns[2].parent.parent, IfBlock) + # Check that the if-body of the parent IfBlock also contains + # an Assignment + assert isinstance(assigns[2].parent.parent.children[1], Schedule) + assert isinstance(assigns[2].parent.parent.children[1].children[0], + Assignment) + + +@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") +def test_handling_case_list(): + ''' Test that the Case_Construct handler correctly processes CASE + statements involving a list of conditions. + + TODO #754 fix test so that 'disable_declaration_check' fixture is not + required. + ''' + reader = FortranStringReader( + '''SELECT CASE (my_var) + CASE (label2, label3) + branch2 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert len(fake_parent.children) == 1 + ifnode = fake_parent.children[0] + assert isinstance(ifnode, IfBlock) + assert isinstance(ifnode.condition, BinaryOperation) + assert ifnode.condition.operator == BinaryOperation.Operator.OR + eqnode = ifnode.condition.children[0] + assert eqnode.operator == BinaryOperation.Operator.EQ + assert "my_var" in str(eqnode.children[0]) + assert "label2" in str(eqnode.children[1]) + eqnode = ifnode.children[0].children[1] + assert eqnode.operator == BinaryOperation.Operator.EQ + assert "my_var" in str(eqnode.children[0]) + assert "label3" in str(eqnode.children[1]) + + assert "Reference[name:'branch2']" in str(ifnode.if_body[0].lhs) + + +@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") +def test_handling_case_range(): + ''' Test that the Case_Construct handler correctly processes CASE + statements involving a range. + + TODO #754 fix test so that 'disable_declaration_check' fixture is not + required. + ''' + reader = FortranStringReader( + '''SELECT CASE (my_var) + CASE (label4:label5) + branch3 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert len(fake_parent.children) == 1 + ifnode = fake_parent.children[0] + assert isinstance(ifnode, IfBlock) + assert isinstance(ifnode.children[0], BinaryOperation) + assert ifnode.condition.operator == BinaryOperation.Operator.AND + assert ifnode.condition.children[0].operator == BinaryOperation.Operator.GE + assert ifnode.condition.children[1].operator == BinaryOperation.Operator.LE + assert "branch3" in str(ifnode.if_body[0].lhs) + + +@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") +def test_handling_case_range_list(): + ''' Test that the Case_Construct handler correctly processes CASE + statements involving a list of ranges. + + TODO #754 fix test so that 'disable_declaration_check' fixture is not + required. + ''' + reader = FortranStringReader( + '''SELECT CASE (my_var) + CASE (:label1, label5:, label6) + branch4 = 1 + END SELECT''') + # We should end up with: + # my_var <= label1 OR my_var >= label5 OR my_var == label6 + fparser2case_construct = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert len(fake_parent.children) == 1 + ifnode = fake_parent.children[0] + assert isinstance(ifnode, IfBlock) + assert isinstance(ifnode.condition, BinaryOperation) + assert ifnode.condition.operator == BinaryOperation.Operator.OR + assert ifnode.condition.children[0].operator == BinaryOperation.Operator.LE + assert "label1" in str(ifnode.condition.children[0].children[1]) + orop = ifnode.condition.children[1] + assert orop.operator == BinaryOperation.Operator.OR + assert orop.children[0].operator == BinaryOperation.Operator.GE + assert "label5" in str(orop.children[0].children[1]) + assert orop.children[1].operator == BinaryOperation.Operator.EQ + assert "label6" in str(orop.children[1].children[1]) + assert "branch4" in str(ifnode.if_body[0].lhs) + + +@pytest.mark.usefixtures("parser") +def test_handling_labelled_case_construct(): + ''' Test that a labelled case construct results in a CodeBlock. ''' + reader = FortranStringReader( + '''999 SELECT CASE (selector) + CASE (pick_me) + branch3 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + fake_parent.symbol_table.new_symbol("selector", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + fake_parent.symbol_table.new_symbol("pick_me", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + fake_parent.symbol_table.new_symbol("branch3", symbol_type=DataSymbol, + datatype=INTEGER_TYPE) + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert len(fake_parent.children) == 1 + assert isinstance(fake_parent.children[0], CodeBlock) + + +@pytest.mark.usefixtures("f2008_parser") +def test_case_default_only(): + ''' Check that we handle a select case that contains only a + default clause and is thus redundant. The PSyIR should represent + only the code that is within the default case. + + ''' + fake_parent = Schedule() + fake_parent.symbol_table.add(Symbol("a")) + processor = Fparser2Reader() + reader = FortranStringReader( + '''SELECT CASE ( jprstlib ) + CASE DEFAULT + WRITE(numout,*) 'open ice restart NetCDF file: ' + a = 1 + END SELECT''') + exe_part = Execution_Part.match(reader) + processor.process_nodes(fake_parent, exe_part[0]) + # We should have no IfBlock in the resulting PSyIR + assert len(fake_parent.children) == 2 + assert isinstance(fake_parent.children[0], CodeBlock) + assert isinstance(fake_parent.children[1], Assignment) + + +@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") +def test_handling_invalid_case_construct(): + ''' Test that the Case_Construct handler raises the proper errors when + it parses invalid or unsupported fparser2 trees. + + TODO #754 fix test so that 'disable_declaration_check' fixture is not + required. + ''' + # CASE (default) is just a regular symbol named default + reader = FortranStringReader( + '''SELECT CASE (selector) + CASE (default) + branch3 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + + fake_parent = Schedule() + processor = Fparser2Reader() + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert isinstance(fake_parent.children[0], IfBlock) + + # Test with no opening Select_Case_Stmt + reader = FortranStringReader( + '''SELECT CASE (selector) + CASE (label1) + branch1 = 1 + CASE (label2) + branch2 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + del fparser2case_construct.content[0] + with pytest.raises(InternalError) as error: + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert "Failed to find opening case statement in:" in str(error.value) + + # Test with no closing End_Select_Stmt + reader = FortranStringReader( + '''SELECT CASE (selector) + CASE (label1) + branch1 = 1 + CASE (label2) + branch2 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + del fparser2case_construct.content[-1] + with pytest.raises(InternalError) as error: + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert "Failed to find closing case statement in:" in str(error.value) + + # Test when one clause is not of the expected type + reader = FortranStringReader( + '''SELECT CASE (selector) + CASE (label1) + branch1 = 1 + CASE (label2) + branch2 = 1 + END SELECT''') + fparser2case_construct = Execution_Part.match(reader)[0][0] + fparser2case_construct.content[1].items = (Name("Fake"), None) + with pytest.raises(InternalError) as error: + processor.process_nodes(fake_parent, [fparser2case_construct]) + assert "to be a Case_Selector but got" in str(error.value) + + +def test_logical_literal_case(fortran_reader, fortran_writer): + '''Test that a select case statement comparing to logical literals + results in if statements with .eqv. checks.''' + code = '''subroutine test_subroutine(a) + logical :: a + + SELECT CASE(A) + CASE(.FALSE.) + print *, "Not hello" + CASE(.TRUE.) + print *, "hello" + END SELECT + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + pass_through = fortran_writer(psyir) + assert "a == .true." not in pass_through + assert "a .EQV. .true." in pass_through + + # Test with an unknown comparison to a literal. + code = '''subroutine test_subroutine() + use mymod, only: a + + SELECT CASE(A) + CASE(.FALSE.) + print *, "Not hello" + CASE(.TRUE.) + print *, "hello" + END SELECT + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + pass_through = fortran_writer(psyir) + assert "a == .true." not in pass_through + assert "a .EQV. .true." in pass_through + + +def test_logical_reference_case(fortran_reader, fortran_writer): + '''Test that a select case statement comparing to logical references + results in if statements with .eqv. checks.''' + code = '''subroutine test_subroutine(a, b, c) + logical :: a, b, c + + SELECT CASE(A) + CASE(b) + print *, "Not hello" + CASE(c) + print *, "hello" + END SELECT + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + pass_through = fortran_writer(psyir) + assert "a == b" not in pass_through + assert "a .EQV. b" in pass_through + assert "a .EQV. c" in pass_through + + +def test_nonlogical_literal_case(fortran_reader, fortran_writer): + '''Test that a select case statement comparing to non-logical literals + results in if statements with == checks.''' + code = '''subroutine test_subroutine(a) + integer :: a + + SELECT CASE(A) + CASE(1) + print *, "Not hello" + CASE(2) + print *, "hello" + END SELECT + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + pass_through = fortran_writer(psyir) + assert "a == 1" in pass_through + assert "a == 2" in pass_through + + +def test_nonlogical_reference_case(fortran_reader, fortran_writer): + '''Test that a select case statement comparing to non-logical references + results in if statements with == checks.''' + code = '''subroutine test_subroutine(a, b, c) + integer :: a, b, c + + SELECT CASE(A) + CASE(b) + print *, "Not hello" + CASE(c) + print *, "hello" + END SELECT + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + pass_through = fortran_writer(psyir) + assert "a == b" in pass_through + assert "a == c" in pass_through + + +def test_expression_case(fortran_reader): + '''Test that a select case statement comparing two expressions + results in a code block.''' + code = '''subroutine test_subroutine(a, b, c) + integer :: a, b, c + + SELECT CASE(a*a) + CASE(b-c) + print *, "Not hello" + CASE(c-b) + print *, "hello" + END SELECT + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + assert isinstance(psyir.children[0].children[0], CodeBlock) + + +def test_unknown_types_case(fortran_reader): + '''Test that a select case statement comparing two unknown types + results in a code block.''' + code = '''subroutine test_subroutine() + use my_mod, only : a, b, c + + SELECT CASE(a) + CASE(b) + print *, "Not hello" + CASE(c) + print *, "hello" + END SELECT + end subroutine''' + + psyir = fortran_reader.psyir_from_source(code) + assert isinstance(psyir.children[0].children[0], CodeBlock) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py index eff5e129c12..e73d2e57813 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_subroutine_handler_test.py @@ -32,6 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Authors: R. W. Ford, A. R. Porter, S. Siso and N. Nobre, STFC Daresbury Lab +# Modified: A. B. G. Chalk, STFC Daresbury Lab '''Module containing pytest tests for the _subroutine_handler method in the class Fparser2Reader. This handler deals with the translation @@ -413,3 +414,75 @@ def test_unsupported_char_len_function(fortran_reader): fsym = psyir.children[0].symbol_table.lookup("my_func") assert isinstance(fsym, RoutineSymbol) assert isinstance(fsym.datatype, DeferredType) + + +def test_unsupported_contains_subroutine(fortran_reader): + '''Test that a Subroutine with Contains results in a Codeblock''' + code = '''subroutine a(b, c, d) + real b, c, d + + b = my_func(c, d) + + contains + real function my_func(a1, a2) + real a1, a2 + my_func = a1 * a2 + end function + end subroutine''' + psyir = fortran_reader.psyir_from_source(code) + cblock = psyir.children[0] + assert isinstance(cblock, CodeBlock) + assert "FUNCTION" in str(cblock.get_ast_nodes[0]) + + code = '''subroutine a(b, c, d) + real b, c, d + + call my_func(c, d) + + contains + subroutine my_func(a1, a2) + real a1, a2 + a1 = a1 * a2 + end subroutine + end subroutine''' + psyir = fortran_reader.psyir_from_source(code) + cblock = psyir.children[0] + assert isinstance(cblock, CodeBlock) + assert "CONTAINS\n SUBROUTINE" in str(cblock.get_ast_nodes[0]) + + +def test_unsupported_contains_function(fortran_reader): + '''Test that a Function with Contains results in a Codeblock''' + code = '''function a(b, c, d) + real b, c, d + + b = my_func(c, d) + a = b * b + + contains + real function my_func(a1, a2) + real a1, a2 + my_func = a1 * a2 + end function + end function''' + psyir = fortran_reader.psyir_from_source(code) + cblock = psyir.children[0] + assert isinstance(cblock, CodeBlock) + assert "CONTAINS\n REAL FUNCTION" in str(cblock.get_ast_nodes[0]) + + code = '''function a(b, c, d) + real b, c, d + + call my_func(c, d) + a = c + d + + contains + subroutine my_func(a1, a2) + real a1, a2 + a1 = a1 * a2 + end subroutine + end function''' + psyir = fortran_reader.psyir_from_source(code) + cblock = psyir.children[0] + assert isinstance(cblock, CodeBlock) + assert "SUBROUTINE" in str(cblock.get_ast_nodes[0]) diff --git a/src/psyclone/tests/psyir/frontend/fparser2_test.py b/src/psyclone/tests/psyir/frontend/fparser2_test.py index 63e5cb41a4c..1562485987f 100644 --- a/src/psyclone/tests/psyir/frontend/fparser2_test.py +++ b/src/psyclone/tests/psyir/frontend/fparser2_test.py @@ -44,7 +44,7 @@ from fparser.common.sourceinfo import FortranFormat from fparser.two import Fortran2003 from fparser.two.Fortran2003 import ( - Assignment_Stmt, Dimension_Attr_Spec, Execution_Part, Name, Return_Stmt, + Dimension_Attr_Spec, Execution_Part, Name, Return_Stmt, Specification_Part, Stmt_Function_Stmt, Subroutine_Subprogram, Type_Declaration_Stmt) from fparser.two.utils import walk @@ -689,6 +689,73 @@ def test_get_routine_schedules_interface(interface_code, parser): assert scheds[1].name.lower() == "dummy_code_64" +@pytest.mark.usefixtures("f2008_parser") +def test_get_partial_datatype(): + '''Test that the _get_partial_datatype method of Fparser2Reader + works as expected. + + ''' + fake_parent = KernelSchedule("dummy_schedule") + processor = Fparser2Reader() + + # Entry in symbol table with unmodified properties. + reader = FortranStringReader("integer :: l1") + node = Specification_Part(reader).content[0] + ids = [id(entry) for entry in walk(node)] + datatype = processor._get_partial_datatype(node, fake_parent, {}) + assert isinstance(datatype, ScalarType) + assert datatype.intrinsic is ScalarType.Intrinsic.INTEGER + # Check fparser2 tree is unmodified + assert ids == [id(entry) for entry in walk(node)] + + # Entry in symbol table with partial information. Example has one + # unsupported attribute (and no others) and an unsupported assignment. + reader = FortranStringReader("integer, pointer :: l1 => null()") + node = Specification_Part(reader).content[0] + ids = [id(entry) for entry in walk(node)] + datatype = processor._get_partial_datatype(node, fake_parent, {}) + assert isinstance(datatype, ScalarType) + assert datatype.intrinsic is ScalarType.Intrinsic.INTEGER + # Check fparser2 tree is unmodified + assert ids == [id(entry) for entry in walk(node)] + + # Entry in symbol table with partial information. Example has one + # unsupported attribute and one supported attribute. + reader = FortranStringReader("real*4, target, dimension(10,20) :: l1") + node = Specification_Part(reader).content[0] + ids = [id(entry) for entry in walk(node)] + datatype = processor._get_partial_datatype(node, fake_parent, {}) + assert isinstance(datatype, ArrayType) + assert datatype.intrinsic is ScalarType.Intrinsic.REAL + assert datatype.precision == 4 + assert datatype.shape[0].upper.value == '10' + assert datatype.shape[1].upper.value == '20' + # Check fparser2 tree is unmodified + assert ids == [id(entry) for entry in walk(node)] + + # No entry in symbol table. + # Notice the space before complex keyword. This avoids it being + # treated as a comment. + reader = FortranStringReader(" complex :: c\n") + node = Specification_Part(reader).content[0] + ids = [id(entry) for entry in walk(node)] + assert not processor._get_partial_datatype(node, fake_parent, {}) + # Check fparser2 tree is unmodified + assert ids == [id(entry) for entry in walk(node)] + + # Multiple variables in the declaration are also supported but are + # not used by PSyclone at the moment. + reader = FortranStringReader( + "integer, pointer :: l1 => null(), l2 => null()") + node = Specification_Part(reader).content[0] + ids = [id(entry) for entry in walk(node)] + datatype = processor._get_partial_datatype(node, fake_parent, {}) + assert isinstance(datatype, ScalarType) + assert datatype.intrinsic is ScalarType.Intrinsic.INTEGER + # Check fparser2 tree is unmodified + assert ids == [id(entry) for entry in walk(node)] + + @pytest.mark.usefixtures("f2008_parser") def test_process_declarations(): '''Test that process_declarations method of Fparser2Reader @@ -796,6 +863,28 @@ def test_process_declarations(): "interface" in str(error.value)) +@pytest.mark.usefixtures("f2008_parser") +def test_process_declarations_unknownfortrantype(): + '''Test that process_declarations method of Fparser2Reader adds + datatype information to an UnknownFortranType by calling the + get_partial_datatype method, also from Fparser2Reader. + + ''' + fake_parent = KernelSchedule("dummy_schedule") + symtab = fake_parent.symbol_table + processor = Fparser2Reader() + reader = FortranStringReader( + "integer, pointer :: l1 => null(), l2 => null()") + fparser2spec = Specification_Part(reader).content[0] + processor.process_declarations(fake_parent, [fparser2spec], []) + for varname in ("l1", "l2"): + var_symbol = symtab.lookup(varname) + assert isinstance(var_symbol.datatype, UnknownFortranType) + assert isinstance(var_symbol.datatype.partial_datatype, ScalarType) + assert (var_symbol.datatype.partial_datatype.intrinsic is + ScalarType.Intrinsic.INTEGER) + + @pytest.mark.usefixtures("f2008_parser") def test_process_declarations_errors(): '''Test that process_declarations method of Fparser2Reader @@ -2403,309 +2492,6 @@ def test_handling_complex_if_construct(): assert nested_if2.children[1].children[0].children[0].name == 'found' -@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") -def test_handling_case_construct(): - ''' Test that fparser2 Case_Construct is converted to the expected PSyIR - tree structure. - - TODO #754 fix test so that 'disable_declaration_check' fixture is not - required. - ''' - reader = FortranStringReader( - '''SELECT CASE (selector) - CASE (label1) - branch1 = 1 - CASE (label2) - branch2 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - - fake_parent = Schedule() - processor = Fparser2Reader() - processor.process_nodes(fake_parent, [fparser2case_construct]) - - # Check a new node was properly generated and connected to parent - assert len(fake_parent.children) == 1 - ifnode = fake_parent.children[0] - assert isinstance(ifnode, IfBlock) - assert ifnode.if_body.ast is fparser2case_construct.content[2] - assert ifnode.if_body.ast_end is fparser2case_construct.content[2] - assert 'was_case' in ifnode.annotations - assert ifnode.condition.children[0].name == 'selector' - assert ifnode.condition.children[1].name == 'label1' - assert ifnode.if_body[0].children[0].name == 'branch1' - assert isinstance(ifnode.else_body[0], IfBlock) - assert ifnode.else_body[0].condition.children[1].name == 'label2' - assert ifnode.else_body[0].if_body[0].children[0].name == 'branch2' - assert ifnode.else_body[0].ast is \ - fparser2case_construct.content[4] - assert ifnode.else_body[0].children[1].ast is \ - fparser2case_construct.content[4] - assert ifnode.else_body[0].children[1].ast_end is \ - fparser2case_construct.content[4] - assert len(ifnode.else_body[0].children) == 2 # SELECT CASE ends here - - -@pytest.mark.usefixtures("f2008_parser") -def test_case_default(): - ''' Check that the fparser2Reader handles SELECT blocks with - a default clause. - - ''' - case_clauses = ["CASE default\nbranch3 = 1\nbranch3 = branch3 * 2\n", - "CASE (label1)\nbranch1 = 1\n", - "CASE (label2)\nbranch2 = 1\n"] - - # Create the symbols that the frontend will expect to already be - # present in the symbol table. - symbols = [] - for idx in [1, 2, 3]: - symbols.append(DataSymbol(f"branch{idx}", INTEGER_TYPE)) - for var_name in ["selector", "label1", "label2"]: - symbols.append(DataSymbol(var_name, INTEGER_TYPE)) - - # Loop over the 3 possible locations for the 'default' clause - for idx1, idx2, idx3 in [(0, 1, 2), (1, 0, 2), (1, 2, 0)]: - fortran_text = ( - f"SELECT CASE (selector)\n" - f"{case_clauses[idx1]}{case_clauses[idx2]}{case_clauses[idx3]}" - f"END SELECT\n") - reader = FortranStringReader(fortran_text) - fparser2case_construct = Execution_Part.match(reader)[0][0] - - fake_parent = Schedule() - # Ensure we have the necessary symbols in the symbol table. - for sym in symbols: - fake_parent.symbol_table.add(sym) - - processor = Fparser2Reader() - processor.process_nodes(fake_parent, [fparser2case_construct]) - assigns = fake_parent.walk(Assignment) - # Check that the assignment to 'branch 3' (in the default clause) is - # the deepest in the tree - assert "branch3" in str(assigns[2]) - assert isinstance(assigns[2].ast, Assignment_Stmt) - assert isinstance(assigns[2].parent, Schedule) - assert isinstance(assigns[2].parent.ast, Assignment_Stmt) - assert "branch3 * 2" in str(assigns[2].parent.ast_end) - assert isinstance(assigns[2].parent.parent, IfBlock) - # Check that the if-body of the parent IfBlock also contains - # an Assignment - assert isinstance(assigns[2].parent.parent.children[1], Schedule) - assert isinstance(assigns[2].parent.parent.children[1].children[0], - Assignment) - - -@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") -def test_handling_case_list(): - ''' Test that the Case_Construct handler correctly processes CASE - statements involving a list of conditions. - - TODO #754 fix test so that 'disable_declaration_check' fixture is not - required. - ''' - reader = FortranStringReader( - '''SELECT CASE (my_var) - CASE (label2, label3) - branch2 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - - fake_parent = Schedule() - processor = Fparser2Reader() - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert len(fake_parent.children) == 1 - ifnode = fake_parent.children[0] - assert isinstance(ifnode, IfBlock) - assert isinstance(ifnode.condition, BinaryOperation) - assert ifnode.condition.operator == BinaryOperation.Operator.OR - eqnode = ifnode.condition.children[0] - assert eqnode.operator == BinaryOperation.Operator.EQ - assert "my_var" in str(eqnode.children[0]) - assert "label2" in str(eqnode.children[1]) - eqnode = ifnode.children[0].children[1] - assert eqnode.operator == BinaryOperation.Operator.EQ - assert "my_var" in str(eqnode.children[0]) - assert "label3" in str(eqnode.children[1]) - - assert "Reference[name:'branch2']" in str(ifnode.if_body[0].lhs) - - -@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") -def test_handling_case_range(): - ''' Test that the Case_Construct handler correctly processes CASE - statements involving a range. - - TODO #754 fix test so that 'disable_declaration_check' fixture is not - required. - ''' - reader = FortranStringReader( - '''SELECT CASE (my_var) - CASE (label4:label5) - branch3 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - - fake_parent = Schedule() - processor = Fparser2Reader() - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert len(fake_parent.children) == 1 - ifnode = fake_parent.children[0] - assert isinstance(ifnode, IfBlock) - assert isinstance(ifnode.children[0], BinaryOperation) - assert ifnode.condition.operator == BinaryOperation.Operator.AND - assert ifnode.condition.children[0].operator == BinaryOperation.Operator.GE - assert ifnode.condition.children[1].operator == BinaryOperation.Operator.LE - assert "branch3" in str(ifnode.if_body[0].lhs) - - -@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") -def test_handling_case_range_list(): - ''' Test that the Case_Construct handler correctly processes CASE - statements involving a list of ranges. - - TODO #754 fix test so that 'disable_declaration_check' fixture is not - required. - ''' - reader = FortranStringReader( - '''SELECT CASE (my_var) - CASE (:label1, label5:, label6) - branch4 = 1 - END SELECT''') - # We should end up with: - # my_var <= label1 OR my_var >= label5 OR my_var == label6 - fparser2case_construct = Execution_Part.match(reader)[0][0] - - fake_parent = Schedule() - processor = Fparser2Reader() - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert len(fake_parent.children) == 1 - ifnode = fake_parent.children[0] - assert isinstance(ifnode, IfBlock) - assert isinstance(ifnode.condition, BinaryOperation) - assert ifnode.condition.operator == BinaryOperation.Operator.OR - assert ifnode.condition.children[0].operator == BinaryOperation.Operator.LE - assert "label1" in str(ifnode.condition.children[0].children[1]) - orop = ifnode.condition.children[1] - assert orop.operator == BinaryOperation.Operator.OR - assert orop.children[0].operator == BinaryOperation.Operator.GE - assert "label5" in str(orop.children[0].children[1]) - assert orop.children[1].operator == BinaryOperation.Operator.EQ - assert "label6" in str(orop.children[1].children[1]) - assert "branch4" in str(ifnode.if_body[0].lhs) - - -@pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") -def test_handling_invalid_case_construct(): - ''' Test that the Case_Construct handler raises the proper errors when - it parses invalid or unsupported fparser2 trees. - - TODO #754 fix test so that 'disable_declaration_check' fixture is not - required. - ''' - # CASE (default) is just a regular symbol named default - reader = FortranStringReader( - '''SELECT CASE (selector) - CASE (default) - branch3 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - - fake_parent = Schedule() - processor = Fparser2Reader() - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert isinstance(fake_parent.children[0], IfBlock) - - # Test with no opening Select_Case_Stmt - reader = FortranStringReader( - '''SELECT CASE (selector) - CASE (label1) - branch1 = 1 - CASE (label2) - branch2 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - del fparser2case_construct.content[0] - with pytest.raises(InternalError) as error: - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert "Failed to find opening case statement in:" in str(error.value) - - # Test with no closing End_Select_Stmt - reader = FortranStringReader( - '''SELECT CASE (selector) - CASE (label1) - branch1 = 1 - CASE (label2) - branch2 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - del fparser2case_construct.content[-1] - with pytest.raises(InternalError) as error: - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert "Failed to find closing case statement in:" in str(error.value) - - # Test when one clause is not of the expected type - reader = FortranStringReader( - '''SELECT CASE (selector) - CASE (label1) - branch1 = 1 - CASE (label2) - branch2 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - fparser2case_construct.content[1].items = (Name("Fake"), None) - with pytest.raises(InternalError) as error: - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert "to be a Case_Selector but got" in str(error.value) - - -@pytest.mark.usefixtures("parser") -def test_handling_labelled_case_construct(): - ''' Test that a labelled case construct results in a CodeBlock. ''' - reader = FortranStringReader( - '''999 SELECT CASE (selector) - CASE (pick_me) - branch3 = 1 - END SELECT''') - fparser2case_construct = Execution_Part.match(reader)[0][0] - - fake_parent = Schedule() - fake_parent.symbol_table.new_symbol("selector", symbol_type=DataSymbol, - datatype=INTEGER_TYPE) - fake_parent.symbol_table.new_symbol("pick_me", symbol_type=DataSymbol, - datatype=INTEGER_TYPE) - fake_parent.symbol_table.new_symbol("branch3", symbol_type=DataSymbol, - datatype=INTEGER_TYPE) - processor = Fparser2Reader() - processor.process_nodes(fake_parent, [fparser2case_construct]) - assert len(fake_parent.children) == 1 - assert isinstance(fake_parent.children[0], CodeBlock) - - -@pytest.mark.usefixtures("f2008_parser") -def test_case_default_only(): - ''' Check that we handle a select case that contains only a - default clause and is thus redundant. The PSyIR should represent - only the code that is within the default case. - - ''' - fake_parent = Schedule() - fake_parent.symbol_table.add(Symbol("a")) - processor = Fparser2Reader() - reader = FortranStringReader( - '''SELECT CASE ( jprstlib ) - CASE DEFAULT - WRITE(numout,*) 'open ice restart NetCDF file: ' - a = 1 - END SELECT''') - exe_part = Execution_Part.match(reader) - processor.process_nodes(fake_parent, exe_part[0]) - # We should have no IfBlock in the resulting PSyIR - assert len(fake_parent.children) == 2 - assert isinstance(fake_parent.children[0], CodeBlock) - assert isinstance(fake_parent.children[1], Assignment) - - @pytest.mark.usefixtures("disable_declaration_check", "f2008_parser") def test_handling_binaryopbase(): ''' Test that fparser2 BinaryOpBase is converted to the expected PSyIR @@ -2724,7 +2510,7 @@ def test_handling_binaryopbase(): assert len(new_node.children) == 2 assert new_node._operator == BinaryOperation.Operator.ADD - # Test parsing all supported binary operators. + # Test parsing all supported arithmetic binary operators. testlist = (('+', BinaryOperation.Operator.ADD), ('-', BinaryOperation.Operator.SUB), ('*', BinaryOperation.Operator.MUL), @@ -2742,9 +2528,7 @@ def test_handling_binaryopbase(): ('>=', BinaryOperation.Operator.GE), ('.ge.', BinaryOperation.Operator.GE), ('<=', BinaryOperation.Operator.LE), - ('.LE.', BinaryOperation.Operator.LE), - ('.and.', BinaryOperation.Operator.AND), - ('.or.', BinaryOperation.Operator.OR)) + ('.LE.', BinaryOperation.Operator.LE)) for opstring, expected in testlist: # Manipulate the fparser2 ParseTree so that it contains the operator @@ -2760,6 +2544,25 @@ def test_handling_binaryopbase(): assert fake_parent[0].rhs._operator == expected, \ "Fails when parsing '" + opstring + "'" + # Test parsing all supported logical binary operators. + testlist = (('.and.', BinaryOperation.Operator.AND), + ('.eqv.', BinaryOperation.Operator.EQV), + ('.neqv.', BinaryOperation.Operator.NEQV), + ('.or.', BinaryOperation.Operator.OR)) + for opstring, expected in testlist: + # Manipulate the fparser2 ParseTree so that it contains the operator + # under test + reader = FortranStringReader("x=a" + opstring + ".true.") + fp2binaryop = Execution_Part.match(reader)[0][0] + # And then translate it to PSyIR again. + fake_parent = Schedule() + processor.process_nodes(fake_parent, [fp2binaryop]) + assert len(fake_parent.children) == 1 + assert isinstance(fake_parent[0].rhs, BinaryOperation), \ + "Fails when parsing '" + opstring + "'" + assert fake_parent[0].rhs._operator == expected, \ + "Fails when parsing '" + opstring + "'" + # Test that an unsupported binary operator creates a CodeBlock fake_parent = Schedule() fp2binaryop.items = (fp2binaryop.items[0], fp2binaryop.items[1], diff --git a/src/psyclone/tests/psyir/nodes/node_test.py b/src/psyclone/tests/psyir/nodes/node_test.py index 5c1e28d6443..f5c3b76664b 100644 --- a/src/psyclone/tests/psyir/nodes/node_test.py +++ b/src/psyclone/tests/psyir/nodes/node_test.py @@ -624,6 +624,62 @@ def test_node_ancestor(): assert kern.ancestor(Loop, limit=sched) is kern.parent.parent +def test_node_ancestor_shared_with(fortran_reader): + ''' Test the shared_with parameter of the Node.ancestor() method. ''' + code = '''Subroutine test() + integer :: x + x = 1 + 2 * 3 + End Subroutine test + ''' + psyir = fortran_reader.psyir_from_source(code) + assignment = psyir.children[0].children[0] + add_binop = assignment.rhs + one_lit = add_binop.children[0] + mul_binop = add_binop.children[1] + two_lit = mul_binop.children[0] + three_lit = mul_binop.children[1] + + assert (two_lit.ancestor(BinaryOperation, shared_with=three_lit) is + mul_binop) + assert (two_lit.ancestor(BinaryOperation, shared_with=mul_binop, + include_self=True) is mul_binop) + assert (two_lit.ancestor(BinaryOperation, shared_with=mul_binop, + include_self=False) is add_binop) + assert (two_lit.ancestor(Node, shared_with=one_lit, + excluding=(BinaryOperation)) is assignment) + # Check the inverse of previous statements is the same. + assert (three_lit.ancestor(BinaryOperation, shared_with=two_lit) is + mul_binop) + assert (mul_binop.ancestor(BinaryOperation, shared_with=two_lit, + include_self=True) is mul_binop) + assert (mul_binop.ancestor(BinaryOperation, shared_with=two_lit, + include_self=False) is add_binop) + assert (one_lit.ancestor(Node, shared_with=two_lit, + excluding=(BinaryOperation)) is assignment) + + # Check cases where we don't find a valid ancestor + assert (two_lit.ancestor(BinaryOperation, shared_with=one_lit, + limit=mul_binop) is None) + assert (assignment.ancestor(BinaryOperation, shared_with=one_lit) + is None) + + code2 = '''Subroutine test2() + integer :: x + x = 1 + 2 + End Subroutine test2 + ''' + psyir2 = fortran_reader.psyir_from_source(code2) + assignment2 = psyir2.children[0].children[0] + + # Tests where node (one_list) and the shared_with argument are + # from separate psyir trees, i.e. they have no shared ancestors. + assert one_lit.ancestor(Node, shared_with=assignment2) is None + # Test when supplied a limit argument that is not an ancestor of + # the node (one_lit) but is an ancestor of the shared_with parameter. + assert one_lit.ancestor(Node, shared_with=assignment2, + limit=assignment2.parent) is None + + def test_dag_names(): ''' Test that the dag_name method returns the correct value for the node class and its specialisations. ''' diff --git a/src/psyclone/tests/psyir/nodes/omp_directives_test.py b/src/psyclone/tests/psyir/nodes/omp_directives_test.py index 494a016453d..3d0a5ab93e7 100644 --- a/src/psyclone/tests/psyir/nodes/omp_directives_test.py +++ b/src/psyclone/tests/psyir/nodes/omp_directives_test.py @@ -59,7 +59,7 @@ from psyclone.errors import InternalError, GenerationError from psyclone.transformations import Dynamo0p3OMPLoopTrans, OMPParallelTrans, \ OMPParallelLoopTrans, DynamoOMPParallelLoopTrans, OMPSingleTrans, \ - OMPMasterTrans, OMPTaskloopTrans + OMPMasterTrans, OMPTaskloopTrans, OMPLoopTrans from psyclone.tests.utilities import get_invoke BASE_PATH = os.path.join(os.path.dirname(os.path.dirname(os.path.dirname( @@ -69,9 +69,10 @@ "gocean1p0") -def test_ompparallel_changes_begin_string(fortran_reader): - ''' Check that when the code inside an OMP Parallel region changes, the - parallel clause changes appropriately. ''' +def test_ompparallel_lowering(fortran_reader, monkeypatch): + ''' Check that lowering an OMP Parallel region leaves it with the + appropriate begin_string and clauses for the backend to generate + the right code''' code = ''' subroutine my_subroutine() integer, dimension(320) :: A @@ -101,7 +102,10 @@ def test_ompparallel_changes_begin_string(fortran_reader): assert isinstance(pdir.children[3], OMPFirstprivateClause) priv_clause = pdir.children[2] - # Make acopy of the loop + # If the code inside the region changes after lowering, the next lowering + # will update the clauses appropriately + # TODO 2157: Alternatively, we could invalidate the clauses with an + # upwards signal when changed, or not store them at all. new_loop = pdir.children[0].children[0].children[0].children[0].copy() # Change the loop variable to j jvar = DataSymbol("j", INTEGER_SINGLE_TYPE) @@ -112,10 +116,32 @@ def test_ompparallel_changes_begin_string(fortran_reader): pdir.lower_to_language_level() assert pdir.children[2] is not priv_clause + # Monkeypatch a case with private and firstprivate clauses + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({Symbol("a")}, {Symbol("b")}, None)) + + pdir.lower_to_language_level() + assert isinstance(pdir.children[2], OMPPrivateClause) + assert len(pdir.children[2].children) == 1 + assert pdir.children[2].children[0].name == 'a' + assert isinstance(pdir.children[3], OMPFirstprivateClause) + assert len(pdir.children[3].children) == 1 + assert pdir.children[3].children[0].name == 'b' + + # Monkeypatch a case with shared variables that need synchronisation + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({}, {}, {Symbol("a")})) + with pytest.raises(GenerationError) as err: + pdir.lower_to_language_level() + assert ("Lowering OMPParallelDirective does not support symbols that " + "need synchronisation, but found: ['a']" in str(err.value)) -def test_ompparallel_changes_gen_code(monkeypatch): - ''' Check that when the code inside an OMP Parallel region changes, the - private clause changes appropriately. ''' + +def test_ompparallel_gen_code_clauses(monkeypatch): + ''' Check that the OMP Parallel region clauses are generated + appropriately. ''' + + # Check with an LFRic kernel, the cell variable must be private _, invoke_info = parse(os.path.join(BASE_PATH, "1_single_invoke_w3.f90"), api="dynamo0.3") psy = PSyFactory("dynamo0.3", distributed_memory=False).create(invoke_info) @@ -136,7 +162,8 @@ def test_ompparallel_changes_gen_code(monkeypatch): assert len(pdir.children) == 4 assert "private(cell)" in code - # Make acopy of the loop + # Check that making a change (add private k variable) after the first + # time psy.gen is called recomputes the clauses attributes new_loop = pdir.children[0].children[0].children[0].children[0].copy() routine = pdir.ancestor(Routine) routine.symbol_table.add(DataSymbol("k", INTEGER_SINGLE_TYPE)) @@ -152,20 +179,27 @@ def test_ompparallel_changes_gen_code(monkeypatch): assert "private(cell,k)" in code # Monkeypatch a case with private and firstprivate clauses - pclause = OMPPrivateClause(children=[Reference(Symbol("a"))]) - fpclause = OMPFirstprivateClause(children=[Reference(Symbol("b"))]) - monkeypatch.setattr(pdir, "_get_private_clauses", - lambda: (pclause, fpclause)) + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({Symbol("a")}, {Symbol("b")}, None)) code = str(psy.gen).lower() assert "private(a)" in code assert "firstprivate(b)" in code + # Monkeypatch a case with shared variables that need synchronisation + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({}, {}, {Symbol("a")})) + with pytest.raises(GenerationError) as err: + code = str(psy.gen).lower() + assert ("OMPParallelDirective.gen_code() does not support symbols that " + "need synchronisation, but found: ['a']" in str(err.value)) + -def test_omp_paralleldo_changes_gen_code(monkeypatch): - ''' Check that when the code inside an OMP Parallel Do region changes, the - private clause changes appropriately. Also check that changing the schedule - is correctly picked up.''' +def test_omp_paralleldo_clauses_gen_code(monkeypatch): + ''' Check that the OMP ParallelDo clauses are generated + appropriately. ''' + + # Check with an LFRic kernel, the cell variable must be private _, invoke_info = parse(os.path.join(BASE_PATH, "1_single_invoke_w3.f90"), api="dynamo0.3") psy = PSyFactory("dynamo0.3", distributed_memory=False).create(invoke_info) @@ -182,7 +216,8 @@ def test_omp_paralleldo_changes_gen_code(monkeypatch): assert "schedule(auto)" in code assert "firstprivate" not in code - # Modify the loop + # Check that making a change (add private k variable) after the first + # time psy.gen is called recomputes the clauses attributes routine = pdir.ancestor(Routine) routine.symbol_table.add(DataSymbol("k", INTEGER_SINGLE_TYPE)) # Change the loop variable to k @@ -198,19 +233,26 @@ def test_omp_paralleldo_changes_gen_code(monkeypatch): assert "firstprivate" not in code # Monkeypatch a case with firstprivate clauses - pclause = OMPPrivateClause(children=[Reference(Symbol("a"))]) - fpclause = OMPFirstprivateClause(children=[Reference(Symbol("b"))]) - monkeypatch.setattr(pdir, "_get_private_clauses", - lambda: (pclause, fpclause)) + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({Symbol("a")}, {Symbol("b")}, None)) code = str(psy.gen).lower() assert "private(a)" in code assert "firstprivate(b)" in code + # Monkeypatch a case with shared variables that need synchronisation + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({}, {}, {Symbol("a")})) + with pytest.raises(GenerationError) as err: + code = str(psy.gen).lower() + assert ("OMPParallelDoDirective.gen_code() does not support symbols that " + "need synchronisation, but found: ['a']" in str(err.value)) + -def test_omp_parallel_do_changes_begin_str(fortran_reader): - ''' Check that when the code inside an OMP Parallel Do region changes, the - private clause changes appropriately. ''' +def test_omp_parallel_do_lowering(fortran_reader, monkeypatch): + ''' Check that lowering an OMP Parallel Do leaves it with the + appropriate begin_string and clauses for the backend to generate + the right code''' code = ''' subroutine my_subroutine() integer, dimension(321, 10) :: A @@ -240,7 +282,8 @@ def test_omp_parallel_do_changes_begin_str(fortran_reader): fpriv_clause = pdir.children[3] sched_clause = pdir.children[4] - # Make acopy of the loop + # If the code inside the region changes after lowering, the next lowering + # will update the clauses appropriately routine = pdir.ancestor(Routine) routine.symbol_table.add(DataSymbol("k", INTEGER_SINGLE_TYPE)) # Change the loop variable to j @@ -258,6 +301,26 @@ def test_omp_parallel_do_changes_begin_str(fortran_reader): assert pdir.children[4] is not sched_clause assert isinstance(pdir.children[4], OMPScheduleClause) + # Monkeypatch a case with private and firstprivate clauses + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({Symbol("a")}, {Symbol("b")}, None)) + + pdir.lower_to_language_level() + assert isinstance(pdir.children[2], OMPPrivateClause) + assert len(pdir.children[2].children) == 1 + assert pdir.children[2].children[0].name == 'a' + assert isinstance(pdir.children[3], OMPFirstprivateClause) + assert len(pdir.children[3].children) == 1 + assert pdir.children[3].children[0].name == 'b' + + # Monkeypatch a case with shared variables that need synchronisation + monkeypatch.setattr(pdir, "_infer_sharing_attributes", + lambda: ({}, {}, {Symbol("a")})) + with pytest.raises(GenerationError) as err: + pdir.lower_to_language_level() + assert ("Lowering OMPParallelDoDirective does not support symbols that " + "need synchronisation, but found: ['a']" in str(err.value)) + def test_omp_teams_distribute_parallel_do_strings( fortran_reader, fortran_writer): @@ -531,8 +594,10 @@ def test_omp_do_children_err(): "this Node has a child of type 'Return'" in str(err.value)) -def test_directive_get_private_lfric(): - ''' Tests for the _get_private_clauses() method of OMPParallelDirective. +def test_directive_infer_sharing_attributes_lfric(): + ''' Tests for the _infer_sharing_attributes() method of + OMPParallelDirective containing an LFRic kernel. + Note: this test does not apply colouring so the loops must be over discontinuous function spaces. @@ -558,27 +623,57 @@ def test_directive_get_private_lfric(): # replaced by a `lower_to_language_level` call. # pylint: disable=pointless-statement psy.gen - # Now check that _get_private_clause returns what we expect - pvars, fpvars = directive._get_private_clauses() - assert isinstance(pvars, OMPPrivateClause) - assert isinstance(fpvars, OMPFirstprivateClause) - assert len(pvars.children) == 1 - assert len(fpvars.children) == 0 - assert pvars.children[0].name == 'cell' + # Now check that _infer_sharing_attributes returns what we expect + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert isinstance(pvars, set) + assert isinstance(fpvars, set) + assert len(pvars) == 1 + assert len(fpvars) == 0 + assert len(sync) == 0 + assert list(pvars)[0].name == 'cell' directive.children[1] = OMPDefaultClause( clause_type=OMPDefaultClause.DefaultClauseTypes.NONE) with pytest.raises(GenerationError) as excinfo: - _ = directive._get_private_clauses() + _ = directive._infer_sharing_attributes() assert ("OMPParallelClause cannot correctly generate the private clause " "when its default data sharing attribute in its default clause is " - "not shared." in str(excinfo.value)) + "not 'shared'." in str(excinfo.value)) -def test_directive_get_private(fortran_reader): - ''' Tests for the _get_private_clauses() method of OpenMP directives.''' +def test_directive_infer_sharing_attributes(fortran_reader): + ''' Tests for the _infer_sharing_attributes() method of OpenMP directives + with generic code inside the directive body. + ''' + + # Example with arrays, read-only and only-writen-once variables, this are + # all shared (only the iteration index is private in this parallel region) + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + integer :: i, scalar1, scalar2 + real, dimension(10) :: array + scalar2 = scalar1 + do i = 1, 10 + array(i) = scalar2 + enddo + end subroutine''') + omplooptrans = OMPLoopTrans() + loop = psyir.walk(Loop)[0] + omplooptrans.apply(loop) + omptrans = OMPParallelTrans() + routine = psyir.walk(Routine)[0] + omptrans.apply(routine.children) + directive = psyir.walk(OMPParallelDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 1 + assert list(pvars)[0].name == 'i' + assert len(fpvars) == 0 + assert len(sync) == 0 # Example with private and firstprivate variables on OMPParallelDoDirective + # In this case scalar1 is firstprivate because it has a conditional-write + # that it is not guaranteed to happen on each iteration, so the private + # variable needs to be initialised with the value it has before the loop. psyir = fortran_reader.psyir_from_source(''' subroutine my_subroutine() integer :: i, scalar1, scalar2 @@ -596,15 +691,16 @@ def test_directive_get_private(fortran_reader): loop = psyir.walk(Loop)[0] omplooptrans.apply(loop) directive = psyir.walk(OMPParallelDoDirective)[0] - pvars, fpvars = directive._get_private_clauses() - assert len(pvars.children) == 2 - assert len(fpvars.children) == 1 - assert pvars.children[0].name == 'i' - assert pvars.children[1].name == 'scalar2' - assert fpvars.children[0].name == 'scalar1' - - # Another example with OMPParallelDirective (not actual worksharing) - # and scalars set outside the loop (this should be shared by convention) + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 2 + assert sorted(pvars, key=lambda x: x.name)[0].name == 'i' + assert sorted(pvars, key=lambda x: x.name)[1].name == 'scalar2' + assert len(fpvars) == 1 + assert list(fpvars)[0].name == 'scalar1' + assert len(sync) == 0 + + # Another example with only a OMPParallelDirective (not actual worksharing) + # and scalars set outside the loop (only-written-once), these are shared psyir = fortran_reader.psyir_from_source(''' subroutine my_subroutine() integer :: i, scalar1, scalar2, scalar3, scalar4 @@ -616,17 +712,231 @@ def test_directive_get_private(fortran_reader): array(i) = scalar2 enddo end subroutine''') - omplooptrans = OMPParallelTrans() - loop = psyir.walk(Routine)[0] - omplooptrans.apply(loop.children) + omptrans = OMPParallelTrans() + routine = psyir.walk(Routine)[0] + omptrans.apply(routine.children) + directive = psyir.walk(OMPParallelDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 2 + assert len(fpvars) == 0 + assert len(sync) == 0 + assert sorted(pvars, key=lambda x: x.name)[0].name == 'i' + assert sorted(pvars, key=lambda x: x.name)[1].name == 'scalar2' + # scalar1 is shared because is read-only and scalar3 and scalar4 are + # shared because they are set outside a loop (only written once) + + # Another example with only a OMPParallelDirective (not actual worksharing) + # and one scalar (scalar2) it is used as a private variable inside the loop + # but it is first read before the loop, and therefore it should be + # firstprivate + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + integer :: i, scalar1, scalar2, scalar3 + real, dimension(10) :: array + scalar3 = scalar2 + do i = 1, 10 + scalar2 = scalar1 + scalar3 + array(i) + array(i) = scalar2 + enddo + end subroutine''') + omptrans = OMPParallelTrans() + routine = psyir.walk(Routine)[0] + omptrans.apply(routine.children) + directive = psyir.walk(OMPParallelDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 1 + assert len(fpvars) == 1 + assert len(sync) == 0 + assert list(pvars)[0].name == 'i' + assert list(fpvars)[0].name == 'scalar2' + + # Similar but with a OMPParallelDoDirective and the firstprivate is + # in the same loop but before the loop body + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + integer :: i, scalar1 + real, dimension(10) :: array + do i = 1, 10, scalar1 + scalar1 = array(i) + array(i) = scalar1 + enddo + end subroutine''') + omplooptrans = OMPParallelLoopTrans() + loop = psyir.walk(Loop)[0] + omplooptrans.apply(loop, options={'force': True}) + directive = psyir.walk(OMPParallelDoDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 1 + assert len(fpvars) == 1 + assert len(sync) == 0 + assert list(pvars)[0].name == 'i' + assert list(fpvars)[0].name == 'scalar1' + + # In this example the scalar2 variable is shared but it needs + # synchronisation to avoid race conditions (write-after-read + # in the same statement) + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + integer :: i, scalar1, scalar2 + real, dimension(10) :: array + do i = 1, 10 + scalar2 = scalar2 + scalar1 + enddo + end subroutine''') + omplooptrans = OMPParallelLoopTrans() + loop = psyir.walk(Loop)[0] + omplooptrans.apply(loop, options={"force": True}) + directive = psyir.walk(OMPParallelDoDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 1 + assert list(pvars)[0].name == 'i' + assert len(fpvars) == 0 + assert len(sync) == 1 + assert list(sync)[0].name == 'scalar2' + + # In this example the scalar2 variable is shared but it needs + # synchronisation to avoid race conditions (write-after-read + # in different statements) + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + integer :: i, scalar1, scalar2, tmp + real, dimension(10) :: array + do i = 1, 10 + tmp = scalar2 + scalar1 + scalar2 = tmp + enddo + end subroutine''') + omplooptrans = OMPParallelLoopTrans() + loop = psyir.walk(Loop)[0] + omplooptrans.apply(loop, options={"force": True}) + directive = psyir.walk(OMPParallelDoDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 2 + assert sorted(pvars, key=lambda x: x.name)[0].name == 'i' + assert sorted(pvars, key=lambda x: x.name)[1].name == 'tmp' + assert len(fpvars) == 0 + assert len(sync) == 1 + assert list(sync)[0].name == 'scalar2' + + # Example tiling routine (k is a reduction - needs sync) + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine(k) + integer :: i, ii + integer :: j, jj + integer :: k + do i = 1, 320, 32 + do j = 1, 320, 32 + do ii=i, i+32 + do jj = j,j+32 + k = k + ii + k = k * jj + end do + end do + end do + end do + end subroutine''') + ptrans = OMPParallelTrans() + loop = psyir.walk(Loop)[0] + ptrans.apply(loop) directive = psyir.walk(OMPParallelDirective)[0] - pvars, fpvars = directive._get_private_clauses() - assert len(pvars.children) == 2 - assert len(fpvars.children) == 0 - assert pvars.children[0].name == 'i' - assert pvars.children[1].name == 'scalar2' - # scalar 1 is shared because is read-only and scalar3 and scalar4 are - # shared because they are set outside a loop + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 4 + assert sorted(pvars, key=lambda x: x.name)[0].name == 'i' + assert sorted(pvars, key=lambda x: x.name)[1].name == 'ii' + assert sorted(pvars, key=lambda x: x.name)[2].name == 'j' + assert sorted(pvars, key=lambda x: x.name)[3].name == 'jj' + assert len(fpvars) == 0 + assert len(sync) == 1 + assert list(sync)[0].name == 'k' + + +def test_directive_infer_sharing_attributes_with_structures(fortran_reader): + ''' Tests for the _infer_sharing_attributes() method of OpenMP directives + with code that contains structure accesses. + ''' + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + use my_mod + integer :: i, scalar1 + type(my_type) :: mt1, mt2 + real, dimension(10) :: array + mt1%scalar1 = 3 + do i = 1, 10 + if (i .eq. 4) then + mt2%field1%scalar1 = array(i) + endif + scalar1 = mt2%field1%scalar1 + mt1%scalar1 + array(i) = scalar1 + enddo + end subroutine''') + omptrans = OMPParallelTrans() + routine = psyir.walk(Routine)[0] + omptrans.apply(routine.children) + directive = psyir.walk(OMPParallelDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 2 + assert sorted(pvars, key=lambda x: x.name)[0].name == 'i' + assert sorted(pvars, key=lambda x: x.name)[1].name == 'scalar1' + assert len(fpvars) == 1 + assert list(fpvars)[0].name == 'mt2' + assert len(sync) == 0 + + # In this example a sub-part of mt1 should be shared and another + # firstprivate + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + use my_mod + integer :: i, scalar1 + type(my_type) :: mt1 + real, dimension(10) :: array + do i = 1, 10 + if (i .eq. 4) then + mt1%scalar1 = 3 + endif + mt1%array(i) = mt1%scalar1 + enddo + end subroutine''') + omptrans = OMPParallelTrans() + routine = psyir.walk(Routine)[0] + omptrans.apply(routine.children) + directive = psyir.walk(OMPParallelDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 1 + assert list(pvars)[0].name == 'i' + assert len(fpvars) == 1 + if list(fpvars)[0].name == 'mt2': + pytest.xfail("#2094: Currently we only support top-level derived types" + "as OpenMP sharing attributes, but there are cases that " + "more detail is necessary.") + + +def test_infer_sharing_attributes_sequential_semantics(fortran_reader): + ''' _infer_sharing_attributes() tries to conserve the same semantics + as the sequential loop, however, for loops that are not possible to + parallelise (but we force the transformation anyway). For now this + may return different results than the original code. + + #TODO #598: This could be a lastprivate? + ''' + + # In this example the result will take the value of the last i in + # sequential order, but an arbitrary i in parallel. + psyir = fortran_reader.psyir_from_source(''' + subroutine my_subroutine() + integer :: i, result + do i = 1, 10 + result = i + enddo + end subroutine''') + omplooptrans = OMPParallelLoopTrans() + loop = psyir.walk(Loop)[0] + omplooptrans.apply(loop, options={"force": True}) + directive = psyir.walk(OMPParallelDoDirective)[0] + pvars, fpvars, sync = directive._infer_sharing_attributes() + assert len(pvars) == 1 + assert list(pvars)[0].name == 'i' + assert len(fpvars) == 0 + assert len(sync) == 0 def test_directive_lastprivate(fortran_reader, fortran_writer): diff --git a/src/psyclone/tests/psyir/symbols/datatype_test.py b/src/psyclone/tests/psyir/symbols/datatype_test.py index 6480958b46e..bb6cdf15b01 100644 --- a/src/psyclone/tests/psyir/symbols/datatype_test.py +++ b/src/psyclone/tests/psyir/symbols/datatype_test.py @@ -43,9 +43,9 @@ from psyclone.psyir.nodes import Literal, BinaryOperation, Reference, \ Container, KernelSchedule from psyclone.psyir.symbols import ( - ArrayType, DataType, DeferredType, ScalarType, UnknownType, - UnknownFortranType, DataSymbol, StructureType, NoType, - INTEGER_TYPE, REAL_TYPE, Symbol, DataTypeSymbol, SymbolTable) + ArrayType, DataType, DeferredType, ScalarType, UnknownFortranType, + DataSymbol, StructureType, NoType, INTEGER_TYPE, REAL_TYPE, Symbol, + DataTypeSymbol, SymbolTable) # Abstract DataType class @@ -565,23 +565,6 @@ def test_arraytype_eq(): assert data_type1 != ArrayType(iscalar_type, [10, 10]) -# UnknownType tests - -def test_unknown_type_declaration_setter(): - '''Test the declaration setter/getter in UnknownType. We have to subclass - UnknownType as it is virtual.''' - - class HardType(UnknownType): - '''Concrete sub-class of UnknownType for testing.''' - def __str__(self): - return "HardType" - - htype = HardType("real var2;") - assert htype.declaration == "real var2;" - htype.declaration = "real var;" - assert htype.declaration == "real var;" - - # UnknownFortranType tests def test_unknown_fortran_type(): @@ -593,8 +576,35 @@ def test_unknown_fortran_type(): "string but got an argument of type 'int'" in str(err.value)) decl = "type(some_type) :: var" utype = UnknownFortranType(decl) - assert str(utype) == "UnknownFortranType('" + decl + "')" - assert utype.declaration == decl + assert utype._type_text == "" + assert utype._partial_datatype is None + assert str(utype) == f"UnknownFortranType('{decl}')" + assert utype._declaration == decl + + +def test_unknown_fortran_type_optional_arg(): + '''Check the optional 'partial_datatype' argument of the + UnknownFortranType class works as expected. Also check the getter + method and the string methods work as expected when + partial_datatype information is supplied. + + ''' + decl = "type(some_type) :: var" + with pytest.raises(TypeError) as err: + _ = UnknownFortranType(decl, partial_datatype="invalid") + assert ("partial_datatype argument in UnknownFortranType initialisation " + "should be a DataType, DataTypeSymbol, or NoneType, but found " + "'str'." in str(err.value)) + utype = UnknownFortranType(decl, partial_datatype=None) + assert utype._partial_datatype is None + assert utype.partial_datatype is None + + utype = UnknownFortranType( + decl, partial_datatype=DataTypeSymbol("some_type", DeferredType())) + assert isinstance(utype._partial_datatype, DataTypeSymbol) + assert isinstance(utype.partial_datatype, DataTypeSymbol) + assert utype.partial_datatype.name == "some_type" + assert str(utype) == f"UnknownFortranType('{decl}')" def test_unknown_fortran_type_text(): @@ -609,9 +619,6 @@ def test_unknown_fortran_type_text(): # Calling it a second time should just return the previously cached # result. assert utype.type_text is text - # Changing the declaration text should wipe the cache - utype.declaration = decl - assert utype.type_text is not text def test_unknown_fortran_type_eq(): diff --git a/src/psyclone/tests/psyir/symbols/symbol_table_test.py b/src/psyclone/tests/psyir/symbols/symbol_table_test.py index 06cbbf1463c..60a9cfc4f59 100644 --- a/src/psyclone/tests/psyir/symbols/symbol_table_test.py +++ b/src/psyclone/tests/psyir/symbols/symbol_table_test.py @@ -1547,7 +1547,8 @@ def test_deep_copy(): sym1 = DataSymbol("symbol1", INTEGER_TYPE, interface=ArgumentInterface( ArgumentInterface.Access.READ)) - sym2 = Symbol("symbol2", interface=ImportInterface(mod)) + sym2 = Symbol("symbol2", interface=ImportInterface(mod, + orig_name="altsym2")) sym3 = DataSymbol("symbol3", INTEGER_TYPE) symtab.add(mod) symtab.add(sym1) @@ -1576,6 +1577,8 @@ def test_deep_copy(): # ContainerSymbols have been updated assert symtab2.lookup("symbol2").interface.container_symbol is \ symtab2.lookup("my_mod") + # Check that the orig_name is copied across. + assert symtab2.lookup("symbol2").interface.orig_name == "altsym2" # Add new symbols and rename symbols in both symbol tables and check # they are not added/renamed in the other symbol table diff --git a/src/psyclone/tests/psyir/tools/dependency_tools_test.py b/src/psyclone/tests/psyir/tools/dependency_tools_test.py index 61da0cc40d9..8e018e55645 100644 --- a/src/psyclone/tests/psyir/tools/dependency_tools_test.py +++ b/src/psyclone/tests/psyir/tools/dependency_tools_test.py @@ -32,7 +32,7 @@ # POSSIBILITY OF SUCH DAMAGE. # ----------------------------------------------------------------------------- # Author J. Henrichs, Bureau of Meteorology -# Modifications: A. R. Porter and R. W. Ford, STFC Daresbury Lab +# Modifications: A. R. Porter, R. W. Ford and S.Siso, STFC Daresbury Lab ''' Module containing tests for the dependency tools.''' @@ -44,7 +44,6 @@ from psyclone.core import Signature, VariablesAccessInfo from psyclone.errors import InternalError from psyclone.psyGen import PSyFactory -from psyclone.psyir.backend.fortran import FortranWriter from psyclone.psyir.tools import DependencyTools, DTCode, ReadWriteInfo from psyclone.tests.utilities import get_invoke @@ -573,7 +572,7 @@ def test_scalar_parallelise(declaration, variable, fortran_reader): end program test''' psyir = fortran_reader.psyir_from_source(source) loops = psyir.children[0].children - dep_tools = DependencyTools(language_writer=FortranWriter()) + dep_tools = DependencyTools() jj_symbol = psyir.children[0].scope.symbol_table.lookup("jj") diff --git a/src/psyclone/tests/psyir/transformations/transformations_test.py b/src/psyclone/tests/psyir/transformations/transformations_test.py index 4f56d0a6b75..e669daf1069 100644 --- a/src/psyclone/tests/psyir/transformations/transformations_test.py +++ b/src/psyclone/tests/psyir/transformations/transformations_test.py @@ -494,47 +494,6 @@ def test_omplooptrans_apply_firstprivate(fortran_reader, fortran_writer, assert expected in gen assert Compile(tmpdir).string_compiles(gen) - # Example with a read before write - psyir = fortran_reader.psyir_from_source(''' - module my_mod - contains - subroutine my_subroutine() - integer :: ji, jj, jk, jpkm1, jpjm1, jpim1, scalar1, scalar2 - real, dimension(10, 10, 10) :: zwt, zwd, zwi, zws - do jk = 2, jpkm1, 1 - do jj = 2, jpjm1, 1 - do ji = 2, jpim1, 1 - scalar2 = scalar1 + zwt(ji,jj,jk) - scalar1 = 3 - zws(ji,jj,jk) = scalar2 + scalar1 - enddo - enddo - enddo - end subroutine - end module my_mod''') - omplooptrans = OMPParallelLoopTrans() - loop = psyir.walk(Loop)[0] - # This need to be forced since the DependencyAnalysis wrongly considers - # it a reduction - omplooptrans.apply(loop, options={"force": True}) - expected = '''\ - !$omp parallel do default(shared), private(ji,jj,jk,scalar2), \ -firstprivate(scalar1), schedule(auto) - do jk = 2, jpkm1, 1 - do jj = 2, jpjm1, 1 - do ji = 2, jpim1, 1 - scalar2 = scalar1 + zwt(ji,jj,jk) - scalar1 = 3 - zws(ji,jj,jk) = scalar2 + scalar1 - enddo - enddo - enddo - !$omp end parallel do''' - - gen = fortran_writer(psyir) - assert expected in gen - assert Compile(tmpdir).string_compiles(gen) - def test_omplooptrans_apply_firstprivate_fail(fortran_reader): ''' Test applying the OMPLoopTrans in cases where a firstprivate