Opt: Check for side-effects in DCEInst()

This function now checks for side-effects before adding operand
instructions to the dead instruction work list.

Because this fix puts more pressure on IsCombinatorInstruction() to
be correct, this commit adds all OpConstant* and OpType* instructions
to combinator_ops_ set.

Fixes #1341.
This commit is contained in:
GregF 2018-02-21 16:43:33 -07:00 committed by Steven Perron
parent 01760d2f0f
commit 46a9ec9d23
3 changed files with 114 additions and 2 deletions

View File

@ -264,6 +264,33 @@ void IRContext::AddCombinatorsForCapability(uint32_t capability) {
combinator_ops_[0].insert({
SpvOpNop,
SpvOpUndef,
SpvOpConstant,
SpvOpConstantTrue,
SpvOpConstantFalse,
SpvOpConstantComposite,
SpvOpConstantSampler,
SpvOpConstantNull,
SpvOpTypeVoid,
SpvOpTypeBool,
SpvOpTypeInt,
SpvOpTypeFloat,
SpvOpTypeVector,
SpvOpTypeMatrix,
SpvOpTypeImage,
SpvOpTypeSampler,
SpvOpTypeSampledImage,
SpvOpTypeArray,
SpvOpTypeRuntimeArray,
SpvOpTypeStruct,
SpvOpTypeOpaque,
SpvOpTypePointer,
SpvOpTypeFunction,
SpvOpTypeEvent,
SpvOpTypeDeviceEvent,
SpvOpTypeReserveId,
SpvOpTypeQueue,
SpvOpTypePipe,
SpvOpTypeForwardPointer,
SpvOpVariable,
SpvOpImageTexelPointer,
SpvOpLoad,

View File

@ -222,8 +222,10 @@ void MemPass::DCEInst(ir::Instruction* inst,
// For all operands with no remaining uses, add their instruction
// to the dead instruction queue.
for (auto id : ids)
if (HasOnlyNamesAndDecorates(id))
deadInsts.push(get_def_use_mgr()->GetDef(id));
if (HasOnlyNamesAndDecorates(id)) {
ir::Instruction* odi = get_def_use_mgr()->GetDef(id);
if (context()->IsCombinatorInstruction(odi)) deadInsts.push(odi);
}
// if a load was deleted and it was the variable's
// last load, add all its stores to dead queue
if (varId != 0 && !IsLiveVar(varId)) AddStores(varId, &deadInsts);

View File

@ -722,6 +722,89 @@ OpFunctionEnd
predefs_before + before, predefs_after + after, true, true);
}
TEST_F(LocalSingleStoreElimTest, OptStoreOfNonCombinator) {
// Unused local store is removed but non-combinator operand remains
const std::string predefs_before =
R"(OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %gl_GlobalInvocationID
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 140
OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId
%void = OpTypeVoid
%6 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%v2uint = OpTypeVector %uint 2
%_ptr_Function_uint = OpTypePointer Function %uint
%_ptr_Uniform_uint = OpTypePointer Uniform %uint
%_ptr_Input_v2uint = OpTypePointer Input %v2uint
%164 = OpTypeImage %uint 2D 0 0 0 2 R32ui
%_ptr_UniformConstant_164 = OpTypePointer UniformConstant %164
%5661 = OpVariable %_ptr_UniformConstant_164 UniformConstant
%gl_GlobalInvocationID = OpVariable %_ptr_Input_v2uint Input
%4930 = OpVariable %_ptr_Uniform_uint Uniform
%_ptr_Image_uint = OpTypePointer Image %uint
%uint_0 = OpConstant %uint 0
%uint_1 = OpConstant %uint 1
)";
const std::string predefs_after =
R"(OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %2 "main" %gl_GlobalInvocationID
OpExecutionMode %2 OriginUpperLeft
OpSource GLSL 140
OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId
%void = OpTypeVoid
%5 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%v2uint = OpTypeVector %uint 2
%_ptr_Function_uint = OpTypePointer Function %uint
%_ptr_Uniform_uint = OpTypePointer Uniform %uint
%_ptr_Input_v2uint = OpTypePointer Input %v2uint
%11 = OpTypeImage %uint 2D 0 0 0 2 R32ui
%_ptr_UniformConstant_11 = OpTypePointer UniformConstant %11
%13 = OpVariable %_ptr_UniformConstant_11 UniformConstant
%gl_GlobalInvocationID = OpVariable %_ptr_Input_v2uint Input
%14 = OpVariable %_ptr_Uniform_uint Uniform
%_ptr_Image_uint = OpTypePointer Image %uint
%uint_0 = OpConstant %uint 0
%uint_1 = OpConstant %uint 1
)";
const std::string before =
R"(%main = OpFunction %void None %6
%12 = OpLabel
%5297 = OpVariable %_ptr_Function_uint Function
%4061 = OpLoad %164 %5661
%16843 = OpLoad %v2uint %gl_GlobalInvocationID
%24748 = OpLoad %uint %4930
%15928 = OpImageTexelPointer %_ptr_Image_uint %4061 %16843 %uint_0
%21946 = OpAtomicExchange %uint %15928 %uint_1 %uint_0 %24748
OpStore %5297 %21946
OpReturn
OpFunctionEnd
)";
const std::string after =
R"(%2 = OpFunction %void None %5
%18 = OpLabel
%20 = OpLoad %11 %13
%21 = OpLoad %v2uint %gl_GlobalInvocationID
%22 = OpLoad %uint %14
%23 = OpImageTexelPointer %_ptr_Image_uint %20 %21 %uint_0
%24 = OpAtomicExchange %uint %23 %uint_1 %uint_0 %22
OpReturn
OpFunctionEnd
)";
SinglePassRunAndCheck<opt::LocalSingleStoreElimPass>(
predefs_before + before, predefs_after + after, true, true);
}
TEST_F(LocalSingleStoreElimTest, PointerVariable) {
// Test that checks if a pointer variable is removed.