encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

spirv-reader: Handle vector insert dynamic into a hoisted variable 

VectorInsertDynamic normaly inserts a temporary variable.
But if the source vector is already a hoisted variable, then
reuse that instead.  This avoids defining the same name twice.

Bug: tint:804
Change-Id: I69c20c11d462c148261bb0646db698dc7850495d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/55362
Auto-Submit: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
David Neto 2021-06-21 21:45:36 +00:00 committed by Tint LUCI CQ
parent f19e0e4360
commit 7aac7b521e
2 changed files with 133 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
src/reader/spirv/function.cc
View File

@ -5434,45 +5434,68 @@ bool FunctionEmitter::MakeVectorInsertDynamic(
const spvtools::opt::Instruction& inst) {
// For
// %result = OpVectorInsertDynamic %type %src_vector %component %index
// generate statements like this:
// there are two cases.
//
// Case 1:
// The %src_vector value has already been hoisted into a variable.
// In this case, assign %src_vector to that variable, then write the
// component into the right spot:
//
// hoisted = src_vector;
// hoisted[index] = component;
//
// Case 2:
// The %src_vector value is not hoisted. In this case, make a temporary
// variable with the %src_vector contents, then write the component,
// and then make a let-declaration that reads the value out:
//
// var temp : type = src_vector;
// temp[index] = component;
// let result : type = temp;
//
// Then use result everywhere the original SPIR-V id is used. Using a const
// like this avoids constantly reloading the value many times.
// Then use result everywhere the original SPIR-V id is used. Using a const
// like this avoids constantly reloading the value many times.
// TODO(dneto): crbug.com/tint/804: handle the case where %src_vector is
// has been hoisted into a variable.
auto* ast_type = parser_impl_.ConvertType(inst.type_id());
auto* type = parser_impl_.ConvertType(inst.type_id());
auto src_vector = MakeOperand(inst, 0);
auto component = MakeOperand(inst, 1);
auto index = MakeOperand(inst, 2);
// Synthesize the temporary variable.
// It doesn't correspond to a SPIR-V ID, so we don't use the ordinary
// API in parser_impl_.
auto result_name = namer_.Name(inst.result_id());
auto temp_name = namer_.MakeDerivedName(result_name);
auto registered_temp_name = builder_.Symbols().Register(temp_name);
std::string var_name;
auto original_value_name = namer_.Name(inst.result_id());
const bool hoisted = WriteIfHoistedVar(inst, src_vector);
if (hoisted) {
// The variable was already declared in an earlier block.
var_name = original_value_name;
// Assign the source vector value to it.
builder_.Assign({}, builder_.Expr(var_name), src_vector.expr);
} else {
// Synthesize the temporary variable.
// It doesn't correspond to a SPIR-V ID, so we don't use the ordinary
// API in parser_impl_.
var_name = namer_.MakeDerivedName(original_value_name);
auto* temp_var = create<ast::Variable>(
Source{}, registered_temp_name, ast::StorageClass::kNone,
ast::Access::kUndefined, ast_type->Build(builder_), false,
src_vector.expr, ast::DecorationList{});
AddStatement(create<ast::VariableDeclStatement>(Source{}, temp_var));
auto* temp_var = builder_.Var(var_name, type->Build(builder_),
ast::StorageClass::kNone, src_vector.expr);
AddStatement(builder_.Decl({}, temp_var));
}
auto* lhs = create<ast::ArrayAccessorExpression>(
Source{}, create<ast::IdentifierExpression>(registered_temp_name),
index.expr);
Source{}, builder_.Expr(var_name), index.expr);
if (!lhs) {
return false;
}
AddStatement(create<ast::AssignmentStatement>(Source{}, lhs, component.expr));
AddStatement(builder_.Assign(lhs, component.expr));
return EmitConstDefinition(
inst,
{ast_type, create<ast::IdentifierExpression>(registered_temp_name)});
if (hoisted) {
// The hoisted variable itself stands for this result ID.
return success();
}
// Create a new let-declaration that is initialized by the contents
// of the temporary variable.
return EmitConstDefinition(inst, {type, builder_.Expr(var_name)});
}
bool FunctionEmitter::MakeCompositeInsert(
@ -5538,8 +5561,7 @@ bool FunctionEmitter::MakeCompositeInsert(
return false;
}
AddStatement(
create<ast::AssignmentStatement>(Source{}, lhs.expr, component.expr));
AddStatement(builder_.Assign(lhs.expr, component.expr));
if (hoisted) {
// The hoisted variable itself stands for this result ID.

90
src/reader/spirv/function_var_test.cc
View File

@ -60,6 +60,8 @@ std::string CommonTypes() {
%uint_1 = OpConstant %uint 1
%int_m1 = OpConstant %int -1
%int_0 = OpConstant %int 0
%int_1 = OpConstant %int 1
%int_3 = OpConstant %int 3
%uint_2 = OpConstant %uint 2
%uint_3 = OpConstant %uint 3
%uint_4 = OpConstant %uint 4
@ -2286,7 +2288,7 @@ TEST_F(SpvParserFunctionVarTest,
auto got = ToString(p->builder(), fe.ast_body());
auto* expect = R"(VariableDeclStatement{
Variable{
x_38_phi
x_41_phi
none
undefined
__u32
@ -2312,14 +2314,14 @@ Switch{
Else{
{
Assignment{
Identifier[not set]{x_38_phi}
Identifier[not set]{x_41_phi}
ScalarConstructor[not set]{0u}
}
Break{}
}
}
Assignment{
Identifier[not set]{x_38_phi}
Identifier[not set]{x_41_phi}
ScalarConstructor[not set]{1u}
}
}
@ -2327,12 +2329,12 @@ Switch{
}
VariableDeclStatement{
VariableConst{
x_38
x_41
none
undefined
__u32
{
Identifier[not set]{x_38_phi}
Identifier[not set]{x_41_phi}
}
}
}
@ -2602,6 +2604,84 @@ Return{}
EXPECT_EQ(got, expected);
}
TEST_F(SpvParserFunctionVarTest, EmitStatement_Hoist_VectorInsertDynamic) {
// Spawned from crbug.com/tint/804
const auto assembly = Preamble() + R"(
%100 = OpFunction %void None %voidfn
%10 = OpLabel
OpSelectionMerge %50 None
OpBranchConditional %true %20 %30
%20 = OpLabel
%200 = OpVectorInsertDynamic %v2int %v2int_null %int_3 %int_1
OpBranch %50
%30 = OpLabel
OpReturn
%50 = OpLabel ; dominated by %20, but %200 needs to be hoisted
%201 = OpCopyObject %v2int %200
OpReturn
OpFunctionEnd
)";
auto p = parser(test::Assemble(assembly));
ASSERT_TRUE(p->BuildAndParseInternalModule()) << p->error() << assembly;
auto fe = p->function_emitter(100);
EXPECT_TRUE(fe.EmitBody()) << p->error();
const auto got = ToString(p->builder(), fe.ast_body());
const auto* expected = R"(VariableDeclStatement{
Variable{
x_200
none
undefined
__vec_2__i32
}
}
If{
(
ScalarConstructor[not set]{true}
)
{
Assignment{
Identifier[not set]{x_200}
TypeConstructor[not set]{
__vec_2__i32
ScalarConstructor[not set]{0}
ScalarConstructor[not set]{0}
}
}
Assignment{
ArrayAccessor[not set]{
Identifier[not set]{x_200}
ScalarConstructor[not set]{1}
}
ScalarConstructor[not set]{3}
}
}
}
Else{
{
Return{}
}
}
VariableDeclStatement{
VariableConst{
x_201
none
undefined
__vec_2__i32
{
Identifier[not set]{x_200}
}
}
}
Return{}
)";
EXPECT_EQ(got, expected) << got;
}
} // namespace
} // namespace spirv
} // namespace reader