[PATCH] D52127: [MF][MBB]: Add the ability to register callbacks for removal and insertion of MachineInstrs

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

Some additional coding style comments



================
Comment at: include/llvm/CodeGen/MachineFunction.h:355-356
+  using MICallbackFnTy = std::function<void(const MachineInstr*)>;
+  MICallbackFnTy InsertCallbackFn;
+  MICallbackFnTy RemoveCallbackFn;
+
----------------
Use `InsertCallbackFn = nullptr;` here instead of setting it in `init()`.


================
Comment at: include/llvm/CodeGen/MachineFunction.h:386-394
+  void doInsertCallback(const MachineInstr *MI) {
+    if (InsertCallbackFn)
+      InsertCallbackFn(MI);
+  }
+
+  void doRemoveCallback(const MachineInstr *MI) {
+    if (RemoveCallbackFn)
----------------
You should try to make these `private` with some friend declaration for the ilist traits.


================
Comment at: lib/CodeGen/MachineFunction.cpp:238-239
   }
+  InsertCallbackFn = nullptr;
+  RemoveCallbackFn = nullptr;
 }
----------------
I think this function is mostly about freeing memory. Setting the pointer should not be necessary.


Repository:
  rL LLVM

https://reviews.llvm.org/D52127