[SYCL] Add SYCL property set registry class

[jzc]

This PR adds the PropertySetRegistry and PropertyValue classes, which are used to communicate metadata to the SYCL runtime for a single device image. After device code split, module metadata will be computed for each IR module and stored inside a PropertySetRegistry. The PropertySetRegistry is then serialized as a JSON object and stored under the "sycl_properties" key in the string table of the output OffloadingImage of the clang-sycl-linker. Finally, clang-linker-wrapper will consume and deserialize this PropertySetRegistry when performing wrapping for SYCL images.

[jzc]

I've simplified the byte array code compared to the downstream code, but they won't compatible downstream and would cause ABI-break and runtime changes. The downstream implementation always wrote the number of bits the data uses in the first 8 bytes of a byte-array property value, but after some tests, I found this values way never used, except for one place that could be worked around.

https://github.com/intel/llvm/blob/a9db58476ab1e1ff87f128088ac203539d87d22b/llvm/lib/Support/PropertySetIO.cpp#L153-L169

[jzc]

For now, I've placed the class in Frontend/Offloading/Utility.h. If we get Frontend/Offloading/SYCL later on then I then that would be the most appropriate place to put it.

[asudarsa]

I think it might be a better idea to add this in a separate file called PropertySet.h (associated impl in PropertySet.cpp).

Thanks

[asudarsa]

Adding @sarnex for comment as he has worked with Utility.h before.

Thanks

[asudarsa]

Why do we need this?

Thanks

[asudarsa]

Changes to ClangSYCLLinker.cpp can be done in a separate PR. WDYT?

Thanks

[maksimsab]

I would suggest simple "properties".

[maksimsab]

I would discourage usage of SmallString<16>.
https://github.com/intel/llvm/blob/sycl/sycl/doc/design/PropertySets.md
Typically, keys are larger than 16 and we wouldn't make use of this limit.
Also, LLVM's Programmers Manual discourages putting SmallString as the member of a frequently-allocated heap data structure.

I would rather make it std::map<std::string, std::map<std::string, PropertyValue>>.

We aren't going to have many keys here and it would make the interface extremely simple.

[maksimsab]

I think that this class was overengineered from the very beginning.
This class can be replaced by simple std::variant<uint64_t, std::string>. In the text format it is not going to have much difference either it is uint64_t or uint32_t.

Do you have any objections againts the following declaration?

using PropertyValue = std::variant<uint64_t, std::string>;

[maksimsab]

I've just read what monostate is. It allows std::variant to be empty. However, I don't think we should allow our variants to be empty. Therefore, I would discourage a use of monostate.

[maksimsab]

Summary of my opinion.

I think, that the initial design was very poor and it doesn't worth to save it anyhow.

We could come up with the simple design like the following:

namespace sycl {

using PropertyValue = std::variant<uint64_t, std::string>;

using Properties = std::map<std::string, std::map<std::string, PropertyValue>>;

void writePropertiesToJSON(const Properties &P, raw_ostream &O);

Expected<Properties> readPropertiesFromJSON(MemoryBufferRef MBR);

} // namespace sycl

Everything else could be thrown away. And this has the most chances to get upstreamed in my opinion.
Please, share your opinion.