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

transforms: (canonicalize) Add full dce #3267

Merged
merged 3 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
12 changes: 12 additions & 0 deletions tests/filecheck/dialects/cf/canonicalize.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,15 @@ func.func @br_passthrough(%arg0 : i32, %arg1 : i32) -> (i32, i32) {
^2(%arg4 : i32, %arg5 : i32):
return %arg4, %arg5 : i32, i32
}

/// Test that dead branches don't affect passthrough
// CHECK: func.func @br_dead_passthrough() {
// CHECK-NEXT: func.return
// CHECK-NEXT: }
func.func @br_dead_passthrough() {
cf.br ^1
^0:
cf.br ^1
^1:
func.return
}
12 changes: 3 additions & 9 deletions tests/filecheck/dialects/riscv_cf/canonicalize.mlir
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just saw this deleted the first riscv.label as it appears in a dead block. Is this acceptable?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! The only label we care about is the one at the start of the block we jump to

Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,9 @@ riscv_func.func @never() {

// CHECK-NEXT: riscv_func.func @never() {
// CHECK-NEXT: %one = riscv.li 1 : !riscv.reg
// CHECK-NEXT: %three = riscv.li 3 : !riscv.reg
// CHECK-NEXT: %{{.*}} = riscv.mv %one : (!riscv.reg) -> !riscv.reg
// CHECK-NEXT: riscv_cf.j ^{{\d+}}(%{{.*}} : !riscv.reg) attributes {"comment" = "Constant folded riscv_cf.bge"}
// CHECK-NEXT: ^{{\d+}}(%i : !riscv.reg):
// CHECK-NEXT: riscv.label "scf_body_0_for"
// CHECK-NEXT: "test.op"(%i) : (!riscv.reg) -> ()
// CHECK-NEXT: %{{\d+}} = riscv.addi %i, 1 : (!riscv.reg) -> !riscv.reg
// CHECK-NEXT: riscv_cf.blt %{{\d+}} : !riscv.reg, %three : !riscv.reg, ^{{\d+}}(%{{\d+}} : !riscv.reg), ^{{\d+}}(%{{\d+}} : !riscv.reg)
// CHECK-NEXT: ^{{\d+}}(%{{\d+}} : !riscv.reg):
// CHECK-NEXT: %0 = riscv.mv %one : (!riscv.reg) -> !riscv.reg
// CHECK-NEXT: riscv_cf.j ^0(%0 : !riscv.reg) attributes {"comment" = "Constant folded riscv_cf.bge"}
// CHECK-NEXT: ^0(%1 : !riscv.reg):
// CHECK-NEXT: riscv.label "scf_body_end_0_for"
// CHECK-NEXT: riscv_func.return
// CHECK-NEXT: }
11 changes: 11 additions & 0 deletions xdsl/pattern_rewriter.py
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,13 @@ class PatternRewriteWalker:
That way, all uses are replaced before the definitions.
"""

post_walk_pass: Callable[[Operation, PatternRewriterListener], bool] | None = field(
alexarice marked this conversation as resolved.
Show resolved Hide resolved
default=None
)
"""
Pass to call between each walk of the IR.
alexarice marked this conversation as resolved.
Show resolved Hide resolved
"""

listener: PatternRewriterListener = field(default_factory=PatternRewriterListener)
"""The listener that will be called when an operation or block is modified."""

Expand Down Expand Up @@ -755,6 +762,8 @@ def rewrite_op(self, op: Operation) -> bool:

self._populate_worklist(op)
op_was_modified = self._process_worklist(pattern_listener)
if self.post_walk_pass is not None:
op_was_modified |= self.post_walk_pass(op, pattern_listener)
alexarice marked this conversation as resolved.
Show resolved Hide resolved

if not self.apply_recursively:
return op_was_modified
Expand All @@ -764,6 +773,8 @@ def rewrite_op(self, op: Operation) -> bool:
while op_was_modified:
self._populate_worklist(op)
op_was_modified = self._process_worklist(pattern_listener)
if self.post_walk_pass is not None:
op_was_modified |= self.post_walk_pass(op, pattern_listener)
alexarice marked this conversation as resolved.
Show resolved Hide resolved

return result

Expand Down
4 changes: 2 additions & 2 deletions xdsl/transforms/canonicalize.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
RewritePattern,
)
from xdsl.traits import HasCanonicalizationPatternsTrait
from xdsl.transforms.dead_code_elimination import RemoveUnusedOperations
from xdsl.transforms.dead_code_elimination import RemoveUnusedOperations, op_dce


class CanonicalizationRewritePattern(RewritePattern):
Expand Down Expand Up @@ -37,4 +37,4 @@ def apply(self, ctx: MLContext, op: builtin.ModuleOp) -> None:
pattern = GreedyRewritePatternApplier(
[RemoveUnusedOperations(), CanonicalizationRewritePattern()]
)
PatternRewriteWalker(pattern).rewrite_module(op)
PatternRewriteWalker(pattern, post_walk_pass=op_dce).rewrite_module(op)
alexarice marked this conversation as resolved.
Show resolved Hide resolved
26 changes: 19 additions & 7 deletions xdsl/transforms/dead_code_elimination.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
from xdsl.ir import Operation, Region, SSAValue
from xdsl.ir.post_order import PostOrderIterator
from xdsl.passes import ModulePass
from xdsl.pattern_rewriter import PatternRewriter, PatternRewriteWalker, RewritePattern
from xdsl.pattern_rewriter import (
PatternRewriter,
PatternRewriterListener,
PatternRewriteWalker,
RewritePattern,
)
from xdsl.traits import (
IsTerminator,
MemoryEffectKind,
Expand Down Expand Up @@ -120,7 +125,7 @@ def propagate_region_liveness(self, region: Region):
for operation in reversed(block.ops):
self.propagate_op_liveness(operation)

def delete_dead(self, region: Region):
def delete_dead(self, region: Region, listener: PatternRewriterListener | None):
first = region.first_block
if first is None:
return
Expand All @@ -135,26 +140,33 @@ def delete_dead(self, region: Region):
for operation in reversed(block.ops):
if not self.is_live(operation):
self.changed = True
if listener is not None:
listener.handle_operation_removal(operation)
block.erase_op(operation, safe_erase=False)
else:
for r in operation.regions:
self.delete_dead(r)
self.delete_dead(r, listener)


def region_dce(region: Region) -> bool:
def region_dce(region: Region, listener: PatternRewriterListener | None = None) -> bool:
live_set = LiveSet()

while live_set.changed:
live_set.changed = False
live_set.propagate_region_liveness(region)

live_set.delete_dead(region)
live_set.delete_dead(region, listener)
return live_set.changed


def op_dce(op: Operation, listener: PatternRewriterListener | None = None):
changed = tuple(region_dce(region, listener) for region in op.regions)

return any(changed)


class DeadCodeElimination(ModulePass):
name = "dce"

def apply(self, ctx: MLContext, op: ModuleOp) -> None:
for region in op.regions:
region_dce(region)
op_dce(op)
Loading