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.
diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp
index 5ea2090..6d1b7fb 100644
--- a/source/opt/aggressive_dead_code_elim_pass.cpp
+++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -34,6 +34,8 @@
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 @@
}
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 @@
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 @@
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 @@
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) {
diff --git a/test/opt/aggressive_dead_code_elim_test.cpp b/test/opt/aggressive_dead_code_elim_test.cpp
index 372883a..b0435b2 100644
--- a/test/opt/aggressive_dead_code_elim_test.cpp
+++ b/test/opt/aggressive_dead_code_elim_test.cpp
@@ -5549,6 +5549,222 @@
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
