[PATCH] D11691: [WebAssembly] Add Relooper

[JF Bastien via llvm-commits]

jfb added a comment.

I think this looks pretty good (besides the few other comments). @sunfish and I intend to iterate on this code (he had ideas on how to generate better ASTs), and:

- Add extensive testing using WebAssembly's textual format.
- Tune the code (so I'd ignore `std::deque` versus `std::list` discussions for now).
- Make sure to clarify object ownership where possible, instead of using explicit memory management.

I'd still leave the patch open until next week to make sure anyone who wants to comment has a change to. Otherwise, I look forward to having control flow :-)


================
Comment at: lib/Target/WebAssembly/Relooper.cpp:758
@@ +757,3 @@
+                      if (Details->Type == Branch::Break) {
+                        Details->Type = Branch::Direct;
+                        if (MultipleShape *Multiple =
----------------
I discussed this with a few LLVM folks, and they'd rather start off with lambdas instead. We'll make sure to measure perf and ensure inlining isn't an issue.

================
Comment at: lib/Target/WebAssembly/Relooper.h:116
@@ +115,3 @@
+//  Loop: An infinite loop.
+//
+
----------------
It would be nice to move this to the Shape class, and use doxygen-style comments (same for other classes).

================
Comment at: lib/Target/WebAssembly/Relooper.h:140
@@ +139,3 @@
+  Shape(ShapeKind KindInit) : Id(-1), Next(nullptr), Kind(KindInit) {}
+  virtual ~Shape() {}
+};
----------------
Could you make this one pure virtual, and implement an override dtor in derived classes?


Repository:
  rL LLVM

http://reviews.llvm.org/D11691