]> Cypherpunks repositories - gostls13.git/commitdiff
[dev.simd] cmd/compile: use the right type for spill slot
authorCherry Mui <cherryyz@google.com>
Tue, 16 Sep 2025 01:27:19 +0000 (21:27 -0400)
committerCherry Mui <cherryyz@google.com>
Tue, 16 Sep 2025 14:21:17 +0000 (07:21 -0700)
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 <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
src/cmd/compile/internal/ssa/regalloc.go
src/cmd/compile/internal/ssa/value.go
src/cmd/internal/testdir/testdir_test.go
test/simd/bug1.go [new file with mode: 0644]

index e43e544bd5a786d4d8456e48cf804d4ef99ce3a3..7ed5bda28cee11f8ec07b507105e199656a6c8de 100644 (file)
@@ -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 {
index ba28a7b92888fb95431d8ec90666380c0f8f1854..4d0c4fb50f843cb8e50db38fb4b4773493045e33 100644 (file)
@@ -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
        }
index 5781276afadba7ebb9a013f3d70875f6559949e5..f502a2cd3166185eba2fab62df8d2a820eb63125 100644 (file)
@@ -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 (file)
index 0000000..dd450df
--- /dev/null
@@ -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]))))
+       }
+}