From bfaa6f8e573e3cddb678d092ad57a46b5308da8e Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Wed, 19 Aug 2020 18:57:09 +0000 Subject: [PATCH] [hlsl-writer] Emit uniform variables. This CL adds emission of uniform storage class variables to the HLSL backend. If the variable is a base type (float, int, etc) it is emitted as a `cbuffer`. If the variable is a struct it will emit as a `ConstantBuffer`. Bug: tint:7 Change-Id: I9932d30df24c023c58d3a2ba4167bcb7ccb85dae Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/26920 Commit-Queue: dan sinclair Reviewed-by: David Neto --- src/writer/hlsl/generator_impl.cc | 44 +++++++++ .../hlsl/generator_impl_function_test.cc | 91 ++++++++++++++++++- src/writer/msl/generator_impl.cc | 4 + 3 files changed, 137 insertions(+), 2 deletions(-) diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index 750c52d5a3..a5a50e9e42 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -787,6 +787,50 @@ bool GeneratorImpl::EmitEntryPointData(ast::EntryPoint* ep) { } } + bool emitted_uniform = false; + for (auto data : func->referenced_uniform_variables()) { + auto* var = data.first; + // TODO(dsinclair): We're using the binding to make up the buffer number but + // we should instead be using a provided mapping that uses both buffer and + // set. https://bugs.chromium.org/p/tint/issues/detail?id=104 + auto* binding = data.second.binding; + if (binding == nullptr) { + error_ = "unable to find binding information for uniform: " + var->name(); + return false; + } + // auto* set = data.second.set; + + auto* type = var->type()->UnwrapAliasesIfNeeded(); + if (type->IsStruct()) { + auto* strct = type->AsStruct(); + + out_ << "ConstantBuffer<" << strct->name() << "> " << var->name() + << " : register(b" << binding->value() << ");" << std::endl; + } else { + // TODO(dsinclair): There is outstanding spec work to require all uniform + // buffers to be [[block]] decorated, which means structs. This is + // currently not the case, so this code handles the cases where the data + // is not a block. + // Relevant: https://github.com/gpuweb/gpuweb/issues/1004 + // https://github.com/gpuweb/gpuweb/issues/1008 + out_ << "cbuffer : register(b" << binding->value() << ") {" << std::endl; + + increment_indent(); + make_indent(); + if (!EmitType(type, "")) { + return false; + } + out_ << " " << var->name() << ";" << std::endl; + decrement_indent(); + out_ << "};" << std::endl; + } + + emitted_uniform = true; + } + if (emitted_uniform) { + out_ << std::endl; + } + auto ep_name = ep->name(); if (ep_name.empty()) { ep_name = ep->function_name(); diff --git a/src/writer/hlsl/generator_impl_function_test.cc b/src/writer/hlsl/generator_impl_function_test.cc index deeede917d..b5c23a7995 100644 --- a/src/writer/hlsl/generator_impl_function_test.cc +++ b/src/writer/hlsl/generator_impl_function_test.cc @@ -29,9 +29,12 @@ #include "src/ast/scalar_constructor_expression.h" #include "src/ast/set_decoration.h" #include "src/ast/sint_literal.h" +#include "src/ast/struct.h" +#include "src/ast/type/alias_type.h" #include "src/ast/type/array_type.h" #include "src/ast/type/f32_type.h" #include "src/ast/type/i32_type.h" +#include "src/ast/type/struct_type.h" #include "src/ast/type/vector_type.h" #include "src/ast/type/void_type.h" #include "src/ast/variable.h" @@ -280,7 +283,7 @@ frag_main_out frag_main(frag_main_in tint_in) { )"); } -TEST_F(HlslGeneratorImplTest, DISABLED_Emit_Function_EntryPoint_With_Uniform) { +TEST_F(HlslGeneratorImplTest, Emit_Function_EntryPoint_With_Uniform) { ast::type::VoidType void_type; ast::type::F32Type f32; ast::type::VectorType vec4(&f32, 4); @@ -326,7 +329,91 @@ TEST_F(HlslGeneratorImplTest, DISABLED_Emit_Function_EntryPoint_With_Uniform) { GeneratorImpl g(&mod); ASSERT_TRUE(g.Generate()) << g.error(); - EXPECT_EQ(g.result(), R"( ... )"); + EXPECT_EQ(g.result(), R"(cbuffer : register(b0) { + vector coord; +}; + +void frag_main() { + float v = coord.x; + return; +} + +)"); +} + +TEST_F(HlslGeneratorImplTest, Emit_Function_EntryPoint_With_UniformStruct) { + ast::type::VoidType void_type; + ast::type::F32Type f32; + ast::type::VectorType vec4(&f32, 4); + + ast::StructMemberList members; + members.push_back(std::make_unique( + "coord", &vec4, ast::StructMemberDecorationList{})); + + auto str = std::make_unique(); + str->set_members(std::move(members)); + + ast::type::StructType s(std::move(str)); + s.set_name("Uniforms"); + auto alias = std::make_unique("Uniforms", &s); + + Context ctx; + ast::Module mod; + TypeDeterminer td(&ctx, &mod); + + auto coord_var = + std::make_unique(std::make_unique( + "uniforms", ast::StorageClass::kUniform, alias.get())); + + mod.AddAliasType(alias.get()); + + ast::VariableDecorationList decos; + decos.push_back(std::make_unique(0)); + decos.push_back(std::make_unique(1)); + coord_var->set_decorations(std::move(decos)); + + td.RegisterVariableForTesting(coord_var.get()); + mod.AddGlobalVariable(std::move(coord_var)); + + ast::VariableList params; + auto func = std::make_unique("frag_main", std::move(params), + &void_type); + + auto var = + std::make_unique("v", ast::StorageClass::kFunction, &f32); + var->set_constructor(std::make_unique( + std::make_unique( + std::make_unique("uniforms"), + std::make_unique("coord")), + std::make_unique("x"))); + + auto body = std::make_unique(); + body->append(std::make_unique(std::move(var))); + body->append(std::make_unique()); + func->set_body(std::move(body)); + + mod.AddFunction(std::move(func)); + + auto ep = std::make_unique(ast::PipelineStage::kFragment, "", + "frag_main"); + mod.AddEntryPoint(std::move(ep)); + + ASSERT_TRUE(td.Determine()) << td.error(); + + GeneratorImpl g(&mod); + ASSERT_TRUE(g.Generate()) << g.error(); + EXPECT_EQ(g.result(), R"(typedef struct { + vector coord; +} Uniforms; + +ConstantBuffer uniforms : register(b0); + +void frag_main() { + float v = uniforms.coord.x; + return; +} + +)"); } TEST_F(HlslGeneratorImplTest, diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index 11f29f35ef..4a7f48e262 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -1347,6 +1347,10 @@ bool GeneratorImpl::EmitEntryPointFunction(ast::EntryPoint* ep) { // we should instead be using a provided mapping that uses both buffer and // set. https://bugs.chromium.org/p/tint/issues/detail?id=104 auto* binding = data.second.binding; + if (binding == nullptr) { + error_ = "unable to find binding information for uniform: " + var->name(); + return false; + } // auto* set = data.second.set; out_ << "constant ";