cmd/gc: don't give credit for NOPs during register allocation

The register allocator decides which variables should be placed into registers by charging for each load/store and crediting for each use, and then selecting an allocation with minimal cost. NOPs will be eliminated, however, so using a variable in a NOP should not generate credit.

Issue 7867 arises from attempted registerization of multi-word variables because they are used in NOPs. By not crediting for that use, they will no longer be considered for registerization.

This fix could theoretically lead to better register allocation, but NOPs are rare relative to other instructions.

Fixes #7867.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/94810044

diff --git a/src/cmd/5g/reg.c b/src/cmd/5g/reg.c
index b4032fff8d4a2197400c25f08233cadc87cb5dbf..80a14db3c437b7b6f851bd9fee8f39b642883506 100644 (file)
--- a/src/cmd/5g/reg.c
+++ b/src/cmd/5g/reg.c
@@ -1097,18 +1097,20 @@ paint1(Reg *r, int bn)
                r->act.b[z] |= bb;
                p = r->f.prog;
 
-               if(r->use1.b[z] & bb) {
-                       change += CREF * r->f.loop;
-                       if(debug['R'] > 1)
-                               print("%d%P\tu1 %Q $%d\n", r->f.loop,
-                                       p, blsh(bn), change);
-               }
 
-               if((r->use2.b[z]|r->set.b[z]) & bb) {
-                       change += CREF * r->f.loop;
-                       if(debug['R'] > 1)
-                               print("%d%P\tu2 %Q $%d\n", r->f.loop,
-                                       p, blsh(bn), change);
+               if(r->f.prog->as != ANOP) { // don't give credit for NOPs
+                       if(r->use1.b[z] & bb) {
+                               change += CREF * r->f.loop;
+                               if(debug['R'] > 1)
+                                       print("%d%P\tu1 %Q $%d\n", r->f.loop,
+                                               p, blsh(bn), change);
+                       }
+                       if((r->use2.b[z]|r->set.b[z]) & bb) {
+                               change += CREF * r->f.loop;
+                               if(debug['R'] > 1)
+                                       print("%d%P\tu2 %Q $%d\n", r->f.loop,
+                                               p, blsh(bn), change);
+                       }
                }
 
                if(STORE(r) & r->regdiff.b[z] & bb) {

diff --git a/src/cmd/6g/reg.c b/src/cmd/6g/reg.c
index 0c72d6c95cdf4eaa228345ad9b4d8f0e1d123871..484c1c0cde582e1e8806fbc252cdf908d959087d 100644 (file)
--- a/src/cmd/6g/reg.c
+++ b/src/cmd/6g/reg.c
@@ -942,12 +942,11 @@ paint1(Reg *r, int bn)
        for(;;) {
                r->act.b[z] |= bb;
 
-               if(r->use1.b[z] & bb) {
-                       change += CREF * r->f.loop;
-               }
-
-               if((r->use2.b[z]|r->set.b[z]) & bb) {
-                       change += CREF * r->f.loop;
+               if(r->f.prog->as != ANOP) { // don't give credit for NOPs
+                       if(r->use1.b[z] & bb)
+                               change += CREF * r->f.loop;
+                       if((r->use2.b[z]|r->set.b[z]) & bb)
+                               change += CREF * r->f.loop;
                }
 
                if(STORE(r) & r->regdiff.b[z] & bb) {

diff --git a/src/cmd/8g/reg.c b/src/cmd/8g/reg.c
index 1e8a31dd62ea4b4c991c0a5aeaad052512219f0e..d17e18b22709a099a1cbc3cdb92f7d55979123a2 100644 (file)
--- a/src/cmd/8g/reg.c
+++ b/src/cmd/8g/reg.c
@@ -909,18 +909,19 @@ paint1(Reg *r, int bn)
                r->act.b[z] |= bb;
                p = r->f.prog;
 
-               if(r->use1.b[z] & bb) {
-                       change += CREF * r->f.loop;
-                       if(p->as == AFMOVL || p->as == AFMOVW)
-                               if(BtoR(bb) != D_F0)
-                                       change = -CINF;
-               }
-
-               if((r->use2.b[z]|r->set.b[z]) & bb) {
-                       change += CREF * r->f.loop;
-                       if(p->as == AFMOVL || p->as == AFMOVW)
-                               if(BtoR(bb) != D_F0)
-                                       change = -CINF;
+               if(r->f.prog->as != ANOP) { // don't give credit for NOPs
+                       if(r->use1.b[z] & bb) {
+                               change += CREF * r->f.loop;
+                               if(p->as == AFMOVL || p->as == AFMOVW)
+                                       if(BtoR(bb) != D_F0)
+                                               change = -CINF;
+                       }
+                       if((r->use2.b[z]|r->set.b[z]) & bb) {
+                               change += CREF * r->f.loop;
+                               if(p->as == AFMOVL || p->as == AFMOVW)
+                                       if(BtoR(bb) != D_F0)
+                                               change = -CINF;
+                       }
                }
 
                if(STORE(r) & r->regdiff.b[z] & bb) {

diff --git a/test/fixedbugs/issue7867.go b/test/fixedbugs/issue7867.go
new file mode 100644 (file)
index 0000000..9f28a71
--- /dev/null
+++ b/test/fixedbugs/issue7867.go
@@ -0,0 +1,43 @@
+// runoutput
+
+// Copyright 2014 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 7867.
+
+package main
+
+import "fmt"
+
+const tpl = `
+func Test%d(t %s) {
+       _ = t
+       _ = t
+}
+`
+
+func main() {
+       fmt.Println("package main")
+       types := []string{
+               // These types always passed
+               "bool", "int", "rune",
+               "*int", "uintptr",
+               "float32", "float64",
+               "chan struct{}",
+               "map[string]struct{}",
+               "func()", "func(string)error",
+
+               // These types caused compilation failures
+               "complex64", "complex128",
+               "struct{}", "struct{n int}", "struct{e error}", "struct{m map[string]string}",
+               "string",
+               "[4]byte",
+               "[]byte",
+               "interface{}", "error",
+       }
+       for i, typ := range types {
+               fmt.Printf(tpl, i, typ)
+       }
+       fmt.Println("func main() {}")
+}