Use min for bounded array accessing.
This CL updates the BoundArrayAccess transform to use `min(u32(val),
size)` instead of `clamp(val, 0, size)` so as to reduce the number of
instructions needed to clamp within range.
Bug: tint:285
Change-Id: Ic12bd67f3d755c8e52590f0585bac114ba9eaa94
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/31360
Commit-Queue: David Neto <dneto@google.com>
Reviewed-by: David Neto <dneto@google.com>
diff --git a/src/transform/bound_array_accessors_transform.cc b/src/transform/bound_array_accessors_transform.cc
index 39ace7e..f0f6985 100644
--- a/src/transform/bound_array_accessors_transform.cc
+++ b/src/transform/bound_array_accessors_transform.cc
@@ -237,16 +237,17 @@
} else {
auto* u32 = ctx_->type_mgr().Get(std::make_unique<ast::type::U32Type>());
+ ast::ExpressionList cast_expr;
+ cast_expr.push_back(expr->take_idx_expr());
+
ast::ExpressionList params;
- params.push_back(expr->take_idx_expr());
- params.push_back(std::make_unique<ast::ScalarConstructorExpression>(
- std::make_unique<ast::UintLiteral>(u32, 0)));
+ params.push_back(std::make_unique<ast::TypeConstructorExpression>(
+ u32, std::move(cast_expr)));
params.push_back(std::make_unique<ast::ScalarConstructorExpression>(
std::make_unique<ast::UintLiteral>(u32, size - 1)));
auto call_expr = std::make_unique<ast::CallExpression>(
- std::make_unique<ast::IdentifierExpression>("clamp"),
- std::move(params));
+ std::make_unique<ast::IdentifierExpression>("min"), std::move(params));
call_expr->set_result_type(u32);
expr->set_idx_expr(std::move(call_expr));
diff --git a/src/transform/bound_array_accessors_transform_test.cc b/src/transform/bound_array_accessors_transform_test.cc
index b1b774d..b235a14 100644
--- a/src/transform/bound_array_accessors_transform_test.cc
+++ b/src/transform/bound_array_accessors_transform_test.cc
@@ -35,6 +35,7 @@
#include "src/ast/type/u32_type.h"
#include "src/ast/type/vector_type.h"
#include "src/ast/type/void_type.h"
+#include "src/ast/type_constructor_expression.h"
#include "src/ast/uint_literal.h"
#include "src/ast/variable.h"
#include "src/ast/variable_decl_statement.h"
@@ -82,7 +83,7 @@
// const c : u32 = 1;
// const b : ptr<function, f32> = a[c]
//
- // -> const b : ptr<function, i32> = a[clamp(c, 0, 2)]
+ // -> const b : ptr<function, i32> = a[min(u32(c), 2)]
ast::type::F32Type f32;
ast::type::U32Type u32;
@@ -119,21 +120,21 @@
auto* idx = ptr->idx_expr()->AsCall();
ASSERT_TRUE(idx->func()->IsIdentifier());
- EXPECT_EQ(idx->func()->AsIdentifier()->name(), "clamp");
+ EXPECT_EQ(idx->func()->AsIdentifier()->name(), "min");
- ASSERT_EQ(idx->params().size(), 3u);
- ASSERT_EQ(idx->params()[0].get(), access_ptr);
+ ASSERT_EQ(idx->params().size(), 2u);
+
+ ASSERT_TRUE(idx->params()[0]->IsConstructor());
+ ASSERT_TRUE(idx->params()[0]->AsConstructor()->IsTypeConstructor());
+ auto* tc = idx->params()[0]->AsConstructor()->AsTypeConstructor();
+ EXPECT_TRUE(tc->type()->IsU32());
+ ASSERT_EQ(tc->values().size(), 1u);
+ ASSERT_EQ(tc->values()[0].get(), access_ptr);
ASSERT_TRUE(idx->params()[1]->IsConstructor());
ASSERT_TRUE(idx->params()[1]->AsConstructor()->IsScalarConstructor());
auto* scalar = idx->params()[1]->AsConstructor()->AsScalarConstructor();
ASSERT_TRUE(scalar->literal()->IsUint());
- EXPECT_EQ(scalar->literal()->AsUint()->value(), 0u);
-
- ASSERT_TRUE(idx->params()[2]->IsConstructor());
- ASSERT_TRUE(idx->params()[2]->AsConstructor()->IsScalarConstructor());
- scalar = idx->params()[2]->AsConstructor()->AsScalarConstructor();
- ASSERT_TRUE(scalar->literal()->IsUint());
EXPECT_EQ(scalar->literal()->AsUint()->value(), 2u);
ASSERT_NE(ptr->idx_expr()->result_type(), nullptr);
@@ -146,7 +147,7 @@
// var i : u32;
// var c : f32 = a[b[i]];
//
- // -> var c : f32 = a[clamp(b[clamp(i, 0, 4)], 0, 2)];
+ // -> var c : f32 = a[min(u32(b[min(u32(i), 4)]), 2)];
ast::type::F32Type f32;
ast::type::U32Type u32;
@@ -186,42 +187,41 @@
auto* idx = ptr->idx_expr()->AsCall();
ASSERT_TRUE(idx->func()->IsIdentifier());
- EXPECT_EQ(idx->func()->AsIdentifier()->name(), "clamp");
+ EXPECT_EQ(idx->func()->AsIdentifier()->name(), "min");
- ASSERT_EQ(idx->params().size(), 3u);
+ ASSERT_EQ(idx->params().size(), 2u);
- auto* sub = idx->params()[0].get();
+ ASSERT_TRUE(idx->params()[0]->IsConstructor());
+ ASSERT_TRUE(idx->params()[0]->AsConstructor()->IsTypeConstructor());
+ auto* tc = idx->params()[0]->AsConstructor()->AsTypeConstructor();
+ EXPECT_TRUE(tc->type()->IsU32());
+ ASSERT_EQ(tc->values().size(), 1u);
+
+ auto* sub = tc->values()[0].get();
ASSERT_TRUE(sub->IsArrayAccessor());
ASSERT_TRUE(sub->AsArrayAccessor()->idx_expr()->IsCall());
auto* sub_idx = sub->AsArrayAccessor()->idx_expr()->AsCall();
ASSERT_TRUE(sub_idx->func()->IsIdentifier());
- EXPECT_EQ(sub_idx->func()->AsIdentifier()->name(), "clamp");
+ EXPECT_EQ(sub_idx->func()->AsIdentifier()->name(), "min");
- ASSERT_EQ(sub_idx->params()[0].get(), b_access_ptr);
+ ASSERT_TRUE(sub_idx->params()[0]->IsConstructor());
+ ASSERT_TRUE(sub_idx->params()[0]->AsConstructor()->IsTypeConstructor());
+ tc = sub_idx->params()[0]->AsConstructor()->AsTypeConstructor();
+ EXPECT_TRUE(tc->type()->IsU32());
+ ASSERT_EQ(tc->values().size(), 1u);
+ ASSERT_EQ(tc->values()[0].get(), b_access_ptr);
ASSERT_TRUE(sub_idx->params()[1]->IsConstructor());
ASSERT_TRUE(sub_idx->params()[1]->AsConstructor()->IsScalarConstructor());
auto* scalar = sub_idx->params()[1]->AsConstructor()->AsScalarConstructor();
ASSERT_TRUE(scalar->literal()->IsUint());
- EXPECT_EQ(scalar->literal()->AsUint()->value(), 0u);
-
- ASSERT_TRUE(sub_idx->params()[2]->IsConstructor());
- ASSERT_TRUE(sub_idx->params()[2]->AsConstructor()->IsScalarConstructor());
- scalar = sub_idx->params()[2]->AsConstructor()->AsScalarConstructor();
- ASSERT_TRUE(scalar->literal()->IsUint());
EXPECT_EQ(scalar->literal()->AsUint()->value(), 4u);
ASSERT_TRUE(idx->params()[1]->IsConstructor());
ASSERT_TRUE(idx->params()[1]->AsConstructor()->IsScalarConstructor());
scalar = idx->params()[1]->AsConstructor()->AsScalarConstructor();
ASSERT_TRUE(scalar->literal()->IsUint());
- EXPECT_EQ(scalar->literal()->AsUint()->value(), 0u);
-
- ASSERT_TRUE(idx->params()[2]->IsConstructor());
- ASSERT_TRUE(idx->params()[2]->AsConstructor()->IsScalarConstructor());
- scalar = idx->params()[2]->AsConstructor()->AsScalarConstructor();
- ASSERT_TRUE(scalar->literal()->IsUint());
EXPECT_EQ(scalar->literal()->AsUint()->value(), 2u);
ASSERT_NE(ptr->idx_expr()->result_type(), nullptr);
@@ -273,7 +273,7 @@
// var c : u32;
// var b : f32 = a[c + 2 - 3]
//
- // -> var b : f32 = a[clamp((c + 2 - 3), 0, 2)]
+ // -> var b : f32 = a[min(u32(c + 2 - 3), 2)]
ast::type::F32Type f32;
ast::type::U32Type u32;
@@ -312,21 +312,21 @@
auto* idx = ptr->idx_expr()->AsCall();
ASSERT_TRUE(idx->func()->IsIdentifier());
- EXPECT_EQ(idx->func()->AsIdentifier()->name(), "clamp");
+ EXPECT_EQ(idx->func()->AsIdentifier()->name(), "min");
- ASSERT_EQ(idx->params().size(), 3u);
- ASSERT_EQ(idx->params()[0].get(), access_ptr);
+ ASSERT_EQ(idx->params().size(), 2u);
+
+ ASSERT_TRUE(idx->params()[0]->IsConstructor());
+ ASSERT_TRUE(idx->params()[0]->AsConstructor()->IsTypeConstructor());
+ auto* tc = idx->params()[0]->AsConstructor()->AsTypeConstructor();
+ EXPECT_TRUE(tc->type()->IsU32());
+ ASSERT_EQ(tc->values().size(), 1u);
+ ASSERT_EQ(tc->values()[0].get(), access_ptr);
ASSERT_TRUE(idx->params()[1]->IsConstructor());
ASSERT_TRUE(idx->params()[1]->AsConstructor()->IsScalarConstructor());
auto* scalar = idx->params()[1]->AsConstructor()->AsScalarConstructor();
ASSERT_TRUE(scalar->literal()->IsUint());
- EXPECT_EQ(scalar->literal()->AsUint()->value(), 0u);
-
- ASSERT_TRUE(idx->params()[2]->IsConstructor());
- ASSERT_TRUE(idx->params()[2]->AsConstructor()->IsScalarConstructor());
- scalar = idx->params()[2]->AsConstructor()->AsScalarConstructor();
- ASSERT_TRUE(scalar->literal()->IsUint());
EXPECT_EQ(scalar->literal()->AsUint()->value(), 2u);
ASSERT_NE(ptr->idx_expr()->result_type(), nullptr);
@@ -458,7 +458,7 @@
// var c : u32;
// var b : f32 = a[c + 2 - 3]
//
- // -> var b : f32 = a[clamp((c + 2 - 3), 0, 2)]
+ // -> var b : f32 = a[min(u32(c + 2 - 3), 2)]
ast::type::F32Type f32;
ast::type::U32Type u32;
@@ -497,21 +497,20 @@
auto* idx = ptr->idx_expr()->AsCall();
ASSERT_TRUE(idx->func()->IsIdentifier());
- EXPECT_EQ(idx->func()->AsIdentifier()->name(), "clamp");
+ EXPECT_EQ(idx->func()->AsIdentifier()->name(), "min");
- ASSERT_EQ(idx->params().size(), 3u);
- ASSERT_EQ(idx->params()[0].get(), access_ptr);
+ ASSERT_EQ(idx->params().size(), 2u);
+ ASSERT_TRUE(idx->params()[0]->IsConstructor());
+ ASSERT_TRUE(idx->params()[0]->AsConstructor()->IsTypeConstructor());
+ auto* tc = idx->params()[0]->AsConstructor()->AsTypeConstructor();
+ EXPECT_TRUE(tc->type()->IsU32());
+ ASSERT_EQ(tc->values().size(), 1u);
+ ASSERT_EQ(tc->values()[0].get(), access_ptr);
ASSERT_TRUE(idx->params()[1]->IsConstructor());
ASSERT_TRUE(idx->params()[1]->AsConstructor()->IsScalarConstructor());
auto* scalar = idx->params()[1]->AsConstructor()->AsScalarConstructor();
ASSERT_TRUE(scalar->literal()->IsUint());
- EXPECT_EQ(scalar->literal()->AsUint()->value(), 0u);
-
- ASSERT_TRUE(idx->params()[2]->IsConstructor());
- ASSERT_TRUE(idx->params()[2]->AsConstructor()->IsScalarConstructor());
- scalar = idx->params()[2]->AsConstructor()->AsScalarConstructor();
- ASSERT_TRUE(scalar->literal()->IsUint());
EXPECT_EQ(scalar->literal()->AsUint()->value(), 2u);
ASSERT_NE(ptr->idx_expr()->result_type(), nullptr);
@@ -659,7 +658,7 @@
// var c : u32;
// var b : f32 = a[c + 2 - 3][1]
//
- // -> var b : f32 = a[clamp((c + 2 - 3), 0, 2)][1]
+ // -> var b : f32 = a[min(u32(c + 2 - 3), 2)][1]
ast::type::F32Type f32;
ast::type::U32Type u32;
@@ -705,21 +704,21 @@
ASSERT_TRUE(ary->idx_expr()->IsCall());
auto* idx = ary->idx_expr()->AsCall();
ASSERT_TRUE(idx->func()->IsIdentifier());
- EXPECT_EQ(idx->func()->AsIdentifier()->name(), "clamp");
+ EXPECT_EQ(idx->func()->AsIdentifier()->name(), "min");
- ASSERT_EQ(idx->params().size(), 3u);
- ASSERT_EQ(idx->params()[0].get(), access_ptr);
+ ASSERT_EQ(idx->params().size(), 2u);
+
+ ASSERT_TRUE(idx->params()[0]->IsConstructor());
+ ASSERT_TRUE(idx->params()[0]->AsConstructor()->IsTypeConstructor());
+ auto* tc = idx->params()[0]->AsConstructor()->AsTypeConstructor();
+ EXPECT_TRUE(tc->type()->IsU32());
+ ASSERT_EQ(tc->values().size(), 1u);
+ ASSERT_EQ(tc->values()[0].get(), access_ptr);
ASSERT_TRUE(idx->params()[1]->IsConstructor());
ASSERT_TRUE(idx->params()[1]->AsConstructor()->IsScalarConstructor());
auto* scalar = idx->params()[1]->AsConstructor()->AsScalarConstructor();
ASSERT_TRUE(scalar->literal()->IsUint());
- EXPECT_EQ(scalar->literal()->AsUint()->value(), 0u);
-
- ASSERT_TRUE(idx->params()[2]->IsConstructor());
- ASSERT_TRUE(idx->params()[2]->AsConstructor()->IsScalarConstructor());
- scalar = idx->params()[2]->AsConstructor()->AsScalarConstructor();
- ASSERT_TRUE(scalar->literal()->IsUint());
EXPECT_EQ(scalar->literal()->AsUint()->value(), 2u);
ASSERT_NE(ary->idx_expr()->result_type(), nullptr);
@@ -741,7 +740,7 @@
// var c : u32;
// var b : f32 = a[1][c + 2 - 3]
//
- // -> var b : f32 = a[1][clamp((c + 2 - 3), 0, 1)]
+ // -> var b : f32 = a[1][min(u32(c + 2 - 3), 1)]
ast::type::F32Type f32;
ast::type::U32Type u32;
@@ -794,21 +793,21 @@
ASSERT_TRUE(ptr->idx_expr()->IsCall());
auto* idx = ptr->idx_expr()->AsCall();
ASSERT_TRUE(idx->func()->IsIdentifier());
- EXPECT_EQ(idx->func()->AsIdentifier()->name(), "clamp");
+ EXPECT_EQ(idx->func()->AsIdentifier()->name(), "min");
- ASSERT_EQ(idx->params().size(), 3u);
- ASSERT_EQ(idx->params()[0].get(), access_ptr);
+ ASSERT_EQ(idx->params().size(), 2u);
+
+ ASSERT_TRUE(idx->params()[0]->IsConstructor());
+ ASSERT_TRUE(idx->params()[0]->AsConstructor()->IsTypeConstructor());
+ auto* tc = idx->params()[0]->AsConstructor()->AsTypeConstructor();
+ EXPECT_TRUE(tc->type()->IsU32());
+ ASSERT_EQ(tc->values().size(), 1u);
+ ASSERT_EQ(tc->values()[0].get(), access_ptr);
ASSERT_TRUE(idx->params()[1]->IsConstructor());
ASSERT_TRUE(idx->params()[1]->AsConstructor()->IsScalarConstructor());
scalar = idx->params()[1]->AsConstructor()->AsScalarConstructor();
ASSERT_TRUE(scalar->literal()->IsUint());
- EXPECT_EQ(scalar->literal()->AsUint()->value(), 0u);
-
- ASSERT_TRUE(idx->params()[2]->IsConstructor());
- ASSERT_TRUE(idx->params()[2]->AsConstructor()->IsScalarConstructor());
- scalar = idx->params()[2]->AsConstructor()->AsScalarConstructor();
- ASSERT_TRUE(scalar->literal()->IsUint());
EXPECT_EQ(scalar->literal()->AsUint()->value(), 1u);
ASSERT_NE(ary->idx_expr()->result_type(), nullptr);
@@ -1046,7 +1045,7 @@
// var a : vec3<f32>
// var b : f32 = a[idx]
//
- // ->var b : f32 = a[clamp(idx, 0, 2)]
+ // ->var b : f32 = a[min(u32(idx), 2)]
}
// TODO(dsinclair): Implement when constant_id exists
@@ -1055,7 +1054,7 @@
// var a : array<f32, 4>
// var b : f32 = a[idx]
//
- // -> var b : f32 = a[clamp(idx, 0, 3)]
+ // -> var b : f32 = a[min(u32(idx), 3)]
}
// TODO(dsinclair): Implement when constant_id exists
@@ -1064,7 +1063,7 @@
// var a : mat3x2<f32>
// var b : f32 = a[idx][1]
//
- // -> var b : f32 = a[clamp(idx, 0, 2)][1]
+ // -> var b : f32 = a[min(u32(idx), 2)][1]
}
// TODO(dsinclair): Implement when constant_id exists
@@ -1073,7 +1072,7 @@
// var a : mat3x2<f32>
// var b : f32 = a[1][idx]
//
- // -> var b : f32 = a[1][clamp(idx, 0, 1)]
+ // -> var b : f32 = a[1][min(u32(idx), 0, 1)]
}
// TODO(dsinclair): Implement when we have arrayLength for Runtime Arrays
@@ -1085,7 +1084,7 @@
// S s;
// var b : f32 = s.b[25]
//
- // -> var b : f32 = s.b[clamp(25, 0, arrayLength(s.b))]
+ // -> var b : f32 = s.b[min(u32(25), arrayLength(s.b))]
}
// TODO(dsinclair): Clamp atomics when available.
@@ -1109,8 +1108,8 @@
// }
// var c : f32 = a[i];
//
- // -> var b : f32 = a[clamp(i, 0, 4)];
- // var c : f32 = a[clamp(i, 0, 2)];
+ // -> var b : f32 = a[min(u32(i), 4)];
+ // var c : f32 = a[min(u32(i), 2)];
FAIL();
}
