[PATCH] D69836: [MIR] Target specific MIR formating and parsing

[Peng Guo via Phabricator via llvm-commits]

pguo marked 2 inline comments as done.
pguo added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:1795-1799
+  virtual const MIRFormatter *getMIRFormatter() const {
+    if (!Formatter.get())
+      Formatter = std::make_unique<MIRFormatter>();
+    return Formatter.get();
+  }
----------------
arsenm wrote:
> arsenm wrote:
> > This requires the target implementation to recreate the null check and construct pattern Make this non-virtual and have a virtual getMIRFormatterImpl ?
> I also just realized this should really go in the TargetMachine, not TargeInstrInfo, similar to the target specific YAML functions. The name should also be createMIRFormatter for consistency
Thank you for pointing me the target specific YAML functions, I didn't aware of them before.  Yes, I agree these target specific MIR serialization/de-serialization functions should be put together. I moved to TargetMachine, while I still use getMIRFormatter and get rid of the lazy creation (since I think this object is pretty light).  Maybe we should move those target specific YAML functions into MIRFormatter?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69836/new/

https://reviews.llvm.org/D69836