[PATCH] D11691: [WebAssembly] Add Relooper

Alon Zakai azakai at mozilla.com
Mon Aug 3 13:28:06 PDT 2015


azakai added inline comments.

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:83
@@ +82,3 @@
+  // FIXME: move from char* to LLVM data structures
+  free(static_cast<void *>(const_cast<char *>(Code)));
+  free(static_cast<void *>(const_cast<char *>(BranchVar)));
----------------
jfb wrote:
> Yeah I'd like to avoid `shared_ptr` overheads.
> 
> It does seem like either `unique_ptr` or a `BumpPtrAllocator` would work though.
unique_ptr can't work for BranchesIn because the blocks are used in more places, the branches also refer to blocks, for example.

[Processed]BranchesOut could use a unique_ptr+swapping, but it would seem a little inconsistent to me to use it only there. What do you think?

The relooper has a list of blocks. That could be a bump allocator in theory, but the API lets people create blocks and then takes ownership of them. These could alternatively be a unique_ptr, but other things will need to point to those same blocks (branches), so all those would be forced to be simple dumb pointers, and the mix of those seems ugly to me personally. But if you prefer it I can do it.

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:656
@@ +655,3 @@
+      (void) Simple;                                                           \
+      simple;                                                                  \
+    }                                                                          \
----------------
jfb wrote:
> `break;` here and below.
yikes! thanks!


Repository:
  rL LLVM

http://reviews.llvm.org/D11691







More information about the llvm-commits mailing list