Skip to content

Commit 304f769

Browse files
committed
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 #43570 Change-Id: Ifa518bb1a6f3a8dd46c352d4fd54ea9713b3eb1a Reviewed-on: https://go-review.googlesource.com/c/go/+/282492 Trust: Keith Randall <[email protected]> Trust: Josh Bleecher Snyder <[email protected]> Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Josh Bleecher Snyder <[email protected]>
1 parent ae97717 commit 304f769

File tree

3 files changed

+46
-6
lines changed

3 files changed

+46
-6
lines changed

src/cmd/compile/internal/ssa/gen/generic.rules

+2-2
Original file line numberDiff line numberDiff line change
@@ -2512,7 +2512,7 @@
25122512
(Move {t1} [s] dst tmp1 midmem:(Move {t2} [s] tmp2 src _))
25132513
&& t1.Compare(t2) == types.CMPeq
25142514
&& isSamePtr(tmp1, tmp2)
2515-
&& isStackPtr(src)
2515+
&& isStackPtr(src) && !isVolatile(src)
25162516
&& disjoint(src, s, tmp2, s)
25172517
&& (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config))
25182518
=> (Move {t1} [s] dst src midmem)
@@ -2521,7 +2521,7 @@
25212521
(Move {t1} [s] dst tmp1 midmem:(VarDef (Move {t2} [s] tmp2 src _)))
25222522
&& t1.Compare(t2) == types.CMPeq
25232523
&& isSamePtr(tmp1, tmp2)
2524-
&& isStackPtr(src)
2524+
&& isStackPtr(src) && !isVolatile(src)
25252525
&& disjoint(src, s, tmp2, s)
25262526
&& (disjoint(src, s, dst, s) || isInlinableMemmove(dst, src, s, config))
25272527
=> (Move {t1} [s] dst src midmem)

src/cmd/compile/internal/ssa/rewritegeneric.go

+4-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixedbugs/issue43570.go

+40
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// run
2+
3+
// Copyright 2021 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
import "fmt"
10+
11+
type T [8]*int
12+
13+
//go:noinline
14+
func f(x int) T {
15+
return T{}
16+
}
17+
18+
//go:noinline
19+
func g(x int, t T) {
20+
if t != (T{}) {
21+
panic(fmt.Sprintf("bad: %v", t))
22+
}
23+
}
24+
25+
func main() {
26+
const N = 10000
27+
var q T
28+
func() {
29+
for i := 0; i < N; i++ {
30+
q = f(0)
31+
g(0, q)
32+
sink = make([]byte, 1024)
33+
}
34+
}()
35+
// Note that the closure is a trick to get the write to q to be a
36+
// write to a pointer that is known to be non-nil and requires
37+
// a write barrier.
38+
}
39+
40+
var sink []byte

0 commit comments

Comments
 (0)