Skip to content

Commit

Permalink
core: Fix bug in pattern rewriter when ErasedSSAValue is present (#2365)
Browse files Browse the repository at this point in the history
When an operation is deleted, the pattern rewriter adds its operands
with 1 use to the worklist of operations to change.
This change checks that these operands are not `ErasedSSAValue`,
otherwise an erased operation would be added to the worklist.

I'm starting to think that the `ErasedSSAValue` design is bad, I'll be
thinking of a way to change this.
  • Loading branch information
math-fehr authored Mar 24, 2024
1 parent fa79f3d commit 07108d9
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 1 deletion.
32 changes: 32 additions & 0 deletions tests/pattern_rewriter/test_pattern_rewriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -1344,6 +1344,38 @@ def match_and_rewrite(self, op: test.TestOp, rewriter: PatternRewriter):
)


def test_erased_ssavalue():
prog = """\
builtin.module {
"test.op"() ({
%0 = "test.op"() : () -> i32
"test.op"(%0) : (i32) -> ()
}) : () -> ()
}
"""

expected = """\
"builtin.module"() ({
"test.op"() ({
^0:
}) : () -> ()
}) : () -> ()
"""

class Rewrite(RewritePattern):
@op_type_rewrite_pattern
def match_and_rewrite(self, op: test.TestOp, rewriter: PatternRewriter):
if op.results or op.operands:
rewriter.erase_matched_op(safe_erase=False)

rewrite_and_compare(
prog,
expected,
PatternRewriteWalker(Rewrite(), apply_recursively=True),
op_removed=2,
)


def test_type_conversion():
"""Test rewriter on ops without results"""
prog = """\
Expand Down
7 changes: 6 additions & 1 deletion xdsl/pattern_rewriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
Attribute,
Block,
BlockArgument,
ErasedSSAValue,
Operation,
ParametrizedAttribute,
Region,
Expand Down Expand Up @@ -627,7 +628,11 @@ def _add_operands_to_worklist(self, operands: Iterable[SSAValue]) -> None:
have more canonicalization opportunities.
"""
for operand in operands:
if len(operand.uses) == 1 and isinstance((op := operand.owner), Operation):
if (
len(operand.uses) == 1
and not isinstance(operand, ErasedSSAValue)
and isinstance((op := operand.owner), Operation)
):
self._worklist.push(op)

def _handle_operation_insertion(self, op: Operation) -> None:
Expand Down

0 comments on commit 07108d9

Please sign in to comment.