Use a C-safe return type for __rust_[ui]128_* overflowing intrinsics
#735
Conversation
Most of our Rust-specific overflowing intrinsics currently return `(i128, bool)`, which is not guaranteed to have a stable ABI. Switch to returning the overflow via a mutable parameter and only directly returning the integer result. `__rust_i128_mulo` now matches the function signature of `__muloti4`, but they do not share the same ABI on Windows so we cannot easily deduplicate them.
Combined with [1], this will change the overflowing multiplication operations to return an `extern "C"`-safe type. Link: rust-lang/compiler-builtins#735 [1]
| @@ -66,31 +66,39 @@ intrinsics! { | |||
| AddSub::add(a,b) | |||
| } | |||
|
|
|||
| pub extern "C" fn __rust_i128_addo(a: i128, b: i128) -> (i128, bool) { | |||
| a.addo(b) | |||
| pub extern "C" fn __rust_i128_addo(a: i128, b: i128, oflow: &mut i32) -> i128 { | |||
There was a problem hiding this comment.
Would you mind keeping the existing functions for a bit and adding new functions with the new ABI? That way the cg_clif update and the compiler-builtins update don't have to be done at the exact same time.
There was a problem hiding this comment.
I can do so if it helps, but why would this be needed? We pin the version of compiler_builtins with = now so it shouldn't get out of sync, and I won't merge this until the r-l/rust PR gets approved.
There was a problem hiding this comment.
We pin the version of compiler_builtins with = now so it shouldn't get out of sync, and I won't merge this until the r-l/rust PR gets approved.
In that case I guess it is fine to keep as-is.
Combined with [1], this will change the overflowing multiplication operations to return an `extern "C"`-safe type. Link: rust-lang/compiler-builtins#735 [1]
Combined with [1], this will change the overflowing multiplication operations to return an `extern "C"`-safe type. Link: rust-lang/compiler-builtins#735 [1]
| pub extern "C" fn __rust_u128_subo(a: u128, b: u128, oflow: &mut i32) -> u128 { | ||
| let (sub, o) = a.subo(b); | ||
| *oflow = o.into(); | ||
| sub |
There was a problem hiding this comment.
cg_clif doesn't actually use the add/sub intrinsics, so I opened #745 to remove them.
Combined with [1], this will change the overflowing multiplication operations to return an `extern "C"`-safe type. Link: rust-lang/compiler-builtins#735 [1]
Combined with [1], this will change the overflowing multiplication operations to return an `extern "C"`-safe type. Link: rust-lang/compiler-builtins#735 [1]
|
Thanks for the review. I won't merge this until bumping builtins is unblocked, and the rust-lang/rust PR is ready to go. |
Most of our Rust-specific overflowing intrinsics currently return
(i128, bool), which is not guaranteed to have a stable ABI. Switch to returning the overflow via a mutable parameter and only directly returning the integer result.__rust_i128_mulonow matches the function signature of__muloti4, but they do not share the same ABI on Windows so we cannot easily deduplicate them.