Stream: git-wasmtime

Topic: wasmtime / Issue #2192 Convert Souper optimizations to Pe...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 20:16):

fitzgen commented on Issue #2192:

cc @jubitaneja @regehr

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 20:34):

github-actions[bot] commented on Issue #2192:

Subscribe to Label Action

cc @bnjbvr, @fitzgen

<details>
This issue or pull request has been labeled: "cranelift", "cranelift:area:peepmatic"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 20:56):

fitzgen commented on Issue #2192:

Looks like this is failing because the souper-ir crate uses the matches! macro:

$ cargo +1.41.0 check -p cranelift-tools
...
error[E0658]: use of unstable library feature 'matches_macro'
   --> /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/souper-ir-1.0.0/src/parse.rs:815:24
    |
815 |         let is_block = matches!(assignment.value, ast::AssignmentRhs::Block(_));
    |                        ^^^^^^^

@cfallin did Firefox ever update to a newer rustc since we put this check in CI? And if not, does clif-util really need to build on rustc 1.41.0? It doesn't get included in Firefox/SpiderMonkey, so I'd expect that the resriction doesn't matter here, like the way it does for cranelift-codegen.

If necessary I can also make a new release of souper-ir that doesn't use matches! but that seems like it shouldn't be necessary.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 21:42):

fitzgen commented on Issue #2192:

TODO: define a convert_commutative_operation function to make sure that we canonicalize iadds into iadd_imm (etc...) for if either the first or second operand is constant as we convert souper to peepmatic. Right now, a bunch of conversions only check one of the two operands. h/t @jubitaneja

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 21:50):

jubitaneja commented on Issue #2192:

Besides the commutative check (mentioned above), it LGTM. Thanks for doing this work, it will also help us use peepmatic.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 22:27):

cfallin commented on Issue #2192:

@cfallin did Firefox ever update to a newer rustc since we put this check in CI?

It looks like Firefox's minimum Rust is now 1.43.0 (link), so I think we can update the CI check to reflect that (and support matches!()). I'll create a PR.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 22:31):

cfallin commented on Issue #2192:

PR: #2193

view this post on Zulip Wasmtime GitHub notifications bot (Sep 10 2020 at 22:41):

fitzgen commented on Issue #2192:

It looks like Firefox's minimum Rust is now 1.43.0 (link), so I think we can update the CI check to reflect that (and support matches!()). I'll create a PR.

Nice! matches! is available in >= 1.42, see the version in the upper right, by the [src] link: https://doc.rust-lang.org/nightly/std/macro.matches.html

Thanks for making that PR!

Last updated: Nov 22 2024 at 16:03 UTC