Skip to content

Commit

Permalink
Address more review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
phschaad committed Dec 10, 2024
1 parent 3a2b342 commit 2f7d6aa
Show file tree
Hide file tree
Showing 11 changed files with 47 additions and 93 deletions.
6 changes: 6 additions & 0 deletions dace/sdfg/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,9 @@ def nodes(self):
def edges(self):
return []

def sub_regions(self) -> List['AbstractControlFlowRegion']:
return []

def set_default_lineinfo(self, lineinfo: dace.dtypes.DebugInfo):
"""
Sets the default source line information to be lineinfo, or None to
Expand Down Expand Up @@ -3389,6 +3392,9 @@ def __init__(self, label: str = '', sdfg: Optional['SDFG'] = None, parent: Optio
super().__init__(label, sdfg, parent)
self._branches = []

def sub_regions(self):
return [b for _, b in self.branches]

def __str__(self):
return self._label

Expand Down
16 changes: 9 additions & 7 deletions dace/sdfg/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1305,13 +1305,13 @@ def fuse_states(sdfg: SDFG, permissive: bool = False, progress: bool = None) ->


def inline_control_flow_regions(sdfg: SDFG, types: Optional[List[Type[AbstractControlFlowRegion]]] = None,
blacklist: Optional[List[Type[AbstractControlFlowRegion]]] = None,
ignore_region_types: Optional[List[Type[AbstractControlFlowRegion]]] = None,
progress: bool = None) -> int:
if types:
blocks = [n for n, _ in sdfg.all_nodes_recursive() if type(n) in types]
elif blacklist:
elif ignore_region_types:
blocks = [n for n, _ in sdfg.all_nodes_recursive()
if isinstance(n, AbstractControlFlowRegion) and type(n) not in blacklist]
if isinstance(n, AbstractControlFlowRegion) and type(n) not in ignore_region_types]
else:
blocks = [n for n, _ in sdfg.all_nodes_recursive() if isinstance(n, AbstractControlFlowRegion)]
count = 0
Expand All @@ -1327,6 +1327,8 @@ def inline_control_flow_regions(sdfg: SDFG, types: Optional[List[Type[AbstractCo
if block.inline()[0]:
count += 1

sdfg.reset_cfg_list()

return count


Expand Down Expand Up @@ -1557,10 +1559,10 @@ def _tswds_cf_region(
for _, b in region.branches:
yield from _tswds_cf_region(sdfg, b, symbols, recursive)
return
elif isinstance(region, LoopRegion):
# Add the own loop variable to the defined symbols, if present.
loop_syms = region.new_symbols(symbols)
symbols.update({k: v for k, v in loop_syms.items() if v is not None})

# Add the own loop variable to the defined symbols, if present.
loop_syms = region.new_symbols(symbols)
symbols.update({k: v for k, v in loop_syms.items() if v is not None})

# Add symbols from inter-state edges along the state machine
start_region = region.start_block
Expand Down
6 changes: 3 additions & 3 deletions dace/sdfg/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def validate_control_flow_region(sdfg: 'SDFG',
if r is not None:
validate_control_flow_region(sdfg, r, initialized_transients, symbols, references, **context)
elif isinstance(edge.src, ControlFlowRegion):
lsyms = copy.deepcopy(symbols)
lsyms = copy.copy(symbols)
if isinstance(edge.src, LoopRegion) and not edge.src.loop_variable in lsyms:
lsyms[edge.src.loop_variable] = None
validate_control_flow_region(sdfg, edge.src, initialized_transients, lsyms, references, **context)
Expand Down Expand Up @@ -147,7 +147,7 @@ def validate_control_flow_region(sdfg: 'SDFG',
if r is not None:
validate_control_flow_region(sdfg, r, initialized_transients, symbols, references, **context)
elif isinstance(edge.dst, ControlFlowRegion):
lsyms = copy.deepcopy(symbols)
lsyms = copy.copy(symbols)
if isinstance(edge.dst, LoopRegion) and not edge.dst.loop_variable in lsyms:
lsyms[edge.dst.loop_variable] = None
validate_control_flow_region(sdfg, edge.dst, initialized_transients, lsyms, references, **context)
Expand All @@ -163,7 +163,7 @@ def validate_control_flow_region(sdfg: 'SDFG',
if r is not None:
validate_control_flow_region(sdfg, r, initialized_transients, symbols, references, **context)
elif isinstance(start_block, ControlFlowRegion):
lsyms = copy.deepcopy(symbols)
lsyms = copy.copy(symbols)
if isinstance(start_block, LoopRegion) and not start_block.loop_variable in lsyms:
lsyms[start_block.loop_variable] = None
validate_control_flow_region(sdfg, start_block, initialized_transients, lsyms, references, **context)
Expand Down
2 changes: 1 addition & 1 deletion dace/transformation/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ def nest_sdfg_subgraph(sdfg: SDFG, subgraph: SubgraphView, start: Optional[SDFGS
# `symbolic.pystr_to_symbolic` may return bool, which doesn't have attribute `args`
pass
for b in all_blocks:
if isinstance(b, LoopRegion) and b.loop_variable is not None and b.loop_variable != '':
if isinstance(b, LoopRegion) and b.loop_variable:
defined_symbols.add(b.loop_variable)
if b.loop_variable not in sdfg.symbols:
if b.init_statement:
Expand Down
7 changes: 3 additions & 4 deletions dace/transformation/interstate/fpga_transform_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,12 @@ def apply(self, graph: ControlFlowRegion, sdfg: SDFG):
# Input nodes may also be nodes with WCR memlets
# We have to recur across nested SDFGs to find them
wcr_input_nodes = set()
stack = []

for node, pGraph in state.all_nodes_recursive():
for node, node_parent_graph in state.all_nodes_recursive():
if isinstance(node, dace.sdfg.nodes.AccessNode):
for e in pGraph.in_edges(node):
for e in node_parent_graph.in_edges(node):
if e.data.wcr is not None:
trace = dace.sdfg.trace_nested_access(node, pGraph, pGraph.sdfg)
trace = dace.sdfg.trace_nested_access(node, node_parent_graph, node_parent_graph.sdfg)
for node_trace, memlet_trace, state_trace, sdfg_trace in trace:
# Find the name of the accessed node in our scope
if state_trace == state and sdfg_trace == sdfg:
Expand Down
7 changes: 1 addition & 6 deletions dace/transformation/interstate/multistate_inline.py
Original file line number Diff line number Diff line change
Expand Up @@ -145,12 +145,7 @@ def apply(self, outer_state: SDFGState, sdfg: SDFG):
from dace.transformation.passes.fusion_inline import InlineControlFlowRegions
from dace.transformation.passes.simplification.control_flow_raising import ControlFlowRaising

inline_pass = InlineControlFlowRegions()
inline_pass.no_inline_conditional = False
inline_pass.no_inline_named_regions = False
inline_pass.no_inline_function_call_regions = False
inline_pass.no_inline_loops = False
inline_pass.apply_pass(nsdfg, {})
sdutil.inline_control_flow_regions(nsdfg)
# After inlining, try to lift out control flow again, essentially preserving all control flow that can be
# preserved while removing the return blocks.
ControlFlowRaising().apply_pass(nsdfg, {})
Expand Down
16 changes: 8 additions & 8 deletions dace/transformation/passes/fusion_inline.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,21 @@ def apply_pass(self, sdfg: SDFG, _: Dict[str, Any]) -> Optional[int]:
:return: The total number of states fused, or None if did not apply.
"""
blacklist = []
ignore_region_types = []
if self.no_inline_loops:
blacklist.append(LoopRegion)
ignore_region_types.append(LoopRegion)
if self.no_inline_conditional:
blacklist.append(ConditionalBlock)
ignore_region_types.append(ConditionalBlock)
if self.no_inline_named_regions:
blacklist.append(NamedRegion)
ignore_region_types.append(NamedRegion)
if self.no_inline_function_call_regions:
blacklist.append(FunctionCallRegion)
if len(blacklist) < 1:
blacklist = None
ignore_region_types.append(FunctionCallRegion)
if len(ignore_region_types) < 1:
ignore_region_types = None

inlined = 0
while True:
inlined_in_iteration = inline_control_flow_regions(sdfg, None, blacklist, self.progress)
inlined_in_iteration = inline_control_flow_regions(sdfg, None, ignore_region_types, self.progress)
if inlined_in_iteration < 1:
break
inlined += inlined_in_iteration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
@properties.make_properties
@transformation.experimental_cfg_block_compatible
class PruneEmptyConditionalBranches(ppl.ControlFlowRegionPass):
"""
Prunes empty (or no-op) conditional branches from conditional blocks.
"""

CATEGORY: str = 'Simplification'

Expand Down
2 changes: 1 addition & 1 deletion dace/transformation/subgraph/composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def can_be_applied(self, sdfg: SDFG, subgraph: StateSubgraphView) -> bool:
break
if not par_graph_copy:
return False
graph_copy = par_graph_copy.nodes()[graph.block_id]
graph_copy = par_graph_copy.node(graph.block_id)
subgraph_copy = SubgraphView(graph_copy, [graph_copy.nodes()[i] for i in graph_indices])
expansion.cfg_id = par_graph_copy.cfg_id

Expand Down
58 changes: 9 additions & 49 deletions tests/passes/writeset_underapproximation_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
from typing import Dict
import dace
from dace.sdfg.analysis.writeset_underapproximation import UnderapproximateWrites, UnderapproximateWritesDict
from dace.sdfg.utils import inline_control_flow_regions
from dace.subsets import Range
from dace.transformation.pass_pipeline import Pipeline
from dace.transformation.passes.fusion_inline import InlineControlFlowRegions

N = dace.symbol("N")
M = dace.symbol("M")
Expand Down Expand Up @@ -309,12 +309,7 @@ def loop(A: dace.float64[N, M]):
sdfg = loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
results = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand All @@ -341,12 +336,7 @@ def loop(A: dace.float64[N, M]):
sdfg = loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
results = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand Down Expand Up @@ -474,12 +464,7 @@ def nested_loop(A: dace.float64[M, N]):
sdfg = nested_loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
result = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand Down Expand Up @@ -517,12 +502,7 @@ def nested_loop(A: dace.float64[M, N]):
sdfg = nested_loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
result = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand Down Expand Up @@ -558,12 +538,7 @@ def nested_loop(A: dace.float64[M, N]):
sdfg = nested_loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
result = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand Down Expand Up @@ -600,12 +575,7 @@ def nested_loop(A: dace.float64[M, N]):
sdfg = nested_loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
result: Dict[int, UnderapproximateWritesDict] = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand Down Expand Up @@ -874,12 +844,7 @@ def nested_loop(A: dace.float64[M, N]):
sdfg = nested_loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
result = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand Down Expand Up @@ -912,12 +877,7 @@ def nested_loop(A: dace.float64[M, N]):
sdfg = nested_loop.to_sdfg(simplify=True)

# NOTE: Until the analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)

pipeline = Pipeline([UnderapproximateWrites()])
result = pipeline.apply_pass(sdfg, {})[UnderapproximateWrites.__name__]
Expand Down
17 changes: 3 additions & 14 deletions tests/sdfg/work_depth_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,12 @@
import sympy as sp
import numpy as np

from dace.sdfg.utils import inline_control_flow_regions
from dace.transformation.interstate import NestSDFG
from dace.transformation.dataflow import MapExpansion

from pytest import raises

from dace.transformation.passes.fusion_inline import InlineControlFlowRegions

N = dc.symbol('N')
M = dc.symbol('M')
K = dc.symbol('K')
Expand Down Expand Up @@ -221,12 +220,7 @@ def test_work_depth(test_name):
sdfg.apply_transformations(MapExpansion)

# NOTE: Until the W/D Analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)
for sd in sdfg.all_sdfgs_recursive():
sd.using_experimental_blocks = False

Expand Down Expand Up @@ -279,12 +273,7 @@ def test_avg_par(test_name: str):
sdfg.apply_transformations(MapExpansion)

# NOTE: Until the W/D Analysis is changed to make use of the new blocks, inline control flow for the analysis.
inliner = InlineControlFlowRegions()
inliner.no_inline_conditional = False
inliner.no_inline_loops = False
inliner.no_inline_function_call_regions = False
inliner.no_inline_named_regions = False
inliner.apply_pass(sdfg, {})
inline_control_flow_regions(sdfg)
for sd in sdfg.all_sdfgs_recursive():
sd.using_experimental_blocks = False

Expand Down

0 comments on commit 2f7d6aa

Please sign in to comment.