Skip to content

Commit 6b52169

Browse files
committed
spirv-opt: Fix OpUndef placement in ADCE
ADCE was modifying an existing instruction in order to get the OpUndef. This would leave the OpUndef in an invalid position. Even through it is allowed for an OpUndef to be in a function. We are moving away from that. This commit create a new instruction for the OpUndef, and places it in the right place. Fixes KhronosGroup#6316
1 parent 33b1bcb commit 6b52169

File tree

2 files changed

+66
-9
lines changed

2 files changed

+66
-9
lines changed

source/opt/aggressive_dead_code_elim_pass.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -306,16 +306,14 @@ void AggressiveDCEPass::ProcessDebugInformation(
306306
auto def = get_def_use_mgr()->GetDef(id);
307307
if (!live_insts_.Set(def->unique_id())) {
308308
AddToWorklist(inst);
309+
uint32_t undef_id = Type2Undef(def->type_id());
310+
inst->SetInOperand(kDebugValueValue, {undef_id});
309311
context()->get_def_use_mgr()->UpdateDefUse(inst);
310-
worklist_.push(def);
311-
def->SetOpcode(spv::Op::OpUndef);
312-
def->SetInOperands({});
313312
id = inst->GetSingleWordInOperand(kDebugValueLocalVariable);
314313
auto localVar = get_def_use_mgr()->GetDef(id);
315314
AddToWorklist(localVar);
316315
context()->get_def_use_mgr()->UpdateDefUse(localVar);
317316
AddOperandsToWorkList(localVar);
318-
context()->get_def_use_mgr()->UpdateDefUse(def);
319317
id = inst->GetSingleWordInOperand(kDebugValueExpression);
320318
auto expression = get_def_use_mgr()->GetDef(id);
321319
AddToWorklist(expression);

test/opt/aggressive_dead_code_elim_test.cpp

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8648,6 +8648,7 @@ OpExecutionMode %main LocalSize 1 1 1
86488648
%17 = OpString "int"
86498649
%25 = OpString "main"
86508650
%26 = OpString ""
8651+
; CHECK: [[VarName:%\w+]] = OpString "some_real_obvious_name"
86518652
%30 = OpString "some_real_obvious_name"
86528653
%33 = OpString "__dxc_setup"
86538654
%35 = OpString "cb000c74"
@@ -8684,6 +8685,10 @@ OpDecorate %b Binding 0
86848685
%b = OpVariable %_ptr_UniformConstant_type_buffer_image UniformConstant
86858686
%int_1 = OpConstant %int 1
86868687
%int_2 = OpConstant %int 2
8688+
; CHECK: [[undef:%\w+]] = OpUndef %int
8689+
; ADCE will create another undef and use the new one.
8690+
; CHECK: [[undef:%\w+]] = OpUndef %int
8691+
%198 = OpUndef %int
86878692
%38 = OpExtInst %void %1 DebugInfoNone
86888693
%16 = OpExtInst %void %1 DebugExpression
86898694
%18 = OpExtInst %void %1 DebugTypeBasic %17 %uint_32 %uint_4 %uint_0
@@ -8694,7 +8699,7 @@ OpDecorate %b Binding 0
86948699
%27 = OpExtInst %void %1 DebugFunction %25 %21 %22 %uint_4 %uint_1 %23 %26 %uint_3 %uint_4
86958700
%28 = OpExtInst %void %1 DebugLexicalBlock %22 %uint_4 %uint_13 %27
86968701
%31 = OpExtInst %void %1 DebugLocalVariable %30 %20 %22 %uint_5 %uint_7 %28 %uint_4
8697-
; CHECK: %31 = OpExtInst %void %1 DebugLocalVariable %30 %20 %22 %uint_5 %uint_7 %28 %uint_4
8702+
; CHECK: [[var:%\w+]] = OpExtInst %void {{%\w+}} DebugLocalVariable [[VarName]]
86988703
%34 = OpExtInst %void %1 DebugFunction %33 %21 %22 %uint_4 %uint_1 %23 %26 %uint_3 %uint_4
86998704
%41 = OpExtInst %void %1 DebugTypeComposite %39 %uint_0 %22 %uint_0 %uint_0 %23 %40 %38 %uint_3
87008705
%43 = OpExtInst %void %1 DebugTypeTemplateParameter %42 %18 %38 %22 %uint_0 %uint_0
@@ -8708,13 +8713,12 @@ OpDecorate %b Binding 0
87088713
%52 = OpExtInst %void %1 DebugFunctionDefinition %34 %main
87098714
%304 = OpExtInst %void %1 DebugScope %28 %133
87108715
%199 = OpExtInst %void %1 DebugLine %22 %uint_5 %uint_5 %uint_3 %uint_52
8711-
%198 = OpUndef %int
87128716
%245 = OpExtInst %void %1 DebugValue %31 %198 %16 %int_0
87138717
%242 = OpExtInst %void %1 DebugValue %31 %198 %16 %int_1
87148718
%239 = OpExtInst %void %1 DebugValue %31 %198 %16 %int_2
8715-
; CHECK: %245 = OpExtInst %void %1 DebugValue %31 %198 %16 %int_0
8716-
; CHECK-NOT: %242 = OpExtInst %void %1 DebugValue %31 %198 %16 %int_1
8717-
; CHECK-NOT: %239 = OpExtInst %void %1 DebugValue %31 %198 %16 %int_2
8719+
; CHECK: {{%\w+}} = OpExtInst %void {{%\w+}} DebugValue [[var]] [[undef]] {{%\w+}} %int_0
8720+
; CHECK-NOT: {{%\w+}} = OpExtInst %void {{%\w+}} DebugValue
8721+
; CHECK-NOT: {{%\w+}} = OpExtInst %void {{%\w+}} DebugValue
87188722
%160 = OpExtInst %void %1 DebugLine %22 %uint_6 %uint_6 %uint_3 %uint_10
87198723
%147 = OpLoad %type_buffer_image %b
87208724
OpImageWrite %147 %uint_0 %int_0 None
@@ -8731,6 +8735,61 @@ OpFunctionEnd
87318735
SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES);
87328736
SinglePassRunAndMatch<AggressiveDCEPass>(before, false);
87338737
}
8738+
8739+
TEST_F(AggressiveDCETest, UndefIsOutsideFunction) {
8740+
const std::string spirv = R"(
8741+
; CHECK: OpUndef
8742+
; CHECK: OpFunction %void
8743+
OpCapability Shader
8744+
OpExtension "SPV_KHR_non_semantic_info"
8745+
%1 = OpExtInstImport "NonSemantic.Shader.DebugInfo.100"
8746+
OpMemoryModel Logical GLSL450
8747+
OpEntryPoint GLCompute %2 "main"
8748+
OpExecutionMode %2 LocalSize 1 1 1
8749+
%3 = OpString ""
8750+
%4 = OpString "int"
8751+
%5 = OpString "x"
8752+
%6 = OpString "Val.set"
8753+
%7 = OpString "Val"
8754+
%8 = OpString "this"
8755+
%void = OpTypeVoid
8756+
%uint = OpTypeInt 32 0
8757+
%uint_11 = OpConstant %uint 11
8758+
%uint_5 = OpConstant %uint 5
8759+
%uint_100 = OpConstant %uint 100
8760+
%14 = OpTypeFunction %void
8761+
%int = OpTypeInt 32 1
8762+
%_ptr_Function_int = OpTypePointer Function %int
8763+
%uint_0 = OpConstant %uint 0
8764+
%uint_15 = OpConstant %uint 15
8765+
%uint_6 = OpConstant %uint 6
8766+
%uint_32 = OpConstant %uint 32
8767+
%uint_4 = OpConstant %uint 4
8768+
%uint_131072 = OpConstant %uint 131072
8769+
%uint_1 = OpConstant %uint 1
8770+
%uint_10 = OpConstant %uint 10
8771+
%uint_8 = OpConstant %uint 8
8772+
%26 = OpExtInst %void %1 DebugOperation %uint_0
8773+
%27 = OpExtInst %void %1 DebugSource %3 %3
8774+
%28 = OpExtInst %void %1 DebugCompilationUnit %uint_100 %uint_5 %27 %uint_11
8775+
%29 = OpExtInst %void %1 DebugTypeBasic %4 %uint_32 %uint_4 %uint_131072
8776+
%30 = OpExtInst %void %1 DebugTypeFunction %uint_0 %void
8777+
%31 = OpExtInst %void %1 DebugFunction %6 %30 %27 %uint_6 %uint_10 %28 %6 %uint_0 %uint_6
8778+
%32 = OpExtInst %void %1 DebugTypeMember %5 %29 %27 %uint_1 %uint_8 %uint_0 %uint_32 %uint_0
8779+
%33 = OpExtInst %void %1 DebugTypeComposite %7 %uint_1 %27 %uint_1 %uint_8 %28 %7 %uint_32 %uint_131072 %32
8780+
%34 = OpExtInst %void %1 DebugLocalVariable %8 %33 %27 %uint_6 %uint_10 %31 %uint_0 %uint_1
8781+
%35 = OpExtInst %void %1 DebugExpression %26
8782+
%2 = OpFunction %void None %14
8783+
%36 = OpLabel
8784+
%37 = OpVariable %_ptr_Function_int Function
8785+
%38 = OpExtInst %void %1 DebugValue %34 %37 %35 %uint_0
8786+
OpReturn
8787+
OpFunctionEnd
8788+
8789+
)";
8790+
8791+
SinglePassRunAndMatch<AggressiveDCEPass>(spirv, true);
8792+
}
87348793
} // namespace
87358794
} // namespace opt
87368795
} // namespace spvtools

0 commit comments

Comments
 (0)