Page MenuHomePhabricator

[mlir] Fix subtensor_insert bufferization.
ClosedPublic

Authored by silvas on Nov 12 2020, 11:04 AM.

Details

Summary

It was incorrect in the presence of a tensor argument with multiple
uses.

The bufferization of subtensor_insert was writing into a converted
memref operand, but there is no guarantee that the converted memref for
that operand is safe to write into. In this case, the same converted
memref is written to in-place by the subtensor_insert bufferization,
violating the tensor-level semantics.

I left some comments in a TODO about ways forward on this. I will be
working actively on this problem in the coming days.

Diff Detail

Event Timeline

silvas created this revision.Nov 12 2020, 11:04 AM
silvas requested review of this revision.Nov 12 2020, 11:04 AM
silvas retitled this revision from [mlir] Fix subtensor bufferization. to [mlir] Fix subtensor_insert bufferization..Nov 12 2020, 11:11 AM

LGTM once the subtensor_update part is addressed, thanks @silvas !

mlir/lib/Dialect/Linalg/Transforms/Bufferize.cpp
320

As discussed offline, this particular proposal of subtensor_update has deeper ramifications, let's fix the bug and stash the comment if you don't mind.

329

Ok, given the expected update to tensor_to_memref documentation and the fact that we'll create and interface + attribute to determine when it is safe to write in place, cloning is indeed the proper behavior here.

This revision is now accepted and ready to land.Nov 12 2020, 11:50 AM
silvas updated this revision to Diff 304922.Nov 12 2020, 11:53 AM

Address Nicolas' comments.

This revision was automatically updated to reflect the committed changes.