Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

amd64: saves clobbered xmm8-xmm15 beyond runtime.memmove #2202

Merged
merged 2 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions internal/engine/wazevo/backend/isa/amd64/instr.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,8 +298,8 @@ func (i *instruction) String() string {
}
return fmt.Sprintf("lock xadd.%s %s, %s", suffix, i.op1.format(true), i.op2.format(true))

case keepAlive:
return fmt.Sprintf("keepAlive %s", i.op1.format(true))
case nopUseReg:
return fmt.Sprintf("nop_use_reg %s", i.op1.format(true))

default:
panic(fmt.Sprintf("BUG: %d", int(i.kind)))
Expand Down Expand Up @@ -860,8 +860,8 @@ const (
// lockxadd is xadd https://www.felixcloutier.com/x86/xadd with a lock prefix.
lockxadd

// keepAlive is a meta instruction that uses one register and does nothing.
keepAlive
// nopUseReg is a meta instruction that uses one register and does nothing.
nopUseReg

instrMax
)
Expand All @@ -871,8 +871,8 @@ func (i *instruction) asMFence() *instruction {
return i
}

func (i *instruction) asKeepAlive(r regalloc.VReg) *instruction {
i.kind = keepAlive
func (i *instruction) asNopUseReg(r regalloc.VReg) *instruction {
i.kind = nopUseReg
i.op1 = newOperandReg(r)
return i
}
Expand Down Expand Up @@ -2366,7 +2366,7 @@ var defKinds = [instrMax]defKind{
lockcmpxchg: defKindNone,
lockxadd: defKindNone,
neg: defKindNone,
keepAlive: defKindNone,
nopUseReg: defKindNone,
}

// String implements fmt.Stringer.
Expand Down Expand Up @@ -2449,7 +2449,7 @@ var useKinds = [instrMax]useKind{
lockcmpxchg: useKindRaxOp1RegOp2,
lockxadd: useKindOp1RegOp2,
neg: useKindOp1,
keepAlive: useKindOp1,
nopUseReg: useKindOp1,
}

func (u useKind) String() string {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

func (i *instruction) encode(c backend.Compiler) (needsLabelResolution bool) {
switch kind := i.kind; kind {
case nop0, sourceOffsetInfo, defineUninitializedReg, fcvtToSintSequence, fcvtToUintSequence, keepAlive:
case nop0, sourceOffsetInfo, defineUninitializedReg, fcvtToSintSequence, fcvtToUintSequence, nopUseReg:
case ret:
encodeRet(c)
case imm:
Expand Down
20 changes: 18 additions & 2 deletions internal/engine/wazevo/backend/isa/amd64/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -1051,7 +1051,7 @@ func (m *machine) lowerAtomicRmw(op ssa.AtomicRmwOp, addr, val ssa.Value, size u
}

// valCopied must be alive at the end of the loop.
m.insert(m.allocateInstr().asKeepAlive(valCopied))
m.insert(m.allocateInstr().asNopUseReg(valCopied))

// At this point, accumulator contains the result.
m.clearHigherBitsForAtomic(accumulator, size, ret.Type())
Expand Down Expand Up @@ -1758,10 +1758,11 @@ func (m *machine) lowerCall(si *ssa.Instruction) {
var directCallee ssa.FuncRef
var sigID ssa.SignatureID
var args []ssa.Value
var isMemmove bool
if isDirectCall {
directCallee, sigID, args = si.CallData()
} else {
indirectCalleePtr, sigID, args = si.CallIndirectData()
indirectCalleePtr, sigID, args, isMemmove = si.CallIndirectData()
}
calleeABI := m.c.GetFunctionABI(m.c.SSABuilder().ResolveSignature(sigID))

Expand All @@ -1779,6 +1780,15 @@ func (m *machine) lowerCall(si *ssa.Instruction) {
m.callerGenVRegToFunctionArg(calleeABI, i, reg, def, stackSlotSize)
}

if isMemmove {
// Go's memmove *might* use all xmm0-xmm15, so we need to release them.
// https://github.com/golang/go/blob/49d42128fd8594c172162961ead19ac95e247d24/src/cmd/compile/abi-internal.md#architecture-specifics
// https://github.com/golang/go/blob/49d42128fd8594c172162961ead19ac95e247d24/src/runtime/memmove_amd64.s#L271-L286
for i := regalloc.RealReg(0); i < 16; i++ {
m.insert(m.allocateInstr().asDefineUninitializedReg(regInfo.RealRegToVReg[xmm0+i]))
}
}

if isDirectCall {
call := m.allocateInstr().asCall(directCallee, calleeABI)
m.insert(call)
Expand All @@ -1788,6 +1798,12 @@ func (m *machine) lowerCall(si *ssa.Instruction) {
m.insert(callInd)
}

if isMemmove {
for i := regalloc.RealReg(0); i < 16; i++ {
m.insert(m.allocateInstr().asNopUseReg(regInfo.RealRegToVReg[xmm0+i]))
}
}

var index int
r1, rs := si.Returns()
if r1.Valid() {
Expand Down
2 changes: 1 addition & 1 deletion internal/engine/wazevo/backend/isa/arm64/abi.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ func (m *machine) lowerCall(si *ssa.Instruction) {
if isDirectCall {
directCallee, sigID, args = si.CallData()
} else {
indirectCalleePtr, sigID, args = si.CallIndirectData()
indirectCalleePtr, sigID, args, _ /* on arm64, the calling convention is compatible with the Go runtime */ = si.CallIndirectData()
}
calleeABI := m.compiler.GetFunctionABI(m.compiler.SSABuilder().ResolveSignature(sigID))

Expand Down
2 changes: 1 addition & 1 deletion internal/engine/wazevo/frontend/lower.go
Original file line number Diff line number Diff line change
Expand Up @@ -3743,7 +3743,7 @@ func (c *Compiler) callMemmove(dst, src, size ssa.Value) {
wazevoapi.ExecutionContextOffsetMemmoveAddress.U32(),
ssa.TypeI64,
).Insert(builder).Return()
builder.AllocateInstruction().AsCallIndirect(memmovePtr, &c.memmoveSig, args).Insert(builder)
builder.AllocateInstruction().AsCallGoRuntimeMemmove(memmovePtr, &c.memmoveSig, args).Insert(builder)
}

func (c *Compiler) reloadAfterCall() {
Expand Down
10 changes: 9 additions & 1 deletion internal/engine/wazevo/ssa/instructions.go
Original file line number Diff line number Diff line change
Expand Up @@ -2180,14 +2180,22 @@ func (i *Instruction) AsCallIndirect(funcPtr Value, sig *Signature, args Values)
return i
}

// AsCallGoRuntimeMemmove is the same as AsCallIndirect, but with a special flag set to indicate that it is a call to the Go runtime memmove function.
func (i *Instruction) AsCallGoRuntimeMemmove(funcPtr Value, sig *Signature, args Values) *Instruction {
i.AsCallIndirect(funcPtr, sig, args)
i.u2 = 1
return i
}

// CallIndirectData returns the call indirect data for this instruction necessary for backends.
func (i *Instruction) CallIndirectData() (funcPtr Value, sigID SignatureID, args []Value) {
func (i *Instruction) CallIndirectData() (funcPtr Value, sigID SignatureID, args []Value, isGoMemmove bool) {
if i.opcode != OpcodeCallIndirect {
panic("BUG: CallIndirectData only available for OpcodeCallIndirect")
}
funcPtr = i.v
sigID = SignatureID(i.u1)
args = i.vs.View()
isGoMemmove = i.u2 == 1
return
}

Expand Down
3 changes: 0 additions & 3 deletions internal/integration_test/fuzzcases/fuzzcases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1082,8 +1082,5 @@ func Test2201(t *testing.T) {
if !platform.CompilerSupported() {
return
}
if runtime.GOARCH == "amd64" {
t.Skip("TODO: #2198")
}
nodiff.RequireNoDiffT(t, getWasmBinary(t, "2201"), false, false)
}
Loading