hlsl/writer: Fix output for texture intrinsics and types
Fixed: tint:146
Change-Id: Icf356faf7bee364a0783c717ee76ea98056b4401
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42022
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc
index 9a60d21..4a7d5f4 100644
--- a/src/writer/hlsl/generator_impl.cc
+++ b/src/writer/hlsl/generator_impl.cc
@@ -58,7 +58,9 @@
#include "src/type/f32_type.h"
#include "src/type/i32_type.h"
#include "src/type/matrix_type.h"
+#include "src/type/multisampled_texture_type.h"
#include "src/type/pointer_type.h"
+#include "src/type/sampled_texture_type.h"
#include "src/type/sampler_type.h"
#include "src/type/storage_texture_type.h"
#include "src/type/struct_type.h"
@@ -161,7 +163,10 @@
}
}
+ // emitted_globals is a set used to ensure that globals are emitted once even
+ // if they are used by multiple entry points.
std::unordered_set<Symbol> emitted_globals;
+
// Make sure all entry point data is emitted before the entry point functions
for (auto* func : builder_.AST().Functions()) {
if (!func->IsEntryPoint()) {
@@ -791,9 +796,9 @@
case semantic::IntrinsicType::kTextureNumLevels:
case semantic::IntrinsicType::kTextureNumSamples: {
// All of these intrinsics use the GetDimensions() method on the texture
+ bool is_ms = texture_type->Is<type::MultisampledTexture>();
int num_dimensions = 0;
- const char* swizzle = "";
- bool add_mip_level_in = false;
+ std::string swizzle;
switch (intrinsic->Type()) {
case semantic::IntrinsicType::kTextureDimensions:
@@ -809,10 +814,11 @@
swizzle = ".x";
break;
case type::TextureDimension::k2d:
- num_dimensions = 2;
+ num_dimensions = is_ms ? 3 : 2;
+ swizzle = is_ms ? ".xy" : "";
break;
case type::TextureDimension::k2dArray:
- num_dimensions = 3;
+ num_dimensions = is_ms ? 4 : 3;
swizzle = ".xy";
break;
case type::TextureDimension::k3d:
@@ -838,10 +844,13 @@
TINT_ICE(diagnostics_) << "texture dimension is not arrayed";
return false;
case type::TextureDimension::k1dArray:
- num_dimensions = 2;
+ num_dimensions = is_ms ? 3 : 2;
swizzle = ".y";
break;
case type::TextureDimension::k2dArray:
+ num_dimensions = is_ms ? 4 : 3;
+ swizzle = ".z";
+ break;
case type::TextureDimension::kCubeArray:
num_dimensions = 3;
swizzle = ".z";
@@ -849,7 +858,6 @@
}
break;
case semantic::IntrinsicType::kTextureNumLevels:
- add_mip_level_in = true;
switch (texture_type->dim()) {
default:
TINT_ICE(diagnostics_)
@@ -889,34 +897,63 @@
return false;
}
+ auto* level_arg = arg(Usage::kLevel);
+
+ if (level_arg) {
+ // `NumberOfLevels` is a non-optional argument if `MipLevel` was passed.
+ // Increment the number of dimensions for the temporary vector to
+ // accommodate this.
+ num_dimensions++;
+
+ // If the swizzle was empty, the expression will evaluate to the whole
+ // vector. As we've grown the vector by one element, we now need to
+ // swizzle to keep the result expression equivalent.
+ if (swizzle.empty()) {
+ static constexpr const char* swizzles[] = {"", ".x", ".xy", ".xyz"};
+ swizzle = swizzles[num_dimensions - 1];
+ }
+ }
+
+ if (num_dimensions > 4) {
+ TINT_ICE(diagnostics_)
+ << "Texture query intrinsic temporary vector has " << num_dimensions
+ << " dimensions";
+ return false;
+ }
+
// Declare a variable to hold the queried texture info
auto dims = generate_name(kTempNamePrefix);
if (num_dimensions == 1) {
- pre << "int " << dims << ";\n";
+ pre << "int " << dims << ";";
} else {
- pre << "int" << num_dimensions << " " << dims << ";\n";
+ pre << "int" << num_dimensions << " " << dims << ";";
}
+ pre << std::endl;
+ make_indent(pre);
+
if (!EmitExpression(pre, pre, texture)) {
return false;
}
pre << ".GetDimensions(";
- if (auto* level = arg(Usage::kLevel)) {
- if (!EmitExpression(pre, pre, level)) {
+
+ if (level_arg) {
+ if (!EmitExpression(pre, pre, level_arg)) {
return false;
}
pre << ", ";
- } else if (add_mip_level_in) {
+ } else if (intrinsic->Type() ==
+ semantic::IntrinsicType::kTextureNumLevels) {
pre << "0, ";
}
if (num_dimensions == 1) {
pre << dims;
} else {
+ static constexpr char xyzw[] = {'x', 'y', 'z', 'w'};
assert(num_dimensions > 0);
assert(num_dimensions <= 4);
- static constexpr char xyzw[] = {'x', 'y', 'z', 'w'};
for (int i = 0; i < num_dimensions; i++) {
if (i > 0) {
pre << ", ";
@@ -924,7 +961,9 @@
pre << dims << "." << xyzw[i];
}
}
- pre << ");";
+
+ pre << ");" << std::endl;
+ make_indent(pre);
// The out parameters of the GetDimensions() call is now in temporary
// `dims` variable. This may be packed with other data, so the final
@@ -1578,6 +1617,11 @@
for (auto data : func_sem->ReferencedUniformVariables()) {
auto* var = data.first;
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
@@ -1590,13 +1634,6 @@
}
// auto* set = data.second.set;
- // If the global has already been emitted we skip it, it's been emitted by
- // a previous entry point.
- if (emitted_globals.count(decl->symbol()) != 0) {
- continue;
- }
- emitted_globals.insert(decl->symbol());
-
auto* type = decl->type()->UnwrapIfNeeded();
if (auto* strct = type->As<type::Struct>()) {
out << "ConstantBuffer<" << builder_.Symbols().NameFor(strct->symbol())
@@ -1634,15 +1671,12 @@
for (auto data : func_sem->ReferencedStorageBufferVariables()) {
auto* var = data.first;
auto* decl = var->Declaration();
- auto* binding = data.second.binding;
- // If the global has already been emitted we skip it, it's been emitted by
- // a previous entry point.
- if (emitted_globals.count(decl->symbol()) != 0) {
- continue;
+ if (!emitted_globals.emplace(decl->symbol()).second) {
+ continue; // Global already emitted
}
- emitted_globals.insert(decl->symbol());
+ auto* binding = data.second.binding;
auto* ac = decl->type()->As<type::AccessControl>();
if (ac == nullptr) {
diagnostics_.add_error("access control type required for storage buffer");
@@ -1762,6 +1796,34 @@
out << "};" << std::endl << std::endl;
}
+ {
+ bool add_newline = false;
+ for (auto* var : func_sem->ReferencedModuleVariables()) {
+ auto* decl = var->Declaration();
+
+ auto* unwrapped_type = decl->type()->UnwrapAll();
+ if (!unwrapped_type->Is<type::Texture>() &&
+ !unwrapped_type->Is<type::Sampler>()) {
+ continue; // Not interested in this type
+ }
+
+ if (!emitted_globals.emplace(decl->symbol()).second) {
+ continue; // Global already emitted
+ }
+
+ if (!EmitType(out, decl->type(), "")) {
+ return false;
+ }
+ out << " " << namer_.NameFor(builder_.Symbols().NameFor(decl->symbol()))
+ << ";" << std::endl;
+
+ add_newline = true;
+ }
+ if (add_newline) {
+ out << std::endl;
+ }
+ }
+
return true;
}
@@ -2314,6 +2376,9 @@
return false;
}
out << pre.str();
+ if (!TypeOf(c->expr())->Is<type::Void>()) {
+ out << "(void) ";
+ }
out << call_out.str() << ";" << std::endl;
return true;
}
@@ -2439,6 +2504,8 @@
}
out << "Texture";
+ auto* ms = tex->As<type::MultisampledTexture>();
+
switch (tex->dim()) {
case type::TextureDimension::k1d:
out << "1D";
@@ -2447,10 +2514,10 @@
out << "1DArray";
break;
case type::TextureDimension::k2d:
- out << "2D";
+ out << (ms ? "2DMS" : "2D");
break;
case type::TextureDimension::k2dArray:
- out << "2DArray";
+ out << (ms ? "2DMSArray" : "2DArray");
break;
case type::TextureDimension::k3d:
out << "3D";
@@ -2462,11 +2529,31 @@
out << "CubeArray";
break;
default:
- diagnostics_.add_error("Invalid texture dimensions");
+ TINT_UNREACHABLE(diagnostics_)
+ << "unexpected TextureDimension " << tex->dim();
return false;
}
- if (auto* st = tex->As<type::StorageTexture>()) {
+ if (ms) {
+ out << "<";
+ if (ms->type()->Is<type::F32>()) {
+ out << "float4";
+ } else if (ms->type()->Is<type::I32>()) {
+ out << "int4";
+ } else if (ms->type()->Is<type::U32>()) {
+ out << "uint4";
+ } else {
+ TINT_ICE(diagnostics_) << "Unsupported multisampled texture type";
+ return false;
+ }
+
+ // TODO(ben-clayton): The HLSL docs claim that the MS texture type should
+ // also contain the number of samples, which is not part of the WGSL type.
+ // However, DXC seems to consider this optional.
+ // See: https://github.com/gpuweb/gpuweb/issues/1445
+
+ out << ">";
+ } else if (auto* st = tex->As<type::StorageTexture>()) {
auto* component = image_format_to_rwtexture_type(st->image_format());
if (component == nullptr) {
TINT_ICE(diagnostics_) << "Unsupported StorageTexture ImageFormat: "
diff --git a/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc b/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc
index ce84d3c..b9ddb0f 100644
--- a/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc
+++ b/src/writer/hlsl/generator_impl_intrinsic_texture_test.cc
@@ -44,127 +44,153 @@
case ValidTextureOverload::kDimensionsStorageRO1d:
case ValidTextureOverload::kDimensionsStorageWO1d:
return {
- "int _tint_tmp;\n"
- "texture_tint_0.GetDimensions(_tint_tmp);",
+ R"(int _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp);
+)",
"_tint_tmp",
};
case ValidTextureOverload::kDimensions1dArray:
case ValidTextureOverload::kDimensionsStorageRO1dArray:
case ValidTextureOverload::kDimensionsStorageWO1dArray:
return {
- "int2 _tint_tmp;\n"
- "texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);",
+ R"(int2 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);
+)",
"_tint_tmp.x",
};
case ValidTextureOverload::kDimensions2d:
- case ValidTextureOverload::kDimensionsMultisampled2d:
case ValidTextureOverload::kDimensionsDepth2d:
case ValidTextureOverload::kDimensionsStorageRO2d:
case ValidTextureOverload::kDimensionsStorageWO2d:
return {
- "int2 _tint_tmp;\n"
- "texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);",
+ R"(int2 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);
+)",
"_tint_tmp",
};
+ case ValidTextureOverload::kDimensionsMultisampled2d:
+ return {
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
+ "_tint_tmp.xy",
+ };
+
case ValidTextureOverload::kDimensions2dArray:
- case ValidTextureOverload::kDimensionsMultisampled2dArray:
case ValidTextureOverload::kDimensionsDepth2dArray:
case ValidTextureOverload::kDimensionsStorageRO2dArray:
case ValidTextureOverload::kDimensionsStorageWO2dArray:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
+ "_tint_tmp.xy",
+ };
+ case ValidTextureOverload::kDimensionsMultisampled2dArray:
+ return {
+ R"(int4 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);
+)",
"_tint_tmp.xy",
};
case ValidTextureOverload::kDimensions3d:
case ValidTextureOverload::kDimensionsStorageRO3d:
case ValidTextureOverload::kDimensionsStorageWO3d:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
"_tint_tmp",
};
case ValidTextureOverload::kDimensionsCube:
case ValidTextureOverload::kDimensionsDepthCube:
return {
"int2 _tint_tmp;\n"
- "texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);",
+ "texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);\n",
"_tint_tmp.xyy",
};
case ValidTextureOverload::kDimensionsCubeArray:
case ValidTextureOverload::kDimensionsDepthCubeArray:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
"_tint_tmp.xyy",
};
case ValidTextureOverload::kDimensions2dLevel:
case ValidTextureOverload::kDimensionsDepth2dLevel:
return {
- "int2 _tint_tmp;\n"
- "texture_tint_0.GetDimensions(1, _tint_tmp.x, _tint_tmp.y);",
- "_tint_tmp",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
+ "_tint_tmp.xy",
};
case ValidTextureOverload::kDimensions2dArrayLevel:
case ValidTextureOverload::kDimensionsDepth2dArrayLevel:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int4 _tint_tmp;
+texture_tint_0.GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);
+)",
"_tint_tmp.xy",
};
case ValidTextureOverload::kDimensions3dLevel:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
- "_tint_tmp",
+ R"(int4 _tint_tmp;
+texture_tint_0.GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);
+)",
+ "_tint_tmp.xyz",
};
case ValidTextureOverload::kDimensionsCubeLevel:
case ValidTextureOverload::kDimensionsDepthCubeLevel:
return {
- "int2 _tint_tmp;\n"
- "texture_tint_0.GetDimensions(1, _tint_tmp.x, _tint_tmp.y);",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
"_tint_tmp.xyy",
};
case ValidTextureOverload::kDimensionsCubeArrayLevel:
case ValidTextureOverload::kDimensionsDepthCubeArrayLevel:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int4 _tint_tmp;
+texture_tint_0.GetDimensions(1, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);
+)",
"_tint_tmp.xyy",
};
case ValidTextureOverload::kNumLayers1dArray:
case ValidTextureOverload::kNumLayersStorageWO1dArray:
return {
- "int2 _tint_tmp;\n"
- "texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);",
+ R"(int2 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y);
+)",
"_tint_tmp.y",
};
case ValidTextureOverload::kNumLayers2dArray:
- case ValidTextureOverload::kNumLayersMultisampled2dArray:
case ValidTextureOverload::kNumLayersDepth2dArray:
case ValidTextureOverload::kNumLayersCubeArray:
case ValidTextureOverload::kNumLayersDepthCubeArray:
case ValidTextureOverload::kNumLayersStorageWO2dArray:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
"_tint_tmp.z",
};
+ case ValidTextureOverload::kNumLayersMultisampled2dArray:
+ return {
+ R"(int4 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);
+)",
+ "_tint_tmp.z",
+ };
+
case ValidTextureOverload::kNumLevels2d:
case ValidTextureOverload::kNumLevelsCube:
case ValidTextureOverload::kNumLevelsDepth2d:
case ValidTextureOverload::kNumLevelsDepthCube:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(0, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(0, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
"_tint_tmp.z",
};
case ValidTextureOverload::kNumLevels2dArray:
@@ -173,23 +199,23 @@
case ValidTextureOverload::kNumLevelsDepth2dArray:
case ValidTextureOverload::kNumLevelsDepthCubeArray:
return {
- "int4 _tint_tmp;\n"
- "texture_tint_0.GetDimensions(0, "
- "_tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);",
+ R"(int4 _tint_tmp;
+texture_tint_0.GetDimensions(0, _tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);
+)",
"_tint_tmp.w",
};
case ValidTextureOverload::kNumSamplesMultisampled2d:
return {
- "int3 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);",
+ R"(int3 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z);
+)",
"_tint_tmp.z",
};
case ValidTextureOverload::kNumSamplesMultisampled2dArray:
return {
- "int4 _tint_tmp;\n"
- "texture_tint_0."
- "GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);",
+ R"(int4 _tint_tmp;
+texture_tint_0.GetDimensions(_tint_tmp.x, _tint_tmp.y, _tint_tmp.z, _tint_tmp.w);
+)",
"_tint_tmp.w",
};
case ValidTextureOverload::kSample1dF32:
diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc
index 6f29eed..c75a5e2 100644
--- a/src/writer/hlsl/generator_impl_type_test.cc
+++ b/src/writer/hlsl/generator_impl_type_test.cc
@@ -389,7 +389,7 @@
GeneratorImpl& gen = Build();
ASSERT_TRUE(gen.EmitType(out, &s, "")) << gen.error();
- EXPECT_EQ(result(), "Texture2D");
+ EXPECT_EQ(result(), "Texture2DMS<float4>");
}
struct HlslStorageTextureData {
