Skip to content

Commit ca37349

Browse files
authored
[opt] Fix pointer stores in DCE (KhronosGroup#5739)
When a trying to mark store that use the same address as a load live, we consider any use of the pointer in the store instruction enough to make the store live. This is not correct. We should only mark the store as live if it store to the pointer, and not storing the pointer to another memory location. This causes DCE to miss some dead code.
1 parent a081752 commit ca37349

File tree

2 files changed

+83
-1
lines changed

2 files changed

+83
-1
lines changed

source/opt/aggressive_dead_code_elim_pass.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,12 @@ void AggressiveDCEPass::AddStores(Function* func, uint32_t ptrId) {
134134
}
135135
break;
136136
// If default, assume it stores e.g. frexp, modf, function call
137-
case spv::Op::OpStore:
137+
case spv::Op::OpStore: {
138+
const uint32_t kStoreTargetAddrInIdx = 0;
139+
if (user->GetSingleWordInOperand(kStoreTargetAddrInIdx) == ptrId)
140+
AddToWorklist(user);
141+
break;
142+
}
138143
default:
139144
AddToWorklist(user);
140145
break;

test/opt/aggressive_dead_code_elim_test.cpp

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7992,6 +7992,83 @@ OpFunctionEnd
79927992
SinglePassRunAndCheck<AggressiveDCEPass>(test, test, true, true);
79937993
}
79947994

7995+
TEST_F(AggressiveDCETest, StoringAPointer) {
7996+
// A store that stores a pointer should not be kept live because the value
7997+
// being stored is eventually loaded from.
7998+
7999+
const std::string text = R"(
8000+
OpCapability CooperativeMatrixKHR
8001+
OpCapability Shader
8002+
OpExtension "SPV_KHR_cooperative_matrix"
8003+
OpMemoryModel Logical GLSL450
8004+
OpEntryPoint GLCompute %1 "main" %2
8005+
OpExecutionMode %1 LocalSize 64 1 1
8006+
OpSource HLSL 600
8007+
OpDecorate %2 DescriptorSet 0
8008+
OpDecorate %2 Binding 0
8009+
OpDecorate %_runtimearr_int ArrayStride 4
8010+
OpMemberDecorate %_struct_4 0 Offset 0
8011+
OpDecorate %_struct_4 Block
8012+
%int = OpTypeInt 32 1
8013+
%int_0 = OpConstant %int 0
8014+
%int_1 = OpConstant %int 1
8015+
%uint = OpTypeInt 32 0
8016+
%uint_0 = OpConstant %uint 0
8017+
%uint_64 = OpConstant %uint 64
8018+
%uint_3 = OpConstant %uint 3
8019+
%uint_16 = OpConstant %uint 16
8020+
%uint_4 = OpConstant %uint 4
8021+
%_runtimearr_int = OpTypeRuntimeArray %int
8022+
%_struct_4 = OpTypeStruct %_runtimearr_int
8023+
%_ptr_StorageBuffer__struct_4 = OpTypePointer StorageBuffer %_struct_4
8024+
%void = OpTypeVoid
8025+
%16 = OpTypeFunction %void
8026+
; CHECK: [[mat:%\w+]] = OpTypeCooperativeMatrixKHR %int %uint_3 %uint_16 %uint_4 %uint_0
8027+
%17 = OpTypeCooperativeMatrixKHR %int %uint_3 %uint_16 %uint_4 %uint_0
8028+
; CHECK: [[struct:%\w+]] = OpTypeStruct [[mat]]
8029+
%_struct_18 = OpTypeStruct %17
8030+
; CHECK: [[ptr:%\w+]] = OpTypePointer Function [[struct]]
8031+
%_ptr_Function__struct_18 = OpTypePointer Function %_struct_18
8032+
%_ptr_StorageBuffer_int = OpTypePointer StorageBuffer %int
8033+
%_ptr_Function_17 = OpTypePointer Function %17
8034+
%_ptr_Function_int = OpTypePointer Function %int
8035+
%_ptr_Function__ptr_Function_int = OpTypePointer Function %_ptr_Function_int
8036+
%2 = OpVariable %_ptr_StorageBuffer__struct_4 StorageBuffer
8037+
8038+
; The stored to the fist two variables should be removed and the variables
8039+
; as well. The only function scope variable should be the cooperative matrix.
8040+
; CHECK: OpFunction
8041+
; CHECK-NOT: OpVariable %_ptr_Function__ptr_Function_int Function
8042+
; CHECK: OpVariable [[ptr]] Function
8043+
; CHECK-NOT: OpVariable
8044+
%1 = OpFunction %void None %16
8045+
%24 = OpLabel
8046+
%25 = OpVariable %_ptr_Function__ptr_Function_int Function
8047+
%26 = OpVariable %_ptr_Function__ptr_Function_int Function
8048+
%27 = OpVariable %_ptr_Function__struct_18 Function
8049+
%28 = OpAccessChain %_ptr_StorageBuffer_int %2 %int_0 %uint_0
8050+
%29 = OpCooperativeMatrixLoadKHR %17 %28 %int_1
8051+
%30 = OpCompositeConstruct %_struct_18 %29
8052+
OpStore %27 %30
8053+
%31 = OpAccessChain %_ptr_Function_17 %27 %int_0
8054+
%32 = OpAccessChain %_ptr_Function_int %27 %int_0 %uint_0
8055+
OpStore %26 %32
8056+
%33 = OpLoad %int %32
8057+
%34 = OpIAdd %int %33 %int_1
8058+
OpStore %25 %32
8059+
OpStore %32 %34
8060+
%35 = OpAccessChain %_ptr_StorageBuffer_int %2 %int_0 %uint_64
8061+
%36 = OpLoad %17 %31
8062+
OpCooperativeMatrixStoreKHR %35 %36 %int_0
8063+
OpReturn
8064+
OpFunctionEnd
8065+
)";
8066+
8067+
// For physical storage buffer support
8068+
SetTargetEnv(SPV_ENV_VULKAN_1_2);
8069+
SinglePassRunAndMatch<AggressiveDCEPass>(text, true);
8070+
}
8071+
79958072
} // namespace
79968073
} // namespace opt
79978074
} // namespace spvtools

0 commit comments

Comments
 (0)