Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests for cache system #230

Merged
merged 1 commit into from Aug 19, 2019
Merged

Conversation

mrowqa
Copy link
Contributor

@mrowqa mrowqa commented Jul 29, 2019

Based on #223. Draft to avoid merging before #223.

@mrowqa mrowqa force-pushed the cache-tests branch 3 times, most recently from 61570c8 to 4f4a01d Compare August 7, 2019 22:17
@mrowqa mrowqa mentioned this pull request Aug 10, 2019
@mrowqa mrowqa force-pushed the cache-tests branch 2 times, most recently from b3ff7c2 to 3ce5f16 Compare August 19, 2019 18:52
@mrowqa mrowqa marked this pull request as ready for review August 19, 2019 18:52
Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall it looks good!

}
}

// cranelift's types (including SecondaryMap) doesn't implement PartialEq
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I originally suggested using this approach, but I actually think we can go back and implement Eq/PartialEq for SecondaryMap, with the approach of trimming the trailing default-value elements. That should hopefully allow the code here to be much simpler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As we discussed, I'll do it as a follow up refactor later, also implementing the serialization for SecondaryMap.

wasmtime-environ/src/cache.rs Outdated Show resolved Hide resolved
}
}

// binemit::Reloc doesn't implement PartialEq
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make binemit::Reloc implement PartialEq and Eq.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing as with SecondaryMap - I'll do it later as follow up refactor.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@sunfishcode sunfishcode merged commit c3215f4 into bytecodealliance:master Aug 19, 2019
@mrowqa mrowqa deleted the cache-tests branch August 19, 2019 23:57
grishasobol pushed a commit to grishasobol/wasmtime that referenced this pull request Nov 29, 2021
* Fix wasmi no_std support (bytecodealliance#218)

Two issues are fixed:

 1. num-bigint 0.2 requires std, and is used to perform float to int conversions
    since bytecodealliance#186. This patch disables this change in no_std environments.
    This change can be reverted once num-bigint 0.3 is released, which will
    support no_std.

 2. The `f{32,64}::fract` method is currently not implemented by core, only std.
    This patch uses the no_std supporting implementation from num-trait's
    FloatCore trait.

* Ensure wasmi builds without std in CI

The no-std-check cargo subcommand uses a custom sysroot to prevent the crate
from using std while checking a local target. This should help ensure that
changes which introduce std dependencies are caught at review time.
mooori added a commit to mooori/wasmtime that referenced this pull request Feb 28, 2024
feat: add ISA specific setting `instrument_inst` for zkASM
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants