From be443ee8bc64e93bc56d760f3e28f4318c00a8bd Mon Sep 17 00:00:00 2001
From: Russ Cox <rsc@golang.org>
Date: Sun, 12 Sep 2010 00:06:45 -0400
Subject: [PATCH] 5g: register allocation bugs

Fixes #1099.

R=ken2, r
CC=golang-dev
https://golang.org/cl/2147047
---
 src/cmd/5g/cgen.c                            | 23 +++++++---
 src/cmd/5g/ggen.c                            | 17 +++++++-
 src/cmd/5g/gsubr.c                           | 44 ++++++++++++++++++--
 src/cmd/gc/gen.c                             |  7 ++++
 src/pkg/compress/flate/huffman_bit_writer.go | 10 +----
 src/pkg/gob/decode.go                        | 11 ++---
 src/pkg/log/log.go                           |  6 +--
 src/pkg/net/ip.go                            | 14 ++-----
 src/pkg/time/format.go                       | 16 +++----
 src/pkg/xml/xml.go                           | 10 +----
 10 files changed, 98 insertions(+), 60 deletions(-)

diff --git a/src/cmd/5g/cgen.c b/src/cmd/5g/cgen.c
index 310ea99c5b8..9306436ee6d 100644
--- a/src/cmd/5g/cgen.c
+++ b/src/cmd/5g/cgen.c
@@ -496,6 +496,7 @@ agen(Node *n, Node *res)
 	Prog *p1, *p2;
 	uint32 w;
 	uint64 v;
+	int r;
 
 	if(debug['g']) {
 		dump("\nagen-res", res);
@@ -527,7 +528,22 @@ agen(Node *n, Node *res)
 		break;
 
 	case OCALLMETH:
-		cgen_callmeth(n, 0);
+	case OCALLFUNC:
+		// Release res so that it is available for cgen_call.
+		// Pick it up again after the call.
+		r = -1;
+		if(n->ullman >= UINF) {
+			if(res->op == OREGISTER || res->op == OINDREG) {
+				r = res->val.u.reg;
+				reg[r]--;
+			}
+		}
+		if(n->op == OCALLMETH)
+			cgen_callmeth(n, 0);
+		else
+			cgen_call(n, 0);
+		if(r >= 0)
+			reg[r]++;
 		cgen_aret(n, res);
 		break;
 
@@ -536,11 +552,6 @@ agen(Node *n, Node *res)
 		cgen_aret(n, res);
 		break;
 
-	case OCALLFUNC:
-		cgen_call(n, 0);
-		cgen_aret(n, res);
-		break;
-
 	case OINDEX:
 		p2 = nil;  // to be patched to panicindex.
 		w = n->type->width;
diff --git a/src/cmd/5g/ggen.c b/src/cmd/5g/ggen.c
index c906064584e..77d4e9496f1 100644
--- a/src/cmd/5g/ggen.c
+++ b/src/cmd/5g/ggen.c
@@ -68,6 +68,7 @@ compile(Node *fn)
 	ptxt = gins(ATEXT, curfn->nname, &nod1);
 	afunclit(&ptxt->from);
 
+	ginit();
 	genlist(curfn->enter);
 	
 	pret = nil;
@@ -80,6 +81,7 @@ compile(Node *fn)
 	}
 
 	genlist(curfn->nbody);
+	gclean();
 	checklabels();
 	if(nerrors != 0)
 		goto ret;
@@ -89,10 +91,12 @@ compile(Node *fn)
 
 	if(pret)
 		patch(pret, pc);
+	ginit();
 	if(hasdefer)
 		ginscall(deferreturn, 0);
 	if(curfn->exit)
 		genlist(curfn->exit);
+	gclean();
 	if(nerrors != 0)
 		goto ret;
 	if(curfn->endlineno)
@@ -207,6 +211,7 @@ ginscall(Node *f, int proc)
 void
 cgen_callinter(Node *n, Node *res, int proc)
 {
+	int r;
 	Node *i, *f;
 	Node tmpi, nodo, nodr, nodsp;
 
@@ -220,6 +225,14 @@ cgen_callinter(Node *n, Node *res, int proc)
 
 	i = i->left;		// interface
 
+	// Release res register during genlist and cgen,
+	// which might have their own function calls.
+	r = -1;
+	if(res != N && (res->op == OREGISTER || res->op == OINDREG)) {
+		r = res->val.u.reg;
+		reg[r]--;
+	}
+
 	if(!i->addable) {
 		tempname(&tmpi, i->type);
 		cgen(i, &tmpi);
@@ -227,6 +240,8 @@ cgen_callinter(Node *n, Node *res, int proc)
 	}
 
 	genlist(n->list);			// args
+	if(r >= 0)
+		reg[r]++;
 
 	regalloc(&nodr, types[tptr], res);
 	regalloc(&nodo, types[tptr], &nodr);
@@ -548,7 +563,7 @@ cgen_shift(int op, Node *nl, Node *nr, Node *res)
 		cgen(nl, &n1);
 		sc = mpgetfix(nr->val.u.xval);
 		if(sc == 0) {
-			return;
+			// nothing to do
 		} else if(sc >= nl->type->width*8) {
 			if(op == ORSH && issigned[nl->type->etype])
 				gshift(AMOVW, &n1, SHIFT_AR, w, &n1);
diff --git a/src/cmd/5g/gsubr.c b/src/cmd/5g/gsubr.c
index 584a0f62b84..88837c35676 100644
--- a/src/cmd/5g/gsubr.c
+++ b/src/cmd/5g/gsubr.c
@@ -197,9 +197,50 @@ afunclit(Addr *a)
 	}
 }
 
+static	int	resvd[] =
+{
+	9,	// reserved for m
+	10,	// reserved for g
+};
+
+void
+ginit(void)
+{
+	int i;
+
+	for(i=0; i<nelem(reg); i++)
+		reg[i] = 0;
+	for(i=0; i<nelem(resvd); i++)
+		reg[resvd[i]]++;
+}
+
+void
+gclean(void)
+{
+	int i;
+
+	for(i=0; i<nelem(resvd); i++)
+		reg[resvd[i]]--;
+
+	for(i=0; i<nelem(reg); i++)
+		if(reg[i])
+			yyerror("reg %R left allocated\n", i);
+}
+
 int32
 anyregalloc(void)
 {
+	int i, j;
+
+	for(i=0; i<nelem(reg); i++) {
+		if(reg[i] == 0)
+			goto ok;
+		for(j=0; j<nelem(resvd); j++)
+			if(resvd[j] == i)
+				goto ok;
+		return 1;
+	ok:;
+	}
 	return 0;
 }
 
@@ -213,9 +254,6 @@ regalloc(Node *n, Type *t, Node *o)
 {
 	int i, et, fixfree, floatfree;
 
-	// guarantee R9 and R10 (m and g) are left alone. BUG.
-	reg[9] = 1;
-	reg[10] = 1;
 	if(debug['r']) {
 		fixfree = 0;
 		for(i=REGALLOC_R0; i<=REGALLOC_RMAX; i++)
diff --git a/src/cmd/gc/gen.c b/src/cmd/gc/gen.c
index fd8a7f39b43..724f16ae610 100644
--- a/src/cmd/gc/gen.c
+++ b/src/cmd/gc/gen.c
@@ -139,8 +139,10 @@ gen(Node *n)
 	Prog *scontin, *sbreak;
 	Prog *p1, *p2, *p3;
 	Label *lab;
+	int32 wasregalloc;
 
 	lno = setlineno(n);
+	wasregalloc = anyregalloc();
 
 	if(n == N)
 		goto ret;
@@ -342,6 +344,11 @@ gen(Node *n)
 	}
 
 ret:
+	if(anyregalloc() != wasregalloc) {
+		dump("node", n);
+		fatal("registers left allocated");
+	}
+
 	lineno = lno;
 }
 
diff --git a/src/pkg/compress/flate/huffman_bit_writer.go b/src/pkg/compress/flate/huffman_bit_writer.go
index 294cdf65329..abff82dd694 100644
--- a/src/pkg/compress/flate/huffman_bit_writer.go
+++ b/src/pkg/compress/flate/huffman_bit_writer.go
@@ -110,14 +110,8 @@ func newHuffmanBitWriter(w io.Writer) *huffmanBitWriter {
 }
 
 func (err WrongValueError) String() string {
-	// BUG: work around bug in 5g by simplifying expression.
-	// return "huffmanBitWriter: " + err.name + " should belong to [" + strconv.Itoa64(int64(err.from)) + ";" +
-	// 	strconv.Itoa64(int64(err.to)) + "] but actual value is " + strconv.Itoa64(int64(err.value))
-	str := "huffmanBitWriter: " + err.name + " should belong to ["
-	str += strconv.Itoa64(int64(err.from)) + ";"
-	str += strconv.Itoa64(int64(err.to)) + "] but actual value is "
-	str += strconv.Itoa64(int64(err.value))
-	return str
+	return "huffmanBitWriter: " + err.name + " should belong to [" + strconv.Itoa64(int64(err.from)) + ";" +
+		strconv.Itoa64(int64(err.to)) + "] but actual value is " + strconv.Itoa64(int64(err.value))
 }
 
 func (w *huffmanBitWriter) flushBits() {
diff --git a/src/pkg/gob/decode.go b/src/pkg/gob/decode.go
index a91a98bf157..a70799e9a72 100644
--- a/src/pkg/gob/decode.go
+++ b/src/pkg/gob/decode.go
@@ -867,14 +867,9 @@ func (dec *Decoder) compileDec(remoteId typeId, rt reflect.Type) (engine *decEng
 			continue
 		}
 		if !dec.compatibleType(localField.Type, wireField.id) {
-			// BUG: work around bug in 5g by simplifying expression.
-			// return nil, os.ErrorString("gob: wrong type (" +
-			//	localField.Type.String() + ") for received field " +
-			//	wireStruct.name + "." + wireField.name)
-			str := "gob: wrong type ("
-			str += localField.Type.String() + ") for received field "
-			str += wireStruct.name + "." + wireField.name
-			return nil, os.ErrorString(str)
+			return nil, os.ErrorString("gob: wrong type (" +
+				localField.Type.String() + ") for received field " +
+				wireStruct.name + "." + wireField.name)
 		}
 		op, indir, err := dec.decOpFor(wireField.id, localField.Type, localField.Name)
 		if err != nil {
diff --git a/src/pkg/log/log.go b/src/pkg/log/log.go
index cfc1bd7767f..28d6204eb68 100644
--- a/src/pkg/log/log.go
+++ b/src/pkg/log/log.go
@@ -86,11 +86,7 @@ func (l *Logger) formatHeader(ns int64, calldepth int) string {
 	if l.flag&(Ldate|Ltime|Lmicroseconds) != 0 {
 		t := time.SecondsToLocalTime(ns / 1e9)
 		if l.flag&(Ldate) != 0 {
-			// BUG: work around bug in 5g by simplifying expression.
-			// h += itoa(int(t.Year), 4) + "/" + itoa(t.Month, 2) + "/" + itoa(t.Day, 2) + " "
-			h += itoa(int(t.Year), 4)
-			h += "/" + itoa(t.Month, 2)
-			h += "/" + itoa(t.Day, 2) + " "
+			h += itoa(int(t.Year), 4) + "/" + itoa(t.Month, 2) + "/" + itoa(t.Day, 2) + " "
 		}
 		if l.flag&(Ltime|Lmicroseconds) != 0 {
 			h += itoa(t.Hour, 2) + ":" + itoa(t.Minute, 2) + ":" + itoa(t.Second, 2)
diff --git a/src/pkg/net/ip.go b/src/pkg/net/ip.go
index 29a72971f5a..e82224a2836 100644
--- a/src/pkg/net/ip.go
+++ b/src/pkg/net/ip.go
@@ -200,16 +200,10 @@ func (ip IP) String() string {
 
 	// If IPv4, use dotted notation.
 	if p4 := p.To4(); len(p4) == 4 {
-		// BUG: work around bug in 5g by simplifying expression.
-		// return itod(uint(p4[0])) + "." +
-		//	itod(uint(p4[1])) + "." +
-		//	itod(uint(p4[2])) + "." +
-		//	itod(uint(p4[3]))
-		str := itod(uint(p4[0])) + "."
-		str += itod(uint(p4[1])) + "."
-		str += itod(uint(p4[2])) + "."
-		str += itod(uint(p4[3]))
-		return str
+		return itod(uint(p4[0])) + "." +
+			itod(uint(p4[1])) + "." +
+			itod(uint(p4[2])) + "." +
+			itod(uint(p4[3]))
 	}
 	if len(p) != IPv6len {
 		return "?"
diff --git a/src/pkg/time/format.go b/src/pkg/time/format.go
index bc94cb78e24..355721e1839 100644
--- a/src/pkg/time/format.go
+++ b/src/pkg/time/format.go
@@ -356,17 +356,11 @@ type ParseError struct {
 // String is the string representation of a ParseError.
 func (e *ParseError) String() string {
 	if e.Message == "" {
-		// BUG: work around bug in 5g by simplifying expression.
-		// return "parsing time " +
-		//	strconv.Quote(e.Value) + " as " +
-		//	strconv.Quote(e.Layout) + ": cannot parse " +
-		//	strconv.Quote(e.ValueElem) + " as " +
-		//	strconv.Quote(e.LayoutElem)
-		str := "parsing time " + strconv.Quote(e.Value) + " as "
-		str += strconv.Quote(e.Layout) + ": cannot parse "
-		str += strconv.Quote(e.ValueElem) + " as "
-		str += strconv.Quote(e.LayoutElem)
-		return str
+		return "parsing time " +
+			strconv.Quote(e.Value) + " as " +
+			strconv.Quote(e.Layout) + ": cannot parse " +
+			strconv.Quote(e.ValueElem) + " as " +
+			strconv.Quote(e.LayoutElem)
 	}
 	return "parsing time " +
 		strconv.Quote(e.Value) + e.Message
diff --git a/src/pkg/xml/xml.go b/src/pkg/xml/xml.go
index c72d726c76e..cd67f6e2657 100644
--- a/src/pkg/xml/xml.go
+++ b/src/pkg/xml/xml.go
@@ -371,14 +371,8 @@ func (p *Parser) popElement(t *EndElement) bool {
 		p.err = p.syntaxError("element <" + s.name.Local + "> closed by </" + name.Local + ">")
 		return false
 	case s.name.Space != name.Space:
-		// BUG: work around bug in 5g by simplifying expression.
-		// p.err = p.syntaxError("element <" + s.name.Local + "> in space " + s.name.Space +
-		//	"closed by </" + name.Local + "> in space " + name.Space)
-		str := "element <" + s.name.Local
-		str += "> in space " + s.name.Space
-		str += "closed by </" + name.Local
-		str += "> in space " + name.Space
-		p.err = p.syntaxError(str)
+		p.err = p.syntaxError("element <" + s.name.Local + "> in space " + s.name.Space +
+			"closed by </" + name.Local + "> in space " + name.Space)
 		return false
 	}