[spirv] Adopt new changes in SPIRV-Tools (#846)

Uses the legalization pass recipe from SPIRV-Tools.

Also change -fcgl to disable both legalization and optimization.

Validation on some tests are turned off because of new image
instruction checking in SPIRV-Tools. Enabling them is a TODO.

external/SPIRV-Tools

@@ -1 +1 @@
-Subproject commit e28edd458b729da7bbfd51e375feb33103709e6f
+Subproject commit aec60b815806a0de1de4e56f747c4a9673601d73

include/dxc/Support/HLSLOptions.h

@@ -158,7 +158,6 @@ public:
   // SPIRV Change Starts
 #ifdef ENABLE_SPIRV_CODEGEN
   bool GenSPIRV; // OPT_spirv
-  bool DisableSpirvLegalization; // OPT_fcgl
   llvm::StringRef VkStageIoOrder; // OPT_fvk_stage_io_order
   llvm::SmallVector<uint32_t, 4> VkBShift; // OPT_fvk_b_shift
   llvm::SmallVector<uint32_t, 4> VkTShift; // OPT_fvk_t_shift

lib/DxcSupport/HLSLOptions.cpp

@@ -481,7 +481,6 @@ int ReadDxcOpts(const OptTable *optionTable, unsigned flagsToInclude,
   // SPIRV Change Starts
 #ifdef ENABLE_SPIRV_CODEGEN
   const bool genSpirv = opts.GenSPIRV = Args.hasFlag(OPT_spirv, OPT_INVALID, false);
-  opts.DisableSpirvLegalization = Args.hasFlag(OPT_fcgl, OPT_INVALID, false);
 
   // Collects the arguments for -fvk-{b|s|t|u}-shift.
   const auto handleVkShiftArgs = [genSpirv, &Args, &errors](

tools/clang/include/clang/SPIRV/EmitSPIRVOptions.h

@@ -15,7 +15,8 @@
 namespace clang {
 /// Structs for controlling behaviors of SPIR-V codegen.
 struct EmitSPIRVOptions {
-  bool disableLegalization;
+  /// Disable legalization and optimization and emit raw SPIR-V
+  bool codeGenHighLevel;
   llvm::StringRef stageIoOrder;
   llvm::SmallVector<uint32_t, 4> bShift;
   llvm::SmallVector<uint32_t, 4> tShift;

tools/clang/lib/SPIRV/SPIRVEmitter.cpp

@@ -182,24 +182,9 @@ bool spirvToolsLegalize(std::vector<uint32_t> *module, std::string *messages) {
                  const spv_position_t & /*position*/,
                  const char *message) { *messages += message; });
 
-  optimizer.RegisterPass(spvtools::CreateInlineExhaustivePass());
-  optimizer.RegisterPass(spvtools::CreateLocalAccessChainConvertPass());
-  optimizer.RegisterPass(spvtools::CreateLocalSingleBlockLoadStoreElimPass());
-  optimizer.RegisterPass(spvtools::CreateLocalSingleStoreElimPass());
-  optimizer.RegisterPass(spvtools::CreateInsertExtractElimPass());
-  optimizer.RegisterPass(spvtools::CreateAggressiveDCEPass());
-
-  optimizer.RegisterPass(spvtools::CreateDeadBranchElimPass());
-  optimizer.RegisterPass(spvtools::CreateBlockMergePass());
-  optimizer.RegisterPass(spvtools::CreateLocalMultiStoreElimPass());
-  optimizer.RegisterPass(spvtools::CreateInsertExtractElimPass());
-  optimizer.RegisterPass(spvtools::CreateAggressiveDCEPass());
+  optimizer.RegisterLegalizationPasses();
 
   optimizer.RegisterPass(spvtools::CreateEliminateDeadFunctionsPass());
-  optimizer.RegisterPass(spvtools::CreateCFGCleanupPass());
-  optimizer.RegisterPass(spvtools::CreateDeadVariableEliminationPass());
-  optimizer.RegisterPass(spvtools::CreateEliminateDeadConstantPass());
-
   optimizer.RegisterPass(spvtools::CreateCompactIdsPass());
 
   return optimizer.Run(module->data(), module->size(), module);
@@ -378,22 +363,23 @@ void SPIRVEmitter::HandleTranslationUnit(ASTContext &context) {
   // Output the constructed module.
   std::vector<uint32_t> m = theBuilder.takeModule();
 
-  // Run legalization passes
-  if (!spirvOptions.disableLegalization && needsLegalization) {
-    std::string messages;
-    if (!spirvToolsLegalize(&m, &messages)) {
-      emitFatalError("failed to legalize SPIR-V: %0", {}) << messages;
-      return;
+  if (!spirvOptions.codeGenHighLevel) {
+    // Run legalization passes
+    if (needsLegalization) {
+      std::string messages;
+      if (!spirvToolsLegalize(&m, &messages)) {
+        emitFatalError("failed to legalize SPIR-V: %0", {}) << messages;
+        return;
+      }
     }
-  }
-
-  const auto optLevel = theCompilerInstance.getCodeGenOpts().OptimizationLevel;
 
-  if (optLevel > 0) {
-    std::string messages;
-    if (!spirvToolsOptimize(&m, &messages)) {
-      emitFatalError("failed to optimize SPIR-V: %0", {}) << messages;
-      return;
+    // Run optimization passes
+    if (theCompilerInstance.getCodeGenOpts().OptimizationLevel > 0) {
+      std::string messages;
+      if (!spirvToolsOptimize(&m, &messages)) {
+        emitFatalError("failed to optimize SPIR-V: %0", {}) << messages;
+        return;
+      }
     }
   }
 
@@ -4649,13 +4635,13 @@ uint32_t SPIRVEmitter::processIntrinsicMsad4(const CallExpr *callExpr) {
   //     for (UINT i = 0; i < 4; i++)
   //     {
   //         BYTE refByte, srcByte, absDiff;
-  // 
+  //
   //         refByte = (BYTE)(ref >> (i * 8));
   //         if (!refByte)
   //         {
   //             continue;
   //         }
-  // 
+  //
   //         srcByte = (BYTE)(src >> (i * 8));
   //         if (refByte >= srcByte)
   //         {
@@ -4665,7 +4651,7 @@ uint32_t SPIRVEmitter::processIntrinsicMsad4(const CallExpr *callExpr) {
   //         {
   //             absDiff = srcByte - refByte;
   //         }
-  // 
+  //
   //         // The recommended overflow behavior for MSAD is
   //         // to do a 32-bit saturate. This is not
   //         // required, however, and wrapping is allowed.
@@ -4678,7 +4664,7 @@ uint32_t SPIRVEmitter::processIntrinsicMsad4(const CallExpr *callExpr) {
   //         }
   //         accum += absDiff;
   //     }
-  // 
+  //
   //     return accum;
   // }
 

tools/clang/tools/dxcompiler/dxcompilerobj.cpp

@@ -464,7 +464,7 @@ public:
 #ifdef ENABLE_SPIRV_CODEGEN
       else if (opts.GenSPIRV) {
           clang::EmitSPIRVOptions spirvOpts;
-          spirvOpts.disableLegalization = opts.DisableSpirvLegalization;
+          spirvOpts.codeGenHighLevel = opts.CodeGenHighLevel;
           spirvOpts.stageIoOrder = opts.VkStageIoOrder;
           spirvOpts.bShift = opts.VkBShift;
           spirvOpts.tShift = opts.VkTShift;

tools/clang/unittests/SPIRV/CodeGenSPIRVTest.cpp

@@ -141,7 +141,7 @@ TEST_F(FileTest, BinaryOpAssignComposite) {
 TEST_F(FileTest, BinaryOpAssignResource) {
   runFileTest("binary-op.assign.resource.hlsl", Expect::Success,
               // It generates invalid SPIR-V code right now.
-              /*noValidation=*/true);
+              /*runValidation=*/false);
 }
 
 // For comma binary operator
@@ -247,8 +247,14 @@ TEST_F(FileTest, OpStructAccess) { runFileTest("op.struct.access.hlsl"); }
 TEST_F(FileTest, OpArrayAccess) { runFileTest("op.array.access.hlsl"); }
 
 // For buffer accessing operator
-TEST_F(FileTest, OpBufferAccess) { runFileTest("op.buffer.access.hlsl"); }
-TEST_F(FileTest, OpRWBufferAccess) { runFileTest("op.rwbuffer.access.hlsl"); }
+TEST_F(FileTest, OpBufferAccess) {
+  runFileTest("op.buffer.access.hlsl", Expect::Success,
+              /*runValidation=*/false);
+}
+TEST_F(FileTest, OpRWBufferAccess) {
+  runFileTest("op.rwbuffer.access.hlsl", Expect::Success,
+              /*runValidation=*/false);
+}
 TEST_F(FileTest, OpCBufferAccess) { runFileTest("op.cbuffer.access.hlsl"); }
 TEST_F(FileTest, OpConstantBufferAccess) {
   runFileTest("op.constant-buffer.access.hlsl");
@@ -267,10 +273,12 @@ TEST_F(FileTest, OpRWStructuredBufferAccess) {
 // For Texture/RWTexture accessing operator (operator[])
 TEST_F(FileTest, OpTextureAccess) { runFileTest("op.texture.access.hlsl"); }
 TEST_F(FileTest, OpRWTextureAccessRead) {
-  runFileTest("op.rwtexture.access.read.hlsl");
+  runFileTest("op.rwtexture.access.read.hlsl", Expect::Success,
+              /*runValidation=*/false);
 }
 TEST_F(FileTest, OpRWTextureAccessWrite) {
-  runFileTest("op.rwtexture.access.write.hlsl");
+  runFileTest("op.rwtexture.access.write.hlsl", Expect::Success,
+              /*runValidation=*/false);
 }
 
 // For Texture.mips[][] operator
@@ -587,10 +595,12 @@ TEST_F(FileTest, TextureArrayGatherCmp) {
   runFileTest("texture.array.gather-cmp.hlsl");
 }
 TEST_F(FileTest, TextureGatherCmpRed) {
-  runFileTest("texture.gather-cmp-red.hlsl");
+  runFileTest("texture.gather-cmp-red.hlsl", Expect::Success,
+              /*runValidation=*/false);
 }
 TEST_F(FileTest, TextureArrayGatherCmpRed) {
-  runFileTest("texture.array.gather-cmp-red.hlsl");
+  runFileTest("texture.array.gather-cmp-red.hlsl", Expect::Success,
+              /*runValidation=*/false);
 }
 TEST_F(FileTest, TextureArrayGatherCmpGreen) {
   runFileTest("texture.gather-cmp-green.hlsl", Expect::Failure);
@@ -664,13 +674,19 @@ TEST_F(FileTest, RWByteAddressBufferAtomicMethods) {
 }
 
 // For Buffer/RWBuffer methods
-TEST_F(FileTest, BufferLoad) { runFileTest("method.buffer.load.hlsl"); }
+TEST_F(FileTest, BufferLoad) {
+  runFileTest("method.buffer.load.hlsl", Expect::Success,
+              /*runValidation=*/false);
+}
 TEST_F(FileTest, BufferGetDimensions) {
   runFileTest("method.buffer.get-dimensions.hlsl");
 }
 
 // For RWTexture methods
-TEST_F(FileTest, RWTextureLoad) { runFileTest("method.rwtexture.load.hlsl"); }
+TEST_F(FileTest, RWTextureLoad) {
+  runFileTest("method.rwtexture.load.hlsl", Expect::Success,
+              /*runValidation=*/false);
+}
 TEST_F(FileTest, RWTextureGetDimensions) {
   runFileTest("method.rwtexture.get-dimensions.hlsl");
 }

tools/clang/unittests/SPIRV/FileTestFixture.cpp

@@ -61,7 +61,7 @@ bool FileTest::parseInputFile() {
 }
 
 void FileTest::runFileTest(llvm::StringRef filename, Expect expect,
-                           bool noValidation) {
+                           bool runValidation) {
   inputFilePath = utils::getAbsPathOfInputDataFile(filename);
 
   // Parse the input file.
@@ -124,7 +124,7 @@ void FileTest::runFileTest(llvm::StringRef filename, Expect expect,
   ASSERT_EQ(result.status(), effcee::Result::Status::Ok);
 
   // Run SPIR-V validation for successful compilations
-  if (!noValidation && expect != Expect::Failure) {
+  if (runValidation && expect != Expect::Failure) {
     EXPECT_TRUE(utils::validateSpirvBinary(generatedBinary));
   }
 }

tools/clang/unittests/SPIRV/FileTestFixture.h

@@ -27,7 +27,7 @@ public:
 
   /// \brief Runs a File Test! (See class description for more info)
   void runFileTest(llvm::StringRef path, Expect expect = Expect::Success,
-                   bool noValidation = false);
+                   bool runValidation = true);
 
 private:
   /// \brief Reads in the given input file.

tools/clang/unittests/SPIRV/FileTestUtils.cpp

@@ -137,8 +137,8 @@ bool runCompilerWithSpirvGeneration(const llvm::StringRef inputFilePath,
     flags.push_back(L"-T");
     flags.push_back(profile.c_str());
     flags.push_back(L"-spirv");
-    flags.push_back(L"-fcgl"); // Disable legalization for testing
-    flags.push_back(L"-O0");   // Disable optimization for testing
+    // Disable legalization and optimization for testing
+    flags.push_back(L"-fcgl");
     for (const auto &arg : rest)
       flags.push_back(arg.c_str());