Browse Source

[spirv] fix OpLine emission bug (#2276)

Based on SPIR-V spec, OpSelectionMerge must immediately precede
either an OpBranchConditional or OpSwitch instruction. Similarly
OpLoopMerge must immediately precede either an OpBranch or
OpBranchConditional instruction.
Jaebaek Seo 6 years ago
parent
commit
6d949395aa

+ 11 - 0
tools/clang/lib/SPIRV/EmitVisitor.cpp

@@ -121,6 +121,17 @@ void EmitVisitor::emitDebugNameForInstruction(uint32_t resultId,
 }
 
 void EmitVisitor::emitDebugLine(spv::Op op, const SourceLocation &loc) {
+  // Based on SPIR-V spec, OpSelectionMerge must immediately precede either an
+  // OpBranchConditional or OpSwitch instruction. Similarly OpLoopMerge must
+  // immediately precede either an OpBranch or OpBranchConditional instruction.
+  if (lastOpWasMergeInst) {
+    lastOpWasMergeInst = false;
+    return;
+  }
+
+  if (op == spv::Op::OpSelectionMerge || op == spv::Op::OpLoopMerge)
+    lastOpWasMergeInst = true;
+
   if (!isOpLineLegalForOp(op))
     return;
 

+ 4 - 1
tools/clang/lib/SPIRV/EmitVisitor.h

@@ -191,7 +191,8 @@ public:
         typeHandler(astCtx, spvCtx, &debugBinary, &annotationsBinary,
                     &typeConstantBinary,
                     [this]() -> uint32_t { return takeNextId(); }),
-        debugFileId(0), debugLine(0), debugColumn(0) {}
+        debugFileId(0), debugLine(0), debugColumn(0),
+        lastOpWasMergeInst(false) {}
 
   // Visit different SPIR-V constructs for emitting.
   bool visit(SpirvModule *, Phase phase);
@@ -339,6 +340,8 @@ private:
   uint32_t debugLine;
   // The last debug column number information emitted by OpLine.
   uint32_t debugColumn;
+  // True if the last emitted instruction was OpSelectionMerge or OpLoopMerge.
+  bool lastOpWasMergeInst;
 };
 
 } // namespace spirv

+ 27 - 0
tools/clang/test/CodeGenSPIRV/spirv.debug.opline.precedence.hlsl

@@ -0,0 +1,27 @@
+// Run: %dxc -T cs_6_0 -E main -Zi
+
+// CHECK:      [[file:%\d+]] = OpString
+// CHECK-SAME: spirv.debug.opline.precedence.hlsl
+
+void main() {
+  int a;
+  int b;
+
+//CHECK:      OpLine [[file]] 13 3
+//CHECK-NEXT: OpSelectionMerge %switch_merge None
+  switch (a) {
+  default:
+    b = 0;
+  case 1:
+    b = 1;
+    break;
+  case 2:
+    b = 2;
+  }
+
+//CHECK:      OpLine [[file]] 25 23
+//CHECK-NEXT: OpLoopMerge %for_merge %for_continue None
+  for (int i = 0; i < 4; i++) {
+    b += i;
+  }
+}

+ 3 - 0
tools/clang/unittests/SPIRV/CodeGenSpirvTest.cpp

@@ -1431,6 +1431,9 @@ TEST_F(FileTest, SpirvDebugOpLine) { runFileTest("spirv.debug.opline.hlsl"); }
 TEST_F(FileTest, SpirvDebugOpLineBranch) {
   runFileTest("spirv.debug.opline.branch.hlsl");
 }
+TEST_F(FileTest, SpirvDebugOpLinePrecendence) {
+  runFileTest("spirv.debug.opline.precedence.hlsl");
+}
 TEST_F(FileTest, SpirvDebugOpLineComposite) {
   runFileTest("spirv.debug.opline.composite.hlsl");
 }