Repository: KhronosGroup/SPIRV-Tools (language: python)

Issue:
Strip unused capabilities (i.e. Float64, PhysicalStorageBuffer)
It has come to our attention that DXC likes to emit SPIR-V capabilities even if the code using them is not reachable, for example `Float64` being declared here even though nobody is using the `static double` we declared
https://godbolt.org/z/ca5M318GE

The reason we're filing this against SPIRV-Tools and not DXC is because we know DXC uses spv-opt to legalize the SPIR-V it emits, and we believe that the legalization should strip unused extensions and capabilities.


Discussion / hints:
A capabilities trimming pass has been added recently: https://github.com/KhronosGroup/SPIRV-Tools/blob/main/source/opt/trim_capabilities_pass.cpp. This is now automatically called by DXC, and capabilities are being added on a case by case basis.

Fix patch:
diff --git a/source/opt/trim_capabilities_pass.cpp b/source/opt/trim_capabilities_pass.cpp
index 4b3d74af65..5df1999140 100644
--- a/source/opt/trim_capabilities_pass.cpp
+++ b/source/opt/trim_capabilities_pass.cpp
@@ -36,6 +36,7 @@ namespace spvtools {
 namespace opt {
 
 namespace {
+constexpr uint32_t kOpTypeFloatSizeIndex = 0;
 constexpr uint32_t kOpTypePointerStorageClassIndex = 0;
 constexpr uint32_t kTypeArrayTypeIndex = 0;
 constexpr uint32_t kOpTypeScalarBitWidthIndex = 0;
@@ -132,6 +133,16 @@ static bool Has16BitCapability(const FeatureManager* feature_manager) {
 // Handler names follow the following convention:
 //  Handler_<Opcode>_<Capability>()
 
+static std::optional<spv::Capability> Handler_OpTypeFloat_Float64(
+    const Instruction* instruction) {
+  assert(instruction->opcode() == spv::Op::OpTypeFloat &&
+         "This handler only support OpTypeFloat opcodes.");
+
+  const uint32_t size =
+      instruction->GetSingleWordInOperand(kOpTypeFloatSizeIndex);
+  return size == 64 ? std::optional(spv::Capability::Float64) : std::nullopt;
+}
+
 static std::optional<spv::Capability>
 Handler_OpTypePointer_StorageInputOutput16(const Instruction* instruction) {
   assert(instruction->opcode() == spv::Op::OpTypePointer &&
@@ -286,8 +297,9 @@ static std::optional<spv::Capability> Handler_OpTypeImage_ImageMSArray(
 }
 
 // Opcode of interest to determine capabilities requirements.
-constexpr std::array<std::pair<spv::Op, OpcodeHandler>, 7> kOpcodeHandlers{{
+constexpr std::array<std::pair<spv::Op, OpcodeHandler>, 8> kOpcodeHandlers{{
     // clang-format off
+    {spv::Op::OpTypeFloat,   Handler_OpTypeFloat_Float64 },
     {spv::Op::OpTypeImage,   Handler_OpTypeImage_ImageMSArray},
     {spv::Op::OpTypeInt,     Handler_OpTypeInt_Int64 },
     {spv::Op::OpTypePointer, Handler_OpTypePointer_StorageInputOutput16},
diff --git a/source/opt/trim_capabilities_pass.h b/source/opt/trim_capabilities_pass.h
index b9ad7a938e..9202b2e9af 100644
--- a/source/opt/trim_capabilities_pass.h
+++ b/source/opt/trim_capabilities_pass.h
@@ -74,6 +74,7 @@ class TrimCapabilitiesPass : public Pass {
   // contains unsupported instruction, the pass could yield bad results.
   static constexpr std::array kSupportedCapabilities{
       // clang-format off
+      spv::Capability::Float64,
       spv::Capability::FragmentShaderPixelInterlockEXT,
       spv::Capability::FragmentShaderSampleInterlockEXT,
       spv::Capability::FragmentShaderShadingRateInterlockEXT,


Test patch:
diff --git a/test/opt/trim_capabilities_pass_test.cpp b/test/opt/trim_capabilities_pass_test.cpp
index d7bdafe1c0..8aaf860dc9 100644
--- a/test/opt/trim_capabilities_pass_test.cpp
+++ b/test/opt/trim_capabilities_pass_test.cpp
@@ -2090,6 +2090,45 @@ TEST_F(TrimCapabilitiesPassTest, ImageMSArray_RemovedIfSampledNot2) {
   EXPECT_EQ(std::get<1>(result), Pass::Status::SuccessWithChange);
 }
 
+TEST_F(TrimCapabilitiesPassTest, Float64_RemovedWhenUnused) {
+  const std::string kTest = R"(
+               OpCapability Float64
+; CHECK-NOT:   OpCapability Float64
+               OpCapability Shader
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %1 "main"
+       %void = OpTypeVoid
+          %3 = OpTypeFunction %void
+          %1 = OpFunction %void None %3
+          %6 = OpLabel
+               OpReturn
+               OpFunctionEnd;
+  )";
+  const auto result =
+      SinglePassRunAndMatch<TrimCapabilitiesPass>(kTest, /* skip_nop= */ false);
+  EXPECT_EQ(std::get<1>(result), Pass::Status::SuccessWithChange);
+}
+
+TEST_F(TrimCapabilitiesPassTest, Float64_RemainsWhenUsed) {
+  const std::string kTest = R"(
+               OpCapability Float64
+; CHECK:       OpCapability Float64
+               OpCapability Shader
+               OpMemoryModel Logical GLSL450
+               OpEntryPoint GLCompute %1 "main"
+       %void = OpTypeVoid
+      %float = OpTypeFloat 64
+          %3 = OpTypeFunction %void
+          %1 = OpFunction %void None %3
+          %6 = OpLabel
+               OpReturn
+               OpFunctionEnd;
+  )";
+  const auto result =
+      SinglePassRunAndMatch<TrimCapabilitiesPass>(kTest, /* skip_nop= */ false);
+  EXPECT_EQ(std::get<1>(result), Pass::Status::SuccessWithoutChange);
+}
+
 }  // namespace
 }  // namespace opt
 }  // namespace spvtools