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>

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 @@ void Robustness::Run(CloneContext& ctx, const DataMap&, DataMap&) {
   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::ArrayAccessorExpression* Robustness::Transform(
   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 @@ ast::ArrayAccessorExpression* Robustness::Transform(
     } 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 @@ ast::ArrayAccessorExpression* Robustness::Transform(
   // 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

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 @@ class Robustness : public Castable<Robustness, Transform> {
  private:
   ast::ArrayAccessorExpression* Transform(ast::ArrayAccessorExpression* expr,
                                           CloneContext* ctx);
+  ast::CallExpression* Transform(ast::CallExpression* expr, CloneContext* ctx);
 };
 
 using BoundArrayAccessors = Robustness;

src/transform/robustness_test.cc

@@ -571,14 +571,90 @@ TEST_F(RobustnessTest, DISABLED_Atomics_Clamp) {
   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;
   // {