[spirv-reader][ir] Ignore `cull_distance`, skip unused builtins
This CL adds the ability to skip the `cull_distance` annotated structure
members when converting from SPIR-V. These get added as a standard part
of the `gl_PerVertex` structure created when emitting GLSL as SPIR-V.
This CL updates the ShaderIO routines to skip emitting unused members
of builtin `kPointSize` or `kClipDistances` as they also get emitted
even if unused. Any member which doesn't not have a location, builtin or
or colour attribute attached are skipped.
Bug: 42250952
Change-Id: Id6cd9cd9be45102ead9988ff53bf2932022fae31
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/220414
Reviewed-by: James Price <jrprice@google.com>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc
index b5ab776..5d1a57a 100644
--- a/src/tint/lang/core/ir/validator.cc
+++ b/src/tint/lang/core/ir/validator.cc
@@ -348,6 +348,17 @@
/* type_error */ "__point_size must be a f32",
};
+constexpr BuiltinChecker kCullDistanceChecker{
+ /* name */ "__cull_distance",
+ /* stages */ EnumSet<Function::PipelineStage>(Function::PipelineStage::kVertex),
+ /* direction */ BuiltinChecker::IODirection::kOutput,
+ /* type_check */
+ [](const core::type::Type* ty) -> bool {
+ return ty->Is<core::type::Array>() && ty->DeepestElement()->Is<core::type::F32>();
+ },
+ /* type_error */ "__cull_distance must be an array of f32",
+};
+
constexpr BuiltinChecker kFragDepthChecker{
/* name */ "frag_depth",
/* stages */ EnumSet<Function::PipelineStage>(Function::PipelineStage::kFragment),
@@ -465,6 +476,8 @@
switch (builtin) {
case BuiltinValue::kPointSize:
return kPointSizeChecker;
+ case BuiltinValue::kCullDistance:
+ return kCullDistanceChecker;
case BuiltinValue::kFragDepth:
return kFragDepthChecker;
case BuiltinValue::kFrontFacing:
diff --git a/src/tint/lang/spirv/reader/lower/shader_io.cc b/src/tint/lang/spirv/reader/lower/shader_io.cc
index e1f375c..ddff8a4 100644
--- a/src/tint/lang/spirv/reader/lower/shader_io.cc
+++ b/src/tint/lang/spirv/reader/lower/shader_io.cc
@@ -198,6 +198,10 @@
if (auto* str = var_type->As<core::type::Struct>()) {
// Add an output for each member of the struct.
for (auto* member : str->Members()) {
+ if (ShouldSkipMemberEmission(var, member)) {
+ continue;
+ }
+
// Use the base variable attributes if not specified directly on the member.
auto member_attributes = member->Attributes();
if (auto base_loc = var_attributes.location) {
@@ -253,6 +257,72 @@
}
}
+ /// Returns true if the struct member should be skipped on emission
+ /// @param var the var which references the structure
+ /// @param member the member to check
+ /// @returns true if the member should be skipped.
+ bool ShouldSkipMemberEmission(core::ir::Var* var, const core::type::StructMember* member) {
+ auto var_attributes = var->Attributes();
+ auto member_attributes = member->Attributes();
+
+ // If neither the var, nor the member has attributes, then skip
+ if (!var_attributes.builtin.has_value() && !var_attributes.color.has_value() &&
+ !var_attributes.location.has_value()) {
+ if (!member_attributes.builtin.has_value() && !member_attributes.color.has_value() &&
+ !member_attributes.location.has_value()) {
+ return true;
+ }
+ }
+
+ // The `gl_PerVertex` structure always gets emitted by glslang, but it may only be used by
+ // the `gl_Position` variable. The structure will also contain the `gl_PointSize`,
+ // `gl_ClipDistance` and `gl_CullDistance`.
+
+ if (member_attributes.builtin == core::BuiltinValue::kPointSize) {
+ // TODO(dsinclair): Validate that all accesses of this member are then used only to
+ // assign the value of 1.0.
+ return true;
+ }
+ if (member_attributes.builtin == core::BuiltinValue::kCullDistance) {
+ TINT_ASSERT(!IsIndexAccessed(var->Result(0), member->Index()));
+ return true;
+ }
+ if (member_attributes.builtin == core::BuiltinValue::kClipDistances) {
+ return !IsIndexAccessed(var->Result(0), member->Index());
+ }
+ return false;
+ }
+
+ /// Returns true if the `idx` member of `val` is accessed. The `val` must be of type
+ /// `Structure` which contains the given member index.
+ /// @param val the value to check
+ /// @param idx the index to look for
+ /// @returns true if `idx` is accessed.
+ bool IsIndexAccessed(core::ir::Value* val, uint32_t idx) {
+ for (auto& usage : val->UsagesUnsorted()) {
+ // Only care about access chains
+ auto* chain = usage->instruction->As<core::ir::Access>();
+ if (!chain) {
+ continue;
+ }
+ if (chain->Indices().IsEmpty()) {
+ continue;
+ }
+
+ // A member access has to be a constant index
+ auto* cnst = chain->Indices()[0]->As<core::ir::Constant>();
+ if (!cnst) {
+ continue;
+ }
+
+ uint32_t v = cnst->Value()->ValueAs<uint32_t>();
+ if (v == idx) {
+ return true;
+ }
+ }
+ return false;
+ }
+
/// Replace a use of an input pointer value.
/// @param var the originating input variable
/// @param value the input pointer value
diff --git a/src/tint/lang/spirv/reader/lower/shader_io_test.cc b/src/tint/lang/spirv/reader/lower/shader_io_test.cc
index a19b8e2..9b06a0b 100644
--- a/src/tint/lang/spirv/reader/lower/shader_io_test.cc
+++ b/src/tint/lang/spirv/reader/lower/shader_io_test.cc
@@ -2144,5 +2144,266 @@
EXPECT_EQ(expect, str());
}
+TEST_F(SpirvReader_ShaderIOTest, PointSize) {
+ auto* builtin_str =
+ ty.Struct(mod.symbols.New("Builtins"), Vector{
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("position"),
+ ty.vec4<f32>(),
+ BuiltinAttrs(core::BuiltinValue::kPosition),
+ },
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("point_size"),
+ ty.f32(),
+ BuiltinAttrs(core::BuiltinValue::kPointSize),
+ },
+ });
+ auto* builtins = b.Var("builtins", ty.ptr(core::AddressSpace::kOut, builtin_str));
+ mod.root_block->Append(builtins);
+
+ auto* ep = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kVertex);
+ b.Append(ep->Block(), [&] { //
+ auto* ptr = ty.ptr(core::AddressSpace::kOut, ty.vec4<f32>());
+ b.Store(b.Access(ptr, builtins, 0_u), b.Splat<vec4<f32>>(1_f));
+ b.Store(b.Access(ty.ptr(core::AddressSpace::kOut, ty.f32()), builtins, 1_u), 1_f);
+ b.Return(ep);
+ });
+
+ auto* src = R"(
+Builtins = struct @align(16) {
+ position:vec4<f32> @offset(0), @builtin(position)
+ point_size:f32 @offset(16), @builtin(__point_size)
+}
+
+$B1: { # root
+ %builtins:ptr<__out, Builtins, read_write> = var
+}
+
+%foo = @vertex func():void {
+ $B2: {
+ %3:ptr<__out, vec4<f32>, read_write> = access %builtins, 0u
+ store %3, vec4<f32>(1.0f)
+ %4:ptr<__out, f32, read_write> = access %builtins, 1u
+ store %4, 1.0f
+ ret
+ }
+}
+)";
+ EXPECT_EQ(src, str());
+
+ auto* expect = R"(
+Builtins = struct @align(16) {
+ position:vec4<f32> @offset(0)
+ point_size:f32 @offset(16)
+}
+
+$B1: { # root
+ %builtins:ptr<private, Builtins, read_write> = var
+}
+
+%foo_inner = func():void {
+ $B2: {
+ %3:ptr<private, vec4<f32>, read_write> = access %builtins, 0u
+ store %3, vec4<f32>(1.0f)
+ %4:ptr<private, f32, read_write> = access %builtins, 1u
+ store %4, 1.0f
+ ret
+ }
+}
+%foo = @vertex func():vec4<f32> [@position] {
+ $B3: {
+ %6:void = call %foo_inner
+ %7:ptr<private, vec4<f32>, read_write> = access %builtins, 0u
+ %8:vec4<f32> = load %7
+ ret %8
+ }
+}
+)";
+
+ Run(ShaderIO);
+
+ EXPECT_EQ(expect, str());
+}
+
+TEST_F(SpirvReader_ShaderIOTest, Outputs_Struct_UnusedOutputs) {
+ auto* builtin_str = ty.Struct(mod.symbols.New("Builtins"),
+ Vector{
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("position"),
+ ty.vec4<f32>(),
+ BuiltinAttrs(core::BuiltinValue::kPosition),
+ },
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("clip_distance"),
+ ty.array<f32, 3>(),
+ BuiltinAttrs(core::BuiltinValue::kClipDistances),
+ },
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("point_size"),
+ ty.f32(),
+ BuiltinAttrs(core::BuiltinValue::kPointSize),
+ },
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("cull_distance"),
+ ty.array<f32, 3>(),
+ BuiltinAttrs(core::BuiltinValue::kCullDistance),
+ },
+ });
+ auto* builtins = b.Var("builtins", ty.ptr(core::AddressSpace::kOut, builtin_str));
+ mod.root_block->Append(builtins);
+
+ auto* ep = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kVertex);
+ b.Append(ep->Block(), [&] { //
+ auto* ptr = ty.ptr(core::AddressSpace::kOut, ty.vec4<f32>());
+ b.Store(b.Access(ptr, builtins, 0_u), b.Splat<vec4<f32>>(1_f));
+ b.Return(ep);
+ });
+
+ auto* src = R"(
+Builtins = struct @align(16) {
+ position:vec4<f32> @offset(0), @builtin(position)
+ clip_distance:array<f32, 3> @offset(16), @builtin(clip_distances)
+ point_size:f32 @offset(28), @builtin(__point_size)
+ cull_distance:array<f32, 3> @offset(32), @builtin(__cull_distance)
+}
+
+$B1: { # root
+ %builtins:ptr<__out, Builtins, read_write> = var
+}
+
+%foo = @vertex func():void {
+ $B2: {
+ %3:ptr<__out, vec4<f32>, read_write> = access %builtins, 0u
+ store %3, vec4<f32>(1.0f)
+ ret
+ }
+}
+)";
+ EXPECT_EQ(src, str());
+
+ auto* expect = R"(
+Builtins = struct @align(16) {
+ position:vec4<f32> @offset(0)
+ clip_distance:array<f32, 3> @offset(16)
+ point_size:f32 @offset(28)
+ cull_distance:array<f32, 3> @offset(32)
+}
+
+$B1: { # root
+ %builtins:ptr<private, Builtins, read_write> = var
+}
+
+%foo_inner = func():void {
+ $B2: {
+ %3:ptr<private, vec4<f32>, read_write> = access %builtins, 0u
+ store %3, vec4<f32>(1.0f)
+ ret
+ }
+}
+%foo = @vertex func():vec4<f32> [@position] {
+ $B3: {
+ %5:void = call %foo_inner
+ %6:ptr<private, vec4<f32>, read_write> = access %builtins, 0u
+ %7:vec4<f32> = load %6
+ ret %7
+ }
+}
+)";
+
+ Run(ShaderIO);
+
+ EXPECT_EQ(expect, str());
+}
+
+TEST_F(SpirvReader_ShaderIOTest, Outputs_Struct_MultipledUsed) {
+ auto* builtin_str = ty.Struct(mod.symbols.New("Builtins"),
+ Vector{
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("position"),
+ ty.vec4<f32>(),
+ BuiltinAttrs(core::BuiltinValue::kPosition),
+ },
+ core::type::Manager::StructMemberDesc{
+ mod.symbols.New("clip_distance"),
+ ty.array<f32, 3>(),
+ BuiltinAttrs(core::BuiltinValue::kClipDistances),
+ },
+ });
+ auto* builtins = b.Var("builtins", ty.ptr(core::AddressSpace::kOut, builtin_str));
+ mod.root_block->Append(builtins);
+
+ auto* ep = b.Function("foo", ty.void_(), core::ir::Function::PipelineStage::kVertex);
+ b.Append(ep->Block(), [&] { //
+ auto* ptr1 = ty.ptr(core::AddressSpace::kOut, ty.vec4<f32>());
+ b.Store(b.Access(ptr1, builtins, 0_u), b.Splat<vec4<f32>>(1_f));
+
+ auto* ptr2 = ty.ptr(core::AddressSpace::kOut, ty.array<f32, 3>());
+ b.Store(b.Access(ptr2, builtins, 1_u), b.Splat<array<f32, 3>>(1_f));
+ b.Return(ep);
+ });
+
+ auto* src = R"(
+Builtins = struct @align(16) {
+ position:vec4<f32> @offset(0), @builtin(position)
+ clip_distance:array<f32, 3> @offset(16), @builtin(clip_distances)
+}
+
+$B1: { # root
+ %builtins:ptr<__out, Builtins, read_write> = var
+}
+
+%foo = @vertex func():void {
+ $B2: {
+ %3:ptr<__out, vec4<f32>, read_write> = access %builtins, 0u
+ store %3, vec4<f32>(1.0f)
+ %4:ptr<__out, array<f32, 3>, read_write> = access %builtins, 1u
+ store %4, array<f32, 3>(1.0f)
+ ret
+ }
+}
+)";
+ EXPECT_EQ(src, str());
+
+ auto* expect = R"(
+Builtins = struct @align(16) {
+ position:vec4<f32> @offset(0)
+ clip_distance:array<f32, 3> @offset(16)
+}
+
+tint_symbol = struct @align(16) {
+ position:vec4<f32> @offset(0), @builtin(position)
+ clip_distance:array<f32, 3> @offset(16), @builtin(clip_distances)
+}
+
+$B1: { # root
+ %builtins:ptr<private, Builtins, read_write> = var
+}
+
+%foo_inner = func():void {
+ $B2: {
+ %3:ptr<private, vec4<f32>, read_write> = access %builtins, 0u
+ store %3, vec4<f32>(1.0f)
+ %4:ptr<private, array<f32, 3>, read_write> = access %builtins, 1u
+ store %4, array<f32, 3>(1.0f)
+ ret
+ }
+}
+%foo = @vertex func():tint_symbol {
+ $B3: {
+ %6:void = call %foo_inner
+ %7:ptr<private, vec4<f32>, read_write> = access %builtins, 0u
+ %8:vec4<f32> = load %7
+ %9:ptr<private, array<f32, 3>, read_write> = access %builtins, 1u
+ %10:array<f32, 3> = load %9
+ %11:tint_symbol = construct %8, %10
+ ret %11
+ }
+}
+)";
+
+ Run(ShaderIO);
+
+ EXPECT_EQ(expect, str());
+}
} // namespace
} // namespace tint::spirv::reader::lower
diff --git a/src/tint/lang/spirv/reader/parser/parser.cc b/src/tint/lang/spirv/reader/parser/parser.cc
index 271973b..c43fe01 100644
--- a/src/tint/lang/spirv/reader/parser/parser.cc
+++ b/src/tint/lang/spirv/reader/parser/parser.cc
@@ -155,6 +155,8 @@
return core::BuiltinValue::kWorkgroupId;
case spv::BuiltIn::ClipDistance:
return core::BuiltinValue::kClipDistances;
+ case spv::BuiltIn::CullDistance:
+ return core::BuiltinValue::kCullDistance;
default:
TINT_UNIMPLEMENTED() << "unhandled SPIR-V BuiltIn: " << static_cast<uint32_t>(b);
}
