Skip to content

Commit

Permalink
tint: Fix / remove tests that access OOB
Browse files Browse the repository at this point in the history 
With constant indices.
Also fix the SPIR-V builder tests that did something completely different from the test name / comment.

Bug: tint:1665
Change-Id: I83537cf6e44ffcb14d54de52649d1f9da1ef7e1b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/101182
Commit-Queue: Ben Clayton <[email protected]>
Reviewed-by: David Neto <[email protected]>
Kokoro: Kokoro <[email protected]>
  • Loading branch information
@ben-clayton
ben-clayton authored and Dawn LUCI CQ committed Sep 6, 2022
1 parent 2b0fce7 commit 8b4ed50
Show file tree
Hide file tree
Showing 15 changed files with 67 additions and 1,188 deletions.
4 changes: 2 additions & 2 deletions src/tint/resolver/array_accessor_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ TEST_F(ResolverIndexAccessorTest, Matrix_BothDimension_Dynamic) {
TEST_F(ResolverIndexAccessorTest, Matrix) {
GlobalVar("my_var", ty.mat2x3<f32>(), ast::StorageClass::kPrivate);

auto* acc = IndexAccessor("my_var", 2_i);
auto* acc = IndexAccessor("my_var", 1_i);
WrapInFunction(acc);

EXPECT_TRUE(r()->Resolve()) << r()->error();
Expand All @@ -123,7 +123,7 @@ TEST_F(ResolverIndexAccessorTest, Matrix) {
TEST_F(ResolverIndexAccessorTest, Matrix_BothDimensions) {
GlobalVar("my_var", ty.mat2x3<f32>(), ast::StorageClass::kPrivate);

auto* acc = IndexAccessor(IndexAccessor("my_var", 2_i), 1_i);
auto* acc = IndexAccessor(IndexAccessor("my_var", 0_i), 1_i);
WrapInFunction(acc);

EXPECT_TRUE(r()->Resolve()) << r()->error();
Expand Down
42 changes: 31 additions & 11 deletions src/tint/resolver/materialize_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -954,14 +954,6 @@ constexpr Method kScalarMethods[] = {
Method::kBitcastF32Arg,
};

/// Methods that support abstract-integer materialization
/// Note: Doesn't contain kWorkgroupSize or kArrayLength as they have tighter constraints on the
/// range of allowed integer values.
constexpr Method kAIntMethods[] = {
Method::kSwitch,
Method::kIndex,
};

/// Methods that support vector materialization
constexpr Method kVectorMethods[] = {
Method::kLet, Method::kVar, Method::kBuiltinArg, Method::kBitcastVec3F32Arg,
Expand Down Expand Up @@ -1039,11 +1031,36 @@ INSTANTIATE_TEST_SUITE_P(
Types<f32M, AFloatM>(AFloat(-kSubnormalF32), -kSubnormalF32), //
})));

INSTANTIATE_TEST_SUITE_P(MaterializeAInt,
MaterializeAbstractNumericToDefaultType,
testing::Combine(testing::Values(Expectation::kMaterialize),
testing::Values(Method::kWorkgroupSize,
Method::kArrayLength),
testing::ValuesIn(std::vector<Data>{
Types<i32, AInt>(1_a, 1.0), //
Types<i32, AInt>(10_a, 10.0), //
Types<i32, AInt>(100_a, 100.0), //
Types<i32, AInt>(1000_a, 1000.0), //
Types<i32, AInt>(10000_a, 10000.0), //
Types<i32, AInt>(65535_a, 65535.0), //
})));

INSTANTIATE_TEST_SUITE_P(MaterializeAIntIndex,
MaterializeAbstractNumericToDefaultType,
testing::Combine(testing::Values(Expectation::kMaterialize),
testing::Values(Method::kIndex),
testing::ValuesIn(std::vector<Data>{
Types<i32, AInt>(0_a, 0.0), //
Types<i32, AInt>(1_a, 1.0), //
Types<i32, AInt>(2_a, 2.0), //
Types<i32, AInt>(3_a, 3.0), //
})));

INSTANTIATE_TEST_SUITE_P(
MaterializeAInt,
MaterializeAIntSwitch,
MaterializeAbstractNumericToDefaultType,
testing::Combine(testing::Values(Expectation::kMaterialize),
testing::ValuesIn(kAIntMethods),
testing::Values(Method::kSwitch),
testing::ValuesIn(std::vector<Data>{
Types<i32, AInt>(0_a, 0.0), //
Types<i32, AInt>(10_a, 10.0), //
Expand Down Expand Up @@ -1133,7 +1150,10 @@ INSTANTIATE_TEST_SUITE_P(
AIntValueCannotBeRepresented,
MaterializeAbstractNumericToDefaultType,
testing::Combine(testing::Values(Expectation::kValueCannotBeRepresented),
testing::ValuesIn(kAIntMethods),
testing::Values(Method::kWorkgroupSize,
Method::kArrayLength,
Method::kSwitch,
Method::kIndex),
testing::ValuesIn(std::vector<Data>{
Types<i32, AInt>(0_a, static_cast<double>(i32::kHighestValue) + 1), //
Types<i32, AInt>(0_a, static_cast<double>(i32::kLowestValue) - 1), //
Expand Down
4 changes: 2 additions & 2 deletions src/tint/resolver/variable_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,14 +466,14 @@ TEST_F(ResolverVariableTest, LocalLet_InheritsAccessFromOriginatingVariable) {
// }
// @group(0) @binding(0) var<storage, read_write> s : S;
// fn f() {
// let p = &s.inner.arr[4];
// let p = &s.inner.arr[3];
// }
auto* inner = Structure("Inner", utils::Vector{Member("arr", ty.array<i32, 4>())});
auto* buf = Structure("S", utils::Vector{Member("inner", ty.Of(inner))});
auto* storage = GlobalVar("s", ty.Of(buf), ast::StorageClass::kStorage, ast::Access::kReadWrite,
Binding(0_a), Group(0_a));

auto* expr = IndexAccessor(MemberAccessor(MemberAccessor(storage, "inner"), "arr"), 4_i);
auto* expr = IndexAccessor(MemberAccessor(MemberAccessor(storage, "inner"), "arr"), 3_i);
auto* ptr = Let("p", AddressOf(expr));

WrapInFunction(ptr);
Expand Down
16 changes: 8 additions & 8 deletions src/tint/transform/std140_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1584,7 +1584,7 @@ fn f() {
let l_a_3_a_1 : Inner = a[3].a[1];
let l_a_0_a_2_m : mat4x2<f32> = a[0].a[2].m;
let l_a_1_a_3_m_0 : vec2<f32> = a[1].a[3].m[0];
let l_a_2_a_0_m_1_2 : f32 = a[2].a[0].m[1][2];
let l_a_2_a_0_m_1_0 : f32 = a[2].a[0].m[1][0];
}
)";

Expand Down Expand Up @@ -1647,7 +1647,7 @@ fn f() {
let l_a_3_a_1 : Inner = conv_Inner(a[3u].a[1u]);
let l_a_0_a_2_m : mat4x2<f32> = load_a_0_a_2_m();
let l_a_1_a_3_m_0 : vec2<f32> = a[1u].a[3u].m_0;
let l_a_2_a_0_m_1_2 : f32 = a[2u].a[0u].m_1[2u];
let l_a_2_a_0_m_1_0 : f32 = a[2u].a[0u].m_1[0u];
}
)";

Expand Down Expand Up @@ -2029,7 +2029,7 @@ struct S {
@group(0) @binding(0) var<uniform> u : S;
fn f() {
for (var i = u32(u.m[0][1]); i < u32(u.m[i][2]); i += u32(u.m[1][i])) {
for (var i = u32(u.m[0][0]); i < u32(u.m[i][1]); i += u32(u.m[1][i])) {
}
}
)";
Expand All @@ -2050,16 +2050,16 @@ struct S_std140 {
@group(0) @binding(0) var<uniform> u : S_std140;
fn load_u_m_p0_2(p0 : u32) -> f32 {
fn load_u_m_p0_1(p0 : u32) -> f32 {
switch(p0) {
case 0u: {
return u.m_0[2u];
return u.m_0[1u];
}
case 1u: {
return u.m_1[2u];
return u.m_1[1u];
}
case 2u: {
return u.m_2[2u];
return u.m_2[1u];
}
default: {
return f32();
Expand All @@ -2068,7 +2068,7 @@ fn load_u_m_p0_2(p0 : u32) -> f32 {
}
fn f() {
for(var i = u32(u.m_0[1u]); (i < u32(load_u_m_p0_2(u32(i)))); i += u32(u.m_1[i])) {
for(var i = u32(u.m_0[0u]); (i < u32(load_u_m_p0_1(u32(i)))); i += u32(u.m_1[i])) {
}
}
)";
Expand Down
47 changes: 24 additions & 23 deletions src/tint/writer/spirv/builder_accessor_expression_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -653,11 +653,11 @@ OpReturn
Validate(b);
}

TEST_F(BuilderTest, Runtime_IndexAccessor_Nested_Array_f32) {
// var pos : array<array<f32, 2>, 3u>;
TEST_F(BuilderTest, Runtime_IndexAccessor_Array_Vec3_f32) {
// var pos : array<vec3<f32>, 3u>;
// var x = pos[1u][2u];

auto* pos = Var("pos", ty.array(ty.vec2<f32>(), 3_u));
auto* pos = Var("pos", ty.array(ty.vec3<f32>(), 3_a));
auto* x = Var("x", IndexAccessor(IndexAccessor(pos, 1_u), 2_u));
WrapInFunction(pos, x);

Expand All @@ -668,7 +668,7 @@ TEST_F(BuilderTest, Runtime_IndexAccessor_Nested_Array_f32) {
EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid
%1 = OpTypeFunction %2
%9 = OpTypeFloat 32
%8 = OpTypeVector %9 2
%8 = OpTypeVector %9 3
%10 = OpTypeInt 32 0
%11 = OpConstant %10 3
%7 = OpTypeArray %8 %11
Expand All @@ -693,11 +693,11 @@ OpReturn
}

TEST_F(BuilderTest, Dynamic_IndexAccessor_Nested_Array_f32) {
// var pos : array<array<f32, 2>, 3u>;
// var pos : array<array<f32, 4>, 3u>;
// var one = 1u;
// var x = pos[one][2u];

auto* pos = Var("pos", ty.array(ty.vec2<f32>(), 3_u));
auto* pos = Var("pos", ty.array(ty.array<f32, 4>(), 3_u));
auto* one = Var("one", Expr(2_u));
auto* x = Var("x", IndexAccessor(IndexAccessor(pos, "one"), 2_u));
WrapInFunction(pos, one, x);
Expand All @@ -709,27 +709,28 @@ TEST_F(BuilderTest, Dynamic_IndexAccessor_Nested_Array_f32) {
EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid
%1 = OpTypeFunction %2
%9 = OpTypeFloat 32
%8 = OpTypeVector %9 2
%10 = OpTypeInt 32 0
%11 = OpConstant %10 3
%7 = OpTypeArray %8 %11
%11 = OpConstant %10 4
%8 = OpTypeArray %9 %11
%12 = OpConstant %10 3
%7 = OpTypeArray %8 %12
%6 = OpTypePointer Function %7
%12 = OpConstantNull %7
%13 = OpConstant %10 2
%15 = OpTypePointer Function %10
%16 = OpConstantNull %10
%18 = OpTypePointer Function %9
%22 = OpConstantNull %9
%13 = OpConstantNull %7
%14 = OpConstant %10 2
%16 = OpTypePointer Function %10
%17 = OpConstantNull %10
%19 = OpTypePointer Function %9
%23 = OpConstantNull %9
)");
EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), R"(%5 = OpVariable %6 Function %12
%14 = OpVariable %15 Function %16
%21 = OpVariable %18 Function %22
EXPECT_EQ(DumpInstructions(b.functions()[0].variables()), R"(%5 = OpVariable %6 Function %13
%15 = OpVariable %16 Function %17
%22 = OpVariable %19 Function %23
)");
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()), R"(OpStore %14 %13
%17 = OpLoad %10 %14
%19 = OpAccessChain %18 %5 %17 %13
%20 = OpLoad %9 %19
OpStore %21 %20
EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()), R"(OpStore %15 %14
%18 = OpLoad %10 %15
%20 = OpAccessChain %19 %5 %18 %14
%21 = OpLoad %9 %20
OpStore %22 %21
OpReturn
)");

Expand Down
Loading

0 comments on commit 8b4ed50

Please sign in to comment.