This is an archive of the discontinued LLVM Phabricator instance.

[mlir:Bytecode] Add encoding support for builtin location attributes
ClosedPublic

Authored by rriddle on Aug 24 2022, 2:40 AM.

Details

Summary

This provides a significantly more efficient encoding for locations.

Diff Detail

Event Timeline

rriddle created this revision.Aug 24 2022, 2:40 AM
rriddle requested review of this revision.Aug 24 2022, 2:40 AM
mehdi_amini added inline comments.Aug 24 2022, 9:18 AM
mlir/lib/IR/BuiltinDialectBytecode.cpp
405

I wonder why you split the writer/reader?
I felt it may be easier to have for each attribute/type/loc the reader and writer method side-by-side: after all they are supposed to be symmetric.

rriddle added inline comments.Aug 24 2022, 9:31 AM
mlir/lib/IR/BuiltinDialectBytecode.cpp
405

It was just easier for me to reason about in the beginning. I don't have a huge preference and could move them next to each other, let me know if you want that I'll move them in a follow up.

mehdi_amini accepted this revision.Aug 24 2022, 9:37 AM
mehdi_amini added inline comments.
mlir/lib/IR/BuiltinDialectBytecode.cpp
405

Take it as a suggestion for a future revision :)

This revision is now accepted and ready to land.Aug 24 2022, 9:37 AM
rriddle updated this revision to Diff 455287.Aug 24 2022, 10:56 AM
rriddle edited the summary of this revision. (Show Details)
jpienaar accepted this revision.Aug 24 2022, 12:25 PM
jpienaar added inline comments.
mlir/lib/IR/BuiltinDialectBytecode.cpp
379

Should there be an error emitted about failure here? (I don't know where the errors are reported from or how we ensure at least an error is reported)

rriddle marked an inline comment as done.Aug 24 2022, 12:27 PM
rriddle added inline comments.
mlir/lib/IR/BuiltinDialectBytecode.cpp
379

All of the API provided by the DialectBytecodeReader/DialectBytecodeWriter call into the main bytecode API, which will emit errors on failure (so we don't emit here to avoid double-emit).

rriddle marked 3 inline comments as done.Aug 24 2022, 12:28 PM
rriddle added inline comments.
mlir/lib/IR/BuiltinDialectBytecode.cpp
405

Done in D132583

rriddle updated this revision to Diff 455360.Aug 24 2022, 1:46 PM
rriddle marked an inline comment as done.
rriddle updated this revision to Diff 455862.Aug 26 2022, 3:55 AM