This is an archive of the discontinued LLVM Phabricator instance.

[MLIR] TilingInterface: Avoid map when tile divides iteration domain
ClosedPublic

Authored by chelini on Aug 3 2022, 9:56 AM.

Diff Detail

Event Timeline

chelini created this revision.Aug 3 2022, 9:56 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2022, 9:56 AM
chelini requested review of this revision.Aug 3 2022, 9:56 AM
ftynse added a subscriber: ftynse.Aug 3 2022, 10:29 AM
ftynse added inline comments.
mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
137

Can't we use makeComposedFoldedAffineMin and let it fold the map?

chelini added inline comments.Aug 4 2022, 12:00 AM
mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
137

Thanks for the pointer. Even using makeComposedFoldedAffineMin or makeComposedAffineMin I cannot get rid of the map, but we get better IR with what we have now where the constants are folded into the map.

For example, affine_map<(d0)[s0, s1] -> (20, -d0 + s1)> turns to affine_map<(d0) -> (20, -d0 + 300)>. Current IR:

#map1 = affine_map<(d0) -> (20, -d0 + 300)>

scf.for %arg4 = %c0 to %c300 step %c20 {
   %0 = affine.min #map1(%arg4)
    use(%0)
}

Do we have something to simplify the map here? As a reference also here we do check if we can avoid the map: https://github.com/llvm/llvm-project/blob/bc32896e9f39c1c64528840d78c8a07aad2ad313/mlir/lib/Dialect/Linalg/Utils/Utils.cpp#L866

chelini edited reviewers, added: mravishankar; removed: maheshkhanwalkar.
ftynse added inline comments.Aug 4 2022, 3:58 AM
mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
93

Take either a Range or just three separate variables to avoid magic number-based access below.

100–104

This matching is insufficiently robust, m_Constant is recommended instead. You can also just call getAsOpFoldResult for the stride (offset and size are already available as OpFoldResult at the call side) and dyn_cast those to Attribute.

137

I see, you need to be able to reason about the set of induction variable values. Scrap that.

152

Note that offset and size are created artificially from an OpFoldResult a couple of lines above. Use that instead and you won't do the redundant work of matching constant operations that you have just created from actual constants.

chelini updated this revision to Diff 449945.Aug 4 2022, 5:50 AM

Address comments.

chelini marked 3 inline comments as done.Aug 4 2022, 5:53 AM
chelini added inline comments.
mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
100–104

found getConstantIntValue that does what suggested.

ftynse accepted this revision.Aug 4 2022, 5:54 AM
This revision is now accepted and ready to land.Aug 4 2022, 5:54 AM
This revision was automatically updated to reflect the committed changes.
chelini marked an inline comment as done.