[release-branch.go1.13] cmd/compile: fix deallocation of live value copies in regalloc
When deallocating the input register to a phi so that the phi
itself could be allocated to that register the code was also
deallocating all copies of that phi input value. Those copies
of the value could still be live and if they were the register
allocator could reuse them incorrectly to hold speculative
copies of other phi inputs. This causes strange bugs.
No test because this is a very obscure scenario that is hard
to replicate but CL 228060 adds an assertion to the compiler
that does trigger when running the std tests on linux/s390x
without this CL applied. Hopefully that assertion will prevent
future regressions.
Fixes #38442.
Change-Id: Id975dadedd731c7bb21933b9ea6b17daaa5c9e1d
Reviewed-on: https://go-review.googlesource.com/c/go/+/228061
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 382fe3e2498f2066400e7e7007aa9903440e339d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/230358
diff --git a/src/cmd/compile/internal/ssa/regalloc.go b/src/cmd/compile/internal/ssa/regalloc.go
index 8abbf615..e92ab00 100644
--- a/src/cmd/compile/internal/ssa/regalloc.go
+++ b/src/cmd/compile/internal/ssa/regalloc.go
@@ -976,25 +976,22 @@
}
}
- // Second pass - deallocate any phi inputs which are now dead.
+ // Second pass - deallocate all in-register phi inputs.
for i, v := range phis {
if !s.values[v.ID].needReg {
continue
}
a := v.Args[idx]
- if !regValLiveSet.contains(a.ID) {
- // Input is dead beyond the phi, deallocate
- // anywhere else it might live.
- s.freeRegs(s.values[a.ID].regs)
- } else {
- // Input is still live.
+ r := phiRegs[i]
+ if r == noRegister {
+ continue
+ }
+ if regValLiveSet.contains(a.ID) {
+ // Input value is still live (it is used by something other than Phi).
// Try to move it around before kicking out, if there is a free register.
// We generate a Copy in the predecessor block and record it. It will be
- // deleted if never used.
- r := phiRegs[i]
- if r == noRegister {
- continue
- }
+ // deleted later if never used.
+ //
// Pick a free register. At this point some registers used in the predecessor
// block may have been deallocated. Those are the ones used for Phis. Exclude
// them (and they are not going to be helpful anyway).
@@ -1010,8 +1007,8 @@
s.assignReg(r2, a, c)
s.endRegs[p.ID] = append(s.endRegs[p.ID], endReg{r2, a, c})
}
- s.freeReg(r)
}
+ s.freeReg(r)
}
// Copy phi ops into new schedule.
@@ -1842,6 +1839,11 @@
e.process()
}
}
+
+ if s.f.pass.debug > regDebug {
+ fmt.Printf("post shuffle %s\n", s.f.Name)
+ fmt.Println(s.f.String())
+ }
}
type edgeState struct {
