writer/hlsl: Emit binding group as spaceN

When combined with the new transform::BindingRemapper, Dawn can now correctly emit resource bindings.

Change-Id: I3e7fd7b2fc5368c5da112ec8835f5c70d45ce6c8
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46263
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/writer/hlsl/generator_impl.cc

@@ -79,6 +79,22 @@ const char* image_format_to_rwtexture_type(type::ImageFormat image_format) {
   }
 }
 
+// Helper for writing " : register(RX, spaceY)", where R is the register, X is
+// the binding point binding value, and Y is the binding point group value.
+struct RegisterAndSpace {
+  RegisterAndSpace(char r, ast::Variable::BindingPoint bp)
+      : reg(r), binding_point(bp) {}
+
+  char const reg;
+  ast::Variable::BindingPoint const binding_point;
+};
+
+std::ostream& operator<<(std::ostream& s, const RegisterAndSpace& rs) {
+  s << " : register(" << rs.reg << rs.binding_point.binding->value()
+    << ", space" << rs.binding_point.group->value() << ")";
+  return s;
+}
+
 }  // namespace
 
 GeneratorImpl::GeneratorImpl(const Program* program)
@@ -1579,29 +1595,18 @@ bool GeneratorImpl::EmitEntryPointData(
   bool emitted_uniform = false;
   for (auto data : func_sem->ReferencedUniformVariables()) {
     auto* var = data.first;
+    auto& binding_point = data.second;
     auto* decl = var->Declaration();
 
     if (!emitted_globals.emplace(decl->symbol()).second) {
       continue;  // Global already emitted
     }
 
-    // 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) {
-      diagnostics_.add_error(
-          "unable to find binding information for uniform: " +
-          builder_.Symbols().NameFor(decl->symbol()));
-      return false;
-    }
-    // auto* set = data.second.set;
-
     auto* type = var->Type()->UnwrapIfNeeded();
     if (auto* strct = type->As<type::Struct>()) {
       out << "ConstantBuffer<" << builder_.Symbols().NameFor(strct->symbol())
           << "> " << builder_.Symbols().NameFor(decl->symbol())
-          << " : register(b" << binding->value() << ");" << std::endl;
+          << RegisterAndSpace('b', binding_point) << ";" << std::endl;
     } else {
       // TODO(dsinclair): There is outstanding spec work to require all uniform
       // buffers to be [[block]] decorated, which means structs. This is
@@ -1610,7 +1615,7 @@ bool GeneratorImpl::EmitEntryPointData(
       // Relevant: https://github.com/gpuweb/gpuweb/issues/1004
       //           https://github.com/gpuweb/gpuweb/issues/1008
       auto name = "cbuffer_" + builder_.Symbols().NameFor(decl->symbol());
-      out << "cbuffer " << name << " : register(b" << binding->value() << ") {"
+      out << "cbuffer " << name << RegisterAndSpace('b', binding_point) << " {"
           << std::endl;
 
       increment_indent();
@@ -1633,13 +1638,13 @@ bool GeneratorImpl::EmitEntryPointData(
   bool emitted_storagebuffer = false;
   for (auto data : func_sem->ReferencedStorageBufferVariables()) {
     auto* var = data.first;
+    auto& binding_point = data.second;
     auto* decl = var->Declaration();
 
     if (!emitted_globals.emplace(decl->symbol()).second) {
       continue;  // Global already emitted
     }
 
-    auto* binding = data.second.binding;
     auto* ac = var->Type()->As<type::AccessControl>();
     if (ac == nullptr) {
       diagnostics_.add_error("access control type required for storage buffer");
@@ -1650,8 +1655,8 @@ bool GeneratorImpl::EmitEntryPointData(
       out << "RW";
     }
     out << "ByteAddressBuffer " << builder_.Symbols().NameFor(decl->symbol())
-        << " : register(" << (ac->IsReadOnly() ? "t" : "u") << binding->value()
+        << RegisterAndSpace(ac->IsReadOnly() ? 't' : 'u', binding_point) << ";"
-        << ");" << std::endl;
+        << std::endl;
     emitted_storagebuffer = true;
   }
   if (emitted_storagebuffer) {

src/writer/hlsl/generator_impl_function_test.cc

@@ -260,7 +260,7 @@ TEST_F(HlslGeneratorImplTest_Function,
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_EQ(result(), R"(cbuffer cbuffer_coord : register(b0) {
+  EXPECT_EQ(result(), R"(cbuffer cbuffer_coord : register(b0, space1) {
   float4 coord;
 };
 
@@ -302,7 +302,7 @@ TEST_F(HlslGeneratorImplTest_Function,
   float4 coord;
 };
 
-ConstantBuffer<Uniforms> uniforms : register(b0);
+ConstantBuffer<Uniforms> uniforms : register(b0, space1);
 
 void frag_main() {
   float v = uniforms.coord.x;
@@ -342,7 +342,8 @@ TEST_F(HlslGeneratorImplTest_Function,
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_THAT(result(), HasSubstr(R"(RWByteAddressBuffer coord : register(u0);
+  EXPECT_THAT(result(),
+              HasSubstr(R"(RWByteAddressBuffer coord : register(u0, space1);
 
 void frag_main() {
   float v = asfloat(coord.Load(4));
@@ -380,7 +381,8 @@ TEST_F(HlslGeneratorImplTest_Function,
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_THAT(result(), HasSubstr(R"(ByteAddressBuffer coord : register(t0);
+  EXPECT_THAT(result(),
+              HasSubstr(R"(ByteAddressBuffer coord : register(t0, space1);
 
 void frag_main() {
   float v = asfloat(coord.Load(4));
@@ -416,7 +418,8 @@ TEST_F(HlslGeneratorImplTest_Function,
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_THAT(result(), HasSubstr(R"(RWByteAddressBuffer coord : register(u0);
+  EXPECT_THAT(result(),
+              HasSubstr(R"(RWByteAddressBuffer coord : register(u0, space1);
 
 void frag_main() {
   coord.Store(4, asuint(2.0f));
@@ -452,7 +455,8 @@ TEST_F(HlslGeneratorImplTest_Function,
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_THAT(result(), HasSubstr(R"(RWByteAddressBuffer coord : register(u0);
+  EXPECT_THAT(result(),
+              HasSubstr(R"(RWByteAddressBuffer coord : register(u0, space1);
 
 void frag_main() {
   coord.Store(4, asuint(2.0f));
@@ -666,7 +670,7 @@ TEST_F(HlslGeneratorImplTest_Function,
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_EQ(result(), R"(cbuffer cbuffer_coord : register(b0) {
+  EXPECT_EQ(result(), R"(cbuffer cbuffer_coord : register(b0, space1) {
   float4 coord;
 };
 
@@ -714,7 +718,8 @@ TEST_F(HlslGeneratorImplTest_Function,
   GeneratorImpl& gen = Build();
 
   ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_THAT(result(), HasSubstr(R"(RWByteAddressBuffer coord : register(u0);
+  EXPECT_THAT(result(),
+              HasSubstr(R"(RWByteAddressBuffer coord : register(u0, space1);
 
 float sub_func(float param) {
   return asfloat(coord.Load((4 * 0)));
@@ -915,7 +920,7 @@ TEST_F(HlslGeneratorImplTest_Function,
   float d;
 };
 
-RWByteAddressBuffer data : register(u0);
+RWByteAddressBuffer data : register(u0, space0);
 
 [numthreads(1, 1, 1)]
 void a() {