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.

Sign up for GitHub

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

Use a C-safe return type for __rust_[ui]128_* overflowing intrinsics #735

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Use a C-safe return type for __rust_[ui]128_* overflowing intrinsics #735

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions src/int/addsub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

let (add, o) = a.addo(b);
*oflow = o.into();
add
}

pub extern "C" fn __rust_u128_add(a: u128, b: u128) -> u128 {
AddSub::add(a,b)
}

pub extern "C" fn __rust_u128_addo(a: u128, b: u128) -> (u128, bool) {
a.addo(b)
pub extern "C" fn __rust_u128_addo(a: u128, b: u128, oflow: &mut i32) -> u128 {
let (add, o) = a.addo(b);
*oflow = o.into();
add
}

pub extern "C" fn __rust_i128_sub(a: i128, b: i128) -> i128 {
AddSub::sub(a,b)
}

pub extern "C" fn __rust_i128_subo(a: i128, b: i128) -> (i128, bool) {
a.subo(b)
pub extern "C" fn __rust_i128_subo(a: i128, b: i128, oflow: &mut i32) -> i128 {
let (sub, o) = a.subo(b);
*oflow = o.into();
sub
}

pub extern "C" fn __rust_u128_sub(a: u128, b: u128) -> u128 {
AddSub::sub(a,b)
}

pub extern "C" fn __rust_u128_subo(a: u128, b: u128) -> (u128, bool) {
a.subo(b)
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
Copy link
Member

Choose a reason for hiding this comment

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

cg_clif doesn't actually use the add/sub intrinsics, so I opened #745 to remove them.

}
}
12 changes: 8 additions & 4 deletions src/int/mul.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,15 @@ intrinsics! {
mul
}

pub extern "C" fn __rust_i128_mulo(a: i128, b: i128) -> (i128, bool) {
i128_overflowing_mul(a, b)
pub extern "C" fn __rust_i128_mulo(a: i128, b: i128, oflow: &mut i32) -> i128 {
let (mul, o) = i128_overflowing_mul(a, b);
*oflow = o.into();
mul
}

pub extern "C" fn __rust_u128_mulo(a: u128, b: u128) -> (u128, bool) {
a.mulo(b)
pub extern "C" fn __rust_u128_mulo(a: u128, b: u128, oflow: &mut i32) -> u128 {
let (mul, o) = a.mulo(b);
*oflow = o.into();
mul
}
}
18 changes: 10 additions & 8 deletions testcrate/tests/addsub.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,22 @@ mod int_addsub {
use compiler_builtins::int::addsub::{$fn_add, $fn_sub};

fuzz_2(N, |x: $i, y: $i| {
let add0 = x.overflowing_add(y);
let sub0 = x.overflowing_sub(y);
let add1: ($i, bool) = $fn_add(x, y);
let sub1: ($i, bool) = $fn_sub(x, y);
if add0.0 != add1.0 || add0.1 != add1.1 {
let (add0, add_o0)= x.overflowing_add(y);
let (sub0, sub_o0)= x.overflowing_sub(y);
tgross35 marked this conversation as resolved.
Show resolved Hide resolved
let mut add_o1 = 0;
let mut sub_o1 = 0;
let add1: $i = $fn_add(x, y, &mut add_o1);
let sub1: $i = $fn_sub(x, y, &mut sub_o1);
if add0 != add1 || i32::from(add_o0) != add_o1 {
panic!(
"{}({}, {}): std: {:?}, builtins: {:?}",
stringify!($fn_add), x, y, add0, add1
stringify!($fn_add), x, y, (add0, add_o0) , (add1, add_o1)
tgross35 marked this conversation as resolved.
Show resolved Hide resolved
);
}
if sub0.0 != sub1.0 || sub0.1 != sub1.1 {
if sub0 != sub1 || i32::from(sub_o0) != sub_o1 {
panic!(
"{}({}, {}): std: {:?}, builtins: {:?}",
stringify!($fn_sub), x, y, sub0, sub1
stringify!($fn_sub), x, y, (sub0, sub_o0) , (sub1, sub_o1)
tgross35 marked this conversation as resolved.
Show resolved Hide resolved
);
}
});
Expand Down
9 changes: 5 additions & 4 deletions testcrate/tests/mul.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ mod int_overflowing_mul {
use compiler_builtins::int::mul::{__rust_i128_mulo, __rust_u128_mulo};

fuzz_2(N, |x: u128, y: u128| {
let mut o1 = 0;
let (mul0, o0) = x.overflowing_mul(y);
let (mul1, o1) = __rust_u128_mulo(x, y);
if mul0 != mul1 || o0 != o1 {
let mul1 = __rust_u128_mulo(x, y, &mut o1);
if mul0 != mul1 || i32::from(o0) != o1 {
panic!(
"__rust_u128_mulo({}, {}): std: ({}, {}), builtins: ({}, {})",
x, y, mul0, o0, mul1, o1
Expand All @@ -84,8 +85,8 @@ mod int_overflowing_mul {
let x = x as i128;
let y = y as i128;
let (mul0, o0) = x.overflowing_mul(y);
let (mul1, o1) = __rust_i128_mulo(x, y);
if mul0 != mul1 || o0 != o1 {
let mul1 = __rust_i128_mulo(x, y, &mut o1);
if mul0 != mul1 || i32::from(o0) != o1 {
panic!(
"__rust_i128_mulo({}, {}): std: ({}, {}), builtins: ({}, {})",
x, y, mul0, o0, mul1, o1
Expand Down
Loading