Page MenuHomePhabricator

[mlir] Refactor StorageUniquer to require registration of possible storage types
ClosedPublic

Authored by rriddle on Jun 25 2020, 1:36 PM.

Details

Summary

This allows for bucketing the different possible storage types, with each bucket having its own allocator/mutex/instance map. This greatly reduces the amount of lock contention when multi-threading is enabled. On some non-trivial .mlir modules (>300K operations), this led to a compile time decrease of a single conversion pass by around half a second(>25%).

Depends On D82595

Diff Detail

Event Timeline

rriddle created this revision.Jun 25 2020, 1:36 PM
Herald added a project: Restricted Project. · View Herald Transcript
jpienaar added inline comments.Jul 4 2020, 9:38 AM
mlir/lib/Dialect/SDBM/SDBMDialect.cpp
18

Could we keep these sorted?

mlir/lib/Support/StorageUniquer.cpp
19

Mmm, this is one definition of simple, but simple is not very descriptive. KindUniquer ? OneOfKindUniquer ?

Or could go ParametricKindUniquer vs NonparametricKindUniquer as this is about whether the kind's uniquer has additional parameters to consider when uniquering.

Perhaps others would disagree here. This is also a local class ...

80

Can both be used at the same time? Would a given kind be able to use both?

rriddle updated this revision to Diff 282761.Aug 3 2020, 4:17 PM
rriddle marked an inline comment as done.

Resolve comments

rriddle updated this revision to Diff 282762.Aug 3 2020, 4:20 PM
rriddle marked an inline comment as done.

Run clang-format

rriddle added inline comments.Aug 5 2020, 3:31 PM
mlir/lib/Support/StorageUniquer.cpp
19

Used simple/complex because that is how it is described everywhere else, including user facing docs.

80

Not really, but I haven't been able to get a good merged representation that I liked. Having a union between the two always feels weird. Merged them together for now.

mehdi_amini accepted this revision.Aug 7 2020, 12:10 PM
mehdi_amini added inline comments.
mlir/lib/Support/StorageUniquer.cpp
19

Can you document the struct?

This revision is now accepted and ready to land.Aug 7 2020, 12:10 PM
rriddle updated this revision to Diff 284014.Aug 7 2020, 1:27 PM
rriddle marked an inline comment as done.

Resolve comments

This revision was landed with ongoing or failed builds.Aug 7 2020, 1:43 PM
This revision was automatically updated to reflect the committed changes.