Stream: git-wasmtime

Topic: wasmtime / PR #4450 cranelift: Use requested ISA Flags in...

Wasmtime GitHub notifications bot (Jul 15 2022 at 01:00):

afonso360 opened PR #4450 from testrunner-flags to main:

:wave: Hey,

This PR Enables using the ISA flags that run tests request. We skip tests that request flags that the host arch does not support.

I did a quick pass on the backends to find ops that we have optimizations based on flags and enabled the tests for those.

Fixes #1891

Wasmtime GitHub notifications bot (Jul 15 2022 at 01:11):

afonso360 updated PR #4450 from testrunner-flags to main.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:26):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:26):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:26):

jameysharp created PR review comment:

Can we have a more helpful message in case somebody tries putting a non-boolean flag in a test? Something like:

            k => unimplemented!("ISA flag {} of kind {:?}", value.name, k),

It's not really important if there's a similar check added to is_isa_compatible as that will get hit first, but I'd still do it both places just in case.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:26):

jameysharp created PR review comment:

It seems surprising to me to silently ignore non-bool flags here. How do you feel about this?

    for req_value in requested_flags {
        if let Some(requested) = req_value.as_bool() {
            let available_in_host = host
                .isa_flags()
                .iter()
                .find(|val| val.name == req_value.name)
                .and_then(|val| val.as_bool())
                .unwrap_or(false);

            if requested && !available_in_host {
                return Err(format!(
                    "skipped {}: host does not support ISA flag {}",
                    context.file_path, req_value.name
                ));
            }
        } else {
            unimplemented!("ISA flag {} of kind {:?}", req_value.name, req_value.kind());
        }
    }

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:41):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:41):

afonso360 created PR review comment:

Yeah, sounds good, ill update to that.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:41):

afonso360 submitted PR review.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:41):

afonso360 created PR review comment:

I'm going to backport what I did in #4453 which adds a value_string function and makes this a whole lot cleaner, and supports all flag kinds.

Wasmtime GitHub notifications bot (Jul 15 2022 at 17:51):

afonso360 updated PR #4450 from testrunner-flags to main.

Wasmtime GitHub notifications bot (Jul 15 2022 at 18:33):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jul 15 2022 at 19:07):

jameysharp submitted PR review.

Wasmtime GitHub notifications bot (Jul 15 2022 at 19:09):

jameysharp merged PR #4450.

Last updated: Dec 23 2024 at 12:05 UTC