writer/hlsl: Fixes for textures

Don't use RWTextureXD types for read-only textures.

Emit registers for texture variables.

Change-Id: I904c38df74bd57baf462a36dad72a5d37622d3e9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46264
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>

src/writer/hlsl/generator_impl.cc

@@ -28,7 +28,9 @@
 #include "src/semantic/struct.h"
 #include "src/semantic/variable.h"
 #include "src/type/access_control_type.h"
+#include "src/type/depth_texture_type.h"
 #include "src/type/multisampled_texture_type.h"
+#include "src/type/sampled_texture_type.h"
 #include "src/type/storage_texture_type.h"
 #include "src/writer/append_vector.h"
 #include "src/writer/float_to_string.h"
@@ -978,9 +980,7 @@ bool GeneratorImpl::EmitTextureCall(std::ostream& pre,
       break;
     case semantic::IntrinsicType::kTextureLoad:
       out << ".Load(";
-      if (!texture_type->Is<type::StorageTexture>()) {
       pack_mip_in_coords = true;
-      }
       break;
     case semantic::IntrinsicType::kTextureStore:
       out << "[";
@@ -1781,8 +1781,7 @@ bool GeneratorImpl::EmitEntryPointData(
       auto* decl = var->Declaration();
 
       auto* unwrapped_type = var->Type()->UnwrapAll();
-      if (!unwrapped_type->Is<type::Texture>() &&
+      if (!unwrapped_type->IsAnyOf<type::Texture, type::Sampler>()) {
-          !unwrapped_type->Is<type::Sampler>()) {
         continue;  // Not interested in this type
       }
 
@@ -1793,8 +1792,32 @@ bool GeneratorImpl::EmitEntryPointData(
       if (!EmitType(out, var->Type(), "")) {
         return false;
       }
-      out << " " << namer_.NameFor(builder_.Symbols().NameFor(decl->symbol()))
+      out << " " << namer_.NameFor(builder_.Symbols().NameFor(decl->symbol()));
-          << ";" << std::endl;
+
+      const char* register_space = nullptr;
+
+      if (unwrapped_type->Is<type::Texture>()) {
+        register_space = "t";
+        if (unwrapped_type->Is<type::StorageTexture>()) {
+          if (auto* ac = var->Type()
+                             ->UnwrapAliasIfNeeded()
+                             ->As<type::AccessControl>()) {
+            if (!ac->IsReadOnly()) {
+              register_space = "u";
+            }
+          }
+        }
+      } else if (unwrapped_type->Is<type::Sampler>()) {
+        register_space = "s";
+      }
+
+      if (register_space) {
+        auto bp = decl->binding_point();
+        out << " : register(" << register_space << bp.binding->value()
+            << ", space" << bp.group->value() << ")";
+      }
+
+      out << ";" << std::endl;
 
       add_newline = true;
     }
@@ -2448,10 +2471,9 @@ bool GeneratorImpl::EmitSwitch(std::ostream& out, ast::SwitchStatement* stmt) {
 bool GeneratorImpl::EmitType(std::ostream& out,
                              type::Type* type,
                              const std::string& name) {
-  // HLSL doesn't have the read/write only markings so just unwrap the access
+  auto* access = type->As<type::AccessControl>();
-  // control type.
+  if (access) {
-  if (auto* ac = type->As<type::AccessControl>()) {
+    type = access->type();
-    return EmitType(out, ac->type(), name);
   }
 
   if (auto* alias = type->As<type::Alias>()) {
@@ -2505,8 +2527,10 @@ bool GeneratorImpl::EmitType(std::ostream& out,
     out << builder_.Symbols().NameFor(str->symbol());
   } else if (auto* tex = type->As<type::Texture>()) {
     if (tex->Is<type::StorageTexture>()) {
+      if (access && !access->IsReadOnly()) {
         out << "RW";
       }
+    }
     out << "Texture";
 
     auto* ms = tex->As<type::MultisampledTexture>();

src/writer/hlsl/generator_impl_intrinsic_texture_test.cc

@@ -338,7 +338,7 @@ texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w)
     case ValidTextureOverload::kLoadDepth2dArrayLevelF32:
       return R"(texture_tint_0.Load(int4(1, 2, 3, 0), 4))";
     case ValidTextureOverload::kLoadStorageRO1dRgba32float:
-      return R"(texture_tint_0.Load(1))";
+      return R"(texture_tint_0.Load(int2(1, 0)))";
     case ValidTextureOverload::kLoadStorageRO2dRgba8unorm:
     case ValidTextureOverload::kLoadStorageRO2dRgba8snorm:
     case ValidTextureOverload::kLoadStorageRO2dRgba8uint:
@@ -355,11 +355,11 @@ texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w)
     case ValidTextureOverload::kLoadStorageRO2dRgba32uint:
     case ValidTextureOverload::kLoadStorageRO2dRgba32sint:
     case ValidTextureOverload::kLoadStorageRO2dRgba32float:
-      return R"(texture_tint_0.Load(int2(1, 2)))";
+      return R"(texture_tint_0.Load(int3(1, 2, 0)))";
     case ValidTextureOverload::kLoadStorageRO2dArrayRgba32float:
-      return R"(texture_tint_0.Load(int3(1, 2, 3)))";
+      return R"(texture_tint_0.Load(int4(1, 2, 3, 0)))";
     case ValidTextureOverload::kLoadStorageRO3dRgba32float:
-      return R"(texture_tint_0.Load(int3(1, 2, 3)))";
+      return R"(texture_tint_0.Load(int4(1, 2, 3, 0)))";
     case ValidTextureOverload::kStoreWO1dRgba32float:
       return R"(texture_tint_0[1] = float4(2.0f, 3.0f, 4.0f, 5.0f))";
     case ValidTextureOverload::kStoreWO2dRgba32float:

src/writer/hlsl/generator_impl_type_test.cc

@@ -12,6 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include "gmock/gmock.h"
+#include "src/ast/call_statement.h"
+#include "src/ast/stage_decoration.h"
 #include "src/ast/struct_block_decoration.h"
 #include "src/type/access_control_type.h"
 #include "src/type/depth_texture_type.h"
@@ -25,6 +28,8 @@ namespace writer {
 namespace hlsl {
 namespace {
 
+using ::testing::HasSubstr;
+
 using HlslGeneratorImplTest_Type = TestHelper;
 
 TEST_F(HlslGeneratorImplTest_Type, EmitType_Alias) {
@@ -323,58 +328,91 @@ inline std::ostream& operator<<(std::ostream& out, HlslDepthTextureData data) {
   out << data.dim;
   return out;
 }
-using HlslDepthtexturesTest = TestParamHelper<HlslDepthTextureData>;
+using HlslDepthTexturesTest = TestParamHelper<HlslDepthTextureData>;
-TEST_P(HlslDepthtexturesTest, Emit) {
+TEST_P(HlslDepthTexturesTest, Emit) {
   auto params = GetParam();
 
-  type::DepthTexture s(params.dim);
+  auto* t = create<type::DepthTexture>(params.dim);
+
+  Global("tex", t, ast::StorageClass::kUniformConstant, nullptr,
+         ast::DecorationList{
+             create<ast::BindingDecoration>(1),
+             create<ast::GroupDecoration>(2),
+         });
+
+  Func("main", {}, ty.void_(),
+       {create<ast::CallStatement>(Call("textureDimensions", "tex"))},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitType(out, &s, "")) << gen.error();
+  ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_EQ(result(), params.result);
+  EXPECT_THAT(result(), HasSubstr(params.result));
+
+  Validate();
 }
 INSTANTIATE_TEST_SUITE_P(
     HlslGeneratorImplTest_Type,
-    HlslDepthtexturesTest,
+    HlslDepthTexturesTest,
     testing::Values(
-        HlslDepthTextureData{type::TextureDimension::k2d, "Texture2D"},
+        HlslDepthTextureData{type::TextureDimension::k2d,
+                             "Texture2D tex : register(t1, space2);"},
         HlslDepthTextureData{type::TextureDimension::k2dArray,
-                             "Texture2DArray"},
+                             "Texture2DArray tex : register(t1, space2);"},
-        HlslDepthTextureData{type::TextureDimension::kCube, "TextureCube"},
+        HlslDepthTextureData{type::TextureDimension::kCube,
+                             "TextureCube tex : register(t1, space2);"},
         HlslDepthTextureData{type::TextureDimension::kCubeArray,
-                             "TextureCubeArray"}));
+                             "TextureCubeArray tex : register(t1, space2);"}));
 
-struct HlslTextureData {
+struct HlslSampledTextureData {
   type::TextureDimension dim;
   std::string result;
 };
-inline std::ostream& operator<<(std::ostream& out, HlslTextureData data) {
+inline std::ostream& operator<<(std::ostream& out,
+                                HlslSampledTextureData data) {
   out << data.dim;
   return out;
 }
-using HlslSampledtexturesTest = TestParamHelper<HlslTextureData>;
+using HlslSampledTexturesTest = TestParamHelper<HlslSampledTextureData>;
-TEST_P(HlslSampledtexturesTest, Emit) {
+TEST_P(HlslSampledTexturesTest, Emit) {
   auto params = GetParam();
 
-  type::SampledTexture s(params.dim, ty.f32());
+  auto* t = create<type::SampledTexture>(params.dim, ty.f32());
+
+  Global("tex", t, ast::StorageClass::kUniformConstant, nullptr,
+         ast::DecorationList{
+             create<ast::BindingDecoration>(1),
+             create<ast::GroupDecoration>(2),
+         });
+
+  Func("main", {}, ty.void_(),
+       {create<ast::CallStatement>(Call("textureDimensions", "tex"))},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitType(out, &s, "")) << gen.error();
+  ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_EQ(result(), params.result);
+  EXPECT_THAT(result(), HasSubstr(params.result));
+
+  Validate();
 }
 INSTANTIATE_TEST_SUITE_P(
     HlslGeneratorImplTest_Type,
-    HlslSampledtexturesTest,
+    HlslSampledTexturesTest,
     testing::Values(
-        HlslTextureData{type::TextureDimension::k1d, "Texture1D"},
+        HlslSampledTextureData{type::TextureDimension::k1d,
-        HlslTextureData{type::TextureDimension::k2d, "Texture2D"},
+                               "Texture1D tex : register(t1, space2);"},
-        HlslTextureData{type::TextureDimension::k2dArray, "Texture2DArray"},
+        HlslSampledTextureData{type::TextureDimension::k2d,
-        HlslTextureData{type::TextureDimension::k3d, "Texture3D"},
+                               "Texture2D tex : register(t1, space2);"},
-        HlslTextureData{type::TextureDimension::kCube, "TextureCube"},
+        HlslSampledTextureData{type::TextureDimension::k2dArray,
-        HlslTextureData{type::TextureDimension::kCubeArray,
+                               "Texture2DArray tex : register(t1, space2);"},
-                        "TextureCubeArray"}));
+        HlslSampledTextureData{type::TextureDimension::k3d,
+                               "Texture3D tex : register(t1, space2);"},
+        HlslSampledTextureData{type::TextureDimension::kCube,
+                               "TextureCube tex : register(t1, space2);"},
+        HlslSampledTextureData{
+            type::TextureDimension::kCubeArray,
+            "TextureCubeArray tex : register(t1, space2);"}));
 
 TEST_F(HlslGeneratorImplTest_Type, EmitMultisampledTexture) {
   type::MultisampledTexture s(type::TextureDimension::k2d, ty.f32());
@@ -396,64 +434,77 @@ inline std::ostream& operator<<(std::ostream& out,
   out << data.dim << (data.ro ? "ReadOnly" : "WriteOnly");
   return out;
 }
-using HlslStoragetexturesTest = TestParamHelper<HlslStorageTextureData>;
+using HlslStorageTexturesTest = TestParamHelper<HlslStorageTextureData>;
-TEST_P(HlslStoragetexturesTest, Emit) {
+TEST_P(HlslStorageTexturesTest, Emit) {
   auto params = GetParam();
 
   auto* subtype = type::StorageTexture::SubtypeFor(params.imgfmt, Types());
-  auto* s = create<type::StorageTexture>(params.dim, params.imgfmt, subtype);
+  auto* t = create<type::StorageTexture>(params.dim, params.imgfmt, subtype);
   auto* ac =
       create<type::AccessControl>(params.ro ? ast::AccessControl::kReadOnly
                                             : ast::AccessControl::kWriteOnly,
-                                  s);
+                                  t);
+
+  Global("tex", ac, ast::StorageClass::kUniformConstant, nullptr,
+         ast::DecorationList{
+             create<ast::BindingDecoration>(1),
+             create<ast::GroupDecoration>(2),
+         });
+
+  Func("main", {}, ty.void_(),
+       {create<ast::CallStatement>(Call("textureDimensions", "tex"))},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
 
   GeneratorImpl& gen = Build();
 
-  ASSERT_TRUE(gen.EmitType(out, ac, "")) << gen.error();
+  ASSERT_TRUE(gen.Generate(out)) << gen.error();
-  EXPECT_EQ(result(), params.result);
+  EXPECT_THAT(result(), HasSubstr(params.result));
+
+  Validate();
 }
 INSTANTIATE_TEST_SUITE_P(
     HlslGeneratorImplTest_Type,
-    HlslStoragetexturesTest,
+    HlslStorageTexturesTest,
-    testing::Values(HlslStorageTextureData{type::TextureDimension::k1d,
+    testing::Values(
+        HlslStorageTextureData{type::TextureDimension::k1d,
                                type::ImageFormat::kRgba8Unorm, true,
-                                           "RWTexture1D<float4>"},
+                               "Texture1D<float4> tex : register(t1, space2);"},
         HlslStorageTextureData{type::TextureDimension::k2d,
-                                           type::ImageFormat::kRgba16Float,
+                               type::ImageFormat::kRgba16Float, true,
-                                           true, "RWTexture2D<float4>"},
+                               "Texture2D<float4> tex : register(t1, space2);"},
-                    HlslStorageTextureData{type::TextureDimension::k2dArray,
+        HlslStorageTextureData{
-                                           type::ImageFormat::kR32Float, true,
+            type::TextureDimension::k2dArray, type::ImageFormat::kR32Float,
-                                           "RWTexture2DArray<float4>"},
+            true, "Texture2DArray<float4> tex : register(t1, space2);"},
         HlslStorageTextureData{type::TextureDimension::k3d,
                                type::ImageFormat::kRg32Float, true,
-                                           "RWTexture3D<float4>"},
+                               "Texture3D<float4> tex : register(t1, space2);"},
-                    HlslStorageTextureData{type::TextureDimension::k1d,
+        HlslStorageTextureData{
-                                           type::ImageFormat::kRgba32Float,
+            type::TextureDimension::k1d, type::ImageFormat::kRgba32Float, false,
-                                           false, "RWTexture1D<float4>"},
+            "RWTexture1D<float4> tex : register(u1, space2);"},
-                    HlslStorageTextureData{type::TextureDimension::k2d,
+        HlslStorageTextureData{
-                                           type::ImageFormat::kRgba16Uint,
+            type::TextureDimension::k2d, type::ImageFormat::kRgba16Uint, false,
-                                           false, "RWTexture2D<uint4>"},
+            "RWTexture2D<uint4> tex : register(u1, space2);"},
-                    HlslStorageTextureData{type::TextureDimension::k2dArray,
+        HlslStorageTextureData{
-                                           type::ImageFormat::kR32Uint, false,
+            type::TextureDimension::k2dArray, type::ImageFormat::kR32Uint,
-                                           "RWTexture2DArray<uint4>"},
+            false, "RWTexture2DArray<uint4> tex : register(u1, space2);"},
-                    HlslStorageTextureData{type::TextureDimension::k3d,
+        HlslStorageTextureData{
-                                           type::ImageFormat::kRg32Uint, false,
+            type::TextureDimension::k3d, type::ImageFormat::kRg32Uint, false,
-                                           "RWTexture3D<uint4>"},
+            "RWTexture3D<uint4> tex : register(u1, space2);"},
         HlslStorageTextureData{type::TextureDimension::k1d,
                                type::ImageFormat::kRgba32Uint, true,
-                                           "RWTexture1D<uint4>"},
+                               "Texture1D<uint4> tex : register(t1, space2);"},
         HlslStorageTextureData{type::TextureDimension::k2d,
                                type::ImageFormat::kRgba16Sint, true,
-                                           "RWTexture2D<int4>"},
+                               "Texture2D<int4> tex : register(t1, space2);"},
-                    HlslStorageTextureData{type::TextureDimension::k2dArray,
+        HlslStorageTextureData{
-                                           type::ImageFormat::kR32Sint, true,
+            type::TextureDimension::k2dArray, type::ImageFormat::kR32Sint, true,
-                                           "RWTexture2DArray<int4>"},
+            "Texture2DArray<int4> tex : register(t1, space2);"},
         HlslStorageTextureData{type::TextureDimension::k3d,
                                type::ImageFormat::kRg32Sint, true,
-                                           "RWTexture3D<int4>"},
+                               "Texture3D<int4> tex : register(t1, space2);"},
-                    HlslStorageTextureData{type::TextureDimension::k1d,
+        HlslStorageTextureData{
-                                           type::ImageFormat::kRgba32Sint,
+            type::TextureDimension::k1d, type::ImageFormat::kRgba32Sint, false,
-                                           false, "RWTexture1D<int4>"}));
+            "RWTexture1D<int4> tex : register(u1, space2);"}));
 
 }  // namespace
 }  // namespace hlsl