This is an archive of the discontinued LLVM Phabricator instance.

[tosa][mlir] Support dynamic batch dimension for ops where the batch dim is explicit
ClosedPublic

Authored by NatashaKnk on Jan 11 2022, 8:42 AM.

Details

Summary

Dynamic batch for rescale, gather, max_pool, avg_pool, conv2D and depthwise_conv2D. Split helper functions into a separate header file.

Diff Detail

Event Timeline

NatashaKnk created this revision.Jan 11 2022, 8:42 AM
NatashaKnk requested review of this revision.Jan 11 2022, 8:42 AM
rsuderman requested changes to this revision.Jan 11 2022, 9:45 AM

Move mlir/lib/Conversion/TosaToLinalg/TosaToLinalgCommon.h to mlir/include/mlir/Dialect/Tosa/Utils/utils.h. You can check mlir/include/mlir/Dialect/Linalg/Utils/Utils.h for an example on how to declare utilities for dialects.

mlir/lib/Conversion/TosaToLinalg/TosaToLinalg.cpp
14

This should be a full path and not local.

mlir/lib/Conversion/TosaToLinalg/TosaToLinalgCommon.h
23 ↗(On Diff #398966)

Add an additional tosa namespace so that these helpers are not placed in the global mlir namespace.

24 ↗(On Diff #398966)

I believe you can just use static to avoid the namespace collision. This is usually preferred to inline.

57 ↗(On Diff #398966)

Rename to checkHasDynamicBatchDims. We can likely drop the Only and Has is more commonly used in names.

mlir/lib/Conversion/TosaToLinalg/TosaToLinalgNamed.cpp
13

ditto on full path.

This revision now requires changes to proceed.Jan 11 2022, 9:45 AM
rsuderman added inline comments.Jan 11 2022, 9:47 AM
mlir/lib/Conversion/TosaToLinalg/TosaToLinalgCommon.h
29 ↗(On Diff #398966)

Also, add documentation for what each function does. Once we move to Utils we can expect other libraries to use these functions.

Moved header util file and created a .cpp file. Added a couple of fixes

rsuderman accepted this revision.Jan 11 2022, 8:53 PM
rsuderman added inline comments.
mlir/include/mlir/Dialect/Tosa/Utils/CoversionUtils.h
29

Small typo, should be condenseValues

This revision is now accepted and ready to land.Jan 11 2022, 8:53 PM