Wasmtime GitHub notifications bot (Dec 07 2022 at 06:53): Wasmtime GitHub notifications bot (Dec 07 2022 at 06:53):jameysharp opened PR #5392 from isle-partial-flag to main:
I speculate that this PR may have a performance impact, which I want to measure before merging, but I'd appreciate review on the implementation. Also I wanted to get the CI run going overnight.
Instead of tying fallibility of constructors to whether they're either internal or pure, this commit assumes all constructors are infallible unless tagged otherwise with a "partial" flag.
Internal constructors without the "partial" flag are not allowed to use constructors which have the "partial" flag on the right-hand side of any rules, because they have no way to report last-minute match failures.
Multi-constructors should never be "partial"; they report match failures with an empty iterator instead. In turn this means you can't use partial constructors on the right-hand side of internal multi-constructor rules. However, you can use the same constructors on the left-hand side with
iforif-letinstead.In many cases, ISLE can already trivially prove that an internal constructor always returns
Some. With this commit, those cases are laregly unchanged, except for removing all theOptions andSomes from the generated code for those terms.However, for internal non-partial constructors where ISLE could not prove that, it now emits an
unreachable!panic as the last-resort, instead of returningNonelike it used to do. Among the existing backends, here's how many constructors have these panic cases:
- x64: 14% (53/374)
- aarch64: 15% (41/277)
- riscv64: 23% (26/114)
- s390x: 47% (268/567)
It's often possible to rewrite rules so that ISLE can tell the panic can never be hit. Just ensure that there's a lowest-priority rule which has no constraints on the left-hand side.
But in many of these constructors, it's difficult to statically prove the unhandled cases are unreachable because that's only down to knowledge about how they're called or other preconditions.
So this commit does not try to enforce that all terms have a last-resort fallback rule.
Wasmtime GitHub notifications bot (Dec 07 2022 at 06:53):jameysharp requested cfallin for a review on PR #5392.
Wasmtime GitHub notifications bot (Dec 07 2022 at 19:44):jameysharp updated PR #5392 from isle-partial-flag to main.
Wasmtime GitHub notifications bot (Dec 07 2022 at 22:59):jameysharp updated PR #5392 from isle-partial-flag to main.
Wasmtime GitHub notifications bot (Dec 07 2022 at 23:03):jameysharp updated PR #5392 from isle-partial-flag to main.
Wasmtime GitHub notifications bot (Dec 07 2022 at 23:06):jameysharp has marked PR #5392 as ready for review.
Wasmtime GitHub notifications bot (Dec 08 2022 at 00:08):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Dec 08 2022 at 01:16):jameysharp merged PR #5392.
Last updated: Dec 23 2024 at 12:05 UTC