From e97ab0a0acec9281dbca086d99d965100a196100 Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Thu, 13 Aug 2015 12:25:19 -0700 Subject: [PATCH] cmd/compile: remove stale register use array The reg[] array in .../gc is where truth lies. The copy in .../ARCH is incorrect as it is mostly not updated to reflect regalloc decisions. This bug was introduced in the rewrite https://go-review.googlesource.com/#/c/7853/. The new reg[] array was introduced in .../gc but not all of the uses were removed in the .../ARCH directories. Fixes #12133 Change-Id: I6364fc403cdab92d802d17f2913ba1607734037c Reviewed-on: https://go-review.googlesource.com/13630 Reviewed-by: Russ Cox --- src/cmd/compile/internal/amd64/ggen.go | 8 ++++---- src/cmd/compile/internal/amd64/reg.go | 2 -- src/cmd/compile/internal/arm64/ggen.go | 6 +++--- src/cmd/compile/internal/arm64/reg.go | 2 -- src/cmd/compile/internal/gc/gsubr.go | 7 +++++++ src/cmd/compile/internal/x86/ggen.go | 4 ++-- src/cmd/compile/internal/x86/reg.go | 2 -- test/fixedbugs/issue12133.go | 26 ++++++++++++++++++++++++++ 8 files changed, 42 insertions(+), 15 deletions(-) create mode 100644 test/fixedbugs/issue12133.go diff --git a/src/cmd/compile/internal/amd64/ggen.go b/src/cmd/compile/internal/amd64/ggen.go index 6425633818..65cf6947be 100644 --- a/src/cmd/compile/internal/amd64/ggen.go +++ b/src/cmd/compile/internal/amd64/ggen.go @@ -306,7 +306,7 @@ func dodiv(op int, nl *gc.Node, nr *gc.Node, res *gc.Node) { * known to be dead. */ func savex(dr int, x *gc.Node, oldx *gc.Node, res *gc.Node, t *gc.Type) { - r := reg[dr] + r := uint8(gc.GetReg(dr)) // save current ax and dx if they are live // and not the destination @@ -319,14 +319,14 @@ func savex(dr int, x *gc.Node, oldx *gc.Node, res *gc.Node, t *gc.Type) { gmove(x, oldx) x.Type = t oldx.Etype = r // squirrel away old r value - reg[dr] = 1 + gc.SetReg(dr, 1) } } func restx(x *gc.Node, oldx *gc.Node) { if oldx.Op != 0 { x.Type = gc.Types[gc.TINT64] - reg[x.Reg] = oldx.Etype + gc.SetReg(int(x.Reg), int(oldx.Etype)) gmove(oldx, x) gc.Regfree(oldx) } @@ -411,7 +411,7 @@ func cgen_shift(op int, bounded bool, nl *gc.Node, nr *gc.Node, res *gc.Node) { nr = &n5 } - rcx := int(reg[x86.REG_CX]) + rcx := gc.GetReg(x86.REG_CX) var n1 gc.Node gc.Nodreg(&n1, gc.Types[gc.TUINT32], x86.REG_CX) diff --git a/src/cmd/compile/internal/amd64/reg.go b/src/cmd/compile/internal/amd64/reg.go index 7d4f40641d..8fab6399b1 100644 --- a/src/cmd/compile/internal/amd64/reg.go +++ b/src/cmd/compile/internal/amd64/reg.go @@ -40,8 +40,6 @@ const ( NREGVAR = 32 ) -var reg [x86.MAXREG]uint8 - var regname = []string{ ".AX", ".CX", diff --git a/src/cmd/compile/internal/arm64/ggen.go b/src/cmd/compile/internal/arm64/ggen.go index 6b0b40efbd..ff223087a1 100644 --- a/src/cmd/compile/internal/arm64/ggen.go +++ b/src/cmd/compile/internal/arm64/ggen.go @@ -418,7 +418,7 @@ func clearfat(nl *gc.Node) { c := uint64(w % 8) // bytes q := uint64(w / 8) // dwords - if reg[arm64.REGRT1-arm64.REG_R0] > 0 { + if gc.GetReg(arm64.REGRT1) > 0 { gc.Fatal("R%d in use during clearfat", arm64.REGRT1-arm64.REG_R0) } @@ -426,7 +426,7 @@ func clearfat(nl *gc.Node) { gc.Nodreg(&r0, gc.Types[gc.TUINT64], arm64.REGZERO) var dst gc.Node gc.Nodreg(&dst, gc.Types[gc.Tptr], arm64.REGRT1) - reg[arm64.REGRT1-arm64.REG_R0]++ + gc.SetReg(arm64.REGRT1, gc.GetReg(arm64.REGRT1)+1) gc.Agen(nl, &dst) var boff uint64 @@ -485,7 +485,7 @@ func clearfat(nl *gc.Node) { p.To.Offset = int64(t + boff) } - reg[arm64.REGRT1-arm64.REG_R0]-- + gc.SetReg(arm64.REGRT1, gc.GetReg(arm64.REGRT1)-1) } // Called after regopt and peep have run. diff --git a/src/cmd/compile/internal/arm64/reg.go b/src/cmd/compile/internal/arm64/reg.go index 7bc756b7bf..b84359a637 100644 --- a/src/cmd/compile/internal/arm64/reg.go +++ b/src/cmd/compile/internal/arm64/reg.go @@ -39,8 +39,6 @@ const ( NREGVAR = 64 /* 32 general + 32 floating */ ) -var reg [arm64.NREG + arm64.NFREG]uint8 - var regname = []string{ ".R0", ".R1", diff --git a/src/cmd/compile/internal/gc/gsubr.go b/src/cmd/compile/internal/gc/gsubr.go index 14dc927442..2c575f3d78 100644 --- a/src/cmd/compile/internal/gc/gsubr.go +++ b/src/cmd/compile/internal/gc/gsubr.go @@ -602,6 +602,13 @@ func unpatch(p *obj.Prog) *obj.Prog { var reg [100]int // count of references to reg var regstk [100][]byte // allocation sites, when -v is given +func GetReg(r int) int { + return reg[r-Thearch.REGMIN] +} +func SetReg(r, v int) { + reg[r-Thearch.REGMIN] = v +} + func ginit() { for r := range reg { reg[r] = 1 diff --git a/src/cmd/compile/internal/x86/ggen.go b/src/cmd/compile/internal/x86/ggen.go index dabc139f30..ae9881d273 100644 --- a/src/cmd/compile/internal/x86/ggen.go +++ b/src/cmd/compile/internal/x86/ggen.go @@ -319,7 +319,7 @@ func dodiv(op int, nl *gc.Node, nr *gc.Node, res *gc.Node, ax *gc.Node, dx *gc.N } func savex(dr int, x *gc.Node, oldx *gc.Node, res *gc.Node, t *gc.Type) { - r := int(reg[dr]) + r := gc.GetReg(dr) gc.Nodreg(x, gc.Types[gc.TINT32], dr) // save current ax and dx if they are live @@ -408,7 +408,7 @@ func cgen_shift(op int, bounded bool, nl *gc.Node, nr *gc.Node, res *gc.Node) { var oldcx gc.Node var cx gc.Node gc.Nodreg(&cx, gc.Types[gc.TUINT32], x86.REG_CX) - if reg[x86.REG_CX] > 1 && !gc.Samereg(&cx, res) { + if gc.GetReg(x86.REG_CX) > 1 && !gc.Samereg(&cx, res) { gc.Tempname(&oldcx, gc.Types[gc.TUINT32]) gmove(&cx, &oldcx) } diff --git a/src/cmd/compile/internal/x86/reg.go b/src/cmd/compile/internal/x86/reg.go index 8c97171e47..b3a5fdf4e0 100644 --- a/src/cmd/compile/internal/x86/reg.go +++ b/src/cmd/compile/internal/x86/reg.go @@ -37,8 +37,6 @@ const ( NREGVAR = 16 /* 8 integer + 8 floating */ ) -var reg [x86.MAXREG]uint8 - var regname = []string{ ".ax", ".cx", diff --git a/test/fixedbugs/issue12133.go b/test/fixedbugs/issue12133.go new file mode 100644 index 0000000000..0b66c56a51 --- /dev/null +++ b/test/fixedbugs/issue12133.go @@ -0,0 +1,26 @@ +// run + +// Copyright 2015 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. + +// Issue 12133. The CX register was getting clobbered +// because we did not keep track of its allocation correctly. + +package main + +import "fmt" + +func main() { + want := uint(48) + got := f1(48) + if got != want { + fmt.Println("got", got, ", wanted", want) + panic("bad") + } +} +func f1(v1 uint) uint { + switch { + } // prevent inlining + return v1 >> ((1 >> v1) + (1 >> v1)) +} -- 2.48.1