[PATCH] D44134: [WebAssembly] Add WebAssemblyException information analysis

Heejin Ahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 20 21:55:41 PDT 2018


aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp:79-90
+  for (auto &MI : *MBB) {
+    if (MI.getOpcode() == WebAssembly::CATCH_ALL) {
+      SeenCatchAll = true;
+      continue;
+    }
+    if (SeenCatchAll && MI.isCall()) {
+      const MachineOperand &CalleeOp = MI.getOperand(0);
----------------
majnemer wrote:
> Is it not possible for the MBB to branch to block which then calls std::terminate?
Hmm, good point... I assumed a terminatepad would be always a single BB but it may not hold. Do you think I should DFS all descendants from here?


================
Comment at: lib/Target/WebAssembly/WebAssemblyExceptionInfo.h:38-39
+  ~WebAssemblyException() {
+    for (size_t I = 0, E = SubExceptions.size(); I != E; ++I)
+      delete SubExceptions[I];
+  }
----------------
majnemer wrote:
> DeleteContainerPointers(SubExceptions);
> 
> Even better would be to use a std::vector<unique_ptr<WebAssemblyException>>
To change this to `std::vector<unique_ptr<WebAssemblyException>>`, should I RAII `WebAssemblyException` object at the first creation time and use `std::move` whenever I pass it to a function until I figure out which container to insert this?


Repository:
  rL LLVM

https://reviews.llvm.org/D44134





More information about the llvm-commits mailing list