This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Refactor code out of BufferPlacement.cpp
ClosedPublic

Authored by silvas on Oct 12 2020, 2:04 PM.

Details

Diff Detail

Event Timeline

silvas created this revision.Oct 12 2020, 2:04 PM
silvas requested review of this revision.Oct 12 2020, 2:04 PM
rriddle added inline comments.Oct 12 2020, 2:19 PM
mlir/lib/Transforms/Bufferize.cpp
23–31
39

Are the conversions cheep to copy? If not please use & here. May be more clear if you use the proper type instead of auto, same comment applied to several places below.

60

nit: Can you merge this variable definition into the if condition? (I'm assuming the result here is Optional<T>?

98
152

Please prefer array_pod_sort over std::sort when possible.

157

Why this instead of a normal range based for? This seems more verbose.

192

Is the this-> necessary here?

silvas marked 6 inline comments as done.Oct 12 2020, 2:49 PM

Thanks River for the comments. To keep this patch a pure move, I've put the stylistic fixes in https://reviews.llvm.org/D89272

mlir/lib/Transforms/Bufferize.cpp
152

llvm::sort has a specialization for POD that does the right thing.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 14 2020, 12:41 PM
This revision was automatically updated to reflect the committed changes.