Page MenuHomePhabricator

[mlir][Linalg] Add TensorsToBuffers support for Constant ops.
ClosedPublic

Authored by nicolasvasilache on Oct 7 2020, 12:00 PM.

Details

Summary

This revision also inserts an end-to-end test that lowers tensors to buffers all the way to executable code on CPU.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 7 2020, 12:00 PM
nicolasvasilache requested review of this revision.Oct 7 2020, 12:00 PM
mlir/lib/Dialect/Linalg/Transforms/TensorsToBuffers.cpp
214

Nice!

Was reluctant to invest supporting more complex cases and I am glad I didn't try.

Can we move your code to core?
Should I move it in a subsequent CL that will deletes this pattern?

Your choice.

silvas added inline comments.Oct 7 2020, 2:20 PM
mlir/lib/Dialect/Linalg/Transforms/TensorsToBuffers.cpp
214

I'm happy moving the code to core, but it takes a couple opinions that I don't think we have fleshed out in core. Probably the biggest one is having a "global" op and a lowering of the "global" to LLVM.

Maybe there's a bigger discussion here about support for "tensor to memref" conversion in core. I've seen scattered patches upstream related to it, but no real clear design. I'll send a discussion to discourse.

For now, feel free to carry on with this patch.

silvas added inline comments.Oct 7 2020, 2:41 PM
mlir/lib/Dialect/Linalg/Transforms/TensorsToBuffers.cpp
214
pifon2a accepted this revision.Oct 8 2020, 4:47 AM
This revision is now accepted and ready to land.Oct 8 2020, 4:47 AM

Generalize tensor constant -> buffer by going through vectors.

pifon2a accepted this revision.Oct 8 2020, 4:52 AM

Rebase, make vector.type_cast a ViewLikeOpInterface so it plays nicely with buffer placement, fix test.

Harbormaster completed remote builds in B74428: Diff 296937.

Add comments to new patterns.

replaceOp -> replaceOpWithNewOp

This revision was landed with ongoing or failed builds.Oct 8 2020, 6:16 AM
This revision was automatically updated to reflect the committed changes.
mlir/lib/Dialect/Linalg/Transforms/TensorsToBuffers.cpp
214

Thanks for starting the discussion @silvas , I replied there.
Indeed it feels like this is reaching a point of maturity that we need to sync better on.
In the meantime, this is compatible with the existing DFKI design and allows progress towards e2e, we can adapt as needed.

Harbormaster completed remote builds in B74434: Diff 296945.