validation: textureSample*() offset argument must be literal or const_expr

Bug: tint:939
Change-Id: I4ba44902948e38aa9ce3e9ecae9b3b4b593bd93d
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/57660
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Sarah Mashayekhi <sarahmashay@google.com>

src/ast/intrinsic_texture_helper_test.cc

@@ -1367,7 +1367,7 @@ std::vector<TextureOverloadCase> TextureOverloadCase::ValidCases() {
                                b->vec2<f32>(1.f, 2.f),  // coords
                                b->vec2<f32>(3.f, 4.f),  // ddx
                                b->vec2<f32>(5.f, 6.f),  // ddy
-                               b->vec2<i32>(7, 8));     // offset
+                               b->vec2<i32>(7, 7));     // offset
           },
       },
       {
@@ -1413,7 +1413,7 @@ std::vector<TextureOverloadCase> TextureOverloadCase::ValidCases() {
                                3,                       // array_index
                                b->vec2<f32>(4.f, 5.f),  // ddx
                                b->vec2<f32>(6.f, 7.f),  // ddy
-                               b->vec2<i32>(8, 9));     // offset
+                               b->vec2<i32>(6, 7));     // offset
           },
       },
       {
@@ -1455,7 +1455,7 @@ std::vector<TextureOverloadCase> TextureOverloadCase::ValidCases() {
                                b->vec3<f32>(1.f, 2.f, 3.f),  // coords
                                b->vec3<f32>(4.f, 5.f, 6.f),  // ddx
                                b->vec3<f32>(7.f, 8.f, 9.f),  // ddy
-                               b->vec3<i32>(10, 11, 12));    // offset
+                               b->vec3<i32>(0, 1, 2));       // offset
           },
       },
       {

src/resolver/intrinsic_validation_test.cc

@@ -12,40 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "src/resolver/resolver.h"
-
-#include "gmock/gmock.h"
-#include "src/ast/assignment_statement.h"
-#include "src/ast/bitcast_expression.h"
-#include "src/ast/break_statement.h"
-#include "src/ast/call_statement.h"
-#include "src/ast/continue_statement.h"
-#include "src/ast/if_statement.h"
 #include "src/ast/intrinsic_texture_helper_test.h"
-#include "src/ast/loop_statement.h"
-#include "src/ast/return_statement.h"
-#include "src/ast/stage_decoration.h"
-#include "src/ast/struct_block_decoration.h"
-#include "src/ast/switch_statement.h"
-#include "src/ast/unary_op_expression.h"
-#include "src/ast/variable_decl_statement.h"
 #include "src/resolver/resolver_test_helper.h"
-#include "src/sem/call.h"
-#include "src/sem/function.h"
-#include "src/sem/member_accessor_expression.h"
-#include "src/sem/sampled_texture_type.h"
-#include "src/sem/statement.h"
-#include "src/sem/variable.h"
-
-using ::testing::ElementsAre;
-using ::testing::HasSubstr;
 
 namespace tint {
 namespace resolver {
 namespace {
 
-using IntrinsicType = sem::IntrinsicType;
-
 using ResolverIntrinsicValidationTest = ResolverTest;
 
 TEST_F(ResolverIntrinsicValidationTest, InvalidPipelineStageDirect) {
@@ -90,6 +63,207 @@ TEST_F(ResolverIntrinsicValidationTest, InvalidPipelineStageIndirect) {
 7:8 note: called by entry point 'main')");
 }
 
+namespace TextureSamplerOffset {
+
+using TextureOverloadCase = ast::intrinsic::test::TextureOverloadCase;
+using ValidTextureOverload = ast::intrinsic::test::ValidTextureOverload;
+using TextureKind = ast::intrinsic::test::TextureKind;
+using TextureDataType = ast::intrinsic::test::TextureDataType;
+using u32 = ProgramBuilder::u32;
+using i32 = ProgramBuilder::i32;
+using f32 = ProgramBuilder::f32;
+
+static std::vector<TextureOverloadCase> ValidCases() {
+  std::vector<TextureOverloadCase> cases;
+  for (auto c : TextureOverloadCase::ValidCases()) {
+    if (std::string(c.function).find("textureSample") == 0) {
+      if (std::string(c.description).find("offset ") != std::string::npos) {
+        cases.push_back(c);
+      }
+    }
+  }
+  return cases;
+}
+
+struct OffsetCase {
+  bool is_valid;
+  int32_t x;
+  int32_t y;
+  int32_t z;
+  int32_t illegal_value = 0;
+};
+
+static std::vector<OffsetCase> OffsetCases() {
+  return {
+      {true, 0, 1, 2},          //
+      {true, 7, -8, 7},         //
+      {false, 10, 10, 20, 10},  //
+      {false, -9, 0, 0, -9},    //
+      {false, 0, 8, 0, 8},      //
+  };
+}
+
+using IntrinsicTextureSamplerValidationTest =
+    ResolverTestWithParam<std::tuple<TextureOverloadCase,  // texture info
+                                     OffsetCase            // offset info
+                                     >>;
+TEST_P(IntrinsicTextureSamplerValidationTest, ConstExpr) {
+  auto& p = GetParam();
+  auto param = std::get<0>(p);
+  auto offset = std::get<1>(p);
+  param.buildTextureVariable(this);
+  param.buildSamplerVariable(this);
+
+  auto args = param.args(this);
+  // Make Resolver visit the Node about to be removed
+  WrapInFunction(args.back());
+  args.pop_back();
+  if (NumCoordinateAxes(param.texture_dimension) == 2) {
+    args.push_back(
+        Construct(Source{{12, 34}}, ty.vec2<i32>(), offset.x, offset.y));
+  } else if (NumCoordinateAxes(param.texture_dimension) == 3) {
+    args.push_back(Construct(Source{{12, 34}}, ty.vec3<i32>(), offset.x,
+                             offset.y, offset.z));
+  }
+
+  auto* call = Call(param.function, args);
+  Func("func", {}, ty.void_(), {Ignore(call)},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
+
+  if (offset.is_valid) {
+    EXPECT_TRUE(r()->Resolve()) << r()->error();
+  } else {
+    EXPECT_FALSE(r()->Resolve());
+    std::stringstream err;
+    err << "12:34 error: each offset component of '" << param.function
+        << "' must be at least -8 and at most 7. found: '"
+        << std::to_string(offset.illegal_value) << "'";
+    EXPECT_EQ(r()->error(), err.str());
+  }
+}
+
+TEST_P(IntrinsicTextureSamplerValidationTest, ConstExprOfConstExpr) {
+  auto& p = GetParam();
+  auto param = std::get<0>(p);
+  auto offset = std::get<1>(p);
+  param.buildTextureVariable(this);
+  param.buildSamplerVariable(this);
+
+  auto args = param.args(this);
+  // Make Resolver visit the Node about to be removed
+  WrapInFunction(args.back());
+  args.pop_back();
+  if (NumCoordinateAxes(param.texture_dimension) == 2) {
+    args.push_back(Construct(Source{{12, 34}}, ty.vec2<i32>(),
+                             Construct(ty.i32(), offset.x), offset.y));
+  } else if (NumCoordinateAxes(param.texture_dimension) == 3) {
+    args.push_back(Construct(Source{{12, 34}}, ty.vec3<i32>(), offset.x,
+                             Construct(ty.vec2<i32>(), offset.y, offset.z)));
+  }
+  auto* call = Call(param.function, args);
+  Func("func", {}, ty.void_(), {Ignore(call)},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
+  if (offset.is_valid) {
+    EXPECT_TRUE(r()->Resolve()) << r()->error();
+  } else {
+    EXPECT_FALSE(r()->Resolve());
+    std::stringstream err;
+    err << "12:34 error: each offset component of '" << param.function
+        << "' must be at least -8 and at most 7. found: '"
+        << std::to_string(offset.illegal_value) << "'";
+    EXPECT_EQ(r()->error(), err.str());
+  }
+}
+
+TEST_P(IntrinsicTextureSamplerValidationTest, EmptyVectorConstructor) {
+  auto& p = GetParam();
+  auto param = std::get<0>(p);
+  param.buildTextureVariable(this);
+  param.buildSamplerVariable(this);
+
+  auto args = param.args(this);
+  // Make Resolver visit the Node about to be removed
+  WrapInFunction(args.back());
+  args.pop_back();
+  if (NumCoordinateAxes(param.texture_dimension) == 2) {
+    args.push_back(Construct(Source{{12, 34}}, ty.vec2<i32>()));
+  } else if (NumCoordinateAxes(param.texture_dimension) == 3) {
+    args.push_back(Construct(Source{{12, 34}}, ty.vec3<i32>()));
+  }
+
+  auto* call = Call(param.function, args);
+  Func("func", {}, ty.void_(), {Ignore(call)},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
+  EXPECT_TRUE(r()->Resolve()) << r()->error();
+}
+
+TEST_P(IntrinsicTextureSamplerValidationTest, GlobalConst) {
+  auto& p = GetParam();
+  auto param = std::get<0>(p);
+  auto offset = std::get<1>(p);
+  param.buildTextureVariable(this);
+  param.buildSamplerVariable(this);
+
+  auto args = param.args(this);
+  // Make Resolver visit the Node about to be removed
+  WrapInFunction(args.back());
+  args.pop_back();
+  GlobalConst("offset_2d", ty.vec2<i32>(), vec2<i32>(offset.x, offset.y));
+  GlobalConst("offset_3d", ty.vec3<i32>(),
+              vec3<i32>(offset.x, offset.y, offset.z));
+  if (NumCoordinateAxes(param.texture_dimension) == 2) {
+    args.push_back(Expr(Source{{12, 34}}, "offset_2d"));
+  } else if (NumCoordinateAxes(param.texture_dimension) == 3) {
+    args.push_back(Expr(Source{{12, 34}}, "offset_3d"));
+  }
+
+  auto* call = Call(param.function, args);
+  Func("func", {}, ty.void_(), {Ignore(call)},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
+  EXPECT_FALSE(r()->Resolve());
+  std::stringstream err;
+  err << "12:34 error: '" << param.function
+      << "' offset parameter must be provided as"
+      << " a literal or const_expr expression";
+  EXPECT_EQ(r()->error(), err.str());
+}
+
+TEST_P(IntrinsicTextureSamplerValidationTest, ScalarConst) {
+  auto& p = GetParam();
+  auto param = std::get<0>(p);
+  auto offset = std::get<1>(p);
+  param.buildTextureVariable(this);
+  param.buildSamplerVariable(this);
+  auto* x = Const("x", ty.i32(), Construct(ty.i32(), offset.x));
+
+  auto args = param.args(this);
+  // Make Resolver visit the Node about to be removed
+  WrapInFunction(args.back());
+  args.pop_back();
+  if (NumCoordinateAxes(param.texture_dimension) == 2) {
+    args.push_back(Construct(Source{{12, 34}}, ty.vec2<i32>(), x, offset.y));
+  } else if (NumCoordinateAxes(param.texture_dimension) == 3) {
+    args.push_back(
+        Construct(Source{{12, 34}}, ty.vec3<i32>(), x, offset.y, offset.z));
+  }
+
+  auto* call = Call(param.function, args);
+  Func("func", {}, ty.void_(), {Decl(x), Ignore(call)},
+       {create<ast::StageDecoration>(ast::PipelineStage::kFragment)});
+  EXPECT_FALSE(r()->Resolve());
+  std::stringstream err;
+  err << "12:34 error: '" << param.function
+      << "' offset parameter must be provided as"
+      << " a literal or const_expr expression";
+  EXPECT_EQ(r()->error(), err.str());
+}
+
+INSTANTIATE_TEST_SUITE_P(IntrinsicTextureSamplerValidationTest,
+                         IntrinsicTextureSamplerValidationTest,
+                         testing::Combine(testing::ValuesIn(ValidCases()),
+                                          testing::ValuesIn(OffsetCases())));
+}  // namespace TextureSamplerOffset
+
 }  // namespace
 }  // namespace resolver
 }  // namespace tint

src/resolver/resolver.cc

@@ -2431,13 +2431,67 @@ bool Resolver::IntrinsicCall(ast::CallExpression* call,
     AddWarning("use of deprecated intrinsic", call->source());
   }
 
-  builder_->Sem().Add(
+  auto* out = builder_->create<sem::Call>(call, result, current_statement_);
-      call, builder_->create<sem::Call>(call, result, current_statement_));
+  builder_->Sem().Add(call, out);
   SetExprInfo(call, result->ReturnType());
 
   current_function_->intrinsic_calls.emplace_back(
       IntrinsicCallInfo{call, result});
 
+  if (IsTextureIntrinsic(intrinsic_type) &&
+      !ValidateTextureIntrinsicFunction(call, out)) {
+    return false;
+  }
+
+  return true;
+}
+
+bool Resolver::ValidateTextureIntrinsicFunction(
+    const ast::CallExpression* ast_call,
+    const sem::Call* sem_call) {
+  auto* intrinsic = sem_call->Target()->As<sem::Intrinsic>();
+  if (!intrinsic) {
+    return false;
+  }
+  std::string func_name = intrinsic->str();
+  auto index =
+      sem::IndexOf(intrinsic->Parameters(), sem::ParameterUsage::kOffset);
+  if (index > -1) {
+    auto* param = ast_call->params()[index];
+    if (param->Is<ast::TypeConstructorExpression>()) {
+      auto values = ConstantValueOf(param);
+      if (!values.IsValid()) {
+        AddError("'" + func_name +
+                     "' offset parameter must be provided as a literal or "
+                     "const_expr expression",
+                 param->source());
+        return false;
+      }
+      if (!values.Type()->Is<sem::Vector>() ||
+          !values.ElementType()->Is<sem::I32>()) {
+        TINT_ICE(Resolver, diagnostics_)
+            << "failed to resolve '" + func_name + "' offset parameter type";
+        return false;
+      }
+      for (auto offset : values.Elements()) {
+        auto offset_value = offset.i32;
+        if (offset_value < -8 || offset_value > 7) {
+          AddError("each offset component of '" + func_name +
+                       "' must be at least -8 and at most 7. "
+                       "found: '" +
+                       std::to_string(offset_value) + "'",
+                   param->source());
+          return false;
+        }
+      }
+    } else {
+      AddError("'" + func_name +
+                   "' offset parameter must be provided as a literal or "
+                   "const_expr expression",
+               param->source());
+      return false;
+    }
+  }
   return true;
 }
 

src/resolver/resolver.h

@@ -323,6 +323,8 @@ class Resolver {
   bool ValidateArrayConstructor(const ast::TypeConstructorExpression* ctor,
                                 const sem::Array* arr_type);
   bool ValidateTypeDecl(const ast::TypeDecl* named_type) const;
+  bool ValidateTextureIntrinsicFunction(const ast::CallExpression* ast_call,
+                                        const sem::Call* sem_call);
   bool ValidateNoDuplicateDecorations(const ast::DecorationList& decorations);
   // sem::Struct is assumed to have at least one member
   bool ValidateStorageClassLayout(const sem::Struct* type,

src/writer/hlsl/generator_impl_intrinsic_texture_test.cc

@@ -254,15 +254,15 @@ ExpectedResult expected_texture_overload(
     case ValidTextureOverload::kSampleGrad2dF32:
       return R"(tint_symbol.SampleGrad(tint_symbol_1, float2(1.0f, 2.0f), float2(3.0f, 4.0f), float2(5.0f, 6.0f));)";
     case ValidTextureOverload::kSampleGrad2dOffsetF32:
-      return R"(tint_symbol.SampleGrad(tint_symbol_1, float2(1.0f, 2.0f), float2(3.0f, 4.0f), float2(5.0f, 6.0f), int2(7, 8));)";
+      return R"(tint_symbol.SampleGrad(tint_symbol_1, float2(1.0f, 2.0f), float2(3.0f, 4.0f), float2(5.0f, 6.0f), int2(7, 7));)";
     case ValidTextureOverload::kSampleGrad2dArrayF32:
       return R"(tint_symbol.SampleGrad(tint_symbol_1, float3(1.0f, 2.0f, float(3)), float2(4.0f, 5.0f), float2(6.0f, 7.0f));)";
     case ValidTextureOverload::kSampleGrad2dArrayOffsetF32:
-      return R"(tint_symbol.SampleGrad(tint_symbol_1, float3(1.0f, 2.0f, float(3)), float2(4.0f, 5.0f), float2(6.0f, 7.0f), int2(8, 9));)";
+      return R"(tint_symbol.SampleGrad(tint_symbol_1, float3(1.0f, 2.0f, float(3)), float2(4.0f, 5.0f), float2(6.0f, 7.0f), int2(6, 7));)";
     case ValidTextureOverload::kSampleGrad3dF32:
       return R"(tint_symbol.SampleGrad(tint_symbol_1, float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f));)";
     case ValidTextureOverload::kSampleGrad3dOffsetF32:
-      return R"(tint_symbol.SampleGrad(tint_symbol_1, float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f), int3(10, 11, 12));)";
+      return R"(tint_symbol.SampleGrad(tint_symbol_1, float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f), int3(0, 1, 2));)";
     case ValidTextureOverload::kSampleGradCubeF32:
       return R"(tint_symbol.SampleGrad(tint_symbol_1, float3(1.0f, 2.0f, 3.0f), float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f));)";
     case ValidTextureOverload::kSampleGradCubeArrayF32:

src/writer/msl/generator_impl_intrinsic_texture_test.cc

@@ -153,15 +153,15 @@ std::string expected_texture_overload(
     case ValidTextureOverload::kSampleGrad2dF32:
       return R"(texture.sample(sampler, float2(1.0f, 2.0f), gradient2d(float2(3.0f, 4.0f), float2(5.0f, 6.0f))))";
     case ValidTextureOverload::kSampleGrad2dOffsetF32:
-      return R"(texture.sample(sampler, float2(1.0f, 2.0f), gradient2d(float2(3.0f, 4.0f), float2(5.0f, 6.0f)), int2(7, 8)))";
+      return R"(texture.sample(sampler, float2(1.0f, 2.0f), gradient2d(float2(3.0f, 4.0f), float2(5.0f, 6.0f)), int2(7, 7)))";
     case ValidTextureOverload::kSampleGrad2dArrayF32:
       return R"(texture.sample(sampler, float2(1.0f, 2.0f), 3, gradient2d(float2(4.0f, 5.0f), float2(6.0f, 7.0f))))";
     case ValidTextureOverload::kSampleGrad2dArrayOffsetF32:
-      return R"(texture.sample(sampler, float2(1.0f, 2.0f), 3, gradient2d(float2(4.0f, 5.0f), float2(6.0f, 7.0f)), int2(8, 9)))";
+      return R"(texture.sample(sampler, float2(1.0f, 2.0f), 3, gradient2d(float2(4.0f, 5.0f), float2(6.0f, 7.0f)), int2(6, 7)))";
     case ValidTextureOverload::kSampleGrad3dF32:
       return R"(texture.sample(sampler, float3(1.0f, 2.0f, 3.0f), gradient3d(float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f))))";
     case ValidTextureOverload::kSampleGrad3dOffsetF32:
-      return R"(texture.sample(sampler, float3(1.0f, 2.0f, 3.0f), gradient3d(float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f)), int3(10, 11, 12)))";
+      return R"(texture.sample(sampler, float3(1.0f, 2.0f, 3.0f), gradient3d(float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f)), int3(0, 1, 2)))";
     case ValidTextureOverload::kSampleGradCubeF32:
       return R"(texture.sample(sampler, float3(1.0f, 2.0f, 3.0f), gradientcube(float3(4.0f, 5.0f, 6.0f), float3(7.0f, 8.0f, 9.0f))))";
     case ValidTextureOverload::kSampleGradCubeArrayF32:

src/writer/spirv/builder_intrinsic_texture_test.cc

@@ -2070,14 +2070,13 @@ OpCapability SampledCubeArray
 %25 = OpTypeInt 32 1
 %24 = OpTypeVector %25 2
 %26 = OpConstant %25 7
-%27 = OpConstant %25 8
+%27 = OpConstantComposite %24 %26 %26
-%28 = OpConstantComposite %24 %26 %27
 )",
           R"(
 %10 = OpLoad %7 %5
 %11 = OpLoad %3 %1
 %13 = OpSampledImage %12 %11 %10
-%8 = OpImageSampleExplicitLod %9 %13 %17 Grad|ConstOffset %20 %23 %28
+%8 = OpImageSampleExplicitLod %9 %13 %17 Grad|ConstOffset %20 %23 %27
 )",
           R"(
 )"};
@@ -2141,8 +2140,8 @@ OpCapability SampledCubeArray
 %26 = OpConstant %4 7
 %27 = OpConstantComposite %21 %25 %26
 %28 = OpTypeVector %18 2
-%29 = OpConstant %18 8
+%29 = OpConstant %18 6
-%30 = OpConstant %18 9
+%30 = OpConstant %18 7
 %31 = OpConstantComposite %28 %29 %30
 )",
           R"(
@@ -2216,9 +2215,9 @@ OpCapability SampledCubeArray
 %26 = OpConstantComposite %14 %23 %24 %25
 %28 = OpTypeInt 32 1
 %27 = OpTypeVector %28 3
-%29 = OpConstant %28 10
+%29 = OpConstant %28 0
-%30 = OpConstant %28 11
+%30 = OpConstant %28 1
-%31 = OpConstant %28 12
+%31 = OpConstant %28 2
 %32 = OpConstantComposite %27 %29 %30 %31
 )",
           R"(