Rename builtin(sample_mask_{in,out}) to builtin(sample_mask)

Removes the need to pass the storage class to the SPIR-V reader
builtin mapping function.

Added a deprecation warning for sample_mask_{in,out}.

Bug: tint:715
Change-Id: I948ff2de2d5de7bd05e1c6ff45bd721c856900e3
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/47743
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
James Price
2021-04-14 16:44:38 +00:00
committed by Commit Bot service account
parent 88d7edcf7a
commit edd4d3cc3b
19 changed files with 144 additions and 86 deletions

View File

@@ -66,7 +66,7 @@ ast::StorageClass EnumConverter::ToStorageClass(const SpvStorageClass sc) {
return ast::StorageClass::kNone;
}
ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b, ast::StorageClass sc) {
ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b) {
switch (b) {
case SpvBuiltInPosition:
return ast::Builtin::kPosition;
@@ -89,8 +89,7 @@ ast::Builtin EnumConverter::ToBuiltin(SpvBuiltIn b, ast::StorageClass sc) {
case SpvBuiltInSampleId:
return ast::Builtin::kSampleIndex;
case SpvBuiltInSampleMask:
return sc == ast::StorageClass::kInput ? ast::Builtin::kSampleMaskIn
: ast::Builtin::kSampleMaskOut;
return ast::Builtin::kSampleMask;
default:
break;
}

View File

@@ -50,9 +50,8 @@ 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::StorageClass sc);
ast::Builtin ToBuiltin(SpvBuiltIn b);
/// Converts a possibly arrayed SPIR-V Dim to a Tint texture dimension.
/// On failure, logs an error and returns kNone

View File

@@ -162,12 +162,11 @@ 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) << " sc:" << int(bc.sc)
out << "BuiltinCase{ SpvBuiltIn:" << int(bc.builtin)
<< " expect_success?:" << int(bc.expect_success)
<< " expected:" << int(bc.expected) << "}";
return out;
@@ -192,7 +191,7 @@ class SpvBuiltinTest : public testing::TestWithParam<BuiltinCase> {
TEST_P(SpvBuiltinTest, Samples) {
const auto params = GetParam();
const auto result = converter_.ToBuiltin(params.builtin, params.sc);
const auto result = converter_.ToBuiltin(params.builtin);
EXPECT_EQ(success_, params.expect_success);
if (params.expect_success) {
EXPECT_EQ(result, params.expected);
@@ -207,46 +206,35 @@ INSTANTIATE_TEST_SUITE_P(
EnumConverterGood_Input,
SpvBuiltinTest,
testing::Values(
BuiltinCase{SpvBuiltInPosition, ast::StorageClass::kInput, true,
ast::Builtin::kPosition},
BuiltinCase{SpvBuiltInInstanceIndex, ast::StorageClass::kInput, true,
BuiltinCase{SpvBuiltInPosition, true, ast::Builtin::kPosition},
BuiltinCase{SpvBuiltInInstanceIndex, true,
ast::Builtin::kInstanceIndex},
BuiltinCase{SpvBuiltInFrontFacing, ast::StorageClass::kInput, true,
ast::Builtin::kFrontFacing},
BuiltinCase{SpvBuiltInFragCoord, ast::StorageClass::kInput, true,
ast::Builtin::kPosition},
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::kSampleIndex},
BuiltinCase{SpvBuiltInSampleMask, ast::StorageClass::kInput, true,
ast::Builtin::kSampleMaskIn}));
BuiltinCase{SpvBuiltInFrontFacing, true, ast::Builtin::kFrontFacing},
BuiltinCase{SpvBuiltInFragCoord, true, ast::Builtin::kPosition},
BuiltinCase{SpvBuiltInLocalInvocationId, true,
ast::Builtin::kLocalInvocationId},
BuiltinCase{SpvBuiltInLocalInvocationIndex, true,
ast::Builtin::kLocalInvocationIndex},
BuiltinCase{SpvBuiltInGlobalInvocationId, true,
ast::Builtin::kGlobalInvocationId},
BuiltinCase{SpvBuiltInSampleId, true, ast::Builtin::kSampleIndex},
BuiltinCase{SpvBuiltInSampleMask, true, ast::Builtin::kSampleMask}));
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}));
testing::Values(
BuiltinCase{SpvBuiltInPosition, true, ast::Builtin::kPosition},
BuiltinCase{SpvBuiltInFragDepth, true, ast::Builtin::kFragDepth},
BuiltinCase{SpvBuiltInSampleMask, true, ast::Builtin::kSampleMask}));
INSTANTIATE_TEST_SUITE_P(
EnumConverterBad,
SpvBuiltinTest,
testing::Values(
BuiltinCase{static_cast<SpvBuiltIn>(9999), ast::StorageClass::kInput,
false, ast::Builtin::kNone},
BuiltinCase{static_cast<SpvBuiltIn>(9999), ast::StorageClass::kOutput,
false, ast::Builtin::kNone},
BuiltinCase{SpvBuiltInNumWorkgroups, ast::StorageClass::kInput, false,
ast::Builtin::kNone}));
BuiltinCase{static_cast<SpvBuiltIn>(9999), false, ast::Builtin::kNone},
BuiltinCase{static_cast<SpvBuiltIn>(9999), false, ast::Builtin::kNone},
BuiltinCase{SpvBuiltInNumWorkgroups, false, ast::Builtin::kNone}));
// Dim

View File

@@ -1304,7 +1304,7 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id,
default:
break;
}
auto ast_builtin = enum_converter_.ToBuiltin(spv_builtin, sc);
auto ast_builtin = enum_converter_.ToBuiltin(spv_builtin);
if (ast_builtin == ast::Builtin::kNone) {
return nullptr;
}

View File

@@ -2473,7 +2473,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_U32_Direct) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_in}
BuiltinDecoration{sample_mask}
}
x_1
in
@@ -2515,7 +2515,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_U32_CopyObject) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_in}
BuiltinDecoration{sample_mask}
}
x_1
in
@@ -2557,7 +2557,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_U32_AccessChain) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_in}
BuiltinDecoration{sample_mask}
}
x_1
in
@@ -2598,7 +2598,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_I32_Direct) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_in}
BuiltinDecoration{sample_mask}
}
x_1
in
@@ -2643,7 +2643,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_I32_CopyObject) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_in}
BuiltinDecoration{sample_mask}
}
x_1
in
@@ -2688,7 +2688,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_In_I32_AccessChain) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_in}
BuiltinDecoration{sample_mask}
}
x_1
in
@@ -2751,7 +2751,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_Out_U32_Direct) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_out}
BuiltinDecoration{sample_mask}
}
x_1
out
@@ -2787,7 +2787,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_Out_U32_CopyObject) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_out}
BuiltinDecoration{sample_mask}
}
x_1
out
@@ -2823,7 +2823,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_Out_U32_AccessChain) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_out}
BuiltinDecoration{sample_mask}
}
x_1
out
@@ -2858,7 +2858,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_Out_I32_Direct) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_out}
BuiltinDecoration{sample_mask}
}
x_1
out
@@ -2900,7 +2900,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_Out_I32_CopyObject) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_out}
BuiltinDecoration{sample_mask}
}
x_1
out
@@ -2942,7 +2942,7 @@ TEST_F(SpvModuleScopeVarParserTest, SampleMask_Out_I32_AccessChain) {
EXPECT_THAT(module_str, HasSubstr(R"(
Variable{
Decorations{
BuiltinDecoration{sample_mask_out}
BuiltinDecoration{sample_mask}
}
x_1
out

View File

@@ -88,6 +88,9 @@ ast::Builtin ident_to_builtin(const std::string& str) {
if (str == "sample_index") {
return ast::Builtin::kSampleIndex;
}
if (str == "sample_mask") {
return ast::Builtin::kSampleMask;
}
if (str == "sample_mask_in") {
return ast::Builtin::kSampleMaskIn;
}
@@ -1382,6 +1385,12 @@ Expect<ast::Builtin> ParserImpl::expect_builtin() {
if (builtin == ast::Builtin::kFragCoord) {
deprecated(ident.source, "use 'position' instead of 'frag_coord'");
}
if (builtin == ast::Builtin::kSampleMaskIn) {
deprecated(ident.source, "use 'sample_mask' instead of 'sample_mask_in'");
}
if (builtin == ast::Builtin::kSampleMaskOut) {
deprecated(ident.source, "use 'sample_mask' instead of 'sample_mask_out'");
}
return {builtin, ident.source};
}

View File

@@ -122,6 +122,7 @@ INSTANTIATE_TEST_SUITE_P(
ast::Builtin::kLocalInvocationIndex},
BuiltinData{"global_invocation_id", ast::Builtin::kGlobalInvocationId},
BuiltinData{"sample_index", ast::Builtin::kSampleIndex},
BuiltinData{"sample_mask", ast::Builtin::kSampleMask},
BuiltinData{"sample_mask_in", ast::Builtin::kSampleMaskIn},
BuiltinData{"sample_mask_out", ast::Builtin::kSampleMaskOut}));
@@ -327,6 +328,50 @@ builtin(frag_coord)
)");
}
TEST_F(ParserImplTest, Decoration_SampleMaskIn_Deprecated) {
auto p = parser("builtin(sample_mask_in)");
auto deco = p->decoration();
EXPECT_TRUE(deco.matched);
EXPECT_FALSE(deco.errored);
ASSERT_NE(deco.value, nullptr);
auto* var_deco = deco.value->As<ast::Decoration>();
ASSERT_NE(var_deco, nullptr);
ASSERT_FALSE(p->has_error());
ASSERT_TRUE(var_deco->Is<ast::BuiltinDecoration>());
auto* builtin = var_deco->As<ast::BuiltinDecoration>();
EXPECT_EQ(builtin->value(), ast::Builtin::kSampleMaskIn);
EXPECT_EQ(
p->builder().Diagnostics().str(),
R"(test.wgsl:1:9 warning: use of deprecated language feature: use 'sample_mask' instead of 'sample_mask_in'
builtin(sample_mask_in)
^^^^^^^^^^^^^^
)");
}
TEST_F(ParserImplTest, Decoration_SampleMaskOut_Deprecated) {
auto p = parser("builtin(sample_mask_out)");
auto deco = p->decoration();
EXPECT_TRUE(deco.matched);
EXPECT_FALSE(deco.errored);
ASSERT_NE(deco.value, nullptr);
auto* var_deco = deco.value->As<ast::Decoration>();
ASSERT_NE(var_deco, nullptr);
ASSERT_FALSE(p->has_error());
ASSERT_TRUE(var_deco->Is<ast::BuiltinDecoration>());
auto* builtin = var_deco->As<ast::BuiltinDecoration>();
EXPECT_EQ(builtin->value(), ast::Builtin::kSampleMaskOut);
EXPECT_EQ(
p->builder().Diagnostics().str(),
R"(test.wgsl:1:9 warning: use of deprecated language feature: use 'sample_mask' instead of 'sample_mask_out'
builtin(sample_mask_out)
^^^^^^^^^^^^^^^
)");
}
} // namespace
} // namespace wgsl
} // namespace reader