[transform] Add BufferArrayAccessors transform

This CL adds a buffer array accessor clamping transform to
the available transforms in Tint.

Bug: tint:101
Change-Id: If9d5b0fb2c3adba723ce2185870b0e10981103a6
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/28980
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

BUILD.gn

@@ -373,6 +373,8 @@ source_set("libtint_core_src") {
     "src/reader/reader.h",
     "src/scope_stack.h",
     "src/source.h",
+    "src/transform/bound_array_accessors_transform.cc",
+    "src/transform/bound_array_accessors_transform.h",
     "src/transform/vertex_pulling_transform.cc",
     "src/transform/vertex_pulling_transform.h",
     "src/type_determiner.cc",
@@ -745,6 +747,7 @@ source_set("tint_unittests_core_src") {
     "src/ast/variable_test.cc",
     "src/ast/workgroup_decoration_test.cc",
     "src/scope_stack_test.cc",
+    "src/transform/bound_array_accessors_transform_test.cc",
     "src/transform/vertex_pulling_transform_test.cc",
     "src/type_determiner_test.cc",
     "src/type_manager_test.cc",

src/CMakeLists.txt

@@ -194,6 +194,8 @@ set(TINT_LIB_SRCS
   reader/reader.h
   scope_stack.h
   source.h
+  transform/bound_array_accessors_transform.cc
+  transform/bound_array_accessors_transform.h
   transform/vertex_pulling_transform.cc
   transform/vertex_pulling_transform.h
   type_determiner.cc
@@ -355,6 +357,7 @@ set(TINT_TEST_SRCS
   ast/variable_test.cc
   ast/workgroup_decoration_test.cc
   scope_stack_test.cc
+  transform/bound_array_accessors_transform_test.cc
   transform/vertex_pulling_transform_test.cc
   type_determiner_test.cc
   type_manager_test.cc

src/ast/array_accessor_expression.h

@@ -60,6 +60,9 @@ class ArrayAccessorExpression : public Expression {
   }
   /// @returns the index expression
   Expression* idx_expr() const { return idx_expr_.get(); }
+  /// Removes the index expression from the array accessor
+  /// @returns the unique pointer to the index expression
+  std::unique_ptr<Expression> take_idx_expr() { return std::move(idx_expr_); }
 
   /// @returns true if this is an array accessor expression
   bool IsArrayAccessor() const override;

src/ast/case_statement.h

@@ -72,6 +72,8 @@ class CaseStatement : public Statement {
   }
   /// @returns the case body
   const BlockStatement* body() const { return body_.get(); }
+  /// @returns the case body
+  BlockStatement* body() { return body_.get(); }
 
   /// @returns true if this is a case statement
   bool IsCase() const override;

src/ast/else_statement.h

@@ -71,6 +71,8 @@ class ElseStatement : public Statement {
   }
   /// @returns the else body
   const BlockStatement* body() const { return body_.get(); }
+  /// @returns the else body
+  BlockStatement* body() { return body_.get(); }
 
   /// @returns true if this is a else statement
   bool IsElse() const override;

src/ast/function.h

@@ -163,7 +163,9 @@ class Function : public Node {
     body_ = std::move(body);
   }
   /// @returns the function body
-  BlockStatement* body() const { return body_.get(); }
+  const BlockStatement* body() const { return body_.get(); }
+  /// @returns the function body
+  BlockStatement* body() { return body_.get(); }
 
   /// @returns true if the name and type are both present
   bool IsValid() const override;

src/ast/if_statement.h

@@ -62,6 +62,8 @@ class IfStatement : public Statement {
   }
   /// @returns the if body
   const BlockStatement* body() const { return body_.get(); }
+  /// @returns the if body
+  BlockStatement* body() { return body_.get(); }
 
   /// Sets the else statements
   /// @param else_statements the else statements to set
@@ -70,6 +72,9 @@ class IfStatement : public Statement {
   }
   /// @returns the else statements
   const ElseStatementList& else_statements() const { return else_statements_; }
+  /// @returns the else statements
+  ElseStatementList& else_statements() { return else_statements_; }
+
   /// @returns true if there are else statements
   bool has_else_statements() const { return !else_statements_.empty(); }
 

src/ast/loop_statement.h

@@ -51,6 +51,8 @@ class LoopStatement : public Statement {
   }
   /// @returns the body statements
   const BlockStatement* body() const { return body_.get(); }
+  /// @returns the body statements
+  BlockStatement* body() { return body_.get(); }
 
   /// Sets the continuing statements
   /// @param continuing the continuing statements
@@ -59,6 +61,8 @@ class LoopStatement : public Statement {
   }
   /// @returns the continuing statements
   const BlockStatement* continuing() const { return continuing_.get(); }
+  /// @returns the continuing statements
+  BlockStatement* continuing() { return continuing_.get(); }
   /// @returns true if there are continuing statements in the loop
   bool has_continuing() const {
     return continuing_ != nullptr && !continuing_->empty();

src/ast/sint_literal.h

@@ -34,6 +34,9 @@ class SintLiteral : public IntLiteral {
   /// @returns true if this is a signed int literal
   bool IsSint() const override;
 
+  /// Updates the literals value
+  /// @param val the value to set
+  void set_value(int32_t val) { value_ = val; }
   /// @returns the int literal value
   int32_t value() const { return value_; }
 

src/ast/uint_literal.h

@@ -34,6 +34,9 @@ class UintLiteral : public IntLiteral {
   /// @returns true if this is a uint literal
   bool IsUint() const override;
 
+  /// Updates the literals value
+  /// @param val the value to set
+  void set_value(uint32_t val) { value_ = val; }
   /// @returns the uint literal value
   uint32_t value() const { return value_; }
 

src/transform/bound_array_accessors_transform.cc

@@ -0,0 +1,258 @@
+// Copyright 2020 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "src/transform/bound_array_accessors_transform.h"
+
+#include "src/ast/assignment_statement.h"
+#include "src/ast/binary_expression.h"
+#include "src/ast/bitcast_expression.h"
+#include "src/ast/block_statement.h"
+#include "src/ast/call_expression.h"
+#include "src/ast/call_statement.h"
+#include "src/ast/case_statement.h"
+#include "src/ast/else_statement.h"
+#include "src/ast/if_statement.h"
+#include "src/ast/loop_statement.h"
+#include "src/ast/member_accessor_expression.h"
+#include "src/ast/return_statement.h"
+#include "src/ast/scalar_constructor_expression.h"
+#include "src/ast/sint_literal.h"
+#include "src/ast/switch_statement.h"
+#include "src/ast/type/array_type.h"
+#include "src/ast/type/matrix_type.h"
+#include "src/ast/type/u32_type.h"
+#include "src/ast/type/vector_type.h"
+#include "src/ast/type_constructor_expression.h"
+#include "src/ast/uint_literal.h"
+#include "src/ast/unary_op_expression.h"
+#include "src/ast/variable.h"
+#include "src/ast/variable_decl_statement.h"
+#include "src/type_manager.h"
+
+namespace tint {
+namespace transform {
+
+BoundArrayAccessorsTransform::BoundArrayAccessorsTransform(Context* ctx,
+                                                           ast::Module* mod)
+    : ctx_(ctx), mod_(mod) {}
+
+BoundArrayAccessorsTransform::~BoundArrayAccessorsTransform() = default;
+
+bool BoundArrayAccessorsTransform::Run() {
+  // We skip over global variables as the constructor for a global must be a
+  // constant expression. There can't be any array accessors as per the current
+  // grammar.
+
+  for (auto& func : mod_->functions()) {
+    scope_stack_.push_scope();
+    if (!ProcessStatement(func->body())) {
+      return false;
+    }
+    scope_stack_.pop_scope();
+  }
+  return true;
+}
+
+bool BoundArrayAccessorsTransform::ProcessStatement(ast::Statement* stmt) {
+  if (stmt->IsAssign()) {
+    auto* as = stmt->AsAssign();
+    return ProcessExpression(as->lhs()) && ProcessExpression(as->rhs());
+  } else if (stmt->IsBlock()) {
+    for (auto& s : *(stmt->AsBlock())) {
+      if (!ProcessStatement(s.get())) {
+        return false;
+      }
+    }
+  } else if (stmt->IsBreak()) {
+    /* nop */
+  } else if (stmt->IsCall()) {
+    return ProcessExpression(stmt->AsCall()->expr());
+  } else if (stmt->IsCase()) {
+    return ProcessStatement(stmt->AsCase()->body());
+  } else if (stmt->IsContinue()) {
+    /* nop */
+  } else if (stmt->IsDiscard()) {
+    /* nop */
+  } else if (stmt->IsElse()) {
+    auto* e = stmt->AsElse();
+    return ProcessExpression(e->condition()) && ProcessStatement(e->body());
+  } else if (stmt->IsFallthrough()) {
+    /* nop */
+  } else if (stmt->IsIf()) {
+    auto* e = stmt->AsIf();
+    if (!ProcessExpression(e->condition()) || !ProcessStatement(e->body())) {
+      return false;
+    }
+    for (auto& s : e->else_statements()) {
+      if (!ProcessStatement(s.get())) {
+        return false;
+      }
+    }
+  } else if (stmt->IsLoop()) {
+    auto* l = stmt->AsLoop();
+    if (l->has_continuing() && !ProcessStatement(l->continuing())) {
+      return false;
+    }
+    return ProcessStatement(l->body());
+  } else if (stmt->IsReturn()) {
+    if (stmt->AsReturn()->has_value()) {
+      return ProcessExpression(stmt->AsReturn()->value());
+    }
+  } else if (stmt->IsSwitch()) {
+    auto* s = stmt->AsSwitch();
+    if (!ProcessExpression(s->condition())) {
+      return false;
+    }
+
+    for (auto& c : s->body()) {
+      if (!ProcessStatement(c.get())) {
+        return false;
+      }
+    }
+  } else if (stmt->IsVariableDecl()) {
+    auto* v = stmt->AsVariableDecl()->variable();
+    if (v->has_constructor() && !ProcessExpression(v->constructor())) {
+      return false;
+    }
+    scope_stack_.set(v->name(), v);
+  } else {
+    error_ = "unknown statement in bound array accessors transform";
+    return false;
+  }
+  return true;
+}
+
+bool BoundArrayAccessorsTransform::ProcessExpression(ast::Expression* expr) {
+  if (expr->IsArrayAccessor()) {
+    return ProcessArrayAccessor(expr->AsArrayAccessor());
+  } else if (expr->IsBitcast()) {
+    return ProcessExpression(expr->AsBitcast()->expr());
+  } else if (expr->IsCall()) {
+    auto* c = expr->AsCall();
+    if (!ProcessExpression(c->func())) {
+      return false;
+    }
+    for (auto& e : c->params()) {
+      if (!ProcessExpression(e.get())) {
+        return false;
+      }
+    }
+  } else if (expr->IsIdentifier()) {
+    /* nop */
+  } else if (expr->IsConstructor()) {
+    auto* c = expr->AsConstructor();
+    if (c->IsTypeConstructor()) {
+      for (auto& e : c->AsTypeConstructor()->values()) {
+        if (!ProcessExpression(e.get())) {
+          return false;
+        }
+      }
+    }
+  } else if (expr->IsMemberAccessor()) {
+    auto* m = expr->AsMemberAccessor();
+    return ProcessExpression(m->structure()) && ProcessExpression(m->member());
+  } else if (expr->IsBinary()) {
+    auto* b = expr->AsBinary();
+    return ProcessExpression(b->lhs()) && ProcessExpression(b->rhs());
+  } else if (expr->IsUnaryOp()) {
+    return ProcessExpression(expr->AsUnaryOp()->expr());
+  } else {
+    error_ = "unknown statement in bound array accessors transform";
+    return false;
+  }
+  return true;
+}
+
+bool BoundArrayAccessorsTransform::ProcessArrayAccessor(
+    ast::ArrayAccessorExpression* expr) {
+  if (!ProcessExpression(expr->array()) ||
+      !ProcessExpression(expr->idx_expr())) {
+    return false;
+  }
+
+  auto* ret_type = expr->array()->result_type()->UnwrapAliasPtrAlias();
+  if (!ret_type->IsArray() && !ret_type->IsMatrix() && !ret_type->IsVector()) {
+    return true;
+  }
+
+  if (ret_type->IsVector() || ret_type->IsArray()) {
+    uint32_t size = ret_type->IsVector() ? ret_type->AsVector()->size()
+                                         : ret_type->AsArray()->size();
+    if (size == 0) {
+      error_ = "invalid 0 size for array or vector";
+      return false;
+    }
+
+    if (!ProcessAccessExpression(expr, size)) {
+      return false;
+    }
+  } else {
+    // The row accessor would have been an embedded array accessor and already
+    // handled, so we just need to do columns here.
+    uint32_t size = ret_type->AsMatrix()->columns();
+    if (!ProcessAccessExpression(expr, size)) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool BoundArrayAccessorsTransform::ProcessAccessExpression(
+    ast::ArrayAccessorExpression* expr,
+    uint32_t size) {
+  // Scalar constructor we can re-write the value to be within bounds.
+  if (expr->idx_expr()->IsConstructor() &&
+      expr->idx_expr()->AsConstructor()->IsScalarConstructor()) {
+    auto* lit =
+        expr->idx_expr()->AsConstructor()->AsScalarConstructor()->literal();
+    if (lit->IsSint()) {
+      int32_t val = lit->AsSint()->value();
+      if (val < 0) {
+        val = 0;
+      } else if (val >= int32_t(size)) {
+        val = int32_t(size) - 1;
+      }
+      lit->AsSint()->set_value(val);
+    } else if (lit->IsUint()) {
+      uint32_t val = lit->AsUint()->value();
+      if (val >= size - 1) {
+        val = size - 1;
+      }
+      lit->AsUint()->set_value(val);
+    } else {
+      error_ = "unknown scalar constructor type for accessor";
+      return false;
+    }
+  } else {
+    auto* u32 = ctx_->type_mgr().Get(std::make_unique<ast::type::U32Type>());
+
+    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::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));
+    call_expr->set_result_type(u32);
+
+    expr->set_idx_expr(std::move(call_expr));
+  }
+  return true;
+}
+
+}  // namespace transform
+}  // namespace tint

src/transform/bound_array_accessors_transform.h

@@ -0,0 +1,64 @@
+// Copyright 2020 The Tint Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#ifndef SRC_TRANSFORM_BOUND_ARRAY_ACCESSORS_TRANSFORM_H_
+#define SRC_TRANSFORM_BOUND_ARRAY_ACCESSORS_TRANSFORM_H_
+
+#include "src/ast/array_accessor_expression.h"
+#include "src/ast/expression.h"
+#include "src/ast/module.h"
+#include "src/ast/statement.h"
+#include "src/context.h"
+#include "src/scope_stack.h"
+
+#include <string>
+
+namespace tint {
+namespace transform {
+
+/// This transformer is responsible for clamping all array accesses to be within
+/// the bounds of the array. Any access before the start of the array will clamp
+/// to zero and any access past the end of the array will clamp to
+/// (array length - 1).
+class BoundArrayAccessorsTransform {
+ public:
+  /// Constructor
+  /// @param ctx the Tint context object
+  /// @param mod the module transform
+  explicit BoundArrayAccessorsTransform(Context* ctx, ast::Module* mod);
+  ~BoundArrayAccessorsTransform();
+
+  /// @returns true if the transformation was successful
+  bool Run();
+
+  /// @returns error messages
+  const std::string& error() { return error_; }
+
+ private:
+  bool ProcessStatement(ast::Statement* stmt);
+  bool ProcessExpression(ast::Expression* expr);
+  bool ProcessArrayAccessor(ast::ArrayAccessorExpression* expr);
+  bool ProcessAccessExpression(ast::ArrayAccessorExpression* expr,
+                               uint32_t size);
+
+  Context* ctx_ = nullptr;
+  ast::Module* mod_ = nullptr;
+  std::string error_;
+  ScopeStack<ast::Variable*> scope_stack_;
+};
+
+}  // namespace transform
+}  // namespace tint
+
+#endif  // SRC_TRANSFORM_BOUND_ARRAY_ACCESSORS_TRANSFORM_H_

src/transform/vertex_pulling_transform.cc

@@ -37,7 +37,6 @@
 
 namespace tint {
 namespace transform {
-
 namespace {
 
 static const char kVertexBufferNamePrefix[] = "_tint_pulling_vertex_buffer_";