[llvm] [SandboxIR] Add callbacks for instruction insert/remove/move ops (PR #112965)

via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 18 15:03:21 PDT 2024


================
@@ -9,18 +9,34 @@
 #ifndef LLVM_SANDBOXIR_CONTEXT_H
 #define LLVM_SANDBOXIR_CONTEXT_H
 
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/SandboxIR/Tracker.h"
 #include "llvm/SandboxIR/Type.h"
 
 namespace llvm::sandboxir {
 
-class Module;
-class Value;
 class Argument;
+class BBIterator;
 class Constant;
+class Module;
+class Value;
 
 class Context {
+public:
+  // A RemoveInstrCallback receives the instruction about to be removed.
+  using RemoveInstrCallback = std::function<void(Instruction *)>;
+  // A InsertInstrCallback receives the instruction about to be created.
+  using InsertInstrCallback = std::function<void(Instruction *)>;
+  // A MoveInstrCallback receives the instruction about to be moved, the
+  // destination BB and an iterator pointing to the insertion position.
+  using MoveInstrCallback =
+      std::function<void(Instruction *, const BBIterator &)>;
+
+  /// An ID for a registered callback. Used for deregistration.
+  using CallbackID = int;
----------------
vporpo wrote:

> How is another layer of indirection and an extra heap allocation better than keeping an integer around?

Yeah, it's definitely not great, and we should try to make the code that runs the callbacks as fast as possible.

Since @tschuett brought up non-determinism, should we try to make sure that the callbacks are run in the same order as registered? Because we are currently not only not getting that order but since we are iterating over the DenseMap we are also getting a non-deterministic order. Using a MapVector will fix non-determinism, but perhaps we should try to stick to registration order, and try to change it later if it turns out that it's not needed. I think we could use a vector instead of a map, because removal of a callback shouldn't happen too often (and we won't have too many callbacks) so a linear-time search should be fine. Wdyt?

https://github.com/llvm/llvm-project/pull/112965


More information about the llvm-commits mailing list