[PATCH] D77423: [WebAssembly] Fix wasm.lsda() optimization in WasmEHPrepare

[Heejin Ahn via Phabricator via llvm-commits]

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: llvm-commits, sunfish, hiraditya, jgravelle-google, sbc100.
Herald added a project: LLVM.
aheejin updated this revision to Diff 254872.
aheejin added a comment.

- clang-format


The explanation below is a little long, but it is mostly about the
background on why this change was necessary and not about the change
itself.

When we insert a call to the personality function wrapper
(`_Unwind_CallPersonality`) for a catch pad, we store some necessarily
info in `__wasm_lpad_context` struct and pass it. One of the info is the
LSDA address for the function. For this, we insert a call to
`wasm.lsda()`, which will be lowered down to the address of LSDA, and
store it in a field in `__wasm_lpad_context`.

There are exceptions to this personality call insertion: catchpads for
`catch (...)` and cleanuppads (for destructors) don't need personality
function calls, because we don't need to figure out whether the current
exception should be caught or not. (They always should.)

There was a little optimization to `wasm.lsda()` call insertion. Because
the LSDA address is the same throughout a function, we don't need to
insert a store of `wasm.lsda()` return value in every catchpad. For
example:

  try {
    foo();
  } catch (int) {
    // wasm.lsda() call and a store is inserted here, like, in pseudocode,
    // %lsda = wasm.lsda();
    // store %lsda to a field in __wasm_lpad_context
    try {
      foo();
    } catch (int) {
      // We don't need to insert the wasm.lsda() and store again, because
      // to arrive here, we have already stored the LSDA address to
      // __wasm_lpad_context in the outer catch.
    }
  }

So the previous algorithm checked if the current catch has a parent EH
pad, we don't insert a call to `wasm.lsda()` and its store.

But this was incorrect, because what if the outer catch is `catch (...)`
or a cleanuppad?

  try {
    foo();
  } catch (...) {
    // wasm.lsda() call and a store is NOT inserted here
    try {
      foo();
    } catch (int) {
      // We need wasm.lsda() here!
    }
  }

In this case we need to insert `wasm.lsda()` in the inner catchpad,
because the outer catchpad does not have one.

To minimize the number of inserted `wasm.lsda()` calls and stores, we
need a way to figure out whether we have encountered `wasm.lsda()` call
in any of EH pads that dominates the current EH pad. To figure that
out, we now visit EH pads in BFS order in the dominator tree so that we
visit parent BBs first before visiting its child BBs in the domtree.

We keep a set named `ExecutedLSDA`, which basically means "Do we have
`wasm.lsda() either in the current EH pad or any of its parent EH
pads in the dominator tree?". This is to prevent scanning the domtree up
to the root in the worst case every time we examine an EH pad: each EH
pad only needs to examine its immediate parent EH pad.

- If any of its parent EH pads in the domtree has `wasm.lsda`, this means we don't need `wasm.lsda()` in the current EH pad. We also insert the current EH pad in `ExecutedLSDA` set.
- If none of its parent EH pad has `wasm.lsda()`
  - If the current EH pad is a `catch (...)` or a cleanuppad, done.
  - If the current EH pad is neither a `catch (...)` nor a cleanuppad, add `wasm.lsda()` and the store in the current EH pad, and add the current EH pad to `ExecutedLSDA` set.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77423

Files:
  llvm/lib/CodeGen/WasmEHPrepare.cpp
  llvm/test/CodeGen/WebAssembly/wasmehprepare.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D77423.254872.patch
Type: text/x-patch
Size: 19310 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200403/29e03a46/attachment.bin>