[OPT] Update DebugDeclare if the var is not an OpVariable (#6014)
When inlining, DXC can pass the result of an OpAccessChain as a
parameter. Even if this is not valid SPIR-V, HLSL allows this for the
`this` pointer in member function. Part of legalizing is to inline these
functions.
When this happens, the `var` operand on a debug declare for the parameter
will be replaced by the result of the OpAccessChain. This in invalid.
The debug declare needs the variable with the indexes.
This commit add a pass over each function after inlining that will fix
up the debug declares that are invalid in this way.
Fixes https://github.com/microsoft/DirectXShaderCompiler/issues/5191
diff --git a/source/opt/inline_exhaustive_pass.cpp b/source/opt/inline_exhaustive_pass.cpp
index bef4501..9cdea43 100644
--- a/source/opt/inline_exhaustive_pass.cpp
+++ b/source/opt/inline_exhaustive_pass.cpp
@@ -55,6 +55,11 @@
}
}
}
+
+ if (modified) {
+ FixDebugDeclares(func);
+ }
+
return (modified ? Status::SuccessWithChange : Status::SuccessWithoutChange);
}
diff --git a/source/opt/inline_pass.cpp b/source/opt/inline_pass.cpp
index 3186433..193e276 100644
--- a/source/opt/inline_pass.cpp
+++ b/source/opt/inline_pass.cpp
@@ -30,6 +30,8 @@
constexpr int kSpvFunctionCallFunctionId = 2;
constexpr int kSpvFunctionCallArgumentId = 3;
constexpr int kSpvReturnValueId = 0;
+constexpr int kSpvDebugDeclareVarInIdx = 3;
+constexpr int kSpvAccessChainBaseInIdx = 0;
} // namespace
uint32_t InlinePass::AddPointerToType(uint32_t type_id,
@@ -858,5 +860,68 @@
InlinePass::InlinePass() {}
+void InlinePass::FixDebugDeclares(Function* func) {
+ std::map<uint32_t, Instruction*> access_chains;
+ std::vector<Instruction*> debug_declare_insts;
+
+ func->ForEachInst([&access_chains, &debug_declare_insts](Instruction* inst) {
+ if (inst->opcode() == spv::Op::OpAccessChain) {
+ access_chains[inst->result_id()] = inst;
+ }
+ if (inst->GetCommonDebugOpcode() == CommonDebugInfoDebugDeclare) {
+ debug_declare_insts.push_back(inst);
+ }
+ });
+
+ for (auto& inst : debug_declare_insts) {
+ FixDebugDeclare(inst, access_chains);
+ }
+}
+
+void InlinePass::FixDebugDeclare(
+ Instruction* dbg_declare_inst,
+ const std::map<uint32_t, Instruction*>& access_chains) {
+ do {
+ uint32_t var_id =
+ dbg_declare_inst->GetSingleWordInOperand(kSpvDebugDeclareVarInIdx);
+
+ // The def-use chains are not kept up to date while inlining, so we need to
+ // get the variable by traversing the functions.
+ auto it = access_chains.find(var_id);
+ if (it == access_chains.end()) {
+ return;
+ }
+ Instruction* access_chain = it->second;
+
+ // If the variable id in the debug declare is an access chain, it is
+ // invalid. it needs to be fixed up. The debug declare will be updated so
+ // that its Var operand becomes the base of the access chain. The indexes of
+ // the access chain are prepended before the indexes of the debug declare.
+
+ std::vector<Operand> operands;
+ for (int i = 0; i < kSpvDebugDeclareVarInIdx; i++) {
+ operands.push_back(dbg_declare_inst->GetInOperand(i));
+ }
+
+ uint32_t access_chain_base =
+ access_chain->GetSingleWordInOperand(kSpvAccessChainBaseInIdx);
+ operands.push_back(Operand(SPV_OPERAND_TYPE_ID, {access_chain_base}));
+ operands.push_back(
+ dbg_declare_inst->GetInOperand(kSpvDebugDeclareVarInIdx + 1));
+
+ for (uint32_t i = kSpvAccessChainBaseInIdx + 1;
+ i < access_chain->NumInOperands(); ++i) {
+ operands.push_back(access_chain->GetInOperand(i));
+ }
+
+ for (uint32_t i = kSpvDebugDeclareVarInIdx + 2;
+ i < dbg_declare_inst->NumInOperands(); ++i) {
+ operands.push_back(dbg_declare_inst->GetInOperand(i));
+ }
+
+ dbg_declare_inst->SetInOperands(std::move(operands));
+ } while (true);
+}
+
} // namespace opt
} // namespace spvtools
diff --git a/source/opt/inline_pass.h b/source/opt/inline_pass.h
index 1c9d60e..7bea31d 100644
--- a/source/opt/inline_pass.h
+++ b/source/opt/inline_pass.h
@@ -150,6 +150,12 @@
// Initialize state for optimization of |module|
void InitializeInline();
+ // Fixes invalid debug declare functions in `func` that were caused by
+ // inlining. This function cannot be called while in the middle of inlining
+ // because it needs to be able to find the instructions that define an
+ // id.
+ void FixDebugDeclares(Function* func);
+
// Map from function's result id to function.
std::unordered_map<uint32_t, Function*> id2function_;
@@ -241,6 +247,11 @@
// structural dominance.
void UpdateSingleBlockLoopContinueTarget(
uint32_t new_id, std::vector<std::unique_ptr<BasicBlock>>* new_blocks);
+
+ // Replaces the `var` operand of `dbg_declare_inst` and updates the indexes
+ // accordingly, if it is the id of an access chain in `access_chains`.
+ void FixDebugDeclare(Instruction* dbg_declare_inst,
+ const std::map<uint32_t, Instruction*>& access_chains);
};
} // namespace opt
diff --git a/source/val/validate_function.cpp b/source/val/validate_function.cpp
index 4879a7d..50ae0a2 100644
--- a/source/val/validate_function.cpp
+++ b/source/val/validate_function.cpp
@@ -258,7 +258,8 @@
_.HasCapability(spv::Capability::VariablePointers) &&
sc == spv::StorageClass::Workgroup;
const bool uc_ptr = sc == spv::StorageClass::UniformConstant;
- if (!ssbo_vptr && !wg_vptr && !uc_ptr) {
+ if (!_.options()->before_hlsl_legalization && !ssbo_vptr &&
+ !wg_vptr && !uc_ptr) {
return _.diag(SPV_ERROR_INVALID_ID, inst)
<< "Pointer operand " << _.getIdName(argument_id)
<< " must be a memory object declaration";
diff --git a/test/opt/inline_test.cpp b/test/opt/inline_test.cpp
index ef7ac37..ecfa7c7 100644
--- a/test/opt/inline_test.cpp
+++ b/test/opt/inline_test.cpp
@@ -4471,6 +4471,86 @@
SinglePassRunAndMatch<InlineExhaustivePass>(text, true);
}
+TEST_F(InlineTest, DebugDeclareWithAccessChain) {
+ const std::string text = R"(
+; CHECK: [[EmptyStruct:%[\w]+]] = OpTypeStruct %float
+; CHECK-DAG: [[Struct:%[\w]+]] = OpTypeStruct [[EmptyStruct]]
+; CHECK-DAG: [[PtrType:%[\w]+]] = OpTypePointer Function [[Struct]]
+; CHECK-DAG: [[EmptyPtrType:%[\w]+]] = OpTypePointer Function [[EmptyStruct]]
+ OpCapability Shader
+ OpExtension "SPV_KHR_non_semantic_info"
+ OpExtension "SPV_KHR_relaxed_extended_instruction"
+ %1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint GLCompute %2 "computeMain"
+ OpExecutionMode %2 LocalSize 1 1 1
+ %3 = OpString "s.hlsl"
+ %4 = OpString "float"
+ %5 = OpString "source"
+ %6 = OpString "a"
+ %7 = OpString "SomeStruct"
+ %8 = OpString "SomeStruct.getA"
+ %9 = OpString ""
+ %10 = OpString "this"
+ %int = OpTypeInt 32 1
+ %int_0 = OpConstant %int 0
+ %uint = OpTypeInt 32 0
+ %uint_0 = OpConstant %uint 0
+ %uint_32 = OpConstant %uint 32
+ %float = OpTypeFloat 32
+ %void = OpTypeVoid
+ %uint_3 = OpConstant %uint 3
+ %uint_1 = OpConstant %uint 1
+ %uint_4 = OpConstant %uint 4
+ %uint_5 = OpConstant %uint 5
+ %uint_11 = OpConstant %uint 11
+ %uint_8 = OpConstant %uint 8
+ %uint_288 = OpConstant %uint 288
+ %25 = OpTypeFunction %void
+ %_struct_26 = OpTypeStruct %float
+ %_struct_27 = OpTypeStruct %_struct_26
+%_ptr_Function__struct_27 = OpTypePointer Function %_struct_27
+%_ptr_Function__struct_26 = OpTypePointer Function %_struct_26
+%_ptr_Function_float = OpTypePointer Function %float
+ %30 = OpTypeFunction %float %_ptr_Function__struct_26 %_ptr_Function_float
+ %31 = OpUndef %float
+ %32 = OpExtInst %void %1 DebugTypeBasic %4 %uint_32 %uint_3 %uint_0
+ %33 = OpExtInst %void %1 DebugSource %3 %5
+ %34 = OpExtInst %void %1 DebugCompilationUnit %uint_1 %uint_4 %33 %uint_5
+ %35 = OpExtInst %void %1 DebugTypeMember %6 %32 %33 %uint_3 %uint_11 %uint_0 %uint_32 %uint_3
+ %36 = OpExtInstWithForwardRefsKHR %void %1 DebugTypeComposite %7 %uint_1 %33 %uint_1 %uint_8 %34 %7 %uint_32 %uint_3 %35 %37
+ %38 = OpExtInst %void %1 DebugTypeFunction %uint_3 %32 %36
+ %37 = OpExtInst %void %1 DebugFunction %8 %38 %33 %uint_4 %uint_5 %36 %9 %uint_3 %uint_4
+ %39 = OpExtInst %void %1 DebugLocalVariable %10 %36 %33 %uint_4 %uint_5 %37 %uint_288 %uint_1
+ %52 = OpExtInst %void %1 DebugLocalVariable %10 %32 %33 %uint_4 %uint_5 %37 %uint_288 %uint_1
+ %40 = OpExtInst %void %1 DebugExpression
+; CHECK: OpFunction %void None
+; CHECK: [[Var:%[\w]+]] = OpVariable [[PtrType]] Function
+; CHECK: OpExtInst %void {{%[\w+]+}} DebugDeclare {{%[\w+]+}} [[Var]] {{%[\w+]+}} %int_0
+; CHECK: OpExtInst %void {{%[\w+]+}} DebugDeclare {{%[\w+]+}} [[Var]] {{%[\w+]+}} %int_0 %int_0
+ %2 = OpFunction %void None %25
+ %41 = OpLabel
+ %42 = OpVariable %_ptr_Function__struct_27 Function
+ %43 = OpAccessChain %_ptr_Function__struct_26 %42 %int_0
+ %49 = OpAccessChain %_ptr_Function_float %43 %int_0
+ %44 = OpFunctionCall %float %45 %43 %49
+ OpReturn
+ OpFunctionEnd
+; CHECK: OpFunction %float None
+ %45 = OpFunction %float None %30
+ %46 = OpFunctionParameter %_ptr_Function__struct_26
+ %50 = OpFunctionParameter %_ptr_Function_float
+ %47 = OpLabel
+ %48 = OpExtInst %void %1 DebugDeclare %39 %46 %40
+ %51 = OpExtInst %void %1 DebugDeclare %52 %50 %40
+ OpReturnValue %31
+ OpFunctionEnd
+)";
+
+ SetTargetEnv(SPV_ENV_VULKAN_1_2);
+ SinglePassRunAndMatch<InlineExhaustivePass>(text, true);
+}
+
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Empty modules
