Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SandboxIR][Tracker] Track InsertIntoBB #101595

Merged
merged 1 commit into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions llvm/include/llvm/SandboxIR/Tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,22 @@ class MoveInstr : public IRChangeBase {
#endif // NDEBUG
};

class InsertIntoBB final : public IRChangeBase {
Instruction *InsertedI = nullptr;

public:
InsertIntoBB(Instruction *InsertedI, Tracker &Tracker);
void revert() final;
void accept() final {}
#ifndef NDEBUG
void dump(raw_ostream &OS) const final {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you may consider some CRTP to automate dump methods, like

StringRef Name = getTypeName<DerivedT>();

(still doesn't address the gdb issue of not being able to call methods in parent classes, but something to think about)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I have to fix both these dumps and the Value dumps.

dumpCommon(OS);
OS << "InsertIntoBB";
}
LLVM_DUMP_METHOD void dump() const final;
#endif // NDEBUG
};

/// The tracker collects all the change objects and implements the main API for
/// saving / reverting / accepting.
class Tracker {
Expand Down
14 changes: 13 additions & 1 deletion llvm/lib/SandboxIR/SandboxIR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,11 @@ void Instruction::insertBefore(Instruction *BeforeI) {
assert(is_sorted(getLLVMInstrs(),
[](auto *I1, auto *I2) { return I1->comesBefore(I2); }) &&
"Expected program order!");

auto &Tracker = Ctx.getTracker();
if (Tracker.isTracking())
Tracker.track(std::make_unique<InsertIntoBB>(this, Tracker));

// Insert the LLVM IR Instructions in program order.
for (llvm::Instruction *I : getLLVMInstrs())
I->insertBefore(BeforeTopI);
Expand All @@ -443,14 +448,21 @@ void Instruction::insertInto(BasicBlock *BB, const BBIterator &WhereIt) {
llvm::BasicBlock *LLVMBB = cast<llvm::BasicBlock>(BB->Val);
llvm::Instruction *LLVMBeforeI;
llvm::BasicBlock::iterator LLVMBeforeIt;
Instruction *BeforeI;
if (WhereIt != BB->end()) {
Instruction *BeforeI = &*WhereIt;
BeforeI = &*WhereIt;
LLVMBeforeI = BeforeI->getTopmostLLVMInstruction();
LLVMBeforeIt = LLVMBeforeI->getIterator();
} else {
BeforeI = nullptr;
LLVMBeforeI = nullptr;
LLVMBeforeIt = LLVMBB->end();
}

auto &Tracker = Ctx.getTracker();
if (Tracker.isTracking())
Tracker.track(std::make_unique<InsertIntoBB>(this, Tracker));

// Insert the LLVM IR Instructions in program order.
for (llvm::Instruction *I : getLLVMInstrs())
I->insertInto(LLVMBB, LLVMBeforeIt);
Expand Down
12 changes: 12 additions & 0 deletions llvm/lib/SandboxIR/Tracker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,18 @@ void MoveInstr::dump() const {
}
#endif

void InsertIntoBB::revert() { InsertedI->removeFromParent(); }

InsertIntoBB::InsertIntoBB(Instruction *InsertedI, Tracker &Tracker)
: IRChangeBase(Tracker), InsertedI(InsertedI) {}

#ifndef NDEBUG
void InsertIntoBB::dump() const {
dump(dbgs());
dbgs() << "\n";
}
#endif

void Tracker::track(std::unique_ptr<IRChangeBase> &&Change) {
assert(State == TrackerState::Record && "The tracker should be tracking!");
Changes.push_back(std::move(Change));
Expand Down
63 changes: 63 additions & 0 deletions llvm/unittests/SandboxIR/TrackerTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,69 @@ define i32 @foo(i32 %arg) {
EXPECT_EQ(It, BB->end());
}

// TODO: Test multi-instruction patterns.
TEST_F(TrackerTest, InsertIntoBB) {
parseIR(C, R"IR(
define void @foo(i32 %arg) {
%add0 = add i32 %arg, %arg
ret void
}
)IR");
Function &LLVMF = *M->getFunction("foo");
sandboxir::Context Ctx(C);

auto *F = Ctx.createFunction(&LLVMF);
auto *BB = &*F->begin();
auto It = BB->begin();
sandboxir::Instruction *Add0 = &*It++;
sandboxir::Instruction *Ret = &*It++;
// Detach `Add0` before we save.
Add0->removeFromParent();

// Check insertBefore(Instruction *) with tracking enabled.
Ctx.save();
Add0->insertBefore(Ret);
It = BB->begin();
EXPECT_EQ(&*It++, Add0);
EXPECT_EQ(&*It++, Ret);
EXPECT_EQ(It, BB->end());
// Check revert().
Ctx.revert();
It = BB->begin();
EXPECT_EQ(&*It++, Ret);
EXPECT_EQ(It, BB->end());

// Check insertAfter(Instruction *) with tracking enabled.
Ctx.save();
Add0->insertAfter(Ret);
It = BB->begin();
EXPECT_EQ(&*It++, Ret);
EXPECT_EQ(&*It++, Add0);
EXPECT_EQ(It, BB->end());
// Check revert().
Ctx.revert();
It = BB->begin();
EXPECT_EQ(&*It++, Ret);
EXPECT_EQ(It, BB->end());

// Check insertInto(BasicBlock *, BasicBlock::iterator) with tracking enabled.
Ctx.save();
Add0->insertInto(BB, Ret->getIterator());
It = BB->begin();
EXPECT_EQ(&*It++, Add0);
EXPECT_EQ(&*It++, Ret);
EXPECT_EQ(It, BB->end());
// Check revert().
Ctx.revert();
It = BB->begin();
EXPECT_EQ(&*It++, Ret);
EXPECT_EQ(It, BB->end());

// To make sure we don't leak memory insert `Add0` back into the BB before the
// end of the test.
Add0->insertBefore(Ret);
}

TEST_F(TrackerTest, CallBaseSetters) {
parseIR(C, R"IR(
declare void @bar1(i8)
Expand Down
Loading