cmd/gc: correct handling of globals, func args, results

Globals, function arguments, and results are special cases in
registerization.

Globals must be flushed aggressively, because nearly any
operation can cause a panic, and the recovery code must see
the latest values. Globals also must be loaded aggressively,
because nearly any store through a pointer might be updating a
global: the compiler cannot see all the "address of"
operations on globals, especially exported globals. To
accomplish this, mark all globals as having their address
taken, which effectively disables registerization.

If a function contains a defer statement, the function results
must be flushed aggressively, because nearly any operation can
cause a panic, and the deferred code may call recover, causing
the original function to return the current values of its
function results. To accomplish this, mark all function
results as having their address taken if the function contains
any defer statements. This causes not just aggressive flushing
but also aggressive loading. The aggressive loading is
overkill but the best we can do in the current code.

Function arguments must be considered live at all safe points
in a function, because garbage collection always preserves
them: they must be up-to-date in order to be preserved
correctly. Accomplish this by marking them live at all call
sites. An earlier attempt at this marked function arguments as
having their address taken, which disabled registerization
completely, making programs slower. This CL's solution allows
registerization while preserving safety. The benchmark speedup
is caused by being able to registerize again (the earlier CL
lost the same amount).

benchmark                old ns/op     new ns/op     delta
BenchmarkEqualPort32     61.4          56.0          -8.79%

benchmark                old MB/s     new MB/s     speedup
BenchmarkEqualPort32     521.56       570.97       1.09x

Fixes issue 1304. (again)
Fixes issue 7944. (again)
Fixes issue 7984.
Fixes issue 7995.

LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, r
https://codereview.appspot.com/97500044

1 files changed, 23 insertions, 44 deletions

diff --git a/src/cmd/6g/reg.c b/src/cmd/6g/reg.c
index a1f0c756a..919a07d7b 100644
--- a/src/cmd/6g/reg.c
+++ b/src/cmd/6g/reg.c
@@ -55,28 +55,6 @@ rcmp(const void *a1, const void *a2)
 }
 
 static void
-setvar(Bits *dst, Type **args)
-{
-	Type *t;
-	Node *n;
-	Addr a;
-	Iter save;
-	Bits bit;
-	int z;
-
-	t = structfirst(&save, args);
-	while(t != T) {
-		n = nodarg(t, 1);
-		a = zprog.from;
-		naddr(n, &a, 0);
-		bit = mkvar(R, &a);
-		for(z=0; z<BITS; z++)
-			dst->b[z] |= bit.b[z];
-		t = structnext(&save);
-	}
-}
-
-static void
 setaddrs(Bits bit)
 {
 	int i, n;
@@ -178,11 +156,6 @@ regopt(Prog *firstp)
 		ovar.b[z] = 0;
 	}
 
-	// build lists of parameters and results
-	setvar(&ivar, getthis(curfn->type));
-	setvar(&ivar, getinarg(curfn->type));
-	setvar(&ovar, getoutarg(curfn->type));
-
 	/*
 	 * pass 1
 	 * build aux data structure
@@ -690,11 +663,6 @@ mkvar(Reg *r, Adr *a)
 	v->nextinnode = node->opt;
 	node->opt = v;
 
-	if(debug['R'])
-		print("bit=%2d et=%2E w=%lld+%lld %#N %D flag=%d\n", i, et, o, w, node, a, v->addr);
-
-	ostats.nvar++;
-
 	bit = blsh(i);
 	if(n == D_EXTERN || n == D_STATIC)
 		for(z=0; z<BITS; z++)
@@ -703,6 +671,13 @@ mkvar(Reg *r, Adr *a)
 		for(z=0; z<BITS; z++)
 			params.b[z] |= bit.b[z];
 
+	if(node->class == PPARAM)
+		for(z=0; z<BITS; z++)
+			ivar.b[z] |= bit.b[z];
+	if(node->class == PPARAMOUT)
+		for(z=0; z<BITS; z++)
+			ovar.b[z] |= bit.b[z];
+
 	// Treat values with their address taken as live at calls,
 	// because the garbage collector's liveness analysis in ../gc/plive.c does.
 	// These must be consistent or else we will elide stores and the garbage
@@ -719,7 +694,22 @@ mkvar(Reg *r, Adr *a)
 	// The broader := in a closure problem is mentioned in a comment in
 	// closure.c:/^typecheckclosure and dcl.c:/^oldname.
 	if(node->addrtaken)
-		setaddrs(bit);
+		v->addr = 1;
+
+	// Disable registerization for globals, because:
+	// (1) we might panic at any time and we want the recovery code
+	// to see the latest values (issue 1304).
+	// (2) we don't know what pointers might point at them and we want
+	// loads via those pointers to see updated values and vice versa (issue 7995).
+	//
+	// Disable registerization for results if using defer, because the deferred func
+	// might recover and return, causing the current values to be used.
+	if(node->class == PEXTERN || (hasdefer && node->class == PPARAMOUT))
+		v->addr = 1;
+
+	if(debug['R'])
+		print("bit=%2d et=%2E w=%lld+%lld %#N %D flag=%d\n", i, et, o, w, node, a, v->addr);
+	ostats.nvar++;
 
 	return bit;
 
@@ -816,17 +806,6 @@ prop(Reg *r, Bits ref, Bits cal)
 				ref.b[z] = 0;
 			}
 			break;
-
-		default:
-			// Work around for issue 1304:
-			// flush modified globals before each instruction.
-			for(z=0; z<BITS; z++) {
-				cal.b[z] |= externs.b[z];
-				// issue 4066: flush modified return variables in case of panic
-				if(hasdefer)
-					cal.b[z] |= ovar.b[z];
-			}
-			break;
 		}
 		for(z=0; z<BITS; z++) {
 			ref.b[z] = (ref.b[z] & ~r1->set.b[z]) |