Commit c39918a0 authored by Austin Clements's avatar Austin Clements

cmd/compile: disable various write barrier optimizations

Several of our current write barrier elision optimizations are invalid
with the hybrid barrier. Eliding the hybrid barrier requires that
*both* the current and new pointer be already shaded and, since we
don't have the flow analysis to figure out anything about the slot's
current value, for now we have to just disable several of these
optimizations.

This has a slight impact on binary size. On linux/amd64, the go tool
binary increases by 0.7% and the compile binary increases by 1.5%.

It also has a slight impact on performance, as one would expect. We'll
win some of this back in subsequent commits.

name                      old time/op    new time/op    delta
BinaryTree17-12              2.38s ± 1%     2.40s ± 1%  +0.82%  (p=0.000 n=18+20)
Fannkuch11-12                2.84s ± 1%     2.70s ± 0%  -4.97%  (p=0.000 n=18+18)
FmtFprintfEmpty-12          44.2ns ± 1%    46.4ns ± 2%  +4.89%  (p=0.000 n=16+18)
FmtFprintfString-12          131ns ± 0%     134ns ± 1%  +2.05%  (p=0.000 n=12+19)
FmtFprintfInt-12             114ns ± 1%     117ns ± 1%  +3.26%  (p=0.000 n=19+20)
FmtFprintfIntInt-12          176ns ± 1%     181ns ± 1%  +3.25%  (p=0.000 n=20+20)
FmtFprintfPrefixedInt-12     185ns ± 1%     190ns ± 1%  +2.77%  (p=0.000 n=19+18)
FmtFprintfFloat-12           249ns ± 1%     254ns ± 1%  +1.71%  (p=0.000 n=18+20)
FmtManyArgs-12               747ns ± 1%     743ns ± 1%  -0.58%  (p=0.000 n=19+18)
GobDecode-12                6.57ms ± 1%    6.61ms ± 0%  +0.73%  (p=0.000 n=19+20)
GobEncode-12                5.58ms ± 1%    5.60ms ± 0%  +0.27%  (p=0.001 n=18+18)
Gzip-12                      223ms ± 1%     223ms ± 1%    ~     (p=0.351 n=19+20)
Gunzip-12                   37.9ms ± 0%    37.9ms ± 1%    ~     (p=0.095 n=16+20)
HTTPClientServer-12         77.8µs ± 1%    78.5µs ± 1%  +0.97%  (p=0.000 n=19+20)
JSONEncode-12               14.8ms ± 1%    14.8ms ± 1%    ~     (p=0.079 n=20+19)
JSONDecode-12               53.7ms ± 1%    54.2ms ± 1%  +0.92%  (p=0.000 n=20+19)
Mandelbrot200-12            3.81ms ± 1%    3.81ms ± 0%    ~     (p=0.916 n=19+18)
GoParse-12                  3.19ms ± 1%    3.19ms ± 1%    ~     (p=0.175 n=20+19)
RegexpMatchEasy0_32-12      71.9ns ± 1%    70.6ns ± 1%  -1.87%  (p=0.000 n=19+20)
RegexpMatchEasy0_1K-12       946ns ± 0%     944ns ± 0%  -0.22%  (p=0.000 n=19+16)
RegexpMatchEasy1_32-12      67.3ns ± 2%    66.8ns ± 1%  -0.72%  (p=0.008 n=20+20)
RegexpMatchEasy1_1K-12       374ns ± 1%     384ns ± 1%  +2.69%  (p=0.000 n=18+20)
RegexpMatchMedium_32-12      107ns ± 1%     107ns ± 1%    ~     (p=1.000 n=20+20)
RegexpMatchMedium_1K-12     34.3µs ± 1%    34.6µs ± 1%  +0.90%  (p=0.000 n=20+20)
RegexpMatchHard_32-12       1.78µs ± 1%    1.80µs ± 1%  +1.45%  (p=0.000 n=20+19)
RegexpMatchHard_1K-12       53.6µs ± 0%    54.5µs ± 1%  +1.52%  (p=0.000 n=19+18)
Revcomp-12                   417ms ± 5%     391ms ± 1%  -6.42%  (p=0.000 n=16+19)
Template-12                 61.1ms ± 1%    64.2ms ± 0%  +5.07%  (p=0.000 n=19+20)
TimeParse-12                 302ns ± 1%     305ns ± 1%  +0.90%  (p=0.000 n=18+18)
TimeFormat-12                319ns ± 1%     315ns ± 1%  -1.25%  (p=0.000 n=18+18)
[Geo mean]                  54.0µs         54.3µs       +0.58%

name         old time/op  new time/op  delta
XGarbage-12  2.24ms ± 2%  2.28ms ± 1%  +1.68%  (p=0.000 n=18+17)
XHTTP-12     11.4µs ± 1%  11.6µs ± 2%  +1.63%  (p=0.000 n=18+18)
XJSON-12     11.6ms ± 0%  12.5ms ± 0%  +7.84%  (p=0.000 n=18+17)

Updates #17503.

Change-Id: I1899f8e35662971e24bf692b517dfbe2b533c00c
Reviewed-on: https://go-review.googlesource.com/31572Reviewed-by: 's avatarKeith Randall <khr@golang.org>
parent c3163d23
...@@ -423,7 +423,7 @@ func ordermapassign(n *Node, order *Order) { ...@@ -423,7 +423,7 @@ func ordermapassign(n *Node, order *Order) {
// We call writebarrierfat only for values > 4 pointers long. See walk.go. // We call writebarrierfat only for values > 4 pointers long. See walk.go.
// TODO(mdempsky): writebarrierfat doesn't exist anymore, but removing that // TODO(mdempsky): writebarrierfat doesn't exist anymore, but removing that
// logic causes net/http's tests to become flaky; see CL 21242. // logic causes net/http's tests to become flaky; see CL 21242.
if needwritebarrier(n.Left, n.Right) && n.Left.Type.Width > int64(4*Widthptr) && !isaddrokay(n.Right) { if needwritebarrier(n.Left, n.Right) && n.Left.Type.Width > int64(4*Widthptr) && n.Right != nil && !isaddrokay(n.Right) {
m := n.Left m := n.Left
n.Left = ordertemp(m.Type, order, false) n.Left = ordertemp(m.Type, order, false)
a := nod(OAS, m, n.Left) a := nod(OAS, m, n.Left)
......
...@@ -750,8 +750,13 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes) ...@@ -750,8 +750,13 @@ func fixedlit(ctxt initContext, kind initKind, n *Node, var_ *Node, init *Nodes)
switch kind { switch kind {
case initKindStatic: case initKindStatic:
a = walkexpr(a, init) // add any assignments in r to top a = walkexpr(a, init) // add any assignments in r to top
if a.Op == OASWB {
// Static initialization never needs
// write barriers.
a.Op = OAS
}
if a.Op != OAS { if a.Op != OAS {
Fatalf("fixedlit: not as") Fatalf("fixedlit: not as, is %v", a)
} }
a.IsStatic = true a.IsStatic = true
case initKindDynamic, initKindLocalCode: case initKindDynamic, initKindLocalCode:
......
...@@ -721,7 +721,8 @@ opswitch: ...@@ -721,7 +721,8 @@ opswitch:
break break
} }
if n.Right == nil || iszero(n.Right) && !instrumenting { if n.Right == nil {
// TODO(austin): Check all "implicit zeroing"
break break
} }
...@@ -2255,17 +2256,20 @@ func needwritebarrier(l *Node, r *Node) bool { ...@@ -2255,17 +2256,20 @@ func needwritebarrier(l *Node, r *Node) bool {
return false return false
} }
// No write barrier for implicit zeroing.
if r == nil {
return false
}
// No write barrier if this is a pointer to a go:notinheap // No write barrier if this is a pointer to a go:notinheap
// type, since the write barrier's inheap(ptr) check will fail. // type, since the write barrier's inheap(ptr) check will fail.
if l.Type.IsPtr() && l.Type.Elem().NotInHeap { if l.Type.IsPtr() && l.Type.Elem().NotInHeap {
return false return false
} }
// Implicit zeroing is still zeroing, so it needs write
// barriers. In practice, these are all to stack variables
// (even if isstack isn't smart enough to figure that out), so
// they'll be eliminated by the backend.
if r == nil {
return true
}
// Ignore no-op conversions when making decision. // Ignore no-op conversions when making decision.
// Ensures that xp = unsafe.Pointer(&x) is treated // Ensures that xp = unsafe.Pointer(&x) is treated
// the same as xp = &x. // the same as xp = &x.
...@@ -2273,15 +2277,13 @@ func needwritebarrier(l *Node, r *Node) bool { ...@@ -2273,15 +2277,13 @@ func needwritebarrier(l *Node, r *Node) bool {
r = r.Left r = r.Left
} }
// No write barrier for zeroing or initialization to constant. // TODO: We can eliminate write barriers if we know *both* the
if iszero(r) || r.Op == OLITERAL { // current and new content of the slot must already be shaded.
return false // We know a pointer is shaded if it's nil, or points to
} // static data, a global (variable or function), or the stack.
// The nil optimization could be particularly useful for
// No write barrier for storing static (read-only) data. // writes to just-allocated objects. Unfortunately, knowing
if r.Op == ONAME && strings.HasPrefix(r.Sym.Name, "statictmp_") { // the "current" value of the slot requires flow analysis.
return false
}
// No write barrier for storing address of stack values, // No write barrier for storing address of stack values,
// which are guaranteed only to be written to the stack. // which are guaranteed only to be written to the stack.
...@@ -2289,18 +2291,6 @@ func needwritebarrier(l *Node, r *Node) bool { ...@@ -2289,18 +2291,6 @@ func needwritebarrier(l *Node, r *Node) bool {
return false return false
} }
// No write barrier for storing address of global, which
// is live no matter what.
if r.Op == OADDR && r.Left.isGlobal() {
return false
}
// No write barrier for storing global function, which is live
// no matter what.
if r.Op == ONAME && r.Class == PFUNC {
return false
}
// Otherwise, be conservative and use write barrier. // Otherwise, be conservative and use write barrier.
return true return true
} }
......
...@@ -34,7 +34,7 @@ func f1(q *Q, xx []byte) interface{} { // ERROR "live at entry to f1: xx" "live ...@@ -34,7 +34,7 @@ func f1(q *Q, xx []byte) interface{} { // ERROR "live at entry to f1: xx" "live
//go:noinline //go:noinline
func f2(d []byte, n int) (odata, res []byte, e interface{}) { // ERROR "live at entry to f2: d" func f2(d []byte, n int) (odata, res []byte, e interface{}) { // ERROR "live at entry to f2: d"
if n > len(d) { if n > len(d) {
return d, nil, &T{M: "hello"} // ERROR "live at call to newobject: d" return d, nil, &T{M: "hello"} // ERROR "live at call to newobject: d" "live at call to writebarrierptr: d"
} }
res = d[:n] res = d[:n]
odata = d[n:] odata = d[n:]
......
...@@ -164,8 +164,9 @@ type T17 struct { ...@@ -164,8 +164,9 @@ type T17 struct {
} }
func f17(x *T17) { func f17(x *T17) {
// See golang.org/issue/13901 // Originally from golang.org/issue/13901, but the hybrid
x.f = f17 // no barrier // barrier requires both to have barriers.
x.f = f17 // ERROR "write barrier"
x.f = func(y *T17) { *y = *x } // ERROR "write barrier" x.f = func(y *T17) { *y = *x } // ERROR "write barrier"
} }
...@@ -207,8 +208,8 @@ func f21(x *int) { ...@@ -207,8 +208,8 @@ func f21(x *int) {
// Global -> heap pointer updates must have write barriers. // Global -> heap pointer updates must have write barriers.
x21 = x // ERROR "write barrier" x21 = x // ERROR "write barrier"
y21.x = x // ERROR "write barrier" y21.x = x // ERROR "write barrier"
x21 = &z21 // no barrier x21 = &z21 // ERROR "write barrier"
y21.x = &z21 // no barrier y21.x = &z21 // ERROR "write barrier"
y21 = struct{ x *int }{x} // ERROR "write barrier" y21 = struct{ x *int }{x} // ERROR "write barrier"
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment