From 21cada6c02e5687c861c83f9d69fc6ef0f0b9b53 Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Wed, 6 Dec 2023 11:19:59 +0000 Subject: [PATCH 1/5] Suggested fix to inline trans --- src/psyclone/psyir/transformations/inline_trans.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/psyclone/psyir/transformations/inline_trans.py b/src/psyclone/psyir/transformations/inline_trans.py index 3e5a3ec0a4..04e3935245 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -132,7 +132,6 @@ def apply(self, node, options=None): ''' self.validate(node, options) - # The table associated with the scoping region holding the Call. table = node.scope.symbol_table # Find the routine to be inlined. @@ -160,7 +159,6 @@ def apply(self, node, options=None): # Shallow copy the symbols from the routine into the table at the # call site. table.merge(routine_table, include_arguments=False) - # When constructing new references to replace references to formal # args, we need to know whether any of the actual arguments are array # accesses. If they use 'array notation' (i.e. represent a whole array) @@ -181,6 +179,8 @@ def apply(self, node, options=None): for ref in refs[:]: self._replace_formal_arg(ref, node, formal_args) + ancestor_table = node.ancestor(Routine).scope.symbol_table + scope = node.scope # Copy the nodes from the Routine into the call site. if isinstance(new_stmts[-1], Return): # If the final statement of the routine is a return then @@ -210,6 +210,11 @@ def apply(self, node, options=None): idx += 1 parent.addchild(child, idx) + ancestor_table.merge(scope.symbol_table) + replacement = type(scope.symbol_table)() + scope.symbol_table.detach() + replacement.attach(scope) + def _replace_formal_arg(self, ref, call_node, formal_args): ''' Recursively combines any References to formal arguments in the supplied @@ -596,7 +601,7 @@ def validate(self, node, options=None): raise TransformationError( f"Cannot inline an IntrinsicCall ('{node.routine.name}')") name = node.routine.name - + return # Check that we can find the source of the routine being inlined. routine = self._find_routine(node) From 3a3cdac2ad45246e6e7f2d24b7ac2cf43d3e606b Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Wed, 6 Dec 2023 13:43:47 +0000 Subject: [PATCH 2/5] Fixes for the tests --- .../psyir/transformations/inline_trans.py | 10 ++++----- .../transformations/inline_trans_test.py | 22 ++++++++++--------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/psyclone/psyir/transformations/inline_trans.py b/src/psyclone/psyir/transformations/inline_trans.py index 04e3935245..a1ef25ed00 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -210,10 +210,11 @@ def apply(self, node, options=None): idx += 1 parent.addchild(child, idx) - ancestor_table.merge(scope.symbol_table) - replacement = type(scope.symbol_table)() - scope.symbol_table.detach() - replacement.attach(scope) + if(ancestor_table is not scope.symbol_table): + ancestor_table.merge(scope.symbol_table) + replacement = type(scope.symbol_table)() + scope.symbol_table.detach() + replacement.attach(scope) def _replace_formal_arg(self, ref, call_node, formal_args): ''' @@ -601,7 +602,6 @@ def validate(self, node, options=None): raise TransformationError( f"Cannot inline an IntrinsicCall ('{node.routine.name}')") name = node.routine.name - return # Check that we can find the source of the routine being inlined. routine = self._find_routine(node) diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index ce19fce50a..423e421643 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -323,21 +323,22 @@ def test_apply_struct_arg(fortran_reader, fortran_writer, tmpdir): inline_trans.apply(routine) output = fortran_writer(psyir) + print(output) assert (" do i = 1, 5, 1\n" - " do i_3 = 1, 10, 1\n" - " var%data(i_3) = 2.0 * i\n" + " do i_1 = 1, 10, 1\n" + " var%data(i_1) = 2.0 * i\n" " enddo\n" " var%data(:) = -1.0\n" " var%data = -5.0\n" " var%data(1:2) = 0.0\n" - " do i_1 = 1, 10, 1\n" - " var_list(i)%data(i_1) = 2.0 * i\n" + " do i_2 = 1, 10, 1\n" + " var_list(i)%data(i_2) = 2.0 * i\n" " enddo\n" " var_list(i)%data(:) = -1.0\n" " var_list(i)%data = -5.0\n" " var_list(i)%data(1:2) = 0.0\n" - " do i_2 = 1, 10, 1\n" - " var2(i)%region%data(i_2) = 2.0 * i\n" + " do i_3 = 1, 10, 1\n" + " var2(i)%region%data(i_3) = 2.0 * i\n" " enddo\n" " var2(i)%region%data(:) = -1.0\n" " var2(i)%region%data = -5.0\n" @@ -718,16 +719,17 @@ def test_apply_array_slice_arg(fortran_reader, fortran_writer, tmpdir): for call in psyir.walk(Routine)[0].walk(Call, stop_type=Call): inline_trans.apply(call) output = fortran_writer(psyir) + print(output) assert (" do i = 1, 10, 1\n" - " do i_4 = 1, 10, 1\n" - " a(1,i_4,i) = 2.0 * i_4\n" + " do i_1 = 1, 10, 1\n" + " a(1,i_1,i) = 2.0 * i_1\n" " enddo\n" " enddo\n" " a(1,1,:) = 3.0 * a(1,1,:)\n" " a(:,1,:) = 2.0 * a(:,1,:)\n" " b(:,:) = 2.0 * b(:,:)\n" - " do i_3 = 1, 10, 1\n" - " b(i_3,:5) = 2.0 * b(i_3,:5)\n" in output) + " do i_4 = 1, 10, 1\n" + " b(i_4,:5) = 2.0 * b(i_4,:5)\n" in output) assert Compile(tmpdir).string_compiles(output) From 46b42c520ee66b59d3b6978c369fda34b350f890 Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Wed, 6 Dec 2023 14:11:16 +0000 Subject: [PATCH 3/5] remove print statement --- src/psyclone/tests/psyir/transformations/inline_trans_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index 423e421643..250cfdf32e 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -719,7 +719,6 @@ def test_apply_array_slice_arg(fortran_reader, fortran_writer, tmpdir): for call in psyir.walk(Routine)[0].walk(Call, stop_type=Call): inline_trans.apply(call) output = fortran_writer(psyir) - print(output) assert (" do i = 1, 10, 1\n" " do i_1 = 1, 10, 1\n" " a(1,i_1,i) = 2.0 * i_1\n" From 3d1c1154f9164b2291ded0ad4f64a080ec247f1d Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Mon, 11 Dec 2023 14:04:49 +0000 Subject: [PATCH 4/5] Updates for the review --- .../psyir/transformations/inline_trans.py | 8 ++++- .../transformations/inline_trans_test.py | 32 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/psyclone/psyir/transformations/inline_trans.py b/src/psyclone/psyir/transformations/inline_trans.py index a1ef25ed00..5cba118bdb 100644 --- a/src/psyclone/psyir/transformations/inline_trans.py +++ b/src/psyclone/psyir/transformations/inline_trans.py @@ -179,6 +179,8 @@ def apply(self, node, options=None): for ref in refs[:]: self._replace_formal_arg(ref, node, formal_args) + # Store the Routine level symbol table and node's current scope + # so we can merge symbol tables later if required. ancestor_table = node.ancestor(Routine).scope.symbol_table scope = node.scope # Copy the nodes from the Routine into the call site. @@ -210,7 +212,11 @@ def apply(self, node, options=None): idx += 1 parent.addchild(child, idx) - if(ancestor_table is not scope.symbol_table): + # If the scope we merged the inlined functions symbol table into + # is not a Routine scope then we now merge that symbol table into + # the ancestor Routine. This avoids issues like #2424 when + # applying ParallelLoopTrans to loops containing inlined calls. + if ancestor_table is not scope.symbol_table: ancestor_table.merge(scope.symbol_table) replacement = type(scope.symbol_table)() scope.symbol_table.detach() diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index 250cfdf32e..7017433c31 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -2406,3 +2406,35 @@ def test_find_routine_in_container(fortran_reader): call_node, call_node.routine.interface.container_symbol) assert isinstance(result, Routine) assert result.name == "sub" + + +def test_apply_merges_symbol_table_with_routine(fortran_reader): + ''' + Check that the apply method merges the inlined function's symbol table to + the containing Routine when the call node is inside a child ScopingNode. + ''' + code = ( + "module test_mod\n" + "contains\n" + " subroutine run_it()\n" + " integer :: i\n" + " real :: a(10)\n" + " do i=1,10\n" + " call sub(a, i)\n" + " end do\n" + " end subroutine run_it\n" + " subroutine sub(x, ivar)\n" + " real, intent(inout), dimension(10) :: x\n" + " integer, intent(in) :: ivar\n" + " integer :: i\n" + " do i = 1, 10\n" + " x(i) = 2.0*ivar\n" + " end do\n" + " end subroutine sub\n" + "end module test_mod\n") + psyir = fortran_reader.psyir_from_source(code) + routine = psyir.walk(Call)[0] + inline_trans = InlineTrans() + inline_trans.apply(routine) + # The i_1 symbol is the renamed i from the inlined call. + assert psyir.walk(Routine)[0].symbol_table.get_symbols()['i_1'] is not None From 1ad9e428276d606211099f0754fc1925764a3fad Mon Sep 17 00:00:00 2001 From: Aidan Chalk <3043914+LonelyCat124@users.noreply.github.com> Date: Mon, 11 Dec 2023 14:51:33 +0000 Subject: [PATCH 5/5] removed print statement --- src/psyclone/tests/psyir/transformations/inline_trans_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/psyclone/tests/psyir/transformations/inline_trans_test.py b/src/psyclone/tests/psyir/transformations/inline_trans_test.py index 7017433c31..8e25c8597f 100644 --- a/src/psyclone/tests/psyir/transformations/inline_trans_test.py +++ b/src/psyclone/tests/psyir/transformations/inline_trans_test.py @@ -323,7 +323,6 @@ def test_apply_struct_arg(fortran_reader, fortran_writer, tmpdir): inline_trans.apply(routine) output = fortran_writer(psyir) - print(output) assert (" do i = 1, 5, 1\n" " do i_1 = 1, 10, 1\n" " var%data(i_1) = 2.0 * i\n"