Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(#Closes #2592 and 2582(?)) Routine has a _symbol attribute. #2596

Merged
merged 55 commits into from
Oct 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
7bbed9b
First Routine change, many failing tests expected but want to show im…
LonelyCat124 May 23, 2024
b6a803b
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Jun 18, 2024
c68ebdf
Now deleted the symbol when the parent is disconnected
LonelyCat124 Jun 21, 2024
a801951
Merge branch '2592_Routine_symbol' of github.com:stfc/PSyclone into 2…
LonelyCat124 Jun 21, 2024
b4ea95b
Routine now handles its own symbols, and this is supported for Module…
LonelyCat124 Jun 21, 2024
d747f64
Merged master, tests broken [skip-ci]
LonelyCat124 Jun 26, 2024
34f69e2
Removing the generate_container functionality as its out of date (tow…
LonelyCat124 Jul 10, 2024
6e623fb
Removing the generate_container functionality as its out of date (tow…
LonelyCat124 Jul 10, 2024
248f27e
Improvements and fixes to tests to ensure PSyclone behaviour matches …
LonelyCat124 Jul 15, 2024
8bf55eb
Fixed the issue in KernelModuleInlineTrans with repeated symbol addition
LonelyCat124 Jul 16, 2024
77d4e38
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Jul 16, 2024
c47a850
Fixes for the failing tests
LonelyCat124 Jul 17, 2024
6d4ad93
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Jul 17, 2024
efa0730
Fixed linting
LonelyCat124 Jul 17, 2024
9734b68
Merge branch '2592_Routine_symbol' of github.com:stfc/PSyclone into 2…
LonelyCat124 Jul 17, 2024
c4715d4
Fixed missing coverage and remaining unreachable code
LonelyCat124 Jul 18, 2024
351da44
Removed more unreachable code
LonelyCat124 Jul 18, 2024
23eebf9
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Jul 18, 2024
7dd7840
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Jul 18, 2024
88b25f9
changes from review
LonelyCat124 Jul 24, 2024
cda4d1c
Merge branch '2592_Routine_symbol' of github.com:stfc/PSyclone into 2…
LonelyCat124 Jul 24, 2024
57ed1b2
update for the review
LonelyCat124 Jul 24, 2024
b3b43a1
Merged master
LonelyCat124 Jul 25, 2024
5a9a0c4
Add missing coverage
LonelyCat124 Jul 25, 2024
0aee901
Forgot the routine adjustment
LonelyCat124 Jul 25, 2024
c5bc99d
More changes towards PR review
LonelyCat124 Jul 29, 2024
3dfd87f
Flaking errors
LonelyCat124 Jul 31, 2024
77bd320
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Jul 31, 2024
ec8c85d
Fixing some coverage mistakes
LonelyCat124 Jul 31, 2024
13e9b2c
More changes for review
LonelyCat124 Aug 1, 2024
876e948
doc fixes and refactor
LonelyCat124 Aug 1, 2024
f6cb7a6
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Aug 1, 2024
5dcdcb4
fixed broken test in module info - maybe will break coverage
LonelyCat124 Aug 1, 2024
ab32093
Changes to how _parent is set in routine
LonelyCat124 Aug 2, 2024
a355683
Add a symbol setter to Routine
LonelyCat124 Aug 8, 2024
369429f
Fix the discussed changes
LonelyCat124 Aug 12, 2024
b870bd2
Merged master
LonelyCat124 Aug 12, 2024
ed71563
Routine's now add their routine symbol to their own table when detach…
LonelyCat124 Aug 23, 2024
df5fdcf
Removed dead code
LonelyCat124 Aug 23, 2024
64effba
Missing coverage for .symbol type checking
LonelyCat124 Aug 23, 2024
7e8a78a
Changes towards the review
LonelyCat124 Aug 30, 2024
e63caf7
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Aug 30, 2024
443e942
Merged master
LonelyCat124 Sep 12, 2024
bab2459
Review update
LonelyCat124 Sep 20, 2024
45ec6ef
changed comment positions
LonelyCat124 Sep 20, 2024
8445909
Fixed a bug with subroutines contained within other scopes and introd…
LonelyCat124 Sep 27, 2024
4a7f7d2
Need an empty list for routines when there are none
LonelyCat124 Sep 30, 2024
02abd0b
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Sep 30, 2024
01ff331
Fixed the reverted coverage by fixing module_info and removing now un…
LonelyCat124 Sep 30, 2024
76bfccc
Merge branch 'master' into 2592_Routine_symbol
LonelyCat124 Sep 30, 2024
802b692
Fixed the case with storng in NEMO and added a test corresponding to …
LonelyCat124 Oct 1, 2024
d167757
Merge branch '2592_Routine_symbol' of github.com:stfc/PSyclone into 2…
LonelyCat124 Oct 1, 2024
0511465
#2596 Remove instances of own_routine_symbol tabs and other unneeded …
sergisiso Oct 2, 2024
f87dea0
#2596 Update changelog
sergisiso Oct 2, 2024
ab65d7a
#2596 Fix flake8 issues
sergisiso Oct 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@
79) PR #2687 for #2684. Add support for WHERE constructs that contain
references without range-notation (and prevent some incorrect cases).

80) PR #2596 for #2592 and #2582. Routine nodes manage their own
symbols from their parent scope.

release 2.5.0 14th of February 2024

1) PR #2199 for #2189. Fix bugs with missing maps in enter data
Expand Down
5 changes: 1 addition & 4 deletions examples/xdsl/backend/xdsl.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,10 +285,7 @@ def gen_access_stmt(self, symbol_table):
def gen_routine_access_stmts(self, symbol_table):
# Find the symbol that represents itself, this one will not need
# an accessibility statement
try:
itself = symbol_table.lookup_with_tag('own_routine_symbol')
except KeyError:
itself = None
itself = self.symbol

public_routines = []
private_routines = []
Expand Down
2 changes: 1 addition & 1 deletion src/psyclone/domain/common/extract_driver_creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ def create(self, nodes, read_write_info, prefix, postfix, region_name):
file_container = FileContainer(unit_name)

# Create the program and add it to the file container:
program = Routine(unit_name, is_program=True)
program = Routine.create(unit_name, is_program=True)
program_symbol_table = program.symbol_table
file_container.addchild(program)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,14 +377,12 @@ def apply(self, node, options=None):

container = node.ancestor(Container)
if not existing_symbol:
# If it doesn't exist already, module-inline the subroutine by:
# 1) Registering the subroutine symbol in the Container
routine_symbol = RoutineSymbol(
callee_name, interface=DefaultModuleInterface(),
visibility=Symbol.Visibility.PRIVATE)
container.symbol_table.add(routine_symbol)
# 2) Insert the relevant code into the tree.
container.addchild(code_to_inline.detach())
# If it doesn't exist already, module-inline the subroutine by
# inserting the relevant code into the tree.
# We need to set the visibility of the routine's symbol to
# be private.
code_to_inline.symbol.visibility = Symbol.Visibility.PRIVATE
node.ancestor(Container).addchild(code_to_inline.detach())
else:
if existing_symbol.is_import:
# The RoutineSymbol is in the table but that is because it is
Expand All @@ -400,7 +398,10 @@ def apply(self, node, options=None):
existing_symbol.visibility = Symbol.Visibility.PRIVATE
if remove_csym:
ctable.remove(csym)
container.addchild(code_to_inline.detach())
code_to_inline = code_to_inline.detach()
# Set the routine's symbol to the existing_symbol
code_to_inline.symbol = existing_symbol
container.addchild(code_to_inline)
else:
# The routine symbol already exists, and we know from the
# validation that it's a Routine. Now check if they are
Expand All @@ -427,8 +428,14 @@ def apply(self, node, options=None):
routine_symbol = existing_symbol
table = routine_symbol.find_symbol_table(node)
if table.node is not container:
container.symbol_table.add(routine_symbol)
table.remove(routine_symbol)
# Set the visibility of the symbol to always be private.
sym = container.symbol_table.lookup(routine_symbol.name)
sym.visibility = Symbol.Visibility.PRIVATE
# Force removal of the routine_symbol if its also present in
# the Routine's symbol table.
table.lookup(routine_symbol.name)
norm_name = table._normalize(routine_symbol.name)
table._symbols.pop(norm_name)

# We only modify the kernel call name after the equality check to
# ensure the apply will succeed and we don't leave with an inconsistent
Expand Down
101 changes: 49 additions & 52 deletions src/psyclone/domain/gocean/transformations/gocean_opencl_trans.py
Original file line number Diff line number Diff line change
Expand Up @@ -932,9 +932,12 @@ def _insert_ocl_arg_setter_routine(node, kernel):
# will generate it.
pass

# Create the new Routine and RoutineSymbol
node.symbol_table.add(RoutineSymbol(sub_name), tag=sub_name)
argsetter = Routine(sub_name)
# Create the new Routine
sub_name = node.symbol_table.next_available_name(sub_name)
sub_symbol = node.symbol_table.new_symbol(
sub_name, tag=kernel.name + "_set_args",
symbol_type=RoutineSymbol)
argsetter = Routine(sub_symbol)
arg_list = []

# Add subroutine imported symbols
Expand Down Expand Up @@ -1018,7 +1021,7 @@ def _insert_ocl_arg_setter_routine(node, kernel):
# Add the subroutine as child of the provided node
node.addchild(argsetter)

return node.symbol_table.lookup_with_tag(sub_name)
return node.symbol_table.lookup_with_tag(kernel.name + "_set_args")
sergisiso marked this conversation as resolved.
Show resolved Hide resolved

def _insert_opencl_init_routine(self, node):
'''
Expand All @@ -1044,11 +1047,6 @@ def _insert_opencl_init_routine(self, node):
# will generate it.
pass

# Create the symbol for the routine and add it to the symbol table.
subroutine_name = symtab.new_symbol("psy_init",
symbol_type=RoutineSymbol,
tag="ocl_init_routine").name

# Choose a round-robin device number if it has MPI and multiple
# accelerators.
distributed_memory = Config.get().distributed_memory
Expand Down Expand Up @@ -1093,15 +1091,19 @@ def _insert_opencl_init_routine(self, node):
end if
end subroutine psy_init'''

# Create the symbol for the routine.
subroutine_symbol = RoutineSymbol("psy_init")

# Obtain the PSyIR representation of the code above
fortran_reader = FortranReader()
container = fortran_reader.psyir_from_source(code)
subroutine = container.children[0]
# Rename subroutine
subroutine.name = subroutine_name
subroutine.detach()
node.symbol_table.add(subroutine_symbol, tag="ocl_init_routine")
subroutine.symbol = subroutine_symbol

# Add the subroutine as child of the provided node
node.addchild(subroutine.detach())
node.addchild(subroutine)

return symtab.lookup_with_tag("ocl_init_routine")

Expand All @@ -1128,11 +1130,6 @@ def _insert_initialise_grid_buffers(node):
# will generate it.
pass

# Create the symbol for the routine and add it to the symbol table.
subroutine_name = symtab.new_symbol("initialise_grid_device_buffers",
symbol_type=RoutineSymbol,
tag="ocl_init_grid_buffers").name

# Get the GOcean API property names used in this routine
api_config = Config.get().api_conf("gocean")
props = api_config.grid_properties
Expand Down Expand Up @@ -1188,15 +1185,19 @@ def _insert_initialise_grid_buffers(node):
end subroutine initialise_device_grid
'''

# Create the symbol for the routine.
subroutine_symbol = RoutineSymbol("initialise_grid_device_buffers")

# Obtain the PSyIR representation of the code above
fortran_reader = FortranReader()
container = fortran_reader.psyir_from_source(code)
subroutine = container.children[0]
# Rename subroutine
subroutine.name = subroutine_name

symtab.add(subroutine_symbol, tag="ocl_init_grid_buffers")
subroutine.detach()
subroutine.symbol = subroutine_symbol
# Add the subroutine as child of the provided node
node.addchild(subroutine.detach())
node.addchild(subroutine)

return symtab.lookup_with_tag("ocl_init_grid_buffers")

Expand All @@ -1222,11 +1223,6 @@ def _insert_write_grid_buffers(self, node):
# will generate it.
pass

# Create the symbol for the routine and add it to the symbol table.
subroutine_name = symtab.new_symbol("write_grid_buffers",
symbol_type=RoutineSymbol,
tag="ocl_write_grid_buffers").name

# Get the GOcean API property names used in this routine
api_config = Config.get().api_conf("gocean")
props = api_config.grid_properties
Expand Down Expand Up @@ -1271,15 +1267,19 @@ def _insert_write_grid_buffers(self, node):
code += write_str.format(grid_prop, self._OCL_MANAGEMENT_QUEUE)
code += "end subroutine write_device_grid"

# Create the symbol for the routine.
subroutine_symbol = RoutineSymbol("write_grid_buffers")

# Obtain the PSyIR representation of the code above
fortran_reader = FortranReader()
container = fortran_reader.psyir_from_source(code)
subroutine = container.children[0]
# Rename subroutine
subroutine.name = subroutine_name

symtab.add(subroutine_symbol, tag="ocl_write_grid_buffers")
subroutine.detach()
subroutine.symbol = subroutine_symbol
# Add the subroutine as child of the provided node
node.addchild(subroutine.detach())
node.addchild(subroutine)

return symtab.lookup_with_tag("ocl_write_grid_buffers")

Expand All @@ -1304,11 +1304,6 @@ def _insert_ocl_read_from_device_function(self, node):
# generated first.
pass

# Create the symbol for the routine and add it to the symbol table.
subroutine_name = symtab.new_symbol("read_from_device",
symbol_type=RoutineSymbol,
tag="ocl_read_func").name

# Code of the subroutine in Fortran
code = f'''
subroutine read_sub(from, to, startx, starty, nx, ny, blocking)
Expand Down Expand Up @@ -1363,16 +1358,20 @@ def _insert_ocl_read_from_device_function(self, node):
end subroutine read_sub
'''

# Create the symbol for the routine.
subroutine_symbol = RoutineSymbol("read_from_device")

# Obtain the PSyIR representation of the code above
fortran_reader = FortranReader()
container = fortran_reader.psyir_from_source(code)
subroutine = container.children[0]

# Rename subroutine
subroutine.name = subroutine_name
symtab.add(subroutine_symbol, tag="ocl_read_func")
subroutine.detach()
subroutine.symbol = subroutine_symbol

# Add the subroutine as child of the provided node
node.addchild(subroutine.detach())
node.addchild(subroutine)

return symtab.lookup_with_tag("ocl_read_func")

Expand All @@ -1397,11 +1396,6 @@ def _insert_ocl_write_to_device_function(self, node):
# generated first.
pass

# Create the symbol for the routine and add it to the symbol table.
subroutine_name = symtab.new_symbol("write_to_device",
symbol_type=RoutineSymbol,
tag="ocl_write_func").name

# Code of the subroutine in Fortran
code = f'''
subroutine write_sub(from, to, startx, starty, nx, ny, blocking)
Expand Down Expand Up @@ -1455,16 +1449,20 @@ def _insert_ocl_write_to_device_function(self, node):
endif
end subroutine write_sub
'''
# Create the symbol for the routine.
subroutine_symbol = RoutineSymbol("write_to_device")

# Obtain the PSyIR representation of the code above
fortran_reader = FortranReader()
container = fortran_reader.psyir_from_source(code)
subroutine = container.children[0]
# Rename subroutine
subroutine.name = subroutine_name

symtab.add(subroutine_symbol, tag="ocl_write_func")
subroutine.detach()
subroutine.symbol = subroutine_symbol

# Add the subroutine as child of the provided node
node.addchild(subroutine.detach())
node.addchild(subroutine)

return symtab.lookup_with_tag("ocl_write_func")

Expand All @@ -1490,11 +1488,6 @@ def _insert_ocl_initialise_buffer(node):
# will generate it.
pass

# Create the symbol for the routine and add it to the symbol table.
subroutine_name = symtab.new_symbol("initialise_device_buffer",
symbol_type=RoutineSymbol,
tag="ocl_init_buffer_func").name

# Get the GOcean API property names used in this routine
api_config = Config.get().api_conf("gocean")
host_buff = \
Expand Down Expand Up @@ -1532,15 +1525,19 @@ def _insert_ocl_initialise_buffer(node):
end subroutine initialise_device_buffer
'''

# Create the symbol for the routine .
subroutine_symbol = RoutineSymbol("initialise_device_buffer")

# Obtain the PSyIR representation of the code above
fortran_reader = FortranReader()
container = fortran_reader.psyir_from_source(code)
subroutine = container.children[0]
# Rename subroutine
subroutine.name = subroutine_name
symtab.add(subroutine_symbol, tag="ocl_init_buffer_func")
subroutine.detach()
subroutine.symbol = subroutine_symbol

# Add the subroutine as child of the provided node
node.addchild(subroutine.detach())
node.addchild(subroutine)

return symtab.lookup_with_tag("ocl_init_buffer_func")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ def apply(self, node, options=None):
# The validate() method has already checked that the routine exists.
# pylint: disable=undefined-loop-variable
gotable = GOSymbolTable.create_from_table(routine.symbol_table)
gokernsched = GOKernelSchedule(metadata.procedure_name,
gokernsched = GOKernelSchedule(routine.symbol,
symbol_table=gotable.detach())
for child in routine.pop_all_children():
gokernsched.addchild(child)
Expand Down
2 changes: 1 addition & 1 deletion src/psyclone/domain/lfric/algorithm/lfric_alg.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def create_alg_routine(name):
# pylint: disable=protected-access
# TODO #1954 Remove the protected access using a factory
ScopingNode._symbol_table_class = LFRicSymbolTable
alg_sub = Routine(name)
alg_sub = Routine.create(name)
table = alg_sub.symbol_table

# Create Container and Type Symbols for each of the modules/types that
Expand Down
2 changes: 1 addition & 1 deletion src/psyclone/domain/lfric/lfric_extract_driver_creator.py
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ def create(self, nodes, read_write_info, prefix, postfix, region_name):
file_container = FileContainer(unit_name)

# Create the program and add it to the file container:
program = Routine(unit_name, is_program=True)
program = Routine.create(unit_name, is_program=True)
program_symbol_table = program.symbol_table
file_container.addchild(program)

Expand Down
1 change: 0 additions & 1 deletion src/psyclone/domain/lfric/lfric_invoke.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ def __init__(self, alg_invocation, idx, invokes):
# Import here to avoid circular dependency
# pylint: disable=import-outside-toplevel
from psyclone.domain.lfric import LFRicInvokeSchedule
self._schedule = LFRicInvokeSchedule('name', None) # for pyreverse
reserved_names_list = []
const = LFRicConstants()
reserved_names_list.extend(const.STENCIL_MAPPING.values())
Expand Down
23 changes: 16 additions & 7 deletions src/psyclone/domain/lfric/lfric_invoke_schedule.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,30 @@ class LFRicInvokeSchedule(InvokeSchedule):
specific factories for creating kernel and infrastructure calls
to the base class so it creates the ones we require.
:param str name: name of the Invoke.
:param arg: list of KernelCalls parsed from the algorithm layer.
:type arg: list of :py:class:`psyclone.parse.algorithm.KernelCall`
:param symbol: symbol representing the Invoke.
:type symbol: :py:class:`psyclone.psyir.symbols.RoutineSymbol`
:param alg_calls: optional list of KernelCalls parsed from the
algorithm layer.
:type alg_calls: Optional[list of
:py:class:`psyclone.parse.algorithm.KernelCall`]
:param reserved_names: optional list of names that are not allowed in the
new InvokeSchedule SymbolTable.
:type reserved_names: list[str]
:param parent: the parent of this node in the PSyIR.
:type parent: :py:class:`psyclone.psyir.nodes.Node`
'''
# LFRicInvokeSchedule always uses an LFRicSymbolTable for its inner scope
# symbol table.
_symbol_table_class = LFRicSymbolTable

def __init__(self, name, arg, reserved_names=None, parent=None):
super().__init__(name, LFRicKernCallFactory,
LFRicBuiltInCallFactory, arg, reserved_names,
parent=parent, symbol_table=LFRicSymbolTable())
def __init__(self, symbol, alg_calls=None, reserved_names=None,
parent=None, **kwargs):
if not alg_calls:
alg_calls = []
super().__init__(symbol, LFRicKernCallFactory,
LFRicBuiltInCallFactory, alg_calls, reserved_names,
parent=parent, **kwargs)

def node_str(self, colour=True):
''' Creates a text summary of this node.
Expand Down
2 changes: 1 addition & 1 deletion src/psyclone/domain/lfric/lfric_kern.py
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,7 @@ class creates the PSyIR schedule on first invocation which is
# symbols. For the moment we just return the unmodified PSyIR schedule
# but this should use RaisePSyIR2LFRicKernTrans once KernelInterface
# is fully functional (#928).
ksched = KernelSchedule(sched.name,
ksched = KernelSchedule(sched.symbol,
symbol_table=sched.symbol_table.detach())
for child in sched.pop_all_children():
ksched.addchild(child)
Expand Down
Loading
Loading