writer/spirv: Fix static array accessors
If the array accessor expression uses a literal index, generate an
OpCompositeExtract instruction. Dynamic indices will be handled in a
follow-up patch.
Fixed: tint:767
Change-Id: I79a980d7d558a49def30816d04dcaa566f284c8f
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49701
Reviewed-by: David Neto <dneto@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: James Price <jrprice@google.com>
Auto-Submit: James Price <jrprice@google.com>
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index cc2d184..defc9b0 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -839,21 +839,48 @@
return false;
}
- // We don't have a pointer, so we have to extract value from the vector
+ // We don't have a pointer, so we can just directly extract the value.
auto extract = result_op();
auto extract_id = extract.to_i();
- if (!push_function_inst(
- spv::Op::OpVectorExtractDynamic,
- {Operand::Int(result_type_id), extract, Operand::Int(info->source_id),
- Operand::Int(idx_id)})) {
- return false;
+ // If the index is a literal, we use OpCompositeExtract.
+ if (auto* scalar = expr->idx_expr()->As<ast::ScalarConstructorExpression>()) {
+ auto* literal = scalar->literal()->As<ast::IntLiteral>();
+ if (!literal) {
+ TINT_ICE(builder_.Diagnostics()) << "bad literal in array accessor";
+ return false;
+ }
+
+ if (!push_function_inst(spv::Op::OpCompositeExtract,
+ {Operand::Int(result_type_id), extract,
+ Operand::Int(info->source_id),
+ Operand::Int(literal->value_as_u32())})) {
+ return false;
+ }
+
+ info->source_id = extract_id;
+ info->source_type = TypeOf(expr);
+
+ return true;
}
- info->source_id = extract_id;
- info->source_type = TypeOf(expr);
+ // If the source is a vector, we use OpVectorExtractDynamic.
+ if (info->source_type->Is<sem::Vector>()) {
+ if (!push_function_inst(
+ spv::Op::OpVectorExtractDynamic,
+ {Operand::Int(result_type_id), extract,
+ Operand::Int(info->source_id), Operand::Int(idx_id)})) {
+ return false;
+ }
- return true;
+ info->source_id = extract_id;
+ info->source_type = TypeOf(expr);
+
+ return true;
+ }
+
+ TINT_ICE(builder_.Diagnostics()) << "unsupported array accessor expression";
+ return false;
}
bool Builder::GenerateMemberAccessor(ast::MemberAccessorExpression* expr,
diff --git a/src/writer/spirv/builder_accessor_expression_test.cc b/src/writer/spirv/builder_accessor_expression_test.cc
index adcb694..d0c409b 100644
--- a/src/writer/spirv/builder_accessor_expression_test.cc
+++ b/src/writer/spirv/builder_accessor_expression_test.cc
@@ -676,7 +676,7 @@
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
R"(%6 = OpLoad %3 %1
%7 = OpVectorShuffle %3 %6 %6 1 0 2
-%10 = OpVectorExtractDynamic %4 %7 %9
+%10 = OpCompositeExtract %4 %7 1
)");
}
@@ -806,7 +806,7 @@
spirv::Builder& b = Build();
b.push_function(Function{});
- ASSERT_TRUE(b.GenerateFunctionVariable(var)) << b.error();
+ ASSERT_TRUE(b.GenerateGlobalVariable(var)) << b.error();
EXPECT_EQ(b.GenerateAccessorExpression(expr), 8u) << b.error();
EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
@@ -819,16 +819,89 @@
)");
EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), "");
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
- R"(%8 = OpVectorExtractDynamic %2 %5 %7
+ R"(%8 = OpCompositeExtract %2 %5 1
)");
}
-TEST_F(BuilderTest, DISABLED_Accessor_Array_NonPointer) {
+TEST_F(BuilderTest, Accessor_Const_Vec_Dynamic) {
+ // let pos : vec2<f32> = vec2<f32>(0.0, 0.5);
+ // idx : i32
+ // pos[idx]
+
+ auto* var = GlobalConst("pos", ty.vec2<f32>(), vec2<f32>(0.0f, 0.5f));
+
+ auto* idx = Var("idx", ty.i32(), ast::StorageClass::kFunction);
+ auto* expr = IndexAccessor("pos", idx);
+
+ ast::StatementList body;
+ body.push_back(WrapInStatement(idx));
+ body.push_back(WrapInStatement(expr));
+ WrapInFunction(body);
+
+ spirv::Builder& b = Build();
+
+ b.push_function(Function{});
+ ASSERT_TRUE(b.GenerateGlobalVariable(var)) << b.error();
+ ASSERT_TRUE(b.GenerateFunctionVariable(idx)) << b.error();
+ EXPECT_EQ(b.GenerateAccessorExpression(expr), 11u) << b.error();
+
+ EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
+%1 = OpTypeVector %2 2
+%3 = OpConstant %2 0
+%4 = OpConstant %2 0.5
+%5 = OpConstantComposite %1 %3 %4
+%8 = OpTypeInt 32 1
+%7 = OpTypePointer Function %8
+%9 = OpConstantNull %8
+)");
+ EXPECT_EQ(DumpInstructions(b.functions()[0].variables()),
+ R"(%6 = OpVariable %7 Function %9
+)");
+ EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
+ R"(%10 = OpLoad %8 %6
+%11 = OpVectorExtractDynamic %2 %5 %10
+)");
+}
+
+TEST_F(BuilderTest, Accessor_Array_NonPointer) {
// let a : array<f32, 3>;
// a[2]
+
+ auto* var = GlobalConst("a", ty.array<f32, 3>(),
+ Construct(ty.array<f32, 3>(), 0.0f, 0.5f, 1.0f));
+ auto* expr = IndexAccessor("a", 2);
+ WrapInFunction(expr);
+
+ spirv::Builder& b = Build();
+
+ b.push_function(Function{});
+ ASSERT_TRUE(b.GenerateGlobalVariable(var)) << b.error();
+ EXPECT_EQ(b.GenerateAccessorExpression(expr), 11u) << b.error();
+
+ EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
+%3 = OpTypeInt 32 0
+%4 = OpConstant %3 3
+%1 = OpTypeArray %2 %4
+%5 = OpConstant %2 0
+%6 = OpConstant %2 0.5
+%7 = OpConstant %2 1
+%8 = OpConstantComposite %1 %5 %6 %7
+%9 = OpTypeInt 32 1
+%10 = OpConstant %9 2
+)");
+ EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), "");
+ EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
+ R"(%11 = OpCompositeExtract %2 %8 2
+)");
+}
+
+TEST_F(BuilderTest, DISABLED_Accessor_Array_NonPointer_Dynamic) {
+ // let a : array<f32, 3>;
+ // idx : i32
+ // a[idx]
//
- // This has to generate an OpConstantExtract and will need to read the 3 value
- // out of the ScalarConstructor as extract requires integer indices.
+ // This needs to copy the array to an OpVariable in the Function storage class
+ // and then access chain into it and load the result.
}
} // namespace
