Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle more cases in dead member elim #3289

Merged
merged 2 commits into from
Apr 9, 2020
Merged

Handle more cases in dead member elim #3289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64e6817
Handle more cases in dead member elim
s-perron Apr 9, 2020
a09641e
Fixes based on code review.
s-perron Apr 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 72 additions & 18 deletions source/opt/eliminate_dead_members_pass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

namespace {
const uint32_t kRemovedMember = 0xFFFFFFFF;
const uint32_t kSpecConstOpOpcodeIdx = 0;
}

namespace spvtools {
Expand All @@ -40,7 +41,22 @@ void EliminateDeadMembersPass::FindLiveMembers() {
// we have to mark them as fully used just to be safe.
for (auto& inst : get_module()->types_values()) {
if (inst.opcode() == SpvOpSpecConstantOp) {
MarkTypeAsFullyUsed(inst.type_id());
switch (inst.GetSingleWordInOperand(kSpecConstOpOpcodeIdx)) {
case SpvOpCompositeExtract:
MarkMembersAsLiveForExtract(&inst);
break;
case SpvOpCompositeInsert:
// Nothing specific to do.
break;
case SpvOpAccessChain:
case SpvOpInBoundsAccessChain:
case SpvOpPtrAccessChain:
case SpvOpInBoundsPtrAccessChain:
assert(false && "Not implemented yet.");
break;
default:
break;
}
} else if (inst.opcode() == SpvOpVariable) {
switch (inst.GetSingleWordInOperand(0)) {
case SpvStorageClassInput:
Expand Down Expand Up @@ -153,13 +169,17 @@ void EliminateDeadMembersPass::MarkMembersAsLiveForCopyMemory(

void EliminateDeadMembersPass::MarkMembersAsLiveForExtract(
const Instruction* inst) {
assert(inst->opcode() == SpvOpCompositeExtract);
assert(inst->opcode() == SpvOpCompositeExtract ||
(inst->opcode() == SpvOpSpecConstantOp &&
inst->GetSingleWordInOperand(kSpecConstOpOpcodeIdx) ==
SpvOpCompositeExtract));

uint32_t composite_id = inst->GetSingleWordInOperand(0);
uint32_t first_operand = (inst->opcode() == SpvOpSpecConstantOp ? 1 : 0);
uint32_t composite_id = inst->GetSingleWordInOperand(first_operand);
Instruction* composite_inst = get_def_use_mgr()->GetDef(composite_id);
uint32_t type_id = composite_inst->type_id();

for (uint32_t i = 1; i < inst->NumInOperands(); ++i) {
for (uint32_t i = first_operand + 1; i < inst->NumInOperands(); ++i) {
Instruction* type_inst = get_def_use_mgr()->GetDef(type_id);
uint32_t member_idx = inst->GetSingleWordInOperand(i);
switch (type_inst->opcode()) {
Expand Down Expand Up @@ -295,10 +315,22 @@ bool EliminateDeadMembersPass::RemoveDeadMembers() {
modified |= UpdateOpArrayLength(inst);
break;
case SpvOpSpecConstantOp:
assert(false && "Not yet implemented.");
// with OpCompositeExtract, OpCompositeInsert
// For kernels: OpAccessChain, OpInBoundsAccessChain, OpPtrAccessChain,
// OpInBoundsPtrAccessChain
switch (inst->GetSingleWordInOperand(kSpecConstOpOpcodeIdx)) {
case SpvOpCompositeExtract:
modified |= UpdateCompsiteExtract(inst);
break;
case SpvOpCompositeInsert:
modified |= UpdateCompositeInsert(inst);
break;
case SpvOpAccessChain:
case SpvOpInBoundsAccessChain:
case SpvOpPtrAccessChain:
case SpvOpInBoundsPtrAccessChain:
assert(false && "Not implemented yet.");
break;
default:
break;
}
break;
default:
break;
Expand Down Expand Up @@ -393,7 +425,8 @@ bool EliminateDeadMembersPass::UpdateOpGroupMemberDecorate(Instruction* inst) {
}

bool EliminateDeadMembersPass::UpdateConstantComposite(Instruction* inst) {
assert(inst->opcode() == SpvOpConstantComposite ||
assert(inst->opcode() == SpvOpSpecConstantComposite ||
inst->opcode() == SpvOpConstantComposite ||
inst->opcode() == SpvOpCompositeConstruct);
uint32_t type_id = inst->type_id();

Expand Down Expand Up @@ -506,14 +539,25 @@ uint32_t EliminateDeadMembersPass::GetNewMemberIndex(uint32_t type_id,
}

bool EliminateDeadMembersPass::UpdateCompsiteExtract(Instruction* inst) {
uint32_t object_id = inst->GetSingleWordInOperand(0);
assert(inst->opcode() == SpvOpCompositeExtract ||
(inst->opcode() == SpvOpSpecConstantOp &&
inst->GetSingleWordInOperand(kSpecConstOpOpcodeIdx) ==
SpvOpCompositeExtract));

uint32_t first_operand = 0;
if (inst->opcode() == SpvOpSpecConstantOp) {
first_operand = 1;
}
uint32_t object_id = inst->GetSingleWordInOperand(first_operand);
Instruction* object_inst = get_def_use_mgr()->GetDef(object_id);
uint32_t type_id = object_inst->type_id();

Instruction::OperandList new_operands;
bool modified = false;
new_operands.emplace_back(inst->GetInOperand(0));
for (uint32_t i = 1; i < inst->NumInOperands(); ++i) {
for (uint32_t i = 0; i < first_operand + 1; i++) {
new_operands.emplace_back(inst->GetInOperand(i));
}
for (uint32_t i = first_operand + 1; i < inst->NumInOperands(); ++i) {
uint32_t member_idx = inst->GetSingleWordInOperand(i);
uint32_t new_member_idx = GetNewMemberIndex(type_id, member_idx);
assert(new_member_idx != kRemovedMember);
Expand All @@ -526,8 +570,6 @@ bool EliminateDeadMembersPass::UpdateCompsiteExtract(Instruction* inst) {
Instruction* type_inst = get_def_use_mgr()->GetDef(type_id);
switch (type_inst->opcode()) {
case SpvOpTypeStruct:
assert(i != 1 || (inst->opcode() != SpvOpPtrAccessChain &&
inst->opcode() != SpvOpInBoundsPtrAccessChain));
// The type will have already been rewriten, so use the new member
// index.
type_id = type_inst->GetSingleWordInOperand(new_member_idx);
Expand All @@ -552,15 +594,27 @@ bool EliminateDeadMembersPass::UpdateCompsiteExtract(Instruction* inst) {
}

bool EliminateDeadMembersPass::UpdateCompositeInsert(Instruction* inst) {
uint32_t composite_id = inst->GetSingleWordInOperand(1);
assert(inst->opcode() == SpvOpCompositeInsert ||
(inst->opcode() == SpvOpSpecConstantOp &&
inst->GetSingleWordInOperand(kSpecConstOpOpcodeIdx) ==
SpvOpCompositeInsert));

uint32_t first_operand = 0;
if (inst->opcode() == SpvOpSpecConstantOp) {
first_operand = 1;
}

uint32_t composite_id = inst->GetSingleWordInOperand(first_operand + 1);
Instruction* composite_inst = get_def_use_mgr()->GetDef(composite_id);
uint32_t type_id = composite_inst->type_id();

Instruction::OperandList new_operands;
bool modified = false;
new_operands.emplace_back(inst->GetInOperand(0));
new_operands.emplace_back(inst->GetInOperand(1));
for (uint32_t i = 2; i < inst->NumInOperands(); ++i) {

for (uint32_t i = 0; i < first_operand + 2; ++i) {
new_operands.emplace_back(inst->GetInOperand(i));
}
for (uint32_t i = first_operand + 2; i < inst->NumInOperands(); ++i) {
uint32_t member_idx = inst->GetSingleWordInOperand(i);
uint32_t new_member_idx = GetNewMemberIndex(type_id, member_idx);
if (new_member_idx == kRemovedMember) {
Expand Down
99 changes: 99 additions & 0 deletions test/opt/eliminate_dead_member_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1085,4 +1085,103 @@ TEST_F(EliminateDeadMemberTest, DontChangeOutputStructs) {
EXPECT_EQ(opt::Pass::Status::SuccessWithoutChange, std::get<1>(result));
}

TEST_F(EliminateDeadMemberTest, UpdateSpecConstOpExtract) {
// Test that an extract in an OpSpecConstantOp is correctly updated.
const std::string text = R"(
; CHECK: OpName
; CHECK-NEXT: OpMemberName %type__Globals 0 "y"
; CHECK-NOT: OpMemberName
; CHECK: OpDecorate [[spec_const:%\w+]] SpecId 1
; CHECK: OpMemberDecorate %type__Globals 0 Offset 4
; CHECK: %type__Globals = OpTypeStruct %uint
; CHECK: [[struct:%\w+]] = OpSpecConstantComposite %type__Globals [[spec_const]]
; CHECK: OpSpecConstantOp %uint CompositeExtract [[struct]] 0
OpCapability Shader
OpCapability Addresses
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main"
OpSource HLSL 600
OpName %type__Globals "type.$Globals"
OpMemberName %type__Globals 0 "x"
OpMemberName %type__Globals 1 "y"
OpMemberName %type__Globals 2 "z"
OpName %main "main"
OpDecorate %c_0 SpecId 0
OpDecorate %c_1 SpecId 1
OpDecorate %c_2 SpecId 2
OpMemberDecorate %type__Globals 0 Offset 0
OpMemberDecorate %type__Globals 1 Offset 4
OpMemberDecorate %type__Globals 2 Offset 16
%uint = OpTypeInt 32 0
%c_0 = OpSpecConstant %uint 0
%c_1 = OpSpecConstant %uint 1
%c_2 = OpSpecConstant %uint 2
%uint_0 = OpConstant %uint 0
%uint_1 = OpConstant %uint 1
%uint_2 = OpConstant %uint 2
%uint_3 = OpConstant %uint 3
%type__Globals = OpTypeStruct %uint %uint %uint
%spec_const_global = OpSpecConstantComposite %type__Globals %c_0 %c_1 %c_2
%extract = OpSpecConstantOp %uint CompositeExtract %spec_const_global 1
%void = OpTypeVoid
%14 = OpTypeFunction %void
%main = OpFunction %void None %14
%16 = OpLabel
OpReturn
OpFunctionEnd
)";

SinglePassRunAndMatch<opt::EliminateDeadMembersPass>(text, true);
}

TEST_F(EliminateDeadMemberTest, UpdateSpecConstOpInsert) {
// Test that an insert in an OpSpecConstantOp is correctly updated.
const std::string text = R"(
; CHECK: OpName
; CHECK-NEXT: OpMemberName %type__Globals 0 "y"
; CHECK-NOT: OpMemberName
; CHECK: OpDecorate [[spec_const:%\w+]] SpecId 1
; CHECK: OpMemberDecorate %type__Globals 0 Offset 4
; CHECK: %type__Globals = OpTypeStruct %uint
; CHECK: [[struct:%\w+]] = OpSpecConstantComposite %type__Globals [[spec_const]]
; CHECK: OpSpecConstantOp %type__Globals CompositeInsert %uint_3 [[struct]] 0
OpCapability Shader
OpCapability Addresses
OpMemoryModel Logical GLSL450
OpEntryPoint Vertex %main "main"
OpSource HLSL 600
OpName %type__Globals "type.$Globals"
OpMemberName %type__Globals 0 "x"
OpMemberName %type__Globals 1 "y"
OpMemberName %type__Globals 2 "z"
OpName %main "main"
OpDecorate %c_0 SpecId 0
OpDecorate %c_1 SpecId 1
OpDecorate %c_2 SpecId 2
OpMemberDecorate %type__Globals 0 Offset 0
OpMemberDecorate %type__Globals 1 Offset 4
OpMemberDecorate %type__Globals 2 Offset 16
%uint = OpTypeInt 32 0
%c_0 = OpSpecConstant %uint 0
%c_1 = OpSpecConstant %uint 1
%c_2 = OpSpecConstant %uint 2
%uint_0 = OpConstant %uint 0
%uint_1 = OpConstant %uint 1
%uint_2 = OpConstant %uint 2
%uint_3 = OpConstant %uint 3
%type__Globals = OpTypeStruct %uint %uint %uint
%spec_const_global = OpSpecConstantComposite %type__Globals %c_0 %c_1 %c_2
%insert = OpSpecConstantOp %type__Globals CompositeInsert %uint_3 %spec_const_global 1
%extract = OpSpecConstantOp %uint CompositeExtract %insert 1
%void = OpTypeVoid
%14 = OpTypeFunction %void
%main = OpFunction %void None %14
%16 = OpLabel
OpReturn
OpFunctionEnd
)";

SinglePassRunAndMatch<opt::EliminateDeadMembersPass>(text, true);
}

} // namespace