Skip to content

Commit f5cbe18

Browse files
authored
Change loop order during rewrite (#2427)
Change the loop order when applying a collection of rewrite-rules to all nodes in graph. The order in this PR is preferable, for a couple of reasons. First: rules often have mutual dependences, with one rule enabling another. This loop-ordering handles such dependences better, and does so more efficiently (requiring fewer iterations of yet another outer loop to invoke the rewriter multiple times). It also sets it up for another optimization planned for originally (but not yet implemented): For patterns that end with a definite op (like ScatterND), which are most of the rules/patterns, we can create a dictionary mapping the op-identifier to rules applicable to that op. Then, it is sufficient to iterate only over rules applicable to current node's op. Signed-off-by: Ganesan Ramalingam <[email protected]>
1 parent ff0a132 commit f5cbe18

File tree

1 file changed

+6
-3
lines changed

1 file changed

+6
-3
lines changed

onnxscript/rewriter/_rewrite_rule.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -465,12 +465,12 @@ def _apply_to_graph_or_function(
465465
"""
466466
count = 0
467467

468-
# NOTE: Rules should be prioritized in the order they are added to the RewriteRuleSet.
469-
# And the graph is applied in order.
470468
for rule in self.rules:
471469
if rule.graph_pre_visitor:
472470
rule.graph_pre_visitor()
473-
for node in graph_or_function:
471+
472+
for node in graph_or_function:
473+
for rule in self.rules:
474474
delta = rule.try_rewrite(
475475
model, graph_or_function, node, verbose=verbose, tracer=tracer
476476
)
@@ -549,6 +549,9 @@ def _apply_to_graph_or_function(
549549
)
550550

551551
count += 1
552+
break
553+
554+
for rule in self.rules:
552555
if rule.graph_post_visitor:
553556
rule.graph_post_visitor()
554557

0 commit comments

Comments
 (0)