commit b3d0f3866eb25a27733accafeadaac6e0b4db45f
author Corentin Wallez <cwallez@chromium.org> Wed Feb 07 15:36:23 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Feb 07 15:36:23 2024 +0000
tree 3b389d496de3eb1a86321a79d88154e4afe0867c
parent 71161fd2b1223f835e50aa026d48069d74567a2b

vulkan: Emit SPIR-V PointSize only when rendering point lists

Otherwise we consume one additional vertex output location than what's
accounted for in the WebGPU spec and can run over the Vulkan limit in
MaxLimitTests.MaxInterStageVariable*.

This was caught by the VVL but not reported due to an unrelated issue
that will be fixed in a follow-up CL.

Bug: chromium:323753236
Change-Id: I9e9f963be0cdb508ecd42b150e6365284696cad1
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/173681
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn/native/vulkan/ComputePipelineVk.cpp

diff --git a/src/dawn/native/vulkan/ComputePipelineVk.cpp b/src/dawn/native/vulkan/ComputePipelineVk.cpp
index 1f059c2..225ea5f 100644
--- a/src/dawn/native/vulkan/ComputePipelineVk.cpp
+++ b/src/dawn/native/vulkan/ComputePipelineVk.cpp
@@ -79,6 +79,7 @@
         module->GetHandleAndSpirv(
             SingleShaderStage::Compute, computeStage, layout,
             /*clampFragDepth*/ false,
+            /*emitPointSize*/ false,
             /* maxSubgroupSizeForFullSubgroups */
             IsFullSubgroupsRequired()
                 ? std::make_optional(device->GetLimits().experimentalSubgroupLimits.maxSubgroupSize)

src/dawn/native/vulkan/RenderPipelineVk.cpp

diff --git a/src/dawn/native/vulkan/RenderPipelineVk.cpp b/src/dawn/native/vulkan/RenderPipelineVk.cpp
index 3d3736e..1dc8682 100644
--- a/src/dawn/native/vulkan/RenderPipelineVk.cpp
+++ b/src/dawn/native/vulkan/RenderPipelineVk.cpp
@@ -385,13 +385,13 @@
     uint32_t stageCount = 0;
 
     auto AddShaderStage = [&](SingleShaderStage stage, VkShaderStageFlagBits vkStage,
-                              bool clampFragDepth) -> MaybeError {
+                              bool clampFragDepth, bool emitPointSize) -> MaybeError {
         const ProgrammableStage& programmableStage = GetStage(stage);
         ShaderModule::ModuleAndSpirv moduleAndSpirv;
-        DAWN_TRY_ASSIGN(moduleAndSpirv,
-                        ToBackend(programmableStage.module)
-                            ->GetHandleAndSpirv(stage, programmableStage, layout, clampFragDepth,
-                                                /* fullSubgroups */ {}));
+        DAWN_TRY_ASSIGN(moduleAndSpirv, ToBackend(programmableStage.module)
+                                            ->GetHandleAndSpirv(stage, programmableStage, layout,
+                                                                clampFragDepth, emitPointSize,
+                                                                /* fullSubgroups */ {}));
         // Record cache key for each shader since it will become inaccessible later on.
         StreamIn(&mCacheKey, stream::Iterable(moduleAndSpirv.spirv, moduleAndSpirv.wordCount));
 
@@ -411,13 +411,14 @@
 
     // Add the vertex stage that's always present.
     DAWN_TRY(AddShaderStage(SingleShaderStage::Vertex, VK_SHADER_STAGE_VERTEX_BIT,
-                            /*clampFragDepth*/ false));
+                            /*clampFragDepth*/ false,
+                            GetPrimitiveTopology() == wgpu::PrimitiveTopology::PointList));
 
     // Add the fragment stage if present.
     if (GetStageMask() & wgpu::ShaderStage::Fragment) {
         bool clampFragDepth = UsesFragDepth() && !HasUnclippedDepth();
         DAWN_TRY(AddShaderStage(SingleShaderStage::Fragment, VK_SHADER_STAGE_FRAGMENT_BIT,
-                                clampFragDepth));
+                                clampFragDepth, /*emitPointSize*/ false));
     }
 
     PipelineVertexInputStateCreateInfoTemporaryAllocations tempAllocations;

src/dawn/native/vulkan/ShaderModuleVk.cpp

diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 1938cbc..e498fea 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -211,6 +211,7 @@
     const ProgrammableStage& programmableStage,
     const PipelineLayout* layout,
     bool clampFragDepth,
+    bool emitPointSize,
     std::optional<uint32_t> maxSubgroupSizeForFullSubgroups) {
     TRACE_EVENT0(GetDevice()->GetPlatform(), General, "ShaderModuleVk::GetHandleAndSpirv");
 
@@ -329,7 +330,7 @@
 
     req.tintOptions.clamp_frag_depth = clampFragDepth;
     req.tintOptions.disable_robustness = !GetDevice()->IsRobustnessEnabled();
-    req.tintOptions.emit_vertex_point_size = true;
+    req.tintOptions.emit_vertex_point_size = emitPointSize;
     req.tintOptions.disable_workgroup_init =
         GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
     req.tintOptions.use_zero_initialize_workgroup_memory_extension =

src/dawn/native/vulkan/ShaderModuleVk.h

diff --git a/src/dawn/native/vulkan/ShaderModuleVk.h b/src/dawn/native/vulkan/ShaderModuleVk.h
index 63b415b..ba3b4ee 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.h
+++ b/src/dawn/native/vulkan/ShaderModuleVk.h
@@ -83,6 +83,7 @@
         const ProgrammableStage& programmableStage,
         const PipelineLayout* layout,
         bool clampFragDepth,
+        bool emitPointSize,
         std::optional<uint32_t> maxSubgroupSizeForFullSubgroups);
 
   private: