Skip to content

Commit

Permalink
Merge #880
Browse files Browse the repository at this point in the history 
880: Remove usage of LZCNT and TZCNT from singlepass. r=nlewycky a=nlewycky

# Description
This PR removes usage of LZCNT and TZCNT in singlepass to work on machines that don't support them. Unlike other instructions that fail loudly when not supported, LZCNT and TZCNT silently change behaviour by falling back to BSR and BSF which may produce different answers. They were used in the I32Clz, I32Ctz, I64Clz and I64Ctz instructions. Fixes i32.wast and i64.wast test failures on our Mac buildbot.

As an alternative, we could detect presence of LZCNT and TZCNT at runtime, either by running them directly or by querying CPUID to detect whether the CPU claims support for these instructions. I decided against that because having multiple paths is more complicated, and we aren't concerned with runtime of singlepass code, as a rule.


Co-authored-by: Nick Lewycky <[email protected]>
Co-authored-by: nlewycky <[email protected]>
  • Loading branch information
@bors @nlewycky
bors[bot] and nlewycky authored Oct 17, 2019
2 parents eafb123 + 01e81ee commit 8c308d0
Show file tree
Hide file tree
Showing 3 changed files with 216 additions and 54 deletions.
230 changes: 206 additions & 24 deletions lib/singlepass-backend/src/codegen_x64.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2341,18 +2341,109 @@ impl FunctionCodeGenerator<CodegenError> for X64FunctionCode {
Condition::Equal,
Location::Imm32(0),
),
Operator::I32Clz => Self::emit_xcnt_i32(
a,
&mut self.machine,
&mut self.value_stack,
Assembler::emit_lzcnt,
),
Operator::I32Ctz => Self::emit_xcnt_i32(
a,
&mut self.machine,
&mut self.value_stack,
Assembler::emit_tzcnt,
),
Operator::I32Clz => {
let loc =
get_location_released(a, &mut self.machine, self.value_stack.pop().unwrap());
let src = match loc {
Location::Imm32(_) | Location::Memory(_, _) => {
let tmp = self.machine.acquire_temp_gpr().unwrap();
a.emit_mov(Size::S32, loc, Location::GPR(tmp));
tmp
}
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let ret = self.machine.acquire_locations(
a,
&[(WpType::I32, MachineValue::WasmStack(self.value_stack.len()))],
false,
)[0];
self.value_stack.push(ret);

let dst = match ret {
Location::Memory(_, _) => self.machine.acquire_temp_gpr().unwrap(),
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let zero_path = a.get_label();
let end = a.get_label();

a.emit_test_gpr_64(src);
a.emit_jmp(Condition::Equal, zero_path);
a.emit_bsr(Size::S32, Location::GPR(src), Location::GPR(dst));
a.emit_xor(Size::S32, Location::Imm32(31), Location::GPR(dst));
a.emit_jmp(Condition::None, end);
a.emit_label(zero_path);
a.emit_mov(Size::S32, Location::Imm32(32), Location::GPR(dst));
a.emit_label(end);

match loc {
Location::Imm32(_) | Location::Memory(_, _) => {
self.machine.release_temp_gpr(src);
}
_ => {}
};
match ret {
Location::Memory(_, _) => {
a.emit_mov(Size::S32, Location::GPR(dst), ret);
self.machine.release_temp_gpr(dst);
}
_ => {}
};
}
Operator::I32Ctz => {
let loc =
get_location_released(a, &mut self.machine, self.value_stack.pop().unwrap());
let src = match loc {
Location::Imm32(_) | Location::Memory(_, _) => {
let tmp = self.machine.acquire_temp_gpr().unwrap();
a.emit_mov(Size::S32, loc, Location::GPR(tmp));
tmp
}
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let ret = self.machine.acquire_locations(
a,
&[(WpType::I32, MachineValue::WasmStack(self.value_stack.len()))],
false,
)[0];
self.value_stack.push(ret);

let dst = match ret {
Location::Memory(_, _) => self.machine.acquire_temp_gpr().unwrap(),
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let zero_path = a.get_label();
let end = a.get_label();

a.emit_test_gpr_64(src);
a.emit_jmp(Condition::Equal, zero_path);
a.emit_bsf(Size::S32, Location::GPR(src), Location::GPR(dst));
a.emit_jmp(Condition::None, end);
a.emit_label(zero_path);
a.emit_mov(Size::S32, Location::Imm32(32), Location::GPR(dst));
a.emit_label(end);

match loc {
Location::Imm32(_) | Location::Memory(_, _) => {
self.machine.release_temp_gpr(src);
}
_ => {}
};
match ret {
Location::Memory(_, _) => {
a.emit_mov(Size::S32, Location::GPR(dst), ret);
self.machine.release_temp_gpr(dst);
}
_ => {}
};
}
Operator::I32Popcnt => Self::emit_xcnt_i32(
a,
&mut self.machine,
Expand Down Expand Up @@ -2632,18 +2723,109 @@ impl FunctionCodeGenerator<CodegenError> for X64FunctionCode {
Condition::Equal,
Location::Imm64(0),
),
Operator::I64Clz => Self::emit_xcnt_i64(
a,
&mut self.machine,
&mut self.value_stack,
Assembler::emit_lzcnt,
),
Operator::I64Ctz => Self::emit_xcnt_i64(
a,
&mut self.machine,
&mut self.value_stack,
Assembler::emit_tzcnt,
),
Operator::I64Clz => {
let loc =
get_location_released(a, &mut self.machine, self.value_stack.pop().unwrap());
let src = match loc {
Location::Imm64(_) | Location::Imm32(_) | Location::Memory(_, _) => {
let tmp = self.machine.acquire_temp_gpr().unwrap();
a.emit_mov(Size::S64, loc, Location::GPR(tmp));
tmp
}
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let ret = self.machine.acquire_locations(
a,
&[(WpType::I64, MachineValue::WasmStack(self.value_stack.len()))],
false,
)[0];
self.value_stack.push(ret);

let dst = match ret {
Location::Memory(_, _) => self.machine.acquire_temp_gpr().unwrap(),
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let zero_path = a.get_label();
let end = a.get_label();

a.emit_test_gpr_64(src);
a.emit_jmp(Condition::Equal, zero_path);
a.emit_bsr(Size::S64, Location::GPR(src), Location::GPR(dst));
a.emit_xor(Size::S64, Location::Imm32(63), Location::GPR(dst));
a.emit_jmp(Condition::None, end);
a.emit_label(zero_path);
a.emit_mov(Size::S64, Location::Imm32(64), Location::GPR(dst));
a.emit_label(end);

match loc {
Location::Imm64(_) | Location::Imm32(_) | Location::Memory(_, _) => {
self.machine.release_temp_gpr(src);
}
_ => {}
};
match ret {
Location::Memory(_, _) => {
a.emit_mov(Size::S64, Location::GPR(dst), ret);
self.machine.release_temp_gpr(dst);
}
_ => {}
};
}
Operator::I64Ctz => {
let loc =
get_location_released(a, &mut self.machine, self.value_stack.pop().unwrap());
let src = match loc {
Location::Imm64(_) | Location::Imm32(_) | Location::Memory(_, _) => {
let tmp = self.machine.acquire_temp_gpr().unwrap();
a.emit_mov(Size::S64, loc, Location::GPR(tmp));
tmp
}
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let ret = self.machine.acquire_locations(
a,
&[(WpType::I64, MachineValue::WasmStack(self.value_stack.len()))],
false,
)[0];
self.value_stack.push(ret);

let dst = match ret {
Location::Memory(_, _) => self.machine.acquire_temp_gpr().unwrap(),
Location::GPR(reg) => reg,
_ => unreachable!(),
};

let zero_path = a.get_label();
let end = a.get_label();

a.emit_test_gpr_64(src);
a.emit_jmp(Condition::Equal, zero_path);
a.emit_bsf(Size::S64, Location::GPR(src), Location::GPR(dst));
a.emit_jmp(Condition::None, end);
a.emit_label(zero_path);
a.emit_mov(Size::S64, Location::Imm32(64), Location::GPR(dst));
a.emit_label(end);

match loc {
Location::Imm64(_) | Location::Imm32(_) | Location::Memory(_, _) => {
self.machine.release_temp_gpr(src);
}
_ => {}
};
match ret {
Location::Memory(_, _) => {
a.emit_mov(Size::S64, Location::GPR(dst), ret);
self.machine.release_temp_gpr(dst);
}
_ => {}
};
}
Operator::I64Popcnt => Self::emit_xcnt_i64(
a,
&mut self.machine,
Expand Down
20 changes: 10 additions & 10 deletions lib/singlepass-backend/src/emitter_x64.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ pub trait Emitter {
fn emit_ror(&mut self, sz: Size, src: Location, dst: Location);
fn emit_and(&mut self, sz: Size, src: Location, dst: Location);
fn emit_or(&mut self, sz: Size, src: Location, dst: Location);
fn emit_lzcnt(&mut self, sz: Size, src: Location, dst: Location);
fn emit_tzcnt(&mut self, sz: Size, src: Location, dst: Location);
fn emit_bsr(&mut self, sz: Size, src: Location, dst: Location);
fn emit_bsf(&mut self, sz: Size, src: Location, dst: Location);
fn emit_popcnt(&mut self, sz: Size, src: Location, dst: Location);
fn emit_movzx(&mut self, sz_src: Size, src: Location, sz_dst: Size, dst: Location);
fn emit_movsx(&mut self, sz_src: Size, src: Location, sz_dst: Size, dst: Location);
Expand Down Expand Up @@ -753,17 +753,17 @@ impl Emitter for Assembler {
panic!("singlepass can't emit OR {:?} {:?} {:?}", sz, src, dst)
});
}
fn emit_lzcnt(&mut self, sz: Size, src: Location, dst: Location) {
binop_gpr_gpr!(lzcnt, self, sz, src, dst, {
binop_mem_gpr!(lzcnt, self, sz, src, dst, {
panic!("singlepass can't emit LZCNT {:?} {:?} {:?}", sz, src, dst)
fn emit_bsr(&mut self, sz: Size, src: Location, dst: Location) {
binop_gpr_gpr!(bsr, self, sz, src, dst, {
binop_mem_gpr!(bsr, self, sz, src, dst, {
panic!("singlepass can't emit BSR {:?} {:?} {:?}", sz, src, dst)
})
});
}
fn emit_tzcnt(&mut self, sz: Size, src: Location, dst: Location) {
binop_gpr_gpr!(tzcnt, self, sz, src, dst, {
binop_mem_gpr!(tzcnt, self, sz, src, dst, {
panic!("singlepass can't emit TZCNT {:?} {:?} {:?}", sz, src, dst)
fn emit_bsf(&mut self, sz: Size, src: Location, dst: Location) {
binop_gpr_gpr!(bsf, self, sz, src, dst, {
binop_mem_gpr!(bsf, self, sz, src, dst, {
panic!("singlepass can't emit BSF {:?} {:?} {:?}", sz, src, dst)
})
});
}
Expand Down
20 changes: 0 additions & 20 deletions lib/spectests/tests/excludes.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -902,15 +902,6 @@ singlepass:fail:i32.wast:99 # AssertTrap - expected trap, got Runtime:Error unkn
singlepass:fail:i32.wast:100 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i32.wast:120 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i32.wast:121 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i32.wast:242 # AssertReturn - result I32(31) ("0x1f") does not match expected I32(0) ("0x0")
singlepass:fail:i32.wast:243 # AssertReturn - result I32(0) ("0x0") does not match expected I32(32) ("0x20")
singlepass:fail:i32.wast:244 # AssertReturn - result I32(15) ("0xf") does not match expected I32(16) ("0x10")
singlepass:fail:i32.wast:245 # AssertReturn - result I32(7) ("0x7") does not match expected I32(24) ("0x18")
singlepass:fail:i32.wast:246 # AssertReturn - result I32(31) ("0x1f") does not match expected I32(0) ("0x0")
singlepass:fail:i32.wast:247 # AssertReturn - result I32(0) ("0x0") does not match expected I32(31) ("0x1f")
singlepass:fail:i32.wast:248 # AssertReturn - result I32(1) ("0x1") does not match expected I32(30) ("0x1e")
singlepass:fail:i32.wast:249 # AssertReturn - result I32(30) ("0x1e") does not match expected I32(1) ("0x1")
singlepass:fail:i32.wast:252 # AssertReturn - result I32(0) ("0x0") does not match expected I32(32) ("0x20")
singlepass:fail:i64.wast:62 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i64.wast:63 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i64.wast:64 # AssertTrap - expected trap, got Runtime:Error unknown error
Expand All @@ -920,15 +911,6 @@ singlepass:fail:i64.wast:99 # AssertTrap - expected trap, got Runtime:Error unkn
singlepass:fail:i64.wast:100 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i64.wast:120 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i64.wast:121 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:i64.wast:242 # AssertReturn - result I64(63) ("0x3f") does not match expected I64(0) ("0x0")
singlepass:fail:i64.wast:243 # AssertReturn - result I64(0) ("0x0") does not match expected I64(64) ("0x40")
singlepass:fail:i64.wast:244 # AssertReturn - result I64(15) ("0xf") does not match expected I64(48) ("0x30")
singlepass:fail:i64.wast:245 # AssertReturn - result I64(7) ("0x7") does not match expected I64(56) ("0x38")
singlepass:fail:i64.wast:246 # AssertReturn - result I64(63) ("0x3f") does not match expected I64(0) ("0x0")
singlepass:fail:i64.wast:247 # AssertReturn - result I64(0) ("0x0") does not match expected I64(63) ("0x3f")
singlepass:fail:i64.wast:248 # AssertReturn - result I64(1) ("0x1") does not match expected I64(62) ("0x3e")
singlepass:fail:i64.wast:249 # AssertReturn - result I64(62) ("0x3e") does not match expected I64(1) ("0x1")
singlepass:fail:i64.wast:252 # AssertReturn - result I64(0) ("0x0") does not match expected I64(64) ("0x40")
singlepass:fail:if.wast:440 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:imports.wast:283 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:imports.wast:286 # AssertTrap - expected trap, got Runtime:Error unknown error
Expand Down Expand Up @@ -977,15 +959,13 @@ singlepass:fail:linking.wast:236 # AssertTrap - expected trap, got Runtime:Error
singlepass:fail:linking.wast:248 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:linking.wast:387 # AssertReturn - result I32(0) ("0x0") does not match expected I32(104) ("0x68")
singlepass:fail:linking.wast:388 # AssertReturn - Call failed RuntimeError: unknown error
singlepass:fail:load.wast:201 # AssertReturn - result I32(0) ("0x0") does not match expected I32(32) ("0x20")
singlepass:fail:memory_grow.wast:15 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_grow.wast:16 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_grow.wast:17 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_grow.wast:18 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_grow.wast:24 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_grow.wast:25 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_grow.wast:286 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_grow.wast:299 # AssertReturn - result I32(0) ("0x0") does not match expected I32(31) ("0x1f")
singlepass:fail:memory_trap.wast:23 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_trap.wast:24 # AssertTrap - expected trap, got Runtime:Error unknown error
singlepass:fail:memory_trap.wast:25 # AssertTrap - expected trap, got Runtime:Error unknown error
Expand Down

0 comments on commit 8c308d0

Please sign in to comment.