From c3cce4a7147d7c75ea2530f21ac2e2a9efc34be4 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Fri, 17 May 2019 15:06:06 -0400 Subject: [PATCH 01/11] Support shader specialization in Vulkan and Amberscript Fixes #510 * Parser support for SPECIALIZE subcommand of ATTACH * tests * Documentation * Support Vulkan pipeline creation using shader specialization --- docs/amber_script.md | 4 + src/amberscript/parser.cc | 68 +++++++++++- src/amberscript/parser.h | 1 + src/amberscript/parser_attach_test.cc | 152 ++++++++++++++++++++++++++ src/pipeline.h | 9 ++ 5 files changed, 233 insertions(+), 1 deletion(-) diff --git a/docs/amber_script.md b/docs/amber_script.md index aeea59d08..23f2c7748 100644 --- a/docs/amber_script.md +++ b/docs/amber_script.md @@ -197,6 +197,10 @@ The following commands are all specified within the `PIPELINE` command. # Attach a 'multi' shader to the pipeline of |shader_type| and use the entry # point with |name|. The provided shader _must_ be a 'multi' shader. ATTACH {name_of_multi_shader} TYPE {shader_type} ENTRY_POINT {name} + + # Attach specialized shader. Specialization can be specified multiple times. + ATTACH {name_of_shader} ENTRY_POINT {name} SPECIALIZE 1 AS uint32 4 + ATTACH {name_of_shader} ENTRY_POINT {name} SPECIALIZE 1 AS uint32 4 SPECIALIZE 4 AS float 1.0 ``` ```groovy diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc index 7e1dbf08b..68635ccfd 100644 --- a/src/amberscript/parser.cc +++ b/src/amberscript/parser.cc @@ -488,7 +488,73 @@ Result Parser::ParsePipelineAttach(Pipeline* pipeline) { if (!r.IsSuccess()) return r; - return ValidateEndOfStatement("ATTACH command"); + while (true) { + token = tokenizer_->NextToken(); + if (token->IsString() && token->AsString() == "SPECIALIZE") { + r = ParseShaderSpecialization(pipeline); + if (!r.IsSuccess()) + return r; + } else { + if (token->IsEOL() || token->IsEOS()) + return {}; + return Result("extra parameters after ATTACH command"); + } + } +} + +Result Parser::ParseShaderSpecialization(Pipeline* pipeline) { + auto token = tokenizer_->NextToken(); + if (!token->IsInteger()) + return Result("specialization ID must be an integer"); + + auto spec_id = token->AsUint32(); + if (spec_id == 0) + return Result("specialization ID must be greater than 0"); + + token = tokenizer_->NextToken(); + if (!token->IsString() || token->AsString() != "AS") + return Result("expected AS as next token"); + + token = tokenizer_->NextToken(); + if (!token->IsString()) + return Result("expected data type in SPECIALIZE subcommand"); + + DatumType type; + auto r = ToDatumType(token->AsString(), &type); + if (!r.IsSuccess()) + return r; + + token = tokenizer_->NextToken(); + uint32_t value = 0; + switch (type.GetType()) { + case DataType::kUint32: + value = token->AsUint32(); + break; + case DataType::kInt32: { + union { + uint32_t u; + int32_t i; + } u; + u.i = token->AsInt32(); + value = u.u; + break; + } + case DataType::kFloat: { + union { + uint32_t u; + float f; + } u; + u.f = token->AsFloat(); + value = u.u; + break; + } + default: + return Result( + "only 32-bit types are currently accepted for specialization values"); + } + auto& shader = pipeline->GetShaders()[pipeline->GetShaders().size() - 1]; + shader.AddSpecialization(spec_id, value); + return {}; } Result Parser::ParsePipelineShaderOptimizations(Pipeline* pipeline) { diff --git a/src/amberscript/parser.h b/src/amberscript/parser.h index f6ceff890..f5d7b46b7 100644 --- a/src/amberscript/parser.h +++ b/src/amberscript/parser.h @@ -77,6 +77,7 @@ class Parser : public amber::Parser { Result ParseDerivePipelineBlock(); Result ParsePipelineBody(const std::string& cmd_name, std::unique_ptr pipeline); + Result ParseShaderSpecialization(Pipeline* pipeline); /// Parses a set of values out of the token stream. |name| is the name of the /// current command we're parsing for error purposes. The |type| is the type diff --git a/src/amberscript/parser_attach_test.cc b/src/amberscript/parser_attach_test.cc index 540b7adf1..cf798a7c5 100644 --- a/src/amberscript/parser_attach_test.cc +++ b/src/amberscript/parser_attach_test.cc @@ -307,5 +307,157 @@ END)"; EXPECT_EQ("6: ATTACH missing TYPE for multi shader", r.Error()); } +TEST_F(AmberScriptParserTest, PipelineSpecializationUint32) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE 1 AS uint32 4 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_TRUE(r.IsSuccess()); + + auto script = parser.GetScript(); + const auto& pipelines = script->GetPipelines(); + ASSERT_EQ(1U, pipelines.size()); + + const auto* pipeline = pipelines[0].get(); + const auto& shaders = pipeline->GetShaders(); + ASSERT_EQ(1U, shaders.size()); + + EXPECT_EQ(1, shaders[0].GetSpecialization().size()); + EXPECT_EQ(4, shaders[0].GetSpecialization().at(1)); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationInt32) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE 2 AS int32 -1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_TRUE(r.IsSuccess()); + + auto script = parser.GetScript(); + const auto& pipelines = script->GetPipelines(); + ASSERT_EQ(1U, pipelines.size()); + + const auto* pipeline = pipelines[0].get(); + const auto& shaders = pipeline->GetShaders(); + ASSERT_EQ(1U, shaders.size()); + + EXPECT_EQ(1, shaders[0].GetSpecialization().size()); + EXPECT_EQ(0xffffffff, shaders[0].GetSpecialization().at(2)); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationFloat) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE 3 AS float 1.1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_TRUE(r.IsSuccess()); + + auto script = parser.GetScript(); + const auto& pipelines = script->GetPipelines(); + ASSERT_EQ(1U, pipelines.size()); + + const auto* pipeline = pipelines[0].get(); + const auto& shaders = pipeline->GetShaders(); + ASSERT_EQ(1U, shaders.size()); + + EXPECT_EQ(1, shaders[0].GetSpecialization().size()); + EXPECT_EQ(0x3f8ccccd, shaders[0].GetSpecialization().at(3)); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationIDIsString) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE s3 AS float 1.1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_FALSE(r.IsSuccess()); + EXPECT_EQ("6: specialization ID must be an integer", r.Error()); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationIDIsZero) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE 0 AS float 1.1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_FALSE(r.IsSuccess()); + EXPECT_EQ("6: specialization ID must be greater than 0", r.Error()); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationNoAS) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE 1 ASa float 1.1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_FALSE(r.IsSuccess()); + EXPECT_EQ("6: expected AS as next token", r.Error()); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationNotDataType) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE 1 AS uint 1.1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_FALSE(r.IsSuccess()); + EXPECT_EQ("6: invalid data_type provided", r.Error()); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationBadDataType) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader ENTRY_POINT my_ep SPECIALIZE 1 AS uint8 1.1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_FALSE(r.IsSuccess()); + EXPECT_EQ( + "6: only 32-bit types are currently accepted for specialization values", + r.Error()); +} + } // namespace amberscript } // namespace amber diff --git a/src/pipeline.h b/src/pipeline.h index 554494ce4..af0f85e43 100644 --- a/src/pipeline.h +++ b/src/pipeline.h @@ -15,6 +15,7 @@ #ifndef SRC_PIPELINE_H_ #define SRC_PIPELINE_H_ +#include #include #include #include @@ -59,12 +60,20 @@ class Pipeline { const std::vector GetData() const { return data_; } void SetData(std::vector&& data) { data_ = std::move(data); } + const std::map& GetSpecialization() const { + return specialization_; + } + void AddSpecialization(uint32_t spec_id, uint32_t value) { + specialization_[spec_id] = value; + } + private: Shader* shader_ = nullptr; ShaderType shader_type_; std::vector shader_optimizations_; std::string entry_point_; std::vector data_; + std::map specialization_; }; /// Information on a buffer attached to the pipeline. From 1d8f1fd6d55a817acf6c8de76ef58af2aceb7742 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Tue, 21 May 2019 10:58:05 -0400 Subject: [PATCH 02/11] Hook up spec constants in Vulkan --- src/vulkan/engine_vulkan.cc | 29 +++++++++++++++++++++++++++++ src/vulkan/engine_vulkan.h | 8 ++++++++ 2 files changed, 37 insertions(+) diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index 33c1c727e..f8760322a 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -258,6 +258,31 @@ Result EngineVulkan::SetShader(amber::Pipeline* pipeline, } info.shaders[type] = shader; + + for (auto& shader_info : pipeline->GetShaders()) { + if (shader_info.GetShaderType() == type) { + const auto& shader_spec_info = shader_info.GetSpecialization(); + if (!shader_spec_info.empty()) { + auto& entries = info.specialization_entries[type]; + auto& entry_data = info.specialization_data[type]; + uint32_t i = 0; + for (auto pair : shader_spec_info) { + entries.push_back({pair.first, + static_cast(i * sizeof(uint32_t)), + static_cast(sizeof(uint32_t))}); + entry_data.push_back(pair.second); + ++i; + } + auto& spec_info = info.specialization_info[type]; + spec_info.mapEntryCount = + static_cast(shader_spec_info.size()); + spec_info.pMapEntries = entries.data(); + spec_info.dataSize = sizeof(uint32_t) * shader_spec_info.size(); + spec_info.pData = entry_data.data(); + } + } + } + return {}; } @@ -280,6 +305,10 @@ Result EngineVulkan::GetVkShaderStageInfo( stage_info[stage_count].stage = stage; stage_info[stage_count].module = it.second; stage_info[stage_count].pName = nullptr; + auto spec_iter = info.specialization_info.find(it.first); + if (spec_iter != info.specialization_info.end()) { + stage_info[stage_count].pSpecializationInfo = &spec_iter->second; + } ++stage_count; } *out = stage_info; diff --git a/src/vulkan/engine_vulkan.h b/src/vulkan/engine_vulkan.h index 470ddd23f..55f86c26b 100644 --- a/src/vulkan/engine_vulkan.h +++ b/src/vulkan/engine_vulkan.h @@ -66,6 +66,14 @@ class EngineVulkan : public Engine { std::unique_ptr vertex_buffer; std::unordered_map> shaders; + std::unordered_map, + CastHash> + specialization_entries; + std::unordered_map, CastHash> + specialization_data; + std::unordered_map> + specialization_info; }; Result GetVkShaderStageInfo( From e23f31b201016f0e176e39e7ab74ea3a87bda2ef Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Fri, 24 May 2019 09:44:13 -0400 Subject: [PATCH 03/11] Allow specialization without requiring other subcommands --- src/amberscript/parser.cc | 23 +++++++---- src/amberscript/parser_attach_test.cc | 59 ++++++++++++++++++++++++++- 2 files changed, 73 insertions(+), 9 deletions(-) diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc index 68635ccfd..a1ee47dad 100644 --- a/src/amberscript/parser.cc +++ b/src/amberscript/parser.cc @@ -470,7 +470,7 @@ Result Parser::ParsePipelineAttach(Pipeline* pipeline) { type = token->AsString(); } - if (type != "ENTRY_POINT") + if (set_shader_type && type != "ENTRY_POINT") return Result("Unknown ATTACH parameter: " + type); if (shader->GetType() == ShaderType::kShaderTypeMulti && !set_shader_type) @@ -480,23 +480,30 @@ Result Parser::ParsePipelineAttach(Pipeline* pipeline) { if (!r.IsSuccess()) return r; - token = tokenizer_->NextToken(); - if (!token->IsString()) - return Result("missing shader name in ATTACH ENTRY_POINT command"); + if (type == "ENTRY_POINT") { + token = tokenizer_->NextToken(); + if (!token->IsString()) + return Result("missing shader name in ATTACH ENTRY_POINT command"); - r = pipeline->SetShaderEntryPoint(shader, token->AsString()); - if (!r.IsSuccess()) - return r; + r = pipeline->SetShaderEntryPoint(shader, token->AsString()); + if (!r.IsSuccess()) + return r; - while (true) { token = tokenizer_->NextToken(); + } + + while (true) { if (token->IsString() && token->AsString() == "SPECIALIZE") { r = ParseShaderSpecialization(pipeline); if (!r.IsSuccess()) return r; + + token = tokenizer_->NextToken(); } else { if (token->IsEOL() || token->IsEOS()) return {}; + if (token->IsString()) + return Result("Unknown ATTACH parameter: " + token->AsString()); return Result("extra parameters after ATTACH command"); } } diff --git a/src/amberscript/parser_attach_test.cc b/src/amberscript/parser_attach_test.cc index cf798a7c5..50a4d83e8 100644 --- a/src/amberscript/parser_attach_test.cc +++ b/src/amberscript/parser_attach_test.cc @@ -215,7 +215,7 @@ END)"; Parser parser; Result r = parser.Parse(in); ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("6: extra parameters after ATTACH command", r.Error()); + EXPECT_EQ("6: Unknown ATTACH parameter: INVALID", r.Error()); } TEST_F(AmberScriptParserTest, PiplineMultiShaderAttach) { @@ -318,6 +318,7 @@ END)"; Parser parser; Result r = parser.Parse(in); + EXPECT_EQ(r.Error(), ""); ASSERT_TRUE(r.IsSuccess()); auto script = parser.GetScript(); @@ -459,5 +460,61 @@ END)"; r.Error()); } +TEST_F(AmberScriptParserTest, PipelineSpecializationMultipleSpecializations) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader TYPE compute ENTRY_POINT my_ep \ + SPECIALIZE 1 AS uint32 4 \ + SPECIALIZE 2 AS uint32 5 \ + SPECIALIZE 5 AS uint32 1 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_TRUE(r.IsSuccess()); + + auto script = parser.GetScript(); + const auto& pipelines = script->GetPipelines(); + ASSERT_EQ(1U, pipelines.size()); + + const auto* pipeline = pipelines[0].get(); + const auto& shaders = pipeline->GetShaders(); + ASSERT_EQ(1U, shaders.size()); + + EXPECT_EQ(3, shaders[0].GetSpecialization().size()); + EXPECT_EQ(4, shaders[0].GetSpecialization().at(1)); + EXPECT_EQ(5, shaders[0].GetSpecialization().at(2)); + EXPECT_EQ(1, shaders[0].GetSpecialization().at(5)); +} + +TEST_F(AmberScriptParserTest, PipelineSpecializationNoType) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute my_pipeline + ATTACH my_shader SPECIALIZE 1 AS uint32 4 +END)"; + + Parser parser; + Result r = parser.Parse(in); + ASSERT_TRUE(r.IsSuccess()); + + auto script = parser.GetScript(); + const auto& pipelines = script->GetPipelines(); + ASSERT_EQ(1U, pipelines.size()); + + const auto* pipeline = pipelines[0].get(); + const auto& shaders = pipeline->GetShaders(); + ASSERT_EQ(1U, shaders.size()); + + EXPECT_EQ(1, shaders[0].GetSpecialization().size()); + EXPECT_EQ(4, shaders[0].GetSpecialization().at(1)); +} + + } // namespace amberscript } // namespace amber From 5d38497410280532555833c61e61f70dc0c4ed93 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Fri, 24 May 2019 13:53:58 -0400 Subject: [PATCH 04/11] Refactor shader info data * Moved shader data into PipelineInfo into sub-struct ShaderInfo * added specialization test --- src/vulkan/engine_vulkan.cc | 51 +++++------ src/vulkan/engine_vulkan.h | 19 ++--- tests/cases/shader_specialization.amber | 109 ++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 34 deletions(-) create mode 100644 tests/cases/shader_specialization.amber diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index f8760322a..5e60cc440 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -78,12 +78,12 @@ EngineVulkan::~EngineVulkan() { for (auto it = pipeline_map_.begin(); it != pipeline_map_.end(); ++it) { auto& info = it->second; - for (auto mod_it = info.shaders.begin(); mod_it != info.shaders.end(); + for (auto mod_it = info.shader_info.begin(); mod_it != info.shader_info.end(); ++mod_it) { auto vk_device = device_->GetVkDevice(); - if (vk_device != VK_NULL_HANDLE && mod_it->second != VK_NULL_HANDLE) - device_->GetPtrs()->vkDestroyShaderModule(vk_device, mod_it->second, - nullptr); + if (vk_device != VK_NULL_HANDLE && mod_it->second.shader != VK_NULL_HANDLE) + device_->GetPtrs()->vkDestroyShaderModule( + vk_device, mod_it->second.shader, nullptr); } } } @@ -241,8 +241,8 @@ Result EngineVulkan::SetShader(amber::Pipeline* pipeline, const std::vector& data) { auto& info = pipeline_map_[pipeline]; - auto it = info.shaders.find(type); - if (it != info.shaders.end()) + auto it = info.shader_info.find(type); + if (it != info.shader_info.end()) return Result("Vulkan::Setting Duplicated Shader Types Fail"); VkShaderModuleCreateInfo create_info = VkShaderModuleCreateInfo(); @@ -257,28 +257,31 @@ Result EngineVulkan::SetShader(amber::Pipeline* pipeline, return Result("Vulkan::Calling vkCreateShaderModule Fail"); } - info.shaders[type] = shader; + info.shader_info[type].shader = shader; for (auto& shader_info : pipeline->GetShaders()) { if (shader_info.GetShaderType() == type) { const auto& shader_spec_info = shader_info.GetSpecialization(); if (!shader_spec_info.empty()) { - auto& entries = info.specialization_entries[type]; - auto& entry_data = info.specialization_data[type]; + auto& entries = info.shader_info[type].specialization_entries; + entries.reset(new std::vector()); + auto& entry_data = info.shader_info[type].specialization_data; + entry_data.reset(new std::vector()); uint32_t i = 0; for (auto pair : shader_spec_info) { - entries.push_back({pair.first, - static_cast(i * sizeof(uint32_t)), - static_cast(sizeof(uint32_t))}); - entry_data.push_back(pair.second); + entries->push_back({pair.first, + static_cast(i * sizeof(uint32_t)), + static_cast(sizeof(uint32_t))}); + entry_data->push_back(pair.second); ++i; } - auto& spec_info = info.specialization_info[type]; - spec_info.mapEntryCount = + auto& spec_info = info.shader_info[type].specialization_info; + spec_info.reset(new VkSpecializationInfo()); + spec_info->mapEntryCount = static_cast(shader_spec_info.size()); - spec_info.pMapEntries = entries.data(); - spec_info.dataSize = sizeof(uint32_t) * shader_spec_info.size(); - spec_info.pData = entry_data.data(); + spec_info->pMapEntries = entries->data(); + spec_info->dataSize = sizeof(uint32_t) * shader_spec_info.size(); + spec_info->pData = entry_data->data(); } } } @@ -291,9 +294,9 @@ Result EngineVulkan::GetVkShaderStageInfo( std::vector* out) { auto& info = pipeline_map_[pipeline]; - std::vector stage_info(info.shaders.size()); + std::vector stage_info(info.shader_info.size()); uint32_t stage_count = 0; - for (auto it : info.shaders) { + for (auto& it : info.shader_info) { VkShaderStageFlagBits stage = VK_SHADER_STAGE_FLAG_BITS_MAX_ENUM; Result r = ToVkShaderStage(it.first, &stage); if (!r.IsSuccess()) @@ -303,11 +306,11 @@ Result EngineVulkan::GetVkShaderStageInfo( stage_info[stage_count].sType = VK_STRUCTURE_TYPE_PIPELINE_SHADER_STAGE_CREATE_INFO; stage_info[stage_count].stage = stage; - stage_info[stage_count].module = it.second; + stage_info[stage_count].module = it.second.shader; stage_info[stage_count].pName = nullptr; - auto spec_iter = info.specialization_info.find(it.first); - if (spec_iter != info.specialization_info.end()) { - stage_info[stage_count].pSpecializationInfo = &spec_iter->second; + if (!it.second.specialization_entries->empty()) { + stage_info[stage_count].pSpecializationInfo = + it.second.specialization_info.get(); } ++stage_count; } diff --git a/src/vulkan/engine_vulkan.h b/src/vulkan/engine_vulkan.h index 55f86c26b..184e009c7 100644 --- a/src/vulkan/engine_vulkan.h +++ b/src/vulkan/engine_vulkan.h @@ -64,16 +64,15 @@ class EngineVulkan : public Engine { struct PipelineInfo { std::unique_ptr vk_pipeline; std::unique_ptr vertex_buffer; - std::unordered_map> - shaders; - std::unordered_map, - CastHash> - specialization_entries; - std::unordered_map, CastHash> - specialization_data; - std::unordered_map> - specialization_info; + struct ShaderInfo { + VkShaderModule shader; + std::unique_ptr> + specialization_entries; + std::unique_ptr> specialization_data; + std::unique_ptr specialization_info; + }; + std::unordered_map> + shader_info; }; Result GetVkShaderStageInfo( diff --git a/tests/cases/shader_specialization.amber b/tests/cases/shader_specialization.amber new file mode 100644 index 000000000..b92a0fa4f --- /dev/null +++ b/tests/cases/shader_specialization.amber @@ -0,0 +1,109 @@ +#!amber + +SHADER compute my_compute SPIRV-ASM +OpCapability Shader +OpExtension "SPV_KHR_storage_buffer_storage_class" +OpMemoryModel Logical GLSL450 +OpEntryPoint GLCompute %main "main" %lid_var %gid_var %idx_var %wgid_var +OpDecorate %lid_var BuiltIn LocalInvocationId +OpDecorate %gid_var BuiltIn GlobalInvocationId +OpDecorate %idx_var BuiltIn LocalInvocationIndex +OpDecorate %wgid_var BuiltIn WorkgroupId +OpDecorate %wg_size BuiltIn WorkgroupSize +OpDecorate %x SpecId 1 +OpDecorate %y SpecId 2 +OpDecorate %z SpecId 3 +OpDecorate %struct Block +OpMemberDecorate %struct 0 Offset 0 +OpDecorate %rta ArrayStride 4 +OpDecorate %x_var DescriptorSet 0 +OpDecorate %x_var Binding 0 +OpDecorate %y_var DescriptorSet 0 +OpDecorate %y_var Binding 1 +OpDecorate %z_var DescriptorSet 0 +OpDecorate %z_var Binding 2 +OpDecorate %s_var DescriptorSet 0 +OpDecorate %s_var Binding 3 +%void = OpTypeVoid +%int = OpTypeInt 32 0 +%int_0 = OpConstant %int 0 +%device = OpConstant %int 1 +%relaxed = OpConstant %int 0 +%int3 = OpTypeVector %int 3 +%x = OpSpecConstant %int 1 +%y = OpSpecConstant %int 1 +%z = OpSpecConstant %int 1 +%wg_size = OpSpecConstantComposite %int3 %x %y %z +%rta = OpTypeRuntimeArray %int +%struct = OpTypeStruct %rta +%ptr_input_int3 = OpTypePointer Input %int3 +%ptr_input_int = OpTypePointer Input %int +%lid_var = OpVariable %ptr_input_int3 Input +%gid_var = OpVariable %ptr_input_int3 Input +%idx_var = OpVariable %ptr_input_int Input +%wgid_var = OpVariable %ptr_input_int3 Input +%ptr_ssbo_struct = OpTypePointer StorageBuffer %struct +%ptr_ssbo_int = OpTypePointer StorageBuffer %int +%x_var = OpVariable %ptr_ssbo_struct StorageBuffer +%y_var = OpVariable %ptr_ssbo_struct StorageBuffer +%z_var = OpVariable %ptr_ssbo_struct StorageBuffer +%s_var = OpVariable %ptr_ssbo_struct StorageBuffer +%void_fn = OpTypeFunction %void +%main = OpFunction %void None %void_fn +%entry = OpLabel +%lid = OpLoad %int3 %lid_var +%lid_x = OpCompositeExtract %int %lid 0 +%lid_y = OpCompositeExtract %int %lid 1 +%lid_z = OpCompositeExtract %int %lid 2 +%gid = OpLoad %int3 %gid_var +%gid_x = OpCompositeExtract %int %gid 0 +%gid_y = OpCompositeExtract %int %gid 1 +%gid_z = OpCompositeExtract %int %gid 2 +%wgid = OpLoad %int3 %wgid_var +%wgid_x = OpCompositeExtract %int %wgid 0 +%wgid_y = OpCompositeExtract %int %wgid 1 +%wgid_z = OpCompositeExtract %int %wgid 2 +%local_index = OpLoad %int %idx_var +%wg_x = OpCompositeExtract %int %wg_size 0 +%wg_y = OpCompositeExtract %int %wg_size 1 +%wg_z = OpCompositeExtract %int %wg_size 2 +%x_y = OpIMul %int %wg_x %wg_y +%x_y_z = OpIMul %int %x_y %wg_z +%mul = OpIMul %int %wgid_x %x_y_z +; only support multiple wgs on x +%linear = OpIAdd %int %mul %local_index +%x_gep = OpAccessChain %ptr_ssbo_int %x_var %int_0 %linear +%y_gep = OpAccessChain %ptr_ssbo_int %y_var %int_0 %linear +%z_gep = OpAccessChain %ptr_ssbo_int %z_var %int_0 %linear +%s_gep = OpAccessChain %ptr_ssbo_int %s_var %int_0 %linear +OpStore %x_gep %lid_x +OpStore %y_gep %lid_y +OpStore %z_gep %lid_z +OpStore %s_gep %linear +OpReturn +OpFunctionEnd +END + +BUFFER x_buf DATA_TYPE uint32 SIZE 16 FILL 0 +BUFFER y_buf DATA_TYPE uint32 SIZE 16 FILL 0 +BUFFER z_buf DATA_TYPE uint32 SIZE 16 FILL 0 +BUFFER s_buf DATA_TYPE uint32 SIZE 16 FILL 0 + +PIPELINE compute small + ATTACH my_compute \ + SPECIALIZE 1 AS uint32 2 \ + SPECIALIZE 2 AS uint32 2 \ + SPECIALIZE 3 AS uint32 2 + BIND BUFFER x_buf AS storage DESCRIPTOR_SET 0 BINDING 0 + BIND BUFFER y_buf AS storage DESCRIPTOR_SET 0 BINDING 1 + BIND BUFFER z_buf AS storage DESCRIPTOR_SET 0 BINDING 2 + BIND BUFFER s_buf AS storage DESCRIPTOR_SET 0 BINDING 3 +END + +RUN small 2 1 1 + +EXPECT s_buf IDX 0 EQ 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 +EXPECT x_buf IDX 0 EQ 0 1 0 1 0 1 0 1 0 1 0 1 0 1 0 1 +EXPECT y_buf IDX 0 EQ 0 0 1 1 0 0 1 1 0 0 1 1 0 0 1 1 +EXPECT z_buf IDX 0 EQ 0 0 0 0 1 1 1 1 0 0 0 0 1 1 1 1 + From ab9bcfec81e05b1131a95c565b612f610c5f460a Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Fri, 24 May 2019 13:56:03 -0400 Subject: [PATCH 05/11] update docs --- docs/amber_script.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/amber_script.md b/docs/amber_script.md index 23f2c7748..0f64bf055 100644 --- a/docs/amber_script.md +++ b/docs/amber_script.md @@ -199,8 +199,9 @@ The following commands are all specified within the `PIPELINE` command. ATTACH {name_of_multi_shader} TYPE {shader_type} ENTRY_POINT {name} # Attach specialized shader. Specialization can be specified multiple times. - ATTACH {name_of_shader} ENTRY_POINT {name} SPECIALIZE 1 AS uint32 4 - ATTACH {name_of_shader} ENTRY_POINT {name} SPECIALIZE 1 AS uint32 4 SPECIALIZE 4 AS float 1.0 + # Specialization values must be a 32-bit type. + ATTACH SPECIALIZE 1 AS uint32 4 + ATTACH SPECIALIZE 1 AS uint32 4 SPECIALIZE 4 AS float 1.0 ``` ```groovy From 68563c737d779a24cdba468d063347c80662939c Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Fri, 24 May 2019 14:03:13 -0400 Subject: [PATCH 06/11] formatting --- src/vulkan/engine_vulkan.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index 5e60cc440..235732a43 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -78,10 +78,11 @@ EngineVulkan::~EngineVulkan() { for (auto it = pipeline_map_.begin(); it != pipeline_map_.end(); ++it) { auto& info = it->second; - for (auto mod_it = info.shader_info.begin(); mod_it != info.shader_info.end(); - ++mod_it) { + for (auto mod_it = info.shader_info.begin(); + mod_it != info.shader_info.end(); ++mod_it) { auto vk_device = device_->GetVkDevice(); - if (vk_device != VK_NULL_HANDLE && mod_it->second.shader != VK_NULL_HANDLE) + if (vk_device != VK_NULL_HANDLE && + mod_it->second.shader != VK_NULL_HANDLE) device_->GetPtrs()->vkDestroyShaderModule( vk_device, mod_it->second.shader, nullptr); } @@ -294,7 +295,8 @@ Result EngineVulkan::GetVkShaderStageInfo( std::vector* out) { auto& info = pipeline_map_[pipeline]; - std::vector stage_info(info.shader_info.size()); + std::vector stage_info( + info.shader_info.size()); uint32_t stage_count = 0; for (auto& it : info.shader_info) { VkShaderStageFlagBits stage = VK_SHADER_STAGE_FLAG_BITS_MAX_ENUM; From 72d29e9031f727aa4425e5925d489b3ce623a3ab Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Fri, 24 May 2019 14:10:11 -0400 Subject: [PATCH 07/11] formatting --- src/amberscript/parser_attach_test.cc | 1 - src/vulkan/engine_vulkan.h | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/amberscript/parser_attach_test.cc b/src/amberscript/parser_attach_test.cc index 50a4d83e8..1549d5e94 100644 --- a/src/amberscript/parser_attach_test.cc +++ b/src/amberscript/parser_attach_test.cc @@ -515,6 +515,5 @@ END)"; EXPECT_EQ(4, shaders[0].GetSpecialization().at(1)); } - } // namespace amberscript } // namespace amber diff --git a/src/vulkan/engine_vulkan.h b/src/vulkan/engine_vulkan.h index 184e009c7..7dab17683 100644 --- a/src/vulkan/engine_vulkan.h +++ b/src/vulkan/engine_vulkan.h @@ -72,7 +72,7 @@ class EngineVulkan : public Engine { std::unique_ptr specialization_info; }; std::unordered_map> - shader_info; + shader_info; }; Result GetVkShaderStageInfo( From 6f0cb86614a7a568923e83105ffc61b28802c499 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Mon, 27 May 2019 11:10:07 -0400 Subject: [PATCH 08/11] Changes for review * Fix docs * Add pipeline derivation test for specialization * Fix spec value parsing * Style fixes --- docs/amber_script.md | 7 ++-- src/amberscript/parser.cc | 13 ++----- src/amberscript/parser_pipeline_test.cc | 36 ++++++++++++++++++ src/vulkan/engine_vulkan.cc | 50 +++++++++++++------------ tests/cases/shader_specialization.amber | 13 +++++++ 5 files changed, 83 insertions(+), 36 deletions(-) diff --git a/docs/amber_script.md b/docs/amber_script.md index 0f64bf055..d9109b219 100644 --- a/docs/amber_script.md +++ b/docs/amber_script.md @@ -199,9 +199,10 @@ The following commands are all specified within the `PIPELINE` command. ATTACH {name_of_multi_shader} TYPE {shader_type} ENTRY_POINT {name} # Attach specialized shader. Specialization can be specified multiple times. - # Specialization values must be a 32-bit type. - ATTACH SPECIALIZE 1 AS uint32 4 - ATTACH SPECIALIZE 1 AS uint32 4 SPECIALIZE 4 AS float 1.0 + # Specialization values must be a 32-bit type. Shader type and entry point + # must be specified prior to specializing the shader. + ATTACH {name_of_shader} SPECIALIZE 1 AS uint32 4 + ATTACH {name_of_shader} SPECIALIZE 1 AS uint32 4 SPECIALIZE 4 AS float 1.0 ``` ```groovy diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc index a1ee47dad..408b05acf 100644 --- a/src/amberscript/parser.cc +++ b/src/amberscript/parser.cc @@ -535,18 +535,13 @@ Result Parser::ParseShaderSpecialization(Pipeline* pipeline) { uint32_t value = 0; switch (type.GetType()) { case DataType::kUint32: + case DataType::kInt32: value = token->AsUint32(); break; - case DataType::kInt32: { - union { - uint32_t u; - int32_t i; - } u; - u.i = token->AsInt32(); - value = u.u; - break; - } case DataType::kFloat: { + r = token->ConvertToDouble(); + if (!r.IsSuccess()) + return Result("value is not a floating point value"); union { uint32_t u; float f; diff --git a/src/amberscript/parser_pipeline_test.cc b/src/amberscript/parser_pipeline_test.cc index 386066069..ebd2ddd68 100644 --- a/src/amberscript/parser_pipeline_test.cc +++ b/src/amberscript/parser_pipeline_test.cc @@ -414,5 +414,41 @@ END EXPECT_EQ("3: missing pipeline name for DERIVE_PIPELINE command", r.Error()); } +TEST_F(AmberScriptParserTest, DerivePipelineSpecialized) { + std::string in = R"( +SHADER compute my_shader GLSL +#shaders +END +PIPELINE compute p1 + ATTACH my_shader SPECIALIZE 3 AS uint32 4 +END +DERIVE_PIPELINE p2 FROM p1 +END +)"; + + Parser parser; + Result r = parser.Parse(in); + EXPECT_EQ("", r.Error()); + ASSERT_TRUE(r.IsSuccess()); + + auto script = parser.GetScript(); + const auto& pipelines = script->GetPipelines(); + ASSERT_EQ(2U, pipelines.size()); + + const auto* p1 = pipelines[0].get(); + const auto& s1 = p1->GetShaders(); + ASSERT_EQ(1U, s1.size()); + + EXPECT_EQ(1, s1[0].GetSpecialization().size()); + EXPECT_EQ(4, s1[0].GetSpecialization().at(3)); + + const auto* p2 = pipelines[1].get(); + const auto& s2 = p2->GetShaders(); + ASSERT_EQ(1U, s2.size()); + + EXPECT_EQ(1, s2[0].GetSpecialization().size()); + EXPECT_EQ(4, s2[0].GetSpecialization().at(3)); +} + } // namespace amberscript } // namespace amber diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index 235732a43..95c732807 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -82,9 +82,10 @@ EngineVulkan::~EngineVulkan() { mod_it != info.shader_info.end(); ++mod_it) { auto vk_device = device_->GetVkDevice(); if (vk_device != VK_NULL_HANDLE && - mod_it->second.shader != VK_NULL_HANDLE) + mod_it->second.shader != VK_NULL_HANDLE) { device_->GetPtrs()->vkDestroyShaderModule( vk_device, mod_it->second.shader, nullptr); + } } } } @@ -261,30 +262,31 @@ Result EngineVulkan::SetShader(amber::Pipeline* pipeline, info.shader_info[type].shader = shader; for (auto& shader_info : pipeline->GetShaders()) { - if (shader_info.GetShaderType() == type) { - const auto& shader_spec_info = shader_info.GetSpecialization(); - if (!shader_spec_info.empty()) { - auto& entries = info.shader_info[type].specialization_entries; - entries.reset(new std::vector()); - auto& entry_data = info.shader_info[type].specialization_data; - entry_data.reset(new std::vector()); - uint32_t i = 0; - for (auto pair : shader_spec_info) { - entries->push_back({pair.first, - static_cast(i * sizeof(uint32_t)), - static_cast(sizeof(uint32_t))}); - entry_data->push_back(pair.second); - ++i; - } - auto& spec_info = info.shader_info[type].specialization_info; - spec_info.reset(new VkSpecializationInfo()); - spec_info->mapEntryCount = - static_cast(shader_spec_info.size()); - spec_info->pMapEntries = entries->data(); - spec_info->dataSize = sizeof(uint32_t) * shader_spec_info.size(); - spec_info->pData = entry_data->data(); - } + if (shader_info.GetShaderType() != type) + continue; + + const auto& shader_spec_info = shader_info.GetSpecialization(); + if (shader_spec_info.empty()) + continue; + + auto& entries = info.shader_info[type].specialization_entries; + entries.reset(new std::vector()); + auto& entry_data = info.shader_info[type].specialization_data; + entry_data.reset(new std::vector()); + uint32_t i = 0; + for (auto pair : shader_spec_info) { + entries->push_back({pair.first, + static_cast(i * sizeof(uint32_t)), + static_cast(sizeof(uint32_t))}); + entry_data->push_back(pair.second); + ++i; } + auto& spec_info = info.shader_info[type].specialization_info; + spec_info.reset(new VkSpecializationInfo()); + spec_info->mapEntryCount = static_cast(shader_spec_info.size()); + spec_info->pMapEntries = entries->data(); + spec_info->dataSize = sizeof(uint32_t) * shader_spec_info.size(); + spec_info->pData = entry_data->data(); } return {}; diff --git a/tests/cases/shader_specialization.amber b/tests/cases/shader_specialization.amber index b92a0fa4f..1db2a4f5e 100644 --- a/tests/cases/shader_specialization.amber +++ b/tests/cases/shader_specialization.amber @@ -1,4 +1,17 @@ #!amber +# Copyright 2019 The Amber Authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. SHADER compute my_compute SPIRV-ASM OpCapability Shader From ee2bc2f83c62478cb63a41e8acf23338407eaf56 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Mon, 27 May 2019 11:12:36 -0400 Subject: [PATCH 09/11] Remove 0 spec id restriction --- src/amberscript/parser.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc index 408b05acf..dc217cc0b 100644 --- a/src/amberscript/parser.cc +++ b/src/amberscript/parser.cc @@ -515,8 +515,6 @@ Result Parser::ParseShaderSpecialization(Pipeline* pipeline) { return Result("specialization ID must be an integer"); auto spec_id = token->AsUint32(); - if (spec_id == 0) - return Result("specialization ID must be greater than 0"); token = tokenizer_->NextToken(); if (!token->IsString() || token->AsString() != "AS") From 424b58291bc96bef9cef7f0893a704f0bae324a3 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Mon, 27 May 2019 11:13:23 -0400 Subject: [PATCH 10/11] remove bad test --- src/amberscript/parser_attach_test.cc | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/amberscript/parser_attach_test.cc b/src/amberscript/parser_attach_test.cc index 1549d5e94..166be223a 100644 --- a/src/amberscript/parser_attach_test.cc +++ b/src/amberscript/parser_attach_test.cc @@ -398,21 +398,6 @@ END)"; EXPECT_EQ("6: specialization ID must be an integer", r.Error()); } -TEST_F(AmberScriptParserTest, PipelineSpecializationIDIsZero) { - std::string in = R"( -SHADER compute my_shader GLSL -#shaders -END -PIPELINE compute my_pipeline - ATTACH my_shader TYPE compute ENTRY_POINT my_ep SPECIALIZE 0 AS float 1.1 -END)"; - - Parser parser; - Result r = parser.Parse(in); - ASSERT_FALSE(r.IsSuccess()); - EXPECT_EQ("6: specialization ID must be greater than 0", r.Error()); -} - TEST_F(AmberScriptParserTest, PipelineSpecializationNoAS) { std::string in = R"( SHADER compute my_shader GLSL From ad897a10da44aa8298362227670e9d8593e49ef7 Mon Sep 17 00:00:00 2001 From: Alan Baker Date: Mon, 27 May 2019 11:32:06 -0400 Subject: [PATCH 11/11] Fix access to specialization info --- src/vulkan/engine_vulkan.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc index 95c732807..8c911ba67 100644 --- a/src/vulkan/engine_vulkan.cc +++ b/src/vulkan/engine_vulkan.cc @@ -312,7 +312,8 @@ Result EngineVulkan::GetVkShaderStageInfo( stage_info[stage_count].stage = stage; stage_info[stage_count].module = it.second.shader; stage_info[stage_count].pName = nullptr; - if (!it.second.specialization_entries->empty()) { + if (it.second.specialization_entries && + !it.second.specialization_entries->empty()) { stage_info[stage_count].pSpecializationInfo = it.second.specialization_info.get(); }