[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/lib/CodeGen/MachineOperand.cpp:484-488
+void MIRFormatter::printIRValue(raw_ostream &OS, const Value &V,
+                                ModuleSlotTracker &MST) {
+  printIRValueReference(OS, V, MST);
+}
+
----------------
dsanders wrote:
> pguo wrote:
> > arsenm wrote:
> > > I don't see the point of adding this function? This probably shouldn't be leaking the MIRPrinter into MachineOperand.cpp
> > The idea is a helper function to print IR value in MIR serialization format, which can be used by target custom pseudo source value.  The target custom pseudo source value can hold IR value reference, it's better to print/parse it in a standard way.
> I agree with printing/parsing it the same way but it is a bit odd to have part of the MIRFormatter implementation in this file. Is there somewhere that printIRValueReference() could be to make it available for both MachineOperand.cpp and a newly created MIRFormatter.cpp?
> 
> If not, I don't think it's a huge problem to have this function here. Both objects are implemented in the same library and I've seen this kind of thing elsewhere with some of the reader/writer implementations.
Moved this function to MIRPrinter.cpp which to me seems like a better location.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69836