Stream: git-wasmtime

Topic: wasmtime / issue #1891 Test specific flags in Cranelift f...

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

jameysharp closed issue #1891:

In #1880 @bnjbvr brought up an issue with how Cranelift's filetests use (or don't use) ISA-specific flags. He was trying to use target x86_64 use_new_backend in a test run filetest but the SingleFunctionCompiler doesn't know anything about ISA-specific Flags, only shared Flags. (Perhaps these two sets of flags should be merged in some way, but what I will propose next doesn't require that).

The current behavior is:

• check if the default host architecture (used by SingleFunctionCompiler) is the same as the one specified by the target directive; if not, skip the test (though in the future this should print some sort of IGNORED [filename] so the user knows what happened, see #1558)
• compile and run the test using the default host TargetIsa

I propose we change the behavior to:

• check if the default host TargetIsa _is compatible with_ the one specified by the target directive; if not, skip the test, etc.
• compile and run the test using the target directive's TargetIsa--this will use any special flags assigned by the user, e.g. use_new_backend

To get this "_is compatible with_" behavior, I propose we add a new Flags::matches function to the shared Flags and to each ISA-specific Flags (this likely has to be done as generated code in gen_settings.rs so it only needs to be done once). Then, we expose this function as TargetIsa::matches and implement it in each ISA as self.flags.matches(&other.flags) && self.isa_flags.matches(&other.isa_flags). This way we can compare TargetIsa's for compatibility.

How does Flags::matches work? I think it needs to iterate over the field descriptors and compare them both to the default value and to the other value. Remember that matches only goes one way: if a matches b it does not necessarily mean that b matches a. Though it passes some simple tests, I'm not 100% confident that the following is correct so I would appreciate feedback:

fn get_bit(byte: u8, bit: u8) -> bool {
    let mask = 1 << bit;
    (byte & mask) != 0
}

impl Flags {
    fn matches(&self, other: &Self) -> bool {
        let shared_default = settings::Flags::new(settings::builder());
        let default = Flags::new(&shared_default, builder());

        // Check each detail until we see presets.
        let mut byte_offset = 0;
        for d in &DESCRIPTORS {
            byte_offset = d.offset as usize;
            match d.detail {
                detail::Detail::Bool { bit } => {
                    let self_bit = get_bit(self.bytes[byte_offset], bit);
                    let default_bit = get_bit(default.bytes[byte_offset], bit);
                    let other_bit = get_bit(other.bytes[byte_offset], bit);
                    if self_bit != default_bit && self_bit != other_bit {
                        return false;
                    }
                }
                detail::Detail::Num | detail::Detail::Enum { .. } => {
                    let self_byte = self.bytes[byte_offset];
                    let default_byte = default.bytes[byte_offset];
                    let other_byte = other.bytes[byte_offset];
                    if self_byte != default_byte && self_byte != other_byte {
                        return false;
                    }
                }
                detail::Detail::Preset => break,
            }
        }

        // Then check each preset bit.
        for byte_offset in byte_offset + 1..self.bytes.len() {
            for bit in 0..8 {
                let self_bit = get_bit(self.bytes[byte_offset], bit);
                let default_bit = get_bit(default.bytes[byte_offset], bit);
                let other_bit = get_bit(other.bytes[byte_offset], bit);
                if self_bit != default_bit && self_bit != other_bit {
                    return false;
                }
            }
        }

        true
    }
}

Last updated: Nov 22 2024 at 16:03 UTC