[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