From ec2602d277df26935682aca999f3f1dbbfa770d5 Mon Sep 17 00:00:00 2001 From: Santeri Karppinen Date: Wed, 20 Dec 2023 15:02:52 +0200 Subject: [PATCH 1/4] added bugfix for inliner replacing array dimensions incorrectly --- loki/transform/transform_inline.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/loki/transform/transform_inline.py b/loki/transform/transform_inline.py index edf59e2e1..689e7556d 100644 --- a/loki/transform/transform_inline.py +++ b/loki/transform/transform_inline.py @@ -14,7 +14,7 @@ from loki.expression import ( FindVariables, FindInlineCalls, FindLiterals, - SubstituteExpressions, LokiIdentityMapper + SubstituteExpressions, LokiIdentityMapper, ) from loki.ir import Import, Comment, Assignment, VariableDeclaration, CallStatement from loki.expression import symbols as sym @@ -334,11 +334,24 @@ def inline_subroutine_calls(routine, calls, callee, allowed_aliases=None): ) callee.spec = shadow_mapper.visit(callee.spec) - var_map = {} duplicate_names = {dl.name.lower() for dl in duplicates} - for v in FindVariables(unique=False).visit(callee.body): - if v.name.lower() in duplicate_names: - var_map[v] = v.clone(name=f'{callee.name}_{v.name}') + need_rename = tuple(v for v in FindVariables(unique=False).visit(callee.body) \ + if v.name.lower() in duplicate_names) + nonarr_map = {} + for v in need_rename: + if not isinstance(v, sym.Array): + # Non-arrays handled by just changing name. + nonarr_map[v] = v.clone(name=f'{callee.name}_{v.name}') + + var_map = {} + for v in need_rename: + if isinstance(v, sym.Array): + # Arrays require special handling, since their dimensions may contain variables + # that themselves also need to change. + replacement = v.clone(name=f'{callee.name}_{v.name}', + dimensions = SubstituteExpressions(nonarr_map).visit(v.dimensions)) + var_map[v] = replacement + var_map.update(nonarr_map) callee.body = SubstituteExpressions(var_map).visit(callee.body) # Separate allowed aliases from other variables to ensure clean hoisting From 93a956483a77ce4dacb645c5cde02edff472aaff Mon Sep 17 00:00:00 2001 From: Santeri Karppinen Date: Wed, 20 Dec 2023 15:31:34 +0200 Subject: [PATCH 2/4] added new test to check that bug remains fixed --- tests/test_transform_inline.py | 35 ++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/test_transform_inline.py b/tests/test_transform_inline.py index ec70d1113..9400bddba 100644 --- a/tests/test_transform_inline.py +++ b/tests/test_transform_inline.py @@ -569,6 +569,41 @@ def test_inline_internal_routines_aliasing_declaration(frontend): assert assigns[3].lhs == 'jg' and assigns[3].rhs == '2' assert assigns[4].lhs == 'z' and assigns[4].rhs == 'jlon + jg' +@pytest.mark.parametrize('frontend', available_frontends()) +def test_inline_member_routines_indexing_of_shadowed_array(frontend): + """ + Test special case of inlining of member subroutines when inlined routine contains + shadowed array and array indices. + In particular, this test checks that also the variables indexing + the array in the inlined result get renamed correctly. + """ + fcode = """ + subroutine outer(klon) + integer :: jg, jlon + integer :: arr(3, 3) + + jg = 70000 + call inner2() + + contains + + subroutine inner2() + integer :: jlon, jg + integer :: arr(3, 3) + do jg=1,3 + do jlon=1,3 + arr(jlon, jg) = 11 + end do + end do + end subroutine inner2 + + end subroutine outer + """ + routine = Subroutine.from_source(fcode) + inline_member_procedures(routine) + innerloop = FindNodes(Loop).visit(routine.body)[1] + innerloopvars = FindVariables().visit(innerloop) + assert 'inner2_arr(inner2_jlon,inner2_jg)' in innerloopvars @pytest.mark.parametrize('frontend', available_frontends()) def test_inline_member_routines_sequence_assoc(frontend): From dc163e429728a3714b2139ad3c44e44ac05e7c4f Mon Sep 17 00:00:00 2001 From: Santeri Karppinen Date: Wed, 20 Dec 2023 15:34:54 +0200 Subject: [PATCH 3/4] fix linter warnings --- tests/test_transform_inline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_transform_inline.py b/tests/test_transform_inline.py index 9400bddba..c7430b04d 100644 --- a/tests/test_transform_inline.py +++ b/tests/test_transform_inline.py @@ -599,7 +599,7 @@ def test_inline_member_routines_indexing_of_shadowed_array(frontend): end subroutine outer """ - routine = Subroutine.from_source(fcode) + routine = Subroutine.from_source(fcode, frontend=frontend) inline_member_procedures(routine) innerloop = FindNodes(Loop).visit(routine.body)[1] innerloopvars = FindVariables().visit(innerloop) From af6b05072a117e8504337348c5dfe76d61b0ea69 Mon Sep 17 00:00:00 2001 From: Santeri Karppinen Date: Thu, 21 Dec 2023 09:26:57 +0200 Subject: [PATCH 4/4] simply using recursive_expression_map_update gives a lot simpler fix. added import for it, remove local import found in same file to avoid duplicate imports --- loki/transform/transform_inline.py | 33 +++++++++--------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/loki/transform/transform_inline.py b/loki/transform/transform_inline.py index 689e7556d..62974cf18 100644 --- a/loki/transform/transform_inline.py +++ b/loki/transform/transform_inline.py @@ -14,7 +14,7 @@ from loki.expression import ( FindVariables, FindInlineCalls, FindLiterals, - SubstituteExpressions, LokiIdentityMapper, + SubstituteExpressions, LokiIdentityMapper ) from loki.ir import Import, Comment, Assignment, VariableDeclaration, CallStatement from loki.expression import symbols as sym @@ -24,8 +24,10 @@ from loki.logging import warning, error from loki.pragma_utils import pragmas_attached, is_loki_pragma -from loki.transform.transform_utilities import single_variable_declaration - +from loki.transform.transform_utilities import ( + single_variable_declaration, + recursive_expression_map_update +) __all__ = [ 'inline_constant_parameters', 'inline_elemental_functions', @@ -208,9 +210,6 @@ def map_call_to_procedure_body(call, caller): Procedure (scope) into which the callee's body gets mapped """ - # pylint: disable=import-outside-toplevel,cyclic-import - from loki.transform import recursive_expression_map_update - def _map_unbound_dims(var, val): """ Maps all unbound dimension ranges in the passed array value @@ -334,24 +333,12 @@ def inline_subroutine_calls(routine, calls, callee, allowed_aliases=None): ) callee.spec = shadow_mapper.visit(callee.spec) - duplicate_names = {dl.name.lower() for dl in duplicates} - need_rename = tuple(v for v in FindVariables(unique=False).visit(callee.body) \ - if v.name.lower() in duplicate_names) - nonarr_map = {} - for v in need_rename: - if not isinstance(v, sym.Array): - # Non-arrays handled by just changing name. - nonarr_map[v] = v.clone(name=f'{callee.name}_{v.name}') - var_map = {} - for v in need_rename: - if isinstance(v, sym.Array): - # Arrays require special handling, since their dimensions may contain variables - # that themselves also need to change. - replacement = v.clone(name=f'{callee.name}_{v.name}', - dimensions = SubstituteExpressions(nonarr_map).visit(v.dimensions)) - var_map[v] = replacement - var_map.update(nonarr_map) + duplicate_names = {dl.name.lower() for dl in duplicates} + for v in FindVariables(unique=False).visit(callee.body): + if v.name.lower() in duplicate_names: + var_map[v] = v.clone(name=f'{callee.name}_{v.name}') + var_map = recursive_expression_map_update(var_map) callee.body = SubstituteExpressions(var_map).visit(callee.body) # Separate allowed aliases from other variables to ensure clean hoisting