runtime,cmd/compile: pass strings and slices to convT2{E,I} by value

When we pass these types by reference, we usually have to allocate
temporaries on the stack, initialize them, then pass their address
to the conversion functions. It's simpler to pass these types
directly by value.

This particularly applies to conversions needed for fmt.Printf
(to interface{} for constructing a [...]interface{}).

func f(a, b, c string) {
     fmt.Printf("%s %s\n", a, b)
     fmt.Printf("%s %s\n", b, c)
}

This function's stack frame shrinks from 200 to 136 bytes, and
its code shrinks from 535 to 453 bytes.

The go binary shrinks 0.3%.

Update #24286

Aside: for this function f, we don't really need to allocate
temporaries for the convT2E function. We could use the address
of a, b, and c directly. That might get similar (or maybe better?)
improvements. I investigated a bit, but it seemed complicated
to do it safely. This change was much easier.

Change-Id: I78cbe51b501fb41e1e324ce4203f0de56a1db82d
Reviewed-on: https://go-review.googlesource.com/c/135377
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>

diff --git a/src/cmd/compile/internal/gc/builtin.go b/src/cmd/compile/internal/gc/builtin.go
index ec8f1093b6f5aab8d2064a1bca48b706992f60c5..8051c7d0dfa05bc30d4abeca97fc175360e8b419 100644 (file)
--- a/src/cmd/compile/internal/gc/builtin.go
+++ b/src/cmd/compile/internal/gc/builtin.go
@@ -55,15 +55,15 @@ var runtimeDecls = [...]struct {
        {"convT2E16", funcTag, 52},
        {"convT2E32", funcTag, 52},
        {"convT2E64", funcTag, 52},
-       {"convT2Estring", funcTag, 53},
-       {"convT2Eslice", funcTag, 53},
+       {"convT2Estring", funcTag, 52},
+       {"convT2Eslice", funcTag, 52},
        {"convT2Enoptr", funcTag, 53},
        {"convT2I", funcTag, 53},
        {"convT2I16", funcTag, 52},
        {"convT2I32", funcTag, 52},
        {"convT2I64", funcTag, 52},
-       {"convT2Istring", funcTag, 53},
-       {"convT2Islice", funcTag, 53},
+       {"convT2Istring", funcTag, 52},
+       {"convT2Islice", funcTag, 52},
        {"convT2Inoptr", funcTag, 53},
        {"assertE2I", funcTag, 52},
        {"assertE2I2", funcTag, 54},

diff --git a/src/cmd/compile/internal/gc/builtin/runtime.go b/src/cmd/compile/internal/gc/builtin/runtime.go
index 140b7f3b2d0dc6953d33eac5b1788d67b3efd55b..028936b8758da8a8a5964f45e072b45890e6aa53 100644 (file)
--- a/src/cmd/compile/internal/gc/builtin/runtime.go
+++ b/src/cmd/compile/internal/gc/builtin/runtime.go
@@ -68,16 +68,16 @@ func convT2E(typ *byte, elem *any) (ret any)
 func convT2E16(typ *byte, val any) (ret any)
 func convT2E32(typ *byte, val any) (ret any)
 func convT2E64(typ *byte, val any) (ret any)
-func convT2Estring(typ *byte, elem *any) (ret any)
-func convT2Eslice(typ *byte, elem *any) (ret any)
+func convT2Estring(typ *byte, val any) (ret any) // val must be a string
+func convT2Eslice(typ *byte, val any) (ret any)  // val must be a slice
 func convT2Enoptr(typ *byte, elem *any) (ret any)
 
 func convT2I(tab *byte, elem *any) (ret any)
 func convT2I16(tab *byte, val any) (ret any)
 func convT2I32(tab *byte, val any) (ret any)
 func convT2I64(tab *byte, val any) (ret any)
-func convT2Istring(tab *byte, elem *any) (ret any)
-func convT2Islice(tab *byte, elem *any) (ret any)
+func convT2Istring(tab *byte, val any) (ret any) // val must be a string
+func convT2Islice(tab *byte, val any) (ret any)  // val must be a slice
 func convT2Inoptr(tab *byte, elem *any) (ret any)
 
 // interface type assertions x.(T)

diff --git a/src/cmd/compile/internal/gc/order.go b/src/cmd/compile/internal/gc/order.go
index 8afb136515b87fca7862223bfc0068e739083a72..fbc05b95d2fc723500e861e58a3ce5e7f90f4add 100644 (file)
--- a/src/cmd/compile/internal/gc/order.go
+++ b/src/cmd/compile/internal/gc/order.go
@@ -1042,12 +1042,17 @@ func (o *Order) expr(n, lhs *Node) *Node {
                        n = o.copyExpr(n, n.Type, false)
                }
 
-       // concrete type (not interface) argument must be addressable
-       // temporary to pass to runtime.
+       // concrete type (not interface) argument might need an addressable
+       // temporary to pass to the runtime conversion routine.
        case OCONVIFACE:
                n.Left = o.expr(n.Left, nil)
-
-               if !n.Left.Type.IsInterface() {
+               if n.Left.Type.IsInterface() {
+                       break
+               }
+               if _, needsaddr := convFuncName(n.Left.Type, n.Type); needsaddr || consttype(n.Left) > 0 {
+                       // Need a temp if we need to pass the address to the conversion function.
+                       // We also process constants here, making a named static global whose
+                       // address we can put directly in an interface (see OCONVIFACE case in walk).
                        n.Left = o.addrTemp(n.Left)
                }
 

diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index d33674f221d70e990bb44d15693ef94c56062786..f7676310e9cf6e8386055dcaf3f3e46e5ce23900 100644 (file)
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/src/cmd/compile/internal/gc/walk.go
@@ -417,9 +417,9 @@ func convFuncName(from, to *types.Type) (fnname string, needsaddr bool) {
                        case from.Size() == 8 && from.Align == types.Types[TUINT64].Align && !types.Haspointers(from):
                                return "convT2E64", false
                        case from.IsString():
-                               return "convT2Estring", true
+                               return "convT2Estring", false
                        case from.IsSlice():
-                               return "convT2Eslice", true
+                               return "convT2Eslice", false
                        case !types.Haspointers(from):
                                return "convT2Enoptr", true
                        }
@@ -433,9 +433,9 @@ func convFuncName(from, to *types.Type) (fnname string, needsaddr bool) {
                        case from.Size() == 8 && from.Align == types.Types[TUINT64].Align && !types.Haspointers(from):
                                return "convT2I64", false
                        case from.IsString():
-                               return "convT2Istring", true
+                               return "convT2Istring", false
                        case from.IsSlice():
-                               return "convT2Islice", true
+                               return "convT2Islice", false
                        case !types.Haspointers(from):
                                return "convT2Inoptr", true
                        }

diff --git a/src/runtime/iface.go b/src/runtime/iface.go
index 7ab731151e5a2310e057ef7872b1be798aab4e86..1ef9825a486cc87173f1e9c9109599c88aab85a7 100644 (file)
--- a/src/runtime/iface.go
+++ b/src/runtime/iface.go
@@ -329,38 +329,27 @@ func convT2E64(t *_type, val uint64) (e eface) {
        return
 }
 
-func convT2Estring(t *_type, elem unsafe.Pointer) (e eface) {
-       if raceenabled {
-               raceReadObjectPC(t, elem, getcallerpc(), funcPC(convT2Estring))
-       }
-       if msanenabled {
-               msanread(elem, t.size)
-       }
+func convT2Estring(t *_type, val string) (e eface) {
        var x unsafe.Pointer
-       if *(*string)(elem) == "" {
+       if val == "" {
                x = unsafe.Pointer(&zeroVal[0])
        } else {
-               x = mallocgc(t.size, t, true)
-               *(*string)(x) = *(*string)(elem)
+               x = mallocgc(unsafe.Sizeof(val), t, true)
+               *(*string)(x) = val
        }
        e._type = t
        e.data = x
        return
 }
 
-func convT2Eslice(t *_type, elem unsafe.Pointer) (e eface) {
-       if raceenabled {
-               raceReadObjectPC(t, elem, getcallerpc(), funcPC(convT2Eslice))
-       }
-       if msanenabled {
-               msanread(elem, t.size)
-       }
+func convT2Eslice(t *_type, val []byte) (e eface) {
+       // Note: this must work for any element type, not just byte.
        var x unsafe.Pointer
-       if v := *(*slice)(elem); uintptr(v.array) == 0 {
+       if (*slice)(unsafe.Pointer(&val)).array == nil {
                x = unsafe.Pointer(&zeroVal[0])
        } else {
-               x = mallocgc(t.size, t, true)
-               *(*slice)(x) = *(*slice)(elem)
+               x = mallocgc(unsafe.Sizeof(val), t, true)
+               *(*[]byte)(x) = val
        }
        e._type = t
        e.data = x
@@ -438,40 +427,29 @@ func convT2I64(tab *itab, val uint64) (i iface) {
        return
 }
 
-func convT2Istring(tab *itab, elem unsafe.Pointer) (i iface) {
+func convT2Istring(tab *itab, val string) (i iface) {
        t := tab._type
-       if raceenabled {
-               raceReadObjectPC(t, elem, getcallerpc(), funcPC(convT2Istring))
-       }
-       if msanenabled {
-               msanread(elem, t.size)
-       }
        var x unsafe.Pointer
-       if *(*string)(elem) == "" {
+       if val == "" {
                x = unsafe.Pointer(&zeroVal[0])
        } else {
-               x = mallocgc(t.size, t, true)
-               *(*string)(x) = *(*string)(elem)
+               x = mallocgc(unsafe.Sizeof(val), t, true)
+               *(*string)(x) = val
        }
        i.tab = tab
        i.data = x
        return
 }
 
-func convT2Islice(tab *itab, elem unsafe.Pointer) (i iface) {
+func convT2Islice(tab *itab, val []byte) (i iface) {
+       // Note: this must work for any element type, not just byte.
        t := tab._type
-       if raceenabled {
-               raceReadObjectPC(t, elem, getcallerpc(), funcPC(convT2Islice))
-       }
-       if msanenabled {
-               msanread(elem, t.size)
-       }
        var x unsafe.Pointer
-       if v := *(*slice)(elem); uintptr(v.array) == 0 {
+       if (*slice)(unsafe.Pointer(&val)).array == nil {
                x = unsafe.Pointer(&zeroVal[0])
        } else {
-               x = mallocgc(t.size, t, true)
-               *(*slice)(x) = *(*slice)(elem)
+               x = mallocgc(unsafe.Sizeof(val), t, true)
+               *(*[]byte)(x) = val
        }
        i.tab = tab
        i.data = x

diff --git a/test/fixedbugs/issue20250.go b/test/fixedbugs/issue20250.go
index 47879385d22fb66cf2281037c0f76d880ab6e688..c190515274af21ca0cc186a0d24f810a63e3b488 100644 (file)
--- a/test/fixedbugs/issue20250.go
+++ b/test/fixedbugs/issue20250.go
@@ -11,13 +11,13 @@
 package p
 
 type T struct {
-       s string
+       s [2]string
 }
 
 func f(a T) { // ERROR "live at entry to f: a"
        var e interface{} // ERROR "stack object e interface \{\}$"
        func() {          // ERROR "live at entry to f.func1: a &e"
-               e = a.s // ERROR "live at call to convT2Estring: &e" "stack object a T$"
+               e = a.s // ERROR "live at call to convT2E: &e" "stack object a T$"
        }()
        // Before the fix, both a and e were live at the previous line.
        _ = e

diff --git a/test/live.go b/test/live.go
index 679562d9bf4956827433130632796cbb1b2f6832..ba50f5b779673368e38477ab5bfd3d905b029167 100644 (file)
--- a/test/live.go
+++ b/test/live.go
@@ -141,7 +141,7 @@ var i9 interface{}
 func f9() bool {
        g8()
        x := i9
-       y := interface{}(str()) // ERROR "live at call to convT2Estring: x.data$" "live at call to str: x.data$" "stack object .autotmp_[0-9]+ string$"
+       y := interface{}(g18()) // ERROR "live at call to convT2E: x.data$" "live at call to g18: x.data$" "stack object .autotmp_[0-9]+ \[2\]string$"
        i9 = y                  // make y escape so the line above has to call convT2E
        return x != y
 }
@@ -493,13 +493,13 @@ func f30(b bool) {
 
 func f31(b1, b2, b3 bool) {
        if b1 {
-               g31(str()) // ERROR "stack object .autotmp_[0-9]+ string$"
+               g31(g18()) // ERROR "stack object .autotmp_[0-9]+ \[2\]string$"
        }
        if b2 {
-               h31(str()) // ERROR "live at call to convT2Estring: .autotmp_[0-9]+$" "live at call to newobject: .autotmp_[0-9]+$" "stack object .autotmp_[0-9]+ string$"
+               h31(g18()) // ERROR "live at call to convT2E: .autotmp_[0-9]+$" "live at call to newobject: .autotmp_[0-9]+$" "stack object .autotmp_[0-9]+ \[2\]string$"
        }
        if b3 {
-               panic(str()) // ERROR "stack object .autotmp_[0-9]+ string$"
+               panic(g18()) // ERROR "stack object .autotmp_[0-9]+ \[2\]string$"
        }
        print(b3)
 }