[PATCH] Machine Level IR Serialization: print and parse LLVM IR.

[Philip Reames]

A few random comments, not a systematic review.

General comment: Likely too much complexity for the current functionality.  Start with the simple code, add complexity as justified.


REPOSITORY
  rL LLVM

================
Comment at: include/llvm/CodeGen/Serialization/Parser.h:10
@@ +9,3 @@
+//
+// This file declares the functions that parse the machine level IR.
+//
----------------
I'd strongly suggest adding a clear comment explaining the in progress nature of the work.

Also, terminology wise, I'd strongly suggest you strict with machine instruction representation rather than machine level IR.  It's not clear what the later is, while the former is well defined.

================
Comment at: include/llvm/CodeGen/Serialization/Parser.h:18
@@ +17,3 @@
+#include "llvm/IR/Module.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/SMLoc.h"
----------------
Include order.

================
Comment at: include/llvm/CodeGen/Serialization/Parser.h:30
@@ +29,3 @@
+
+/// \brief This class parses the machine level IR.
+/// It allows the users to parse the machine level IR at a later stage,
----------------
It looks like this is a helpful class which should live in the cpp file?

================
Comment at: lib/CodeGen/Serialization/MIRPrintingPass.cpp:28
@@ +27,3 @@
+/// an output stream.
+struct MachineLevelIRPrintingPass : public MachineFunctionPass {
+  static char ID;
----------------
Is there a MachineModulePass?  The use of the HaveToPrintModule flag feels very dirty.

================
Comment at: lib/CodeGen/Serialization/YAML.h:1
@@ +1,2 @@
+//===- YAML.h - YAML IO utilities for machine level IR --------------------===//
+//
----------------
This is overly generally named.  I'm not really clear what the purpose of this file is at all.

================
Comment at: tools/llc/llc.cpp:221
@@ -216,5 +220,3 @@
   if (!SkipModule) {
-    M = parseIRFile(InputFilename, Err, Context);
-    if (!M) {
-      Err.print(argv[0], errs());
-      return 1;
+    if (StringRef(InputFilename).endswith_lower(".mir")) {
+      auto ParseState =
----------------
Without context, it's impossible to tell if this in the right place.  Please upload with full context.

http://reviews.llvm.org/D9616

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/