diff options
author | Keith Randall <khr@golang.org> | 2021-01-07 14:57:53 -0800 |
---|---|---|
committer | Dmitri Shuralyov <dmitshur@golang.org> | 2021-01-21 21:41:34 +0000 |
commit | cb39368e2487841f5440ec9a622ef988f62c03b7 (patch) | |
tree | 82e07885026a569cf7e429ecdf082c9755db366e | |
parent | b57ea3d3a4a49ffea832fb5f923c2d16eb7d2eb2 (diff) | |
download | go-cb39368e2487841f5440ec9a622ef988f62c03b7.tar.gz go-cb39368e2487841f5440ec9a622ef988f62c03b7.zip |
[release-branch.go1.14] cmd/compile: don't short-circuit copies whose source is volatile
Current optimization: When we copy a->b and then b->c, we might as well
copy a->c instead of b->c (then b might be dead and go away).
*Except* if a is a volatile location (might be clobbered by a call).
In that case, we really do want to copy a immediately, because there
might be a call before we can do the a->c copy.
User calls can't happen in between, because the rule matches up the
memory states. But calls inserted for memory barriers, particularly
runtime.typedmemmove, can.
(I guess we could introduce a register-calling-convention version
of runtime.typedmemmove, but that seems a bigger change than this one.)
Fixes #43574
Change-Id: Ifa518bb1a6f3a8dd46c352d4fd54ea9713b3eb1a
Reviewed-on: https://go-review.googlesource.com/c/go/+/282492
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
(cherry picked from commit 304f769ffc68e64244266b3aadbf91e6738c0064)
Reviewed-on: https://go-review.googlesource.com/c/go/+/282559
-rw-r--r-- | src/cmd/compile/internal/ssa/gen/generic.rules | 4 | ||||
-rw-r--r-- | src/cmd/compile/internal/ssa/rewritegeneric.go | 8 | ||||
-rw-r--r-- | test/fixedbugs/issue43570.go | 40 |
3 files changed, 46 insertions, 6 deletions
diff --git a/src/cmd/compile/internal/ssa/gen/generic.rules b/src/cmd/compile/internal/ssa/gen/generic.rules index 87cfd3da0f..89368ed732 100644 --- a/src/cmd/compile/internal/ssa/gen/generic.rules +++ b/src/cmd/compile/internal/ssa/gen/generic.rules @@ -2351,7 +2351,7 @@ (Move {t1} [s] dst tmp1 midmem:(Move {t2} [s] tmp2 src _)) && t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) - && isStackPtr(src) + && isStackPtr(src) && !isVolatile(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config)) -> (Move {t1} [s] dst src midmem) @@ -2360,7 +2360,7 @@ (Move {t1} [s] dst tmp1 midmem:(VarDef (Move {t2} [s] tmp2 src _))) && t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) - && isStackPtr(src) + && isStackPtr(src) && !isVolatile(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config)) -> (Move {t1} [s] dst src midmem) diff --git a/src/cmd/compile/internal/ssa/rewritegeneric.go b/src/cmd/compile/internal/ssa/rewritegeneric.go index d65d900843..72abaf2f9a 100644 --- a/src/cmd/compile/internal/ssa/rewritegeneric.go +++ b/src/cmd/compile/internal/ssa/rewritegeneric.go @@ -32031,7 +32031,7 @@ func rewriteValuegeneric_OpMove_20(v *Value) bool { return true } // match: (Move {t1} [s] dst tmp1 midmem:(Move {t2} [s] tmp2 src _)) - // cond: t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config)) + // cond: t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && !isVolatile(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config)) // result: (Move {t1} [s] dst src midmem) for { s := v.AuxInt @@ -32047,7 +32047,7 @@ func rewriteValuegeneric_OpMove_20(v *Value) bool { _ = midmem.Args[2] tmp2 := midmem.Args[0] src := midmem.Args[1] - if !(t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config))) { + if !(t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && !isVolatile(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config))) { break } v.reset(OpMove) @@ -32059,7 +32059,7 @@ func rewriteValuegeneric_OpMove_20(v *Value) bool { return true } // match: (Move {t1} [s] dst tmp1 midmem:(VarDef (Move {t2} [s] tmp2 src _))) - // cond: t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config)) + // cond: t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && !isVolatile(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config)) // result: (Move {t1} [s] dst src midmem) for { s := v.AuxInt @@ -32079,7 +32079,7 @@ func rewriteValuegeneric_OpMove_20(v *Value) bool { _ = midmem_0.Args[2] tmp2 := midmem_0.Args[0] src := midmem_0.Args[1] - if !(t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config))) { + if !(t1.(*types.Type).Compare(t2.(*types.Type)) == types.CMPeq && isSamePtr(tmp1, tmp2) && isStackPtr(src) && !isVolatile(src) && disjoint(src, s, tmp2, s) && (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config))) { break } v.reset(OpMove) diff --git a/test/fixedbugs/issue43570.go b/test/fixedbugs/issue43570.go new file mode 100644 index 0000000000..d073fde5f6 --- /dev/null +++ b/test/fixedbugs/issue43570.go @@ -0,0 +1,40 @@ +// run + +// Copyright 2021 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. + +package main + +import "fmt" + +type T [8]*int + +//go:noinline +func f(x int) T { + return T{} +} + +//go:noinline +func g(x int, t T) { + if t != (T{}) { + panic(fmt.Sprintf("bad: %v", t)) + } +} + +func main() { + const N = 10000 + var q T + func() { + for i := 0; i < N; i++ { + q = f(0) + g(0, q) + sink = make([]byte, 1024) + } + }() + // Note that the closure is a trick to get the write to q to be a + // write to a pointer that is known to be non-nil and requires + // a write barrier. +} + +var sink []byte |