Stream: git-wasmtime

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

Wasmtime GitHub notifications bot (Sep 10 2020 at 20:16):

fitzgen commented on Issue #2192:

cc @jubitaneja @regehr

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:

• bnjbvr: cranelift
• fitzgen: cranelift:area:peepmatic

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

Learn more.
</details>

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.

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

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.

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.

Wasmtime GitHub notifications bot (Sep 10 2020 at 22:31):

cfallin commented on Issue #2192:

PR: #2193

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: Dec 23 2024 at 13:07 UTC