[PATCH] D97583: [WebAssembly] Fix reverse mapping in WasmEHFuncInfo

[Heejin Ahn via Phabricator via llvm-commits]

aheejin marked an inline comment as done.
aheejin added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/WasmEHFuncInfo.h:45
+    assert(hasUnwindSrcs(BB));
+    const auto &Vec = UnwindDestToSrcs.lookup(BB);
+    SmallPtrSet<const BasicBlock *, 4> Ret;
----------------
dschuff wrote:
> I guess `Vec` could be named `Set` now since it's a set rather than a vector.
Yeah I made it as a vector first and then changed it to a set, which resulted in this.. Fixed.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyCFGSort.cpp:266
           bool IsDeferred = false;
-          for (Entry &E : reverse(Entries)) {
-            if (E.TheRegion->getHeader() == UnwindSrc) {
+          for (Entry &E : Entries) {
+            if (UnwindSrcs.count(E.TheRegion->getHeader())) {
----------------
dschuff wrote:
> what's the significance of the iteration order here?
Now we shouldn't do a reverse traversal because `Succ` has to be added to `Deferred` list of the outermost `SortRegion`. For example, if exception A contains exception B, and both A's header and B's header's unwind destination is `Succ`, `Entries` will be like `[..., exception A, ..., exception B, ...]`, and `Succ` has to be deferred until all blocks in the exception A are scheduled.

Before when I incorrectly assumed the unwind source is a single BB, it was correct traverse `Entries` either way. I did it backwards just because I thought it would more likely to hit the target first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97583