From: Keith Randall Date: Sun, 24 Apr 2016 05:59:01 +0000 (-0700) Subject: cmd/compile: reorder how slicelit initializes a slice X-Git-Tag: go1.7beta1~511 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=934c3599648ae841668ec753881134347fc28c29;p=gostls13.git cmd/compile: reorder how slicelit initializes a slice func f(x, y, z *int) { a := []*int{x,y,z} ... } We used to use: var tmp [3]*int a := tmp[:] a[0] = x a[1] = y a[2] = z Now we do: var tmp [3]*int tmp[0] = x tmp[1] = y tmp[2] = z a := tmp[:] Doesn't sound like a big deal, but the compiler has trouble eliminating write barriers when using the former method because it doesn't know that the slice points to the stack. In the latter method, the compiler knows the array is on the stack and as a result doesn't emit any write barriers. This turns out to be extremely common when building ... args, like for calls fmt.Printf. Makes go binaries ~1% smaller. Doesn't have a measurable effect on the go1 fmt benchmarks, unfortunately. Fixes #14263 Update #6853 Change-Id: I9074a2788ec9e561a75f3b71c119b69f304d6ba2 Reviewed-on: https://go-review.googlesource.com/22395 Run-TryBot: Keith Randall TryBot-Result: Gobot Gobot Reviewed-by: Josh Bleecher Snyder --- diff --git a/src/cmd/compile/internal/gc/sinit.go b/src/cmd/compile/internal/gc/sinit.go index 2c2ade06f5..cc1d1962d2 100644 --- a/src/cmd/compile/internal/gc/sinit.go +++ b/src/cmd/compile/internal/gc/sinit.go @@ -745,15 +745,15 @@ func slicelit(ctxt int, n *Node, var_ *Node, init *Nodes) { // var vauto *[...]t = new([...]t) // 4. copy the static array to the auto array // *vauto = vstat - // 5. assign slice of allocated heap to var - // var = [0:]*auto - // 6. for each dynamic part assign to the slice - // var[i] = dynamic part + // 5. for each dynamic part assign to the array + // vauto[i] = dynamic part + // 6. assign slice of allocated heap to var + // var = vauto[:] // // an optimization is done if there is no constant part // 3. var vauto *[...]t = new([...]t) - // 5. var = [0:]*auto - // 6. var[i] = dynamic part + // 5. vauto[i] = dynamic part + // 6. var = vauto[:] // if the literal contains constants, // make static initialized array (1),(2) @@ -811,21 +811,14 @@ func slicelit(ctxt int, n *Node, var_ *Node, init *Nodes) { init.Append(a) } - // make slice out of heap (5) - a = Nod(OAS, var_, Nod(OSLICE, vauto, Nod(OKEY, nil, nil))) - - a = typecheck(a, Etop) - a = orderstmtinplace(a) - a = walkstmt(a) - init.Append(a) - // put dynamics into slice (6) + // put dynamics into array (5) for _, r := range n.List.Slice() { if r.Op != OKEY { Fatalf("slicelit: rhs not OKEY: %v", r) } index := r.Left value := r.Right - a := Nod(OINDEX, var_, index) + a := Nod(OINDEX, vauto, index) a.Bounded = true // TODO need to check bounds? @@ -847,7 +840,7 @@ func slicelit(ctxt int, n *Node, var_ *Node, init *Nodes) { continue } - // build list of var[c] = expr + // build list of vauto[c] = expr setlineno(value) a = Nod(OAS, a, value) @@ -856,6 +849,14 @@ func slicelit(ctxt int, n *Node, var_ *Node, init *Nodes) { a = walkstmt(a) init.Append(a) } + + // make slice out of heap (6) + a = Nod(OAS, var_, Nod(OSLICE, vauto, Nod(OKEY, nil, nil))) + + a = typecheck(a, Etop) + a = orderstmtinplace(a) + a = walkstmt(a) + init.Append(a) } func maplit(ctxt int, n *Node, var_ *Node, init *Nodes) { diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go index 27ff045028..7c4d74c8c3 100644 --- a/src/cmd/compile/internal/gc/walk.go +++ b/src/cmd/compile/internal/gc/walk.go @@ -2748,8 +2748,7 @@ func addstr(n *Node, init *Nodes) *Node { prealloc[slice] = prealloc[n] } slice.List.Set(args[1:]) // skip buf arg - args = []*Node{buf} - args = append(args, slice) + args = []*Node{buf, slice} slice.Esc = EscNone } diff --git a/test/writebarrier.go b/test/writebarrier.go index 44e42f0883..2ff0ee9584 100644 --- a/test/writebarrier.go +++ b/test/writebarrier.go @@ -182,3 +182,17 @@ func f18(p *T18, x *[]int) { p.s = p.s[8:9] // ERROR "write barrier" *x = (*x)[3:5] // ERROR "write barrier" } + +func f19(x, y *int, i int) int { + // Constructing a temporary slice on the stack should not + // require any write barriers. See issue 14263. + a := []*int{x, y} // no barrier + return *a[i] +} + +func f20(x, y *int, i int) []*int { + // ... but if that temporary slice escapes, then the + // write barriers are necessary. + a := []*int{x, y} // ERROR "write barrier" + return a +}