tint/transform: Fix array materialization when indexing with override

Fixed: tint:1697
Change-Id: I6de9ea520e8e8fcba281c8cf68ad77021eb3dd22
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/104825
Reviewed-by: David Neto <dneto@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>

src/tint/transform/substitute_override.cc

@@ -17,6 +17,8 @@
 #include <functional>
 
 #include "src/tint/program_builder.h"
+#include "src/tint/sem/builtin.h"
+#include "src/tint/sem/index_accessor_expression.h"
 #include "src/tint/sem/variable.h"
 
 TINT_INSTANTIATE_TYPEINFO(tint::transform::SubstituteOverride);
@@ -82,6 +84,25 @@ void SubstituteOverride::Run(CloneContext& ctx, const DataMap& config, DataMap&)
         return ctx.dst->Const(src, sym, ty, ctor);
     });
 
+    // Ensure that objects that are indexed with an override-expression are materialized.
+    // If the object is not materialized, and the 'override' variable is turned to a 'const', the
+    // resulting type of the index may change. See: crbug.com/tint/1697.
+    ctx.ReplaceAll(
+        [&](const ast::IndexAccessorExpression* expr) -> const ast::IndexAccessorExpression* {
+            if (auto* sem = ctx.src->Sem().Get(expr)) {
+                if (auto* access = sem->UnwrapMaterialize()->As<sem::IndexAccessorExpression>()) {
+                    if (access->Object()->UnwrapMaterialize()->Type()->HoldsAbstract() &&
+                        access->Index()->Stage() == sem::EvaluationStage::kOverride) {
+                        auto& b = *ctx.dst;
+                        auto* obj = b.Call(sem::str(sem::BuiltinType::kTintMaterialize),
+                                           ctx.Clone(expr->object));
+                        return b.IndexAccessor(obj, ctx.Clone(expr->index));
+                    }
+                }
+            }
+            return nullptr;
+        });
+
     ctx.Clone();
 }
 

src/tint/transform/substitute_override_test.cc

@@ -238,5 +238,46 @@ fn main() -> @builtin(position) vec4<f32> {
     EXPECT_EQ(expect, str(got));
 }
 
+TEST_F(SubstituteOverrideTest, IndexMaterialization) {
+    auto* src = R"(
+override O = 0; // Try switching to 'const'
+
+fn f() {
+  const smaller_than_any_f32 = 1e-50;
+  const large_float = 1e27;
+  // When O is an override, the outer index value is not constant, so the
+  // value is not calculated at shader-creation time, and does not error.
+  //
+  // When O is a const, and 'smaller_than_any_f32' *is not* materialized, the
+  // outer index value will evaluate to 10000, resulting in an out-of-bounds
+  // error.
+  //
+  // When O is a const, and 'smaller_than_any_f32' *is* materialized, the
+  // materialization of 'smaller_than_any_f32' to f32 will evaluate to zero,
+  // and so the outer index value will be zero, and we get no error.
+  _ = vec2(0)[i32(vec2(smaller_than_any_f32)[O]*large_float*large_float)];
+}
+)";
+
+    auto* expect = R"(
+const O = 0i;
+
+fn f() {
+  const smaller_than_any_f32 = 1e-50;
+  const large_float = 1000000000000000013287555072.0;
+  _ = _tint_materialize(vec2(0))[i32(((_tint_materialize(vec2(smaller_than_any_f32))[O] * large_float) * large_float))];
+}
+)";
+
+    SubstituteOverride::Config cfg;
+    cfg.map.insert({OverrideId{0}, 0.0});
+
+    DataMap data;
+    data.Add<SubstituteOverride::Config>(cfg);
+    auto got = Run<SubstituteOverride>(src, data);
+
+    EXPECT_EQ(expect, str(got));
+}
+
 }  // namespace
 }  // namespace tint::transform

test/tint/bug/tint/1697.wgsl

@@ -0,0 +1,18 @@
+// flags: --transform substitute_override
+
+override O = 0; // Try switching to 'const'
+
+fn f() {
+  const smaller_than_any_f32 = 1e-50;
+  // When O is an override, the outer index value is not constant, so the
+  // value is not calculated at shader-creation time, and does not error.
+  //
+  // When O is a const, and 'smaller_than_any_f32' *is not* materialized, the
+  // outer index value will evaluate to 10000, resulting in an out-of-bounds
+  // error.
+  //
+  // When O is a const, and 'smaller_than_any_f32' *is* materialized,
+  // the materialization of 'smaller_than_any_f32' to f32 will evaluate to zero,
+  // and so the outer index value will be zero, and we get no error.
+  var v = vec2(0)[i32(vec2(smaller_than_any_f32)[O]*1e27*1e27)];
+}

test/tint/bug/tint/1697.wgsl.expected.dxc.hlsl

@@ -0,0 +1,8 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+  return;
+}
+
+void f() {
+  int v = 0;
+}

test/tint/bug/tint/1697.wgsl.expected.fxc.hlsl

@@ -0,0 +1,8 @@
+[numthreads(1, 1, 1)]
+void unused_entry_point() {
+  return;
+}
+
+void f() {
+  int v = 0;
+}

test/tint/bug/tint/1697.wgsl.expected.glsl

@@ -0,0 +1,10 @@
+#version 310 es
+
+layout(local_size_x = 1, local_size_y = 1, local_size_z = 1) in;
+void unused_entry_point() {
+  return;
+}
+void f() {
+  int v = 0;
+}
+

test/tint/bug/tint/1697.wgsl.expected.msl

@@ -0,0 +1,7 @@
+#include <metal_stdlib>
+
+using namespace metal;
+void f() {
+  int v = 0;
+}
+

test/tint/bug/tint/1697.wgsl.expected.spvasm

@@ -0,0 +1,27 @@
+; SPIR-V
+; Version: 1.3
+; Generator: Google Tint Compiler; 0
+; Bound: 11
+; Schema: 0
+               OpCapability Shader
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %unused_entry_point "unused_entry_point"
+               OpExecutionMode %unused_entry_point LocalSize 1 1 1
+               OpName %unused_entry_point "unused_entry_point"
+               OpName %f "f"
+               OpName %v "v"
+       %void = OpTypeVoid
+          %1 = OpTypeFunction %void
+        %int = OpTypeInt 32 1
+          %8 = OpConstantNull %int
+%_ptr_Function_int = OpTypePointer Function %int
+%unused_entry_point = OpFunction %void None %1
+          %4 = OpLabel
+               OpReturn
+               OpFunctionEnd
+          %f = OpFunction %void None %1
+          %6 = OpLabel
+          %v = OpVariable %_ptr_Function_int Function %8
+               OpStore %v %8
+               OpReturn
+               OpFunctionEnd

test/tint/bug/tint/1697.wgsl.expected.wgsl

@@ -0,0 +1,6 @@
+const O = 0;
+
+fn f() {
+  const smaller_than_any_f32 = 1e-50;
+  var v = _tint_materialize(vec2(0))[i32(((_tint_materialize(vec2(smaller_than_any_f32))[O] * 1000000000000000013287555072.0) * 1000000000000000013287555072.0))];
+}