From ef6eefdc8ff0bd5c52a821519f823a919da0cf31 Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 4 Feb 2021 17:34:00 +0000 Subject: [PATCH] spirv-reader: builtin conversions for sample_mask_in, sample_mask_out Enum conversion of builtins depends on storage class. Bug: tint:471 Change-Id: I0b93c26139f72c70786b9b70f1016e62e1df62f4 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/40122 Commit-Queue: David Neto Auto-Submit: David Neto Reviewed-by: Ben Clayton Reviewed-by: dan sinclair --- src/reader/spirv/enum_converter.cc | 5 ++- src/reader/spirv/enum_converter.h | 3 +- src/reader/spirv/enum_converter_test.cc | 55 +++++++++++++++++-------- src/reader/spirv/parser_impl.cc | 2 +- 4 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/reader/spirv/enum_converter.cc b/src/reader/spirv/enum_converter.cc index 0bf795bbd4..149e26549b 100644 --- a/src/reader/spirv/enum_converter.cc +++ b/src/reader/spirv/enum_converter.cc @@ -66,7 +66,7 @@ ast::StorageClass EnumConverter::ToStorageClass(const SpvStorageClass sc) { return ast::StorageClass::kNone; } -ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b) { +ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b, ast::StorageClass sc) { switch (b) { case SpvBuiltInPosition: return ast::Builtin::kPosition; @@ -88,6 +88,9 @@ ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b) { return ast::Builtin::kGlobalInvocationId; case SpvBuiltInSampleId: return ast::Builtin::kSampleId; + case SpvBuiltInSampleMask: + return sc == ast::StorageClass::kInput ? ast::Builtin::kSampleMaskIn + : ast::Builtin::kSampleMaskOut; default: break; } diff --git a/src/reader/spirv/enum_converter.h b/src/reader/spirv/enum_converter.h index 59c7d042d8..c5545d0c04 100644 --- a/src/reader/spirv/enum_converter.h +++ b/src/reader/spirv/enum_converter.h @@ -51,8 +51,9 @@ class EnumConverter { /// Converts a SPIR-V Builtin value a Tint Builtin. /// On failure, logs an error and returns kNone /// @param b the SPIR-V builtin + /// @param sc the Tint storage class /// @returns a Tint AST builtin - ast::Builtin ToBuiltin(SpvBuiltIn b); + ast::Builtin ToBuiltin(SpvBuiltIn b, ast::StorageClass sc); /// Converts a possibly arrayed SPIR-V Dim to a Tint texture dimension. /// On failure, logs an error and returns kNone diff --git a/src/reader/spirv/enum_converter_test.cc b/src/reader/spirv/enum_converter_test.cc index ad8adc1fbc..bf34e62578 100644 --- a/src/reader/spirv/enum_converter_test.cc +++ b/src/reader/spirv/enum_converter_test.cc @@ -165,11 +165,12 @@ INSTANTIATE_TEST_SUITE_P(EnumConverterBad, struct BuiltinCase { SpvBuiltIn builtin; + ast::StorageClass sc; bool expect_success; ast::Builtin expected; }; inline std::ostream& operator<<(std::ostream& out, BuiltinCase bc) { - out << "BuiltinCase{ SpvBuiltIn:" << int(bc.builtin) + out << "BuiltinCase{ SpvBuiltIn:" << int(bc.builtin) << " sc:" << int(bc.sc) << " expect_success?:" << int(bc.expect_success) << " expected:" << int(bc.expected) << "}"; return out; @@ -194,7 +195,7 @@ class SpvBuiltinTest : public testing::TestWithParam { TEST_P(SpvBuiltinTest, Samples) { const auto params = GetParam(); - const auto result = converter_.ToBuiltin(params.builtin); + const auto result = converter_.ToBuiltin(params.builtin, params.sc); EXPECT_EQ(success_, params.expect_success); if (params.expect_success) { EXPECT_EQ(result, params.expected); @@ -206,29 +207,49 @@ TEST_P(SpvBuiltinTest, Samples) { } INSTANTIATE_TEST_SUITE_P( - EnumConverterGood, + EnumConverterGood_Input, SpvBuiltinTest, testing::Values( - BuiltinCase{SpvBuiltInPosition, true, ast::Builtin::kPosition}, - BuiltinCase{SpvBuiltInVertexIndex, true, ast::Builtin::kVertexIndex}, - BuiltinCase{SpvBuiltInInstanceIndex, true, + BuiltinCase{SpvBuiltInPosition, ast::StorageClass::kInput, true, + ast::Builtin::kPosition}, + BuiltinCase{SpvBuiltInInstanceIndex, ast::StorageClass::kInput, true, ast::Builtin::kInstanceIndex}, - BuiltinCase{SpvBuiltInFrontFacing, true, ast::Builtin::kFrontFacing}, - BuiltinCase{SpvBuiltInFragCoord, true, ast::Builtin::kFragCoord}, - BuiltinCase{SpvBuiltInFragDepth, true, ast::Builtin::kFragDepth}, - BuiltinCase{SpvBuiltInLocalInvocationId, true, - ast::Builtin::kLocalInvocationId}, - BuiltinCase{SpvBuiltInLocalInvocationIndex, true, - ast::Builtin::kLocalInvocationIndex}, - BuiltinCase{SpvBuiltInGlobalInvocationId, true, - ast::Builtin::kGlobalInvocationId})); + BuiltinCase{SpvBuiltInFrontFacing, ast::StorageClass::kInput, true, + ast::Builtin::kFrontFacing}, + BuiltinCase{SpvBuiltInFragCoord, ast::StorageClass::kInput, true, + ast::Builtin::kFragCoord}, + BuiltinCase{SpvBuiltInLocalInvocationId, ast::StorageClass::kInput, + true, ast::Builtin::kLocalInvocationId}, + BuiltinCase{SpvBuiltInLocalInvocationIndex, ast::StorageClass::kInput, + true, ast::Builtin::kLocalInvocationIndex}, + BuiltinCase{SpvBuiltInGlobalInvocationId, ast::StorageClass::kInput, + true, ast::Builtin::kGlobalInvocationId}, + BuiltinCase{SpvBuiltInSampleId, ast::StorageClass::kInput, true, + ast::Builtin::kSampleId}, + BuiltinCase{SpvBuiltInSampleMask, ast::StorageClass::kInput, true, + ast::Builtin::kSampleMaskIn})); + +INSTANTIATE_TEST_SUITE_P( + EnumConverterGood_Output, + SpvBuiltinTest, + testing::Values(BuiltinCase{SpvBuiltInPosition, ast::StorageClass::kOutput, + true, ast::Builtin::kPosition}, + BuiltinCase{SpvBuiltInFragDepth, ast::StorageClass::kOutput, + true, ast::Builtin::kFragDepth}, + BuiltinCase{SpvBuiltInSampleMask, + ast::StorageClass::kOutput, true, + ast::Builtin::kSampleMaskOut})); INSTANTIATE_TEST_SUITE_P( EnumConverterBad, SpvBuiltinTest, testing::Values( - BuiltinCase{static_cast(9999), false, ast::Builtin::kNone}, - BuiltinCase{SpvBuiltInNumWorkgroups, false, ast::Builtin::kNone})); + BuiltinCase{static_cast(9999), ast::StorageClass::kInput, + false, ast::Builtin::kNone}, + BuiltinCase{static_cast(9999), ast::StorageClass::kOutput, + false, ast::Builtin::kNone}, + BuiltinCase{SpvBuiltInNumWorkgroups, ast::StorageClass::kInput, false, + ast::Builtin::kNone})); // Dim diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 2d0b3a27cf..a72b47475e 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -1280,7 +1280,7 @@ ast::Variable* ParserImpl::MakeVariable( default: break; } - auto ast_builtin = enum_converter_.ToBuiltin(spv_builtin); + auto ast_builtin = enum_converter_.ToBuiltin(spv_builtin, sc); if (ast_builtin == ast::Builtin::kNone) { return nullptr; }