From 6dd417dd5bc759baf43878f19ca36bd5a85ccb43 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 15 Jan 2026 14:21:01 +1100 Subject: [PATCH 1/2] #469 Removed the creation of parsable_x90 files. Fixes #458. --- source/fab/parse/fortran_common.py | 6 +- source/fab/steps/psyclone.py | 79 ++----------------- .../test_FortranDependencies.py | 10 +-- .../psyclone/test_psyclone_system_test.py | 31 +------- .../parse/fortran/test_fortran_analyser.py | 25 +++--- 5 files changed, 33 insertions(+), 118 deletions(-) diff --git a/source/fab/parse/fortran_common.py b/source/fab/parse/fortran_common.py index b145aeb70..0a78ab506 100644 --- a/source/fab/parse/fortran_common.py +++ b/source/fab/parse/fortran_common.py @@ -127,8 +127,12 @@ def run(self, fpath: Path) \ return analysed_file, analysis_fpath def _get_analysis_fpath(self, fpath, file_hash) -> Path: + """ + :returns: the name to use for the analysis file. It consists of + the original filename, the hash, and a `.an` suffix. + """ return Path(self.config.prebuild_folder / - f'{fpath.stem}.{file_hash}.an') + f'{fpath.name}.{file_hash}.an') def _parse_file(self, fpath): """Get a node tree from a fortran file.""" diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index aea75cfd0..10923a165 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -10,7 +10,6 @@ """ from dataclasses import dataclass import logging -import re import shutil import warnings from itertools import chain @@ -194,19 +193,15 @@ def _generate_mp_payload(config, analysed_x90, all_kernel_hashes, overrides_fold def _analyse_x90s(config: BuildConfig, x90s: Set[Path]) -> Dict[Path, AnalysedX90]: """ - Analyse parsable versions of the x90s, finding kernel dependencies. + Analyse the x90s, finding kernel dependencies. """ - # make parsable - todo: fast enough not to require prebuilds? - with TimerLogger(f"converting {len(x90s)} x90s into parsable fortran"): - parsable_x90s = run_mp(config, items=x90s, func=make_parsable_x90) - # Parse. Note that there is no need to ignore dependencies: the x90 # files will be converted to algorithm layer f90 files, and then # properly analysed later. x90_analyser = X90Analyser(config=config) - with TimerLogger(f"analysing {len(parsable_x90s)} parsable x90 files"): - x90_results = run_mp(config, items=parsable_x90s, func=x90_analyser.run) + with TimerLogger(f"analysing {len(x90s)} x90 files"): + x90_results = run_mp(config, items=x90s, func=x90_analyser.run) log_or_dot_finish(logger) x90_analyses, x90_artefacts = zip(*x90_results) if x90_results else ((), ()) check_for_errors(results=x90_analyses) @@ -215,11 +210,11 @@ def _analyse_x90s(config: BuildConfig, prebuild_files = list(by_type(x90_artefacts, Path)) config.add_current_prebuilds(prebuild_files) - # record the analysis results against the original x90 filenames (not the parsable versions we analysed) + # record the analysis results against the original x90 filenames analysed_x90 = by_type(x90_analyses, AnalysedX90) analysed_x90 = {result.fpath.with_suffix('.x90'): result for result in analysed_x90} - # make the hashes from the original x90s, not the parsable versions which have invoke names removed. + # make the hashes from the x90s for p in analysed_x90: analysed_x90[p]._file_hash = file_checksum(p).file_hash @@ -259,7 +254,6 @@ def _analyse_kernels( file_lists = [list(file_walk(root, ignore_folders=[config.prebuild_folder])) for root in kernel_roots] all_kernel_files: Set[Path] = set(sum(file_lists, [])) kernel_files: List[Path] = suffix_filter(all_kernel_files, ['.f90']) - # We use the normal Fortran analyser, which records psyclone kernel metadata. # todo: We'd like to separate that from the general fortran analyser at some point, to reduce coupling. # The Analyse step also uses the same fortran analyser. It stores its results so they won't be analysed twice. @@ -393,7 +387,7 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): # todo: hash the psyclone version in case the built-in kernels change? prebuild_hash = sum([ - # the hash of the x90 (not of the parsable version, so includes invoke names) + # the hash of the x90 analysis_result.file_hash, # the hashes of the kernels used by this x90 @@ -437,64 +431,3 @@ def _check_override(check_path: Path, mp_payload: MpCommonArgs): # we didn't have an override, so continue using this file return check_path - - -# regex to convert an x90 into parsable fortran, so it can be analysed using a third party tool - -WHITE = r'[\s&]+' -OPT_WHITE = r'[\s&]*' - -SQ_STRING = "'[^']*'" -DQ_STRING = '"[^"]*"' -STRING = f'({SQ_STRING}|{DQ_STRING})' - -NAME_KEYWORD = 'name' + OPT_WHITE + '=' + OPT_WHITE + STRING + OPT_WHITE + ',' + OPT_WHITE -NAMED_INVOKE = 'call' + WHITE + 'invoke' + OPT_WHITE + r'\(' + OPT_WHITE + NAME_KEYWORD - -_x90_compliance_pattern = None - - -# todo: In the future, we'd like to extend fparser to handle the leading invoke keywords. (Lots of effort.) -def make_parsable_x90(x90_path: Path) -> Path: - """ - Take out the leading name keyword in calls to invoke(), making temporary, parsable fortran from x90s. - - If present it looks like this:: - - call invoke( name = "compute_dry_mass", ... - - Returns the path of the parsable file. - - This function is not slow so we're not creating prebuilds for this work. - - """ - global _x90_compliance_pattern - if not _x90_compliance_pattern: - _x90_compliance_pattern = re.compile(pattern=NAMED_INVOKE) - - # src = open(x90_path, 'rt').read() - - # Before we remove the name keywords to invoke, we must remove any comment lines. - # This is the simplest way to avoid producing bad fortran when the name keyword is followed by a comment line. - # I.e. The comment line doesn't have an "&", so we get "call invoke(!" with no "&", which is a syntax error. - src_lines = open(x90_path, 'rt').readlines() - no_comment_lines = [line for line in src_lines if not line.lstrip().startswith('!')] - src = ''.join(no_comment_lines) - - replaced = [] - - def repl(matchobj): - # matchobj[0] contains the entire matching string, from "call" to the "," after the name keyword. - # matchobj[1] contains the single group in the search pattern, which is defined in STRING. - name = matchobj[1].replace('"', '').replace("'", "") - replaced.append(name) - return 'call invoke(' - - out = _x90_compliance_pattern.sub(repl=repl, string=src) - - out_path = x90_path.with_suffix('.parsable_x90') - open(out_path, 'wt').write(out) - - logger.debug(f'names removed from {str(x90_path)}: {replaced}') - - return out_path diff --git a/tests/system_tests/FortranDependencies/test_FortranDependencies.py b/tests/system_tests/FortranDependencies/test_FortranDependencies.py index 627e403f2..75fb6d36f 100644 --- a/tests/system_tests/FortranDependencies/test_FortranDependencies.py +++ b/tests/system_tests/FortranDependencies/test_FortranDependencies.py @@ -50,29 +50,29 @@ def test_fortran_dependencies(tmp_path): } # check the analysis results - assert AnalysedFortran.load(config.prebuild_folder / 'first.193489053.an') == AnalysedFortran( + assert AnalysedFortran.load(config.prebuild_folder / 'first.f90.193489053.an') == AnalysedFortran( fpath=config.build_output / 'first.f90', file_hash=193489053, program_defs={'first'}, module_defs=None, symbol_defs={'first'}, module_deps={'greeting_mod', 'constants_mod'}, symbol_deps={'greeting_mod', 'constants_mod', 'greet'}) - assert AnalysedFortran.load(config.prebuild_folder / 'two.2557739057.an') == AnalysedFortran( + assert AnalysedFortran.load(config.prebuild_folder / 'two.f90.2557739057.an') == AnalysedFortran( fpath=config.build_output / 'two.f90', file_hash=2557739057, program_defs={'second'}, module_defs=None, symbol_defs={'second'}, module_deps={'constants_mod', 'bye_mod'}, symbol_deps={'constants_mod', 'bye_mod', 'farewell'}) - assert AnalysedFortran.load(config.prebuild_folder / 'greeting_mod.62446538.an') == AnalysedFortran( + assert AnalysedFortran.load(config.prebuild_folder / 'greeting_mod.f90.62446538.an') == AnalysedFortran( fpath=config.build_output / 'greeting_mod.f90', file_hash=62446538, module_defs={'greeting_mod'}, symbol_defs={'greeting_mod'}, module_deps={'constants_mod'}, symbol_deps={'constants_mod'}) - assert AnalysedFortran.load(config.prebuild_folder / 'bye_mod.3332267073.an') == AnalysedFortran( + assert AnalysedFortran.load(config.prebuild_folder / 'bye_mod.f90.3332267073.an') == AnalysedFortran( fpath=config.build_output / 'bye_mod.f90', file_hash=3332267073, module_defs={'bye_mod'}, symbol_defs={'bye_mod'}, module_deps={'constants_mod'}, symbol_deps={'constants_mod'}) - assert AnalysedFortran.load(config.prebuild_folder / 'constants_mod.233796393.an') == AnalysedFortran( + assert AnalysedFortran.load(config.prebuild_folder / 'constants_mod.f90.233796393.an') == AnalysedFortran( fpath=config.build_output / 'constants_mod.f90', file_hash=233796393, module_defs={'constants_mod'}, symbol_defs={'constants_mod'}, module_deps=None, symbol_deps=None) diff --git a/tests/system_tests/psyclone/test_psyclone_system_test.py b/tests/system_tests/psyclone/test_psyclone_system_test.py index 90998f9a3..5154acaa5 100644 --- a/tests/system_tests/psyclone/test_psyclone_system_test.py +++ b/tests/system_tests/psyclone/test_psyclone_system_test.py @@ -3,10 +3,7 @@ # For further details please refer to the file COPYRIGHT # which you should have received as part of this distribution # ############################################################################## -import filecmp -from os import unlink from pathlib import Path -import shutil from typing import Any, Dict from unittest import mock @@ -19,8 +16,7 @@ from fab.steps.grab.folder import grab_folder from fab.steps.preprocess import preprocess_fortran from fab.steps.psyclone import (_analyse_x90s, _analyse_kernels, - make_parsable_x90, preprocess_x90, - psyclone) + preprocess_x90, psyclone) from fab.tools.psyclone import Psyclone from fab.tools.tool_box import ToolBox from fab.util import file_checksum @@ -41,29 +37,6 @@ # Make the skeleton sample call more than one kernel. # Make the skeleton sample include a big X90 and a little x90. - -def test_make_parsable_x90(tmp_path): - # turn x90 into parsable fortran by removing the name keyword from calls to invoke - grab_x90_path = SAMPLE_X90 - input_x90_path = tmp_path / grab_x90_path.name - shutil.copy(grab_x90_path, input_x90_path) - - parsable_x90_path = make_parsable_x90(input_x90_path) - - with BuildConfig('proj', ToolBox(), fab_workspace=tmp_path) as config: - x90_analyser = X90Analyser(config=config) - x90_analyser.run(parsable_x90_path) - - # ensure the files are as expected - assert filecmp.cmp(parsable_x90_path, EXPECT_PARSABLE_X90) - - # make_parsable_x90() puts its output file next to the source. - # Because we're reading sample code from our Fab git repos, - # we don't want to leave this test output in our working copies, so delete it. - # Otherwise, it'll appear in the output from `git status`. - unlink(parsable_x90_path) - - class TestX90Analyser: expected_analysis_result = AnalysedX90( @@ -106,7 +79,7 @@ def test_analyse(self, tmp_path): # analysed_x90 assert analysed_x90 == { SAMPLE_X90: AnalysedX90( - fpath=SAMPLE_X90.with_suffix('.parsable_x90'), + fpath=SAMPLE_X90, file_hash=file_checksum(SAMPLE_X90).file_hash, kernel_deps={'kernel_one_type', 'kernel_two_type'})} diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.py b/tests/unit_tests/parse/fortran/test_fortran_analyser.py index b3f8e81db..d92fa381d 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.py +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.py @@ -9,7 +9,6 @@ from pathlib import Path -from tempfile import NamedTemporaryFile from unittest import mock from fparser.common.readfortran import FortranFileReader # type: ignore @@ -75,8 +74,9 @@ def test_module_file(self, fortran_analyser, module_fpath, with mock.patch('fab.parse.AnalysedFile.save'): analysis, artefact = fortran_analyser.run(fpath=module_fpath) assert analysis == module_expected - assert artefact == (fortran_analyser._config.prebuild_folder / - f'test_fortran_analyser.{analysis.file_hash}.an') + assert artefact == ( + fortran_analyser._config.prebuild_folder / + f'test_fortran_analyser.f90.{analysis.file_hash}.an') def test_module_file_no_openmp(self, fortran_analyser: FortranAnalyser, module_fpath: Path, @@ -95,8 +95,9 @@ def test_module_file_no_openmp(self, fortran_analyser: FortranAnalyser, assert analysis == module_expected assert isinstance(analysis, AnalysedFortran) - assert artefact == (fortran_analyser._config.prebuild_folder / - f'test_fortran_analyser.{analysis.file_hash}.an') + assert artefact == ( + fortran_analyser._config.prebuild_folder / + f'test_fortran_analyser.f90.{analysis.file_hash}.an') def test_module_file_ignore_dependencies( self, @@ -122,15 +123,18 @@ def test_module_file_ignore_dependencies( assert analysis == module_expected assert isinstance(analysis, AnalysedFortran) - assert artefact == (fortran_analyser._config.prebuild_folder / - f'test_fortran_analyser.{analysis.file_hash}.an') + assert artefact == ( + fortran_analyser._config.prebuild_folder / + f'test_fortran_analyser.f90.{analysis.file_hash}.an') def test_program_file(self, + tmp_path: Path, fortran_analyser: FortranAnalyser, module_fpath: Path, module_expected: AnalysedFortran) -> None: # same as test_module_file() but replacing MODULE with PROGRAM - with NamedTemporaryFile(mode='w+t', suffix='.f90') as tmp_file: + prog_path = tmp_path / "prog.f90" + with prog_path.open('w') as tmp_file: tmp_file.write(module_fpath.open().read().replace("MODULE", "PROGRAM")) tmp_file.flush() @@ -148,8 +152,9 @@ def test_program_file(self, assert analysis == module_expected assert isinstance(analysis, AnalysedFortran) - assert artefact == fortran_analyser._config.prebuild_folder \ - / f'{Path(tmp_file.name).stem}.{analysis.file_hash}.an' + assert artefact == ( + fortran_analyser._config.prebuild_folder / + f'{prog_path.name}.{analysis.file_hash}.an') # todo: test more methods! From 9f4c8d456564808c6337f9aca8a29198b3b05127 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 15 Jan 2026 14:39:03 +1100 Subject: [PATCH 2/2] #469 Fixed comment. --- source/fab/steps/psyclone.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/fab/steps/psyclone.py b/source/fab/steps/psyclone.py index 10923a165..b1e591f4f 100644 --- a/source/fab/steps/psyclone.py +++ b/source/fab/steps/psyclone.py @@ -367,7 +367,7 @@ def _gen_prebuild_hash(x90_file: Path, mp_payload: MpCommonArgs): - cli args """ - # We've analysed (a parsable version of) this x90. + # We've analysed this x90. analysis_result = mp_payload.analysed_x90[x90_file] # type: ignore # include the hashes of kernels used by this x90