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

Alex Lorenz arphaman at gmail.com
Wed May 13 13:04:32 PDT 2015


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.
+//
----------------
reames wrote:
> 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.
Done.

I also replaced the machine level IR terminology with MIR in this patch.

================
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"
----------------
reames wrote:
> Include order.
Fixed.

================
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,
----------------
reames wrote:
> It looks like this is a helpful class which should live in the cpp file?
This class will be useful as a public class later, but for now I removed it from this patch.

================
Comment at: lib/CodeGen/Serialization/YAML.h:1
@@ +1,2 @@
+//===- YAML.h - YAML IO utilities for machine level IR --------------------===//
+//
----------------
reames wrote:
> This is overly generally named.  I'm not really clear what the purpose of this file is at all.
I've renamed it and updated the comments. Hopefully it is better now.

================
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 =
----------------
reames wrote:
> Without context, it's impossible to tell if this in the right place.  Please upload with full context.
The updated patch includes more context now.

http://reviews.llvm.org/D9616

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






More information about the llvm-commits mailing list