Skip to content

Use type models#79

Merged
mmcloughlin merged 12 commits intoverify-mainfrom
mbm/use-models
Nov 2, 2023
Merged

Use type models#79
mmcloughlin merged 12 commits intoverify-mainfrom
mbm/use-models

Conversation

@mmcloughlin
Copy link
Collaborator

@mmcloughlin mmcloughlin commented Nov 2, 2023

Currently (model ...) declarations in ISLE are ignored. This PR implements processing for them, and replaces hard-coded types in add_isle_constraints with equivalent model declarations.

Tests pass:

mbm@m1p ~/D/v/v/c/i/v/veri_engine (mbm/use-models)> cargo test
...
running 182 tests
test test_broken_band_fits_in_32 ... ok
test test_broken_bor_fits_in_32 ... ok
test test_broken_cls_8 ... ok
...
test test_named_urem ... ok
test test_named_x64_to_amode_add_const_fold_iadd_lhs_rhs ... ok
test test_named_x64_to_amode_add_const_fold_iadd_rhs_rhs ... ok
test test_named_x64_to_amode_add_const_fold_iadd_lhs_lhs ... ok
test test_named_x64_to_amode_add_const_fold_iadd_rhs_lhs ... ok
test test_named_srem ... ok

test result: ok. 172 passed; 0 failed; 10 ignored; 0 measured; 0 filtered out; finished in 49.93s
...

(UnsignedLessThanOrEqual #x09)))

(spec (smin x y)
(spec (smin x y)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, there's lots of whitespace changes. My editor is cleaning up trailing whitespace on lines.

Comment on lines 9 to +10
;; We will represent this with a msb shift bit
;; and a 12 bit value
;; TODO(mbm): what was intended here? the hard-coded model was 24 bits
(model Imm12 (type (bv 24)))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This appears in multiple of the "broken" tests. It would have had the 24-bit model from type inference, but the comment "msb shift bit and a 12 bit value" sounds to me like it would be modeled as a 13-bit value.

Copy link
Owner

Choose a reason for hiding this comment

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

We chose to model this as the range of immediates it could encode (with require blocks further restricting it to the representable subset), instead of the 12 bits and a shift.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

But the comment is out-of-date, right?

(type u128 (primitive u128))

(model usize (type (bv 64)))
(model usize (type (bv)))
Copy link
Collaborator Author

@mmcloughlin mmcloughlin Nov 2, 2023

Choose a reason for hiding this comment

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

This was annotation_ir::Type::BitVector in the code and bv 64 in the existing (ignored) model. I reverted to what was being used in the code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file contains most of the interesting changes, I think.

@mmcloughlin mmcloughlin marked this pull request as ready for review November 2, 2023 20:37
@mmcloughlin mmcloughlin requested a review from avanhatt November 2, 2023 21:42
Copy link
Owner

@avanhatt avanhatt left a comment

Choose a reason for hiding this comment

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

Woohoo, this is great!

Comment on lines 9 to +10
;; We will represent this with a msb shift bit
;; and a 12 bit value
;; TODO(mbm): what was intended here? the hard-coded model was 24 bits
(model Imm12 (type (bv 24)))
Copy link
Owner

Choose a reason for hiding this comment

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

We chose to model this as the range of immediates it could encode (with require blocks further restricting it to the representable subset), instead of the 12 bits and a shift.

};
model_map.insert(type_id, ir_type);
}
ast::ModelValue::EnumValues(vals) => {
Copy link
Owner

Choose a reason for hiding this comment

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

Weird that the diff shows this all as changing, when it mostly has not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe this was a cargo fmt thing.

annotation_info: &mut AnnotationTypeInfo,
annotation: annotation_ir::TermSignature,
) {
let clif_to_ir_types = HashMap::from([
Copy link
Owner

Choose a reason for hiding this comment

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

Yaaaaay

Base automatically changed from mbm/fix-broken-broken-tests to verify-main November 2, 2023 22:16
@mmcloughlin mmcloughlin merged commit 9f0945f into verify-main Nov 2, 2023
avanhatt pushed a commit to wellesley-prog-sys/wasmtime that referenced this pull request Oct 18, 2024
Migrate the logic in the `veri` binary into a `Runner` class.

This is intended to make testing a little easier by providing a library
entry point into the verifier.

Updates avanhatt#79
avanhatt pushed a commit to wellesley-prog-sys/wasmtime that referenced this pull request Oct 18, 2024
Provides basic support for file-based tests, with a single examples of
passing and broken test cases.

Updates avanhatt#79
avanhatt pushed a commit that referenced this pull request Jan 11, 2026
…-zero

wizer.h: handle the case when wasi command main function returns non-zero code
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.

2 participants