Author: vporpo Date: 2026-05-26T14:49:55-07:00 New Revision: 20e2ca147e4e9b6bf44f6ad9a8fa59a3c231ab48
URL: https://github.com/llvm/llvm-project/commit/20e2ca147e4e9b6bf44f6ad9a8fa59a3c231ab48 DIFF: https://github.com/llvm/llvm-project/commit/20e2ca147e4e9b6bf44f6ad9a8fa59a3c231ab48.diff LOG: Revert "[SandboxIR][Tracker] Implement accept(/*AcceptAll*/) and revert(/*Rev…" This reverts commit 5157be7bd5c12ac2836bdefb4e9cded6871b065c. Added: Modified: llvm/docs/SandboxIR.md llvm/include/llvm/SandboxIR/Context.h llvm/include/llvm/SandboxIR/Tracker.h llvm/lib/SandboxIR/Tracker.cpp llvm/unittests/SandboxIR/TrackerTest.cpp Removed: ################################################################################ diff --git a/llvm/docs/SandboxIR.md b/llvm/docs/SandboxIR.md index 01de088c0f32d..662d8d4eca112 100644 --- a/llvm/docs/SandboxIR.md +++ b/llvm/docs/SandboxIR.md @@ -108,8 +108,7 @@ To start tracking again, the user needs to call `save()`. Sandbox IR supports nested checkpoints, meaning that you can save more than once and revert more than once. Conceptually each `save()` adds a new checkpoint to a stack and each `revert()` rolls back the IR state to that of the checkpoint at the top of the stack and pops the checkpoint off the stack. -A call to `accept()` pops the last checkpoint from the stack. -Reverting or accepting all can be done with `revert(/*RevertAll=*/true)` and `accept(/*AcceptAll=*/true)`. +A call to `accept()` will clear the stack. ## Users of Sandbox IR - [The Sandbox Vectorizer](project:SandboxVectorizer.md) diff --git a/llvm/include/llvm/SandboxIR/Context.h b/llvm/include/llvm/SandboxIR/Context.h index 4f95abccc88b6..1b8ecc86c8c3f 100644 --- a/llvm/include/llvm/SandboxIR/Context.h +++ b/llvm/include/llvm/SandboxIR/Context.h @@ -249,9 +249,9 @@ class Context { /// Convenience function for `getTracker().save()` void save() { IRTracker.save(); } /// Convenience function for `getTracker().revert()` - void revert(bool RevertAll = false) { IRTracker.revert(RevertAll); } + void revert() { IRTracker.revert(); } /// Convenience function for `getTracker().accept()` - void accept(bool AcceptAll = false) { IRTracker.accept(AcceptAll); } + void accept() { IRTracker.accept(); } LLVM_ABI sandboxir::Value *getValue(llvm::Value *V) const; const sandboxir::Value *getValue(const llvm::Value *V) const { diff --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h index f74ff5d29d620..9b6b255d428d3 100644 --- a/llvm/include/llvm/SandboxIR/Tracker.h +++ b/llvm/include/llvm/SandboxIR/Tracker.h @@ -19,18 +19,13 @@ // are saved in the order they are registered with the tracker and are stored in // the `Tracker::Changes` vector. All of this is done transparently to // the user. -// Calling `Tracker::save()` a second time without having accepted/reverted the -// state, creates a second nested checkpoint. // // Reverting changes // ----------------- -// Calling `Tracker::revert()` will restore the state saved when the last +// Calling `Tracker::revert()` will restore the state saved when // `Tracker::save()` was called. Internally this goes through the // change objects in `Tracker::Changes` in reverse order, calling their // `IRChangeBase::revert()` function one by one. -// In the context of a nested checkpoint, this will revert the state -// until the last `Tracker::save()` checkpoint. -// You can revert all changes with `Tracker::revert(/*RevertAll=*/true)`. // // Accepting changes // ----------------- @@ -39,9 +34,6 @@ // This is the job of `Tracker::accept()`. Internally this will go // through the change objects in `Tracker::Changes` in order, calling // `IRChangeBase::accept()`. -// In the context of a nested checkpoint, this will leave the state unchanged -// and will remove the last checkpoint for the stack. -// You can accept all changes with `Tracker::accept(/*AcceptAll=*/true)`. // //===----------------------------------------------------------------------===// @@ -512,16 +504,12 @@ class Tracker { bool isTracking() const { return State == TrackerState::Record; } /// \Returns the current state of the tracker. TrackerState getState() const { return State; } - /// Turns on IR tracking. If tracking is already enabled this creates a new - /// nested checkpoint. + /// Turns on IR tracking. LLVM_ABI void save(); - /// Stops tracking and accept changes. If we have nested checkpoints, this - /// will remove the last checkpoint from the stack without modifying the - /// state. - LLVM_ABI void accept(bool AcceptAll = false); - /// Stops tracking and reverts to saved state. If we have nested checkpoints - /// this will revert the state to the last checkpoint. - LLVM_ABI void revert(bool RevertAll = false); + /// Stops tracking and accept changes. + LLVM_ABI void accept(); + /// Stops tracking and reverts to saved state. + LLVM_ABI void revert(); /// \returns the number of nested (outstanding) checkpoints. unsigned nestingDepth() const { return Snapshots.size(); } diff --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp index 031460c3982ed..7609ded4cc828 100644 --- a/llvm/lib/SandboxIR/Tracker.cpp +++ b/llvm/lib/SandboxIR/Tracker.cpp @@ -338,11 +338,10 @@ void Tracker::save() { #endif } -void Tracker::revert(bool RevertAll) { +void Tracker::revert() { assert(State == TrackerState::Record && "Forgot to save()!"); State = TrackerState::Reverting; - unsigned UntilChangeIdx = RevertAll ? 0 : Snapshots.back(); - const unsigned ToRevert = Changes.size() - UntilChangeIdx; + const unsigned ToRevert = Changes.size() - Snapshots.back(); unsigned CntReverts = 0; for (auto &Change : reverse(Changes)) { // Stop reverting if we reach the index of the last snapshot. @@ -351,10 +350,7 @@ void Tracker::revert(bool RevertAll) { Change->revert(*this); } Changes.erase(Changes.end() - ToRevert, Changes.end()); - if (RevertAll) - Snapshots.clear(); - else - Snapshots.pop_back(); + Snapshots.pop_back(); #if !defined(NDEBUG) && defined(EXPENSIVE_CHECKS) SnapshotChecker.back().expectNoDiff(); SnapshotChecker.pop_back(); @@ -362,13 +358,8 @@ void Tracker::revert(bool RevertAll) { State = Snapshots.empty() ? TrackerState::Disabled : TrackerState::Record; } -void Tracker::accept(bool AcceptAll) { +void Tracker::accept() { assert(State == TrackerState::Record && "Forgot to save()!"); - if (!AcceptAll && Snapshots.size() > 1) { - // Just remove the last stacked checkpoint. - Snapshots.pop_back(); - return; - } State = TrackerState::Disabled; for (auto &Change : Changes) Change->accept(); diff --git a/llvm/unittests/SandboxIR/TrackerTest.cpp b/llvm/unittests/SandboxIR/TrackerTest.cpp index a9370e2a11b17..da9ca52828be3 100644 --- a/llvm/unittests/SandboxIR/TrackerTest.cpp +++ b/llvm/unittests/SandboxIR/TrackerTest.cpp @@ -2053,60 +2053,6 @@ define i32 @foo(i32 %arg0, i32 %arg1) { EXPECT_EQ(Ctx.getTracker().nestingDepth(), 0u); EXPECT_EQ(Add1->getOperand(0), Arg0); Checker.expectNoDiff(); - - // Check revert(/*RevertAll=*/true) - Checker.save(); - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 1u); - Add1->setOperand(0, Arg1); - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 2u); - Add1->setOperand(0, Arg0); - Ctx.revert(/*RevertAll=*/true); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 0u); - Checker.expectNoDiff(); - - // Check revert(/*RevertAll=*/false) - Checker.save(); - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 1u); - Add1->setOperand(0, Arg1); - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 2u); - Add1->setOperand(0, Arg0); - Ctx.revert(/*RevertAll=*/false); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 1u); - EXPECT_EQ(Add1->getOperand(0), Arg1); - Ctx.revert(/*RevertAll=*/false); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 0u); - Checker.expectNoDiff(); - - // Check accept(/*AcceptAll=*/false) - auto *OrigOp = Add1->getOperand(0); - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 1u); - Add1->setOperand(0, Arg1); - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 2u); - Add1->setOperand(0, Arg0); - Ctx.accept(/*RevertAll=*/false); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 1u); - EXPECT_EQ(Add1->getOperand(0), Arg0); - Ctx.accept(/*RevertAll=*/false); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 0u); - EXPECT_EQ(Add1->getOperand(0), Arg0); - Add1->setOperand(0, OrigOp); - - // Check accept(/*AcceptAll=*/true) - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 1u); - Add1->setOperand(0, Arg1); - Ctx.save(); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 2u); - Add1->setOperand(0, Arg0); - Ctx.accept(/*RevertAll=*/true); - EXPECT_EQ(Ctx.getTracker().nestingDepth(), 0u); - EXPECT_EQ(Add1->getOperand(0), Arg0); } #endif // NDEBUG _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
