Page MenuHomePhabricator

[MLIR][SPIRV] Split (De-)serialization into 2 libs.
ClosedPublic

Authored by ergawy on Nov 16 2020, 8:26 AM.

Details

Summary

This commit splits SPIR-V's serialization and deserialization code into
2 separate targets. The motiviation being that the serializer is used
more often the deserializer and therefore lumping them together
unnecessarily increases binary size for the most common case.

Diff Detail

Event Timeline

ergawy created this revision.Nov 16 2020, 8:26 AM
ergawy requested review of this revision.Nov 16 2020, 8:27 AM
antiagainst requested changes to this revision.Nov 17 2020, 9:27 AM

Awesome! Given we are here already; can we move them to the final place they should be? Specifically,

include/mlir/Target/
  SPIRV/
    Serialization.h (for containing the `serialize` entry point)
    Deserialization.h (for containing the `deserialize` entry point)
lib/Target/
  SPIRV/
    (All cpp files here)
This revision now requires changes to proceed.Nov 17 2020, 9:27 AM
ergawy updated this revision to Diff 306049.Nov 18 2020, 4:22 AM

Re-organize files a bit differently.

ergawy updated this revision to Diff 306056.Nov 18 2020, 4:49 AM

Fix header guards.

antiagainst accepted this revision.Nov 19 2020, 7:00 AM
This revision is now accepted and ready to land.Nov 19 2020, 7:00 AM

Hi,

Friendly reminder to land this :). I would like to add (de-)serialization support for OpSpecConstantOp and conflicts will be easier to handle if this patch is merged before I start. If you need to wait on this one for a bit more, then no problem I will work on my new patch and handle conflicts later.

This revision was landed with ongoing or failed builds.Dec 14 2020, 9:28 AM
This revision was automatically updated to reflect the committed changes.

Sorry about the delay! There were issues building with shared library. I fixed them. (You can diff the version here against the landed commit to find out what's gotten changed if curious.) Also need to wire up the BUILD changes for Bazel (https://github.com/google/llvm-bazel) for Google internal uses.