Re-enable ValidationAfterOverrideStorageSize test

Also move Robustness transform to pipeline creation time on Vulkan
backend.

Bug: tint:1660, dawn:1041
Change-Id: I10220f34119d11f29be86fd29463a282175eccdd
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/103780
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Shrek Shao <shrekshao@google.com>

src/dawn/native/null/DeviceNull.cpp

@@ -390,10 +390,12 @@ MaybeError ComputePipeline::Initialize() {
 
     tint::Program transformedProgram;
     const tint::Program* program;
-    if (!computeStage.metadata->overrides.empty()) {
     tint::transform::Manager transformManager;
     tint::transform::DataMap transformInputs;
 
+    transformManager.Add<tint::transform::Robustness>();
+
+    if (!computeStage.metadata->overrides.empty()) {
         transformManager.Add<tint::transform::SingleEntryPoint>();
         transformInputs.Add<tint::transform::SingleEntryPoint::Config>(
             computeStage.entryPoint.c_str());
@@ -403,15 +405,13 @@ MaybeError ComputePipeline::Initialize() {
         transformManager.Add<tint::transform::SubstituteOverride>();
         transformInputs.Add<tint::transform::SubstituteOverride::Config>(
             BuildSubstituteOverridesTransformConfig(computeStage));
+    }
 
     DAWN_TRY_ASSIGN(transformedProgram,
                     RunTransforms(&transformManager, computeStage.module->GetTintProgram(),
                                   transformInputs, nullptr, nullptr));
 
     program = &transformedProgram;
-    } else {
-        program = computeStage.module->GetTintProgram();
-    }
 
     // Do the workgroup size validation as it is actually backend agnostic.
     const CombinedLimits& limits = GetDevice()->GetLimits();

src/dawn/native/vulkan/ShaderModuleVk.cpp

@@ -172,19 +172,7 @@ ShaderModule::ShaderModule(Device* device, const ShaderModuleDescriptor* descrip
 
 MaybeError ShaderModule::Initialize(ShaderModuleParseResult* parseResult,
                                     OwnedCompilationMessages* compilationMessages) {
-    if (GetDevice()->IsRobustnessEnabled()) {
     ScopedTintICEHandler scopedICEHandler(GetDevice());
-
-        tint::transform::Robustness robustness;
-        tint::transform::DataMap transformInputs;
-
-        tint::Program program;
-        DAWN_TRY_ASSIGN(program, RunTransforms(&robustness, parseResult->tintProgram.get(),
-                                               transformInputs, nullptr, nullptr));
-        // Rather than use a new ParseResult object, we just reuse the original parseResult
-        parseResult->tintProgram = std::make_unique<tint::Program>(std::move(program));
-    }
-
     return InitializeBase(parseResult, compilationMessages);
 }
 
@@ -204,6 +192,7 @@ ShaderModule::~ShaderModule() = default;
     X(std::optional<tint::transform::SubstituteOverride::Config>, substituteOverrideConfig) \
     X(LimitsForCompilationRequest, limits)                                                  \
     X(std::string_view, entryPointName)                                                     \
+    X(bool, isRobustnessEnabled)                                                            \
     X(bool, disableWorkgroupInit)                                                           \
     X(bool, useZeroInitializeWorkgroupMemoryExtension)                                      \
     X(CacheKey::UnsafeUnkeyedValue<dawn::platform::Platform*>, tracePlatform)
@@ -284,6 +273,7 @@ ResultOrError<ShaderModule::ModuleAndSpirv> ShaderModule::GetHandleAndSpirv(
     req.bindingPoints = std::move(bindingPoints);
     req.newBindingsMap = std::move(newBindingsMap);
     req.entryPointName = programmableStage.entryPoint;
+    req.isRobustnessEnabled = GetDevice()->IsRobustnessEnabled();
     req.disableWorkgroupInit = GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
     req.useZeroInitializeWorkgroupMemoryExtension =
         GetDevice()->IsToggleEnabled(Toggle::VulkanUseZeroInitializeWorkgroupMemoryExtension);
@@ -298,6 +288,9 @@ ResultOrError<ShaderModule::ModuleAndSpirv> ShaderModule::GetHandleAndSpirv(
         spirv, GetDevice(), std::move(req), Spirv::FromBlob,
         [](SpirvCompilationRequest r) -> ResultOrError<Spirv> {
             tint::transform::Manager transformManager;
+            if (r.isRobustnessEnabled) {
+                transformManager.append(std::make_unique<tint::transform::Robustness>());
+            }
             // Many Vulkan drivers can't handle multi-entrypoint shader modules.
             transformManager.append(std::make_unique<tint::transform::SingleEntryPoint>());
             // Run the binding remapper after SingleEntryPoint to avoid collisions with

src/dawn/tests/end2end/ShaderValidationTests.cpp

@@ -330,8 +330,7 @@ TEST_P(WorkgroupSizeValidationTest, ValidationAfterOverride) {
 
 // Test workgroup size validation after being overrided with invalid values (storage size limits
 // validation).
-// TODO(tint:1660): re-enable after override can be used as array size.
+TEST_P(WorkgroupSizeValidationTest, ValidationAfterOverrideStorageSize) {
-TEST_P(WorkgroupSizeValidationTest, DISABLED_ValidationAfterOverrideStorageSize) {
     wgpu::Limits supportedLimits = GetSupportedLimits().limits;
 
     constexpr uint32_t kVec4Size = 16;
@@ -347,11 +346,11 @@ TEST_P(WorkgroupSizeValidationTest, DISABLED_ValidationAfterOverrideStorageSize)
         ss << "override b: u32;";
         if (vec4_count > 0) {
             ss << "var<workgroup> vec4_data: array<vec4<f32>, a>;";
-            body << "_ = vec4_data;";
+            body << "_ = vec4_data[0];";
         }
         if (mat4_count > 0) {
             ss << "var<workgroup> mat4_data: array<mat4x4<f32>, b>;";
-            body << "_ = mat4_data;";
+            body << "_ = mat4_data[0];";
         }
         ss << "@compute @workgroup_size(1) fn main() { " << body.str() << " }";