Skip to content

Commit a1a1073

Browse files
committed
[SandboxVec][DAG] Remove all matching nodes from bundle
This fixes a bug where `SchedBundle::eraseFromBundle()` would not erase all matching nodes but just the first one.
1 parent f83ef28 commit a1a1073

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,9 @@ class SchedBundle {
112112
ContainerTy Nodes;
113113

114114
/// Called by the DGNode destructor to avoid accessing freed memory.
115-
void eraseFromBundle(DGNode *N) { Nodes.erase(find(Nodes, N)); }
115+
void eraseFromBundle(DGNode *N) {
116+
Nodes.erase(std::remove(Nodes.begin(), Nodes.end(), N), Nodes.end());
117+
}
116118
friend DGNode::~DGNode(); // For eraseFromBundle().
117119

118120
public:

llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,47 @@ define void @foo(ptr %ptr, i8 %v0, i8 %v1) {
145145
testing::ElementsAre(SN0, SN1));
146146
}
147147

148+
// Check that when we erase a DAG node its SchedBundle gets updated.
149+
TEST_F(SchedulerTest, SchedBundleEraseDGNode) {
150+
parseIR(C, R"IR(
151+
define void @foo(ptr %ptr, i8 %v0, i8 %v1, i8 %v2, i8 %v3) {
152+
store i8 %v0, ptr %ptr
153+
store i8 %v1, ptr %ptr
154+
store i8 %v2, ptr %ptr
155+
store i8 %v3, ptr %ptr
156+
ret void
157+
}
158+
)IR");
159+
llvm::Function *LLVMF = &*M->getFunction("foo");
160+
sandboxir::Context Ctx(C);
161+
auto *F = Ctx.createFunction(LLVMF);
162+
auto *BB = &*F->begin();
163+
auto It = BB->begin();
164+
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
165+
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
166+
auto *S2 = cast<sandboxir::StoreInst>(&*It++);
167+
auto *S3 = cast<sandboxir::StoreInst>(&*It++);
168+
169+
sandboxir::DependencyGraph DAG(getAA(*LLVMF), Ctx);
170+
DAG.extend({&*BB->begin(), BB->getTerminator()});
171+
auto *SN0 = DAG.getNode(S0);
172+
auto *SN1 = DAG.getNode(S1);
173+
auto *SN2 = DAG.getNode(S2);
174+
auto *SN3 = DAG.getNode(S3);
175+
{
176+
// Check the common case, when the bundle contains unique nodes.
177+
sandboxir::SchedBundle Bndl({SN0, SN1});
178+
S0->eraseFromParent();
179+
EXPECT_THAT(Bndl, testing::ElementsAre(SN1));
180+
}
181+
{
182+
// Check corner case when the node appears more than once.
183+
sandboxir::SchedBundle Bndl({SN2, SN3, SN2});
184+
S2->eraseFromParent();
185+
EXPECT_THAT(Bndl, testing::ElementsAre(SN3));
186+
}
187+
}
188+
148189
TEST_F(SchedulerTest, Basic) {
149190
parseIR(C, R"IR(
150191
define void @foo(ptr %ptr, i8 %v0, i8 %v1) {

0 commit comments

Comments
 (0)