Get ADCE to handle OpCopyMemory

ADCE does not treat OpCopyMemory as an instruction that references
memory.  Because of that stores are removed that should not be.

This change teaches ADCE that OpCopyMemory and OpCopyMemorySize both
loads from and stores to memory.  This will keep other stores live when
needed, and will allows ADCE to remove OpCopyMemory instructions as
well.

Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1556.
This commit is contained in:
Steven Perron 2018-05-16 10:41:40 -04:00
parent b09e3ce842
commit f1f7cc870e
2 changed files with 244 additions and 1 deletions

View File

@ -34,6 +34,8 @@ const uint32_t kEntryPointFunctionIdInIdx = 1;
const uint32_t kSelectionMergeMergeBlockIdInIdx = 0;
const uint32_t kLoopMergeMergeBlockIdInIdx = 0;
const uint32_t kLoopMergeContinueBlockIdInIdx = 1;
const uint32_t kCopyMemoryTargetAddrInIdx = 0;
const uint32_t kCopyMemorySourceAddrInIdx = 1;
// Sorting functor to present annotation instructions in an easy-to-process
// order. The functor orders by opcode first and falls back on unique id
@ -103,7 +105,7 @@ bool AggressiveDCEPass::IsLocalVar(uint32_t varId) {
}
void AggressiveDCEPass::AddStores(uint32_t ptrId) {
get_def_use_mgr()->ForEachUser(ptrId, [this](ir::Instruction* user) {
get_def_use_mgr()->ForEachUser(ptrId, [this, ptrId](ir::Instruction* user) {
switch (user->opcode()) {
case SpvOpAccessChain:
case SpvOpInBoundsAccessChain:
@ -112,6 +114,12 @@ void AggressiveDCEPass::AddStores(uint32_t ptrId) {
break;
case SpvOpLoad:
break;
case SpvOpCopyMemory:
case SpvOpCopyMemorySized:
if (user->GetSingleWordInOperand(kCopyMemoryTargetAddrInIdx) == ptrId) {
AddToWorklist(user);
}
break;
// If default, assume it stores e.g. frexp, modf, function call
case SpvOpStore:
default:
@ -341,6 +349,17 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
else if (!IsVarOfStorage(varId, SpvStorageClassFunction))
AddToWorklist(&*ii);
} break;
case SpvOpCopyMemory:
case SpvOpCopyMemorySized: {
uint32_t varId;
(void)GetPtr(ii->GetSingleWordInOperand(kCopyMemoryTargetAddrInIdx),
&varId);
if (IsVarOfStorage(varId, SpvStorageClassPrivate) ||
IsVarOfStorage(varId, SpvStorageClassWorkgroup))
private_stores_.push_back(&*ii);
else if (!IsVarOfStorage(varId, SpvStorageClassFunction))
AddToWorklist(&*ii);
} break;
case SpvOpLoopMerge: {
assume_branches_live.push(false);
currentMergeBlockId.push(
@ -418,6 +437,14 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
if (varId != 0) {
ProcessLoad(varId);
}
} else if (liveInst->opcode() == SpvOpCopyMemory ||
liveInst->opcode() == SpvOpCopyMemorySized) {
uint32_t varId;
(void)GetPtr(liveInst->GetSingleWordInOperand(kCopyMemorySourceAddrInIdx),
&varId);
if (varId != 0) {
ProcessLoad(varId);
}
}
// If function call, treat as if it loads from all pointer arguments
else if (liveInst->opcode() == SpvOpFunctionCall) {

View File

@ -5549,6 +5549,222 @@ OpFunctionEnd
preamble + body_before, preamble + body_after, true, true);
}
TEST_F(AggressiveDCETest, CopyMemoryToGlobal) {
// |local| is loaded in an OpCopyMemory instruction. So the store must be
// kept alive.
const std::string test =
R"(OpCapability Geometry
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Geometry %main "main" %global
OpExecutionMode %main Triangles
OpExecutionMode %main Invocations 1
OpExecutionMode %main OutputTriangleStrip
OpExecutionMode %main OutputVertices 5
OpSource GLSL 440
OpName %main "main"
OpName %local "local"
OpName %global "global"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%12 = OpConstantNull %v4float
%_ptr_Function_v4float = OpTypePointer Function %v4float
%_ptr_Output_v4float = OpTypePointer Output %v4float
%global = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %7
%19 = OpLabel
%local = OpVariable %_ptr_Function_v4float Function
OpStore %local %12
OpCopyMemory %global %local
OpEndPrimitive
OpReturn
OpFunctionEnd
)";
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<opt::AggressiveDCEPass>(test, test, true, true);
}
TEST_F(AggressiveDCETest, CopyMemoryToLocal) {
// Make sure the store to |local2| using OpCopyMemory is kept and keeps
// |local1| alive.
const std::string test =
R"(OpCapability Geometry
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Geometry %main "main" %global
OpExecutionMode %main Triangles
OpExecutionMode %main Invocations 1
OpExecutionMode %main OutputTriangleStrip
OpExecutionMode %main OutputVertices 5
OpSource GLSL 440
OpName %main "main"
OpName %local1 "local1"
OpName %local2 "local2"
OpName %global "global"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%12 = OpConstantNull %v4float
%_ptr_Function_v4float = OpTypePointer Function %v4float
%_ptr_Output_v4float = OpTypePointer Output %v4float
%global = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %7
%19 = OpLabel
%local1 = OpVariable %_ptr_Function_v4float Function
%local2 = OpVariable %_ptr_Function_v4float Function
OpStore %local1 %12
OpCopyMemory %local2 %local1
OpCopyMemory %global %local2
OpEndPrimitive
OpReturn
OpFunctionEnd
)";
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<opt::AggressiveDCEPass>(test, test, true, true);
}
TEST_F(AggressiveDCETest, RemoveCopyMemoryToLocal) {
// Test that we remove function scope variables that are stored to using
// OpCopyMemory, but are never loaded. We can remove both |local1| and
// |local2|.
const std::string test =
R"(OpCapability Geometry
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Geometry %main "main" %global
OpExecutionMode %main Triangles
OpExecutionMode %main Invocations 1
OpExecutionMode %main OutputTriangleStrip
OpExecutionMode %main OutputVertices 5
OpSource GLSL 440
OpName %main "main"
OpName %local1 "local1"
OpName %local2 "local2"
OpName %global "global"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%12 = OpConstantNull %v4float
%_ptr_Function_v4float = OpTypePointer Function %v4float
%_ptr_Output_v4float = OpTypePointer Output %v4float
%global = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %7
%19 = OpLabel
%local1 = OpVariable %_ptr_Function_v4float Function
%local2 = OpVariable %_ptr_Function_v4float Function
OpStore %local1 %12
OpCopyMemory %local2 %local1
OpEndPrimitive
OpReturn
OpFunctionEnd
)";
const std::string result =
R"(OpCapability Geometry
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Geometry %main "main" %global
OpExecutionMode %main Triangles
OpExecutionMode %main Invocations 1
OpExecutionMode %main OutputTriangleStrip
OpExecutionMode %main OutputVertices 5
OpSource GLSL 440
OpName %main "main"
OpName %global "global"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%_ptr_Output_v4float = OpTypePointer Output %v4float
%global = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %7
%19 = OpLabel
OpEndPrimitive
OpReturn
OpFunctionEnd
)";
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<opt::AggressiveDCEPass>(test, result, true, true);
}
TEST_F(AggressiveDCETest, RemoveCopyMemoryToLocal2) {
// We are able to remove "local2" because it is not loaded, but have to keep
// the stores to "local1".
const std::string test =
R"(OpCapability Geometry
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Geometry %main "main" %global
OpExecutionMode %main Triangles
OpExecutionMode %main Invocations 1
OpExecutionMode %main OutputTriangleStrip
OpExecutionMode %main OutputVertices 5
OpSource GLSL 440
OpName %main "main"
OpName %local1 "local1"
OpName %local2 "local2"
OpName %global "global"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%12 = OpConstantNull %v4float
%_ptr_Function_v4float = OpTypePointer Function %v4float
%_ptr_Output_v4float = OpTypePointer Output %v4float
%global = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %7
%19 = OpLabel
%local1 = OpVariable %_ptr_Function_v4float Function
%local2 = OpVariable %_ptr_Function_v4float Function
OpStore %local1 %12
OpCopyMemory %local2 %local1
OpCopyMemory %global %local1
OpEndPrimitive
OpReturn
OpFunctionEnd
)";
const std::string result =
R"(OpCapability Geometry
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Geometry %main "main" %global
OpExecutionMode %main Triangles
OpExecutionMode %main Invocations 1
OpExecutionMode %main OutputTriangleStrip
OpExecutionMode %main OutputVertices 5
OpSource GLSL 440
OpName %main "main"
OpName %local1 "local1"
OpName %global "global"
%void = OpTypeVoid
%7 = OpTypeFunction %void
%float = OpTypeFloat 32
%v4float = OpTypeVector %float 4
%12 = OpConstantNull %v4float
%_ptr_Function_v4float = OpTypePointer Function %v4float
%_ptr_Output_v4float = OpTypePointer Output %v4float
%global = OpVariable %_ptr_Output_v4float Output
%main = OpFunction %void None %7
%19 = OpLabel
%local1 = OpVariable %_ptr_Function_v4float Function
OpStore %local1 %12
OpCopyMemory %global %local1
OpEndPrimitive
OpReturn
OpFunctionEnd
)";
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<opt::AggressiveDCEPass>(test, result, true, true);
}
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Check that logical addressing required