[MLIR][SPIRVToLLVM] SPIR-V types size in bytes function

[georgemitenkov]

Summary

Added getTypeNumBytes() function as a class member to several SPIR-V Types:

• Scalar
• Array
• Vector
• Pointer

This is a function that will expose getTypeNumBytes() from SPIRVLowering.cpp to other classes. Support of other SPIR-V types can be added on demand.

[georgemitenkov]

@antiagainst, @mravishankar I haven't linked getTypeNumBytes() from SPIRVLowering.cpp to this new implementation? Should this be done as well?

[antiagainst]

mlir/include/mlir/Dialect/SPIRV/SPIRVTypes.h

113

Nit: The Type in the method does not help much given this is a method for .. type. :)

Typically think the method name and variable name and other name as the first line of documentation. A good name can go a long way for helping others understanding the code without referencing to the documentation and others additional stuff. For example, NumBytes is good here given it's very clear the unit of the return number so one does not need to guess whether is in bytes or bits or whatever. What do you think about getSizeInBytes here?

Also we need to document it here. What if the type can have explicit layout? What if not? What does it mean to return None?

mlir/lib/Dialect/SPIRV/SPIRVTypes.cpp

159

We need to have documentation on the ArrayType mentioning the padding behavior here.

300

Is this correct? Don't we need to multiply with the count?

640

Hmm, this is not correct actually. There is no defined size for SPIR-V pointer type... We should return None here actually. I think even for LLVM, the pointer type size isn't fixed; it depends on the data layout.

906

No need to have these elses given we return for each case.

[georgemitenkov]

Renamed getTypeNumBytes() to getSizeInBytes().
Added documentation for the function declaration and a comment on array strides.
Removed support of PointerType and fixed Vector size.

[antiagainst]

mlir/lib/Dialect/SPIRV/SPIRVTypes.cpp

159

Sorry I meant this should be on the declaration side (.h); not the implementation side (.cpp). :)

[georgemitenkov]

Moved stride comment on SPIRV's array type getSizeInBytes() from .cpp to .h file.