Skip to content

Commit 1c207f1

Browse files
authored
[SandboxVec][DAG] Fix DAG when old interval is mem free (#126983)
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.
1 parent 51c847d commit 1c207f1

File tree

2 files changed

+45
-5
lines changed

2 files changed

+45
-5
lines changed

llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ MemDGNodeIntervalBuilder::getBotMemDGNode(const Interval<Instruction> &Intvl,
122122
Interval<MemDGNode>
123123
MemDGNodeIntervalBuilder::make(const Interval<Instruction> &Instrs,
124124
DependencyGraph &DAG) {
125+
if (Instrs.empty())
126+
return {};
125127
auto *TopMemN = getTopMemDGNode(Instrs, DAG);
126128
// If we couldn't find a mem node in range TopN - BotN then it's empty.
127129
if (TopMemN == nullptr)
@@ -529,8 +531,8 @@ Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
529531
}
530532
}
531533
};
532-
if (DAGInterval.empty()) {
533-
assert(NewInterval == InstrsInterval && "Expected empty DAGInterval!");
534+
auto MemDAGInterval = MemDGNodeIntervalBuilder::make(DAGInterval, *this);
535+
if (MemDAGInterval.empty()) {
534536
FullScan(NewInterval);
535537
}
536538
// 2. The new section is below the old section.
@@ -550,8 +552,7 @@ Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
550552
// range including both NewInterval and DAGInterval until DstN, for each DstN.
551553
else if (DAGInterval.bottom()->comesBefore(NewInterval.top())) {
552554
auto DstRange = MemDGNodeIntervalBuilder::make(NewInterval, *this);
553-
auto SrcRangeFull = MemDGNodeIntervalBuilder::make(
554-
DAGInterval.getUnionInterval(NewInterval), *this);
555+
auto SrcRangeFull = MemDAGInterval.getUnionInterval(DstRange);
555556
for (MemDGNode &DstN : DstRange) {
556557
auto SrcRange =
557558
Interval<MemDGNode>(SrcRangeFull.top(), DstN.getPrevNode());
@@ -589,7 +590,7 @@ Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
589590
// When scanning for deps with destination in DAGInterval we need to
590591
// consider sources from the NewInterval only, because all intra-DAGInterval
591592
// dependencies have already been created.
592-
auto DstRangeOld = MemDGNodeIntervalBuilder::make(DAGInterval, *this);
593+
auto DstRangeOld = MemDAGInterval;
593594
auto SrcRange = MemDGNodeIntervalBuilder::make(NewInterval, *this);
594595
for (MemDGNode &DstN : DstRangeOld)
595596
scanAndAddDeps(DstN, SrcRange);

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,3 +1013,42 @@ define void @foo(ptr %ptr, i8 %v1, i8 %v2, i8 %arg) {
10131013
EXPECT_EQ(S2N->getNextNode(), S1N);
10141014
EXPECT_EQ(S1N->getNextNode(), nullptr);
10151015
}
1016+
1017+
// Extending an "Old" interval with no mem instructions.
1018+
TEST_F(DependencyGraphTest, ExtendDAGWithNoMem) {
1019+
parseIR(C, R"IR(
1020+
define void @foo(ptr %ptr, i8 %v, i8 %v0, i8 %v1, i8 %v2, i8 %v3) {
1021+
store i8 %v0, ptr %ptr
1022+
store i8 %v1, ptr %ptr
1023+
%zext1 = zext i8 %v to i32
1024+
%zext2 = zext i8 %v to i32
1025+
store i8 %v2, ptr %ptr
1026+
store i8 %v3, ptr %ptr
1027+
ret void
1028+
}
1029+
)IR");
1030+
llvm::Function *LLVMF = &*M->getFunction("foo");
1031+
sandboxir::Context Ctx(C);
1032+
auto *F = Ctx.createFunction(LLVMF);
1033+
auto *BB = &*F->begin();
1034+
auto It = BB->begin();
1035+
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
1036+
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
1037+
auto *Z1 = cast<sandboxir::CastInst>(&*It++);
1038+
auto *Z2 = cast<sandboxir::CastInst>(&*It++);
1039+
auto *S2 = cast<sandboxir::StoreInst>(&*It++);
1040+
auto *S3 = cast<sandboxir::StoreInst>(&*It++);
1041+
1042+
sandboxir::DependencyGraph DAG(getAA(*LLVMF), Ctx);
1043+
// Create a non-empty DAG that contains no memory instructions.
1044+
DAG.extend({Z1, Z2});
1045+
// Now extend it downwards.
1046+
DAG.extend({S2, S3});
1047+
EXPECT_TRUE(memDependency(DAG.getNode(S2), DAG.getNode(S3)));
1048+
1049+
// Same but upwards.
1050+
DAG.clear();
1051+
DAG.extend({Z1, Z2});
1052+
DAG.extend({S0, S1});
1053+
EXPECT_TRUE(memDependency(DAG.getNode(S0), DAG.getNode(S1)));
1054+
}

0 commit comments

Comments
 (0)