[PATCH] D11691: [WebAssembly] Add Relooper
JF Bastien
jfb at chromium.org
Fri Jul 31 13:35:50 PDT 2015
jfb added a comment.
Could you also add the `.cpp` file to `CMakeLists.txt`? The `Makefile` picks it up automatically...
================
Comment at: lib/Target/WebAssembly/Relooper.h:19
@@ +18,3 @@
+#include <stdio.h>
+#include <stdarg.h>
+
----------------
It's more common to include the C++ version:
```
#include <cassert>
#include <cstdio>
#include <cstdarg>
```
================
Comment at: lib/Target/WebAssembly/Relooper.h:27
@@ +26,3 @@
+struct Block;
+struct Shape;
+
----------------
You should put everything in a sub-namespace of the `llvm` namespace.
================
Comment at: lib/Target/WebAssembly/Relooper.h:41
@@ +40,3 @@
+ };
+ Shape *Ancestor; // If not NULL, this shape is the relevant one for purposes
+ // of getting to the target block. We break or continue on it
----------------
`nullptr` is the new hotness, here and elsewhere.
================
Comment at: lib/Target/WebAssembly/Relooper.h:60
@@ +59,3 @@
+// of operator<(T, T)). This is useful to ensure deterministic
+// behavior.
+template <typename T> struct InsertOrderedSet {
----------------
Could you use `llvm/ADT/SetVector.h` instead?
================
Comment at: lib/Target/WebAssembly/Relooper.h:104
@@ +103,3 @@
+// of operator<(Key, Key)). This is useful to ensure deterministic
+// behavior.
+template <typename Key, typename T> struct InsertOrderedMap {
----------------
Could you use `llvm/ADT/SetVector.h` instead?
Repository:
rL LLVM
http://reviews.llvm.org/D11691
More information about the llvm-commits
mailing list