From 8fea615e3cf558c575a1df7dbb22c354be6f6f9a Mon Sep 17 00:00:00 2001 From: Jaebaek Seo Date: Mon, 2 Nov 2020 14:11:29 -0500 Subject: [PATCH] [spirv] wrap instructions with OpNoLine (#3229) The current DXC emits `OpLine` for the first instruction in the location and does not emit the same `OpLine` for the following instructions. However, it does not specify the end of the effectiveness of the `OpLine`, which is technically wrong based on the spec of OpLine and OpNoLine. We have to specify the `OpLine` is not applied to the following instructions when we meet an instruction without the location information. --- external/SPIRV-Headers | 2 +- external/SPIRV-Tools | 2 +- tools/clang/lib/SPIRV/EmitVisitor.cpp | 50 +++++++++++++++---- tools/clang/lib/SPIRV/EmitVisitor.h | 2 +- .../CodeGenSPIRV/rich.debug.debugdeclare.hlsl | 18 +++---- .../rich.debug.debugdeclare.without.init.hlsl | 12 ++--- ....debug.scope.after.compound.statement.hlsl | 2 +- .../spirv.debug.opline.branch.hlsl | 2 +- 8 files changed, 59 insertions(+), 31 deletions(-) diff --git a/external/SPIRV-Headers b/external/SPIRV-Headers index c43a43c7c..05836bdba 160000 --- a/external/SPIRV-Headers +++ b/external/SPIRV-Headers @@ -1 +1 @@ -Subproject commit c43a43c7cc3af55910b9bec2a71e3e8a622443cf +Subproject commit 05836bdba63e7debce9fa9feaed42f20cd43af9d diff --git a/external/SPIRV-Tools b/external/SPIRV-Tools index 5c64374dd..82b378d67 160000 --- a/external/SPIRV-Tools +++ b/external/SPIRV-Tools @@ -1 +1 @@ -Subproject commit 5c64374dd6cbfff1294ec78cdae1bc9de870a07d +Subproject commit 82b378d671836b51343b010ca9ec32db14485147 diff --git a/tools/clang/lib/SPIRV/EmitVisitor.cpp b/tools/clang/lib/SPIRV/EmitVisitor.cpp index 3de691c87..7588c3f7d 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.cpp +++ b/tools/clang/lib/SPIRV/EmitVisitor.cpp @@ -202,7 +202,8 @@ void EmitVisitor::emitDebugNameForInstruction(uint32_t resultId, } void EmitVisitor::emitDebugLine(spv::Op op, const SourceLocation &loc, - std::vector *section) { + std::vector *section, + bool isDebugScope) { if (!spvOptions.debugInfoLine) return; @@ -217,6 +218,8 @@ void EmitVisitor::emitDebugLine(spv::Op op, const SourceLocation &loc, // immediately precede either an OpBranch or OpBranchConditional instruction. if (lastOpWasMergeInst) { lastOpWasMergeInst = false; + debugLine = 0; + debugColumn = 0; return; } @@ -233,29 +236,53 @@ void EmitVisitor::emitDebugLine(spv::Op op, const SourceLocation &loc, if (op == spv::Op::OpVariable) return; + // If no SourceLocation is provided, we have to emit OpNoLine to + // specify the previous OpLine is not applied to this instruction. + if (loc == SourceLocation()) { + if (!isDebugScope && (debugLine != 0 || debugColumn != 0)) { + curInst.clear(); + curInst.push_back(static_cast(spv::Op::OpNoLine)); + curInst[0] |= static_cast(curInst.size()) << 16; + section->insert(section->end(), curInst.begin(), curInst.end()); + } + debugLine = 0; + debugColumn = 0; + return; + } + auto fileId = debugMainFileId; const auto &sm = astContext.getSourceManager(); const char *fileName = sm.getPresumedLoc(loc).getFilename(); if (fileName) fileId = getOrCreateOpStringId(fileName); - if (!fileId) - return; - uint32_t line = sm.getPresumedLineNumber(loc); uint32_t column = sm.getPresumedColumnNumber(loc); - if (!line || !column) - return; + // If it is a terminator, just reset the last line and column because + // a terminator makes the OpLine not effective. + bool resetLine = (op >= spv::Op::OpBranch && op <= spv::Op::OpUnreachable) || + op == spv::Op::OpTerminateInvocation; - if (line == debugLine && column == debugColumn) + if (!fileId || !line || !column || + (line == debugLine && column == debugColumn)) { + if (resetLine) { + debugLine = 0; + debugColumn = 0; + } return; + } assert(section); - // We must update these two values to emit the next Opline. - debugLine = line; - debugColumn = column; + if (resetLine) { + debugLine = 0; + debugColumn = 0; + } else { + // Keep the last line and column to avoid printing the duplicated OpLine. + debugLine = line; + debugColumn = column; + } curInst.clear(); curInst.push_back(static_cast(spv::Op::OpLine)); @@ -312,7 +339,8 @@ void EmitVisitor::initInstruction(SpirvInstruction *inst) { isGlobalVar = var->getStorageClass() != spv::StorageClass::Function; const auto op = inst->getopcode(); emitDebugLine(op, inst->getSourceLocation(), - isGlobalVar ? &globalVarsBinary : &mainBinary); + isGlobalVar ? &globalVarsBinary : &mainBinary, + isa(inst)); // Initialize the current instruction for emitting. curInst.clear(); diff --git a/tools/clang/lib/SPIRV/EmitVisitor.h b/tools/clang/lib/SPIRV/EmitVisitor.h index 777cc498a..b694ba225 100644 --- a/tools/clang/lib/SPIRV/EmitVisitor.h +++ b/tools/clang/lib/SPIRV/EmitVisitor.h @@ -312,7 +312,7 @@ private: // Emits an OpLine instruction for the given operation into the given binary // section. void emitDebugLine(spv::Op op, const SourceLocation &loc, - std::vector *section); + std::vector *section, bool isDebugScope = false); // Initiates the creation of a new instruction with the given Opcode. void initInstruction(spv::Op, const SourceLocation &); diff --git a/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.hlsl b/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.hlsl index ada00d1f4..3fac0c3e3 100644 --- a/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.hlsl +++ b/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.hlsl @@ -9,16 +9,16 @@ // CHECK: [[expr:%\d+]] = OpExtInst %void [[set]] DebugExpression // CHECK: [[color:%\d+]] = OpExtInst %void [[set]] DebugLocalVariable {{%\d+}} {{%\d+}} {{%\d+}} 28 20 {{%\d+}} FlagIsLocal 1 -// CHECK: %color = OpFunctionParameter -// CHECK-NEXT: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[color]] %color [[expr]] -// CHECK: %condition = OpVariable -// CHECK: OpStore %condition %false -// CHECK-NEXT: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[condition]] %condition [[expr]] +// CHECK: %color = OpFunctionParameter +// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[color]] %color [[expr]] +// CHECK: %condition = OpVariable +// CHECK: OpStore %condition %false +// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[condition]] %condition [[expr]] -// CHECK: %x = OpFunctionParameter -// CHECK: %y = OpFunctionParameter -// CHECK-NEXT: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[x]] %x [[expr]] -// CHECK-NEXT: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[y]] %y [[expr]] +// CHECK: %x = OpFunctionParameter +// CHECK: %y = OpFunctionParameter +// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[x]] %x [[expr]] +// CHECK: {{%\d+}} = OpExtInst %void [[set]] DebugDeclare [[y]] %y [[expr]] void foo(int x, float y) { diff --git a/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.without.init.hlsl b/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.without.init.hlsl index fda43e2c0..d696f2347 100644 --- a/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.without.init.hlsl +++ b/tools/clang/test/CodeGenSPIRV/rich.debug.debugdeclare.without.init.hlsl @@ -1,11 +1,11 @@ // Run: %dxc -T ps_6_0 -E main -fspv-debug=rich -// CHECK: %i = OpFunctionParameter %_ptr_Function_PS_INPUT -// CHECK-NEXT: DebugDeclare {{%\d+}} %i -// CHECK: %ps_output = OpVariable %_ptr_Function_PS_OUTPUT Function -// CHECK: %c = OpVariable %_ptr_Function_v4float Function -// CHECK: DebugDeclare {{%\d+}} %ps_output -// CHECK: DebugDeclare {{%\d+}} %c +// CHECK: %i = OpFunctionParameter %_ptr_Function_PS_INPUT +// CHECK: DebugDeclare {{%\d+}} %i +// CHECK: %ps_output = OpVariable %_ptr_Function_PS_OUTPUT Function +// CHECK: %c = OpVariable %_ptr_Function_v4float Function +// CHECK: DebugDeclare {{%\d+}} %ps_output +// CHECK: DebugDeclare {{%\d+}} %c Texture2D g_tColor; diff --git a/tools/clang/test/CodeGenSPIRV/rich.debug.scope.after.compound.statement.hlsl b/tools/clang/test/CodeGenSPIRV/rich.debug.scope.after.compound.statement.hlsl index 2df77fb4f..6566c9467 100644 --- a/tools/clang/test/CodeGenSPIRV/rich.debug.scope.after.compound.statement.hlsl +++ b/tools/clang/test/CodeGenSPIRV/rich.debug.scope.after.compound.statement.hlsl @@ -28,7 +28,7 @@ VS_OUTPUT main(float4 pos : POSITION, //CHECK: DebugScope [[bb2]] //CHECK-NEXT: OpLine [[file:%\d+]] 32 //CHECK-NEXT: OpStore [[var_a:%\w+]] %float_6 -//CHECK-NEXT: DebugDeclare [[a]] [[var_a]] +//CHECK: DebugDeclare [[a]] [[var_a]] float a = 6.0; x += a + b + c; } diff --git a/tools/clang/test/CodeGenSPIRV/spirv.debug.opline.branch.hlsl b/tools/clang/test/CodeGenSPIRV/spirv.debug.opline.branch.hlsl index 763db3029..2c556bc75 100644 --- a/tools/clang/test/CodeGenSPIRV/spirv.debug.opline.branch.hlsl +++ b/tools/clang/test/CodeGenSPIRV/spirv.debug.opline.branch.hlsl @@ -52,8 +52,8 @@ void main() { // CHECK: OpLine [[file]] 57 3 // CHECK-NEXT: OpBranch %while_continue // CHECK-NEXT: %while_continue = OpLabel +// CHECK-NEXT: OpLine [[file]] 57 3 // CHECK-NEXT: OpBranch %while_check -// CHECK-NEXT: %while_merge = OpLabel } // CHECK: OpLine [[file]] 61 19