This is an archive of the discontinued LLVM Phabricator instance.

Preserve function argument locations.
ClosedPublic

Authored by dominikgrewe on Jan 18 2022, 2:03 PM.

Details

Summary

Previously the optional locations of function arguments were dropped in parseFunctionArgumentList. This CL adds another output argument to the function through which they are now returned. The values are then plumbed through as an array of optional locations in the various places.

Diff Detail

Event Timeline

dominikgrewe created this revision.Jan 18 2022, 2:03 PM
dominikgrewe requested review of this revision.Jan 18 2022, 2:03 PM

Nice!

mlir/lib/IR/FunctionImplementation.cpp
265

I'd expect one can just pass argLocations, as these shouldn't be populated if empty. What happens if you pass it?

mlir/lib/Parser/Parser.cpp
1820–1821

Could one use llvm::zip here to iterate over both together?

Updated function comments.

dominikgrewe added inline comments.Jan 18 2022, 2:30 PM
mlir/lib/IR/FunctionImplementation.cpp
265

I actually get quite a few test failures due to the assert I added to OperationParser::parseRegionBody.

mlir/lib/Parser/Parser.cpp
1820–1821

Not if argLocations is empty right?

rriddle added inline comments.Jan 18 2022, 3:28 PM
mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
686–688

I'd just drop the explicit numbers now that SmallVector supports it.

mlir/lib/Parser/Parser.cpp
1820–1821

Nit: use llvm::enumerate() here.

1832–1834
mlir/test/lib/Dialect/Test/TestDialect.cpp
598

Please add an argument comment here.

dominikgrewe marked 3 inline comments as done.

Addressed comments.

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp
686–688

Changed it here and in other functions touched by this CL.

mlir/lib/Parser/Parser.cpp
1820–1821

Using llvm::enumerate now as per River's comment.

Addressed comments (with actual changes this time)

rriddle accepted this revision.Jan 19 2022, 3:44 PM

LGTM, thanks for doing this! Really appreciated.

This revision is now accepted and ready to land.Jan 19 2022, 3:44 PM
This revision was automatically updated to reflect the committed changes.