From 556cc5b4ae63f8584aa7f09f3b1afcfb3bb0b7ca Mon Sep 17 00:00:00 2001 From: baldurk Date: Fri, 16 Aug 2019 16:06:18 +0100 Subject: [PATCH] Refactor spec constants to be stored as uint64_t * A single spec constant can only contain one scalar (vectors are created with composites). We can always store a scalar in uint64_t. --- renderdoc/driver/gl/gl_replay.cpp | 4 +- .../driver/shaders/spirv/spirv_reflect.cpp | 3 +- .../driver/shaders/spirv/spirv_reflect.h | 7 ++- renderdoc/driver/vulkan/vk_info.cpp | 8 ++-- renderdoc/driver/vulkan/vk_replay.cpp | 12 +++-- renderdoc/driver/vulkan/vk_shader_cache.cpp | 46 ++++++++----------- 6 files changed, 40 insertions(+), 40 deletions(-) diff --git a/renderdoc/driver/gl/gl_replay.cpp b/renderdoc/driver/gl/gl_replay.cpp index 6fd618527..d54f97178 100644 --- a/renderdoc/driver/gl/gl_replay.cpp +++ b/renderdoc/driver/gl/gl_replay.cpp @@ -2207,8 +2207,8 @@ void GLReplay::FillCBufferVariables(ResourceId shader, std::string entryPoint, u { SpecConstant spec; spec.specID = shaderDetails.specIDs[i]; - spec.data.resize(sizeof(shaderDetails.specValues[i])); - memcpy(&spec.data[0], &shaderDetails.specValues[i], spec.data.size()); + spec.value = shaderDetails.specValues[i]; + spec.dataSize = 4; specconsts.push_back(spec); } diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp index 72f5fe047..f866fa279 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.cpp +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.cpp @@ -47,8 +47,7 @@ void FillSpecConstantVariables(const rdcarray &invars, { if(specInfo[i].specID == invars[v].byteOffset) { - memcpy(outvars[v].value.uv, specInfo[i].data.data(), - RDCMIN(specInfo[i].data.size(), sizeof(outvars[v].value.uv))); + outvars[v].value.u64v[0] = specInfo[i].value; } } } diff --git a/renderdoc/driver/shaders/spirv/spirv_reflect.h b/renderdoc/driver/shaders/spirv/spirv_reflect.h index 59e06e84f..01b5c80e9 100644 --- a/renderdoc/driver/shaders/spirv/spirv_reflect.h +++ b/renderdoc/driver/shaders/spirv/spirv_reflect.h @@ -132,8 +132,11 @@ void ParseSPIRV(uint32_t *spirv, size_t spirvLength, SPVModule &module); struct SpecConstant { - uint32_t specID; - std::vector data; + SpecConstant() = default; + SpecConstant(uint32_t id, uint64_t val, size_t size) : specID(id), value(val), dataSize(size) {} + uint32_t specID = 0; + uint64_t value = 0; + size_t dataSize = 0; }; void FillSpecConstantVariables(const rdcarray &invars, diff --git a/renderdoc/driver/vulkan/vk_info.cpp b/renderdoc/driver/vulkan/vk_info.cpp index 7c0c19178..868d680ab 100644 --- a/renderdoc/driver/vulkan/vk_info.cpp +++ b/renderdoc/driver/vulkan/vk_info.cpp @@ -247,8 +247,8 @@ void VulkanCreationInfo::Pipeline::Init(VulkanResourceManager *resourceMan, Vulk { SpecConstant spec; spec.specID = maps[s].constantID; - spec.data.assign(data + maps[s].offset, data + maps[s].offset + maps[s].size); - // ignore maps[s].size, assume it's enough for the type + memcpy(&spec.value, data + maps[s].offset, maps[s].size); + spec.dataSize = maps[s].size; shad.specialization.push_back(spec); } } @@ -574,8 +574,8 @@ void VulkanCreationInfo::Pipeline::Init(VulkanResourceManager *resourceMan, Vulk { SpecConstant spec; spec.specID = maps[s].constantID; - spec.data.assign(data + maps[s].offset, data + maps[s].offset + maps[s].size); - // ignore maps[s].size, assume it's enough for the type + memcpy(&spec.value, data + maps[s].offset, maps[s].size); + spec.dataSize = maps[s].size; shad.specialization.push_back(spec); } } diff --git a/renderdoc/driver/vulkan/vk_replay.cpp b/renderdoc/driver/vulkan/vk_replay.cpp index 5d5e54f61..23c43712a 100644 --- a/renderdoc/driver/vulkan/vk_replay.cpp +++ b/renderdoc/driver/vulkan/vk_replay.cpp @@ -1118,8 +1118,10 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) stage.specialization.resize(p.shaders[i].specialization.size()); for(size_t s = 0; s < p.shaders[i].specialization.size(); s++) { - stage.specialization[s].specializationId = p.shaders[i].specialization[s].specID; - stage.specialization[s].data = p.shaders[i].specialization[s].data; + const SpecConstant &spec = p.shaders[i].specialization[s]; + stage.specialization[s].specializationId = spec.specID; + stage.specialization[s].data.resize(spec.dataSize); + memcpy(stage.specialization[s].data.data(), &spec.value, spec.dataSize); } } } @@ -1194,8 +1196,10 @@ void VulkanReplay::SavePipelineState(uint32_t eventId) stages[i]->specialization.resize(p.shaders[i].specialization.size()); for(size_t s = 0; s < p.shaders[i].specialization.size(); s++) { - stages[i]->specialization[s].specializationId = p.shaders[i].specialization[s].specID; - stages[i]->specialization[s].data = p.shaders[i].specialization[s].data; + const SpecConstant &spec = p.shaders[i].specialization[s]; + stages[i]->specialization[s].specializationId = spec.specID; + stages[i]->specialization[s].data.resize(spec.dataSize); + memcpy(stages[i]->specialization[s].data.data(), &spec.value, spec.dataSize); } } diff --git a/renderdoc/driver/vulkan/vk_shader_cache.cpp b/renderdoc/driver/vulkan/vk_shader_cache.cpp index e6f4e1bc2..552bab209 100644 --- a/renderdoc/driver/vulkan/vk_shader_cache.cpp +++ b/renderdoc/driver/vulkan/vk_shader_cache.cpp @@ -272,25 +272,23 @@ void VulkanShaderCache::MakeGraphicsPipelineInfo(VkGraphicsPipelineCreateInfo &p static VkPipelineShaderStageCreateInfo stages[6]; static VkSpecializationInfo specInfo[6]; static std::vector specMapEntries; - static std::vector specdata; + + // the specialization constants can't use more than a uint64_t, so we just over-allocate + static std::vector specdata; size_t specEntries = 0; - size_t specSize = 0; for(uint32_t i = 0; i < 6; i++) - { specEntries += pipeInfo.shaders[i].specialization.size(); - for(size_t s = 0; s < pipeInfo.shaders[i].specialization.size(); s++) - specSize += pipeInfo.shaders[i].specialization[s].data.size(); - } specMapEntries.resize(specEntries); - specdata.resize(specSize); + specdata.resize(specEntries); VkSpecializationMapEntry *entry = specMapEntries.data(); + uint64_t *data = specdata.data(); uint32_t stageCount = 0; - specSize = 0; + uint32_t dataOffset = 0; // reserve space for spec constants for(uint32_t i = 0; i < 6; i++) @@ -313,16 +311,15 @@ void VulkanShaderCache::MakeGraphicsPipelineInfo(VkGraphicsPipelineCreateInfo &p for(size_t s = 0; s < pipeInfo.shaders[i].specialization.size(); s++) { entry[s].constantID = pipeInfo.shaders[i].specialization[s].specID; - entry[s].size = pipeInfo.shaders[i].specialization[s].data.size(); - entry[s].offset = (uint32_t)specSize; + entry[s].size = pipeInfo.shaders[i].specialization[s].dataSize; + entry[s].offset = dataOffset * sizeof(uint64_t); - specSize += entry[s].size; + data[dataOffset] = pipeInfo.shaders[i].specialization[s].value; - memcpy(&specdata[0] + entry[s].offset, pipeInfo.shaders[i].specialization[s].data.data(), - entry[s].size); + dataOffset++; } - specInfo[i].dataSize = specdata.size(); + specInfo[i].dataSize = specdata.size() * sizeof(uint64_t); specInfo[i].pData = specdata.data(); entry += specInfo[i].mapEntryCount; @@ -633,18 +630,16 @@ void VulkanShaderCache::MakeComputePipelineInfo(VkComputePipelineCreateInfo &pip VkPipelineShaderStageCreateInfo stage; // Returned by value static VkSpecializationInfo specInfo; static std::vector specMapEntries; - static std::vector specdata; + + // the specialization constants can't use more than a uint64_t, so we just over-allocate + static std::vector specdata; const uint32_t i = 5; // Compute stage RDCASSERT(pipeInfo.shaders[i].module != ResourceId()); size_t specEntries = pipeInfo.shaders[i].specialization.size(); - size_t specSize = 0; - for(size_t s = 0; s < pipeInfo.shaders[i].specialization.size(); s++) - specSize += pipeInfo.shaders[i].specialization[s].data.size(); - - specdata.resize(specSize); + specdata.resize(specEntries); specMapEntries.resize(specEntries); VkSpecializationMapEntry *entry = &specMapEntries[0]; @@ -656,7 +651,7 @@ void VulkanShaderCache::MakeComputePipelineInfo(VkComputePipelineCreateInfo &pip stage.pSpecializationInfo = NULL; stage.flags = VK_SHADER_STAGE_COMPUTE_BIT; - specSize = 0; + uint32_t dataOffset = 0; if(!pipeInfo.shaders[i].specialization.empty()) { @@ -667,13 +662,12 @@ void VulkanShaderCache::MakeComputePipelineInfo(VkComputePipelineCreateInfo &pip for(size_t s = 0; s < pipeInfo.shaders[i].specialization.size(); s++) { entry[s].constantID = pipeInfo.shaders[i].specialization[s].specID; - entry[s].size = pipeInfo.shaders[i].specialization[s].data.size(); - entry[s].offset = (uint32_t)specSize; + entry[s].size = pipeInfo.shaders[i].specialization[s].dataSize; + entry[s].offset = dataOffset; - specSize += entry[s].size; + specdata[dataOffset] = pipeInfo.shaders[i].specialization[s].value; - memcpy(&specdata[0] + entry[s].offset, pipeInfo.shaders[i].specialization[s].data.data(), - entry[s].size); + dataOffset++; } specInfo.dataSize = specdata.size();