[spirv] Move descriptor scalar replacement to legalization stage. (#3261

) * [spirv] `-fspv-flatten-resource-arrays` is not needed for flattening global struct of resources. * [spirv] Move descriptor scalar replacement to legalization stage.
microsoft · Nov 16, 2020 · 78515c2 · 78515c2
1 parent f1f6064
commit 78515c2
Show file tree

Hide file tree

Showing 12 changed files with 45 additions and 18 deletions.
diff --git a/tools/clang/lib/SPIRV/SpirvEmitter.cpp b/tools/clang/lib/SPIRV/SpirvEmitter.cpp
@@ -11937,15 +11937,6 @@ bool SpirvEmitter::spirvToolsOptimize(std::vector<uint32_t> *mod,
     // Add performance passes.
     optimizer.RegisterPerformancePasses();
 
-    // Add flattening of resources if needed.
-    if (spirvOptions.flattenResourceArrays ||
-        declIdMapper.requiresFlatteningCompositeResources()) {
-      optimizer.RegisterPass(spvtools::CreateDescriptorScalarReplacementPass());
-      // ADCE should be run after desc_sroa in order to remove potentially
-      // illegal types such as structures containing opaque types.
-      optimizer.RegisterPass(spvtools::CreateAggressiveDCEPass());
-    }
-
     // Add compact ID pass.
     optimizer.RegisterPass(spvtools::CreateCompactIdsPass());
   } else {
@@ -11972,6 +11963,14 @@ bool SpirvEmitter::spirvToolsLegalize(std::vector<uint32_t> *mod,
   spvtools::OptimizerOptions options;
   options.set_run_validator(false);
   optimizer.RegisterLegalizationPasses();
+  // Add flattening of resources if needed.
+  if (spirvOptions.flattenResourceArrays ||
+      declIdMapper.requiresFlatteningCompositeResources()) {
+    optimizer.RegisterPass(spvtools::CreateDescriptorScalarReplacementPass());
+    // ADCE should be run after desc_sroa in order to remove potentially
+    // illegal types such as structures containing opaque types.
+    optimizer.RegisterPass(spvtools::CreateAggressiveDCEPass());
+  }
   optimizer.RegisterPass(spvtools::CreateReplaceInvalidOpcodePass());
   optimizer.RegisterPass(spvtools::CreateCompactIdsPass());
 

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resource-mix.error.1.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resource-mix.error.1.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+// Run: %dxc -T ps_6_0 -E main
 
 struct S {
   Texture2D t[2];

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resource-mix.error.2.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resource-mix.error.2.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+// Run: %dxc -T ps_6_0 -E main
 
 struct T {
   float2x3 a[4];

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.1.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.1.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+// Run: %dxc -T ps_6_0 -E main
 
 // globalS.t should take binding #0.
 // globalS.s should take binding #1.

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.2.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.2.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+// Run: %dxc -T ps_6_0 -E main
 
 // globalS[0][0].t should take binding #0.
 // globalS[0][0].s should take binding #1.

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.3.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.3.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+// Run: %dxc -T ps_6_0 -E main
 
 // globalS.t should take binding #0.
 // globalS.s should take binding #1.

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.4.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.4.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+// Run: %dxc -T ps_6_0 -E main
 
 // globalS[0].t[0] should take binding #0.
 // globalS[0].t[1] should take binding #1.

diff --git a/.../clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.contains-buffer-error.hlsl b/.../clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.contains-buffer-error.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays
+// Run: %dxc -T ps_6_0 -E main
 
 struct S {
   Texture2D t[2];

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.optimized.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.optimized.hlsl
@@ -1,4 +1,4 @@
-// Run: %dxc -T ps_6_0 -E main -fspv-flatten-resource-arrays -O3
+// Run: %dxc -T ps_6_0 -E main -O3
 
 // Check the names
 //

diff --git a/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.pass-legalization.hlsl b/tools/clang/test/CodeGenSPIRV/vk.binding.global-struct-of-resources.pass-legalization.hlsl
@@ -0,0 +1,23 @@
+// Run: %dxc -T ps_6_0 -E main -O0
+
+// CHECK: OpDecorate %g_CombinedTextureSampler_t DescriptorSet 0
+// CHECK: OpDecorate %g_CombinedTextureSampler_t Binding 0
+// CHECK: OpDecorate %g_CombinedTextureSampler_s DescriptorSet 0
+// CHECK: OpDecorate %g_CombinedTextureSampler_s Binding 1
+
+struct sampler2D {
+  Texture2D t;
+  SamplerState s;
+};
+float4 tex2D(sampler2D x, float2 v) { return x.t.Sample(x.s, v); }
+
+struct v2f {
+  float4 vertex : SV_POSITION;
+  float2 uv : TEXCOORD0;
+};
+
+sampler2D g_CombinedTextureSampler;
+
+float4 main(v2f i) : SV_Target {
+  return tex2D(g_CombinedTextureSampler, i.uv);
+}
diff --git a/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp b/tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp
@@ -1859,6 +1859,11 @@ TEST_F(FileTest, BindingStructureOfResourcesContainsBufferError) {
       "vk.binding.global-struct-of-resources.contains-buffer-error.hlsl",
       Expect::Failure);
 }
+TEST_F(FileTest, BindingStructureOfResourcesPassLegalization) {
+  runFileTest("vk.binding.global-struct-of-resources.pass-legalization.hlsl",
+              Expect::Success,
+              /*runValidation*/ true);
+}
 
 TEST_F(FileTest, VulkanPushConstant) { runFileTest("vk.push-constant.hlsl"); }
 TEST_F(FileTest, VulkanPushConstantOffset) {

diff --git a/tools/clang/unittests/SPIRV/FileTestUtils.cpp b/tools/clang/unittests/SPIRV/FileTestUtils.cpp
@@ -156,7 +156,7 @@ bool runCompilerWithSpirvGeneration(const llvm::StringRef inputFilePath,
 
     bool requires_opt = false;
     for (const auto &arg : rest)
-      if (arg == L"-O3" || arg.substr(0, 8) == L"-Oconfig")
+      if (arg == L"-O3" || arg == L"-O0" || arg.substr(0, 8) == L"-Oconfig")
         requires_opt = true;
 
     std::vector<LPCWSTR> flags;