From: Cherry Mui Date: Tue, 16 Sep 2025 01:27:19 +0000 (-0400) Subject: [dev.simd] cmd/compile: use the right type for spill slot X-Git-Tag: go1.26rc1~147^2~76 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0e590a505d;p=gostls13.git [dev.simd] cmd/compile: use the right type for spill slot Currently, when shuffling registers, if we need to spill a register, we always create a spill slot of type int64. The type doesn't actually matter, as long as it is wide enough to hold the registers. This is no longer true with SIMD registers, which could be wider than a int64. Create the slot with the proper type instead. Change-Id: I85c82e2532001bfdefe98c9446f2dd18583d49b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/704055 TryBot-Bypass: Cherry Mui Reviewed-by: David Chase Reviewed-by: Junyang Shao --- diff --git a/src/cmd/compile/internal/ssa/regalloc.go b/src/cmd/compile/internal/ssa/regalloc.go index e43e544bd5..7ed5bda28c 100644 --- a/src/cmd/compile/internal/ssa/regalloc.go +++ b/src/cmd/compile/internal/ssa/regalloc.go @@ -2705,7 +2705,6 @@ func (e *edgeState) erase(loc Location) { // findRegFor finds a register we can use to make a temp copy of type typ. func (e *edgeState) findRegFor(typ *types.Type) Location { // Which registers are possibilities. - types := &e.s.f.Config.Types m := e.s.compatRegs(typ) // Pick a register. In priority order: @@ -2739,9 +2738,7 @@ func (e *edgeState) findRegFor(typ *types.Type) Location { if !c.rematerializeable() { x := e.p.NewValue1(c.Pos, OpStoreReg, c.Type, c) // Allocate a temp location to spill a register to. - // The type of the slot is immaterial - it will not be live across - // any safepoint. Just use a type big enough to hold any register. - t := LocalSlot{N: e.s.f.NewLocal(c.Pos, types.Int64), Type: types.Int64} + t := LocalSlot{N: e.s.f.NewLocal(c.Pos, c.Type), Type: c.Type} // TODO: reuse these slots. They'll need to be erased first. e.set(t, vid, x, false, c.Pos) if e.s.f.pass.debug > regDebug { diff --git a/src/cmd/compile/internal/ssa/value.go b/src/cmd/compile/internal/ssa/value.go index ba28a7b928..4d0c4fb50f 100644 --- a/src/cmd/compile/internal/ssa/value.go +++ b/src/cmd/compile/internal/ssa/value.go @@ -600,7 +600,7 @@ func (v *Value) removeable() bool { func AutoVar(v *Value) (*ir.Name, int64) { if loc, ok := v.Block.Func.RegAlloc[v.ID].(LocalSlot); ok { if v.Type.Size() > loc.Type.Size() { - v.Fatalf("spill/restore type %s doesn't fit in slot type %s", v.Type, loc.Type) + v.Fatalf("v%d: spill/restore type %v doesn't fit in slot type %v", v.ID, v.Type, loc.Type) } return loc.N, loc.Off } diff --git a/src/cmd/internal/testdir/testdir_test.go b/src/cmd/internal/testdir/testdir_test.go index 5781276afa..f502a2cd31 100644 --- a/src/cmd/internal/testdir/testdir_test.go +++ b/src/cmd/internal/testdir/testdir_test.go @@ -67,7 +67,7 @@ var ( // dirs are the directories to look for *.go files in. // TODO(bradfitz): just use all directories? - dirs = []string{".", "ken", "chan", "interface", "internal/runtime/sys", "syntax", "dwarf", "fixedbugs", "codegen", "abi", "typeparam", "typeparam/mdempsky", "arenas"} + dirs = []string{".", "ken", "chan", "interface", "internal/runtime/sys", "syntax", "dwarf", "fixedbugs", "codegen", "abi", "typeparam", "typeparam/mdempsky", "arenas", "simd"} ) // Test is the main entrypoint that runs tests in the GOROOT/test directory. diff --git a/test/simd/bug1.go b/test/simd/bug1.go new file mode 100644 index 0000000000..dd450df439 --- /dev/null +++ b/test/simd/bug1.go @@ -0,0 +1,81 @@ +// compile + +//go:build amd64 && goexperiment.simd + +// Copyright 2025 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +// Test case for ICE on picking the wrong type for the spill slot. + +package p + +import ( + "simd" + "unsafe" +) + +func F( + dst *[2][4][4]float32, + tos *[2][4][4]float32, + blend int, +) { + tiny := simd.BroadcastFloat32x8(0) + for { + dstCol12 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[0][0:])))) + dstCol34 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[0][2:])))) + dstCol56 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[1][0:])))) + dstCol78 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[1][2:])))) + + tosCol12 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(tos[0][0:])))) + tosCol34 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(tos[0][2:])))) + tosCol56 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(tos[1][0:])))) + tosCol78 := simd.LoadFloat32x8((*[8]float32)(unsafe.Pointer((*[2][4]float32)(tos[1][2:])))) + + var Cr0, Cr1, Cr2 simd.Float32x8 + if blend != 0 { + invas := tosCol78.Max(tiny) + invad := dstCol78.Max(tiny) + Cd0 := dstCol12.Mul(invad) + Cd1 := dstCol34.Mul(invad) + Cd2 := dstCol56.Mul(invad) + Cs0 := tosCol12.Mul(invas) + Cs1 := tosCol34.Mul(invas) + Cs2 := tosCol56.Mul(invas) + var Cm0, Cm1, Cm2 simd.Float32x8 + switch blend { + case 4: + case 10: + case 11: + case 8: + case 5: + case 1: + case 0: + Cm1 = Cs1 + case 2: + Cm0 = Cd0.Add(Cs0) + Cm1 = Cd1.Add(Cs1) + Cm2 = Cd2.Add(Cs2) + } + Cr0 = dstCol78.Mul(Cs0).Mul(Cm0) + Cr1 = dstCol78.Mul(Cs1).Mul(Cm1) + Cr2 = dstCol78.Mul(Cs2).Mul(Cm2) + } + var resR, resG, resB, resA simd.Float32x8 + if blend == 0 { + resR = tosCol12 + resG = tosCol34 + resB = tosCol56 + resA = tosCol78 + } else { + resR = Cr0.Add(dstCol12) + resG = Cr1.Add(dstCol34) + resB = Cr2.Add(dstCol56) + } + + resR.Store((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[0][0:2])))) + resG.Store((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[0][2:4])))) + resB.Store((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[1][0:2])))) + resA.Store((*[8]float32)(unsafe.Pointer((*[2][4]float32)(dst[1][2:4])))) + } +}