Page MenuHomePhabricator

[mlir][Linalg] Add support for fusion between indexed_generic ops and generic ops on tensors.
ClosedPublic

Authored by hanchung on May 20 2020, 5:47 PM.

Details

Summary

Different from the fusion between generic ops, indices are involved. In this
context, we need to re-map the indices for producer since the fused op is built
on consumer's perspective. This patch supports all combination of the fusion
between indexed_generic ops and generic ops, which includes tests case:

  1. generic op as producer and indexed_generic op as consumer.
  2. indexed_generic op as producer and generic op as consumer.
  3. indexed_generic op as producer and indexed_generic op as consumer.

Diff Detail

Event Timeline

hanchung created this revision.May 20 2020, 5:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2020, 5:47 PM
mravishankar requested changes to this revision.May 23 2020, 1:16 AM
mravishankar added inline comments.
mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
764

It seems like the logic used for generating the fused region for the indexed generic op would work for generic op as well. The numProducerIndices and numConsumerIndices would just remain the same. Can we combine these two methods?

This revision now requires changes to proceed.May 23 2020, 1:16 AM
hanchung updated this revision to Diff 266376.May 26 2020, 4:52 PM

Merge methods together for generic ops and indexed_generic ops.

hanchung marked an inline comment as done.May 26 2020, 4:52 PM
hanchung updated this revision to Diff 266377.May 26 2020, 4:54 PM

Update parents.

mravishankar accepted this revision.May 27 2020, 10:12 AM

Really cool! Just one comment. Approving assuming that it will be addressed. Thanks a lot!

mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
441

Oh I really like this! It was something I wanted to get to anyway. Thanks for doing it right here!

531–539

I think this should be outside of the else?

mlir/test/Dialect/Linalg/fusion-tensor.mlir
226

Lets just make all shapes dynamic in the tests. THere shouldnt be any reason for static shape right?

This revision is now accepted and ready to land.May 27 2020, 10:12 AM
hanchung updated this revision to Diff 266680.May 27 2020, 3:00 PM
hanchung marked 4 inline comments as done.

Address comments.

mlir/lib/Dialect/Linalg/Transforms/Fusion.cpp
531–539

Good catch, in the beginning I thought we can't compose the maps sometimes. After review the comment of compose method, I found that the last two maps should swap.

See comments of AffineMap::compose():

/// Example:
///   map1: `(d0, d1)[s0, s1] -> (d0 + 1 + s1, d1 - 1 - s0)`
///   map2: `(d0)[s0] -> (d0 + s0, d0 - s0)`
///   map1.compose(map2):
///     `(d0)[s0, s1, s2] -> (d0 + s1 + s2 + 1, d0 - s0 - s2 - 1)`

Add some comment for it.

Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2020, 2:14 PM
hanchung edited the summary of this revision. (Show Details)Jun 2 2020, 10:28 AM
This revision was automatically updated to reflect the committed changes.