[PATCH] D11691: [WebAssembly] Add Relooper

[JF Bastien]

jfb added inline comments.

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:84
@@ +83,3 @@
+  free(static_cast<void *>(const_cast<char *>(Code)));
+  free(static_cast<void *>(const_cast<char *>(BranchVar)));
+  for (const auto &iter : ProcessedBranchesOut) {
----------------
> unique_ptr can't work for BranchesIn because the blocks are used in more places, the branches also refer to blocks, for example.

Is there one datastructure that always owns the blocks, and others that have references to them (that don't outlive the owner)? For that you could own with `unique_ptr`, and have non-owning pointers in the other datastructures.

I just find it hard / unusual to track lifetime manually, but I don't want to pay the cost of `shared_ptr`. :-)

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:133
@@ +132,3 @@
+      ToInvestigate.push_back(Root);
+      while (ToInvestigate.size() > 0) {
+        Block *Curr = ToInvestigate.front();
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:158
@@ +157,3 @@
+        if (Original->BranchesIn.size() <= 1 ||
+            Original->BranchesOut.size() > 0)
+          continue; // only dead ends, for now
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:279
@@ +278,3 @@
+      BlockSet Queue = Entries;
+      while (Queue.size() > 0) {
+        Block *Curr = *(Queue.begin());
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:291
@@ +290,3 @@
+      }
+      assert(InnerBlocks.size() > 0);
+
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:340
@@ +339,3 @@
+          ToInvalidate.push_back(New);
+          while (ToInvalidate.size() > 0) {
+            Block *Invalidatee = ToInvalidate.front();
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:381
@@ +380,3 @@
+      }
+      while (Queue.size() > 0) {
+        Block *Curr = Queue.front();
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:432
@@ +431,3 @@
+        }
+        while (ToInvalidate.size() > 0) {
+          Block *Invalidatee = ToInvalidate.front();
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:511
@@ +510,3 @@
+    Ret = Temp;                                                                \
+  if (!NextEntries->size()) {                                                  \
+    return Ret;                                                                \
----------------
`.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:542
@@ +541,3 @@
+
+        if (IndependentGroups.size() > 0) {
+          // We can handle a group in a multiple if its entry cannot be reached
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:579
@@ +578,3 @@
+            Block *SmallEntry = iter->first;
+            int SmallSize = iter->second.size();
+            iter++;
----------------
This and two lines down should probably be `size_t` (`auto` would get the right `::size_type`).

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:612
@@ +611,3 @@
+
+          if (IndependentGroups.size() > 0)
+            // Some groups removable ==> Multiple
----------------
`!.empty()`

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:725
@@ +724,3 @@
+                    Simple->Inner->ProcessedBranchesOut.size() == 2 &&
+                    Depth < 20) {
+                  // If there is a next block, we already know at Simple
----------------
Another customization point?

================
Comment at: lib/Target/WebAssembly/Relooper.cpp:840
@@ +839,3 @@
+                Details->Type == Branch::Continue) {
+              assert(LoopStack.size() > 0);
+              if (Details->Ancestor != LoopStack.top() && Details->Labeled) {
----------------
`!.empty()`


Repository:
  rL LLVM

http://reviews.llvm.org/D11691