[PATCH] D94049: [WebAssembly] Fix catch unwind mismatches

[Heejin Ahn via Phabricator via llvm-commits]

aheejin marked 2 inline comments as done.
aheejin added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp:905-906
+    // put the 'delegate' BB in between. We normally create a split BB and make
+    // it a successor of the original BB (PostSplit == true), but in case the BB
+    // is an EH pad and the split pos is before 'catch', we should preserve the
+    // BB's property, including that it is an EH pad, in the later part of the
----------------
tlively wrote:
> What is an example of when the split position could be before `catch`?
When fixing catch mismatches. When an inner try is included in a body of an outer try:
```
bb0:
  try
    try
      ...
bb1 (ehpad):
    catch ...
      ...
bb2: (ehpad)
    end_try
  catch ...
    ...
```

Suppose we need to wrap the inner try with `try`-`delegate`:
```
bb0:
  try
    try          ;; new inst
      try
        ...
bb1 (ehpad):
      catch ...
        ...
bb-pre: (ehpad)  ;; new BB
      end_try
bb-delegate:     ;; new BB
    delegate    ;; new inst
bb2 (ehpad):
  catch ...
    ...
```

The split pos is before the `catch` in bb2. Because `catch`'s BB should be preserved, because that's what `WasmEHFuncInfo` remembers.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyCFGStackify.cpp:1315-1316
+        // anything
+        if (MI.getOpcode() == WebAssembly::CATCH_ALL)
+          ;
+
----------------
tlively wrote:
> ~Is this supposed to be a `continue`?~ Oh, I see it's just there in service of the `else if`s below. I found this confusing when I first saw it (especially without braces). I think it would be clearer if structured a different way.
The reason this is not a `continue` is it needs to run the common code at the very bottom:
```
EHPadStack.push_back(EHPad);
```
I can add the braces. Do you have any other restructuring suggestions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94049