Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array size specialization constant reflection #110

Open
nyorain opened this issue Jan 19, 2021 · 2 comments
Open

Array size specialization constant reflection #110

nyorain opened this issue Jan 19, 2021 · 2 comments

Comments

@nyorain
Copy link

nyorain commented Jan 19, 2021

It's currently not possible to see the specialization constants for a module. Furthermore, it's not possible to get the size of an array when it's dependent on a specialization constant. I would be interested in adding an API for that. Effectively, given the specialization constants for a shader module (e.g. in a vulkan-like VkSpecializationInfo equivalent), it should be possible to retrieve array sizes.

Another (mildly related) issue i've stumbled across is that OpTypeRuntimeArray is completely ignored. The library could mark the dimension of the array with a special value instead, signaling that it's a runtime array.

Below is an API sketch, consider this something like an RFC. Are there any other cases where specialization constants should be considered? Is this out of scope of the library altogether? It would involve evaluating a whole set of specialization constant operations after all.

An implementation problem is that we would need to preserve the nodes from the parser to correctly evaluate a specialization constant operation later on I guess (we could just extract the nodes that are needed for the constant op or build a smaller internal representation of the operations but that adds further complexity). I'd start with int/uint/bool spec constant ops since only those are allowed for shaders but the API could be extended later on for float spec constants ops (for kernels).

// Changes to ArrayTraits:
typedef struct SpvReflectArrayTraits {
  uint32_t                          dims_count;
  // For a variable-sized array (OpTypeRuntimeArray), the value will be 0x0.
  // For a specialization constant size, it will be 0xFFFFFFFF (see spec_const_op_ids)
  uint32_t                          dims[SPV_REFLECT_MAX_ARRAY_DIMS];
  // For dimensions that have a specialization constant operation as size, this
  // array holds the id of the speicalization constant operation.
  // It can be evaluated (given a set of original specialization constants)
  // using spvReflectEvaluateSpecConstOpID
  uint32_t                          spec_const_op_ids[SPV_REFLECT_MAX_ARRAY_DIMS];
  uint32_t                          stride; // Measured in bytes
} SpvReflectArrayTraits;

// Allow to reflect over specialization constants
typedef struct SpvReflectSpecializationConstant {
  const char* name;
  uint32_t spirv_id;
  uint32_t constant_id;
  SpvReflectTypeDescription* type_description;
  uint64_t default_value;
} SpvReflectSpecializationConstant;

// Changes to SpvReflectShaderModule
typedef struct SpvReflectShaderModule {
  // ...
  uint32_t spec_constant_count;
  SpvSpecializationConstant** spec_constants;
} SpvReflectShaderModule;


// Add new SpvReflectSpecializationInfo, SpvReflectSpecializationMapEntry structs mirroring vulkan
// ...


// Like spvReflectCreateShaderModule but stores the parsed spv data inside the
// shader module, allowing future operations at the cost of higher memory consumption.
SpvReflectResult spvReflectCreateShaderModuleWithParser(
  size_t                   size,
  const void*              p_code,
  SpvReflectShaderModule*  p_module
);

// Only valid when the given shader module was created with a parser-preserving function.
// Evaluates the given constant op id using the given specialization info.
SpvReflectResult spvReflectEvaluateSpecConstOpID(
  SpvReflectShaderModule*           p_module,
  uint32_t                          spec_const_op_id,
  SpvReflectSpecializationInfo*     p_spec_info,
  uint32_t*                         p_out_const_op_value);
@mgsegal
Copy link
Contributor

mgsegal commented Jan 26, 2021

Thanks for bringing this up and for providing a comprehensive proposal. We're looking at it, and will likely implement something very much like it, although it won't happen right away.

@shangjiaxuan
Copy link

Working on just this problem. Still WIP though... #154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants