Teach ADCE about OpImageTexelPointer

Currently OpImageTexelPointer operations are treat like a use of the
pointer, but it does
not look for the memory being referenced to make sure stores are not
removed.

This change teaches it so identify the memory being accessed, and
treats it as if that memory is loaded.

Fixes to #1445.
This commit is contained in:
Steven Perron 2018-04-02 13:40:53 -04:00
parent c33af63264
commit 7c5d49bf2a
4 changed files with 57 additions and 8 deletions

View File

@ -433,6 +433,15 @@ bool AggressiveDCEPass::AggressiveDCE(ir::Function* func) {
else if (liveInst->opcode() == SpvOpFunctionParameter) {
ProcessLoad(liveInst->result_id());
}
// We treat an OpImageTexelPointer as a load of the pointer, and
// that value is manipulated to get the result.
else if (liveInst->opcode() == SpvOpImageTexelPointer) {
uint32_t varId;
(void)GetPtr(liveInst, &varId);
if (varId != 0) {
ProcessLoad(varId);
}
}
worklist_.pop();
}

View File

@ -158,8 +158,8 @@ bool Instruction::IsReadOnlyLoad() const {
Instruction* Instruction::GetBaseAddress() const {
assert((IsLoad() || opcode() == SpvOpStore || opcode() == SpvOpAccessChain ||
opcode() == SpvOpInBoundsAccessChain ||
opcode() == SpvOpCopyObject) &&
opcode() == SpvOpInBoundsAccessChain || opcode() == SpvOpCopyObject ||
opcode() == SpvOpImageTexelPointer) &&
"GetBaseAddress should only be called on instructions that take a "
"pointer or image.");
uint32_t base = GetSingleWordInOperand(kLoadBaseIndex);

View File

@ -28,9 +28,7 @@ namespace opt {
namespace {
const uint32_t kCopyObjectOperandInIdx = 0;
const uint32_t kLoadPtrIdInIdx = 0;
const uint32_t kLoopMergeMergeBlockIdInIdx = 0;
const uint32_t kStorePtrIdInIdx = 0;
const uint32_t kStoreValIdInIdx = 1;
const uint32_t kTypePointerStorageClassInIdx = 0;
const uint32_t kTypePointerTypeIdInIdx = 1;
@ -119,10 +117,11 @@ ir::Instruction* MemPass::GetPtr(uint32_t ptrId, uint32_t* varId) {
}
ir::Instruction* MemPass::GetPtr(ir::Instruction* ip, uint32_t* varId) {
const SpvOp op = ip->opcode();
assert(op == SpvOpStore || op == SpvOpLoad);
const uint32_t ptrId = ip->GetSingleWordInOperand(
op == SpvOpStore ? kStorePtrIdInIdx : kLoadPtrIdInIdx);
assert(ip->opcode() == SpvOpStore || ip->opcode() == SpvOpLoad ||
ip->opcode() == SpvOpImageTexelPointer);
// All of these opcode place the pointer in position 0.
const uint32_t ptrId = ip->GetSingleWordInOperand(0);
return GetPtr(ptrId, varId);
}

View File

@ -5369,6 +5369,47 @@ OpFunctionEnd
SinglePassRunAndCheck<opt::AggressiveDCEPass>(text, text, true, true);
}
TEST_F(AggressiveDCETest, AtomicAdd) {
const std::string text = R"(OpCapability SampledBuffer
OpCapability StorageImageExtendedFormats
OpCapability ImageBuffer
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint GLCompute %2 "min" %gl_GlobalInvocationID
OpExecutionMode %2 LocalSize 64 1 1
OpSource HLSL 600
OpDecorate %gl_GlobalInvocationID BuiltIn GlobalInvocationId
OpDecorate %4 DescriptorSet 4
OpDecorate %4 Binding 70
%uint = OpTypeInt 32 0
%6 = OpTypeImage %uint Buffer 0 0 0 2 R32ui
%_ptr_UniformConstant_6 = OpTypePointer UniformConstant %6
%_ptr_Private_6 = OpTypePointer Private %6
%void = OpTypeVoid
%10 = OpTypeFunction %void
%uint_0 = OpConstant %uint 0
%uint_1 = OpConstant %uint 1
%v3uint = OpTypeVector %uint 3
%_ptr_Input_v3uint = OpTypePointer Input %v3uint
%_ptr_Image_uint = OpTypePointer Image %uint
%4 = OpVariable %_ptr_UniformConstant_6 UniformConstant
%16 = OpVariable %_ptr_Private_6 Private
%gl_GlobalInvocationID = OpVariable %_ptr_Input_v3uint Input
%2 = OpFunction %void None %10
%17 = OpLabel
%18 = OpLoad %6 %4
OpStore %16 %18
%19 = OpImageTexelPointer %_ptr_Image_uint %16 %uint_0 %uint_0
%20 = OpAtomicIAdd %uint %19 %uint_1 %uint_0 %uint_1
OpReturn
OpFunctionEnd
)";
SetAssembleOptions(SPV_TEXT_TO_BINARY_OPTION_PRESERVE_NUMERIC_IDS);
SinglePassRunAndCheck<opt::AggressiveDCEPass>(text, text, true, true);
}
// TODO(greg-lunarg): Add tests to verify handling of these cases:
//
// Check that logical addressing required