[llvm] [NFC] Refactoring DXContainerYaml Root Parameter representation (PR #138318)

Justin Bogner via llvm-commits llvm-commits at lists.llvm.org
Fri May 23 09:21:25 PDT 2025


https://github.com/bogner requested changes to this pull request.

Similarly to the discussion in #137284, I'm not really convinced that `std::variant` makes sense here. These two types (RootConstantsYaml and RootDescriptorYaml) are relatively small and similarly sized (12 bytes each), so a strongly typed union like std::variant seems fairly reasonable, but consider what we get once we add Descriptor Table handling - that object contains a SmallVector of objects that are 28 bytes large each, as well as a couple of other fields. Even if the SmallVector's inline storage is only a single element, that's on the order of 5 times the size of the other two variant types, which means most of the objects in the vector of ParameterData will have 4 times as much wasted space as they do actual data.

On the other hand, if we put something like a `std::optional<size_t> IndexInSignature` here, we can store each type of data contiguously, and it doesn't even really change the amount of complexity overall. The one thing that will be a bit messier is the `mapping` function for RootParameterYamlDesc, but we can use `MappingContextTraits` there in order to pass along the `RootSignatureYamlDesc` as a "Context" there, and then define some lazy accessor helpers in RootSignatureYamlDesc, like:

```c++
  template <typename T>
  T &getOrInsertImpl(RootParameterYamlDesc &ParamDesc,
                     SmallVectorImpl<T> &Container) {
    if (!ParamDesc.IndexInSignature) {
      ParamDesc.IndexInSignature = Container.size();
      Container.emplace_back();
    }
    return Container[*ParamDesc.IndexInSignature];
  }
  RootConstantsYaml &getOrInsertConstants(RootParameterYamlDesc &ParamDesc) {
    return getOrInsertImpl(ParamDesc, Constants);
  }
  RootDescriptorYaml &getOrInsertDescriptor(RootParameterYamlDesc &ParamDesc) {
    return getOrInsertImpl(ParamDesc, Descriptors);
  }
``` 

https://github.com/llvm/llvm-project/pull/138318


More information about the llvm-commits mailing list