Merge #880

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 <nick@wasmer.io>
Co-authored-by: nlewycky <nick@wasmer.io>

lib/singlepass-backend/src/codegen_x64.rs

@@ -2341,18 +2341,109 @@ impl FunctionCodeGenerator<CodegenError> for X64FunctionCode {
                 Condition::Equal,
                 Location::Imm32(0),
             ),
-            Operator::I32Clz => Self::emit_xcnt_i32(
+            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,
-                &mut self.machine,
+                    &[(WpType::I32, MachineValue::WasmStack(self.value_stack.len()))],
-                &mut self.value_stack,
+                    false,
-                Assembler::emit_lzcnt,
+                )[0];
-            ),
+                self.value_stack.push(ret);
-            Operator::I32Ctz => Self::emit_xcnt_i32(
+
+                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,
-                &mut self.machine,
+                    &[(WpType::I32, MachineValue::WasmStack(self.value_stack.len()))],
-                &mut self.value_stack,
+                    false,
-                Assembler::emit_tzcnt,
+                )[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,
@@ -2632,18 +2723,109 @@ impl FunctionCodeGenerator<CodegenError> for X64FunctionCode {
                 Condition::Equal,
                 Location::Imm64(0),
             ),
-            Operator::I64Clz => Self::emit_xcnt_i64(
+            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,
-                &mut self.machine,
+                    &[(WpType::I64, MachineValue::WasmStack(self.value_stack.len()))],
-                &mut self.value_stack,
+                    false,
-                Assembler::emit_lzcnt,
+                )[0];
-            ),
+                self.value_stack.push(ret);
-            Operator::I64Ctz => Self::emit_xcnt_i64(
+
+                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,
-                &mut self.machine,
+                    &[(WpType::I64, MachineValue::WasmStack(self.value_stack.len()))],
-                &mut self.value_stack,
+                    false,
-                Assembler::emit_tzcnt,
+                )[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,

lib/singlepass-backend/src/emitter_x64.rs

@@ -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_bsr(&mut self, sz: Size, src: Location, dst: Location);
-    fn emit_tzcnt(&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);
@@ -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) {
+    fn emit_bsr(&mut self, sz: Size, src: Location, dst: Location) {
-        binop_gpr_gpr!(lzcnt, self, sz, src, dst, {
+        binop_gpr_gpr!(bsr, self, sz, src, dst, {
-            binop_mem_gpr!(lzcnt, self, sz, src, dst, {
+            binop_mem_gpr!(bsr, self, sz, src, dst, {
-                panic!("singlepass can't emit LZCNT {:?} {:?} {:?}", sz, src, dst)
+                panic!("singlepass can't emit BSR {:?} {:?} {:?}", sz, src, dst)
             })
         });
     }
-    fn emit_tzcnt(&mut self, sz: Size, src: Location, dst: Location) {
+    fn emit_bsf(&mut self, sz: Size, src: Location, dst: Location) {
-        binop_gpr_gpr!(tzcnt, self, sz, src, dst, {
+        binop_gpr_gpr!(bsf, self, sz, src, dst, {
-            binop_mem_gpr!(tzcnt, self, sz, src, dst, {
+            binop_mem_gpr!(bsf, self, sz, src, dst, {
-                panic!("singlepass can't emit TZCNT {:?} {:?} {:?}", sz, src, dst)
+                panic!("singlepass can't emit BSF {:?} {:?} {:?}", sz, src, dst)
             })
         });
     }

lib/spectests/tests/excludes.txt

@@ -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
@@ -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
@@ -977,7 +959,6 @@ 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
@@ -985,7 +966,6 @@ singlepass:fail:memory_grow.wast:18 # AssertTrap - expected trap, got Runtime:Er
 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