AuroraMiddleware/SPIRV-Tools
1
0
Fork 0
mirror of https://github.com/KhronosGroup/SPIRV-Tools synced 2024-12-27 02:10:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix infinite simulation cycles in SSA propagator.

Browse Source 
This fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/1159.  I
had missed a nuance in the original algorithm.  When simulating Phi
instructions, the SSA edges out of a Phi instruction should never be
added to the list of edges to simulate.

Phi instructions can be in SSA def-use cycles with other Phi
instructions.  This was causing the propagator to fall into an infinite
loop when the same def-use edge kept being added to the queue.

The original algorithm in the paper specifically separates the visit of
a Phi instruction vs the visit of a regular instruction.  This fix makes
the implementation match the original algorithm.
This commit is contained in:
Diego Novillo 2018-01-05 09:34:18 -05:00
parent 120ddffb41
commit 716718a5e9
3 changed files with 82 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
source/opt/propagator.cpp
View File

@ -36,17 +36,27 @@ void SSAPropagator::AddControlEdge(const Edge& edge) {
blocks_.push(dest_bb);
}
void SSAPropagator::AddSSAEdges(uint32_t id) {
get_def_use_mgr()->ForEachUser(id, [this](ir::Instruction* instr) {
// If the basic block for |instr| has not been simulated yet, do nothing.
if (!BlockHasBeenSimulated(ctx_->get_instr_block(instr))) {
return;
}
void SSAPropagator::AddSSAEdges(ir::Instruction* instr) {
if (instr->result_id() == 0 || instr->opcode() == SpvOpPhi) {
// Ignore instructions that produce no result and Phi instructions. The SSA
// edges out of Phi instructions are never added. Phi instructions can
// produce cycles in the def-use web and they are always simulated when a
// block is visited.
return;
}
if (ShouldSimulateAgain(instr)) {
ssa_edge_uses_.push(instr);
}
});
get_def_use_mgr()->ForEachUser(
instr->result_id(), [this](ir::Instruction* use_instr) {
// If the basic block for |use_instr| has not been simulated yet, do
// nothing.
if (!BlockHasBeenSimulated(ctx_->get_instr_block(use_instr))) {
return;
}
if (ShouldSimulateAgain(use_instr)) {
ssa_edge_uses_.push(use_instr);
}
});
}
bool SSAPropagator::IsPhiArgExecutable(ir::Instruction* phi, uint32_t i) const {
@ -74,9 +84,7 @@ bool SSAPropagator::Simulate(ir::Instruction* instr) {
// The statement produces a varying result, add it to the list of statements
// not to simulate anymore and add its SSA def-use edges for simulation.
DontSimulateAgain(instr);
if (instr->result_id() > 0) {
AddSSAEdges(instr->result_id());
}
AddSSAEdges(instr);
// If |instr| is a block terminator, add all the control edges out of its
// block.
@ -88,11 +96,8 @@ bool SSAPropagator::Simulate(ir::Instruction* instr) {
}
return false;
} else if (status == kInteresting) {
// If the instruction produced a new interesting value, add the SSA edge
// for its result ID.
if (instr->result_id() > 0) {
AddSSAEdges(instr->result_id());
}
// Add the SSA edges coming out of this instruction.
AddSSAEdges(instr);
// If there are multiple outgoing control flow edges and we know which one
// will be taken, add the destination block to the CFG work list.

14
source/opt/propagator.h
View File

@ -248,12 +248,18 @@ class SSAPropagator {
return ctx_->get_def_use_mgr();
}
// If the CFG edge |e| has not been executed, add its destination block to the
// work list.
// If the CFG edge |e| has not been executed, this function adds |e|'s
// destination block to the work list.
void AddControlEdge(const Edge& e);
// Add all the instructions that use |id| to the SSA edges work list.
void AddSSAEdges(uint32_t id);
// Adds all the instructions that use the result of |instr| to the SSA edges
// work list. If |instr| produces no result id, this does nothing. This also
// does nothing if |instr| is a Phi instruction. Phi instructions are treated
// specially because (a) they can be in def-use cycles with other
// instructions, and (b) they are always executed when a basic block is
// simulated (see the description of the Sparse Conditional Constant algorithm
// in the original paper).
void AddSSAEdges(ir::Instruction* instr);
// IR context to use.
ir::IRContext* ctx_;

49
test/opt/ccp_test.cpp
View File

@ -415,6 +415,55 @@ TEST_F(CCPTest, HandleAbortInstructions) {
SinglePassRunAndMatch<opt::CCPPass>(spv_asm, true);
}
TEST_F(CCPTest, SSAWebCycles) {
// Test reduced from https://github.com/KhronosGroup/SPIRV-Tools/issues/1159
// When there is a cycle in the SSA def-use web, the propagator was getting
// into an infinite loop. SSA edges for Phi instructions should not be
// added to the edges to simulate.
const std::string spv_asm = R"(
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main"
OpExecutionMode %main OriginUpperLeft
OpSource GLSL 450
OpName %main "main"
%void = OpTypeVoid
%3 = OpTypeFunction %void
%int = OpTypeInt 32 1
%_ptr_Function_int = OpTypePointer Function %int
%int_0 = OpConstant %int 0
%int_4 = OpConstant %int 4
%bool = OpTypeBool
%int_1 = OpConstant %int 1
%_ptr_Output_int = OpTypePointer Output %int
%main = OpFunction %void None %3
%5 = OpLabel
OpBranch %11
%11 = OpLabel
%29 = OpPhi %int %int_0 %5 %22 %14
%30 = OpPhi %int %int_0 %5 %25 %14
OpLoopMerge %13 %14 None
OpBranch %15
%15 = OpLabel
%19 = OpSLessThan %bool %30 %int_4
; CHECK: OpBranchConditional %true {{%\d+}} {{%\d+}}
OpBranchConditional %19 %12 %13
%12 = OpLabel
; CHECK: OpIAdd %int %int_0 %int_0
%22 = OpIAdd %int %29 %30
OpBranch %14
%14 = OpLabel
; CHECK: OpPhi %int %int_0 {{%\d+}} %int_0 {{%\d+}}
%25 = OpPhi %int %int_0 %5 %30 %14
OpBranch %11
%13 = OpLabel
OpReturn
OpFunctionEnd
)";
SinglePassRunAndMatch<opt::CCPPass>(spv_asm, true);
}
#endif
} // namespace