Skip to content

Commit

Permalink
Merge #3211
Browse files Browse the repository at this point in the history
3211: fix popcnt for aarch64 r=ptitSeb a=grishasobol

# Description
Fix problem with infinite `popcnt` WASM instruction execution on M1 processor. See details in corresponded issue: #3210

# Review
1) Make fix for `i32.popcnt`. Use additional execution branch if `src` register equal to `1`:
```
if src == 0 => goto exit_label
if src == 1 => goto src_is_one_label

loop_label:
... // popcnt loop body

goto exit_label

src_is_one_label:
mov dest, 1

exit_label:
```


Co-authored-by: Gregory Sobol <grishasobol@mail.ru>
  • Loading branch information
bors[bot] and grishasobol authored Oct 5, 2022
2 parents 250811a + f995252 commit 47c3491
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 11 deletions.
20 changes: 9 additions & 11 deletions lib/compiler-singlepass/src/machine_arm64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3052,11 +3052,10 @@ impl Machine for MachineARM64 {
self.assembler.emit_cbz_label(Size::S32, src, label_exit)?; // src==0, exit
self.assembler.emit_label(label_loop)?; // loop:
self.assembler
.emit_add(Size::S32, dest, Location::Imm8(1), dest)?; // inc dest
.emit_add(Size::S32, dest, Location::Imm8(1), dest)?; // dest += 1
self.assembler.emit_clz(Size::S32, src, tmp)?; // clz src => tmp
self.assembler
.emit_add(Size::S32, tmp, Location::Imm8(1), tmp)?; // inc tmp
self.assembler.emit_lsl(Size::S32, src, tmp, src)?; // src << tmp => src
self.assembler.emit_lsl(Size::S32, src, Location::Imm8(1), src)?; // src << 1 => src
self.assembler.emit_cbnz_label(Size::S32, src, label_loop)?; // if src!=0 goto loop
self.assembler.emit_label(label_exit)?;
if ret != dest {
Expand Down Expand Up @@ -4086,16 +4085,15 @@ impl Machine for MachineARM64 {
let label_loop = self.assembler.get_label();
let label_exit = self.assembler.get_label();
self.assembler
.emit_mov(Size::S32, Location::GPR(GPR::XzrSp), dest)?;
self.assembler.emit_cbz_label(Size::S64, src, label_exit)?;
.emit_mov(Size::S32, Location::GPR(GPR::XzrSp), dest)?; // dest <= 0
self.assembler.emit_cbz_label(Size::S64, src, label_exit)?; // src == 0, then goto label_exit
self.assembler.emit_label(label_loop)?;
self.assembler
.emit_add(Size::S32, dest, Location::Imm8(1), dest)?;
self.assembler.emit_clz(Size::S64, src, tmp)?;
self.assembler
.emit_add(Size::S32, tmp, Location::Imm8(1), tmp)?;
self.assembler.emit_lsl(Size::S64, src, tmp, src)?;
self.assembler.emit_cbnz_label(Size::S64, src, label_loop)?;
.emit_add(Size::S32, dest, Location::Imm8(1), dest)?; // dest += 1
self.assembler.emit_clz(Size::S64, src, tmp)?; // clz src => tmp
self.assembler.emit_lsl(Size::S64, src, tmp, src)?; // src << tmp => src
self.assembler.emit_lsl(Size::S64, src, Location::Imm8(1), src)?; // src << 1 => src
self.assembler.emit_cbnz_label(Size::S64, src, label_loop)?; // src != 0, then goto label_loop
self.assembler.emit_label(label_exit)?;
if ret != dest {
self.move_location(Size::S64, dest, ret)?;
Expand Down
54 changes: 54 additions & 0 deletions tests/compilers/issues.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,57 @@ fn regression_gpr_exhaustion_for_calls(mut config: crate::Config) -> Result<()>
let instance = Instance::new(&mut store, &module, &imports)?;
Ok(())
}

#[compiler_test(issues)]
fn test_popcnt(mut config: crate::Config) -> Result<()> {
let mut store = config.store();
let mut env = FunctionEnv::new(&mut store, ());
let imports: Imports = imports! {};

let wat = r#"
(module
(func $popcnt_i32 (export "popcnt_i32") (param i32) (result i32)
local.get 0
i32.popcnt
)
(func $popcnt_i64 (export "popcnt_i64") (param i64) (result i64)
local.get 0
i64.popcnt
)
)"#;

let module = Module::new(&store, wat)?;
let instance = Instance::new(&mut store, &module, &imports)?;

let popcnt_i32 = instance.exports.get_function("popcnt_i32").unwrap();
let popcnt_i64 = instance.exports.get_function("popcnt_i64").unwrap();

let get_next_number_i32 = |mut num: i32| {
num ^= num << 13;
num ^= num >> 17;
num ^= num << 5;
num
};
let get_next_number_i64 = |mut num: i64| {
num ^= num << 34;
num ^= num >> 40;
num ^= num << 7;
num
};

let mut num = 1;
for _ in 1..10000 {
let result = popcnt_i32.call(&mut store, &[Value::I32(num)]).unwrap();
assert_eq!(&Value::I32(num.count_ones() as i32), result.get(0).unwrap());
num = get_next_number_i32(num);
}

let mut num = 1;
for _ in 1..10000 {
let result = popcnt_i64.call(&mut store, &[Value::I64(num)]).unwrap();
assert_eq!(&Value::I64(num.count_ones() as i64), result.get(0).unwrap());
num = get_next_number_i64(num);
}

Ok(())
}

0 comments on commit 47c3491

Please sign in to comment.