Isle printer #101
Closed
Isle printer #101
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…bytecodealliance#7892) * Refactor `wasmtime::FuncType` to hold a handle to its registered type Rather than holding a copy of the type directly, it now holds a `RegisteredType` which internally is * A `VMSharedTypeIndex` pointing into the engine's types registry. * An `Arc` handle to the engine's type registry. * An `Arc` handle to the actual type. The last exists only to keep it so that accessing a `wasmtime::FuncType`'s parameters and results fast, avoiding any new locking on call hot paths. This is helping set the stage for further types and `TypeRegistry` refactors needed for Wasm GC. * Update the C API for the function types refactor prtest:full * rustfmt * Fix benches build
* Add the host header to the set of forbidden headers * Test that the host header is forbidden
Additionally give `-Zcheck-cfg` a spin with Cargo and fix some mistakes with our `--cfg` and `#[cfg]` directives.
* x64: Refactor multiplication instructions This commit is inspired after reading over some code from bytecodealliance#7865 and bytecodealliance#7866. The goal of this commit was to refactor scalar multiplication-related instructions in the x64 backend to more closely align with their native instructions. Changes include: * The `MulHi` instruction is renamed to `Mul`. This represents either `mul` or `imul` producing a doublewide result. * A `Mul8` instruction was added to correspond to `Mul` for the 8-bit variants that produce a doublewide result in the `AX` register rather than the other instructions which split between `RAX` and `RDX`. * The `UMulLo` instruction was removed as now it's covered by `Mul` * The `AluRmiROpcode::Mul` opcode was removed in favor of new `IMul` and `IMulImm` instructions. Register allocation and emission already had special cases for `Mul` which felt better as standalone instructions rather than putting in an existing variant. Lowerings using `imul` are not affected in general but the `IMulImm` instruction has different register allocation behavior than before which allows the destination to have a different register than the first operand. The `umulhi` and `smulhi` instructions are also reimplemented with their 8-bit variants instead of extension-plus-16-bit variants. * Remove outdated emit tests These are all covered by the filetests framework now too. * Fix Winch build
This commit adds support for the saturating conversions instructions.
* vet: prune lockfile When running `cargo vet` in bytecodealliance#7900, it warned me that we should consider pruning some unused entries, etc. This is the result of running `cargo vet prune`. * vet: a couple more post-bytecodealliance#7908
This fixes the build on the latest nightly Rust. I was able to vet `ahash` itself but `zerocopy` is such a large and full-of-unsafe dependency I've added an exemption for it. The documentation of it seems to indicate it's a pretty well thought out crate with lots of care behind it, so at least at a first glance it did not seem overly worrisome.
… changes (bytecodealliance#7895) * Use tokio's built in methods to perform the new/bind/listen/accept/connect progression. * cargo fmt * Fix MacOS * Don't spawn a task
* Test with aarch64 macOS on CI Rebase of bytecodealliance#7129 with the `macos-14` string which GitHub indicates should now work for public repos. prtest:full * Fix syntax
…iance#7746) * unionfind: robustly avoid changing `Idx`s in the GVN map Stop hoping that keeping the smallest Idx as canonical will yield good results, and instead explicitly inform the UnionFind of what we expect not to move. Fixes bytecodealliance#6126 * unionfind: track unions of pinned indices as stats Emitting a warning in this situation is too much.
This commit is born out of a fuzz bug on x64 that was discovered recently.
Today, on `main`, and in the 17.0.1 release Wasmtime will panic when compiling
this wasm module for x64:
(module
(func (result v128)
i32.const 0
i32x4.splat
f64x2.convert_low_i32x4_u))
panicking with:
thread '<unnamed>' panicked at /home/alex/.cargo/registry/src/index.crates.io-6f17d22bba15001f/cranelift-codegen-0.104.1/src/machinst/lower.rs:766:21:
should be implemented in ISLE: inst = `v6 = fcvt_from_uint.f64x2 v13 ; v13 = const0`, type = `Some(types::F64X2)`
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Bisections points to the "cause" of this regression as bytecodealliance#7859 which
more-or-less means that this has always been an issue and that PR just
happened to expose the issue. What's happening here is that egraph
optimizations are turning the IR into a form that the x64 backend can't
codegen. Namely there's no general purpose lowering of i64x2 being
converted to f64x2. The Wasm frontend never produces this but the
optimizations internally end up producing this.
Notably here the result of this function is constant and what's
happening is that a convert-of-a-splat is happening. In lieu of adding
the full general lowering to x64 (which is perhaps overdue since this is
the second or third time this panic has been triggered) I've opted to
add constant propagation optimizations for int-to-float conversions.
These are all based on the Rust `as` operator which has the same
semantics as Cranelift. This is enough to fix the issue here for the
time being.
…nce#7790) Signed-off-by: Archisman Mridha <archismanmridha12345@gmail.com>
This commit adds a general purpose lowering for the `fcvt_from_uint` instruction in addition to the previously specialized lowering for what the wasm frontend produces. This is unlikely to get triggered much from the wasm frontend except when intermediate optimizations change the shape of code. The goal of this commit is to address issues such as those identified in bytecodealliance#7915 and historically by ensuring that there's a lowering for the instruction for all input types instead of trying to massage the input into the right form. This instruction lowering was crafted by studying LLVM's output and I've put commentary to the best of my ability as to what's going on.
Rather than an `Arc<RwLock<TypeRegistryInner>>`. This also removes the `Arc` inside `TypeRegistry` and we effectively reuse `Engine`'s internal `Arc` instead. Also, to avoid various `TypeRegistry` methods needing to take an `Engine` to construct their `RegisteredType` results, but then needing to assert that the given engine is this registry's engine, I turned the methods into `TypeRegistry` constructors. These constructors take just an `&Engine` and then access the underlying `TypeRegistry` as needed, effectively making that property hold statically. This is a minor clean up on its own, but is a big help for follow up work I am doing for Wasm GC and typed function references, where being able to grab a reference to the engine that a `FuncType` is registered within will prevent needing to thread in additional engine parameters to various places and then assert that the engine is the engine that the type is registered within, and etc...
…#7902) * Perform stronger typechecks of host-owned resources Right now the abstraction for host-owned resources in Wasmtime is quite simple, it's "just an index". This can be error prone because the host can suffer both from use-after-free and ABA-style problems. While there's not much that can be done about use-after-free the previous implementation would accidentally enable "AB" style problems where if a host-owned resource index was deallocated and then reallocated with another type the original handle would still work. While not a major bug this can be confusing and additionally violate's our guarantees as a component runtime to guests to ensure that resources always have valid `rep`s going into components. This commit adds a new layer of storage which keeps track of the `ResourceType` for all host-owned resources. This means that resources going into a host-owned table now record their type and coming out of a host-owned table a typecheck is always performed. Note that guests can continue to skip this logic as they already have per-type tables and so won't suffer the same issue. This change is inspired by my taking a look at bytecodealliance#7883. The crux of the issue was a typo where a resource was reused by accident which led to confusing behavior due to the reuse. This change instead makes it return an error more quickly and doesn't allow the component to see any invalid state. Closes bytecodealliance#7883 * Fix test * Use generation counters, not types * Review comments
* Update the wasm-tools family of crates Pulling in some updates to improve how WIT is managed in this repository. No changes just yet, however, just pulling in the updates first. * Fix tests * Fix fuzzer build
…sions (bytecodealliance#7876) * fd_filestat_set test: assert that a file descriptor behaves the same... whether opened readonly or readwrite. This test now reproduces the behavior reported in bytecodealliance#7829 Co-authored-by: Trevor Elliott <telliott@fastly.com> * preview1_path_link test: refactor; create and open file separately we create and open the file with separate descriptors because the creation descriptor will always imply writing, whereas the rest do not. there were a number of auxillary functions in here that were obscuring what was going on, and making errors harder to read, so i changed some assertions to use macro-rules so the panic message had the relevant line number, and i inlined the creation / opening functions. * preview2 filesystem: track open mode instead of reporting permissions it n DescriptorFlags FilePerms/DirPerms is a way of having wasmtime-wasi, instead of the operating system, mediate permissions on a preview2 Descriptor. This was conflated with the open mode, which should determine whether DescriptorFlags contains READ or WRITE or MUTATE_DIRECTORY. We no longer mask FilePerms with the DescriptorFlags (oflags) in open_at - instead, track what open mode is requested, and see if the file and directory permissions permit it. Additionally, File and Dir now track their open_mode (represented using OpenMode, which just like FilePerms is just a read and write bit), and report that in DescriptorFlags. Previously, we reported the FilePerms or DirPerms in the DescriptorFlags, which was totally wrong. Co-authored-by: Trevor Elliott <telliott@fastly.com> * different error on windows i guess? prtest:full * apparently set-times of read-only is rejected on windows. just skip testing that * path open read write: another alternate error code for windows * wasi-common actually gives a badf, so accept either * this case too --------- Co-authored-by: Trevor Elliott <telliott@fastly.com>
This commit fixes an issue with the proxy adapter where if the proxy program attempted to look at prestat items, which wasi-libc always does on startup, it would invoke `fd_prestat_get` and receive `ERRNO_NOTSUP` in response (the standard response when using the `cfg_filesystem_available!` macro). This error code is unexpected by wasi-libc and causes wasi-libc to abort. The PR here is to instead return `ERRNO_BADF` which is indeed expected by wasi-libc and is recognized as meaning "that prestat isn't available".
…bytecodealliance#7881) * WIP: try to make wasi-common self contained. * rebase: cargo.lock * remove all dependencies between wasi-common and wasmtime-wasi * use wasi-common directly throughout tests, benches, examples, cli run * wasi-threads: use wasi-common's maybe_exit_on_error in spawned thread not a very modular design, but at this point wasi-common and wasi-threads are forever wed * fix wasmtime's docs * re-introduce wasmtime-wasi's exports of wasi-common definitions behind deprecated * factor out determining i32 process exit code and remove libc dep because rustix provides the same constant * commands/run: inline the logic about aborting on trap since this is the sole place in the codebase its used * Add high-level summary to wasi-common's top-level doc comment. * c-api: fix use of wasi_cap_std_sync => wasi_common::sync, wasmtime_wasi => wasi_common * fix tokio example * think better of combining downcast and masking into one method * fix references to wasmtime_wasi in docs prtest:full * benches: use wasi-common * cfg-if around use of rustix::process because that doesnt exist on windows * wasi-common: include tests, caught by verify-publish * fix another bench * exit requires wasmtime dep. caught by verify-publish.
Use a new `wit_parser::Resolve::push_path` API instead of a combo of `push_dir` and `UnresolvedPackage` which handles the wasm format internally and otherwise is a superset of the prior functionality.
…dealliance#7922) * Remove the use of the union-find structure during elaboration Remove the UnionFind argument to `Elaborator::new`, and from the `Elaborator` structure, relying instead on the `value_to_best_value` table when computing canonical values. Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: L. Pereira <lpereira@fastly.com> * Document the eclass union subtree invariant --------- Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: L. Pereira <lpereira@fastly.com>
This is to get bytecodealliance#7638 passing CI, but I wanted to separate this out from that to have it go through the queue in isolation in case any issues arise.
* Discard 0-sized writes to files This commit comes from bytecodealliance#7633 where Windows and Unix would behave differently when writing at a particular file offset. Notably Unix semantics [indicate]: > Before any action described below is taken, and if nbyte is zero > and the file is a regular file, the write() function may detect > and return errors as described below. In the absence of errors, > or if error detection is not performed, the write() function > shall return zero and have no other results. If nbyte is zero and > the file is not a regular file, the results are unspecified. These semantics are a bit easier to emulate on Windows so the host implementation now discards any attempt to perform I/O if a zero-sized write is detected. [indicate]: https://man7.org/linux/man-pages/man3/write.3p.html Closes bytecodealliance#7633 * Discard empty writes in wasi-common as well
* winch: Optimize calls This commit introduces several optimizations to speed up the compilation of function calls: * Keep track of previously resolved function signatures for local or imported callees to avoid computing the `ABISig` on every function call. * Keep track of previously resolved type signatures for indirect calls to avoid computing the `ABISig` on every function call. * Refactor `CallKnown` and `CallUnknown` instructions to make the `BoxCallInfo` field in the struct optional. Prior to this change, from Winch's perspective each call lowering involved a heap allocation, using the default values for `BoxCallInfo`, which in the end are not used by Winch. * Switch Winch's internal `Stack` to use a `SmallVec` rather than a `Vec`. Many of the operations involving builtin function calls require inserting elements at arbitrary offsets in the stack and using a `SmallVec` makes this process more efficient. With the changes mentioned above, I observed ~30% improvement in compilation times for modules that are call-heavy. * Expect `CallInfo` where applicable and add a comment about the type definition * Remove unneeded types and lifetimes
This commit fully enables usage of Winch in the `differential` fuzzer against all other engines with no special cases. I attempted enabling winch for the other fuzzers as well but Winch doesn't currently implement all methods for generating various trampolines required so it's currently only limited to the `differential` fuzzer. This adds Winch as an "engine" and additionally ensures that when configured various wasm proposals are disabled that Winch doesn't support (similar to how enabling `wasmi` disables proposals that `wasmi` doesn't support). This does reduce fuzzing of Winch slightly in that the reference-types proposal is completely disabled for Winch rather than half-enabled where Winch doesn't implement `externref` operations yet but does implement `funcref` operations. This, however, enables integrating it more cleanly into the rest of the fuzzing infrastructure with fewer special cases.
This commit fixes a bug that was introduced in bytecodealliance#8190 and brought up on [Zulip]. In bytecodealliance#8190 there was a subtle change in how Tokio and the CLI are managed, namely that a Tokio runtime is installed at the start of the CLI to avoid entering/exiting the runtime on each blocking call. This had a small performance improvement relative to entering/exiting on each blocking call. This meant, though, that calls previously blocked with `Runtime::block_on` and now block with `Handle::block_on`. The [documentation of `Handle::block_on`][doc] has a clause that says that for single-threaded runtimes I/O and timers won't work. To fix this issue I've switch the fallback runtime to a multi-threaded runtime to ensure that I/O and timers continue to work. [Zulip]: https://bytecodealliance.zulipchat.com/#narrow/stream/219900-wasi/topic/Wasi-http.20requests.20hang/near/429187256 [doc]: https://docs.rs/tokio/latest/tokio/runtime/struct.Handle.html#method.block_on
…bytecodealliance#8235) This commit slightly improves the error message of feeding a component into a module-taking API. This at least tries to convey that a component was recognized but a module was expected.
…liance#8236) This provides a bit of a nicer experience than the default "build your own test harness" experience by providing things like filters and parallel execution by default. This helps speed up the `disas` test suite, for example, which previously had no parallelism.
) This is like the DataFlowGraph::map_inst_values method, but that method mutably borrows `self`, so it goes to a fair bit of trouble to drop its borrows around callbacks so the mutable borrow can be used in the callback too. This new helper only actually needs to borrow two public fields from the DFG: `value_lists` and `jump_tables`. Therefore it's possible to use other fields in the callback as long as the compiler can see all the fields being used in the same scope. That's good enough for everywhere we were using this pattern, so we can simplify this way. The case which motivated this change isn't shown here: It isn't possible to call `dfg.map_inst_values` on every instruction in `dfg.insts`, because the borrow on `dfg.insts` prevents taking a mutable borrow on `dfg`. But we can call this new helper in that case.
…liance#8238) And demonstrate its use in bugpoint. That doesn't have much impact on anything, which I hope makes this easier to review than changing more important things right away. There are other places we would probably be better off doing a whole-function rewrite once rather than calling `resolve_aliases` a bunch of times, such as the disas tests, the egraph pass, and lowering. I have not changed those in this PR.
…8206) This allows optimization to reorder and combine these loads, even across block boundaries and across other store instructions. Fixes bytecodealliance#8200
This reverts commit 6c51848, "Cranelift: resolve value aliases when printing CLIF functions (bytecodealliance#8214)", then applies the same effect a different way. In discussion on bytecodealliance#8223, we decided to handle this a different way. So I've introduced a method on DataFlowGraph which eliminates all value aliases, and we can call it when necessary. If that function is not called then the CLIF printer should print value aliases the same way it did before bytecodealliance#8214. In this PR, I've specifically called it in disas tests. The changes to write.rs and frontend.rs are from the revert, while the changes to disas.rs are new. In these tests, value aliases are just clutter that distracts from understanding what code will actually be generated. They may change due to changes in legalization, but that doesn't signal anything about whatever the test was intended to check. Because the new helper deletes aliases after it's done resolving them, those aliases now disappear from the test expectations. Aside from that, the test expectations are unchanged compared to what bytecodealliance#8214 did.
* Disassemble `*.cwasm` for `compile` disas tests This commit changes how the `compile` mode of the `disas` test suite works. Previously this would use `--emit-clif` and run the Cranelift pipeline for each individual function and use the custom VCode-based disassembly for instruction output. This commit instead uses the raw binary coming out of Wasmtime. The ELF file itself is parsed and is disassembled in a manner similar to Winch tests. The goal of this commit is somewhat twofold: * Lay the groundwork to migrate all Winch-based filetests to `tests/disas`. * Test the raw output from Cranelift/Wasmtime which includes optimizations like branch chomping in the `MachBuffer`. This commit doesn't itself move the Winch tests yet, that's left for a future commit. * Update all test expectations for new output * Fix PR-based CI when too many files are changed
Signed-off-by: Roman Volosatovs <rvolosatovs@riseup.net>
* Switch Winch tests to ATT syntax * Update all test expectations * Move all winch tests to `disas` folder * Add `test = "winch"` to `disas` * Add `test = "winch"` to all winch test files * Stub out bits to get AArch64 Winch tests working * Update expectations for all aarch64 winch tests * Update flags in Winch tests Use CLI syntax as that's what `flags` was repurposes as in the new test suite. * Update all test expectations for x64 winch * Omit more offsets by default * Delete now-dead code * Update an error message * Update non-winch test expectations
With all Winch tests moved to `tests/disas` in bytecodealliance#8243 plus the support of `wasmtime compile -C compiler=winch` this tool should in theory be supplanted nowadays with other alternatives. This commit removes the executable and the `winch-filetests` support.
Use `(either ..)` to assert one of the two possible results.
…ecodealliance#8246) * Add GrowFrame and ShrinkFrame instructions for moving the frame Co-authored-by: Jamey Sharp <jsharp@fastly.com> * Experimentally emit grow/shrink frame instructions for x64 tail calls Co-authored-by: Jamey Sharp <jsharp@fastly.com> * Reuse the epilogue generation functions for tail call emission Instead of building and copying the new frame over the old one, make use of the frame shrink/grow pseudo-instructions to move the frame, and then reuse the existing epilogue generation functions to setup the tail call. Co-authored-by: Jamey Sharp <jsharp@fastly.com> * Enable callee saves with the tail calling convention on x64 Co-authored-by: Jamey Sharp <jsharp@fastly.com> * Remove the requirement that indirect calls go through r15 with the tail cc * Stop using r14 for a temporary during the stack check with the tail cc * Apply suggestions from code review Co-authored-by: Jamey Sharp <jamey@minilop.net> * Remove constants in favor of reusing values computed for FrameLayout Co-authored-by: Jamey Sharp <jsharp@fastly.com> * Suggestions from review * Rename the grow/shrink frame instructions, and adjust their comments * Comments on ArgLoc * Add more tests for return_call, and fix grow/shrink arg area printing --------- Co-authored-by: Jamey Sharp <jsharp@fastly.com> Co-authored-by: Jamey Sharp <jamey@minilop.net>
* Add documentation and examples for `wasmtime-wasi`
This commit adds lots of missing documentation and examples to top-level
types in `wasmtime-wasi`, mostly related to WASIp2. I've additionally
made a number of small refactorings here to try to make the APIs a bit
more straightforward and symmetric and simplify where I can.
* Remove `bindings::wasi` (reexports are still present under `bindings`)
* Rename `bindings::sync_io` to `bindings::sync`
* Generate fewer bindings in `bindings::sync` that can be pulled in from
the `bindings` module.
* Change `WasiCtxBuilder::preopened_dir` to take a path instead of a
`Dir` argument to avoid the need for another import.
* Synchronize `wasmtime_wasi_http::{add_to_linker, sync::add_to_linker}`
in terms of interfaces added.
* Remove `wasmtime_wasi::command` and move the generated types to the
`bindings` module.
* Move top-level add-to-linker functions to
`wasmtime_wasi::add_to_linker_sync` and
`wasmtime_wasi::add_to_linker_async`.
Closes bytecodealliance#8187
Closes bytecodealliance#8188
* Add documentation for `wasmtime_wasi::preview1` and refactor
This commit adds documentation for the `wasmtime_wasi::preview1` module
and additionally refactors it as well. Previously this was based on a
similar design as WASIp2 with a "view trait" and various bits and
pieces, but the design constraints of WASIp1 lends itself to a simpler
solution of "just" passing around `WasiP1Ctx` instead. This goes back to
what `wasi-common` did of sorts where the `add_to_linker_*` functions
only need a projection from `&mut T` to `&mut WasiP1Ctx`, a concrete
type, which simplifies the module and usage.
* Small refactorings to `preopened_dir`
* Add `WasiCtx::builder`.
* Fix typo
* Review comments
This commit fixes a mistake in bytecodealliance#8181 which meant that the caching for components was no longer working. The mistake is fixed in this commit as well as a new test being added too.
* Plumb coredump feature to `wasmtime-runtime` The `wasmtime` crate already has a `coredump` feature but whether or not it's enabled the `wasmtime-runtime` crate still captures a core dump. Use this flag in the `wasmtime` crate to plumb support to `wasmtime-runtime` to skip capture if it's not enabled. * Fix a typo
…ytecodealliance#8253) Sometimes, when in the course of silly optimizations to make the most of one's registers, one might want to pack two `i64`s into one `v128`, and one might want to do it without any loads or stores. In clang targeting Wasm at least, building an `i64x2` (with `wasm_i64x2_make(a, b)` from `<wasm_simd128.h>`) will generate (i) an `i64x2.splat` to create a new v128 with lane 0's value in both lanes, then `i64x2.replace_lane` to put lane 1's value in place. Or, in the case that one of the lanes is zero, it will generate a `v128.const 0` then insert the other lane. Cranelift's lowerings for both of these patterns on x64 are slightly less optimal than they could be. - For the former (replace-lane of splat), the 64-bit value is moved over to the XMM register, then the rest of the `splat` semantics are implemented by a `pshufd` (shuffle), even though we're just about to overwrite the only other lane. We could omit that shuffle instead, and everything would work fine. This optimization is specific to `i64x2` (that is, only two lanes): we need to know that the only other lane that the `splat` is splatting into is overwritten. We could in theory match a chain of replace-lane operators for higher-lane-count types, but let's save that for the case that we actually need it later. - For the latter (replace-lane of constant zero), the load of a constant zero from the constant pool is the part that bothers me most. While I like zeroed memory as much as the next person, there is a vector XOR instruction *right there* under our noses, and we'd be silly not to use it. This applies to any `vconst 0`, not just ones that occur as a source to replace-lane.
Signed-off-by: Ryan Levick <ryan.levick@fermyon.com>
* Add a `compile` feature to `wasmtime-environ` This commit adds a compile-time feature to remove some dependencies of the `wasmtime-environ` crate. This compiles out support for compiling modules/components and makes the crate slimmer in terms of amount of code compiled along with its dependencies. Much of this should already have been statically removed by native linkers so this likely won't have any compile-size impact, but it's a nice-to-have in terms of organization. This has a fair bit of shuffling around of code, but apart from renamings and movement there are no major changes here. * Fix compile issue * Gate `ModuleTranslation` and its methods on `compile` * Fix doc link * Fix doc link
* Fix rustdoc warnings on Nightly I noticed during a failed doc build of another PR we've got a number of warnings being emitted, so resolve all those here. * Fix more warnings * Fix rebase conflicts
* egraph: Resolve all aliases at once This way we can use the linear-time alias rewriting pass, and then avoid having to think about value aliases ever again. * Resolve aliases in facts and values_labels When resolving aliases in values_labels, this discards debug info on values which are replaced by aliases. However, that is equivalent to the existing behavior in `Lower::get_value_labels`, which resolves value aliases first and only then looks for attached debug info.
avanhatt
pushed a commit
to wellesley-prog-sys/wasmtime
that referenced
this pull request
Oct 9, 2024
Generate AArch64 load specs for `AMode.RegScaledExtended` and `AMode.RegExtended` addressing modes. Updates avanhatt#49 avanhatt#35
avanhatt
pushed a commit
to wellesley-prog-sys/wasmtime
that referenced
this pull request
Oct 9, 2024
Change generation of specs split on match cases so the requires uses `match` also. The change in avanhatt#101 is actually broken, though it did't trigger any errors because the specs are unused on the main branch. When working on avanhatt#99 the error does manifest. Specifically, the error is in the generated requires clauses: https://github.com/mmcloughlin/veriisle-wasmtime/blob/432bd4a86635cd82c76857689ff67c77e99cbd4e/cranelift/codegen/src/isa/aarch64/spec/loads.isle#L111-L113 This references the variable `extendop`, which does not exist. It doesn't exist because it's a field of the enum variant, and we haven't brought its fields into scope. This change updates the match case spec generation to use a match in the requires as well. This is probably a cosmetic regression avanhatt#62. If we care to we could update this in future to only use a match statement in the case where the child requires accesses the variable, but it doesn't seem worth it for now. Updates avanhatt#35 avanhatt#48
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First version of code - needs feedback.