This is an archive of the discontinued LLVM Phabricator instance.

[mlir] NFC: Expose `getElementPtrType` and `getSizes` methods of AllocOpLowering.
ClosedPublic

Authored by pifon2a on Jul 30 2020, 1:45 AM.

Details

Summary

These methods are very useful outside of AllocLikeOpLowering.

Diff Detail

Event Timeline

pifon2a created this revision.Jul 30 2020, 1:45 AM
Herald added a project: Restricted Project. · View Herald Transcript
pifon2a requested review of this revision.Jul 30 2020, 1:45 AM
herhut added inline comments.Jul 30 2020, 2:03 AM
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
934

I would prefer for this method to be split into two pieces

getMemRefDescriptorSizes that gives you the SmallVector of sizes.

and

getMemRefDescriptorSizeInBytes that takes the sizes and gives you the Value for the size.

Longer term, we could then reuse the former in more places.

937

Making this one reused makes sense for a private function but not for a public API I think.

pifon2a updated this revision to Diff 281872.Jul 30 2020, 4:06 AM

Split getSizes() into getMemRefDescriptorsizes and getCumulativeSizeInBytes.

herhut added inline comments.Jul 30 2020, 4:18 AM
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
942

Is this actually correct? If the memref is 0d, it should have no sizes. I think this was convenient for the later computation of the byte size only.

946

Should this just return the value and create its own one? I think it is better to not share the one constant. They are cheap and cse will fix it. And even if not, they don't turn into code anyway.

pifon2a updated this revision to Diff 281879.Jul 30 2020, 4:46 AM

Address the comments.

pifon2a marked 4 inline comments as done.Jul 30 2020, 4:47 AM
herhut accepted this revision.Jul 30 2020, 5:23 AM

Please fix linter error and wait for @ftynse to comment.

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
1954

Accidental edit?

This revision is now accepted and ready to land.Jul 30 2020, 5:23 AM
pifon2a updated this revision to Diff 281894.Jul 30 2020, 5:39 AM
pifon2a marked an inline comment as done.

Fix linter.

ftynse accepted this revision.Jul 30 2020, 10:07 AM
ftynse added inline comments.
mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp
940

Nit: can this use ShapedType::kDynamicSize instead of hardcoding -1 ?

960

Nit: template seems no longer necessary

pifon2a updated this revision to Diff 281995.Jul 30 2020, 11:17 AM
pifon2a marked 2 inline comments as done.

Address the comments.

This revision was landed with ongoing or failed builds.Jul 30 2020, 11:18 AM
This revision was automatically updated to reflect the committed changes.