This is an archive of the discontinued LLVM Phabricator instance.

[MLIR][PDL] Integration test of multi-root matching and related fixes.
ClosedPublic

Authored by sfuniak on Dec 20 2021, 9:54 PM.

Details

Summary

This diff adds an integration test to multi-root PDL matching. It consists of two subtests:

  1. A 1-layer perceptron with split forward / backward operations.
  2. A 2-layer perceptron with fused forward / backward operations.

These tests use a collection of hand-written patterns and TensorFlow operations to be matched. The first test has a DAG / SSA dominant resulting match; the second does not and is therefore stored in a graph region.

This diff also includes two bug fixes:

  1. Mark the pdl_interp dialect as a dependent in the TestPDLByteCodePass. This is needed, because we create ops from that dialect as a part of the PDL-to-PDLInterp lowering.
  2. Fix of the starting index in the liveness range for the ForEach operations (bug exposed by the integration test).

Diff Detail

Event Timeline

sfuniak created this revision.Dec 20 2021, 9:54 PM
sfuniak requested review of this revision.Dec 20 2021, 9:54 PM

Thanks for the contribution of the integration tests.

mlir/lib/Rewrite/ByteCode.cpp
553–569

If it's simpler for the walk to be pre-order, you can make it so

matcherFunc.getBody().walk<WalkOrder::PreOrder>(...)
sfuniak added inline comments.Dec 21 2021, 4:27 PM
mlir/lib/Rewrite/ByteCode.cpp
553–569

It is not enough for the walk to be pre-order, unfortunately. Post-order is correct for the right endpoint of the liveness range, whereas pre-order is correct for the left endpoint. What we would really want is some kind of hybrid walk that visits the operation twice -- once as the walk descends down (marking the left endpoint), and second time as it ascends up (marking the right endpoint). I could code up a custom walk that implements this behavior, but it seemed easier to "patch up" the post-order walk down below instead. Please let me know if you think otherwise. In the meantime, i will update the documentation.

sfuniak added inline comments.Dec 21 2021, 4:29 PM
mlir/lib/Rewrite/ByteCode.cpp
553–569

On the second thought, the current patch-up is error-prone. I will code up a custom walk.

sfuniak updated this revision to Diff 395774.Dec 21 2021, 5:05 PM

Implemented a custom walk to determine the liveness range of an operation.

Mogball accepted this revision.Dec 21 2021, 5:14 PM
This revision is now accepted and ready to land.Dec 21 2021, 5:14 PM
sfuniak updated this revision to Diff 397175.Jan 3 2022, 6:31 PM

Rebased.

This revision was landed with ongoing or failed builds.Jan 3 2022, 6:40 PM
This revision was automatically updated to reflect the committed changes.