From 81355e06b98ba1da2364ea3ec0ecbbc308a49edf Mon Sep 17 00:00:00 2001 From: Michael Staneker Date: Mon, 11 Nov 2024 12:34:13 +0000 Subject: [PATCH 1/3] extending depdendency trafo: rename/transform private/public access specifiers for modules --- .../build_system/dependency.py | 36 ++++++ .../build_system/tests/test_dependency.py | 113 ++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/loki/transformations/build_system/dependency.py b/loki/transformations/build_system/dependency.py index e967b16cd..71721e7b5 100644 --- a/loki/transformations/build_system/dependency.py +++ b/loki/transformations/build_system/dependency.py @@ -133,6 +133,18 @@ def transform_module(self, module, **kwargs): if self.replace_ignore_items and (item := kwargs.get('item')): targets += tuple(str(i).lower() for i in item.ignore) self.rename_imports(module, imports=module.imports, targets=targets) + active_nodes = None + if self.remove_inactive_items and not kwargs.get('items') is None: + active_nodes = [item.scope_ir.name.lower() for item in kwargs['items']] + # rename target names in an access spec for both public and private access specs  + if module.public_access_spec: + module.public_access_spec = self.rename_access_spec_names( + module.public_access_spec, targets=targets, active_nodes=active_nodes + ) + if module.private_access_spec: + module.private_access_spec = self.rename_access_spec_names( + module.private_access_spec, targets=targets, active_nodes=active_nodes + ) def transform_subroutine(self, routine, **kwargs): """ @@ -329,6 +341,30 @@ def rename_imports(self, source, imports, targets=None): if import_map: source.spec = Transformer(import_map).visit(source.spec) + def rename_access_spec_names(self, access_spec, targets=None, active_nodes=None): + """ + Rename target names in an access spec + + For all names in the access spec that are contained in :data:`targets`, rename them as + ``{name}{self.suffix}``. If :data:`active_nodes` are given, then all names + that are not in the list of active nodes, are being removed from the list. + Parameters + ---------- + access_spec : list of str + List of names from an access spec + targets : list of str + Optional list of subroutine names for which to modify access specs + active_nodes : list of str + Optional list of active nodes + """ + if active_nodes: + access_spec = tuple(elem for elem in access_spec if elem in active_nodes) + return tuple( + f'{elem}{self.suffix}' if not targets or elem in targets + else elem + for elem in access_spec + ) + def rename_interfaces(self, intfs, targets=None): """ Update explicit interfaces to actively transformed subroutines. diff --git a/loki/transformations/build_system/tests/test_dependency.py b/loki/transformations/build_system/tests/test_dependency.py index 873691d99..3aef0e66d 100644 --- a/loki/transformations/build_system/tests/test_dependency.py +++ b/loki/transformations/build_system/tests/test_dependency.py @@ -121,6 +121,119 @@ def test_dependency_transformation_globalvar_imports(frontend, use_scheduler, tm assert 'some_const' in [str(s) for s in driver['driver'].spec.body[1].symbols] +@pytest.mark.parametrize('frontend', available_frontends(skip=[(OMNI, 'OMNI removes access specifiers ...')])) +@pytest.mark.parametrize('use_scheduler', [False, True]) +def test_dependency_transformation_access_specs(frontend, use_scheduler, tmp_path, config): + """ + Test that global variable imports are not renamed as a + call statement would be. + """ + + kernel_fcode = """ +MODULE kernel_access_spec_mod + + INTEGER, PUBLIC :: some_const + +PRIVATE +PUBLIC kernel, kernel_2, unused_kernel +CONTAINS + SUBROUTINE kernel(a, b, c) + IMPLICIT NONE + INTEGER, INTENT(INOUT) :: a, b, c + + call kernel_2(a, b) + call kernel_3(c) + END SUBROUTINE kernel + SUBROUTINE kernel_2(a, b) + IMPLICIT NONE + INTEGER, INTENT(INOUT) :: a, b + + a = 1 + b = 2 + END SUBROUTINE kernel_2 + SUBROUTINE kernel_3(a) + IMPLICIT NONE + INTEGER, INTENT(INOUT) :: a + + a = 3 + END SUBROUTINE kernel_3 + SUBROUTINE unused_kernel(a) + IMPLICIT NONE + INTEGER, INTENT(INOUT) :: a + + a = 3 + END SUBROUTINE unused_kernel +END MODULE kernel_access_spec_mod + """.strip() + + driver_fcode = """ +SUBROUTINE driver(a, b, c) + USE kernel_access_spec_mod, only: kernel + USE kernel_access_spec_mod, only: some_const + IMPLICIT NONE + INTEGER, INTENT(INOUT) :: a, b, c + + CALL kernel(a, b ,c) +END SUBROUTINE driver + """.strip() + + transformation = DependencyTransformation(suffix='_test', module_suffix='_mod') + if use_scheduler: + (tmp_path/'kernel_access_spec_mod.F90').write_text(kernel_fcode) + (tmp_path/'driver.F90').write_text(driver_fcode) + scheduler = Scheduler( + paths=[tmp_path], config=SchedulerConfig.from_dict(config), frontend=frontend, xmods=[tmp_path] + ) + scheduler.process(transformation) + + # Check that both, old and new module exist now in the scheduler graph + assert 'kernel_access_spec_test_mod#kernel_test' in scheduler.items # for the subroutine + assert 'kernel_access_spec_mod' in scheduler.items # for the global variable + + kernel = scheduler['kernel_access_spec_test_mod#kernel_test'].source + driver = scheduler['#driver'].source + + # Check that the not-renamed module is indeed the original one + scheduler.item_factory.item_cache[str(tmp_path/'kernel_access_spec_mod.F90')].source.make_complete( + frontend=frontend, xmods=[tmp_path] + ) + assert ( + Sourcefile.from_source(kernel_fcode, frontend=frontend, xmods=[tmp_path]).to_fortran() == + scheduler.item_factory.item_cache[str(tmp_path/'kernel_access_spec_mod.F90')].source.to_fortran() + ) + + else: + kernel = Sourcefile.from_source(kernel_fcode, frontend=frontend, xmods=[tmp_path]) + driver = Sourcefile.from_source(driver_fcode, frontend=frontend, xmods=[tmp_path], + definitions=kernel.definitions) + + kernel.apply(transformation, role='kernel') + driver['driver'].apply(transformation, role='driver', targets=('kernel', 'kernel_access_spec_mod')) + + # Check that the global variable declaration remains unchanged + assert kernel.modules[0].variables[0].name == 'some_const' + + # Check that calls and matching import have been diverted to the re-generated routine + calls = FindNodes(CallStatement).visit(driver['driver'].body) + assert len(calls) == 1 + assert calls[0].name == 'kernel_test' + imports = FindNodes(Import).visit(driver['driver'].spec) + assert len(imports) == 2 + assert isinstance(imports[0], Import) + assert driver['driver'].spec.body[0].module == 'kernel_access_spec_test_mod' + assert 'kernel_test' in [str(s) for s in driver['driver'].spec.body[0].symbols] + + # Check that global variable import remains unchanged + assert isinstance(imports[1], Import) + assert driver['driver'].spec.body[1].module == 'kernel_access_spec_mod' + assert 'some_const' in [str(s) for s in driver['driver'].spec.body[1].symbols] + + if use_scheduler: + assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test') + else: + assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test', 'unused_kernel_test') + + @pytest.mark.parametrize('frontend', available_frontends()) @pytest.mark.parametrize('use_scheduler', [False, True]) def test_dependency_transformation_globalvar_imports_driver_mod(frontend, use_scheduler, tmp_path, config): From 3c4cc95ef1f2ba82b165bde24e7ea04845dca09e Mon Sep 17 00:00:00 2001 From: Michael Staneker Date: Fri, 13 Dec 2024 14:05:29 +0100 Subject: [PATCH 2/3] extending depdendency trafo: take care of module variables in access specifiers --- .../build_system/dependency.py | 23 +++++++++++++------ .../build_system/tests/test_dependency.py | 20 +++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/loki/transformations/build_system/dependency.py b/loki/transformations/build_system/dependency.py index 71721e7b5..de75868a0 100644 --- a/loki/transformations/build_system/dependency.py +++ b/loki/transformations/build_system/dependency.py @@ -137,13 +137,16 @@ def transform_module(self, module, **kwargs): if self.remove_inactive_items and not kwargs.get('items') is None: active_nodes = [item.scope_ir.name.lower() for item in kwargs['items']] # rename target names in an access spec for both public and private access specs  + variables = module.variables if module.public_access_spec: module.public_access_spec = self.rename_access_spec_names( - module.public_access_spec, targets=targets, active_nodes=active_nodes + module.public_access_spec, targets=targets, active_nodes=active_nodes, + variables=variables ) if module.private_access_spec: module.private_access_spec = self.rename_access_spec_names( - module.private_access_spec, targets=targets, active_nodes=active_nodes + module.private_access_spec, targets=targets, active_nodes=active_nodes, + variables=variables ) def transform_subroutine(self, routine, **kwargs): @@ -341,7 +344,7 @@ def rename_imports(self, source, imports, targets=None): if import_map: source.spec = Transformer(import_map).visit(source.spec) - def rename_access_spec_names(self, access_spec, targets=None, active_nodes=None): + def rename_access_spec_names(self, access_spec, targets=None, active_nodes=None, variables=None): """ Rename target names in an access spec @@ -356,14 +359,20 @@ def rename_access_spec_names(self, access_spec, targets=None, active_nodes=None) Optional list of subroutine names for which to modify access specs active_nodes : list of str Optional list of active nodes + variables : list of :any:`Expression` + Optional list of module variables """ - if active_nodes: - access_spec = tuple(elem for elem in access_spec if elem in active_nodes) + module_variables = variables or () + if active_nodes is not None: + access_spec_routines = tuple(elem for elem in access_spec if elem in active_nodes) + else: + access_spec_routines = tuple(elem for elem in access_spec if elem not in module_variables) + access_spec_vars = tuple(elem for elem in access_spec if elem in module_variables) return tuple( f'{elem}{self.suffix}' if not targets or elem in targets else elem - for elem in access_spec - ) + for elem in access_spec_routines + ) + access_spec_vars def rename_interfaces(self, intfs, targets=None): """ diff --git a/loki/transformations/build_system/tests/test_dependency.py b/loki/transformations/build_system/tests/test_dependency.py index 3aef0e66d..e08a1d106 100644 --- a/loki/transformations/build_system/tests/test_dependency.py +++ b/loki/transformations/build_system/tests/test_dependency.py @@ -123,7 +123,7 @@ def test_dependency_transformation_globalvar_imports(frontend, use_scheduler, tm @pytest.mark.parametrize('frontend', available_frontends(skip=[(OMNI, 'OMNI removes access specifiers ...')])) @pytest.mark.parametrize('use_scheduler', [False, True]) -def test_dependency_transformation_access_specs(frontend, use_scheduler, tmp_path, config): +def test_dependency_transformation_access_spec_names(frontend, use_scheduler, tmp_path, config): """ Test that global variable imports are not renamed as a call statement would be. @@ -133,9 +133,10 @@ def test_dependency_transformation_access_specs(frontend, use_scheduler, tmp_pat MODULE kernel_access_spec_mod INTEGER, PUBLIC :: some_const + INTEGER :: another_const PRIVATE -PUBLIC kernel, kernel_2, unused_kernel +PUBLIC kernel, kernel_2, unused_kernel, another_const CONTAINS SUBROUTINE kernel(a, b, c) IMPLICIT NONE @@ -168,12 +169,12 @@ def test_dependency_transformation_access_specs(frontend, use_scheduler, tmp_pat driver_fcode = """ SUBROUTINE driver(a, b, c) - USE kernel_access_spec_mod, only: kernel + USE kernel_access_spec_mod, only: kernel, another_const USE kernel_access_spec_mod, only: some_const IMPLICIT NONE INTEGER, INTENT(INOUT) :: a, b, c - CALL kernel(a, b ,c) + CALL kernel(a, b, c) END SUBROUTINE driver """.strip() @@ -212,13 +213,14 @@ def test_dependency_transformation_access_specs(frontend, use_scheduler, tmp_pat # Check that the global variable declaration remains unchanged assert kernel.modules[0].variables[0].name == 'some_const' + assert kernel.modules[0].variables[1].name == 'another_const' # Check that calls and matching import have been diverted to the re-generated routine calls = FindNodes(CallStatement).visit(driver['driver'].body) assert len(calls) == 1 assert calls[0].name == 'kernel_test' imports = FindNodes(Import).visit(driver['driver'].spec) - assert len(imports) == 2 + assert len(imports) == 3 assert isinstance(imports[0], Import) assert driver['driver'].spec.body[0].module == 'kernel_access_spec_test_mod' assert 'kernel_test' in [str(s) for s in driver['driver'].spec.body[0].symbols] @@ -226,12 +228,14 @@ def test_dependency_transformation_access_specs(frontend, use_scheduler, tmp_pat # Check that global variable import remains unchanged assert isinstance(imports[1], Import) assert driver['driver'].spec.body[1].module == 'kernel_access_spec_mod' - assert 'some_const' in [str(s) for s in driver['driver'].spec.body[1].symbols] + assert 'another_const' in [str(s) for s in driver['driver'].spec.body[1].symbols] + assert 'some_const' in [str(s) for s in driver['driver'].spec.body[2].symbols] if use_scheduler: - assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test') + assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test', 'another_const') else: - assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test', 'unused_kernel_test') + assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test', 'unused_kernel_test', + 'another_const') @pytest.mark.parametrize('frontend', available_frontends()) From 07e81d73d9a37219bee9ab813b43484a1de362f6 Mon Sep 17 00:00:00 2001 From: Michael Staneker Date: Fri, 13 Dec 2024 14:44:12 +0100 Subject: [PATCH 3/3] extending depdendency trafo: take care of typedefs in access specifiers --- .../build_system/dependency.py | 27 +++++++++---------- .../build_system/tests/test_dependency.py | 19 ++++++++++--- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/loki/transformations/build_system/dependency.py b/loki/transformations/build_system/dependency.py index de75868a0..27c616bd6 100644 --- a/loki/transformations/build_system/dependency.py +++ b/loki/transformations/build_system/dependency.py @@ -115,6 +115,8 @@ def transform_module(self, module, **kwargs): """ role = kwargs.get('role') + # remember/keep track of the module subroutines (even if some of those are removed) + routines = tuple(routine.name.lower() for routine in module.subroutines) if role == 'kernel': # Change the name of kernel modules module.name = self.derive_module_name(module.name) @@ -137,16 +139,15 @@ def transform_module(self, module, **kwargs): if self.remove_inactive_items and not kwargs.get('items') is None: active_nodes = [item.scope_ir.name.lower() for item in kwargs['items']] # rename target names in an access spec for both public and private access specs  - variables = module.variables if module.public_access_spec: module.public_access_spec = self.rename_access_spec_names( module.public_access_spec, targets=targets, active_nodes=active_nodes, - variables=variables + routines=routines ) if module.private_access_spec: module.private_access_spec = self.rename_access_spec_names( module.private_access_spec, targets=targets, active_nodes=active_nodes, - variables=variables + routines=routines ) def transform_subroutine(self, routine, **kwargs): @@ -344,7 +345,7 @@ def rename_imports(self, source, imports, targets=None): if import_map: source.spec = Transformer(import_map).visit(source.spec) - def rename_access_spec_names(self, access_spec, targets=None, active_nodes=None, variables=None): + def rename_access_spec_names(self, access_spec, targets=None, active_nodes=None, routines=None): """ Rename target names in an access spec @@ -359,20 +360,18 @@ def rename_access_spec_names(self, access_spec, targets=None, active_nodes=None, Optional list of subroutine names for which to modify access specs active_nodes : list of str Optional list of active nodes - variables : list of :any:`Expression` - Optional list of module variables + routines : list of :any:`Subroutine` + Optional list of subroutines """ - module_variables = variables or () + module_routines = routines or () if active_nodes is not None: - access_spec_routines = tuple(elem for elem in access_spec if elem in active_nodes) - else: - access_spec_routines = tuple(elem for elem in access_spec if elem not in module_variables) - access_spec_vars = tuple(elem for elem in access_spec if elem in module_variables) + access_spec = tuple(elem for elem in access_spec if elem in active_nodes + or elem.lower() not in module_routines) return tuple( - f'{elem}{self.suffix}' if not targets or elem in targets + f'{elem}{self.suffix}' if elem.lower() in module_routines and (not targets or elem in targets) else elem - for elem in access_spec_routines - ) + access_spec_vars + for elem in access_spec + ) def rename_interfaces(self, intfs, targets=None): """ diff --git a/loki/transformations/build_system/tests/test_dependency.py b/loki/transformations/build_system/tests/test_dependency.py index e08a1d106..f43703263 100644 --- a/loki/transformations/build_system/tests/test_dependency.py +++ b/loki/transformations/build_system/tests/test_dependency.py @@ -135,8 +135,16 @@ def test_dependency_transformation_access_spec_names(frontend, use_scheduler, tm INTEGER, PUBLIC :: some_const INTEGER :: another_const + type :: t_type_1 + integer :: i_1 + end type + + type :: t_type_2 + integer :: i_2 + end type + PRIVATE -PUBLIC kernel, kernel_2, unused_kernel, another_const +PUBLIC kernel, kernel_2, unused_kernel, another_const, t_type_2 CONTAINS SUBROUTINE kernel(a, b, c) IMPLICIT NONE @@ -215,6 +223,10 @@ def test_dependency_transformation_access_spec_names(frontend, use_scheduler, tm assert kernel.modules[0].variables[0].name == 'some_const' assert kernel.modules[0].variables[1].name == 'another_const' + # Check that the typedefs remains unchanged + assert kernel.modules[0].typedefs[0].name == 't_type_1' + assert kernel.modules[0].typedefs[1].name == 't_type_2' + # Check that calls and matching import have been diverted to the re-generated routine calls = FindNodes(CallStatement).visit(driver['driver'].body) assert len(calls) == 1 @@ -232,10 +244,11 @@ def test_dependency_transformation_access_spec_names(frontend, use_scheduler, tm assert 'some_const' in [str(s) for s in driver['driver'].spec.body[2].symbols] if use_scheduler: - assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test', 'another_const') + assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test', 'another_const', + 't_type_2') else: assert kernel.modules[0].public_access_spec == ('kernel_test', 'kernel_2_test', 'unused_kernel_test', - 'another_const') + 'another_const', 't_type_2') @pytest.mark.parametrize('frontend', available_frontends())