[tint] Allow @color on f16 for fbf on Metal Update the frambuffer_fetch docs to state that @color attributes are allowed on f16 on Metal (since they already were in practice and were being used downstream), and add corresponding IR validation and tests. Vulkan does not allow 16-bit float types as the sampled type (VUID-StandaloneSpirv-OpTypeImage-04656), but we may need to figure out how to support it on the Vulkan backend in the near future: crbug.com/493892934. Bug: 485523357 Change-Id: I52ec84e4a194f7f8a0896e81f72f3d02b920f46c Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/297855 Reviewed-by: dan sinclair <dsinclair@chromium.org> Commit-Queue: Natalie Chouinard <chouinard@google.com>
diff --git a/docs/dawn/features/framebuffer_fetch.md b/docs/dawn/features/framebuffer_fetch.md index 0b11078..7c53cac 100644 --- a/docs/dawn/features/framebuffer_fetch.md +++ b/docs/dawn/features/framebuffer_fetch.md
@@ -22,7 +22,7 @@ * Requires `enable chromium_experimental_framebuffer_fetch` * Requires fragment shader -* Requires T to be `i32`, `u32`, `f32`, `vec{2,3,4}{f,u,i}` +* Requires T to be `i32`, `u32`, `f32`, or `vec{2,3,4}{f,u,i}`. In the Metal backend, `f16` amd `vec{2,3,4}h` are also supported. * Requires `N` to be unique per `color` * The `N` value must be in the range of `[0..7]` @@ -35,7 +35,7 @@ * Requires a `group`, `binding` value provided for each `color` `N` value from Host * Add the `OpCapability InputAttachment` * For each `color(N)` entry - * Create an `OpTypeImage %float SubpassData 0 0 0 2 Unknown` where `float` is the `f32`, `i32` or`u32` + * Create an `OpTypeImage %float SubpassData 0 0 0 2 Unknown` where `float` is the `f32`, `i32` or `u32` * Create a `OpTypePointer UniformConstant %image` * Create a `OpVariable %ptr_uniform_image UniformConstant` * Decorate the var with `OpDecorate %inputVar InputAttachmentIndex N`
diff --git a/src/tint/lang/core/ir/validator.cc b/src/tint/lang/core/ir/validator.cc index e71b9a7..812b340 100644 --- a/src/tint/lang/core/ir/validator.cc +++ b/src/tint/lang/core/ir/validator.cc
@@ -844,7 +844,7 @@ .type_check = [](const core::type::Type* ty, const Capabilities&) -> bool { return ty->IsNumericScalarOrVector(); }, - .type_error = "must be a scalar or vector", + .type_error = "must be a numeric scalar or vector", }; constexpr IOAttributeChecker kInputAttachmentIndexChecker{
diff --git a/src/tint/lang/core/ir/validator_function_test.cc b/src/tint/lang/core/ir/validator_function_test.cc index c5a5099..6630b01 100644 --- a/src/tint/lang/core/ir/validator_function_test.cc +++ b/src/tint/lang/core/ir/validator_function_test.cc
@@ -2133,7 +2133,7 @@ auto res = ir::Validate(mod); ASSERT_NE(res, Success); EXPECT_THAT(res.Failure().reason, testing::HasSubstr( - R"(:1:27 error: color must be a scalar or vector + R"(:1:27 error: color must be a numeric scalar or vector %my_func = @fragment func(%my_param:mat4x4<f32> [@color(0)]):void { ^^^^^^^^^^^^^^^^^^^^^ )")) << res.Failure(); @@ -3190,4 +3190,33 @@ )")) << res.Failure(); } +TEST_F(IR_ValidatorTest, Function_Param_Color_F16) { + auto* f = FragmentEntryPoint("my_func"); + auto* p = b.FunctionParam("my_param", ty.f16()); + p->SetColor(0); + f->SetParams({p}); + + b.Append(f->Block(), [&] { b.Return(f); }); + + auto res = ir::Validate(mod, Capabilities{Capability::kAllowNonCoreTypes}); + EXPECT_EQ(res, Success); +} + +TEST_F(IR_ValidatorTest, Function_Param_Color_Bool) { + auto* f = FragmentEntryPoint("my_func"); + auto* p = b.FunctionParam("my_param", ty.bool_()); + p->SetColor(0); + f->SetParams({p}); + + b.Append(f->Block(), [&] { b.Return(f); }); + + auto res = ir::Validate(mod); + ASSERT_NE(res, Success); + EXPECT_THAT(res.Failure().reason, testing::HasSubstr( + R"(:1:27 error: color must be a numeric scalar or vector +%my_func = @fragment func(%my_param:bool [@color(0)]):void { + ^^^^^^^^^^^^^^ +)")) << res.Failure(); +} + } // namespace tint::core::ir
diff --git a/src/tint/lang/spirv/writer/writer.cc b/src/tint/lang/spirv/writer/writer.cc index dfad457..b6e75ef 100644 --- a/src/tint/lang/spirv/writer/writer.cc +++ b/src/tint/lang/spirv/writer/writer.cc
@@ -34,7 +34,9 @@ #include "src/tint/lang/core/ir/referenced_module_vars.h" #include "src/tint/lang/core/ir/validator.h" #include "src/tint/lang/core/ir/var.h" +#include "src/tint/lang/core/type/f16.h" #include "src/tint/lang/core/type/pointer.h" +#include "src/tint/lang/core/type/struct.h" #include "src/tint/lang/core/type/u16.h" #include "src/tint/lang/spirv/writer/common/option_helpers.h" #include "src/tint/lang/spirv/writer/printer/printer.h" @@ -107,6 +109,27 @@ return Failure("entry point not found"); } + // Check for unsupported shader IO attributes. + auto check_input_attributes = [&](const core::type::Type* ty, + const core::IOAttributes& attributes) -> Result<SuccessType> { + if (attributes.color.has_value() && ty->DeepestElement()->Is<core::type::F16>()) { + return Failure( + "@color attribute on f16 type is not supported by the Vulkan SPIR-V backend"); + } + return Success; + }; + + // Check input attributes. + for (auto* param : ep_func->Params()) { + if (auto* str = param->Type()->As<core::type::Struct>()) { + for (auto* member : str->Members()) { + TINT_CHECK_RESULT(check_input_attributes(member->Type(), member->Attributes())); + } + } else { + TINT_CHECK_RESULT(check_input_attributes(param->Type(), param->Attributes())); + } + } + core::ir::ReferencedModuleVars<const core::ir::Module> referenced_module_vars{ir}; auto& refs = referenced_module_vars.TransitiveReferences(ep_func);
diff --git a/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl new file mode 100644 index 0000000..3d9f37c --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl
@@ -0,0 +1,12 @@ +enable chromium_experimental_framebuffer_fetch; +enable f16; + +struct Out { + @location(0) x : vec4<f16>, + @location(2) y : vec4<f16>, + @location(4) z : vec4<f16>, +} + +@fragment fn f(@color(1) fbf_1 : vec4<f16>, @color(3) fbf_3 : vec4<f16>) -> Out { + return Out(fbf_1, vec4<f16>(2.0h), fbf_3); +}
diff --git a/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.dxc.hlsl b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.dxc.hlsl new file mode 100644 index 0000000..98643c4 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.dxc.hlsl
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute is not supported by the HLSL backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.fxc.hlsl b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.fxc.hlsl new file mode 100644 index 0000000..98643c4 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.fxc.hlsl
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute is not supported by the HLSL backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.glsl b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.glsl new file mode 100644 index 0000000..37b5e17 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.glsl
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute is not supported by the GLSL backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.msl b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.msl new file mode 100644 index 0000000..4be74ec --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.msl
@@ -0,0 +1,32 @@ +#include <metal_stdlib> +using namespace metal; + +struct Out { + half4 x; + half4 y; + half4 z; +}; + +struct f_outputs { + half4 Out_x [[color(0)]]; + half4 Out_y [[color(2)]]; + half4 Out_z [[color(4)]]; +}; + +struct f_inputs { + half4 fbf_1 [[color(1)]]; + half4 fbf_3 [[color(3)]]; +}; + +Out f_inner(half4 fbf_1, half4 fbf_3) { + return Out{.x=fbf_1, .y=half4(2.0h), .z=fbf_3}; +} + +fragment f_outputs f(f_inputs inputs [[stage_in]]) { + Out const v = f_inner(inputs.fbf_1, inputs.fbf_3); + f_outputs tint_wrapper_result = {}; + tint_wrapper_result.Out_x = v.x; + tint_wrapper_result.Out_y = v.y; + tint_wrapper_result.Out_z = v.z; + return tint_wrapper_result; +}
diff --git a/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.spvasm b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.spvasm new file mode 100644 index 0000000..c637637 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.spvasm
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute on f16 type is not supported by the Vulkan SPIR-V backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.wgsl b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.wgsl new file mode 100644 index 0000000..249b08e --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/multiple_outputs/f16.wgsl.expected.wgsl
@@ -0,0 +1,16 @@ +enable chromium_experimental_framebuffer_fetch; +enable f16; + +struct Out { + @location(0) + x : vec4<f16>, + @location(2) + y : vec4<f16>, + @location(4) + z : vec4<f16>, +} + +@fragment +fn f(@color(1) fbf_1 : vec4<f16>, @color(3) fbf_3 : vec4<f16>) -> Out { + return Out(fbf_1, vec4<f16>(2.0h), fbf_3); +}
diff --git a/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl new file mode 100644 index 0000000..c3fab8e --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl
@@ -0,0 +1,6 @@ +enable chromium_experimental_framebuffer_fetch; +enable f16; + +@fragment fn f(@color(0) fbf : vec4<f16>) -> @location(0) vec4<f16> { + return fbf; +}
diff --git a/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.dxc.hlsl b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.dxc.hlsl new file mode 100644 index 0000000..98643c4 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.dxc.hlsl
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute is not supported by the HLSL backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.fxc.hlsl b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.fxc.hlsl new file mode 100644 index 0000000..98643c4 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.fxc.hlsl
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute is not supported by the HLSL backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.glsl b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.glsl new file mode 100644 index 0000000..37b5e17 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.glsl
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute is not supported by the GLSL backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.msl b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.msl new file mode 100644 index 0000000..317908f --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.msl
@@ -0,0 +1,20 @@ +#include <metal_stdlib> +using namespace metal; + +struct f_outputs { + half4 tint_symbol [[color(0)]]; +}; + +struct f_inputs { + half4 fbf [[color(0)]]; +}; + +half4 f_inner(half4 fbf) { + return fbf; +} + +fragment f_outputs f(f_inputs inputs [[stage_in]]) { + f_outputs tint_wrapper_result = {}; + tint_wrapper_result.tint_symbol = f_inner(inputs.fbf); + return tint_wrapper_result; +}
diff --git a/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.spvasm b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.spvasm new file mode 100644 index 0000000..c637637 --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.spvasm
@@ -0,0 +1,5 @@ +SKIP: INVALID + +@color attribute on f16 type is not supported by the Vulkan SPIR-V backend + +tint executable returned error: exit status 1
diff --git a/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.wgsl b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.wgsl new file mode 100644 index 0000000..11de1ff --- /dev/null +++ b/test/tint/extensions/framebuffer_fetch/one_output/f16.wgsl.expected.wgsl
@@ -0,0 +1,7 @@ +enable chromium_experimental_framebuffer_fetch; +enable f16; + +@fragment +fn f(@color(0) fbf : vec4<f16>) -> @location(0) vec4<f16> { + return fbf; +}