Support old DebugInfo extension

KhronosGroup · Jan 21, 2020 · eadbeb9 · eadbeb9
1 parent 80b9e8a
commit eadbeb9
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 45 deletions.
diff --git a/source/operand.cpp b/source/operand.cpp
@@ -19,6 +19,8 @@
 
 #include <algorithm>
 
+#include "DebugInfo.h"
+#include "OpenCLDebugInfo100.h"
 #include "source/macro.h"
 #include "source/spirv_constant.h"
 #include "source/spirv_target_env.h"
@@ -514,21 +516,35 @@ std::function<bool(unsigned)> spvOperandCanBeForwardDeclaredFunction(
 }
 
 std::function<bool(unsigned)> spvDbgInfoExtOperandCanBeForwardDeclaredFunction(
-    OpenCLDebugInfo100Instructions key) {
-  std::function<bool(unsigned index)> out;
+    spv_ext_inst_type_t ext_type, uint32_t key) {
   // TODO(https://gitlab.khronos.org/spirv/SPIR-V/issues/532): Forward
   // references for debug info instructions are still in discussion. We must
   // update the following lines of code when we conclude the spec.
-  switch (key) {
-    case OpenCLDebugInfo100DebugFunction:
-      out = [](unsigned index) { return index == 13; };
-      break;
-    case OpenCLDebugInfo100DebugTypeComposite:
-      out = [](unsigned index) { return index >= 13; };
-      break;
-    default:
-      out = [](unsigned) { return false; };
-      break;
+  std::function<bool(unsigned index)> out;
+  if (ext_type == SPV_EXT_INST_TYPE_OPENCL_DEBUGINFO_100) {
+    switch (OpenCLDebugInfo100Instructions(key)) {
+      case OpenCLDebugInfo100DebugFunction:
+        out = [](unsigned index) { return index == 13; };
+        break;
+      case OpenCLDebugInfo100DebugTypeComposite:
+        out = [](unsigned index) { return index >= 13; };
+        break;
+      default:
+        out = [](unsigned) { return false; };
+        break;
+    }
+  } else {
+    switch (DebugInfoInstructions(key)) {
+      case DebugInfoDebugFunction:
+        out = [](unsigned index) { return index == 13; };
+        break;
+      case DebugInfoDebugTypeComposite:
+        out = [](unsigned index) { return index >= 12; };
+        break;
+      default:
+        out = [](unsigned) { return false; };
+        break;
+    }
   }
   return out;
 }
diff --git a/source/operand.h b/source/operand.h
@@ -18,7 +18,6 @@
 #include <functional>
 #include <vector>
 
-#include "OpenCLDebugInfo100.h"
 #include "source/table.h"
 #include "spirv-tools/libspirv.h"
 
@@ -147,6 +146,6 @@ std::function<bool(unsigned)> spvOperandCanBeForwardDeclaredFunction(
 // of the operand can be forward declared. This function will
 // used in the SSA validation stage of the pipeline
 std::function<bool(unsigned)> spvDbgInfoExtOperandCanBeForwardDeclaredFunction(
-    OpenCLDebugInfo100Instructions key);
+    spv_ext_inst_type_t ext_type, uint32_t key);
 
 #endif  // SOURCE_OPERAND_H_
diff --git a/source/opt/ir_loader.cpp b/source/opt/ir_loader.cpp
@@ -16,6 +16,7 @@
 
 #include <utility>
 
+#include "DebugInfo.h"
 #include "OpenCLDebugInfo100.h"
 #include "source/ext_inst.h"
 #include "source/opt/log.h"
@@ -143,18 +144,34 @@ bool IrLoader::AddInstruction(const spv_parsed_instruction_t* inst) {
         if (opcode == SpvOpExtInst &&
             spvExtInstIsDebugInfo(inst->ext_inst_type)) {
           const uint32_t ext_inst_index = inst->words[4];
-          const OpenCLDebugInfo100Instructions ext_inst_key =
-              OpenCLDebugInfo100Instructions(ext_inst_index);
-          if (ext_inst_key != OpenCLDebugInfo100DebugScope &&
-              ext_inst_key != OpenCLDebugInfo100DebugNoScope &&
-              ext_inst_key != OpenCLDebugInfo100DebugDeclare &&
-              ext_inst_key != OpenCLDebugInfo100DebugValue) {
-            Errorf(consumer_, src, loc,
-                   "Debug info extension instruction other than DebugScope, "
-                   "DebugNoScope, DebugDeclare, and DebugValue found inside "
-                   "function",
-                   opcode);
-            return false;
+          if (inst->ext_inst_type == SPV_EXT_INST_TYPE_OPENCL_DEBUGINFO_100) {
+            const OpenCLDebugInfo100Instructions ext_inst_key =
+                OpenCLDebugInfo100Instructions(ext_inst_index);
+            if (ext_inst_key != OpenCLDebugInfo100DebugScope &&
+                ext_inst_key != OpenCLDebugInfo100DebugNoScope &&
+                ext_inst_key != OpenCLDebugInfo100DebugDeclare &&
+                ext_inst_key != OpenCLDebugInfo100DebugValue) {
+              Errorf(consumer_, src, loc,
+                     "Debug info extension instruction other than DebugScope, "
+                     "DebugNoScope, DebugDeclare, and DebugValue found inside "
+                     "function",
+                     opcode);
+              return false;
+            }
+          } else {
+            const DebugInfoInstructions ext_inst_key =
+                DebugInfoInstructions(ext_inst_index);
+            if (ext_inst_key != DebugInfoDebugScope &&
+                ext_inst_key != DebugInfoDebugNoScope &&
+                ext_inst_key != DebugInfoDebugDeclare &&
+                ext_inst_key != DebugInfoDebugValue) {
+              Errorf(consumer_, src, loc,
+                     "Debug info extension instruction other than DebugScope, "
+                     "DebugNoScope, DebugDeclare, and DebugValue found inside "
+                     "function",
+                     opcode);
+              return false;
+            }
           }
         }
         block_->AddInstruction(std::move(spv_inst));

diff --git a/source/val/validate_id.cpp b/source/val/validate_id.cpp
@@ -134,7 +134,7 @@ spv_result_t IdPass(ValidationState_t& _, Instruction* inst) {
       inst->opcode() == SpvOpExtInst &&
               spvExtInstIsDebugInfo(inst->ext_inst_type())
           ? spvDbgInfoExtOperandCanBeForwardDeclaredFunction(
-                OpenCLDebugInfo100Instructions(inst->word(4)))
+                inst->ext_inst_type(), inst->word(4))
           : spvOperandCanBeForwardDeclaredFunction(inst->opcode());
 
   // Keep track of a result id defined by this instruction.  0 means it

diff --git a/source/val/validate_layout.cpp b/source/val/validate_layout.cpp
@@ -16,6 +16,7 @@
 
 #include <cassert>
 
+#include "DebugInfo.h"
 #include "OpenCLDebugInfo100.h"
 #include "source/diagnostic.h"
 #include "source/opcode.h"
@@ -197,18 +198,34 @@ spv_result_t FunctionScopedInstructions(ValidationState_t& _,
           }
         } else if (spvExtInstIsDebugInfo(inst->ext_inst_type())) {
           const uint32_t ext_inst_index = inst->word(4);
-          const OpenCLDebugInfo100Instructions ext_inst_key =
-              OpenCLDebugInfo100Instructions(ext_inst_index);
-          if (ext_inst_key == OpenCLDebugInfo100DebugScope ||
-              ext_inst_key == OpenCLDebugInfo100DebugNoScope ||
-              ext_inst_key == OpenCLDebugInfo100DebugDeclare ||
-              ext_inst_key == OpenCLDebugInfo100DebugValue) {
+          bool local_debug_info = false;
+          if (inst->ext_inst_type() == SPV_EXT_INST_TYPE_OPENCL_DEBUGINFO_100) {
+            const OpenCLDebugInfo100Instructions ext_inst_key =
+                OpenCLDebugInfo100Instructions(ext_inst_index);
+            if (ext_inst_key == OpenCLDebugInfo100DebugScope ||
+                ext_inst_key == OpenCLDebugInfo100DebugNoScope ||
+                ext_inst_key == OpenCLDebugInfo100DebugDeclare ||
+                ext_inst_key == OpenCLDebugInfo100DebugValue) {
+              local_debug_info = true;
+            }
+          } else {
+            const DebugInfoInstructions ext_inst_key =
+                DebugInfoInstructions(ext_inst_index);
+            if (ext_inst_key == DebugInfoDebugScope ||
+                ext_inst_key == DebugInfoDebugNoScope ||
+                ext_inst_key == DebugInfoDebugDeclare ||
+                ext_inst_key == DebugInfoDebugValue) {
+              local_debug_info = true;
+            }
+          }
+
+          if (local_debug_info) {
             if (_.in_function_body() == false) {
               // DebugScope, DebugNoScope, DebugDeclare, DebugValue must
               // appear in a function body.
               return _.diag(SPV_ERROR_INVALID_LAYOUT, inst)
-                     << "OpenCL.DebugInfo.100 DebugScope, DebugNoScope, "
-                     << "DebugDeclare, DebugValue must appear in a function "
+                     << "DebugScope, DebugNoScope, DebugDeclare, DebugValue "
+                     << "of debug info extension must appear in a function "
                      << "body";
             }
           } else {
@@ -219,7 +236,7 @@ spv_result_t FunctionScopedInstructions(ValidationState_t& _,
             if (_.current_layout_section() < kLayoutTypes ||
                 _.current_layout_section() >= kLayoutFunctionDeclarations) {
               return _.diag(SPV_ERROR_INVALID_LAYOUT, inst)
-                     << "OpenCL.DebugInfo.100 instructions other than "
+                     << "Debug info extension instructions other than "
                      << "DebugScope, DebugNoScope, DebugDeclare, DebugValue "
                      << "must appear between section 9 (types, constants, "
                      << "global variables) and section 10 (function "

diff --git a/test/opt/ir_loader_test.cpp b/test/opt/ir_loader_test.cpp
@@ -223,16 +223,16 @@ OpLine %7 3 18
 %out_var_COLOR = OpVariable %_ptr_Output_v4float Output
 %34 = OpExtInst %void %1 DebugSource %7 %8
 %35 = OpExtInst %void %1 DebugCompilationUnit 2 4 %34 HLSL
-%36 = OpExtInst %void %1 DebugTypeComposite %9 Structure %34 1 1 %35 %13 %int_128 FlagIsProtected|FlagIsPrivate %34 %34
-%37 = OpExtInst %void %1 DebugTypeBasic %10 %int_32 Float
-%38 = OpExtInst %void %1 DebugTypeVector %37 4
-%39 = OpExtInst %void %1 DebugTypeMember %11 %38 %34 2 3 %36 %int_0 %int_128 FlagIsProtected|FlagIsPrivate
-%40 = OpExtInst %void %1 DebugTypeMember %12 %38 %34 3 3 %36 %int_128 %int_128 FlagIsProtected|FlagIsPrivate
-%41 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %36 %38 %38
+%36 = OpExtInst %void %1 DebugTypeComposite %9 Structure %34 1 1 %35 %13 %int_128 FlagIsProtected|FlagIsPrivate %37 %38
+%39 = OpExtInst %void %1 DebugTypeBasic %10 %int_32 Float
+%40 = OpExtInst %void %1 DebugTypeVector %39 4
+%37 = OpExtInst %void %1 DebugTypeMember %11 %40 %34 2 3 %36 %int_0 %int_128 FlagIsProtected|FlagIsPrivate
+%38 = OpExtInst %void %1 DebugTypeMember %12 %40 %34 3 3 %36 %int_128 %int_128 FlagIsProtected|FlagIsPrivate
+%41 = OpExtInst %void %1 DebugTypeFunction FlagIsProtected|FlagIsPrivate %36 %40 %40
 %42 = OpExtInst %void %1 DebugExpression
 %43 = OpExtInst %void %1 DebugFunction %14 %41 %34 6 1 %35 %15 FlagIsProtected|FlagIsPrivate 7 %main
-%44 = OpExtInst %void %1 DebugLocalVariable %16 %38 %34 6 16 %43 FlagIsLocal 0
-%45 = OpExtInst %void %1 DebugLocalVariable %17 %38 %34 7 16 %43 FlagIsLocal 1
+%44 = OpExtInst %void %1 DebugLocalVariable %16 %40 %34 6 16 %43 FlagIsLocal 0
+%45 = OpExtInst %void %1 DebugLocalVariable %17 %40 %34 7 16 %43 FlagIsLocal 1
 %46 = OpExtInst %void %1 DebugLocalVariable %18 %36 %34 8 3 %43 FlagIsLocal
 %47 = OpExtInst %void %1 DebugDeclare %44 %pos %42
 %48 = OpExtInst %void %1 DebugDeclare %45 %color %42

diff --git a/test/val/val_ext_inst_test.cpp b/test/val/val_ext_inst_test.cpp
@@ -33,6 +33,7 @@ using ::testing::HasSubstr;
 using ::testing::Not;
 
 using ValidateExtInst = spvtest::ValidateBase<bool>;
+using ValidateOldDebugInfo = spvtest::ValidateBase<std::string>;
 using ValidateOpenCL100DebugInfo = spvtest::ValidateBase<std::string>;
 using ValidateGlslStd450SqrtLike = spvtest::ValidateBase<std::string>;
 using ValidateGlslStd450FMinLike = spvtest::ValidateBase<std::string>;
@@ -655,6 +656,24 @@ OpFunctionEnd)";
   return ss.str();
 }
 
+TEST_F(ValidateOldDebugInfo, UseDebugInstructionOutOfFunction) {
+  const std::string src = R"(
+%code = OpString "main() {}"
+)";
+
+  const std::string dbg_inst = R"(
+%cu = OpExtInst %void %DbgExt DebugCompilationUnit %code 1 1
+)";
+
+  const std::string extension = R"(
+%DbgExt = OpExtInstImport "DebugInfo"
+)";
+
+  CompileSuccessfully(GenerateShaderCodeForDebugInfo(src, "", dbg_inst, "",
+                                                     extension, "Vertex"));
+  ASSERT_EQ(SPV_SUCCESS, ValidateInstructions());
+}
+
 TEST_F(ValidateOpenCL100DebugInfo, UseDebugInstructionOutOfFunction) {
   const std::string src = R"(
 %src = OpString "simple.hlsl"
@@ -693,7 +712,7 @@ TEST_F(ValidateOpenCL100DebugInfo, DebugSourceInFunction) {
   ASSERT_EQ(SPV_ERROR_INVALID_LAYOUT, ValidateInstructions());
   EXPECT_THAT(
       getDiagnosticString(),
-      HasSubstr("OpenCL.DebugInfo.100 instructions other than DebugScope, "
+      HasSubstr("Debug info extension instructions other than DebugScope, "
                 "DebugNoScope, DebugDeclare, DebugValue must appear between "
                 "section 9 (types, constants, global variables) and section 10 "
                 "(function declarations)"));