This is an archive of the discontinued LLVM Phabricator instance.

Extract common code to deal with multidimensional vectors.
ClosedPublic

Authored by akuegel on Mar 6 2020, 4:06 AM.

Details

Summary

Also replace dyn_cast_or_null with dyn_cast when possible.

Diff Detail

Event Timeline

akuegel created this revision.Mar 6 2020, 4:06 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2020, 4:06 AM
ftynse accepted this revision.Mar 6 2020, 4:35 AM

A couple of nits

mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
1169

Is it really worth it to add a template (and hence increase compile time and binary size) only to forward it to SmallVector? I'd just use 4 instead, this should cover the vast majority of cases.

1170

Please use LogicalResult instead of bool

1171

Can we use ValueRange instead of ArrayRef<Value>, this is more flexible and ValueRange is implicitly constructible from ArrayRef and other things.

1189

Nit: drop trivial braces

This revision is now accepted and ready to land.Mar 6 2020, 4:35 AM
akuegel updated this revision to Diff 248704.Mar 6 2020, 4:52 AM
akuegel marked 5 inline comments as done.

Addressed review comments.

mlir/lib/Conversion/StandardToLLVM/ConvertStandardToLLVM.cpp
1169

Actually I first did exactly that but then thought maybe I shouldn't introduce an actual change with this CL. But I completely agree with you, so changed it to SmallVector with constant 4.

This revision was automatically updated to reflect the committed changes.