From c3f953522d6d5acf969d55dba729a937b412a08e Mon Sep 17 00:00:00 2001 From: Alexandros Nikolaos Ziogas Date: Wed, 12 Jul 2023 21:36:28 +0200 Subject: [PATCH 01/10] Drop connectors/arguments from (nested) Program/SDFG call, if the connector is not in the SDFG's arrays. --- dace/frontend/python/newast.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/dace/frontend/python/newast.py b/dace/frontend/python/newast.py index 52a6862083..e1629e20c6 100644 --- a/dace/frontend/python/newast.py +++ b/dace/frontend/python/newast.py @@ -3740,6 +3740,15 @@ def _parse_sdfg_call(self, funcname: str, func: Union[SDFG, SDFGConvertible], no for arg in args_to_remove: args.remove(arg) + # Drop args that are not in the SDFG + filtered_args = [] + for conn, arg in args: + if conn not in sdfg.arrays: + warnings.warn(f'Connector {conn} not found in SDFG; dropping it') + else: + filtered_args.append((conn, arg)) + args = filtered_args + # Change connector names updated_args = [] arrays_before = list(sdfg.arrays.items()) @@ -3829,6 +3838,12 @@ def _parse_sdfg_call(self, funcname: str, func: Union[SDFG, SDFGConvertible], no for k, v in argdict.items() if self._is_outputnode(sdfg, k) } + # If an argument does not register as input nor as output, put it in the inputs. + # This may happen with input arguments that are used to set a promoted scalar. + for k, v in argdict.items(): + if k not in inputs.keys() and k not in outputs.keys(): + inputs[k] = v + # Add closure to global inputs/outputs (e.g., if processed as part of a map) for arrname in closure_arrays.keys(): if arrname not in names_to_replace: @@ -3840,13 +3855,6 @@ def _parse_sdfg_call(self, funcname: str, func: Union[SDFG, SDFGConvertible], no if narrname in outputs: self.outputs[arrname] = (state, outputs[narrname], []) - # If an argument does not register as input nor as output, - # put it in the inputs. - # This may happen with input argument that are used to set - # a promoted scalar. - for k, v in argdict.items(): - if k not in inputs.keys() and k not in outputs.keys(): - inputs[k] = v # Unset parent inputs/read accesses that # turn out to be outputs/write accesses. for memlet in outputs.values(): From 866e38f605cde529acbb598d496c8271e2cdc463 Mon Sep 17 00:00:00 2001 From: Alexandros Nikolaos Ziogas Date: Thu, 13 Jul 2023 13:39:24 +0200 Subject: [PATCH 02/10] Ensure that the access node exists in the SDFGState. --- dace/transformation/passes/array_elimination.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dace/transformation/passes/array_elimination.py b/dace/transformation/passes/array_elimination.py index e313f7bf66..d1b80c2327 100644 --- a/dace/transformation/passes/array_elimination.py +++ b/dace/transformation/passes/array_elimination.py @@ -170,6 +170,9 @@ def remove_redundant_copies(self, sdfg: SDFG, state: SDFGState, removable_data: for anode in access_nodes[aname]: if anode in removed_nodes: continue + if anode not in state.nodes(): + removed_nodes.add(anode) + continue if state.out_degree(anode) == 1: succ = state.successors(anode)[0] From 7ae787d7db85b6bbf04ae0be62d3c644b59b9dde Mon Sep 17 00:00:00 2001 From: Alexandros Nikolaos Ziogas Date: Fri, 14 Jul 2023 14:27:29 +0200 Subject: [PATCH 03/10] Don't add child SDFG's closure arrays to parent SDFG's arrays and to child SDFG's arguments if the array is not actually in the child SDFG. --- dace/frontend/python/newast.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/dace/frontend/python/newast.py b/dace/frontend/python/newast.py index e1629e20c6..23e1bd9134 100644 --- a/dace/frontend/python/newast.py +++ b/dace/frontend/python/newast.py @@ -3653,6 +3653,11 @@ def _parse_sdfg_call(self, funcname: str, func: Union[SDFG, SDFGConvertible], no # If the symbol is a callback, but is not used in the nested SDFG, skip it continue + # NOTE: Is it possible that an array in the SDFG's closure is not in the SDFG? + # NOTE: Perhaps its use was simplified/optimized away? + if aname not in sdfg.arrays: + continue + # First, we do an inverse lookup on the already added closure arrays for `arr`. is_new_arr = True for k, v in self.nested_closure_arrays.items(): @@ -3740,15 +3745,6 @@ def _parse_sdfg_call(self, funcname: str, func: Union[SDFG, SDFGConvertible], no for arg in args_to_remove: args.remove(arg) - # Drop args that are not in the SDFG - filtered_args = [] - for conn, arg in args: - if conn not in sdfg.arrays: - warnings.warn(f'Connector {conn} not found in SDFG; dropping it') - else: - filtered_args.append((conn, arg)) - args = filtered_args - # Change connector names updated_args = [] arrays_before = list(sdfg.arrays.items()) From 7343f55e2dca5c06721c7b9bf3448b2ca0f1637e Mon Sep 17 00:00:00 2001 From: Samuel Martin Date: Thu, 20 Jul 2023 13:00:02 +0200 Subject: [PATCH 04/10] state.read_and_write_sets() take into account when read happens directly after write --- dace/sdfg/state.py | 16 +++++++++++++--- tests/sdfg/state_test.py | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) create mode 100644 tests/sdfg/state_test.py diff --git a/dace/sdfg/state.py b/dace/sdfg/state.py index 0796bf00d0..0354dd107b 100644 --- a/dace/sdfg/state.py +++ b/dace/sdfg/state.py @@ -298,10 +298,11 @@ def scope_tree(self) -> 'dace.sdfg.scope.ScopeTree': # Get scopes for node, scopenodes in sdc.items(): + scope_exit_nodes = [v for v in scopenodes if isinstance(v, nd.ExitNode)] if node is None: exit_node = None else: - exit_node = next(v for v in scopenodes if isinstance(v, nd.ExitNode)) + exit_node = next(iter(scope_exit_nodes)) scope = ScopeTree(node, exit_node) result[node] = scope @@ -502,13 +503,22 @@ def _read_and_write_sets(self) -> Tuple[Dict[AnyStr, List[Subset]], Dict[AnyStr, # is read is not counted in the read set for n in utils.dfs_topological_sort(sg, sources=sg.source_nodes()): if isinstance(n, nd.AccessNode): - for e in sg.in_edges(n): + in_edges = sg.in_edges(n) + out_edges = sg.out_edges(n) + # Filter out memlets which go out but the same data is written to the AccessNode by another memlet + for out_edge in out_edges: + for in_edge in in_edges: + if in_edge.data.data == out_edge.data.data and \ + in_edge.data.dst_subset.covers(out_edge.data.src_subset): + out_edges.remove(out_edge) + + for e in in_edges: # skip empty memlets if e.data.is_empty(): continue # Store all subsets that have been written ws[n.data].append(e.data.subset) - for e in sg.out_edges(n): + for e in out_edges: # skip empty memlets if e.data.is_empty(): continue diff --git a/tests/sdfg/state_test.py b/tests/sdfg/state_test.py new file mode 100644 index 0000000000..07e2e8c4c7 --- /dev/null +++ b/tests/sdfg/state_test.py @@ -0,0 +1,23 @@ +import dace + + +def test_read_write_set(): + sdfg = dace.SDFG('graph') + A = sdfg.add_array('A', [10], dace.float64) + B = sdfg.add_array('B', [10], dace.float64) + C = sdfg.add_array('C', [10], dace.float64) + state = sdfg.add_state('state') + task1 = state.add_tasklet('work1', {'A'}, {'B'}, 'B = A + 1') + task2 = state.add_tasklet('work2', {'B'}, {'C'}, 'C = B + 1') + read_a = state.add_access('A') + rw_b = state.add_access('B') + write_c = state.add_access('C') + state.add_memlet_path(read_a, task1, dst_conn='A', memlet=dace.Memlet('A[2]')) + state.add_memlet_path(task1, rw_b, src_conn='B', memlet=dace.Memlet('B[2]')) + state.add_memlet_path(rw_b, task2, dst_conn='B', memlet=dace.Memlet('B[2]')) + state.add_memlet_path(task2, write_c, src_conn='C', memlet=dace.Memlet('C[2]')) + + assert 'B' not in state.read_and_write_sets()[0] + +if __name__ == '__main__': + test_read_write_set() From bb00fea35013b66c153d89c5ba31ede70c0120d2 Mon Sep 17 00:00:00 2001 From: Samuel Martin Date: Thu, 20 Jul 2023 14:07:48 +0200 Subject: [PATCH 05/10] Change RefineNestedAccess to only look at memlets which are in the read and write set --- dace/transformation/interstate/sdfg_nesting.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/dace/transformation/interstate/sdfg_nesting.py b/dace/transformation/interstate/sdfg_nesting.py index 1b9324546a..71d9e22aca 100644 --- a/dace/transformation/interstate/sdfg_nesting.py +++ b/dace/transformation/interstate/sdfg_nesting.py @@ -925,7 +925,12 @@ def _candidates( continue # For now we only detect one element + read_set, write_set = nstate.read_and_write_sets() for e in nstate.in_edges(dnode): + if e.data.data not in write_set: + # Skip data which is not in the read and write set of the state -> there also won't be a + # connector + continue # If more than one unique element detected, remove from # candidates if e.data.data in out_candidates: @@ -941,6 +946,10 @@ def _candidates( continue out_candidates[e.data.data] = (e.data, nstate, set(range(len(e.data.subset)))) for e in nstate.out_edges(dnode): + if e.data.data not in read_set: + # Skip data which is not in the read and write set of the state -> there also won't be a + # connector + continue # If more than one unique element detected, remove from # candidates if e.data.data in in_candidates: From 7d29defb511ffc7ba5cb88d440b8e45bc7988e99 Mon Sep 17 00:00:00 2001 From: Tal Ben-Nun Date: Thu, 20 Jul 2023 15:28:42 -0700 Subject: [PATCH 06/10] Relax test for inter-state edges in default schedules --- dace/sdfg/validation.py | 17 ++++++++++++----- tests/sdfg/disallowed_access_test.py | 23 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/dace/sdfg/validation.py b/dace/sdfg/validation.py index 3bac646479..4fbc808fdd 100644 --- a/dace/sdfg/validation.py +++ b/dace/sdfg/validation.py @@ -42,7 +42,7 @@ def validate_sdfg(sdfg: 'dace.sdfg.SDFG', references: Set[int] = None, **context """ # Avoid import loop from dace.codegen.targets import fpga - from dace.sdfg.scope import is_devicelevel_gpu, is_devicelevel_fpga + from dace.sdfg.scope import is_devicelevel_gpu, is_devicelevel_fpga, is_in_scope references = references or set() @@ -171,10 +171,17 @@ def validate_sdfg(sdfg: 'dace.sdfg.SDFG', references: Set[int] = None, **context for memlet in ise_memlets: container = memlet.data if not _accessible(sdfg, container, context): - eid = sdfg.edge_id(edge) - raise InvalidSDFGInterstateEdgeError( - f'Trying to read an inaccessible data container "{container}" ' - f'(Storage: {sdfg.arrays[container].storage}) in host code interstate edge', sdfg, eid) + # Check context w.r.t. maps + in_default_scope = False + if sdfg.parent_nsdfg_node is not None: + if is_in_scope(sdfg.parent_sdfg, sdfg.parent, sdfg.parent_nsdfg_node, + [dtypes.ScheduleType.Default]): + in_default_scope = True + if not in_default_scope: + eid = sdfg.edge_id(edge) + raise InvalidSDFGInterstateEdgeError( + f'Trying to read an inaccessible data container "{container}" ' + f'(Storage: {sdfg.arrays[container].storage}) in host code interstate edge', sdfg, eid) # Add edge symbols into defined symbols symbols.update(issyms) diff --git a/tests/sdfg/disallowed_access_test.py b/tests/sdfg/disallowed_access_test.py index 8700e34db5..520481ea46 100644 --- a/tests/sdfg/disallowed_access_test.py +++ b/tests/sdfg/disallowed_access_test.py @@ -40,6 +40,7 @@ def test_gpu_access_on_host_interstate_invalid(): @pytest.mark.gpu def test_gpu_access_on_host_tasklet(): + @dace.program def tester(a: dace.float64[20] @ dace.StorageType.GPU_Global): for i in dace.map[0:20] @ dace.ScheduleType.CPU_Multicore: @@ -49,7 +50,29 @@ def tester(a: dace.float64[20] @ dace.StorageType.GPU_Global): tester.to_sdfg(validate=True) +@pytest.mark.gpu +def test_gpu_access_on_device_interstate_edge_default(): + sdfg = dace.SDFG('tester') + sdfg.add_array('A', [20], dace.float64, storage=dace.StorageType.GPU_Global) + state = sdfg.add_state() + + me, mx = state.add_map('test', dict(i='0:20')) + + nsdfg = dace.SDFG('nester') + nsdfg.add_array('A', [20], dace.float64, storage=dace.StorageType.GPU_Global) + state1 = nsdfg.add_state() + state2 = nsdfg.add_state() + nsdfg.add_edge(state1, state2, dace.InterstateEdge(assignments=dict(s='A[4]'))) + + nsdfg_node = state.add_nested_sdfg(nsdfg, None, {'A'}, {}) + state.add_memlet_path(state.add_read('A'), me, nsdfg_node, dst_conn='A', memlet=dace.Memlet('A[0:20]')) + state.add_nedge(nsdfg_node, mx, dace.Memlet()) + + sdfg.validate() + + if __name__ == '__main__': test_gpu_access_on_host_interstate_ok() test_gpu_access_on_host_interstate_invalid() test_gpu_access_on_host_tasklet() + test_gpu_access_on_device_interstate_edge_default() From cbe344c15dfd5668d0ddb5a419279fc12c0ef1da Mon Sep 17 00:00:00 2001 From: Tal Ben-Nun Date: Thu, 20 Jul 2023 15:34:17 -0700 Subject: [PATCH 07/10] Lazy-evaluate in_default_scope --- dace/sdfg/validation.py | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/dace/sdfg/validation.py b/dace/sdfg/validation.py index 4fbc808fdd..aa7674ca45 100644 --- a/dace/sdfg/validation.py +++ b/dace/sdfg/validation.py @@ -111,6 +111,7 @@ def validate_sdfg(sdfg: 'dace.sdfg.SDFG', references: Set[int] = None, **context # Check if SDFG is located within a GPU kernel context['in_gpu'] = is_devicelevel_gpu(sdfg, None, None) context['in_fpga'] = is_devicelevel_fpga(sdfg, None, None) + in_default_scope = None # Check every state separately start_state = sdfg.start_state @@ -172,12 +173,13 @@ def validate_sdfg(sdfg: 'dace.sdfg.SDFG', references: Set[int] = None, **context container = memlet.data if not _accessible(sdfg, container, context): # Check context w.r.t. maps - in_default_scope = False - if sdfg.parent_nsdfg_node is not None: - if is_in_scope(sdfg.parent_sdfg, sdfg.parent, sdfg.parent_nsdfg_node, - [dtypes.ScheduleType.Default]): - in_default_scope = True - if not in_default_scope: + if in_default_scope is None: # Lazy-evaluate in_default_scope + in_default_scope = False + if sdfg.parent_nsdfg_node is not None: + if is_in_scope(sdfg.parent_sdfg, sdfg.parent, sdfg.parent_nsdfg_node, + [dtypes.ScheduleType.Default]): + in_default_scope = True + if in_default_scope is False: eid = sdfg.edge_id(edge) raise InvalidSDFGInterstateEdgeError( f'Trying to read an inaccessible data container "{container}" ' @@ -226,9 +228,17 @@ def validate_sdfg(sdfg: 'dace.sdfg.SDFG', references: Set[int] = None, **context for memlet in ise_memlets: container = memlet.data if not _accessible(sdfg, container, context): - raise InvalidSDFGInterstateEdgeError( - f'Trying to read an inaccessible data container "{container}" ' - f'(Storage: {sdfg.arrays[container].storage}) in host code interstate edge', sdfg, eid) + # Check context w.r.t. maps + if in_default_scope is None: # Lazy-evaluate in_default_scope + in_default_scope = False + if sdfg.parent_nsdfg_node is not None: + if is_in_scope(sdfg.parent_sdfg, sdfg.parent, sdfg.parent_nsdfg_node, + [dtypes.ScheduleType.Default]): + in_default_scope = True + if in_default_scope is False: + raise InvalidSDFGInterstateEdgeError( + f'Trying to read an inaccessible data container "{container}" ' + f'(Storage: {sdfg.arrays[container].storage}) in host code interstate edge', sdfg, eid) except InvalidSDFGError as ex: # If the SDFG is invalid, save it From 08aac34a8fdd6cd38860d5907e7bca925248a20a Mon Sep 17 00:00:00 2001 From: Samuel Martin Date: Fri, 21 Jul 2023 08:47:39 +0200 Subject: [PATCH 08/10] Add review changes --- dace/sdfg/state.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/dace/sdfg/state.py b/dace/sdfg/state.py index 0354dd107b..8059609c36 100644 --- a/dace/sdfg/state.py +++ b/dace/sdfg/state.py @@ -298,11 +298,10 @@ def scope_tree(self) -> 'dace.sdfg.scope.ScopeTree': # Get scopes for node, scopenodes in sdc.items(): - scope_exit_nodes = [v for v in scopenodes if isinstance(v, nd.ExitNode)] if node is None: exit_node = None else: - exit_node = next(iter(scope_exit_nodes)) + exit_node = next(v for v in scopenodes if isinstance(v, nd.ExitNode)) scope = ScopeTree(node, exit_node) result[node] = scope @@ -506,10 +505,10 @@ def _read_and_write_sets(self) -> Tuple[Dict[AnyStr, List[Subset]], Dict[AnyStr, in_edges = sg.in_edges(n) out_edges = sg.out_edges(n) # Filter out memlets which go out but the same data is written to the AccessNode by another memlet - for out_edge in out_edges: - for in_edge in in_edges: - if in_edge.data.data == out_edge.data.data and \ - in_edge.data.dst_subset.covers(out_edge.data.src_subset): + for out_edge in list(out_edges): + for in_edge in list(in_edges): + if (in_edge.data.data == out_edge.data.data and + in_edge.data.dst_subset.covers(out_edge.data.src_subset)): out_edges.remove(out_edge) for e in in_edges: From 851f1fa4f7041c4c3e1be564ae7c92929bb2dbc3 Mon Sep 17 00:00:00 2001 From: Samuel Martin Date: Fri, 21 Jul 2023 08:48:05 +0200 Subject: [PATCH 09/10] Apply suggestions from code review Co-authored-by: alexnick83 <31545860+alexnick83@users.noreply.github.com> --- tests/sdfg/state_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/sdfg/state_test.py b/tests/sdfg/state_test.py index 07e2e8c4c7..c5cb953c4d 100644 --- a/tests/sdfg/state_test.py +++ b/tests/sdfg/state_test.py @@ -1,3 +1,4 @@ +# Copyright 2019-2023 ETH Zurich and the DaCe authors. All rights reserved. import dace From 9a1db886905fb037e92b96b0fbefa5c5074ba73d Mon Sep 17 00:00:00 2001 From: Samuel Martin Date: Fri, 21 Jul 2023 08:49:45 +0200 Subject: [PATCH 10/10] Added myself to AUTHORS file --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 573f142cf9..48cb4c05ec 100644 --- a/AUTHORS +++ b/AUTHORS @@ -36,5 +36,6 @@ Reid Wahl Yihang Luo Alexandru Calotoiu Phillip Lane +Samuel Martin and other contributors listed in https://github.com/spcl/dace/graphs/contributors