This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Vector] Add a Broadcast::createBroadcastOp helper
ClosedPublic

Authored by nicolasvasilache on Nov 30 2022, 4:23 AM.

Details

Summary

This helper handles non trivial cases of broadcast + optional transpose creation
that should not leak to the outside world.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald Transcript
nicolasvasilache requested review of this revision.Nov 30 2022, 4:23 AM
Herald added a project: Restricted Project. · View Herald Transcript
springerm accepted this revision.Nov 30 2022, 5:00 AM
springerm added a subscriber: springerm.
springerm added inline comments.
mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
459

I think this should be a top-level method in VectorOps.h or VectorUtils.h because it it is more than just a BroadcastOp builder.

462

const llvm::SetVector<int64_t> &

mlir/lib/Dialect/Vector/IR/VectorOps.cpp
1744–1745

I would move this comment to computeBroadcastedUnitDims.

1771–1773

Should this be an assert?

1811

nit: nextSrcShapeDim

mlir/test/lib/Dialect/Vector/TestVectorTransforms.cpp
843–860

I would walk the IR for unregistered ops like "test.create_broadcast"(%a) {broadcasted_dims = [2]} : (vector<3x4xf32>) -> (vector<3x4x5xf32>) instead of hardcoding these here.

This revision is now accepted and ready to land.Nov 30 2022, 5:00 AM
nicolasvasilache marked 6 inline comments as done.

Address comments.

Address review.

mlir/include/mlir/Dialect/Vector/IR/VectorOps.td
459

true .. OTOH is is not a builder but a separate createOrFold method that is clearly documented and I really want people to find it rather than reinvent a broken wheel ..
If I move, I'd like it to be in a common place where we have clear guidelines that people must go an check it out first before building or manipulating op internals (i.e. things that should be methods of the op .. and we're back to square 1)

mlir/lib/Dialect/Vector/IR/VectorOps.cpp
1771–1773

yes, I reworked a bunch of things around asserts to guarantee this creates or fold something.

mlir/test/lib/Dialect/Vector/TestVectorTransforms.cpp
843–860

fair enough, thanks!

Drop spurious includes.

This revision was landed with ongoing or failed builds.Nov 30 2022, 5:32 AM
This revision was automatically updated to reflect the committed changes.