Skip to content

Commit 9eb8a56

Browse files
committed
[WIP] cmov: fix aarch64 ASM bug; add regression test
The implementation was using the wrong register type when invoking `csel` for `u64` (`:w` which is 32-bit, should use `:x` for 64-bit) This captures the bug in a regression test and fixes it.
1 parent a164fcb commit 9eb8a56

File tree

2 files changed

+65
-14
lines changed

2 files changed

+65
-14
lines changed

cmov/src/aarch64.rs

Lines changed: 52 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::{Cmov, CmovEq, Condition};
22
use core::arch::asm;
33

4-
macro_rules! csel {
4+
macro_rules! csel_32 {
55
($csel:expr, $dst:expr, $src:expr, $condition:expr) => {
66
unsafe {
77
asm! {
@@ -17,7 +17,23 @@ macro_rules! csel {
1717
};
1818
}
1919

20-
macro_rules! csel_eq {
20+
macro_rules! csel_64 {
21+
($csel:expr, $dst:expr, $src:expr, $condition:expr) => {
22+
unsafe {
23+
asm! {
24+
"cmp {0:x}, 0",
25+
$csel,
26+
in(reg) $condition,
27+
inlateout(reg) *$dst,
28+
in(reg) *$src,
29+
in(reg) *$dst,
30+
options(pure, nomem, nostack),
31+
};
32+
}
33+
};
34+
}
35+
36+
macro_rules! csel_eq_32 {
2137
($instruction:expr, $lhs:expr, $rhs:expr, $condition:expr, $dst:expr) => {
2238
let mut tmp = *$dst as u16;
2339
unsafe {
@@ -39,74 +55,96 @@ macro_rules! csel_eq {
3955
};
4056
}
4157

58+
macro_rules! csel_eq_64 {
59+
($instruction:expr, $lhs:expr, $rhs:expr, $condition:expr, $dst:expr) => {
60+
let mut tmp = *$dst as u16;
61+
unsafe {
62+
asm! {
63+
"eor {0:x}, {1:x}, {2:x}",
64+
"cmp {0:x}, 0",
65+
$instruction,
66+
out(reg) _,
67+
in(reg) *$lhs,
68+
in(reg) *$rhs,
69+
inlateout(reg) tmp,
70+
in(reg) $condition as u16,
71+
in(reg) tmp,
72+
options(pure, nomem, nostack),
73+
};
74+
};
75+
76+
*$dst = tmp as u8;
77+
};
78+
}
79+
4280
impl Cmov for u16 {
4381
#[inline]
4482
fn cmovnz(&mut self, value: &Self, condition: Condition) {
45-
csel!("csel {1:w}, {2:w}, {3:w}, NE", self, value, condition);
83+
csel_32!("csel {1:w}, {2:w}, {3:w}, NE", self, value, condition);
4684
}
4785

4886
#[inline]
4987
fn cmovz(&mut self, value: &Self, condition: Condition) {
50-
csel!("csel {1:w}, {2:w}, {3:w}, EQ", self, value, condition);
88+
csel_32!("csel {1:w}, {2:w}, {3:w}, EQ", self, value, condition);
5189
}
5290
}
5391

5492
impl CmovEq for u16 {
5593
#[inline]
5694
fn cmovne(&self, rhs: &Self, input: Condition, output: &mut Condition) {
57-
csel_eq!("csel {3:w}, {4:w}, {5:w}, NE", self, rhs, input, output);
95+
csel_eq_32!("csel {3:w}, {4:w}, {5:w}, NE", self, rhs, input, output);
5896
}
5997

6098
#[inline]
6199
fn cmoveq(&self, rhs: &Self, input: Condition, output: &mut Condition) {
62-
csel_eq!("csel {3:w}, {4:w}, {5:w}, EQ", self, rhs, input, output);
100+
csel_eq_32!("csel {3:w}, {4:w}, {5:w}, EQ", self, rhs, input, output);
63101
}
64102
}
65103

66104
impl Cmov for u32 {
67105
#[inline]
68106
fn cmovnz(&mut self, value: &Self, condition: Condition) {
69-
csel!("csel {1:w}, {2:w}, {3:w}, NE", self, value, condition);
107+
csel_32!("csel {1:w}, {2:w}, {3:w}, NE", self, value, condition);
70108
}
71109

72110
#[inline]
73111
fn cmovz(&mut self, value: &Self, condition: Condition) {
74-
csel!("csel {1:w}, {2:w}, {3:w}, EQ", self, value, condition);
112+
csel_32!("csel {1:w}, {2:w}, {3:w}, EQ", self, value, condition);
75113
}
76114
}
77115

78116
impl CmovEq for u32 {
79117
#[inline]
80118
fn cmovne(&self, rhs: &Self, input: Condition, output: &mut Condition) {
81-
csel_eq!("csel {3:w}, {4:w}, {5:w}, NE", self, rhs, input, output);
119+
csel_eq_32!("csel {3:w}, {4:w}, {5:w}, NE", self, rhs, input, output);
82120
}
83121

84122
#[inline]
85123
fn cmoveq(&self, rhs: &Self, input: Condition, output: &mut Condition) {
86-
csel_eq!("csel {3:w}, {4:w}, {5:w}, EQ", self, rhs, input, output);
124+
csel_eq_32!("csel {3:w}, {4:w}, {5:w}, EQ", self, rhs, input, output);
87125
}
88126
}
89127

90128
impl Cmov for u64 {
91129
#[inline]
92130
fn cmovnz(&mut self, value: &Self, condition: Condition) {
93-
csel!("csel {1:x}, {2:x}, {3:x}, NE", self, value, condition);
131+
csel_64!("csel {1:x}, {2:x}, {3:x}, NE", self, value, condition);
94132
}
95133

96134
#[inline]
97135
fn cmovz(&mut self, value: &Self, condition: Condition) {
98-
csel!("csel {1:x}, {2:x}, {3:x}, EQ", self, value, condition);
136+
csel_64!("csel {1:x}, {2:x}, {3:x}, EQ", self, value, condition);
99137
}
100138
}
101139

102140
impl CmovEq for u64 {
103141
#[inline]
104142
fn cmovne(&self, rhs: &Self, input: Condition, output: &mut Condition) {
105-
csel_eq!("csel {3:w}, {4:w}, {5:w}, NE", self, rhs, input, output);
143+
csel_eq_64!("csel {3:x}, {4:x}, {5:x}, NE", self, rhs, input, output);
106144
}
107145

108146
#[inline]
109147
fn cmoveq(&self, rhs: &Self, input: Condition, output: &mut Condition) {
110-
csel_eq!("csel {3:w}, {4:w}, {5:w}, EQ", self, rhs, input, output);
148+
csel_eq_64!("csel {3:x}, {4:x}, {5:x}, EQ", self, rhs, input, output);
111149
}
112150
}

cmov/tests/regression.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//! Tests for previous bugs in the implementation.
2+
3+
use cmov::CmovEq;
4+
5+
#[test]
6+
fn u64_cmoveq() {
7+
let n = 0x8200_0000_0000_0000u64;
8+
let mut cond = 0u8;
9+
n.cmoveq(&0, 1u8, &mut cond);
10+
11+
// 0x8200_0000_0000_0000 is not equal to 0
12+
assert_eq!(cond, 0);
13+
}

0 commit comments

Comments
 (0)