Repository: tetratelabs/wazero
Pull request #1981 (2024-01-30 18:00:06)

wazevo(amd64): fixes XOR in zero outs


Patch:
diff --git a/internal/engine/wazevo/backend/isa/amd64/instr.go b/internal/engine/wazevo/backend/isa/amd64/instr.go
index 23e88c27..e1194797 100644
--- a/internal/engine/wazevo/backend/isa/amd64/instr.go
+++ b/internal/engine/wazevo/backend/isa/amd64/instr.go
@@ -237,6 +237,8 @@ func (i *instruction) String() string {
 		return fmt.Sprintf("v128ConstIsland (%#x, %#x)", i.u1, i.u2)
 	case xchg:
 		return fmt.Sprintf("xchg %s, %s", i.op1.format(true), i.op2.format(true))
+	case zeros:
+		return fmt.Sprintf("xor %s, %s", i.op2.format(true), i.op2.format(true))
 	default:
 		panic(fmt.Sprintf("BUG: %d", int(i.kind)))
 	}
@@ -258,7 +260,6 @@ func (i *instruction) Defs(regs *[]regalloc.VReg) []regalloc.VReg {
 		*regs = append(*regs, rdxVReg)
 	case defKindRaxRdx:
 		*regs = append(*regs, raxVReg, rdxVReg)
-
 	default:
 		panic(fmt.Sprintf("BUG: invalid defKind \"%s\" for %s", dk, i))
 	}
@@ -664,6 +665,12 @@ const (
 	// v128ConstIsland is 16 bytes (128-bit) constant that will be loaded into an XMM.
 	v128ConstIsland
 
+	// zeros puts zeros into the destination register. This is implemented as xor reg, reg for
+	// either integer or XMM registers. The reason why we have this instruction instead of using aluRmiR
+	// is that it requires the already-defined registers. From reg alloc's perspective, this defines
+	// the destination register and takes no inputs.
+	zeros
+
 	instrMax
 )
 
@@ -757,6 +764,8 @@ func (k instructionKind) String() string {
 		return "ud2"
 	case xchg:
 		return "xchg"
+	case zeros:
+		return "zeros"
 	default:
 		panic("BUG")
 	}
@@ -870,6 +879,12 @@ func (i *instruction) asAluRmiR(op aluRmiROpcode, rm operand, rd regalloc.VReg,
 	return i
 }
 
+func (i *instruction) asZeros(dst regalloc.VReg) *instruction {
+	i.kind = zeros
+	i.op2 = newOperandReg(dst)
+	return i
+}
+
 func (i *instruction) asXmmRmR(op sseOpcode, rm operand, rd regalloc.VReg) *instruction {
 	if rm.kind != operandKindReg && rm.kind != operandKindMem {
 		panic("BUG")
@@ -1761,6 +1776,7 @@ var defKinds = [instrMax]defKind{
 	lea:             defKindOp2,
 	v128ConstIsland: defKindNone,
 	setcc:           defKindOp2,
+	zeros:           defKindOp2,
 }
 
 // String implements fmt.Stringer.
@@ -1830,6 +1846,7 @@ var useKinds = [instrMax]useKind{
 	v128ConstIsland: useKindNone,
 	jmpTableIsland:  useKindNone,
 	setcc:           useKindNone,
+	zeros:           useKindNone,
 }
 
 func (u useKind) String() string {
diff --git a/internal/engine/wazevo/backend/isa/amd64/instr_encoding.go b/internal/engine/wazevo/backend/isa/amd64/instr_encoding.go
index deb0d4f4..b088d978 100644
--- a/internal/engine/wazevo/backend/isa/amd64/instr_encoding.go
+++ b/internal/engine/wazevo/backend/isa/amd64/instr_encoding.go
@@ -1110,6 +1110,15 @@ func (i *instruction) encode(c backend.Compiler) (needsLabelResolution bool) {
 		enc1, enc2 := regEncodings[r1], regEncodings[r2]
 		encodeRegReg(c, legacyPrefixesNone, 0x87, 1, enc2, enc1, rexInfo(0).setW())
 
+	case zeros:
+		r := i.op2.r
+		if r.RegType() == regalloc.RegTypeInt {
+			i.asAluRmiR(aluRmiROpcodeXor, newOperandReg(r), r, true)
+		} else {
+			i.asXmmRmR(sseOpcodePxor, newOperandReg(r), r)
+		}
+		i.encode(c)
+
 	default:
 		panic(fmt.Sprintf("TODO: %v", i.kind))
 	}
diff --git a/internal/engine/wazevo/backend/isa/amd64/lower_constant.go b/internal/engine/wazevo/backend/isa/amd64/lower_constant.go
index 70cd082f..01e0d1db 100644
--- a/internal/engine/wazevo/backend/isa/amd64/lower_constant.go
+++ b/internal/engine/wazevo/backend/isa/amd64/lower_constant.go
@@ -38,7 +38,7 @@ func (m *machine) InsertLoadConstant(instr *ssa.Instruction, vr regalloc.VReg) {
 
 func (m *machine) lowerFconst(dst regalloc.VReg, c uint64, _64 bool) {
 	if c == 0 {
-		xor := m.allocateInstr().asXmmUnaryRmR(sseOpcodeXorpd, newOperandReg(dst), dst)
+		xor := m.allocateInstr().asZeros(dst)
 		m.insert(xor)
 	} else {
 		var tmpType ssa.Type
@@ -58,6 +58,10 @@ func (m *machine) lowerFconst(dst regalloc.VReg, c uint64, _64 bool) {
 
 func (m *machine) lowerIconst(dst regalloc.VReg, c uint64, _64 bool) {
 	i := m.allocateInstr()
-	i.asImm(dst, c, _64)
+	if c == 0 {
+		i.asZeros(dst)
+	} else {
+		i.asImm(dst, c, _64)
+	}
 	m.insert(i)
 }
diff --git a/internal/engine/wazevo/backend/isa/amd64/machine.go b/internal/engine/wazevo/backend/isa/amd64/machine.go
index edbcaffe..6900db3b 100644
--- a/internal/engine/wazevo/backend/isa/amd64/machine.go
+++ b/internal/engine/wazevo/backend/isa/amd64/machine.go
@@ -1482,9 +1482,8 @@ func (m *machine) lowerIDivRem(si *ssa.Instruction, isDiv bool, signed bool) {
 		m.insert(sed)
 	} else {
 		// Zeros DX register to have 2*x.Type().Bits() dividend over DX and AX registers.
-		xor := m.allocateInstr()
-		xor.asAluRmiR(aluRmiROpcodeXor, newOperandReg(rdxVReg), rdxVReg, _64)
-		m.insert(xor)
+		zeros := m.allocateInstr().asZeros(rdxVReg)
+		m.insert(zeros)
 	}
 
 	div := m.allocateInstr()


Test patch:
diff --git a/internal/engine/wazevo/backend/isa/amd64/instr_encoding_test.go b/internal/engine/wazevo/backend/isa/amd64/instr_encoding_test.go
index e419bd5b..23f00cd5 100644
--- a/internal/engine/wazevo/backend/isa/amd64/instr_encoding_test.go
+++ b/internal/engine/wazevo/backend/isa/amd64/instr_encoding_test.go
@@ -3039,6 +3039,26 @@ func TestInstruction_format_encode(t *testing.T) {
 			want:       "4887f3",
 			wantFormat: "xchg %rbx, %rsi",
 		},
+		{
+			setup:      func(i *instruction) { i.asZeros(rbxVReg) },
+			want:       "4831db",
+			wantFormat: "xor %rbx, %rbx",
+		},
+		{
+			setup:      func(i *instruction) { i.asZeros(r14VReg) },
+			want:       "4d31f6",
+			wantFormat: "xor %r14, %r14",
+		},
+		{
+			setup:      func(i *instruction) { i.asZeros(xmm1VReg) },
+			want:       "660fefc9",
+			wantFormat: "xor %xmm1, %xmm1",
+		},
+		{
+			setup:      func(i *instruction) { i.asZeros(xmm12VReg) },
+			want:       "66450fefe4",
+			wantFormat: "xor %xmm12, %xmm12",
+		},
 	} {
 		tc := tc
 		t.Run(tc.wantFormat, func(t *testing.T) {
diff --git a/internal/engine/wazevo/backend/isa/amd64/lower_mem_test.go b/internal/engine/wazevo/backend/isa/amd64/lower_mem_test.go
index 038d0833..421ab286 100644
--- a/internal/engine/wazevo/backend/isa/amd64/lower_mem_test.go
+++ b/internal/engine/wazevo/backend/isa/amd64/lower_mem_test.go
@@ -95,7 +95,7 @@ func TestMachine_lowerToAddressMode(t *testing.T) {
 				return ishl.Return(), 1 << 30
 			},
 			insts: []string{
-				"movabsq $0, %r100?",
+				"xor %r100?, %r100?",
 			},
 			am: newAmodeRegRegShift(1<<30, nextVReg, raxVReg, 2),
 		},
@@ -313,7 +313,7 @@ func TestMachine_lowerAddendsToAmode(t *testing.T) {
 			x:    addend{r: x1, shift: 3}, y: addend{r: regalloc.VRegInvalid},
 			offset: 1 << 30,
 			insts: []string{
-				"movabsq $0, %r100?",
+				"xor %r100?, %r100?",
 			},
 			exp: newAmodeRegRegShift(1<<30, nextVReg, x1, 3),
 		},
@@ -322,7 +322,7 @@ func TestMachine_lowerAddendsToAmode(t *testing.T) {
 			x:    addend{r: regalloc.VRegInvalid}, y: addend{r: x1, shift: 3},
 			offset: 1 << 30,
 			insts: []string{
-				"movabsq $0, %r100?",
+				"xor %r100?, %r100?",
 			},
 			exp: newAmodeRegRegShift(1<<30, nextVReg, x1, 3),
 		},
diff --git a/internal/engine/wazevo/e2e_test.go b/internal/engine/wazevo/e2e_test.go
index 5ff705fc..7244107d 100644
--- a/internal/engine/wazevo/e2e_test.go
+++ b/internal/engine/wazevo/e2e_test.go
@@ -1266,7 +1266,7 @@ func TestSpectestV1(t *testing.T) {
 		{name: "int_exprs"},
 		{name: "int_literals"},
 		{name: "labels"},
-		//{name: "left-to-right"},
+		{name: "left-to-right"},
 		{name: "linking"},
 		{name: "load"},
 		{name: "loop"},