Page MenuHomePhabricator

[mlir] remove BufferAssignmentPlacer from BufferAssignmentOpConversionPattern
ClosedPublic

Authored by dfki-ehna on Sep 3 2020, 2:26 AM.

Details

Summary

BufferPlacement has been removed, as allocations are no longer placed during the conversion.

Diff Detail

Event Timeline

dfki-ehna created this revision.Sep 3 2020, 2:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 3 2020, 2:26 AM
dfki-ehna requested review of this revision.Sep 3 2020, 2:26 AM
dfki-ehna edited the summary of this revision. (Show Details)Sep 3 2020, 2:30 AM
dfki-ehna edited the summary of this revision. (Show Details)
herhut requested changes to this revision.Sep 7 2020, 2:16 AM

Do you intend to remove the BufferAssignmentPlacer itself in a separate step?

mlir/include/mlir/Transforms/BufferPlacement.h
26–27

Can this also go?

139

If nullptr is not a usable default, then why have it anyway?

mlir/lib/Dialect/Linalg/Transforms/TensorsToBuffers.cpp
47

This should be a no-op as the rewriter should already be at the op positon.

mlir/lib/Transforms/BufferPlacement.cpp
866

This is a no-op, right?

mlir/test/lib/Transforms/TestBufferPlacement.cpp
59

Here, too.

This revision now requires changes to proceed.Sep 7 2020, 2:16 AM
dfki-ehna updated this revision to Diff 290235.Sep 7 2020, 3:34 AM

BufferAssignmentPlacer is removed, and comments are addressed.

dfki-ehna marked 5 inline comments as done.Sep 7 2020, 3:35 AM
herhut accepted this revision.Sep 7 2020, 3:48 AM

Thanks. Please submit a corresponding PR for TensorFlow before landing.

This revision is now accepted and ready to land.Sep 7 2020, 3:48 AM
herhut added a comment.Sep 7 2020, 3:51 AM

Also, as a nit, you could rephrase the summary to just read

BufferPlacement has been removed, as allocations are no longer placed during the conversion.

dfki-ehna edited the summary of this revision. (Show Details)Sep 7 2020, 4:20 AM
This revision was landed with ongoing or failed builds.Sep 8 2020, 4:07 AM
This revision was automatically updated to reflect the committed changes.