[spirv] Support serialization for spv.struct.

[denis0x0D]

Serialization for spv.struct must be added alongside with decoration with BufferBlock.

BufferBlock apply to a structure type to establish it is an SSBO-like shader-interface block.

OpDecorate %structType BufferBlock

Note: it is a blocker for https://github.com/tensorflow/mlir/issues/60

[denis0x0D]

@antiagainst @MaheshRavishankar I've started to work on this. Thanks!

[antiagainst]

Cool, thanks @denis0x0D!

[MaheshRavishankar]

Fantastic @denis0x0D ! Please let me know if you need any help.

[denis0x0D]

@antiagainst @MaheshRavishankar thanks a lot! @MaheshRavishankar serialization for struct type and members decoration looks straight forward, since struct type already has layoutinfo. But I'm not sure about decoration with Buffer or BufferBlock , the spec requires to decorate struct type with BufferBlock :

Apply to a structure type to establish it is an SSBO-like shader-interface block

and Block:

Apply to a structure type to establish it is a non-SSBO-like shader-interface block

it seems like we need an additional field in the StructType to specify SSBO or non-SSBO, and therefore update the ebnf of this type, but I might miss something, please fix me if I'm wrong. Thanks.

[antiagainst]

Good question!

It's a confusing point that by applying BufferBlock decoration, you can actually write to Uniform storage class: Uniform storage class together with BufferBlock actually means StorageBuffer storage class. SPV_KHR_storage_buffer_storage_class extension is introduced to address this and it becomes core since SPIR-V 1.4. BufferBlock decoration is depreciated and will be removed from SPIR-V 1.4. So I'd suggest we always use Block decoration and choose Uniform and StorageBuffer storage class to differentiate to be future proof. (Yes it requires the SPV_KHR_storage_buffer_storage_class extension for using StorageBuffer storage class pre-1.4 but it is fairly commonly implemented.)

During serialization of the struct type, there is another thing to pay attention to. Per the spec:

Block and BufferBlock decorations cannot decorate a structure type that is nested at any level inside another structure type decorated with Block or BufferBlock.

It means we can only attach the Block decoration to the top-level OpTypeStruct that are used for resource variables. I think we should let the type serialization itself only emit the offset decorations and we should leave the Buffer decoration to the resource variable. Specially, when we see a spv.globalVariable of type !spv.ptr<!spv.struct<...>, Uniform>, we call processType for !spv.struct<...> to serialize the type itself and all offset decorations, and then emit an additional Block decoration for the struct type when handling the global variable itself. Thus we can avoid emitting the Block decoration for nested structs.

[denis0x0D]

@antiagainst thanks a lot for detailed explanation! Is it ok to separte this PR into:

1. serialization for spv.struct with offset decorations.
2. another one related to decoration of top-level spv.struct with Buffer

By the way, seems like to test decoration with Buffer requires to write a check for binary form, since it will no affect text format, am I right? Please correct me if I'm missing something. Thanks.

[antiagainst]

SGTM to implement them with separate PRs!

You are right that the Buffer decoration instruction won't be reflected in the MLIR spv.module deserialized. Writing tests to check the result binary is one way to go. Right now we have GoogleTest based DeserializationTest that checks corner cases of the deserializer (lots of tests still need to be added), it would be nice to have something similar for serialization.

[denis0x0D]

@antiagainst thanks, got it.

[denis0x0D]

@antiagainst @MaheshRavishankar is it ok, if I start to work on "decoration of top-level spv.struct with Buffer"?

[antiagainst]

Sure, please feel free to take it! Thanks a lot, @denis0x0D !

[denis0x0D]

closed by https://github.com/tensorflow/mlir/commit/5802e0f9c2b698924b7b5eb790b61b841d6f30f4