[PATCH] D97677: [WebAssembly] Fix more ExceptionInfo grouping bugs

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp:163
+  // Exception A above. But there can still be remaining BBs within Exception A
+  // that are reachable from Exception B. These BBs semantically doesn't belong
+  // to Exception A and were not a part of 'catch' clause or cleanup code in the
----------------



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp:167
+  // because they were dominated by EHPad A. We fix this case by taking those
+  // BBs out of the incorrect exception and its all subexceptions that it
+  // belongs to.
----------------



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.cpp:207
 
-  // Add BBs to exceptions
+  // Add BBs to exceptions' block vector
   for (auto DomNode : post_order(&MDT)) {
----------------
I expected to see code copying the contents of the block set to the block vector, but I don't see that. Do the set and the vector have different contents, or am I missing something else?


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyExceptionInfo.h:48
   std::vector<MachineBasicBlock *> Blocks;
-  SmallPtrSet<const MachineBasicBlock *, 8> BlockSet;
+  SmallPtrSet<MachineBasicBlock *, 8> BlockSet;
 
----------------
If we expect there to be few blocks, then perhaps the cost of deleting them from the vector wouldn't be too bad? Alternatively, we could also "remove" blocks by replacing them in the vector with nullptr, then compact the vector and remove the nullptrs just once at the end. That might be more complicated then just adding this set, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97677/new/

https://reviews.llvm.org/D97677