[all-commits] [llvm/llvm-project] ed9194: [mlir] GreedyPatternRewriter: Add ancestors to wor...

Matthias Springer via All-commits all-commits at lists.llvm.org
Fri Jan 13 01:51:51 PST 2023


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: ed9194be6d55776cbbf8432b93f0f23ec7b25a46
      https://github.com/llvm/llvm-project/commit/ed9194be6d55776cbbf8432b93f0f23ec7b25a46
  Author: Matthias Springer <springerm at google.com>
  Date:   2023-01-13 (Fri, 13 Jan 2023)

  Changed paths:
    M mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
    M mlir/test/Dialect/Math/expand-math.mlir
    M mlir/test/Dialect/SCF/loop-pipelining.mlir
    M mlir/test/IR/greedy-pattern-rewriter-driver.mlir
    M mlir/test/lib/Dialect/SCF/TestSCFUtils.cpp
    M mlir/test/lib/Dialect/Test/TestPatterns.cpp

  Log Message:
  -----------
  [mlir] GreedyPatternRewriter: Add ancestors to worklist

When adding an op to the worklist, also add its ancestors to the worklist. This allows for RewritePatterns to match an op `a` based on what is inside of the body of `a`.

This change fixes a problem that became apparent with `vector.warp_execute_on_lane_0`, but could probably be triggered with similar patterns. The pattern extracts an op `b` with `eligible = true` from the body of an op `a`:
```
test.a {
  %0 = test.b() {eligible = true}
  yield %0
}
```

Afterwards:
```
%0 = test.b() {eligible = true}
test.a {
  yield %0
}
```

The pattern is an `OpRewritePattern<OpA>`. For some reason, `test.a` is not on the GreedyPatternRewriter's worklist. E.g., because no pattern could be applied and it was removed. Now, another pattern updates `test.b`, so that `eligible` is changed from `true` to `false`. The `OpRewritePattern<OpA>` could now be applied, but (without this revision) `test.a` is still not on the worklist.

Note: In the above example, an `OpRewritePattern<OpB>` could have been used instead of an `OpRewritePattern<OpA>`. With such a design, we can run into the same problem (when the `eligible` attr is on `test.a` and `test.b` is removed from the worklist because no patterns could be applied).

Note: This change uncovered an unrelated bug in TestSCFUtils.cpp that was triggered due to a change in the order in which ops are processed. A TODO is added to the broken code and test cases are adapted so that the bug is no longer triggered.

Differential Revision: https://reviews.llvm.org/D140304




More information about the All-commits mailing list