Correctly implement WebGPU related flag exclusions (#2737)

Fixes #2736

test/tools/expect.py

@@ -79,6 +79,15 @@ class ReturnCodeIsZero(SpirvTest):
     return True, ''
 
 
+class ReturnCodeIsNonZero(SpirvTest):
+  """Mixin class for checking that the return code is not zero."""
+
+  def check_return_code_is_nonzero(self, status):
+    if not status.returncode:
+      return False, 'return code is 0'
+    return True, ''
+
+
 class NoOutputOnStdout(SpirvTest):
   """Mixin class for checking that there is no output on stdout."""
 

test/tools/opt/flags.py

@@ -331,3 +331,45 @@ class TestLoopPeelingThresholdArgsInvalidNumber(expect.ErrorMessageSubstr):
 
   spirv_args = ['--loop-peeling-threshold=a10f']
   expected_error_substr = 'must have a positive integer argument'
+
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestWebGPUToVulkanThenVulkanToWebGPUIsInvalid(expect.ReturnCodeIsNonZero, expect.ErrorMessageSubstr):
+  """Tests Vulkan->WebGPU flag cannot be used after WebGPU->Vulkan flag."""
+
+  spirv_args = ['--webgpu-to-vulkan', '--vulkan-to-webgpu']
+  expected_error_substr = 'Cannot use both'
+
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestVulkanToWebGPUThenWebGPUToVulkanIsInvalid(expect.ReturnCodeIsNonZero, expect.ErrorMessageSubstr):
+  """Tests WebGPU->Vulkan flag cannot be used after Vulkan->WebGPU flag."""
+
+  spirv_args = ['--vulkan-to-webgpu', '--webgpu-to-vulkan']
+  expected_error_substr = 'Cannot use both'
+
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestTargetEnvThenVulkanToWebGPUIsInvalid(expect.ReturnCodeIsNonZero, expect.ErrorMessageSubstr):
+  """Tests Vulkan->WebGPU flag cannot be used after target env flag."""
+
+  spirv_args = ['--target-env=opengl4.0', '--vulkan-to-webgpu']
+  expected_error_substr = 'defines the target environment'
+
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestVulkanToWebGPUThenTargetEnvIsInvalid(expect.ReturnCodeIsNonZero, expect.ErrorMessageSubstr):
+  """Tests target env flag cannot be used after Vulkan->WebGPU flag."""
+
+  spirv_args = ['--vulkan-to-webgpu', '--target-env=opengl4.0']
+  expected_error_substr = 'defines the target environment'
+
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestTargetEnvThenWebGPUToVulkanIsInvalid(expect.ReturnCodeIsNonZero, expect.ErrorMessageSubstr):
+  """Tests WebGPU->Vulkan flag cannot be used after target env flag."""
+
+  spirv_args = ['--target-env=opengl4.0', '--webgpu-to-vulkan']
+  expected_error_substr = 'defines the target environment'
+
+@inside_spirv_testsuite('SpirvOptFlags')
+class TestWebGPUToVulkanThenTargetEnvIsInvalid(expect.ReturnCodeIsNonZero, expect.ErrorMessageSubstr):
+  """Tests target env flag cannot be used after WebGPU->Vulkan flag."""
+
+  spirv_args = ['--webgpu-to-vulkan', '--target-env=opengl4.0']
+  expected_error_substr = 'defines the target environment'

tools/opt/opt.cpp

@@ -721,16 +721,17 @@ OptStatus ParseFlags(int argc, const char** argv,
                                              max_id_bound);
       } else if (0 == strncmp(cur_arg,
                               "--target-env=", sizeof("--target-env=") - 1)) {
+        target_env_set = true;
         if (vulkan_to_webgpu_set) {
           spvtools::Error(opt_diagnostic, nullptr, {},
-                          "Cannot use both --vulkan-to-webgpu and --target-env "
-                          "at the same time");
+                          "--vulkan-to-webgpu defines the target environment, "
+                          "so --target-env cannot be set at the same time");
           return {OPT_STOP, 1};
         }
         if (webgpu_to_vulkan_set) {
           spvtools::Error(opt_diagnostic, nullptr, {},
-                          "Cannot use both --webgpu-to-vulkan and --target-env "
-                          "at the same time");
+                          "--webgpu-to-vulkan defines the target environment, "
+                          "so --target-env cannot be set at the same time");
           return {OPT_STOP, 1};
         }
         const auto split_flag = spvtools::utils::SplitFlagArgs(cur_arg);
@@ -743,32 +744,36 @@ OptStatus ParseFlags(int argc, const char** argv,
         }
         optimizer->SetTargetEnv(target_env);
       } else if (0 == strcmp(cur_arg, "--vulkan-to-webgpu")) {
+        vulkan_to_webgpu_set = true;
         if (target_env_set) {
           spvtools::Error(opt_diagnostic, nullptr, {},
-                          "Cannot use both --vulkan-to-webgpu and --target-env "
-                          "at the same time");
+                          "--vulkan-to-webgpu defines the target environment, "
+                          "so --target-env cannot be set at the same time");
           return {OPT_STOP, 1};
         }
         if (webgpu_to_vulkan_set) {
           spvtools::Error(opt_diagnostic, nullptr, {},
-                          "Cannot use both --vulkan-to-webgpu and "
-                          "--webgpu-to-vulkan at the same time");
+                          "Cannot use both --webgpu-to-vulkan and "
+                          "--vulkan-to-webgpu at the same time, invoke twice "
+                          "if you are wanting to go to and from");
           return {OPT_STOP, 1};
         }
 
         optimizer->SetTargetEnv(SPV_ENV_WEBGPU_0);
         optimizer->RegisterVulkanToWebGPUPasses();
       } else if (0 == strcmp(cur_arg, "--webgpu-to-vulkan")) {
+        webgpu_to_vulkan_set = true;
         if (target_env_set) {
           spvtools::Error(opt_diagnostic, nullptr, {},
-                          "Cannot use both --webgpu-to-vulkan and --target-env "
-                          "at the same time");
+                          "--webgpu-to-vulkan defines the target environment, "
+                          "so --target-env cannot be set at the same time");
           return {OPT_STOP, 1};
         }
         if (vulkan_to_webgpu_set) {
           spvtools::Error(opt_diagnostic, nullptr, {},
                           "Cannot use both --webgpu-to-vulkan and "
-                          "--vulkan-to-webgpu at the same time");
+                          "--vulkan-to-webgpu at the same time, invoke twice "
+                          "if you are wanting to go to and from");
           return {OPT_STOP, 1};
         }