[llvm-branch-commits] [llvm] a8f0c9b - Revert "[SandboxIR] Add debug checker to compare IR before/after a revert (#1…"
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Nov 18 09:53:37 PST 2024
Author: Jorge Gorbe Moya
Date: 2024-11-18T09:53:34-08:00
New Revision: a8f0c9ba51bfec56e936d73341135d062c1b59ff
URL: https://github.com/llvm/llvm-project/commit/a8f0c9ba51bfec56e936d73341135d062c1b59ff
DIFF: https://github.com/llvm/llvm-project/commit/a8f0c9ba51bfec56e936d73341135d062c1b59ff.diff
LOG: Revert "[SandboxIR] Add debug checker to compare IR before/after a revert (#1…"
This reverts commit 9161e6ab745adeef67a129b4e1b6724f026125f0.
Added:
Modified:
llvm/include/llvm/SandboxIR/Context.h
llvm/include/llvm/SandboxIR/Instruction.h
llvm/include/llvm/SandboxIR/Tracker.h
llvm/lib/SandboxIR/Tracker.cpp
llvm/unittests/SandboxIR/TrackerTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/SandboxIR/Context.h b/llvm/include/llvm/SandboxIR/Context.h
index b0d6f8335d9e0e..f2056de87cb946 100644
--- a/llvm/include/llvm/SandboxIR/Context.h
+++ b/llvm/include/llvm/SandboxIR/Context.h
@@ -44,12 +44,11 @@ class Context {
protected:
LLVMContext &LLVMCtx;
- friend class Type; // For LLVMCtx.
- friend class PointerType; // For LLVMCtx.
- friend class IntegerType; // For LLVMCtx.
- friend class StructType; // For LLVMCtx.
- friend class Region; // For LLVMCtx.
- friend class IRSnapshotChecker; // To snapshot LLVMModuleToModuleMap.
+ friend class Type; // For LLVMCtx.
+ friend class PointerType; // For LLVMCtx.
+ friend class IntegerType; // For LLVMCtx.
+ friend class StructType; // For LLVMCtx.
+ friend class Region; // For LLVMCtx.
Tracker IRTracker;
diff --git a/llvm/include/llvm/SandboxIR/Instruction.h b/llvm/include/llvm/SandboxIR/Instruction.h
index 2a59d72e285527..d9642365908d28 100644
--- a/llvm/include/llvm/SandboxIR/Instruction.h
+++ b/llvm/include/llvm/SandboxIR/Instruction.h
@@ -11,7 +11,6 @@
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Instructions.h"
-#include "llvm/IR/Module.h"
#include "llvm/IR/PatternMatch.h"
#include "llvm/SandboxIR/BasicBlock.h"
#include "llvm/SandboxIR/Constant.h"
diff --git a/llvm/include/llvm/SandboxIR/Tracker.h b/llvm/include/llvm/SandboxIR/Tracker.h
index 9a031f32708374..dab20eb809ba08 100644
--- a/llvm/include/llvm/SandboxIR/Tracker.h
+++ b/llvm/include/llvm/SandboxIR/Tracker.h
@@ -42,12 +42,13 @@
#include "llvm/ADT/PointerUnion.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StableHashing.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/IR/Instruction.h"
+#include "llvm/IR/Module.h"
#include "llvm/SandboxIR/Use.h"
#include "llvm/Support/Debug.h"
#include <memory>
+#include <regex>
namespace llvm::sandboxir {
@@ -63,56 +64,9 @@ class SwitchInst;
class ConstantInt;
class ShuffleVectorInst;
class CmpInst;
+class Module;
class GlobalVariable;
-#ifndef NDEBUG
-
-/// A class that saves hashes and textual IR snapshots of functions in a
-/// SandboxIR Context, and does hash comparison when `expectNoDiff` is called.
-/// If hashes
diff er, it prints textual IR for both old and new versions to
-/// aid debugging.
-///
-/// This is used as an additional debug check when reverting changes to
-/// SandboxIR, to verify the reverted state matches the initial state.
-class IRSnapshotChecker {
- Context &Ctx;
-
- // A snapshot of textual IR for a function, with a hash for quick comparison.
- struct FunctionSnapshot {
- llvm::stable_hash Hash;
- std::string TextualIR;
- };
-
- // A snapshot for each llvm::Function found in every module in the SandboxIR
- // Context. In practice there will always be one module, but sandbox IR
- // save/restore ops work at the Context level, so we must take the full state
- // into account.
- using ContextSnapshot = DenseMap<const llvm::Function *, FunctionSnapshot>;
-
- ContextSnapshot OrigContextSnapshot;
-
- // Dumps to a string the textual IR for a single Function.
- std::string dumpIR(const llvm::Function &F) const;
-
- // Returns a snapshot of all the modules in the sandbox IR context.
- ContextSnapshot takeSnapshot() const;
-
- // Compares two snapshots and returns true if they
diff er.
- bool
diff (const ContextSnapshot &Orig, const ContextSnapshot &Curr) const;
-
-public:
- IRSnapshotChecker(Context &Ctx) : Ctx(Ctx) {}
-
- /// Saves a snapshot of the current state. If there was any previous snapshot,
- /// it will be replaced with the new one.
- void save();
-
- /// Checks current state against saved state, crashes if
diff erent.
- void expectNoDiff();
-};
-
-#endif // NDEBUG
-
/// The base class for IR Change classes.
class IRChangeBase {
protected:
@@ -451,10 +405,6 @@ class Tracker {
TrackerState State = TrackerState::Disabled;
Context &Ctx;
-#ifndef NDEBUG
- IRSnapshotChecker SnapshotChecker;
-#endif
-
public:
#ifndef NDEBUG
/// Helps catch bugs where we are creating new change objects while in the
@@ -462,15 +412,7 @@ class Tracker {
bool InMiddleOfCreatingChange = false;
#endif // NDEBUG
- explicit Tracker(Context &Ctx)
- : Ctx(Ctx)
-#ifndef NDEBUG
- ,
- SnapshotChecker(Ctx)
-#endif
- {
- }
-
+ explicit Tracker(Context &Ctx) : Ctx(Ctx) {}
~Tracker();
Context &getContext() const { return Ctx; }
/// Record \p Change and take ownership. This is the main function used to
diff --git a/llvm/lib/SandboxIR/Tracker.cpp b/llvm/lib/SandboxIR/Tracker.cpp
index 27ed37aa9bdd37..d35e3ba84990f3 100644
--- a/llvm/lib/SandboxIR/Tracker.cpp
+++ b/llvm/lib/SandboxIR/Tracker.cpp
@@ -10,75 +10,12 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/IR/BasicBlock.h"
#include "llvm/IR/Instruction.h"
-#include "llvm/IR/Module.h"
-#include "llvm/IR/StructuralHash.h"
#include "llvm/SandboxIR/Instruction.h"
#include <sstream>
using namespace llvm::sandboxir;
#ifndef NDEBUG
-
-std::string IRSnapshotChecker::dumpIR(const llvm::Function &F) const {
- std::string Result;
- raw_string_ostream SS(Result);
- F.print(SS, /*AssemblyAnnotationWriter=*/nullptr);
- return Result;
-}
-
-IRSnapshotChecker::ContextSnapshot IRSnapshotChecker::takeSnapshot() const {
- ContextSnapshot Result;
- for (const auto &Entry : Ctx.LLVMModuleToModuleMap)
- for (const auto &F : *Entry.first) {
- FunctionSnapshot Snapshot;
- Snapshot.Hash = StructuralHash(F, /*DetailedHash=*/true);
- Snapshot.TextualIR = dumpIR(F);
- Result[&F] = Snapshot;
- }
- return Result;
-}
-
-bool IRSnapshotChecker::
diff (const ContextSnapshot &Orig,
- const ContextSnapshot &Curr) const {
- bool DifferenceFound = false;
- for (const auto &[F, OrigFS] : Orig) {
- auto CurrFSIt = Curr.find(F);
- if (CurrFSIt == Curr.end()) {
- DifferenceFound = true;
- dbgs() << "Function " << F->getName() << " not found in current IR.\n";
- dbgs() << OrigFS.TextualIR << "\n";
- continue;
- }
- const FunctionSnapshot &CurrFS = CurrFSIt->second;
- if (OrigFS.Hash != CurrFS.Hash) {
- DifferenceFound = true;
- dbgs() << "Found IR
diff erence in Function " << F->getName() << "\n";
- dbgs() << "Original:\n" << OrigFS.TextualIR << "\n";
- dbgs() << "Current:\n" << CurrFS.TextualIR << "\n";
- }
- }
- // Check that Curr doesn't contain any new functions.
- for (const auto &[F, CurrFS] : Curr) {
- if (!Orig.contains(F)) {
- DifferenceFound = true;
- dbgs() << "Function " << F->getName()
- << " found in current IR but not in original snapshot.\n";
- dbgs() << CurrFS.TextualIR << "\n";
- }
- }
- return DifferenceFound;
-}
-
-void IRSnapshotChecker::save() { OrigContextSnapshot = takeSnapshot(); }
-
-void IRSnapshotChecker::expectNoDiff() {
- ContextSnapshot CurrContextSnapshot = takeSnapshot();
- if (
diff (OrigContextSnapshot, CurrContextSnapshot)) {
- llvm_unreachable(
- "Original and current IR
diff er! Probably a checkpointing bug.");
- }
-}
-
void UseSet::dump() const {
dump(dbgs());
dbgs() << "\n";
@@ -338,12 +275,7 @@ void CmpSwapOperands::dump() const {
}
#endif
-void Tracker::save() {
- State = TrackerState::Record;
-#if !defined(NDEBUG) && defined(EXPENSIVE_CHECKS)
- SnapshotChecker.save();
-#endif
-}
+void Tracker::save() { State = TrackerState::Record; }
void Tracker::revert() {
assert(State == TrackerState::Record && "Forgot to save()!");
@@ -351,9 +283,6 @@ void Tracker::revert() {
for (auto &Change : reverse(Changes))
Change->revert(*this);
Changes.clear();
-#if !defined(NDEBUG) && defined(EXPENSIVE_CHECKS)
- SnapshotChecker.expectNoDiff();
-#endif
}
void Tracker::accept() {
diff --git a/llvm/unittests/SandboxIR/TrackerTest.cpp b/llvm/unittests/SandboxIR/TrackerTest.cpp
index cee13222179dce..4f2cfa6b06ecd2 100644
--- a/llvm/unittests/SandboxIR/TrackerTest.cpp
+++ b/llvm/unittests/SandboxIR/TrackerTest.cpp
@@ -1844,66 +1844,3 @@ define void @foo(i32 %arg, float %farg) {
Ctx.revert();
EXPECT_FALSE(FAdd->getFastMathFlags() != OrigFMF);
}
-
-TEST_F(TrackerTest, IRSnapshotCheckerNoChanges) {
- parseIR(C, R"IR(
-define i32 @foo(i32 %arg) {
- %add0 = add i32 %arg, %arg
- ret i32 %add0
-}
-)IR");
- Function &LLVMF = *M->getFunction("foo");
- sandboxir::Context Ctx(C);
-
- [[maybe_unused]] auto *F = Ctx.createFunction(&LLVMF);
- sandboxir::IRSnapshotChecker Checker(Ctx);
- Checker.save();
- Checker.expectNoDiff();
-}
-
-TEST_F(TrackerTest, IRSnapshotCheckerDiesWithUnexpectedChanges) {
- parseIR(C, R"IR(
-define i32 @foo(i32 %arg) {
- %add0 = add i32 %arg, %arg
- %add1 = add i32 %add0, %arg
- ret i32 %add1
-}
-)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 *Add1 = &*It++;
- sandboxir::IRSnapshotChecker Checker(Ctx);
- Checker.save();
- Add1->setOperand(1, Add0);
- EXPECT_DEATH(Checker.expectNoDiff(), "Found IR
diff erence");
-}
-
-TEST_F(TrackerTest, IRSnapshotCheckerSaveMultipleTimes) {
- parseIR(C, R"IR(
-define i32 @foo(i32 %arg) {
- %add0 = add i32 %arg, %arg
- %add1 = add i32 %add0, %arg
- ret i32 %add1
-}
-)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 *Add1 = &*It++;
- sandboxir::IRSnapshotChecker Checker(Ctx);
- Checker.save();
- Add1->setOperand(1, Add0);
- // Now IR
diff ers from the last snapshot. Let's take a new snapshot.
- Checker.save();
- // The new snapshot should have replaced the old one, so this should succeed.
- Checker.expectNoDiff();
-}
More information about the llvm-branch-commits
mailing list