From 90e3795cb6ff5ca5de01970baab01f04aca92b8e Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 21 Mar 2024 00:11:16 +1100 Subject: [PATCH 1/5] Extend Fortran analyser to detect use statements with an OpenMP sentinel. --- source/fab/parse/fortran.py | 24 ++++++++++++++++++- .../parse/fortran/test_fortran_analyser.f90 | 5 ++++ .../parse/fortran/test_fortran_analyser.py | 10 ++++---- 3 files changed, 33 insertions(+), 6 deletions(-) diff --git a/source/fab/parse/fortran.py b/source/fab/parse/fortran.py index 44fc0674..1d701aff 100644 --- a/source/fab/parse/fortran.py +++ b/source/fab/parse/fortran.py @@ -11,6 +11,7 @@ from pathlib import Path from typing import Union, Optional, Iterable, Dict, Any, Set +from fparser.common.readfortran import FortranStringReader from fparser.two.Fortran2003 import ( # type: ignore Entity_Decl_List, Use_Stmt, Module_Stmt, Program_Stmt, Subroutine_Stmt, Function_Stmt, Language_Binding_Spec, Char_Literal_Constant, Interface_Block, Name, Comment, Module, Call_Stmt, Derived_Type_Def, Derived_Type_Stmt, @@ -284,7 +285,8 @@ def _process_comment(self, analysed_file, obj): # TODO: error handling in case we catch a genuine comment # TODO: separate this project-specific code from the generic f analyser? depends_str = "DEPENDS ON:" - if depends_str in obj.items[0]: + comment = obj.items[0].strip() + if depends_str in comment: self.depends_on_comment_found = True dep = obj.items[0].split(depends_str)[-1].strip() # with .o means a c file @@ -293,6 +295,26 @@ def _process_comment(self, analysed_file, obj): # without .o means a fortran symbol else: analysed_file.add_symbol_dep(dep) + if comment[:2] == "!$": + # Check if it is a use statement with an OpenMP sentinel: + # Use fparser's string reader to discard potential comments + reader = FortranStringReader(comment[2:]) + line = reader.next() + try: + # match returns a 5-tuple, the third one being the module name + module_name = Use_Stmt.match(line.strline)[2] + module_name = module_name.string + except Exception: + # Not a use statement in a sentinel, ignore: + return + + # Register the module name + if module_name in self.ignore_mod_deps: + logger.debug(f"ignoring use of {module_name}") + return + if module_name.lower() not in self._intrinsic_modules: + # found a dependency on fortran + analysed_file.add_module_dep(module_name) def _process_subroutine_or_function(self, analysed_file, fpath, obj): # binding? diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 b/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 index 2d5a84f5..b4f250ff 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 @@ -17,6 +17,11 @@ SUBROUTINE internal_sub RETURN END SUBROUTINE internal_sub + SUBROUTINE openmp_sentinel +!$ USE compute_chunk_size_mod, ONLY: compute_chunk_size ! Note OpenMP sentinel +!$ USE test that is not a sentinel with a use statement inside + END SUBROUTINE openmp_sentinel + INTEGER FUNCTION internal_func() internal_func = 456 END FUNCTION internal_func diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.py b/tests/unit_tests/parse/fortran/test_fortran_analyser.py index 6c334d5f..97d39c77 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.py +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.py @@ -31,11 +31,11 @@ def module_fpath(): def module_expected(module_fpath): return AnalysedFortran( fpath=module_fpath, - file_hash=4039845747, + file_hash=1344519263, module_defs={'foo_mod'}, symbol_defs={'external_sub', 'external_func', 'foo_mod'}, - module_deps={'bar_mod'}, - symbol_deps={'monty_func', 'bar_mod'}, + module_deps={'bar_mod', 'compute_chunk_size_mod'}, + symbol_deps={'monty_func', 'bar_mod', 'compute_chunk_size_mod'}, file_deps=set(), mo_commented_file_deps={'some_file.c'}, ) @@ -72,10 +72,10 @@ def test_program_file(self, fortran_analyser, module_fpath, module_expected): analysis, artefact = fortran_analyser.run(fpath=Path(tmp_file.name)) module_expected.fpath = Path(tmp_file.name) - module_expected._file_hash = 768896775 + module_expected._file_hash = 731743441 module_expected.program_defs = {'foo_mod'} module_expected.module_defs = set() - module_expected.symbol_defs.update({'internal_sub', 'internal_func'}) + module_expected.symbol_defs.update({'internal_sub', 'openmp_sentinel', 'internal_func'}) assert analysis == module_expected assert artefact == fortran_analyser._config.prebuild_folder \ From 054921d112ef33f386c3edf4b4f5b9daa74b87bb Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 21 Mar 2024 00:18:23 +1100 Subject: [PATCH 2/5] Use existing variable. --- source/fab/parse/fortran.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/fab/parse/fortran.py b/source/fab/parse/fortran.py index 1d701aff..a16c5b08 100644 --- a/source/fab/parse/fortran.py +++ b/source/fab/parse/fortran.py @@ -288,7 +288,7 @@ def _process_comment(self, analysed_file, obj): comment = obj.items[0].strip() if depends_str in comment: self.depends_on_comment_found = True - dep = obj.items[0].split(depends_str)[-1].strip() + dep = comment.split(depends_str)[-1].strip() # with .o means a c file if dep.endswith(".o"): analysed_file.mo_commented_file_deps.add(dep.replace(".o", ".c")) From 2aa6d306f0ecac704d5fd1de5c332206b31804e4 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Mon, 3 Jun 2024 09:29:21 +1000 Subject: [PATCH 3/5] Added #TODO so that this can be removed once fparser supports sentinels. --- source/fab/parse/fortran.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source/fab/parse/fortran.py b/source/fab/parse/fortran.py index a16c5b08..01a5b9bc 100644 --- a/source/fab/parse/fortran.py +++ b/source/fab/parse/fortran.py @@ -297,7 +297,9 @@ def _process_comment(self, analysed_file, obj): analysed_file.add_symbol_dep(dep) if comment[:2] == "!$": # Check if it is a use statement with an OpenMP sentinel: - # Use fparser's string reader to discard potential comments + # Use fparser's string reader to discard potential comment + # TODO #13: once fparser supports reading the sentinels, + # this can be removed. reader = FortranStringReader(comment[2:]) line = reader.next() try: From 9843e77e60c6eac4508cbb9e86b2182235cada6e Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Mon, 3 Jun 2024 09:53:27 +1000 Subject: [PATCH 4/5] Fix typing problems by ignoring fparser. --- source/fab/parse/fortran.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/fab/parse/fortran.py b/source/fab/parse/fortran.py index 01a5b9bc..6f8b35d3 100644 --- a/source/fab/parse/fortran.py +++ b/source/fab/parse/fortran.py @@ -11,7 +11,7 @@ from pathlib import Path from typing import Union, Optional, Iterable, Dict, Any, Set -from fparser.common.readfortran import FortranStringReader +from fparser.common.readfortran import FortranStringReader # type: ignore from fparser.two.Fortran2003 import ( # type: ignore Entity_Decl_List, Use_Stmt, Module_Stmt, Program_Stmt, Subroutine_Stmt, Function_Stmt, Language_Binding_Spec, Char_Literal_Constant, Interface_Block, Name, Comment, Module, Call_Stmt, Derived_Type_Def, Derived_Type_Stmt, From 7bb451fdc94b284d8b8f20031d0ce7f0c5789f72 Mon Sep 17 00:00:00 2001 From: Joerg Henrichs Date: Thu, 18 Jul 2024 09:33:19 +1000 Subject: [PATCH 5/5] Added more tests for directives that should be ignored, cleaned up some style warnings and line length. --- .../parse/fortran/test_fortran_analyser.f90 | 4 ++ .../parse/fortran/test_fortran_analyser.py | 49 +++++++++++++------ 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 b/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 index b4f250ff..508ba56b 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.f90 @@ -20,6 +20,10 @@ END SUBROUTINE internal_sub SUBROUTINE openmp_sentinel !$ USE compute_chunk_size_mod, ONLY: compute_chunk_size ! Note OpenMP sentinel !$ USE test that is not a sentinel with a use statement inside +!GCC$ unroll 6 +!DIR$ assume (mod(p, 6) == 0) +!$omp do +!$acc parallel copyin (array, scalar). END SUBROUTINE openmp_sentinel INTEGER FUNCTION internal_func() diff --git a/tests/unit_tests/parse/fortran/test_fortran_analyser.py b/tests/unit_tests/parse/fortran/test_fortran_analyser.py index 97d39c77..75621020 100644 --- a/tests/unit_tests/parse/fortran/test_fortran_analyser.py +++ b/tests/unit_tests/parse/fortran/test_fortran_analyser.py @@ -18,20 +18,25 @@ from fab.parse.fortran_common import iter_content from fab.tools import ToolBox +'''Tests the Fortran analyser. +''' # todo: test function binding @pytest.fixture -def module_fpath(): +def module_fpath() -> Path: + '''Simple fixture that sets the name of the module test file.''' return Path(__file__).parent / "test_fortran_analyser.f90" @pytest.fixture -def module_expected(module_fpath): +def module_expected(module_fpath) -> AnalysedFortran: + '''Returns the expected AnalysedFortran instance for the Fortran + test module.''' return AnalysedFortran( fpath=module_fpath, - file_hash=1344519263, + file_hash=1757501304, module_defs={'foo_mod'}, symbol_defs={'external_sub', 'external_func', 'foo_mod'}, module_deps={'bar_mod', 'compute_chunk_size_mod'}, @@ -41,7 +46,7 @@ def module_expected(module_fpath): ) -class Test_Analyser(object): +class TestAnalyser: @pytest.fixture def fortran_analyser(self, tmp_path): @@ -53,29 +58,37 @@ def fortran_analyser(self, tmp_path): def test_empty_file(self, fortran_analyser): # make sure we get back an EmptySourceFile with mock.patch('fab.parse.AnalysedFile.save'): - analysis, artefact = fortran_analyser.run(fpath=Path(Path(__file__).parent / "empty.f90")) - assert type(analysis) is EmptySourceFile + analysis, artefact = fortran_analyser.run( + fpath=Path(Path(__file__).parent / "empty.f90")) + assert isinstance(analysis, EmptySourceFile) assert artefact is None - def test_module_file(self, fortran_analyser, module_fpath, module_expected): + def test_module_file(self, fortran_analyser, module_fpath, + module_expected): 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.{analysis.file_hash}.an') - def test_program_file(self, fortran_analyser, module_fpath, module_expected): + def test_program_file(self, fortran_analyser, module_fpath, + module_expected): # same as test_module_file() but replacing MODULE with PROGRAM with NamedTemporaryFile(mode='w+t', suffix='.f90') as tmp_file: - tmp_file.write(module_fpath.open().read().replace("MODULE", "PROGRAM")) + tmp_file.write(module_fpath.open().read().replace("MODULE", + "PROGRAM")) tmp_file.flush() with mock.patch('fab.parse.AnalysedFile.save'): - analysis, artefact = fortran_analyser.run(fpath=Path(tmp_file.name)) + analysis, artefact = fortran_analyser.run( + fpath=Path(tmp_file.name)) module_expected.fpath = Path(tmp_file.name) - module_expected._file_hash = 731743441 + module_expected._file_hash = 3388519280 module_expected.program_defs = {'foo_mod'} module_expected.module_defs = set() - module_expected.symbol_defs.update({'internal_sub', 'openmp_sentinel', 'internal_func'}) + module_expected.symbol_defs.update({'internal_func', + 'internal_sub', + 'openmp_sentinel'}) assert analysis == module_expected assert artefact == fortran_analyser._config.prebuild_folder \ @@ -84,11 +97,13 @@ def test_program_file(self, fortran_analyser, module_fpath, module_expected): # todo: test more methods! -class Test_process_variable_binding(object): +class TestProcessVariableBinding: + '''This test class tests the variable binding.''' # todo: define and depend, with and without bind name def test_define_without_bind_name(self, tmp_path): + '''Test usage of bind''' fpath = tmp_path / 'temp.f90' open(fpath, 'wt').write(""" @@ -112,13 +127,15 @@ def test_define_without_bind_name(self, tmp_path): tree = f2008_parser(reader) # find the tree node representing the variable binding - var_decl = next(obj for obj in iter_content(tree) if isinstance(obj, Type_Declaration_Stmt)) + var_decl = next(obj for obj in iter_content(tree) + if isinstance(obj, Type_Declaration_Stmt)) # run our handler fpath = Path('foo') analysed_file = AnalysedFortran(fpath=fpath, file_hash=0) analyser = FortranAnalyser() - analyser._process_variable_binding(analysed_file=analysed_file, obj=var_decl) + analyser._process_variable_binding(analysed_file=analysed_file, + obj=var_decl) assert analysed_file.symbol_defs == {'helloworld', }