-
Notifications
You must be signed in to change notification settings - Fork 9
Add support for int to float casts #189
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ pub struct FloatDomainData<'vir> { | |
| pub prim_to_snap: FunctionIdn<'vir, vir::Prim, vir::CSnap>, | ||
| #[allow(unused)] | ||
| pub from_bv: FunctionIdn<'vir, vir::CSnap, vir::CSnap>, | ||
| pub from_real: FunctionIdn<'vir, vir::Perm, vir::CSnap>, | ||
| pub fp_eq: FunctionIdn<'vir, (vir::CSnap, vir::CSnap), vir::Bool>, | ||
| pub fp_add: FunctionIdn<'vir, (vir::CSnap, vir::CSnap), vir::CSnap>, | ||
| pub fp_sub: FunctionIdn<'vir, (vir::CSnap, vir::CSnap), vir::CSnap>, | ||
|
|
@@ -191,6 +192,18 @@ pub(crate) fn ty_pure_float<'vir>( | |
| ty::FloatTy::F128 => "(_ to_fp 15 113)", | ||
| }; | ||
| let from_bv = builder.backend_func("from_bv", (bit_vec.domain)(), builder.self_type(), Some(i)); | ||
| let from_real_name: &str = match float { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| ty::FloatTy::F16 => "(_ to_fp 5 11) RNE", | ||
| ty::FloatTy::F32 => "(_ to_fp 8 24) RNE", | ||
| ty::FloatTy::F64 => "(_ to_fp 11 53) RNE", | ||
| ty::FloatTy::F128 => "(_ to_fp 15 113) RNE", | ||
| }; | ||
| let from_real = builder.backend_func( | ||
| "from_real", | ||
| vir::TYPE_PERM, | ||
| builder.self_type(), | ||
| Some(from_real_name), | ||
| ); | ||
|
|
||
| let fp_to_real = builder.backend_func( | ||
| "to_real", | ||
|
|
@@ -206,6 +219,7 @@ pub(crate) fn ty_pure_float<'vir>( | |
| Ok(FloatDomainData { | ||
| prim_to_snap, | ||
| from_bv, | ||
| from_real, | ||
| fp_eq, | ||
| fp_add, | ||
| fp_sub, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // This currently works with CVC5 but not with Z3.. | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a terribly useful comment (nor the right style). Why does it not work with Z3? Do any of the assertions here work? Can we add assertions to make it work? Either way maybe this should not be in the testsuite. (Yet? Ever?) We could have some tests which are CVC5-specific, in which case we should configure them as such using comments that provide the requisite flags to Prusti (and indirectly Silicon) to make it use CVC5.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Z3 just somehow fails to reason about it. Even cases like 1 -> 1.0 fail. This seems to be more like a Z3 issue. Therefore, I added the test case since we can now create a proper encoding for this cast without crashing. Even if the resulting Viper file doesn't verify with Z3
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interestingly, Z3 only struggles with int -> float. float -> int (which is more complicated because of the bounds checking) is not a problem |
||
|
|
||
| fn main() { | ||
| let x = 100; | ||
| let y = x as f32; | ||
| assert!(y == 100.0); | ||
|
|
||
| let x = u128::MAX; | ||
| let y = x as f32; | ||
| assert!(y == f32::INFINITY); | ||
|
|
||
| let x = u8::MAX; | ||
| let y = x as f64; | ||
| assert!(y == 255.0); | ||
|
|
||
| let x = -5; | ||
| let y = x as f32; | ||
| assert!(y == -5.0); | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -44,6 +44,7 @@ impl<'vir, Curr, Next> BinOpGenData<'vir, Curr, Next> { | |||||||||
| | BinOpKind::PermSub | ||||||||||
| | BinOpKind::PermMul | ||||||||||
| | BinOpKind::PermPermDiv => crate::TYPE_PERM.upcast_ty(), | ||||||||||
| BinOpKind::FractionalPerm => crate::TYPE_PERM.upcast_ty(), | ||||||||||
|
Comment on lines
46
to
+47
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
|
||||||||||
| BinOpKind::SetUnion => return self.lhs.ty(), | ||||||||||
| }; | ||||||||||
|
|
||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Zulip you said:
What would the alternatives be? What happens if we cast integers that can't be represented exactly by a float, and is the behaviour consistent with Rust? For example:
Since
xis beyond the maximum safe integer forf64, this prints:Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just realized during adding support for float->int that it makes more sense to use the SMT function to convert from int to real instead of doing the division. I'll probably close this PR and open a new PR for float->int and int->float
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yes the case you mentioned works