[PATCH] D97583: [WebAssembly] Fix reverse mapping in WasmEHFuncInfo
Heejin Ahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 26 17:06:55 PST 2021
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
More information about the llvm-commits
mailing list