spirv-reader: switch to HLSL-style pipeline IO

- When storing to sample_mask output, write to the 0th element
- Only make a return struct if it has members
- Adjust type signedness coercion when loading special builtins.
- Adapt tests

- Update expectations for end-to-end tests

- Handle sample_mask with stride
  Input variables normally don't have layout. But they can have it
  up through SPIR-V 1.4.
  Handle this case in the SPIR-V reader, by seeing through the
  intermediate alias type created for the strided array type.

Bug: tint:508
Change-Id: I0f19dc1305d3f250dbbc0698a602288c34245274
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54743
Auto-Submit: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
This commit is contained in:
David Neto
2021-06-17 09:10:04 +00:00
committed by Tint LUCI CQ
parent c932b5535f
commit 1e19b55d19
102 changed files with 6487 additions and 3194 deletions

View File

@@ -961,7 +961,7 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() {
auto* forced_store_type = store_type;
ast::DecorationList param_decos;
if (!parser_impl_.ConvertDecorationsForVariable(var_id, &forced_store_type,
&param_decos)) {
&param_decos, true)) {
// This occurs, and is not an error, for the PointSize builtin.
if (!success()) {
// But exit early if an error was logged.
@@ -986,15 +986,23 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() {
// variable.
ast::Expression* param_value =
create<ast::IdentifierExpression>(source, param_sym);
ast::Expression* store_dest =
create<ast::IdentifierExpression>(source, var_sym);
if (HasBuiltinSampleMask(param_decos)) {
// In Vulkan SPIR-V, the sample mask is an array. In WGSL it's a scalar.
// Use the first element only.
param_value = create<ast::ArrayAccessorExpression>(
source, param_value, parser_impl_.MakeNullValue(ty_.I32()));
if (store_type->As<Array>()->type->IsSignedScalarOrVector()) {
// sample_mask is unsigned in WGSL. Bitcast it.
param_value = create<ast::BitcastExpression>(
source, ty_.I32()->Build(builder_), param_value);
store_dest = create<ast::ArrayAccessorExpression>(
source, store_dest, parser_impl_.MakeNullValue(ty_.I32()));
if (const auto* arr_ty = store_type->UnwrapAlias()->As<Array>()) {
if (arr_ty->type->IsSignedScalarOrVector()) {
// sample_mask is unsigned in WGSL. Bitcast it.
param_value = create<ast::BitcastExpression>(
source, ty_.I32()->Build(builder_), param_value);
}
} else {
// Vulkan SPIR-V requires this. Validation should have failed already.
return Fail()
<< "expected SampleMask to be an array of integer scalars";
}
} else if (forced_store_type != store_type) {
// The parameter will have the WGSL type, but we need to add
@@ -1003,9 +1011,8 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() {
source, store_type->Build(builder_), param_value);
}
stmts.push_back(create<ast::AssignmentStatement>(
source, create<ast::IdentifierExpression>(source, var_sym),
param_value));
stmts.push_back(
create<ast::AssignmentStatement>(source, store_dest, param_value));
}
// Call the inner function. It has no parameters.
@@ -1053,7 +1060,7 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() {
store_type = GetVariableStoreType(*var);
param_type = store_type;
if (!parser_impl_.ConvertDecorationsForVariable(var_id, &param_type,
&out_decos)) {
&out_decos, true)) {
// This occurs, and is not an error, for the PointSize builtin.
if (!success()) {
// But exit early if an error was logged.
@@ -1084,10 +1091,16 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() {
// Get the first element only.
return_member_value = create<ast::ArrayAccessorExpression>(
source, return_member_value, parser_impl_.MakeNullValue(ty_.I32()));
if (store_type->As<Array>()->type->IsSignedScalarOrVector()) {
// sample_mask is unsigned in WGSL. Bitcast it.
return_member_value = create<ast::BitcastExpression>(
source, param_type->Build(builder_), return_member_value);
if (const auto* arr_ty = store_type->UnwrapAlias()->As<Array>()) {
if (arr_ty->type->IsSignedScalarOrVector()) {
// sample_mask is unsigned in WGSL. Bitcast it.
return_member_value = create<ast::BitcastExpression>(
source, param_type->Build(builder_), return_member_value);
}
} else {
// Vulkan SPIR-V requires this. Validation should have failed already.
return Fail()
<< "expected SampleMask to be an array of integer scalars";
}
} else {
// No other builtin outputs need signedness conversion.
@@ -1096,16 +1109,22 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() {
return_exprs.push_back(return_member_value);
}
// Create and register the result type.
auto* str = create<ast::Struct>(Source{}, return_struct_sym, return_members,
ast::DecorationList{});
parser_impl_.AddTypeDecl(return_struct_sym, str);
return_type = builder_.ty.Of(str);
if (return_members.empty()) {
// This can occur if only the PointSize member is accessed, because we
// never emit it.
return_type = ty_.Void()->Build(builder_);
} else {
// Create and register the result type.
auto* str = create<ast::Struct>(Source{}, return_struct_sym,
return_members, ast::DecorationList{});
parser_impl_.AddTypeDecl(return_struct_sym, str);
return_type = builder_.ty.Of(str);
// Add the return-value statement.
stmts.push_back(create<ast::ReturnStatement>(
source, create<ast::TypeConstructorExpression>(
source, return_type, std::move(return_exprs))));
// Add the return-value statement.
stmts.push_back(create<ast::ReturnStatement>(
source, create<ast::TypeConstructorExpression>(
source, return_type, std::move(return_exprs))));
}
}
auto* body = create<ast::BlockStatement>(source, stmts);
@@ -3320,6 +3339,8 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
return false;
}
TypedExpression lhs;
// Handle exceptional cases
switch (GetSkipReason(ptr_id)) {
case SkipReason::kPointSizeBuiltinPointer:
@@ -3332,13 +3353,33 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
<< inst.PrettyPrint();
case SkipReason::kSampleMaskOutBuiltinPointer:
ptr_id = sample_mask_out_id;
if (!rhs.type->Is<U32>()) {
// WGSL requires sample_mask_out to be signed.
rhs = TypedExpression{ty_.U32(),
create<ast::TypeConstructorExpression>(
Source{}, builder_.ty.u32(),
ast::ExpressionList{rhs.expr})};
lhs = MakeExpression(sample_mask_out_id);
if (lhs.type->Is<Pointer>()) {
// LHS of an assignment must be a reference type.
// Convert the LHS to a reference by dereferencing it.
lhs = Dereference(lhs);
}
if (parser_impl_.UseHLSLStylePipelineIO()) {
// In the HLSL-style pipeline IO case, the private variable is an
// array whose element type is already of the same type as the value
// being stored into it. Form the reference into the first element.
lhs.expr = create<ast::ArrayAccessorExpression>(
Source{}, lhs.expr, parser_impl_.MakeNullValue(ty_.I32()));
if (auto* ref = lhs.type->As<Reference>()) {
lhs.type = ref->type;
}
if (auto* arr = lhs.type->As<Array>()) {
lhs.type = arr->type;
}
TINT_ASSERT(lhs.type);
} else {
if (!rhs.type->Is<U32>()) {
// WGSL requires sample_mask_out to be unsigned.
rhs = TypedExpression{ty_.U32(),
create<ast::TypeConstructorExpression>(
Source{}, builder_.ty.u32(),
ast::ExpressionList{rhs.expr})};
}
}
break;
default:
@@ -3346,7 +3387,9 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
}
// Handle an ordinary store as an assignment.
auto lhs = MakeExpression(ptr_id);
if (!lhs) {
lhs = MakeExpression(ptr_id);
}
if (!lhs) {
return false;
}
@@ -3367,6 +3410,7 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
// So represent a load by a new const definition.
const auto ptr_id = inst.GetSingleWordInOperand(0);
const auto skip_reason = GetSkipReason(ptr_id);
switch (skip_reason) {
case SkipReason::kPointSizeBuiltinPointer:
GetDefInfo(inst.result_id())->skip =
@@ -3375,7 +3419,6 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
case SkipReason::kSampleIdBuiltinPointer:
case SkipReason::kVertexIndexBuiltinPointer:
case SkipReason::kInstanceIndexBuiltinPointer: {
// The SPIR-V variable is i32, but WGSL requires u32.
auto name = NameForSpecialInputBuiltin(skip_reason);
if (name.empty()) {
return Fail() << "internal error: unhandled special input builtin "
@@ -3384,30 +3427,30 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
}
ast::Expression* id_expr = create<ast::IdentifierExpression>(
Source{}, builder_.Symbols().Register(name));
auto expr = TypedExpression{
ty_.I32(),
create<ast::TypeConstructorExpression>(
Source{}, builder_.ty.i32(), ast::ExpressionList{id_expr})};
auto* loaded_type = parser_impl_.ConvertType(inst.type_id());
auto expr = TypedExpression{loaded_type, id_expr};
return EmitConstDefinition(inst, expr);
}
case SkipReason::kSampleMaskInBuiltinPointer: {
auto name = namer_.Name(sample_mask_in_id);
ast::Expression* id_expr = create<ast::IdentifierExpression>(
Source{}, builder_.Symbols().Register(name));
auto* load_result_type = parser_impl_.ConvertType(inst.type_id());
ast::Expression* ast_expr = nullptr;
if (load_result_type->Is<I32>()) {
ast_expr = create<ast::TypeConstructorExpression>(
Source{}, builder_.ty.i32(), ast::ExpressionList{id_expr});
} else if (load_result_type->Is<U32>()) {
ast_expr = id_expr;
} else {
// SampleMask is an array in Vulkan SPIR-V. Always access the first
// element.
id_expr = create<ast::ArrayAccessorExpression>(
Source{}, id_expr, parser_impl_.MakeNullValue(ty_.I32()));
auto* loaded_type = parser_impl_.ConvertType(inst.type_id());
if (!loaded_type->IsIntegerScalar()) {
return Fail() << "loading the whole SampleMask input array is not "
"supported: "
<< inst.PrettyPrint();
}
return EmitConstDefinition(
inst, TypedExpression{load_result_type, ast_expr});
auto expr = TypedExpression{loaded_type, id_expr};
return EmitConstDefinition(inst, expr);
}
default:
break;

View File

@@ -58,7 +58,6 @@ TEST_F(SpvParserTest, EmitStatement_VoidCallNoParams) {
Return{}
}
Function $2 -> __void
StageDecoration{vertex}
()
{
Call[not set]{
@@ -68,6 +67,16 @@ TEST_F(SpvParserTest, EmitStatement_VoidCallNoParams) {
}
Return{}
}
Function $3 -> __void
StageDecoration{vertex}
()
{
Call[not set]{
Identifier[not set]{$2}
(
)
}
}
}
)";
EXPECT_EQ(expect, got);
@@ -224,7 +233,7 @@ TEST_F(SpvParserTest, EmitStatement_CallWithParams) {
ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error();
EXPECT_TRUE(p->error().empty());
const auto program_ast_str = p->program().to_str();
EXPECT_THAT(program_ast_str, HasSubstr(R"(Module{
const std::string expected = R"(Module{
Function x_50 -> __u32
(
VariableConst{
@@ -251,8 +260,7 @@ TEST_F(SpvParserTest, EmitStatement_CallWithParams) {
}
}
}
Function x_100 -> __void
StageDecoration{vertex}
Function x_100_1 -> __void
()
{
VariableDeclStatement{
@@ -274,7 +282,19 @@ TEST_F(SpvParserTest, EmitStatement_CallWithParams) {
}
Return{}
}
})")) << program_ast_str;
Function x_100 -> __void
StageDecoration{vertex}
()
{
Call[not set]{
Identifier[not set]{x_100_1}
(
)
}
}
}
)";
EXPECT_EQ(program_ast_str, expected);
}
} // namespace

View File

@@ -883,11 +883,20 @@ TEST_F(SpvParserMemoryTest, EmitStatement_AccessChain_DereferenceBase) {
}
Return{}
}
Function main_1 -> __void
()
{
Return{}
}
Function main -> __void
StageDecoration{vertex}
()
{
Return{}
Call[not set]{
Identifier[not set]{main_1}
(
)
}
}
}
)";

View File

@@ -250,6 +250,7 @@ ParserImpl::ParserImpl(const std::vector<uint32_t>& spv_binary)
namer_(fail_stream_),
enum_converter_(fail_stream_),
tools_context_(kInputEnv) {
hlsl_style_pipeline_io_ = true;
// Create a message consumer to propagate error messages from SPIRV-Tools
// out as our own failures.
message_consumer_ = [this](spv_message_level_t level, const char* /*source*/,
@@ -1408,12 +1409,9 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id,
sc = ast::StorageClass::kNone;
}
// In almost all cases, copy the decorations from SPIR-V to the variable.
// But avoid doing so when converting pipeline IO to private variables.
if (sc != ast::StorageClass::kPrivate) {
if (!ConvertDecorationsForVariable(id, &storage_type, &decorations)) {
return nullptr;
}
if (!ConvertDecorationsForVariable(id, &storage_type, &decorations,
sc != ast::StorageClass::kPrivate)) {
return nullptr;
}
std::string name = namer_.Name(id);
@@ -1427,10 +1425,10 @@ ast::Variable* ParserImpl::MakeVariable(uint32_t id,
constructor, decorations);
}
bool ParserImpl::ConvertDecorationsForVariable(
uint32_t id,
const Type** type,
ast::DecorationList* decorations) {
bool ParserImpl::ConvertDecorationsForVariable(uint32_t id,
const Type** store_type,
ast::DecorationList* decorations,
bool transfer_pipeline_io) {
for (auto& deco : GetDecorationsFor(id)) {
if (deco.empty()) {
return Fail() << "malformed decoration on ID " << id << ": it is empty";
@@ -1456,10 +1454,12 @@ bool ParserImpl::ConvertDecorationsForVariable(
// The SPIR-V variable may signed (because GLSL requires signed for
// some of these), but WGSL requires unsigned. Handle specially
// so we always perform the conversion at load and store.
if (auto* forced_type = UnsignedTypeFor(*type)) {
special_builtins_[id] = spv_builtin;
if (auto* forced_type = UnsignedTypeFor(*store_type)) {
// Requires conversion and special handling in code generation.
special_builtins_[id] = spv_builtin;
*type = forced_type;
if (transfer_pipeline_io) {
*store_type = forced_type;
}
}
break;
case SpvBuiltInSampleMask: {
@@ -1473,7 +1473,9 @@ bool ParserImpl::ConvertDecorationsForVariable(
"SampleMask must be an array of 1 element.";
}
special_builtins_[id] = spv_builtin;
*type = ty_.U32();
if (transfer_pipeline_io) {
*store_type = ty_.U32();
}
break;
}
default:
@@ -1484,16 +1486,20 @@ bool ParserImpl::ConvertDecorationsForVariable(
// A diagnostic has already been emitted.
return false;
}
decorations->emplace_back(
create<ast::BuiltinDecoration>(Source{}, ast_builtin));
if (transfer_pipeline_io) {
decorations->emplace_back(
create<ast::BuiltinDecoration>(Source{}, ast_builtin));
}
}
if (deco[0] == SpvDecorationLocation) {
if (deco.size() != 2) {
return Fail() << "malformed Location decoration on ID " << id
<< ": requires one literal operand";
}
decorations->emplace_back(
create<ast::LocationDecoration>(Source{}, deco[1]));
if (transfer_pipeline_io) {
decorations->emplace_back(
create<ast::LocationDecoration>(Source{}, deco[1]));
}
}
if (deco[0] == SpvDecorationDescriptorSet) {
if (deco.size() == 1) {

View File

@@ -226,13 +226,16 @@ class ParserImpl : Reader {
/// a diagnostic), or when the variable should not be emitted, e.g. for a
/// PointSize builtin.
/// @param id the ID of the SPIR-V variable
/// @param type the WGSL store type for the variable, which should be
/// @param store_type the WGSL store type for the variable, which should be
/// prepopulatd
/// @param ast_decos the decoration list to populate
/// @param transfer_pipeline_io true if pipeline IO decorations (builtins,
/// or locations) will update the store type and the decorations list
/// @returns false when the variable should not be emitted as a variable
bool ConvertDecorationsForVariable(uint32_t id,
const Type** type,
ast::DecorationList* ast_decos);
const Type** store_type,
ast::DecorationList* ast_decos,
bool transfer_pipeline_io);
/// Converts a SPIR-V struct member decoration. If the decoration is
/// recognized but deliberately dropped, then returns nullptr without a

View File

@@ -48,22 +48,28 @@ Program ParseAndBuild(std::string spirv) {
TEST_F(SpvParserTest, WorkgroupBarrier) {
auto program = ParseAndBuild(R"(
OpName %helper "helper"
%void = OpTypeVoid
%1 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%uint_2 = OpConstant %uint 2
%uint_264 = OpConstant %uint 264
%main = OpFunction %void None %1
%helper = OpFunction %void None %1
%4 = OpLabel
OpControlBarrier %uint_2 %uint_2 %uint_264
OpReturn
OpFunctionEnd
%main = OpFunction %void None %1
%5 = OpLabel
OpReturn
OpFunctionEnd
)");
ASSERT_TRUE(program.IsValid()) << program.Diagnostics().str();
auto* main = program.AST().Functions().Find(program.Symbols().Get("main"));
ASSERT_NE(main, nullptr);
ASSERT_GT(main->body()->size(), 0u);
auto* call = main->body()->get(0)->As<ast::CallStatement>();
auto* helper =
program.AST().Functions().Find(program.Symbols().Get("helper"));
ASSERT_NE(helper, nullptr);
ASSERT_GT(helper->body()->size(), 0u);
auto* call = helper->body()->get(0)->As<ast::CallStatement>();
ASSERT_NE(call, nullptr);
EXPECT_EQ(call->expr()->params().size(), 0u);
auto* sem_call = program.Sem().Get(call->expr());
@@ -75,23 +81,29 @@ TEST_F(SpvParserTest, WorkgroupBarrier) {
TEST_F(SpvParserTest, StorageBarrier) {
auto program = ParseAndBuild(R"(
OpName %helper "helper"
%void = OpTypeVoid
%1 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%uint_2 = OpConstant %uint 2
%uint_1 = OpConstant %uint 1
%uint_72 = OpConstant %uint 72
%main = OpFunction %void None %1
%helper = OpFunction %void None %1
%4 = OpLabel
OpControlBarrier %uint_2 %uint_1 %uint_72
OpReturn
OpFunctionEnd
%main = OpFunction %void None %1
%5 = OpLabel
OpReturn
OpFunctionEnd
)");
ASSERT_TRUE(program.IsValid()) << program.Diagnostics().str();
auto* main = program.AST().Functions().Find(program.Symbols().Get("main"));
ASSERT_NE(main, nullptr);
ASSERT_GT(main->body()->size(), 0u);
auto* call = main->body()->get(0)->As<ast::CallStatement>();
auto* helper =
program.AST().Functions().Find(program.Symbols().Get("helper"));
ASSERT_NE(helper, nullptr);
ASSERT_GT(helper->body()->size(), 0u);
auto* call = helper->body()->get(0)->As<ast::CallStatement>();
ASSERT_NE(call, nullptr);
EXPECT_EQ(call->expr()->params().size(), 0u);
auto* sem_call = program.Sem().Get(call->expr());

View File

@@ -672,7 +672,7 @@ TEST_F(SpvParserTest, ConvertType_PointerInput) {
auto* ptr_ty = type->As<Pointer>();
EXPECT_NE(ptr_ty, nullptr);
EXPECT_TRUE(ptr_ty->type->Is<F32>());
EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kInput);
EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kPrivate);
EXPECT_TRUE(p->error().empty());
}
@@ -688,7 +688,7 @@ TEST_F(SpvParserTest, ConvertType_PointerOutput) {
auto* ptr_ty = type->As<Pointer>();
EXPECT_NE(ptr_ty, nullptr);
EXPECT_TRUE(ptr_ty->type->Is<F32>());
EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kOutput);
EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kPrivate);
EXPECT_TRUE(p->error().empty());
}
@@ -819,12 +819,12 @@ TEST_F(SpvParserTest, ConvertType_PointerToPointer) {
auto* ptr_ty = type->As<Pointer>();
EXPECT_NE(ptr_ty, nullptr);
EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kInput);
EXPECT_EQ(ptr_ty->storage_class, ast::StorageClass::kPrivate);
EXPECT_TRUE(ptr_ty->type->Is<Pointer>());
auto* ptr_ptr_ty = ptr_ty->type->As<Pointer>();
EXPECT_NE(ptr_ptr_ty, nullptr);
EXPECT_EQ(ptr_ptr_ty->storage_class, ast::StorageClass::kOutput);
EXPECT_EQ(ptr_ptr_ty->storage_class, ast::StorageClass::kPrivate);
EXPECT_TRUE(ptr_ptr_ty->type->Is<F32>());
EXPECT_TRUE(p->error().empty());

View File

@@ -283,14 +283,7 @@ TEST_F(SpvParserTest, EmitFunctions_CalleePrecedesCaller) {
}
}
Return{}
}
Function x_100 -> __void
StageDecoration{vertex}
()
{
Return{}
}
})")) << program_ast;
})")) << program_ast;
}
TEST_F(SpvParserTest, EmitFunctions_NonVoidResultType) {

File diff suppressed because it is too large Load Diff