transform::Robustness: Clamp texture[Load|Store] coordinates
Bug: tint:748
Change-Id: I213a4b14e6ae2e8bc8fd01dc241ad15f7c5c5c6c
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/56544
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Antonio Maiorano <amaiorano@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
diff --git a/src/transform/robustness.cc b/src/transform/robustness.cc
index e59ea5d..66964f8 100644
--- a/src/transform/robustness.cc
+++ b/src/transform/robustness.cc
@@ -18,7 +18,9 @@
#include <utility>
#include "src/program_builder.h"
+#include "src/sem/call.h"
#include "src/sem/expression.h"
+#include "src/sem/statement.h"
TINT_INSTANTIATE_TYPEINFO(tint::transform::Robustness);
@@ -32,18 +34,17 @@
ctx.ReplaceAll([&](ast::ArrayAccessorExpression* expr) {
return Transform(expr, &ctx);
});
-
+ ctx.ReplaceAll(
+ [&](ast::CallExpression* expr) { return Transform(expr, &ctx); });
ctx.Clone();
}
+// Apply bounds clamping to array, vector and matrix indexing
ast::ArrayAccessorExpression* Robustness::Transform(
ast::ArrayAccessorExpression* expr,
CloneContext* ctx) {
- auto& diags = ctx->dst->Diagnostics();
-
auto* ret_type = ctx->src->Sem().Get(expr->array())->Type()->UnwrapRef();
- if (!ret_type->Is<sem::Array>() && !ret_type->Is<sem::Matrix>() &&
- !ret_type->Is<sem::Vector>()) {
+ if (!ret_type->IsAnyOf<sem::Array, sem::Matrix, sem::Vector>()) {
return nullptr;
}
@@ -68,16 +69,16 @@
ast::Expression* new_idx = nullptr;
if (size == 0) {
- if (is_arr) {
- auto* arr = ctx->Clone(expr->array());
- auto* arr_len = b.Call("arrayLength", ctx->dst->AddressOf(arr));
- auto* limit = b.Sub(arr_len, b.Expr(1u));
- new_idx = b.Call("min", b.Construct<u32>(ctx->Clone(old_idx)), limit);
- } else {
- diags.add_error(diag::System::Transform, "invalid 0 size",
- expr->source());
+ if (!is_arr) {
+ b.Diagnostics().add_error(diag::System::Transform,
+ "invalid 0 sized non-array", expr->source());
return nullptr;
}
+ // Runtime sized array
+ auto* arr = ctx->Clone(expr->array());
+ auto* arr_len = b.Call("arrayLength", b.AddressOf(arr));
+ auto* limit = b.Sub(arr_len, b.Expr(1u));
+ new_idx = b.Call("min", b.Construct<u32>(ctx->Clone(old_idx)), limit);
} 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();
@@ -87,9 +88,9 @@
} else if (auto* uint = lit->As<ast::UintLiteral>()) {
new_idx = b.Expr(std::min(uint->value(), size - 1));
} else {
- diags.add_error(diag::System::Transform,
- "unknown scalar constructor type for accessor",
- expr->source());
+ b.Diagnostics().add_error(diag::System::Transform,
+ "unknown scalar constructor type for accessor",
+ expr->source());
return nullptr;
}
} else {
@@ -100,7 +101,38 @@
// Clone arguments outside of create() call to have deterministic ordering
auto src = ctx->Clone(expr->source());
auto* arr = ctx->Clone(expr->array());
- return b.create<ast::ArrayAccessorExpression>(src, arr, new_idx);
+ return b.IndexAccessor(src, arr, new_idx);
+}
+
+// Apply bounds clamping textureLoad() and textureStore() coordinates
+ast::CallExpression* Robustness::Transform(ast::CallExpression* expr,
+ CloneContext* ctx) {
+ auto* call = ctx->src->Sem().Get(expr);
+ auto* call_target = call->Target();
+ auto* intrinsic = call_target->As<sem::Intrinsic>();
+ if (!intrinsic || (intrinsic->Type() != sem::IntrinsicType::kTextureLoad &&
+ intrinsic->Type() != sem::IntrinsicType::kTextureStore)) {
+ return nullptr; // No transform, just clone.
+ }
+
+ // Index of the texture and coords parameters for the intrinsic overload
+ auto texture_idx =
+ sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kTexture);
+ auto coords_idx =
+ sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kCoords);
+
+ auto* texture_arg = expr->params()[texture_idx];
+ auto* coords_arg = expr->params()[coords_idx];
+ auto* coords_ty = intrinsic->Parameters()[coords_idx].type;
+
+ ProgramBuilder& b = *ctx->dst;
+ auto* texture_dims = b.Call("textureDimensions", ctx->Clone(texture_arg));
+ auto* zero_dims = b.Construct(CreateASTTypeFor(ctx, coords_ty));
+ auto* clamped_coords =
+ b.Call("clamp", ctx->Clone(coords_arg), zero_dims, texture_dims);
+
+ ctx->Replace(coords_arg, clamped_coords);
+ return nullptr; // Clone, which will use the coords replacement above.
}
} // namespace transform
diff --git a/src/transform/robustness.h b/src/transform/robustness.h
index b489d07..998b855 100644
--- a/src/transform/robustness.h
+++ b/src/transform/robustness.h
@@ -15,9 +15,16 @@
#ifndef SRC_TRANSFORM_ROBUSTNESS_H_
#define SRC_TRANSFORM_ROBUSTNESS_H_
-#include "src/ast/array_accessor_expression.h"
#include "src/transform/transform.h"
+// Forward declarations
+namespace tint {
+namespace ast {
+class ArrayAccessorExpression;
+class CallExpression;
+} // namespace ast
+} // namespace tint
+
namespace tint {
namespace transform {
@@ -44,6 +51,7 @@
private:
ast::ArrayAccessorExpression* Transform(ast::ArrayAccessorExpression* expr,
CloneContext* ctx);
+ ast::CallExpression* Transform(ast::CallExpression* expr, CloneContext* ctx);
};
using BoundArrayAccessors = Robustness;
diff --git a/src/transform/robustness_test.cc b/src/transform/robustness_test.cc
index 18ac30b..357b0b2 100644
--- a/src/transform/robustness_test.cc
+++ b/src/transform/robustness_test.cc
@@ -571,14 +571,90 @@
FAIL();
}
-// TODO(dsinclair): Clamp texture coord values. Depends on:
-// https://github.com/gpuweb/gpuweb/issues/1107
-TEST_F(RobustnessTest, DISABLED_TextureCoord_Clamp) {
- FAIL();
+// Clamp textureLoad() coord values
+TEST_F(RobustnessTest, TextureLoad_TextureCoord_Clamp) {
+ auto* src = R"(
+[[group(0), binding(0)]] var tex1d : texture_1d<f32>;
+
+[[group(0), binding(1)]] var tex2d : texture_2d<f32>;
+
+[[group(0), binding(2)]] var tex3d : texture_3d<f32>;
+
+[[group(0), binding(3)]] var tex2d_arr : texture_storage_2d_array<rgba8sint, read>;
+
+fn f() {
+ ignore(textureLoad(tex1d, 10, 100));
+ ignore(textureLoad(tex2d, vec2<i32>(10, 20), 100));
+ ignore(textureLoad(tex3d, vec3<i32>(10, 20, 30), 100));
+ ignore(textureLoad(tex2d_arr, vec2<i32>(10, 20), 100));
+}
+)";
+
+ auto* expect = R"(
+[[group(0), binding(0)]] var tex1d : texture_1d<f32>;
+
+[[group(0), binding(1)]] var tex2d : texture_2d<f32>;
+
+[[group(0), binding(2)]] var tex3d : texture_3d<f32>;
+
+[[group(0), binding(3)]] var tex2d_arr : texture_storage_2d_array<rgba8sint, read>;
+
+fn f() {
+ ignore(textureLoad(tex1d, clamp(10, i32(), textureDimensions(tex1d)), 100));
+ ignore(textureLoad(tex2d, clamp(vec2<i32>(10, 20), vec2<i32>(), textureDimensions(tex2d)), 100));
+ ignore(textureLoad(tex3d, clamp(vec3<i32>(10, 20, 30), vec3<i32>(), textureDimensions(tex3d)), 100));
+ ignore(textureLoad(tex2d_arr, clamp(vec2<i32>(10, 20), vec2<i32>(), textureDimensions(tex2d_arr)), 100));
+}
+)";
+
+ auto got = Run<Robustness>(src);
+
+ EXPECT_EQ(expect, str(got));
}
-// TODO(dsinclair): Test for scoped variables when Lexical Scopes implemented
-TEST_F(RobustnessTest, DISABLED_Scoped_Variable) {
+// Clamp textureStore() coord values
+TEST_F(RobustnessTest, TextureStore_TextureCoord_Clamp) {
+ auto* src = R"(
+[[group(0), binding(0)]] var tex1d : texture_storage_1d<rgba8sint, write>;
+
+[[group(0), binding(1)]] var tex2d : texture_storage_2d<rgba8sint, write>;
+
+[[group(0), binding(2)]] var tex3d : texture_storage_3d<rgba8sint, write>;
+
+[[group(0), binding(3)]] var tex2d_arr : texture_storage_2d_array<rgba8sint, write>;
+
+fn f() {
+ textureStore(tex1d, 10, vec4<i32>());
+ textureStore(tex2d, vec2<i32>(10, 20), vec4<i32>());
+ textureStore(tex3d, vec3<i32>(10, 20, 30), vec4<i32>());
+ textureStore(tex2d_arr, vec2<i32>(10, 20), 50, vec4<i32>());
+}
+)";
+
+ auto* expect = R"(
+[[group(0), binding(0)]] var tex1d : texture_storage_1d<rgba8sint, write>;
+
+[[group(0), binding(1)]] var tex2d : texture_storage_2d<rgba8sint, write>;
+
+[[group(0), binding(2)]] var tex3d : texture_storage_3d<rgba8sint, write>;
+
+[[group(0), binding(3)]] var tex2d_arr : texture_storage_2d_array<rgba8sint, write>;
+
+fn f() {
+ textureStore(tex1d, clamp(10, i32(), textureDimensions(tex1d)), vec4<i32>());
+ textureStore(tex2d, clamp(vec2<i32>(10, 20), vec2<i32>(), textureDimensions(tex2d)), vec4<i32>());
+ textureStore(tex3d, clamp(vec3<i32>(10, 20, 30), vec3<i32>(), textureDimensions(tex3d)), vec4<i32>());
+ textureStore(tex2d_arr, clamp(vec2<i32>(10, 20), vec2<i32>(), textureDimensions(tex2d_arr)), 50, vec4<i32>());
+}
+)";
+
+ auto got = Run<Robustness>(src);
+
+ EXPECT_EQ(expect, str(got));
+}
+
+// TODO(dsinclair): Test for scoped variables when shadowing is implemented
+TEST_F(RobustnessTest, DISABLED_Shadowed_Variable) {
// var a : array<f32, 3>;
// var i : u32;
// {
