From 1234633b32b01f22d42ca516e415e7d7515168d6 Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 9 Apr 2020 13:29:17 +0000 Subject: [PATCH] [spirv-reader] Handle OpLoad Bug: tint:3 Change-Id: I25fdf086e49426240a771b70306b417cd8012777 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/19140 Reviewed-by: dan sinclair --- src/reader/spirv/function.cc | 32 +++++- src/reader/spirv/function.h | 2 + src/reader/spirv/function_memory_test.cc | 118 ++++++++++++++++++++++- 3 files changed, 144 insertions(+), 8 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index b92c64f586..e80741552d 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -23,6 +23,7 @@ #include "src/ast/assignment_statement.h" #include "src/ast/identifier_expression.h" #include "src/ast/scalar_constructor_expression.h" +#include "src/ast/storage_class.h" #include "src/ast/uint_literal.h" #include "src/ast/variable.h" #include "src/ast/variable_decl_statement.h" @@ -166,6 +167,8 @@ bool FunctionEmitter::EmitFunctionVariables() { auto var_decl_stmt = std::make_unique(std::move(var)); ast_body_.emplace_back(std::move(var_decl_stmt)); + // Save this as an already-named value. + identifier_values_.insert(inst.result_id()); } return success(); } @@ -174,6 +177,9 @@ std::unique_ptr FunctionEmitter::MakeExpression(uint32_t id) { if (failed()) { return nullptr; } + if (identifier_values_.count(id)) { + return std::make_unique(namer_.Name(id)); + } const auto* spirv_constant = constant_mgr_->FindDeclaredConstant(id); if (spirv_constant) { return parser_impl_.MakeConstantExpression(id); @@ -184,13 +190,10 @@ std::unique_ptr FunctionEmitter::MakeExpression(uint32_t id) { return nullptr; } switch (inst->opcode()) { - case SpvOpVariable: - // This is not a declaration, but a use of an identifier. - return std::make_unique(namer_.Name(id)); default: break; } - Fail() << "unhandled expression for ID " << id; + Fail() << "unhandled expression for ID " << id << "\n" << inst->PrettyPrint(); return nullptr; } @@ -229,6 +232,27 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { std::move(lhs), std::move(rhs))); return success(); } + case SpvOpLoad: { + // Memory accesses must be issued in SPIR-V program order. + // So represent a load by a new const definition. + auto ast_initializer = MakeExpression(inst.GetSingleWordInOperand(0)); + if (!ast_initializer) { + return false; + } + auto ast_const = + parser_impl_.MakeVariable(inst.result_id(), ast::StorageClass::kNone, + parser_impl_.ConvertType(inst.type_id())); + if (!ast_const) { + return false; + } + ast_const->set_constructor(std::move(ast_initializer)); + ast_const->set_is_const(true); + ast_body_.emplace_back( + std::make_unique(std::move(ast_const))); + // Save this as an already-named value. + identifier_values_.insert(inst.result_id()); + return success(); + } case SpvOpFunctionCall: // TODO(dneto): Fill this out. Make this pass, for existing tests return success(); diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index dd4c85ad71..86f518d5ec 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -111,6 +111,8 @@ class FunctionEmitter { Namer& namer_; const spvtools::opt::Function& function_; ast::StatementList ast_body_; + // The set of IDs that have already had an identifier name generated for it. + std::unordered_set identifier_values_; }; } // namespace spirv diff --git a/src/reader/spirv/function_memory_test.cc b/src/reader/spirv/function_memory_test.cc index 6f43419017..1067fa229f 100644 --- a/src/reader/spirv/function_memory_test.cc +++ b/src/reader/spirv/function_memory_test.cc @@ -29,7 +29,7 @@ namespace { using ::testing::HasSubstr; -TEST_F(SpvParserTest, EmitFunctionVariables_StoreBoolConst) { +TEST_F(SpvParserTest, EmitStatement_StoreBoolConst) { auto p = parser(test::Assemble(R"( %void = OpTypeVoid %voidfn = OpTypeFunction %void @@ -64,7 +64,7 @@ Assignment{ })")); } -TEST_F(SpvParserTest, EmitFunctionVariables_StoreUintConst) { +TEST_F(SpvParserTest, EmitStatement_StoreUintConst) { auto p = parser(test::Assemble(R"( %void = OpTypeVoid %voidfn = OpTypeFunction %void @@ -92,7 +92,7 @@ Assignment{ })")); } -TEST_F(SpvParserTest, EmitFunctionVariables_StoreIntConst) { +TEST_F(SpvParserTest, EmitStatement_StoreIntConst) { auto p = parser(test::Assemble(R"( %void = OpTypeVoid %voidfn = OpTypeFunction %void @@ -120,7 +120,7 @@ Assignment{ })")); } -TEST_F(SpvParserTest, EmitFunctionVariables_StoreFloatConst) { +TEST_F(SpvParserTest, EmitStatement_StoreFloatConst) { auto p = parser(test::Assemble(R"( %void = OpTypeVoid %voidfn = OpTypeFunction %void @@ -148,6 +148,116 @@ Assignment{ })")); } +TEST_F(SpvParserTest, EmitStatement_LoadBool) { + auto p = parser(test::Assemble(R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %ty = OpTypeBool + %true = OpConstantTrue %ty + %false = OpConstantFalse %ty + %null = OpConstantNull %ty + %ptr_ty = OpTypePointer Function %ty + %100 = OpFunction %void None %voidfn + %entry = OpLabel + %1 = OpVariable %ptr_ty Function %true + %2 = OpLoad %ty %1 + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"( + Variable{ + x_2 + none + __bool + { + Identifier{x_1} + } + })")); +} + +TEST_F(SpvParserTest, EmitStatement_LoadScalar) { + auto p = parser(test::Assemble(R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %ty = OpTypeInt 32 0 + %ty_42 = OpConstant %ty 42 + %ptr_ty = OpTypePointer Function %ty + %100 = OpFunction %void None %voidfn + %entry = OpLabel + %1 = OpVariable %ptr_ty Function %ty_42 + %2 = OpLoad %ty %1 + %3 = OpLoad %ty %1 + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{ + Variable{ + x_2 + none + __u32 + { + Identifier{x_1} + } + } +} +VariableDeclStatement{ + Variable{ + x_3 + none + __u32 + { + Identifier{x_1} + } + } +})")); +} + +TEST_F(SpvParserTest, EmitStatement_UseLoadedScalarTwice) { + auto p = parser(test::Assemble(R"( + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %ty = OpTypeInt 32 0 + %ty_42 = OpConstant %ty 42 + %ptr_ty = OpTypePointer Function %ty + %100 = OpFunction %void None %voidfn + %entry = OpLabel + %1 = OpVariable %ptr_ty Function %ty_42 + %2 = OpLoad %ty %1 + OpStore %1 %2 + OpStore %1 %2 + OpReturn + OpFunctionEnd + )")); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error(); + FunctionEmitter fe(p, *spirv_function(100)); + EXPECT_TRUE(fe.EmitBody()) << p->error(); + EXPECT_THAT(ToString(fe.ast_body()), HasSubstr(R"(VariableDeclStatement{ + Variable{ + x_2 + none + __u32 + { + Identifier{x_1} + } + } +} +Assignment{ + Identifier{x_1} + Identifier{x_2} +} +Assignment{ + Identifier{x_1} + Identifier{x_2} +} +)")); +} + } // namespace } // namespace spirv } // namespace reader