Page MenuHomePhabricator

[mlir] Rework DialectConversion inlineRegionBefore
ClosedPublic

Authored by tpopp on Nov 10 2020, 10:08 AM.

Details

Summary

The previous logic for inlining a region A with N blocks into region B
would produce incorrect results on rollback for N greater than 1. This
rollback logic would leave blocks 1..N in region B and only move block 0
to region A.

The new inlining action recording stores the block move actions from N-1
to 0. Now on roll back, block 0 is moved to region A and then 1..N is
appended to the list of blocks in region A.

Diff Detail

Event Timeline

tpopp created this revision.Nov 10 2020, 10:08 AM
tpopp requested review of this revision.Nov 10 2020, 10:08 AM
rriddle accepted this revision.Nov 10 2020, 7:17 PM

Can you add a test? The pattern already exists(https://github.com/llvm/llvm-project/blob/659230ff8073ca568cef761e5892ce57eee81610/mlir/test/lib/Dialect/Test/TestPatterns.cpp#L210) so it should be a matter of writing the .mlir test case.

mlir/lib/Transforms/Utils/DialectConversion.cpp
1214

Is the getBlocks here really necessary?

This revision is now accepted and ready to land.Nov 10 2020, 7:17 PM
tpopp updated this revision to Diff 304428.Nov 11 2020, 1:41 AM

Add a test

tpopp marked an inline comment as done.Nov 11 2020, 1:42 AM
tpopp added inline comments.
mlir/lib/Transforms/Utils/DialectConversion.cpp
1214

You're right. Removed.

This revision was landed with ongoing or failed builds.Nov 11 2020, 1:42 AM
This revision was automatically updated to reflect the committed changes.