Skip to content

Commit

Permalink
[SandboxVec][DAG] Fix DAG when old interval is mem free (llvm#126983)
Browse files Browse the repository at this point in the history
This patch fixes a bug in `DependencyGraph::extend()` when the old
interval contains no memory instructions. When this is the case we
should do a full dependency scan of the new interval.
  • Loading branch information
vporpo authored and sivan-shani committed Feb 21, 2025
1 parent b537d3a commit 2c9fa7e
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ MemDGNodeIntervalBuilder::getBotMemDGNode(const Interval<Instruction> &Intvl,
Interval<MemDGNode>
MemDGNodeIntervalBuilder::make(const Interval<Instruction> &Instrs,
DependencyGraph &DAG) {
if (Instrs.empty())
return {};
auto *TopMemN = getTopMemDGNode(Instrs, DAG);
// If we couldn't find a mem node in range TopN - BotN then it's empty.
if (TopMemN == nullptr)
Expand Down Expand Up @@ -529,8 +531,8 @@ Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
}
}
};
if (DAGInterval.empty()) {
assert(NewInterval == InstrsInterval && "Expected empty DAGInterval!");
auto MemDAGInterval = MemDGNodeIntervalBuilder::make(DAGInterval, *this);
if (MemDAGInterval.empty()) {
FullScan(NewInterval);
}
// 2. The new section is below the old section.
Expand All @@ -550,8 +552,7 @@ Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
// range including both NewInterval and DAGInterval until DstN, for each DstN.
else if (DAGInterval.bottom()->comesBefore(NewInterval.top())) {
auto DstRange = MemDGNodeIntervalBuilder::make(NewInterval, *this);
auto SrcRangeFull = MemDGNodeIntervalBuilder::make(
DAGInterval.getUnionInterval(NewInterval), *this);
auto SrcRangeFull = MemDAGInterval.getUnionInterval(DstRange);
for (MemDGNode &DstN : DstRange) {
auto SrcRange =
Interval<MemDGNode>(SrcRangeFull.top(), DstN.getPrevNode());
Expand Down Expand Up @@ -589,7 +590,7 @@ Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
// When scanning for deps with destination in DAGInterval we need to
// consider sources from the NewInterval only, because all intra-DAGInterval
// dependencies have already been created.
auto DstRangeOld = MemDGNodeIntervalBuilder::make(DAGInterval, *this);
auto DstRangeOld = MemDAGInterval;
auto SrcRange = MemDGNodeIntervalBuilder::make(NewInterval, *this);
for (MemDGNode &DstN : DstRangeOld)
scanAndAddDeps(DstN, SrcRange);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1013,3 +1013,42 @@ define void @foo(ptr %ptr, i8 %v1, i8 %v2, i8 %arg) {
EXPECT_EQ(S2N->getNextNode(), S1N);
EXPECT_EQ(S1N->getNextNode(), nullptr);
}

// Extending an "Old" interval with no mem instructions.
TEST_F(DependencyGraphTest, ExtendDAGWithNoMem) {
parseIR(C, R"IR(
define void @foo(ptr %ptr, i8 %v, i8 %v0, i8 %v1, i8 %v2, i8 %v3) {
store i8 %v0, ptr %ptr
store i8 %v1, ptr %ptr
%zext1 = zext i8 %v to i32
%zext2 = zext i8 %v to i32
store i8 %v2, ptr %ptr
store i8 %v3, ptr %ptr
ret void
}
)IR");
llvm::Function *LLVMF = &*M->getFunction("foo");
sandboxir::Context Ctx(C);
auto *F = Ctx.createFunction(LLVMF);
auto *BB = &*F->begin();
auto It = BB->begin();
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
auto *Z1 = cast<sandboxir::CastInst>(&*It++);
auto *Z2 = cast<sandboxir::CastInst>(&*It++);
auto *S2 = cast<sandboxir::StoreInst>(&*It++);
auto *S3 = cast<sandboxir::StoreInst>(&*It++);

sandboxir::DependencyGraph DAG(getAA(*LLVMF), Ctx);
// Create a non-empty DAG that contains no memory instructions.
DAG.extend({Z1, Z2});
// Now extend it downwards.
DAG.extend({S2, S3});
EXPECT_TRUE(memDependency(DAG.getNode(S2), DAG.getNode(S3)));

// Same but upwards.
DAG.clear();
DAG.extend({Z1, Z2});
DAG.extend({S0, S1});
EXPECT_TRUE(memDependency(DAG.getNode(S0), DAG.getNode(S1)));
}

0 comments on commit 2c9fa7e

Please sign in to comment.