[ir] Fix issues with InboundSiblingBranches.
In each of the `continue`, break-if` and `next-iteration` instructions
we have to remove the inbound sibling branches on `Destroy`.
The `continue` instruction when you did a `SetLoop` was incorrectly
using the `Body` branch of the loop instead of the `Continuing` branch.
Change-Id: I4efe166180941154502fcfa75a51253d5858085f
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/244354
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/tint/lang/core/ir/break_if.cc b/src/tint/lang/core/ir/break_if.cc
index 0191868..9ab1be7 100644
--- a/src/tint/lang/core/ir/break_if.cc
+++ b/src/tint/lang/core/ir/break_if.cc
@@ -62,6 +62,13 @@
BreakIf::~BreakIf() = default;
+void BreakIf::Destroy() {
+ if (loop_) {
+ loop_->Body()->RemoveInboundSiblingBranch(this);
+ }
+ Instruction::Destroy();
+}
+
BreakIf* BreakIf::Clone(CloneContext& ctx) {
auto* loop = ctx.Remap(loop_);
auto* cond = ctx.Remap(Condition());
diff --git a/src/tint/lang/core/ir/break_if.h b/src/tint/lang/core/ir/break_if.h
index 132ea20..a5be066 100644
--- a/src/tint/lang/core/ir/break_if.h
+++ b/src/tint/lang/core/ir/break_if.h
@@ -71,6 +71,9 @@
~BreakIf() override;
+ /// @copydoc Instruction::Destroy()
+ void Destroy() override;
+
/// @copydoc Instruction::Clone()
BreakIf* Clone(CloneContext& ctx) override;
diff --git a/src/tint/lang/core/ir/break_if_test.cc b/src/tint/lang/core/ir/break_if_test.cc
index b1c1cc7..be3e5a7 100644
--- a/src/tint/lang/core/ir/break_if_test.cc
+++ b/src/tint/lang/core/ir/break_if_test.cc
@@ -129,5 +129,18 @@
EXPECT_THAT(loop2->Exits(), testing::ElementsAre(brk));
}
+TEST_F(IR_BreakIfTest, Destroy) {
+ auto* loop = b.Loop();
+ auto* cond = b.Constant(true);
+ auto* inst = b.BreakIf(loop, cond);
+
+ ASSERT_EQ(1u, loop->Body()->InboundSiblingBranches().Length());
+ EXPECT_EQ(inst, loop->Body()->InboundSiblingBranches()[0]);
+
+ inst->Destroy();
+
+ EXPECT_EQ(0u, loop->Body()->InboundSiblingBranches().Length());
+}
+
} // namespace
} // namespace tint::core::ir
diff --git a/src/tint/lang/core/ir/continue.cc b/src/tint/lang/core/ir/continue.cc
index c36abe1..ae32299 100644
--- a/src/tint/lang/core/ir/continue.cc
+++ b/src/tint/lang/core/ir/continue.cc
@@ -54,6 +54,13 @@
Continue::~Continue() = default;
+void Continue::Destroy() {
+ if (loop_) {
+ loop_->Continuing()->RemoveInboundSiblingBranch(this);
+ }
+ Instruction::Destroy();
+}
+
Continue* Continue::Clone(CloneContext& ctx) {
auto* loop = ctx.Remap(Loop());
auto args = ctx.Remap<Continue::kDefaultNumOperands>(Args());
@@ -63,11 +70,11 @@
void Continue::SetLoop(ir::Loop* loop) {
if (loop_ && loop_->Body()) {
- loop_->Body()->RemoveInboundSiblingBranch(this);
+ loop_->Continuing()->RemoveInboundSiblingBranch(this);
}
loop_ = loop;
if (loop) {
- loop->Body()->AddInboundSiblingBranch(this);
+ loop->Continuing()->AddInboundSiblingBranch(this);
}
}
diff --git a/src/tint/lang/core/ir/continue.h b/src/tint/lang/core/ir/continue.h
index dab2080..a5eecd7 100644
--- a/src/tint/lang/core/ir/continue.h
+++ b/src/tint/lang/core/ir/continue.h
@@ -61,6 +61,9 @@
/// @copydoc Instruction::Clone()
Continue* Clone(CloneContext& ctx) override;
+ /// @copydoc Instruction::Destroy()
+ void Destroy() override;
+
/// @returns the loop owning the continue block
ir::Loop* Loop() { return loop_; }
diff --git a/src/tint/lang/core/ir/continue_test.cc b/src/tint/lang/core/ir/continue_test.cc
index 85a5e7e..a9f4518 100644
--- a/src/tint/lang/core/ir/continue_test.cc
+++ b/src/tint/lang/core/ir/continue_test.cc
@@ -101,5 +101,17 @@
EXPECT_EQ(2_u, val1->As<core::constant::Scalar<u32>>()->ValueAs<u32>());
}
+TEST_F(IR_ContinueTest, Destroy) {
+ auto* loop = b.Loop();
+ auto* inst = b.Continue(loop);
+
+ ASSERT_EQ(1u, loop->Continuing()->InboundSiblingBranches().Length());
+ EXPECT_EQ(inst, loop->Continuing()->InboundSiblingBranches()[0]);
+
+ inst->Destroy();
+
+ EXPECT_EQ(0u, loop->Continuing()->InboundSiblingBranches().Length());
+}
+
} // namespace
} // namespace tint::core::ir
diff --git a/src/tint/lang/core/ir/next_iteration.cc b/src/tint/lang/core/ir/next_iteration.cc
index bba8de8..425f505 100644
--- a/src/tint/lang/core/ir/next_iteration.cc
+++ b/src/tint/lang/core/ir/next_iteration.cc
@@ -54,6 +54,13 @@
NextIteration::~NextIteration() = default;
+void NextIteration::Destroy() {
+ if (loop_) {
+ loop_->Body()->RemoveInboundSiblingBranch(this);
+ }
+ Instruction::Destroy();
+}
+
NextIteration* NextIteration::Clone(CloneContext& ctx) {
auto* new_loop = ctx.Clone(loop_);
auto args = ctx.Remap<NextIteration::kDefaultNumOperands>(Args());
diff --git a/src/tint/lang/core/ir/next_iteration.h b/src/tint/lang/core/ir/next_iteration.h
index 825f8c0..13e1723 100644
--- a/src/tint/lang/core/ir/next_iteration.h
+++ b/src/tint/lang/core/ir/next_iteration.h
@@ -59,6 +59,9 @@
~NextIteration() override;
+ /// @copydoc Instruction::Destroy()
+ void Destroy() override;
+
/// @copydoc Instruction::Clone()
NextIteration* Clone(CloneContext& ctx) override;
diff --git a/src/tint/lang/core/ir/next_iteration_test.cc b/src/tint/lang/core/ir/next_iteration_test.cc
index 6a7a6e3..b2cc9185 100644
--- a/src/tint/lang/core/ir/next_iteration_test.cc
+++ b/src/tint/lang/core/ir/next_iteration_test.cc
@@ -84,5 +84,17 @@
EXPECT_EQ(new_loop, new_inst->Loop());
}
+TEST_F(IR_NextIterationTest, Destroy) {
+ auto* loop = b.Loop();
+ auto* inst = b.NextIteration(loop);
+
+ ASSERT_EQ(1u, loop->Body()->InboundSiblingBranches().Length());
+ EXPECT_EQ(inst, loop->Body()->InboundSiblingBranches()[0]);
+
+ inst->Destroy();
+
+ EXPECT_EQ(0u, loop->Body()->InboundSiblingBranches().Length());
+}
+
} // namespace
} // namespace tint::core::ir
