#
0b2bc69b |
| 21-Apr-2021 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Put utility functions in Utils directory (NFC)
This CL 1. Creates Utils/ directory under lib/Target/WebAssembly 2. Moves existing WebAssemblyUtilities.cpp|h into the Utils/ directory 3
[WebAssembly] Put utility functions in Utils directory (NFC)
This CL 1. Creates Utils/ directory under lib/Target/WebAssembly 2. Moves existing WebAssemblyUtilities.cpp|h into the Utils/ directory 3. Creates Utils/WebAssemblyTypeUtilities.cpp|h and put type declarataions and type conversion functions scattered in various places into this single place.
It has been suggested several times that it is not easy to share utility functions between subdirectories (AsmParser, DIsassembler, MCTargetDesc, ...). Sometimes we ended up [[ https://reviews.llvm.org/D92840#2478863 | duplicating ]] the same function because of this.
There are already other targets doing this: AArch64, AMDGPU, and ARM have Utils/ subdirectory under their target directory.
This extracts the utility functions into a single directory Utils/ and make them sharable among all passes in WebAssembly/ and its subdirectories. Also I believe gathering all type-related conversion functionalities into a single place makes it more usable. (Actually I was working on another CL that uses various type conversion functions scattered in multiple places, which became the motivation for this CL.)
Reviewed By: dschuff, aardappel
Differential Revision: https://reviews.llvm.org/D100995
show more ...
|
Revision tags: llvmorg-12.0.0, llvmorg-12.0.0-rc5, llvmorg-12.0.0-rc4, llvmorg-12.0.0-rc3 |
|
#
d8b3dc5a |
| 25-Feb-2021 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix remapping branch dests in fixCatchUnwindMismatches
This is a case D97178 tried to solve but missed. D97178 could not handle the case when multiple consecutive delegates are generat
[WebAssembly] Fix remapping branch dests in fixCatchUnwindMismatches
This is a case D97178 tried to solve but missed. D97178 could not handle the case when multiple consecutive delegates are generated: - Before: ``` block br (a) try catch end_try end_block <- (a) ```
- After ``` block br (a) try ... try try catch end_try <- (a) delegate delegate end_block <- (b) ``` (The `br` should point to (b) now)
D97178 assumed `end_block` exists two BBs later than `end_try`, because it assumed the order as `end_try` BB -> `delegate` BB -> `end_block` BB. But it turned out there can be multiple `delegate`s in between. This patch changes the logic so we just search from `end_try` BB until we find `end_block`.
Fixes https://github.com/emscripten-core/emscripten/issues/13515. (More precisely, fixes https://github.com/emscripten-core/emscripten/issues/13515#issuecomment-784711318.)
Reviewed By: dschuff, tlively
Differential Revision: https://reviews.llvm.org/D97569
show more ...
|
Revision tags: llvmorg-12.0.0-rc2 |
|
#
f47a654a |
| 22-Feb-2021 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Remap branch dests after fixCatchUnwindMismatches
Fixing catch unwind mismatches can sometimes invalidate existing branch destinations. This CL remaps those destinations after placing
[WebAssembly] Remap branch dests after fixCatchUnwindMismatches
Fixing catch unwind mismatches can sometimes invalidate existing branch destinations. This CL remaps those destinations after placing try-delegates.
Fixes https://github.com/emscripten-core/emscripten/issues/13515.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D97178
show more ...
|
#
a08e609d |
| 21-Feb-2021 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Rename methods in WasmEHFuncInfo (NFC)
This renames variable and method names in `WasmEHFuncInfo` class to be simpler and clearer. For example, unwind destinations are EH pads by defin
[WebAssembly] Rename methods in WasmEHFuncInfo (NFC)
This renames variable and method names in `WasmEHFuncInfo` class to be simpler and clearer. For example, unwind destinations are EH pads by definition so it doesn't necessarily need to be included in every method name. Also I am planning to add the reverse mapping in a later CL, something like `UnwindDestToSrc`, so this renaming will make meanings clearer.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D97173
show more ...
|
#
da01a9db |
| 15-Feb-2021 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssemblly] Fix EHPadStack update in fixCallUnwindMismatches
Updating `EHPadStack` with respect to `TRY` and `CATCH` instructions have to be done after checking all other conditions, not before.
[WebAssemblly] Fix EHPadStack update in fixCallUnwindMismatches
Updating `EHPadStack` with respect to `TRY` and `CATCH` instructions have to be done after checking all other conditions, not before. Because we did this before checking other conditions, when we encounter `TRY` and we want to record the current mismatching range, we already have popped up the entry from `EHPadStack`, which we need to access to record the range.
The `baz` call in the added test needs try-delegate because the previous TRY marker placement for `quux` was placed before `baz`, because `baz`'s return value was stackified in RegStackify. If this wasn't stackified this try-delegate is not strictly necessary, but at the moment it is not easy to identify cases like this. I plan to transfer `nounwind` attributes from the LLVM IR to prevent cases like this. The call in the test does not have `unwind` attribute in order to test this bug, but in many cases of this pattern the previous call has `nounwind` attribute.
Reviewed By: tlively
Differential Revision: https://reviews.llvm.org/D96711
show more ...
|
#
35f5f797 |
| 11-Feb-2021 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssemblly] Fix rethrow's argument computation
Previously we assumed `rethrow`'s argument was always 0, but it turned out `rethrow` follows the same rule with `br` or `delegate`: https://github.c
[WebAssemblly] Fix rethrow's argument computation
Previously we assumed `rethrow`'s argument was always 0, but it turned out `rethrow` follows the same rule with `br` or `delegate`: https://github.com/WebAssembly/exception-handling/pull/137 https://github.com/WebAssembly/exception-handling/issues/146#issuecomment-777349038
Currently `rethrow`s generated by our backend always rethrow the exception caught by the innermost enclosing catch, so this adds a function to compute that and replaces `rethrow`'s argument with its computed result.
This also renames `EHPadStack` in `InstPrinter` to `TryStack`, because in CFGStackify we use `EHPadStack` to mean the range between `catch`~`end`, while in `InstPrinter` we used it to mean the range between `try`~`catch`, so choosing different names would look clearer. Doesn't contain any functional changes in `InstPrinter`.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D96595
show more ...
|
#
2968611f |
| 11-Feb-2021 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix delegate's argument computation
I previously assumed `delegate`'s immediate argument computation followed a different rule than that of branches, but we agreed to make it the same
[WebAssembly] Fix delegate's argument computation
I previously assumed `delegate`'s immediate argument computation followed a different rule than that of branches, but we agreed to make it the same (https://github.com/WebAssembly/exception-handling/issues/146). This removes the need for a separate `DelegateStack` in both CFGStackify and InstPrinter.
When computing the immediate argument, we use a different function for `delegate` computation because in MIR `DELEGATE`'s instruction's destination is the destination catch BB or delegate BB, and when it is a catch BB, we need an additional step of getting its corresponding `end` marker.
Reviewed By: tlively, dschuff
Differential Revision: https://reviews.llvm.org/D96525
show more ...
|
Revision tags: llvmorg-11.1.0, llvmorg-11.1.0-rc3, llvmorg-12.0.0-rc1, llvmorg-13-init, llvmorg-11.1.0-rc2, llvmorg-11.1.0-rc1 |
|
#
9f770b36 |
| 31-Dec-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix catch unwind mismatches
This fixes unwind destination mismatches caused by 'catch'es, which occur when a foreign exception is not caught by the nearest `catch` and the next outer `
[WebAssembly] Fix catch unwind mismatches
This fixes unwind destination mismatches caused by 'catch'es, which occur when a foreign exception is not caught by the nearest `catch` and the next outer `catch` is not the catch it should unwind to, or the next unwind destination should be the caller instead. This kind of mismatches didn't exist in the previous version of the spec, because in the previous spec `catch` was effectively `catch_all`, catching all exceptions.
Reviewed By: tlively
Differential Revision: https://reviews.llvm.org/D94049
show more ...
|
#
ed41945f |
| 28-Dec-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix call unwind mismatches
This adds `delegate` instruction and use it to fix unwind destination mismatches created by marker placement in CFGStackify.
There are two kinds of unwind d
[WebAssembly] Fix call unwind mismatches
This adds `delegate` instruction and use it to fix unwind destination mismatches created by marker placement in CFGStackify.
There are two kinds of unwind destination mismatches: - Mismatches caused by throwing instructions (here we call it "call unwind mismatches", even though `throw` and `rethrow` can also cause mismatches) - Mismatches caused by `catch`es, in case a foreign exception is not caught by the nearest `catch` and the next outer `catch` is not the catch it should unwind to. This kind of mismatches didn't exist in the previous version of the spec, because in the previous spec `catch` was effectively `catch_all`, catching all exceptions.
This implements routines to fix the first kind of unwind mismatches, which we call "call unwind mismatches". The second mismatch (catch unwind mismatches) will be fixed in a later CL.
This also reenables all previously disabled tests in cfg-stackify-eh.ll and updates FileCheck lines to match the new spec. Two tests were deleted because they specifically tested the way we fixed unwind mismatches before using `exnref`s and branches, which we don't do anymore.
Reviewed By: tlively
Differential Revision: https://reviews.llvm.org/D94048
show more ...
|
#
c93b9559 |
| 29-Dec-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Remove more unnecessary brs in CFGStackify
After placing markers, we removed some unnecessary branches, but it only handled the simplest case. This makes more unnecessary branches to b
[WebAssembly] Remove more unnecessary brs in CFGStackify
After placing markers, we removed some unnecessary branches, but it only handled the simplest case. This makes more unnecessary branches to be removed.
Reviewed By: dschuff, tlively
Differential Revision: https://reviews.llvm.org/D94047
show more ...
|
#
1cc52357 |
| 28-Dec-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Misc. refactoring in CFGStackify (NFC)
Updating `ScopeTops` is something we frequently do in CFGStackify, so this factors it out as a function. This also makes a few utility functions
[WebAssembly] Misc. refactoring in CFGStackify (NFC)
Updating `ScopeTops` is something we frequently do in CFGStackify, so this factors it out as a function. This also makes a few utility functions templated so that they are not dependent on input vector types and simplifies function parameters.
Reviewed By: tlively
Differential Revision: https://reviews.llvm.org/D94046
show more ...
|
#
9f8b2576 |
| 27-Dec-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Ensure terminate pads are a single BB
This ensures every single terminate pad is a single BB in the form of: ``` %exn = catch $__cpp_exception call @__clang_call_terminate(%exn) unreac
[WebAssembly] Ensure terminate pads are a single BB
This ensures every single terminate pad is a single BB in the form of: ``` %exn = catch $__cpp_exception call @__clang_call_terminate(%exn) unreachable ```
This is a preparation for HandleEHTerminatePads pass, which will be added in a later CL and will run after CFGStackify. That pass duplicates terminate pads with a `catch_all` instruction, and duplicating it becomes simpler if we can ensure every terminate pad is a single BB.
Reviewed By: dschuff, tlively
Differential Revision: https://reviews.llvm.org/D94045
show more ...
|
#
52e240a0 |
| 26-Dec-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Remove exnref and br_on_exn
This removes `exnref` type and `br_on_exn` instruction. This is effectively NFC because most uses of these were already removed in the previous CLs.
Review
[WebAssembly] Remove exnref and br_on_exn
This removes `exnref` type and `br_on_exn` instruction. This is effectively NFC because most uses of these were already removed in the previous CLs.
Reviewed By: dschuff, tlively
Differential Revision: https://reviews.llvm.org/D94041
show more ...
|
#
9e4eadeb |
| 26-Dec-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Update basic EH instructions for the new spec
This implements basic instructions for the new spec.
- Adds new versions of instructions: `catch`, `catch_all`, and `rethrow` - Adds supp
[WebAssembly] Update basic EH instructions for the new spec
This implements basic instructions for the new spec.
- Adds new versions of instructions: `catch`, `catch_all`, and `rethrow` - Adds support for instruction selection for the new instructions - `catch` needs a custom routine for the same reason `throw` needs one, to encode `__cpp_exception` tag symbol. - Updates `WebAssembly::isCatch` utility function to include `catch_all` and Change code that compares an instruction's opcode with `catch` to use that function. - LateEHPrepare - Previously in LateEHPrepare we added `catch` instruction to both `catchpad`s (for user catches) and `cleanuppad`s (for destructors). In the new version `catch` is generated from `llvm.catch` intrinsic in instruction selection phase, so we only need to add `catch_all` to the beginning of cleanup pads. - `catch` is generated from instruction selection, but we need to hoist the `catch` instruction to the beginning of every EH pad, because `catch` can be in the middle of the EH pad or even in a split BB from it after various code transformations. - Removes `addExceptionExtraction` function, which was used to generate `br_on_exn` before. - CFGStackfiy: Deletes `fixUnwindMismatches` function. Running this function on the new instruction causes crashes, and the new version will be added in a later CL, whose contents will be completely different. So deleting the whole function will make the diff easier to read. - Reenables all disabled tests in exception.ll and eh-lsda.ll and a single basic test in cfg-stackify-eh.ll. - Updates existing tests to use the new assembly format. And deletes `br_on_exn` instructions from the tests and FileCheck lines.
Reviewed By: dschuff, tlively
Differential Revision: https://reviews.llvm.org/D94040
show more ...
|
Revision tags: llvmorg-11.0.1, llvmorg-11.0.1-rc2 |
|
#
60653e24 |
| 30-Nov-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Support select and block for reference types
This adds missing `select` instruction support and block return type support for reference types. Also refactors WebAssemblyInstrRef.td and
[WebAssembly] Support select and block for reference types
This adds missing `select` instruction support and block return type support for reference types. Also refactors WebAssemblyInstrRef.td and rearranges tests in reference-types.s. Tests don't include `exnref` types, because we currently don't support `exnref` for `ref.null` and the type will be removed soon anyway.
Reviewed By: tlively, sbc100, wingo
Differential Revision: https://reviews.llvm.org/D92359
show more ...
|
Revision tags: llvmorg-11.0.1-rc1, llvmorg-11.0.0, llvmorg-11.0.0-rc6, llvmorg-11.0.0-rc5, llvmorg-11.0.0-rc4, llvmorg-11.0.0-rc3 |
|
#
d25c17f3 |
| 06-Sep-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix fixEndsAtEndOfFunction for try-catch
When the function return type is non-void and `end` instructions are at the very end of a function, CFGStackify's `fixEndsAtEndOfFunction` func
[WebAssembly] Fix fixEndsAtEndOfFunction for try-catch
When the function return type is non-void and `end` instructions are at the very end of a function, CFGStackify's `fixEndsAtEndOfFunction` function fixes the corresponding block/loop/try's type to match the function's return type. This is applied to consecutive `end` markers at the end of a function. For example, when the function return type is `i32`, ``` block i32 ;; return type is fixed to i32 ... loop i32 ;; return type is fixed to i32 ... end_loop end_block end_function ```
But try-catch is a little different, because it consists of two parts: a try part and a catch part, and both parts' return type should satisfy the function's return type. Which means, ``` try i32 ;; return type is fixed to i32 ... block i32 ;; this should be changed i32 too! ... end_block catch ... end_try end_function ``` As you can see in this example, it is not sufficient to only `end` instructions at the end of a function; in case of `try`, we should check instructions before `catch`es, in case their corresponding `try`'s type has been fixed.
This changes `fixEndsAtEndOfFunction`'s algorithm to use a worklist that contains a reverse iterator, each of which is a starting point for a new backward `end` instruction search.
Fixes https://bugs.llvm.org/show_bug.cgi?id=47413.
Reviewed By: dschuff, tlively
Differential Revision: https://reviews.llvm.org/D87207
show more ...
|
Revision tags: llvmorg-11.0.0-rc2 |
|
#
276f9e8c |
| 29-Jul-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix getBottom for loops
When it was first created, CFGSort only made sure BBs in each `MachineLoop` are sorted together. After we added exception support, CFGSort now also sorts BBs in
[WebAssembly] Fix getBottom for loops
When it was first created, CFGSort only made sure BBs in each `MachineLoop` are sorted together. After we added exception support, CFGSort now also sorts BBs in each `WebAssemblyException`, which represents a `catch` block, together, and `Region` class was introduced to be a thin wrapper for both `MachineLoop` and `WebAssemblyException`.
But how we compute those loops and exceptions is different. `MachineLoopInfo` is constructed using the standard loop computation algorithm in LLVM; the definition of loop is "a set of BBs that are dominated by a loop header and have a path back to the loop header". So even if some BBs are semantically contained by a loop in the original program, or in other words dominated by a loop header, if they don't have a path back to the loop header, they are not considered a part of the loop. For example, if a BB is dominated by a loop header but contains `call abort()` or `rethrow`, it wouldn't have a path back to the header, so it is not included in the loop.
But `WebAssemblyException` is wasm-specific data structure, and its algorithm is simple: a `WebAssemblyException` consists of an EH pad and all BBs dominated by the EH pad. So this scenario is possible: (This is also the situation in the newly added test in cfg-stackify-eh.ll)
``` Loop L: header, A, ehpad, latch Exception E: ehpad, latch, B ``` (B contains `abort()`, so it does not have a path back to the loop header, so it is not included in L.)
And it is sorted in this order: ``` header A ehpad latch B ```
And when CFGStackify places `end_loop` or `end_try` markers, it previously used `WebAssembly::getBottom()`, which returns the latest BB in the sorted order, and placed the marker there. So in this case the marker placements will be like this: ``` loop header try A catch ehpad latch end_loop <-- misplaced! B end_try ``` in which nesting between the loop and the exception is not correct. `end_loop` marker has to be placed after `B`, and also after `end_try`.
Maybe the fundamental way to solve this problem is to come up with our own algorithm for computing loop region too, in which we include all BBs dominated by a loop header in a loop. But this takes a lot more effort. The only thing we need to fix is actually, `getBottom()`. If we make it return the right BB, which means in case of a loop, the latest BB of the loop itself and all exceptions contained in there, we are good.
This renames `Region` and `RegionInfo` to `SortRegion` and `SortRegionInfo` and extracts them into their own file. And add `getBottom` to `SortRegionInfo` class, from which it can access `WebAssemblyExceptionInfo`, so that it can compute a correct bottom block for loops.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D84724
show more ...
|
Revision tags: llvmorg-11.0.0-rc1, llvmorg-12-init, llvmorg-10.0.1, llvmorg-10.0.1-rc4, llvmorg-10.0.1-rc3, llvmorg-10.0.1-rc2 |
|
#
83c26eae |
| 15-Jun-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Remove TEEs when dests are unstackified
When created in RegStackify pass, `TEE` has two destinations, where op0 is stackified and op1 is not. But it is possible that op0 becomes unstac
[WebAssembly] Remove TEEs when dests are unstackified
When created in RegStackify pass, `TEE` has two destinations, where op0 is stackified and op1 is not. But it is possible that op0 becomes unstackified in `fixUnwindMismatches` function in CFGStackify pass when a nested try-catch-end is introduced, violating the invariant of `TEE`s destinations.
In this case we convert the `TEE` into two `COPY`s, which will eventually be resolved in ExplicitLocals.
Reviewed By: dschuff
Differential Revision: https://reviews.llvm.org/D81851
show more ...
|
#
a5099ad9 |
| 10-Jun-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix a warning for an unused variable
`ErasedUncondBr` is used only in an `assert`, so it triggers a warning on builds without assertions. Fixed.
|
#
3fe6ea46 |
| 25-May-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix a bug in removing unnecessary branches
Summary: One of the things `removeUnnecessaryInstrs()` in CFGStackify does is to remove an unnecessary unconditinal branch before an EH pad.
[WebAssembly] Fix a bug in removing unnecessary branches
Summary: One of the things `removeUnnecessaryInstrs()` in CFGStackify does is to remove an unnecessary unconditinal branch before an EH pad. When there is an unconditional branch right before a catch instruction and it branches to the end of `end_try` marker, we don't need the branch, because it there is no exception, the control flow transfers to that point anyway. ``` bb0: try ... br bb2 <- Not necessary bb1: catch ... bb2: end ```
This applies when we have a conditional branch followed by an unconditional one, in which case we should only remove the unconditional branch. For example: ``` bb0: try ... br_if someplace_else br bb2 <- Not necessary bb1: catch ... bb2: end ```
But `TargetInstrInfo::removeBranch` we used removed all existing branches when there are multiple ones. This patch fixes it by only deleting the last (= unconditional) branch manually.
Also fixes some `preds` comments in the test file.
Reviewers: dschuff
Subscribers: sbc100, jgravelle-google, hiraditya, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D80572
show more ...
|
#
fe0006c8 |
| 23-May-2020 |
Simon Pilgrim <llvm-dev@redking.me.uk> |
TargetLowering.h - remove unnecessary TargetMachine.h include. NFC
Replace with forward declaration and move dependency down to source files that actually need it.
Both TargetLowering.h and TargetM
TargetLowering.h - remove unnecessary TargetMachine.h include. NFC
Replace with forward declaration and move dependency down to source files that actually need it.
Both TargetLowering.h and TargetMachine.h are 2 of the most expensive headers (top 10) in the ClangBuildAnalyzer report when building llc.
show more ...
|
Revision tags: llvmorg-10.0.1-rc1 |
|
#
834debff |
| 30-Apr-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix block marker placing after fixUnwindMismatches
Summary: This fixes a few things that are connected. It is very hard to provide an independent test case for each of those fixes, bec
[WebAssembly] Fix block marker placing after fixUnwindMismatches
Summary: This fixes a few things that are connected. It is very hard to provide an independent test case for each of those fixes, because they are interconnected and sometimes one masks another. The provided test case triggers some of those bugs below but not all.
---
1. Background: `placeBlockMarker` takes a BB, and if the BB is a destination of some branch, it places `end_block` marker there, and computes the nearest common dominator of all predecessors (what we call 'header') and places a `block` marker there.
When we first place markers, we traverse BBs from top to bottom. For example, when there are 5 BBs A, B, C, D, and E and B, D, and E are branch destinations, if mark the BB given to `placeBlockMarker` with `*` and draw a rectangle representing the border of `block` and `end_block` markers, the process is going to look like ``` ------- ----- |-----| --- |---| ||---|| |A| ||A|| |||A||| --- --> |---| --> ||---|| *B | B | || B || C | C | || C || D ----- |-----| E *D | D | E ------- *E ``` which means when we first place markers, we go from inner to outer scopes. So when we place a `block` marker, if the header already contains other `block` or `try` marker, it has to belong to an inner scope, so the existing `block`/`try` markers should go _after_ the new marker. This was the assumption we had.
But after placing all markers we run `fixUnwindMismatches` function. There we do some control flow transformation and create some branches, and we call `placeBlockMarker` again to place `block`/`end_block` markers for those newly created branches. We can't assume that we are traversing branch destination BBs from top to bottom now because we are basically inserting some new markers in the middle of existing markers.
Fix: In `placeBlockMarker`, we don't have the assumption that the BB given is in the order of top to bottom, and when placing `block` markers, calculates whether existing `block` or `try` markers are inner or outer scopes with respect to the current scope.
---
2. Background: In `fixUnwindMismatches`, when there is a call whose correct unwind destination mismatches the current destination after initially placing `try` markers, we wrap that with a new nested `try`/`catch`/`end` and jump to the correct handler within the new `catch`. The correct handler code is split as a separate BB from its original EH pad so it can be branched to. Here's an example:
- Before ``` mbb: call @foo <- Unwind destination mismatch! wrong-ehpad: catch ... cont: end_try ... correct-ehpad: catch [handler code] ```
- After ``` mbb: try (new) call @foo nested-ehpad: (new) catch (new) local.set n / drop (new) br %handleri (new) nested-end: (new) end_try (new) wrong-ehpad: catch ... cont: end_try ... correct-ehpad: catch local.set n / drop (new) handler: (new) end_try [handler code] ```
Note that after this transformation, it is possible there are no calls to actually unwind to `correct-ehpad` here. `call @foo` now branches to `handler`, and there can be no other calls to unwind to `correct-ehpad`. In this case `correct-ehpad` does not have any predecessors anymore.
This can cause a bug in `placeBlockMarker`, because we may need to place `end_block` marker in `handler`, and `placeBlockMarker` computes the nearest common dominator of all predecessors. If one of `handler`'s predecessor (here `correct-ehpad`) does not have any predecessors, i.e., no way of reaching it, we cannot correctly compute the common dominator of predecessors of `handler`, and end up placing no `block`/`end` markers. This bug actually sometimes masks the bug 1.
Fix: When we have an EH pad that does not have any predecessors after this transformation, deletes all its successors, so that its successors don't have any dangling predecessors.
---
3. Background: Actually the `handler` BB in the example shown in bug 2 doesn't need `end_block` marker, despite it being a new branch destination, because it already has `end_try` marker which can serve the same purpose. I just put that example there for an illustration purpose. There is a case we actually need to place `end_block` marker: when the branch dest is the appendix BB. The appendix BB is created when there is a call that is supposed to unwind to the caller ends up unwinding to a wrong EH pad. In this case we also wrap the call with a nested `try`/`catch`/`end`, create an 'appendix' BB at the very end of the function, and branch to that BB, where we rethrow the exception to the caller.
Fix: When we don't actually need to place block markers, we don't.
---
4. In case we fall through to the continuation BB after the catch block, after extracting handler code in `fixUnwindMismatches` (refer to bug 2 for an example), we now have to add a branch to it to bypass the handler. - Before ``` try ... (falls through to 'cont') catch handler body end <-- cont ```
- After ``` try ... br %cont (new) catch end handler body <-- cont ```
The problem is, we haven't been placing a new `end_block` marker in the `cont` BB in this case. We should, and this fixes it. But it is hard to provide a test case that triggers this bug, because the current compilation pipeline from .ll to .s does not generate this kind of code; we always have a `br` after `invoke`. But code without `br` is still valid, and we can have that kind of code if we have some pipeline changes or optimizations later. Even mir test cases cannot trigger this part for now, because we don't encode auxiliary EH-related data structures (such as `WasmEHFuncInfo`) in mir now. Those functionalities can be added later, but I don't think we should block this fix on that.
Reviewers: dschuff
Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D79324
show more ...
|
#
ba40896f |
| 10-Apr-2020 |
Heejin Ahn <aheejin@gmail.com> |
[WebAssembly] Fix try placement in fixing unwind mismatches
Summary: In CFGStackify, `fixUnwindMismatches` function fixes unwind destination mismatches created by `try` marker placement. For example
[WebAssembly] Fix try placement in fixing unwind mismatches
Summary: In CFGStackify, `fixUnwindMismatches` function fixes unwind destination mismatches created by `try` marker placement. For example, ``` try ... call @qux ;; This should throw to the caller! catch ... end ``` When `call @qux` is supposed to throw to the caller, it is possible that it is wrapped inside a `catch` so in case it throws it ends up unwinding there incorrectly. (Also it is possible `call @qux` is supposed to unwind to another `catch` within the same function.)
To fix this, we wrap this inner `call @qux` with a nested `try`-`catch`-`end` sequence, and within the nested `catch` body, branch to the right destination: ``` block $l0 try ... try ;; new nested try call @qux catch ;; new nested catch local.set n ;; store exnref to a local br $l0 end catch ... end end local.get n ;; retrieve exnref back rethrow ;; rethrow to the caller ```
The previous algorithm placed the nested `try` right before the `call`. But it is possible that there are stackified instructions before the call from which the call takes arguments. ``` try ... i32.const 5 call @qux ;; This should throw to the caller! catch ... end ```
In this case we have to place `try` before those stackified instructions. ``` block $l0 try ... try ;; this should go *before* 'i32.const 5' i32.const 5 call @qux catch local.set n br $l0 end catch ... end end local.get n rethrow ```
We correctly handle this in the first normal `try` placement phase (`placeTryMarker` function), but failed to handle this in this `fixUnwindMismatches`.
Reviewers: dschuff
Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D77950
show more ...
|
Revision tags: llvmorg-10.0.0, llvmorg-10.0.0-rc6, llvmorg-10.0.0-rc5, llvmorg-10.0.0-rc4, llvmorg-10.0.0-rc3, llvmorg-10.0.0-rc2, llvmorg-10.0.0-rc1, llvmorg-11-init, llvmorg-9.0.1, llvmorg-9.0.1-rc3, llvmorg-9.0.1-rc2, llvmorg-9.0.1-rc1 |
|
#
904cd3e0 |
| 19-Oct-2019 |
Reid Kleckner <rnk@google.com> |
Prune a LegacyDivergenceAnalysis and MachineLoopInfo include each
Now X86ISelLowering doesn't depend on many IR analyses.
llvm-svn: 375320
|
#
2cb27072 |
| 15-Oct-2019 |
Thomas Lively <tlively@google.com> |
[WebAssembly] Allow multivalue types in block signature operands
Summary: Renames `ExprType` to the more apt `BlockType` and adds a variant for multivalue blocks. Currently non-void blocks are only
[WebAssembly] Allow multivalue types in block signature operands
Summary: Renames `ExprType` to the more apt `BlockType` and adds a variant for multivalue blocks. Currently non-void blocks are only generated at the end of functions where the block return type needs to agree with the function return type, and that remains true for multivalue blocks. That invariant means that the actual signature does not need to be stored in the block signature `MachineOperand` because it can be inferred by `WebAssemblyMCInstLower` from the return type of the parent function. `WebAssemblyMCInstLower` continues to lower block signature operands to immediates when possible but lowers multivalue signatures to function type symbols. The AsmParser and Disassembler are updated to handle multivalue block types as well.
Reviewers: aheejin, dschuff, aardappel
Subscribers: sbc100, jgravelle-google, hiraditya, sunfish, llvm-commits
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D68889
llvm-svn: 374933
show more ...
|