Why does GreedyPatternRewriteDriver requires region isolated from above?

MLIR
1

Looking at some of the code around commit e7a2ef21f9fc9f10e03b97e9e73055e3447a1aa7 I am wondering where the isolatedFromAbove requirement originates from.

I understand canonicalization may modify operations outside of the op the greedy rewriter would be invoked on but I am not sure why this would be a problem.

I am wondering whether that restriction was needed at the time of the bringup of the passmanager and multi-threading concerns in general or whether there is something specific to applying patterns that also needs such limitation.

In my mind, isolatedFromAbove is key to enabling sane multi-threading in MLIR and to represent the ubiquitous no-capture function-like abstraction.

Still, it seem to me this could be decoupled from pattern application.

Thoughts?

@River707

2

This is to keep the APIs less error prone: we wouldn’t want a user to ask for it to run on a region and have it “escape” outside of it uncontrollably.

That said the variant that takes one or multiple ops does not have this restriction I believe.

3

Yes, the variants that work on a list of ops don’t have this restriction when the strict argument supplied to them is false.

4

For different reasons, we recently added an explicit scope to the GreedyPatternRewriteDriver (⚙ D140304 [mlir] GreedyPatternRewriter: Add ancestors to worklist). With this, we no longer risk “escaping” a region.

So we can relax this restriction now (https://reviews.llvm.org/D141366). Please let me know if you have concerns.