Sink pointer instructions in merge return (#3569)

We cannot create an OpPhi for pointers, so we have to regenerate these
instructions instead.

Fixes #3030
Fixes #3266
This commit is contained in:
Steven Perron 2020-07-22 11:10:58 -04:00 committed by GitHub
parent cf7e922e70
commit dca2c86bc8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 231 additions and 4 deletions

View File

@ -299,9 +299,6 @@ void MergeReturnPass::CreatePhiNodesForInst(BasicBlock* merge_block,
// There is at least one values that needs to be replaced.
// First create the OpPhi instruction.
InstructionBuilder builder(
context(), &*merge_block->begin(),
IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
uint32_t undef_id = Type2Undef(inst.type_id());
std::vector<uint32_t> phi_operands;
const std::set<uint32_t>& new_edges = new_edges_[merge_block];
@ -318,7 +315,50 @@ void MergeReturnPass::CreatePhiNodesForInst(BasicBlock* merge_block,
phi_operands.push_back(pred_id);
}
Instruction* new_phi = builder.AddPhi(inst.type_id(), phi_operands);
Instruction* new_phi = nullptr;
// If the instruction is a pointer and variable pointers are not an option,
// then we have to regenerate the instruction instead of creating an OpPhi
// instruction. If not, the Spir-V will be invalid.
Instruction* inst_type = get_def_use_mgr()->GetDef(inst.type_id());
bool regenerateInstruction = false;
if (inst_type->opcode() == SpvOpTypePointer) {
if (!context()->get_feature_mgr()->HasCapability(
SpvCapabilityVariablePointers)) {
regenerateInstruction = true;
}
uint32_t storage_class = inst_type->GetSingleWordInOperand(0);
if (storage_class != SpvStorageClassWorkgroup &&
storage_class != SpvStorageClassStorageBuffer) {
regenerateInstruction = true;
}
}
if (regenerateInstruction) {
std::unique_ptr<Instruction> regen_inst(inst.Clone(context()));
uint32_t new_id = TakeNextId();
regen_inst->SetResultId(new_id);
Instruction* insert_pos = &*merge_block->begin();
while (insert_pos->opcode() == SpvOpPhi) {
insert_pos = insert_pos->NextNode();
}
new_phi = insert_pos->InsertBefore(std::move(regen_inst));
get_def_use_mgr()->AnalyzeInstDefUse(new_phi);
context()->set_instr_block(new_phi, merge_block);
new_phi->ForEachInId([dom_tree, merge_block, this](uint32_t* use_id) {
Instruction* use = get_def_use_mgr()->GetDef(*use_id);
BasicBlock* use_bb = context()->get_instr_block(use);
if (use_bb != nullptr && !dom_tree->Dominates(use_bb, merge_block)) {
CreatePhiNodesForInst(merge_block, *use);
}
});
} else {
InstructionBuilder builder(
context(), &*merge_block->begin(),
IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping);
new_phi = builder.AddPhi(inst.type_id(), phi_operands);
}
uint32_t result_of_phi = new_phi->result_id();
// Update all of the users to use the result of the new OpPhi.

View File

@ -2380,6 +2380,193 @@ TEST_F(MergeReturnPassTest, PhiWithTooManyEntries) {
SinglePassRunAndMatch<MergeReturnPass>(before, true);
}
TEST_F(MergeReturnPassTest, PointerUsedAfterLoop) {
// Make sure that a Phi instruction is not generated for an id whose type is a
// pointer. It needs to be regenerated.
const std::string before =
R"(
; CHECK: OpFunction %void
; CHECK: OpFunction %void
; CHECK-NEXT: [[param:%\w+]] = OpFunctionParameter %_ptr_Function_v2uint
; CHECK: OpLoopMerge [[merge_bb:%\w+]]
; CHECK: [[merge_bb]] = OpLabel
; CHECK-NEXT: [[ac:%\w+]] = OpAccessChain %_ptr_Function_uint [[param]] %uint_1
; CHECK: OpStore [[ac]] %uint_1
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %2 "main"
OpExecutionMode %2 OriginUpperLeft
OpSource ESSL 310
%void = OpTypeVoid
%4 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%v2uint = OpTypeVector %uint 2
%_ptr_Function_v2uint = OpTypePointer Function %v2uint
%8 = OpTypeFunction %void %_ptr_Function_v2uint
%uint_1 = OpConstant %uint 1
%bool = OpTypeBool
%_ptr_Function_uint = OpTypePointer Function %uint
%false = OpConstantFalse %bool
%2 = OpFunction %void None %4
%13 = OpLabel
%14 = OpVariable %_ptr_Function_v2uint Function
%15 = OpFunctionCall %void %16 %14
OpReturn
OpFunctionEnd
%16 = OpFunction %void None %8
%17 = OpFunctionParameter %_ptr_Function_v2uint
%18 = OpLabel
OpBranch %19
%19 = OpLabel
OpLoopMerge %20 %21 None
OpBranch %22
%22 = OpLabel
OpSelectionMerge %23 None
OpBranchConditional %false %24 %23
%24 = OpLabel
OpReturn
%23 = OpLabel
OpBranch %21
%21 = OpLabel
%25 = OpAccessChain %_ptr_Function_uint %17 %uint_1
OpBranchConditional %false %19 %20
%20 = OpLabel
OpStore %25 %uint_1
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<MergeReturnPass>(before, true);
}
TEST_F(MergeReturnPassTest, VariablePointerFunctionScope) {
// Make sure that a Phi instruction is not generated for an id whose type is a
// function scope pointer, even if the VariablePointers capability is
// available. It needs to be regenerated.
const std::string before =
R"(
; CHECK: OpFunction %void
; CHECK: OpFunction %void
; CHECK-NEXT: [[param:%\w+]] = OpFunctionParameter %_ptr_Function_v2uint
; CHECK: OpLoopMerge [[merge_bb:%\w+]]
; CHECK: [[merge_bb]] = OpLabel
; CHECK-NEXT: [[ac:%\w+]] = OpAccessChain %_ptr_Function_uint [[param]] %uint_1
; CHECK: OpStore [[ac]] %uint_1
OpCapability Shader
OpCapability VariablePointers
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %2 "main"
OpExecutionMode %2 OriginUpperLeft
OpSource ESSL 310
%void = OpTypeVoid
%4 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%v2uint = OpTypeVector %uint 2
%_ptr_Function_v2uint = OpTypePointer Function %v2uint
%8 = OpTypeFunction %void %_ptr_Function_v2uint
%uint_1 = OpConstant %uint 1
%bool = OpTypeBool
%_ptr_Function_uint = OpTypePointer Function %uint
%false = OpConstantFalse %bool
%2 = OpFunction %void None %4
%13 = OpLabel
%14 = OpVariable %_ptr_Function_v2uint Function
%15 = OpFunctionCall %void %16 %14
OpReturn
OpFunctionEnd
%16 = OpFunction %void None %8
%17 = OpFunctionParameter %_ptr_Function_v2uint
%18 = OpLabel
OpBranch %19
%19 = OpLabel
OpLoopMerge %20 %21 None
OpBranch %22
%22 = OpLabel
OpSelectionMerge %23 None
OpBranchConditional %false %24 %23
%24 = OpLabel
OpReturn
%23 = OpLabel
OpBranch %21
%21 = OpLabel
%25 = OpAccessChain %_ptr_Function_uint %17 %uint_1
OpBranchConditional %false %19 %20
%20 = OpLabel
OpStore %25 %uint_1
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<MergeReturnPass>(before, true);
}
TEST_F(MergeReturnPassTest, ChainedPointerUsedAfterLoop) {
// Make sure that a Phi instruction is not generated for an id whose type is a
// pointer. It needs to be regenerated.
const std::string before =
R"(
; CHECK: OpFunction %void
; CHECK: OpFunction %void
; CHECK-NEXT: [[param:%\w+]] = OpFunctionParameter %_ptr_Function_
; CHECK: OpLoopMerge [[merge_bb:%\w+]]
; CHECK: [[merge_bb]] = OpLabel
; CHECK-NEXT: [[ac1:%\w+]] = OpAccessChain %_ptr_Function_v2uint [[param]] %uint_1
; CHECK-NEXT: [[ac2:%\w+]] = OpAccessChain %_ptr_Function_uint [[ac1]] %uint_1
; CHECK: OpStore [[ac2]] %uint_1
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %2 "main"
OpExecutionMode %2 OriginUpperLeft
OpSource ESSL 310
%void = OpTypeVoid
%4 = OpTypeFunction %void
%uint = OpTypeInt 32 0
%uint_1 = OpConstant %uint 1
%uint_2 = OpConstant %uint 2
%v2uint = OpTypeVector %uint 2
%_arr_v2uint_uint_2 = OpTypeArray %v2uint %uint_2
%_ptr_Function_v2uint = OpTypePointer Function %v2uint
%_ptr_Function__arr_v2uint_uint_2 = OpTypePointer Function %_arr_v2uint_uint_2
%_ptr_Function_uint = OpTypePointer Function %uint
%13 = OpTypeFunction %void %_ptr_Function__arr_v2uint_uint_2
%bool = OpTypeBool
%false = OpConstantFalse %bool
%2 = OpFunction %void None %4
%16 = OpLabel
%17 = OpVariable %_ptr_Function__arr_v2uint_uint_2 Function
%18 = OpFunctionCall %void %19 %17
OpReturn
OpFunctionEnd
%19 = OpFunction %void None %13
%20 = OpFunctionParameter %_ptr_Function__arr_v2uint_uint_2
%21 = OpLabel
OpBranch %22
%22 = OpLabel
OpLoopMerge %23 %24 None
OpBranch %25
%25 = OpLabel
OpSelectionMerge %26 None
OpBranchConditional %false %27 %26
%27 = OpLabel
OpReturn
%26 = OpLabel
OpBranch %24
%24 = OpLabel
%28 = OpAccessChain %_ptr_Function_v2uint %20 %uint_1
%29 = OpAccessChain %_ptr_Function_uint %28 %uint_1
OpBranchConditional %false %22 %23
%23 = OpLabel
OpStore %29 %uint_1
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<MergeReturnPass>(before, true);
}
} // namespace
} // namespace opt
} // namespace spvtools