Implement clamping of runtime array accesses
Bug: tint:252
Change-Id: I2b32ab9d69ca39b6178fc4e94ccd090516a37c98
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/36620
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: dan sinclair <dsinclair@chromium.org>
diff --git a/src/transform/bound_array_accessors.cc b/src/transform/bound_array_accessors.cc
index 4543a17..c7ce85e 100644
--- a/src/transform/bound_array_accessors.cc
+++ b/src/transform/bound_array_accessors.cc
@@ -14,6 +14,7 @@
#include "src/transform/bound_array_accessors.h"
+#include <algorithm>
#include <memory>
#include <utility>
@@ -22,6 +23,7 @@
#include "src/ast/bitcast_expression.h"
#include "src/ast/block_statement.h"
#include "src/ast/break_statement.h"
+#include "src/ast/builder.h"
#include "src/ast/call_expression.h"
#include "src/ast/call_statement.h"
#include "src/ast/case_statement.h"
@@ -74,47 +76,47 @@
return nullptr;
}
- uint32_t size = 0;
- if (ret_type->Is<ast::type::Vector>() || ret_type->Is<ast::type::Array>()) {
- size = ret_type->Is<ast::type::Vector>()
- ? ret_type->As<ast::type::Vector>()->size()
- : ret_type->As<ast::type::Array>()->size();
- if (size == 0) {
- diag::Diagnostic err;
- err.severity = diag::Severity::Error;
- err.message = "invalid 0 size for array or vector";
- err.source = expr->source();
- diags->add(std::move(err));
- return nullptr;
- }
+ ast::Builder b(ctx->mod);
+ using u32 = ast::Builder::u32;
+ uint32_t size = 0;
+ bool is_vec = ret_type->Is<ast::type::Vector>();
+ bool is_arr = ret_type->Is<ast::type::Array>();
+ if (is_vec || is_arr) {
+ size = is_vec ? ret_type->As<ast::type::Vector>()->size()
+ : ret_type->As<ast::type::Array>()->size();
} else {
// The row accessor would have been an embedded array accessor and already
// handled, so we just need to do columns here.
size = ret_type->As<ast::type::Matrix>()->columns();
}
- ast::Expression* idx_expr = nullptr;
+ auto* const old_idx = expr->idx_expr();
+ b.SetSource(ctx->Clone(old_idx->source()));
- // Scalar constructor we can re-write the value to be within bounds.
- if (auto* c = expr->idx_expr()->As<ast::ScalarConstructorExpression>()) {
+ ast::Expression* new_idx = nullptr;
+
+ if (size == 0) {
+ if (is_arr) {
+ auto* arr_len = b.Call("arrayLength", ctx->Clone(expr->array()));
+ auto* limit = b.Sub(arr_len, b.Expr(1u));
+ new_idx = b.Call("min", b.Construct<u32>(ctx->Clone(old_idx)), limit);
+ } else {
+ diag::Diagnostic err;
+ err.severity = diag::Severity::Error;
+ err.message = "invalid 0 size";
+ err.source = expr->source();
+ diags->add(std::move(err));
+ return nullptr;
+ }
+ } else if (auto* c = old_idx->As<ast::ScalarConstructorExpression>()) {
+ // Scalar constructor we can re-write the value to be within bounds.
auto* lit = c->literal();
if (auto* sint = lit->As<ast::SintLiteral>()) {
- int32_t val = sint->value();
- if (val < 0) {
- val = 0;
- } else if (val >= int32_t(size)) {
- val = int32_t(size) - 1;
- }
- lit = ctx->mod->create<ast::SintLiteral>(ctx->Clone(sint->source()),
- ctx->Clone(sint->type()), val);
+ int32_t max = static_cast<int32_t>(size) - 1;
+ new_idx = b.Expr(std::max(std::min(sint->value(), max), 0));
} else if (auto* uint = lit->As<ast::UintLiteral>()) {
- uint32_t val = uint->value();
- if (val >= size - 1) {
- val = size - 1;
- }
- lit = ctx->mod->create<ast::UintLiteral>(ctx->Clone(uint->source()),
- ctx->Clone(uint->type()), val);
+ new_idx = b.Expr(std::min(uint->value(), size - 1));
} else {
diag::Diagnostic err;
err.severity = diag::Severity::Error;
@@ -123,32 +125,13 @@
diags->add(std::move(err));
return nullptr;
}
- idx_expr =
- ctx->mod->create<ast::ScalarConstructorExpression>(c->source(), lit);
} else {
- auto* u32 = ctx->mod->create<ast::type::U32>();
-
- ast::ExpressionList cast_expr;
- cast_expr.push_back(ctx->Clone(expr->idx_expr()));
-
- ast::ExpressionList params;
- params.push_back(ctx->mod->create<ast::TypeConstructorExpression>(
- Source{}, u32, cast_expr));
- params.push_back(ctx->mod->create<ast::ScalarConstructorExpression>(
- Source{}, ctx->mod->create<ast::UintLiteral>(Source{}, u32, size - 1)));
-
- auto* call_expr = ctx->mod->create<ast::CallExpression>(
- Source{},
- ctx->mod->create<ast::IdentifierExpression>(
- Source{}, ctx->mod->RegisterSymbol("min"), "min"),
- std::move(params));
- call_expr->set_result_type(u32);
-
- idx_expr = call_expr;
+ new_idx =
+ b.Call("min", b.Construct<u32>(ctx->Clone(old_idx)), b.Expr(size - 1));
}
- return ctx->mod->create<ast::ArrayAccessorExpression>(
- ctx->Clone(expr->source()), ctx->Clone(expr->array()), idx_expr);
+ return b.create<ast::ArrayAccessorExpression>(
+ ctx->Clone(expr->source()), ctx->Clone(expr->array()), new_idx);
}
} // namespace transform
diff --git a/src/transform/bound_array_accessors_test.cc b/src/transform/bound_array_accessors_test.cc
index e7d70ce..14ce062 100644
--- a/src/transform/bound_array_accessors_test.cc
+++ b/src/transform/bound_array_accessors_test.cc
@@ -446,16 +446,35 @@
// -> var b : f32 = a[1][min(u32(idx), 0, 1)]
}
-// TODO(dsinclair): Implement when we have arrayLength for Runtime Arrays
-TEST_F(BoundArrayAccessorsTest, DISABLED_RuntimeArray_Clamps) {
- // struct S {
- // a : f32;
- // b : array<f32>;
- // }
- // S s;
- // var b : f32 = s.b[25]
- //
- // -> var b : f32 = s.b[min(u32(25), arrayLength(s.b))]
+TEST_F(BoundArrayAccessorsTest, RuntimeArray_Clamps) {
+ auto* src = R"(
+struct S {
+ a : f32;
+ b : array<f32>;
+};
+var s : S;
+
+fn f() -> void {
+ var d : f32 = s.b[25];
+}
+)";
+
+ auto* expect = R"(
+struct S {
+ a : f32;
+ b : array<f32>;
+};
+
+var s : S;
+
+fn f() -> void {
+ var d : f32 = s.b[min(u32(25), (arrayLength(s.b) - 1u))];
+}
+)";
+
+ auto got = Transform<BoundArrayAccessors>(src);
+
+ EXPECT_EQ(expect, got);
}
// TODO(dsinclair): Clamp atomics when available.
