Repository: bytecodealliance/wasmtime
Pull request #5691 (2023-02-02 16:02:45)

winch: Small clean-up for x64
This commit contains a small set of clean up items for x64. 

Notably:

* Adds filetests
* Documents why 16 for the arg base offset abi implementation, for clarity.
* Fixes a bug in the spill implementation that was catched while anlyzing the filetests results. The fix consists of emitting a load instead of a store into the scratch register before spiiling its value.
* Remove dead code for pretty printing registers which is not needed anymore since we now have proper disassembly.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

- [ ] This has been discussed in issue #..., or if not, please tell us why
  here.
- [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.
- [ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the [code of
conduct](https://github.com/bytecodealliance/wasmtime/blob/master/CODE_OF_CONDUCT.md).
-->


github-actions[bot]: #### Subscribe to Label Action

cc @saulecabrera

<details>
This issue or pull request has been labeled: "winch"

Thus the following users have been cc'd because of the following labels:

* saulecabrera: winch

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

[Learn more.](https://github.com/bytecodealliance/subscribe-to-label-action)
</details>

Patch:
diff --git a/winch/codegen/src/isa/reg.rs b/winch/codegen/src/isa/reg.rs
index d4a55c11a..c7e47897e 100644
--- a/winch/codegen/src/isa/reg.rs
+++ b/winch/codegen/src/isa/reg.rs
@@ -27,11 +27,6 @@ impl Reg {
         Self::new(PReg::new(enc, RegClass::Float))
     }
 
-    /// Get the class of the underlying register.
-    pub fn class(self) -> RegClass {
-        self.0.class()
-    }
-
     /// Get the encoding of the underlying register.
     pub fn hw_enc(self) -> u8 {
         self.0.hw_enc() as u8
diff --git a/winch/codegen/src/isa/x64/abi.rs b/winch/codegen/src/isa/x64/abi.rs
index 7c5060969..cf7dc2eb5 100644
--- a/winch/codegen/src/isa/x64/abi.rs
+++ b/winch/codegen/src/isa/x64/abi.rs
@@ -40,6 +40,15 @@ impl ABI for X64ABI {
     }
 
     fn arg_base_offset(&self) -> u8 {
+        // Two 8-byte slots, one for the return address and another
+        // one for the frame pointer.
+        // ┌──────────┬───────── Argument base
+        // │   Ret    │
+        // │   Addr   │
+        // ├──────────┼
+        // │          │
+        // │   FP     │
+        // └──────────┴
         16
     }
 
diff --git a/winch/codegen/src/isa/x64/masm.rs b/winch/codegen/src/isa/x64/masm.rs
index 520f46db1..3f73aaf64 100644
--- a/winch/codegen/src/isa/x64/masm.rs
+++ b/winch/codegen/src/isa/x64/masm.rs
@@ -124,6 +124,8 @@ impl Masm for MacroAssembler {
     }
 
     fn epilogue(&mut self, locals_size: u32) {
+        assert!(self.sp_offset == locals_size);
+
         let rsp = rsp();
         if locals_size > 0 {
             self.asm.add_ir(locals_size as i32, rsp, OperandSize::S64);
diff --git a/winch/codegen/src/isa/x64/regs.rs b/winch/codegen/src/isa/x64/regs.rs
index 2e78c1b28..18710024f 100644
--- a/winch/codegen/src/isa/x64/regs.rs
+++ b/winch/codegen/src/isa/x64/regs.rs
@@ -142,97 +142,3 @@ const NON_ALLOCATABLE_GPR: u32 = (1 << ENC_RBP) | (1 << ENC_RSP) | (1 << ENC_R11
 
 /// Bitmask to represent the available general purpose registers.
 pub(crate) const ALL_GPR: u32 = ALLOCATABLE_GPR & !NON_ALLOCATABLE_GPR;
-
-// Temporarily removing the % from the register name
-// for debugging purposes only until winch gets disasm
-// support.
-pub(crate) fn reg_name(reg: Reg, size: u8) -> &'static str {
-    match reg.class() {
-        RegClass::Int => match (reg.hw_enc() as u8, size) {
-            (ENC_RAX, 8) => "rax",
-            (ENC_RAX, 4) => "eax",
-            (ENC_RAX, 2) => "ax",
-            (ENC_RAX, 1) => "al",
-            (ENC_RBX, 8) => "rbx",
-            (ENC_RBX, 4) => "ebx",
-            (ENC_RBX, 2) => "bx",
-            (ENC_RBX, 1) => "bl",
-            (ENC_RCX, 8) => "rcx",
-            (ENC_RCX, 4) => "ecx",
-            (ENC_RCX, 2) => "cx",
-            (ENC_RCX, 1) => "cl",
-            (ENC_RDX, 8) => "rdx",
-            (ENC_RDX, 4) => "edx",
-            (ENC_RDX, 2) => "dx",
-            (ENC_RDX, 1) => "dl",
-            (ENC_RSI, 8) => "rsi",
-            (ENC_RSI, 4) => "esi",
-            (ENC_RSI, 2) => "si",
-            (ENC_RSI, 1) => "sil",
-            (ENC_RDI, 8) => "rdi",
-            (ENC_RDI, 4) => "edi",
-            (ENC_RDI, 2) => "di",
-            (ENC_RDI, 1) => "dil",
-            (ENC_RBP, 8) => "rbp",
-            (ENC_RBP, 4) => "ebp",
-            (ENC_RBP, 2) => "bp",
-            (ENC_RBP, 1) => "bpl",
-            (ENC_RSP, 8) => "rsp",
-            (ENC_RSP, 4) => "esp",
-            (ENC_RSP, 2) => "sp",
-            (ENC_RSP, 1) => "spl",
-            (ENC_R8, 8) => "r8",
-            (ENC_R8, 4) => "r8d",
-            (ENC_R8, 2) => "r8w",
-            (ENC_R8, 1) => "r8b",
-            (ENC_R9, 8) => "r9",
-            (ENC_R9, 4) => "r9d",
-            (ENC_R9, 2) => "r9w",
-            (ENC_R9, 1) => "r9b",
-            (ENC_R10, 8) => "r10",
-            (ENC_R10, 4) => "r10d",
-            (ENC_R10, 2) => "r10w",
-            (ENC_R10, 1) => "r10b",
-            (ENC_R11, 8) => "r11",
-            (ENC_R11, 4) => "r11d",
-            (ENC_R11, 2) => "r11w",
-            (ENC_R11, 1) => "r11b",
-            (ENC_R12, 8) => "r12",
-            (ENC_R12, 4) => "r12d",
-            (ENC_R12, 2) => "r12w",
-            (ENC_R12, 1) => "r12b",
-            (ENC_R13, 8) => "r13",
-            (ENC_R13, 4) => "r13d",
-            (ENC_R13, 2) => "r13w",
-            (ENC_R13, 1) => "r13b",
-            (ENC_R14, 8) => "r14",
-            (ENC_R14, 4) => "r14d",
-            (ENC_R14, 2) => "r14w",
-            (ENC_R14, 1) => "r14b",
-            (ENC_R15, 8) => "r15",
-            (ENC_R15, 4) => "r15d",
-            (ENC_R15, 2) => "r15w",
-            (ENC_R15, 1) => "r15b",
-            _ => panic!("Invalid Reg: {:?}", reg),
-        },
-        RegClass::Float => match reg.hw_enc() {
-            0 => "xmm0",
-            1 => "xmm1",
-            2 => "xmm2",
-            3 => "xmm3",
-            4 => "xmm4",
-            5 => "xmm5",
-            6 => "xmm6",
-            7 => "xmm7",
-            8 => "xmm8",
-            9 => "xmm9",
-            10 => "xmm10",
-            11 => "xmm11",
-            12 => "xmm12",
-            13 => "xmm13",
-            14 => "xmm14",
-            15 => "xmm15",
-            _ => panic!("Invalid Reg: {:?}", reg),
-        },
-    }
-}
diff --git a/winch/codegen/src/regalloc.rs b/winch/codegen/src/regalloc.rs
index 8d29f77a5..0d67808ee 100644
--- a/winch/codegen/src/regalloc.rs
+++ b/winch/codegen/src/regalloc.rs
@@ -134,9 +134,7 @@ impl RegAlloc {
                     .get_local(*index)
                     .expect("valid local at slot");
                 let addr = context.masm.local_address(&slot);
-                context
-                    .masm
-                    .store(RegImm::reg(self.scratch), addr, slot.ty.into());
+                context.masm.load(addr, self.scratch, slot.ty.into());
                 let offset = context.masm.push(self.scratch);
                 *v = Val::Memory(offset);
             }


Test patch:
diff --git a/winch/filetests/filetests/x64/basic_add.wat b/winch/filetests/filetests/x64/basic_add.wat
new file mode 100644
index 000000000..3a2f2cb7b
--- /dev/null
+++ b/winch/filetests/filetests/x64/basic_add.wat
@@ -0,0 +1,16 @@
+;;! target = "x86_64"
+
+(module
+    (export "main" (func $main))
+
+    (func $main (result i32)
+	(i32.const 10)
+	(i32.const 20)
+	i32.add)
+)
+;;    0:	 55                   	push	rbp
+;;    1:	 4889e5               	mov	rbp, rsp
+;;    4:	 b80a000000           	mov	eax, 0xa
+;;    9:	 83c014               	add	eax, 0x14
+;;    c:	 5d                   	pop	rbp
+;;    d:	 c3                   	ret	
diff --git a/winch/filetests/filetests/x64/basic_add_with_locals.wat b/winch/filetests/filetests/x64/basic_add_with_locals.wat
new file mode 100644
index 000000000..4bd237d99
--- /dev/null
+++ b/winch/filetests/filetests/x64/basic_add_with_locals.wat
@@ -0,0 +1,32 @@
+;;! target = "x86_64"
+
+(module
+  (export "main" (func $main))
+
+  (func $main (result i32)
+        (local $foo i32)  
+        (local $bar i32)
+        (i32.const 10)
+        (local.set $foo)
+        (i32.const 20)
+        (local.set $bar)
+
+        (local.get $foo)
+        (local.get $bar)
+        i32.add)
+)
+;;    0:	 55                   	push	rbp
+;;    1:	 4889e5               	mov	rbp, rsp
+;;    4:	 4883ec08             	sub	rsp, 8
+;;    8:	 48c7042400000000     	mov	qword ptr [rsp], 0
+;;   10:	 b80a000000           	mov	eax, 0xa
+;;   15:	 89442404             	mov	dword ptr [rsp + 4], eax
+;;   19:	 b814000000           	mov	eax, 0x14
+;;   1e:	 890424               	mov	dword ptr [rsp], eax
+;;   21:	 8b0424               	mov	eax, dword ptr [rsp]
+;;   24:	 8b4c2404             	mov	ecx, dword ptr [rsp + 4]
+;;   28:	 01c1                 	add	ecx, eax
+;;   2a:	 4889c8               	mov	rax, rcx
+;;   2d:	 4883c408             	add	rsp, 8
+;;   31:	 5d                   	pop	rbp
+;;   32:	 c3                   	ret	
diff --git a/winch/filetests/filetests/x64/basic_add_with_params.wat b/winch/filetests/filetests/x64/basic_add_with_params.wat
new file mode 100644
index 000000000..d3809ac3a
--- /dev/null
+++ b/winch/filetests/filetests/x64/basic_add_with_params.wat
@@ -0,0 +1,22 @@
+;;! target = "x86_64"
+
+(module
+  (export "main" (func $main))
+
+  (func $main (param i32) (param i32) (result i32)
+        (local.get 0)
+        (local.get 1)
+        i32.add)
+)
+;;    0:	 55                   	push	rbp
+;;    1:	 4889e5               	mov	rbp, rsp
+;;    4:	 4883ec08             	sub	rsp, 8
+;;    8:	 897c2404             	mov	dword ptr [rsp + 4], edi
+;;    c:	 893424               	mov	dword ptr [rsp], esi
+;;    f:	 8b0424               	mov	eax, dword ptr [rsp]
+;;   12:	 8b4c2404             	mov	ecx, dword ptr [rsp + 4]
+;;   16:	 01c1                 	add	ecx, eax
+;;   18:	 4889c8               	mov	rax, rcx
+;;   1b:	 4883c408             	add	rsp, 8
+;;   1f:	 5d                   	pop	rbp
+;;   20:	 c3                   	ret