[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