Skip to content

Commit 6e48214

Browse files
authored
[SandboxVec][DAG] Register callback for erase instr (#116742)
This patch adds the callback registration logic in the DAG's constructor and the corresponding deregistration logic in the destructor. It also implements the code that makes sure that SchedBundle and DGNodes can be safely destroyed in any order.
1 parent a54e8b2 commit 6e48214

File tree

4 files changed

+51
-1
lines changed

4 files changed

+51
-1
lines changed

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ class DGNode {
117117
assert(!isMemDepNodeCandidate(I) && "Expected Non-Mem instruction, ");
118118
}
119119
DGNode(const DGNode &Other) = delete;
120-
virtual ~DGNode() = default;
120+
virtual ~DGNode();
121121
/// \Returns the number of unscheduled successors.
122122
unsigned getNumUnscheduledSuccs() const { return UnscheduledSuccs; }
123123
void decrUnscheduledSuccs() {
@@ -292,6 +292,7 @@ class DependencyGraph {
292292

293293
Context *Ctx = nullptr;
294294
std::optional<Context::CallbackID> CreateInstrCB;
295+
std::optional<Context::CallbackID> EraseInstrCB;
295296

296297
std::unique_ptr<BatchAAResults> BatchAA;
297298

@@ -334,17 +335,27 @@ class DependencyGraph {
334335
// TODO: Update the dependencies for the new node.
335336
// TODO: Update the MemDGNode chain to include the new node if needed.
336337
}
338+
/// Called by the callbacks when instruction \p I is about to get deleted.
339+
void notifyEraseInstr(Instruction *I) {
340+
InstrToNodeMap.erase(I);
341+
// TODO: Update the dependencies.
342+
// TODO: Update the MemDGNode chain to remove the node if needed.
343+
}
337344

338345
public:
339346
/// This constructor also registers callbacks.
340347
DependencyGraph(AAResults &AA, Context &Ctx)
341348
: Ctx(&Ctx), BatchAA(std::make_unique<BatchAAResults>(AA)) {
342349
CreateInstrCB = Ctx.registerCreateInstrCallback(
343350
[this](Instruction *I) { notifyCreateInstr(I); });
351+
EraseInstrCB = Ctx.registerEraseInstrCallback(
352+
[this](Instruction *I) { notifyEraseInstr(I); });
344353
}
345354
~DependencyGraph() {
346355
if (CreateInstrCB)
347356
Ctx->unregisterCreateInstrCallback(*CreateInstrCB);
357+
if (EraseInstrCB)
358+
Ctx->unregisterEraseInstrCallback(*EraseInstrCB);
348359
}
349360

350361
DGNode *getNode(Instruction *I) const {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ class SchedBundle {
6969
private:
7070
ContainerTy Nodes;
7171

72+
/// Called by the DGNode destructor to avoid accessing freed memory.
73+
void eraseFromBundle(DGNode *N) { Nodes.erase(find(Nodes, N)); }
74+
friend DGNode::~DGNode(); // For eraseFromBundle().
75+
7276
public:
7377
SchedBundle() = default;
7478
SchedBundle(ContainerTy &&Nodes) : Nodes(std::move(Nodes)) {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "llvm/ADT/ArrayRef.h"
1111
#include "llvm/SandboxIR/Instruction.h"
1212
#include "llvm/SandboxIR/Utils.h"
13+
#include "llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h"
1314

1415
namespace llvm::sandboxir {
1516

@@ -58,6 +59,12 @@ bool PredIterator::operator==(const PredIterator &Other) const {
5859
return OpIt == Other.OpIt && MemIt == Other.MemIt;
5960
}
6061

62+
DGNode::~DGNode() {
63+
if (SB == nullptr)
64+
return;
65+
SB->eraseFromBundle(this);
66+
}
67+
6168
#ifndef NDEBUG
6269
void DGNode::print(raw_ostream &OS, bool PrintDeps) const {
6370
OS << *I << " USuccs:" << UnscheduledSuccs << " Sched:" << Scheduled << "\n";

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -830,3 +830,31 @@ define void @foo(ptr %ptr, i8 %v1, i8 %v2, i8 %v3, i8 %arg) {
830830
// TODO: Check the dependencies to/from NewSN after they land.
831831
// TODO: Check the MemDGNode chain.
832832
}
833+
834+
TEST_F(DependencyGraphTest, EraseInstrCallback) {
835+
parseIR(C, R"IR(
836+
define void @foo(ptr %ptr, i8 %v1, i8 %v2, i8 %v3, i8 %arg) {
837+
store i8 %v1, ptr %ptr
838+
store i8 %v2, ptr %ptr
839+
store i8 %v3, ptr %ptr
840+
ret void
841+
}
842+
)IR");
843+
llvm::Function *LLVMF = &*M->getFunction("foo");
844+
sandboxir::Context Ctx(C);
845+
auto *F = Ctx.createFunction(LLVMF);
846+
auto *BB = &*F->begin();
847+
auto It = BB->begin();
848+
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
849+
auto *S2 = cast<sandboxir::StoreInst>(&*It++);
850+
auto *S3 = cast<sandboxir::StoreInst>(&*It++);
851+
852+
// Check erase instruction callback.
853+
sandboxir::DependencyGraph DAG(getAA(*LLVMF), Ctx);
854+
DAG.extend({S1, S3});
855+
S2->eraseFromParent();
856+
auto *DeletedN = DAG.getNodeOrNull(S2);
857+
EXPECT_TRUE(DeletedN == nullptr);
858+
// TODO: Check the dependencies to/from NewSN after they land.
859+
// TODO: Check the MemDGNode chain.
860+
}

0 commit comments

Comments
 (0)