[spirv-reader][ir] Handle Phi in loop body.
It's possible to have a PHI in a loop body, that isn't in the loop
header itself. In this case, the incoming branches end up being the
header, and unreachable blocks. Handle by ignoring anything other then
the header block in this case.
Change-Id: Idb26c0ab81cdccb64c2d101ecd3bcb155c94e6ef
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/250956
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/tint/lang/spirv/reader/parser/parser.cc b/src/tint/lang/spirv/reader/parser/parser.cc
index 0cc178d..1f6a86c 100644
--- a/src/tint/lang/spirv/reader/parser/parser.cc
+++ b/src/tint/lang/spirv/reader/parser/parser.cc
@@ -2137,6 +2137,13 @@
void EmitPhiInLoop(spvtools::opt::Instruction& inst) {
auto* phi_containing_spirv_block = spirv_context_->get_instr_block(&inst);
+ auto iter = spirv_merge_id_to_header_id_.find(phi_containing_spirv_block->id());
+ // We're in the merge block for the loop
+ if (iter != spirv_merge_id_to_header_id_.end()) {
+ EmitPhiInLoopMerge(inst);
+ return;
+ }
+
// The OpPhi is in the loop header, which means it's the body of the loop. The OpPhi can
// receive values from the parent block, in which case we need to push them through the
// initializer, or from the continuing block, which we will not have emitted yet.
@@ -2145,10 +2152,19 @@
return;
}
- auto iter = spirv_merge_id_to_header_id_.find(phi_containing_spirv_block->id());
- // We're in the merge block for the loop
- if (iter != spirv_merge_id_to_header_id_.end()) {
- EmitPhiInLoopMerge(inst);
+ auto containing_loop_header_id = spirv_context_->GetStructuredCFGAnalysis()->ContainingLoop(
+ phi_containing_spirv_block->id());
+ auto* loop_header_block = spirv_context_->get_instr_block(containing_loop_header_id);
+ TINT_ASSERT(loop_header_block);
+
+ auto* loop_merge_inst = loop_header_block->GetLoopMergeInst();
+ auto continue_id = loop_merge_inst->GetSingleWordInOperand(1);
+
+ // If the spirv-id to block table contains an entry for the continue id then we must have
+ // started emitting the continue.
+ bool has_continue_emitted = spirv_id_to_block_.contains(continue_id);
+ if (!has_continue_emitted) {
+ EmitPhiInLoopBody(inst, loop_merge_inst);
return;
}
@@ -2297,6 +2313,36 @@
}
}
+ void EmitPhiInLoopBody(spvtools::opt::Instruction& inst,
+ spvtools::opt::Instruction* loop_merge_inst) {
+ auto continue_id = loop_merge_inst->GetSingleWordInOperand(1);
+
+ auto* loop_header_block = spirv_context_->get_instr_block(loop_merge_inst);
+
+ // The loop continue is a walk stop block. Since we're in the loop body itself we would have
+ // added the continue target to the stop blocks when processing the header loop merge.
+ auto* loop = StopWalkingAt(continue_id)->As<core::ir::Loop>();
+ TINT_ASSERT(loop);
+
+ // The only phi incoming block that makes sense on the body is from the loop header itself.
+ // Any other incoming branches must be from unreachable blocks, otherwise it would have had
+ // to have come from the loop header which is where the continue target will loop back too,
+ // not the body label directly.
+ std::optional<core::ir::Value*> value = std::nullopt;
+ for (uint32_t i = 0; i < inst.NumInOperands(); i += 2) {
+ auto value_id = inst.GetSingleWordInOperand(i);
+ auto blk_id = inst.GetSingleWordInOperand(i + 1);
+
+ if (blk_id != loop_header_block->id()) {
+ continue;
+ }
+
+ value = Value(value_id);
+ }
+ TINT_ASSERT(value.has_value());
+ AddValue(inst.result_id(), value.value());
+ }
+
void EmitPhiInLoopHeader(spvtools::opt::Instruction& inst,
spvtools::opt::BasicBlock* loop_header) {
// Incoming branches should be from the parent block into the loop, and from the loop
diff --git a/src/tint/lang/spirv/reader/parser/phi_test.cc b/src/tint/lang/spirv/reader/parser/phi_test.cc
index d112385..1e07b99 100644
--- a/src/tint/lang/spirv/reader/parser/phi_test.cc
+++ b/src/tint/lang/spirv/reader/parser/phi_test.cc
@@ -1646,5 +1646,54 @@
)");
}
+TEST_F(SpirvParserTest, Phi_InLoopBody) {
+ EXPECT_IR(R"(
+ OpCapability Shader
+ OpMemoryModel Logical GLSL450
+ OpEntryPoint Fragment %main "main"
+ OpExecutionMode %main OriginUpperLeft
+ OpName %main "main"
+ %void = OpTypeVoid
+ %7 = OpTypeFunction %void
+ %bool = OpTypeBool
+ %float = OpTypeFloat 32
+ %float_1 = OpConstant %float 1
+ %float_0 = OpConstant %float 0
+ %main = OpFunction %void None %7
+ %29 = OpLabel
+ OpBranch %31
+ %31 = OpLabel
+ OpLoopMerge %35 %34 None
+ OpBranch %36
+ %36 = OpLabel
+ %33 = OpPhi %float %float_1 %31 %float_0 %37
+ %99 = OpFAdd %float %33 %33
+ OpReturn
+ %37 = OpLabel
+ OpBranch %36
+ %34 = OpLabel
+ OpBranch %31
+ %35 = OpLabel
+ OpUnreachable
+ OpFunctionEnd
+)",
+ R"(
+%main = @fragment func():void {
+ $B1: {
+ loop [b: $B2, c: $B3] { # loop_1
+ $B2: { # body
+ %2:f32 = add 1.0f, 1.0f
+ ret
+ }
+ $B3: { # continuing
+ next_iteration # -> $B2
+ }
+ }
+ unreachable
+ }
+}
+)");
+}
+
} // namespace
} // namespace tint::spirv::reader
