[opt] Move debug instruction when neccessary in copy prop arrays. (#6142)
Debug instructions for a function scope variable may appear before the
store. We use the store as the insertion point for a new access change
to the source variable assuming all uses of the variable are dominated
by the store. This causes problems for the debug instruction. It will
then receive an operand that has not been defined.
The fix this we move the debug instruction to after the new access chain
if necessary. We opt for this option because:
1. We did not want to avoid doing the optimization because of debug
information.
2. There is no other reasonable place to insert the access chain. If we
move it to the start of the entry block, some operands may not be
defined, and may not be available. If we put it anywhere else, we
potentially get the same problem as we currently have with the store.
3. Moving the debug value or debug declare instructions are not a
problem. Before the store the varible was undefined, so the debug
information would not enable any useful information in the debugger
anyway.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/7025
diff --git a/source/opt/copy_prop_arrays.cpp b/source/opt/copy_prop_arrays.cpp
index d66193b..0ddfbcc 100644
--- a/source/opt/copy_prop_arrays.cpp
+++ b/source/opt/copy_prop_arrays.cpp
@@ -256,11 +256,14 @@
} else if (use->IsDecoration() || use->opcode() == spv::Op::OpName) {
return true;
} else if (use->opcode() == spv::Op::OpStore) {
- // If we are storing to part of the object it is not an candidate.
+ // If we are storing to part of the object it is not a candidate.
return ptr_inst->opcode() == spv::Op::OpVariable &&
store_inst->GetSingleWordInOperand(kStorePointerInOperand) ==
ptr_inst->result_id();
} else if (IsDebugDeclareOrValue(use)) {
+ // The store does not have to dominate debug instructions. We do not
+ // want debugging info to stop the transformation. It will be fixed
+ // up later.
return true;
}
// Some other instruction. Be conservative.
@@ -656,6 +659,20 @@
uint32_t index = pair.second;
if (use->IsCommonDebugInstr()) {
+ // It is possible that the debug instructions are not dominated by
+ // `new_ptr_inst`. If not, move the debug instruction to just after
+ // `new_ptr_inst`.
+ BasicBlock* store_block = context()->get_instr_block(new_ptr_inst);
+ if (store_block) {
+ Function* function = store_block->GetParent();
+ DominatorAnalysis* dominator_analysis =
+ context()->GetDominatorAnalysis(function);
+ if (!dominator_analysis->Dominates(new_ptr_inst, use)) {
+ assert(dominator_analysis->Dominates(use, new_ptr_inst));
+ use->InsertAfter(new_ptr_inst);
+ }
+ }
+
switch (use->GetCommonDebugOpcode()) {
case CommonDebugInfoDebugDeclare: {
if (new_ptr_inst->opcode() == spv::Op::OpVariable ||
@@ -897,9 +914,7 @@
iterator begin, iterator end)
: variable_inst_(var_inst) {
std::transform(begin, end, std::back_inserter(access_chain_),
- [](uint32_t id) {
- return AccessChainEntry{true, {id}};
- });
+ [](uint32_t id) { return AccessChainEntry{true, {id}}; });
}
std::vector<uint32_t> CopyPropagateArrays::MemoryObject::GetAccessIds() const {
diff --git a/test/opt/copy_prop_array_test.cpp b/test/opt/copy_prop_array_test.cpp
index 6d993c7..20f26c6 100644
--- a/test/opt/copy_prop_array_test.cpp
+++ b/test/opt/copy_prop_array_test.cpp
@@ -2313,6 +2313,86 @@
SinglePassRunAndMatch<CopyPropagateArrays>(before, false);
}
+TEST_F(CopyPropArrayPassTest, DebugInstNotDominatingStore) {
+ // Move the debug value to after the new access chain instruction.
+ const std::string before = R"(
+ OpCapability Shader
+ OpCapability MeshShadingEXT
+ OpExtension "SPV_EXT_mesh_shader"
+ %1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint MeshEXT %2 "main" %3
+ OpExecutionMode %2 LocalSize 1 1 1
+ OpExecutionMode %2 OutputTrianglesEXT
+ OpExecutionMode %2 OutputVertices 64
+ OpExecutionMode %2 OutputPrimitivesEXT 124
+ %4 = OpString ""
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+ %uint = OpTypeInt 32 0
+ %uint_32 = OpConstant %uint 32
+%_arr_uint_uint_32 = OpTypeArray %uint %uint_32
+ %_struct_10 = OpTypeStruct %_arr_uint_uint_32
+%_ptr_TaskPayloadWorkgroupEXT__struct_10 = OpTypePointer TaskPayloadWorkgroupEXT %_struct_10
+ %uint_124 = OpConstant %uint 124
+ %uint_64 = OpConstant %uint 64
+ %void = OpTypeVoid
+ %uint_6 = OpConstant %uint 6
+ %uint_0 = OpConstant %uint 0
+ %uint_1 = OpConstant %uint 1
+ %uint_4 = OpConstant %uint 4
+ %uint_5 = OpConstant %uint 5
+ %uint_1024 = OpConstant %uint 1024
+ %uint_7 = OpConstant %uint 7
+ %uint_8 = OpConstant %uint 8
+ %uint_3 = OpConstant %uint 3
+ %uint_96 = OpConstant %uint 96
+ %uint_11 = OpConstant %uint 11
+ %uint_12 = OpConstant %uint 12
+ %uint_10 = OpConstant %uint 10
+ %uint_16 = OpConstant %uint 16
+ %uint_21 = OpConstant %uint 21
+ %uint_17 = OpConstant %uint 17
+ %uint_26 = OpConstant %uint 26
+ %32 = OpTypeFunction %void
+%_ptr_Function__arr_uint_uint_32 = OpTypePointer Function %_arr_uint_uint_32
+ %3 = OpVariable %_ptr_TaskPayloadWorkgroupEXT__struct_10 TaskPayloadWorkgroupEXT
+ %34 = OpExtInst %void %1 DebugOperation %uint_0
+ %35 = OpExtInst %void %1 DebugTypeBasic %4 %uint_32 %uint_6 %uint_0
+ %36 = OpExtInst %void %1 DebugSource %4 %4
+ %37 = OpExtInst %void %1 DebugCompilationUnit %uint_1 %uint_4 %36 %uint_5
+ %38 = OpExtInst %void %1 DebugTypeArray %35 %uint_32
+ %39 = OpExtInst %void %1 DebugTypeMember %4 %38 %36 %uint_7 %uint_8 %uint_0 %uint_1024 %uint_3
+ %40 = OpExtInst %void %1 DebugTypeComposite %4 %uint_1 %36 %uint_6 %uint_8 %37 %4 %uint_1024 %uint_3 %39
+ %41 = OpExtInst %void %1 DebugTypeVector %35 %uint_3
+ %42 = OpExtInst %void %1 DebugTypeArray %41 %uint_124
+ %43 = OpExtInst %void %1 DebugTypeBasic %4 %uint_32 %uint_3 %uint_0
+ %44 = OpExtInst %void %1 DebugTypeVector %43 %uint_3
+ %45 = OpExtInst %void %1 DebugTypeMember %4 %44 %36 %uint_11 %uint_12 %uint_0 %uint_96 %uint_3
+ %46 = OpExtInst %void %1 DebugTypeComposite %4 %uint_1 %36 %uint_10 %uint_8 %37 %4 %uint_96 %uint_3 %45
+ %47 = OpExtInst %void %1 DebugTypeArray %46 %uint_64
+ %48 = OpExtInst %void %1 DebugTypeFunction %uint_3 %void %40 %35 %42 %47
+ %49 = OpExtInst %void %1 DebugFunction %4 %48 %36 %uint_16 %uint_1 %37 %4 %uint_3 %uint_21
+ %50 = OpExtInst %void %1 DebugLocalVariable %4 %40 %36 %uint_17 %uint_26 %49 %uint_4 %uint_1
+ %51 = OpExtInst %void %1 DebugExpression %34
+; CHECK: OpFunction
+ %2 = OpFunction %void None %32
+ %52 = OpLabel
+ %53 = OpVariable %_ptr_Function__arr_uint_uint_32 Function
+; CHECK: [[new_ptr:%\w+]] = OpAccessChain %_ptr_TaskPayloadWorkgroupEXT__arr_uint_uint_32
+; CHECK: OpExtInst %void %1 DebugValue {{%\w+}} [[new_ptr]]
+ %54 = OpExtInst %void %1 DebugValue %50 %53 %51 %int_0
+ %55 = OpLoad %_struct_10 %3
+ %56 = OpCompositeExtract %_arr_uint_uint_32 %55 0
+ OpStore %53 %56
+ OpReturn
+ OpFunctionEnd
+)";
+
+ SetTargetEnv(SPV_ENV_VULKAN_1_1);
+ SinglePassRunAndMatch<CopyPropagateArrays>(before, false);
+}
+
} // namespace
} // namespace opt
} // namespace spvtools
