Skip to content

Commit 78952c0

Browse files
randall77rsc
authored andcommitted
[release-branch.go1.9] cmd/compile: fix sign-extension merging rules
If we have y = <int16> (MOVBQSX x) z = <int32> (MOVWQSX y) We used to use this rewrite rule: (MOVWQSX x:(MOVBQSX _)) -> x But that resulted in replacing z with a value whose type is only int16. Then if z is spilled and restored, it gets zero extended instead of sign extended. Instead use the rule (MOVWQSX (MOVBQSX x)) -> (MOVBQSX x) The result is has the correct type, so it can be spilled and restored correctly. It might mean that a few more extension ops might not be eliminated, but that's the price for correctness. Fixes #21963 Change-Id: I6ec82c3d2dbe43cc1fee6fb2bd6b3a72fca3af00 Reviewed-on: https://go-review.googlesource.com/65290 Reviewed-by: Cherry Zhang <[email protected]> Run-TryBot: Cherry Zhang <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-on: https://go-review.googlesource.com/70986 Run-TryBot: Russ Cox <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent 79996e4 commit 78952c0

File tree

3 files changed

+113
-84
lines changed

3 files changed

+113
-84
lines changed

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

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2405,15 +2405,17 @@
24052405
(BSFQ (ORQconst <t> [1<<16] (MOVWQZX x))) -> (BSFQ (ORQconst <t> [1<<16] x))
24062406

24072407
// Redundant sign/zero extensions
2408-
(MOVLQSX x:(MOVLQSX _)) -> x
2409-
(MOVLQSX x:(MOVWQSX _)) -> x
2410-
(MOVLQSX x:(MOVBQSX _)) -> x
2411-
(MOVWQSX x:(MOVWQSX _)) -> x
2412-
(MOVWQSX x:(MOVBQSX _)) -> x
2413-
(MOVBQSX x:(MOVBQSX _)) -> x
2414-
(MOVLQZX x:(MOVLQZX _)) -> x
2415-
(MOVLQZX x:(MOVWQZX _)) -> x
2416-
(MOVLQZX x:(MOVBQZX _)) -> x
2417-
(MOVWQZX x:(MOVWQZX _)) -> x
2418-
(MOVWQZX x:(MOVBQZX _)) -> x
2419-
(MOVBQZX x:(MOVBQZX _)) -> x
2408+
// Note: see issue 21963. We have to make sure we use the right type on
2409+
// the resulting extension (the outer type, not the inner type).
2410+
(MOVLQSX (MOVLQSX x)) -> (MOVLQSX x)
2411+
(MOVLQSX (MOVWQSX x)) -> (MOVWQSX x)
2412+
(MOVLQSX (MOVBQSX x)) -> (MOVBQSX x)
2413+
(MOVWQSX (MOVWQSX x)) -> (MOVWQSX x)
2414+
(MOVWQSX (MOVBQSX x)) -> (MOVBQSX x)
2415+
(MOVBQSX (MOVBQSX x)) -> (MOVBQSX x)
2416+
(MOVLQZX (MOVLQZX x)) -> (MOVLQZX x)
2417+
(MOVLQZX (MOVWQZX x)) -> (MOVWQZX x)
2418+
(MOVLQZX (MOVBQZX x)) -> (MOVBQZX x)
2419+
(MOVWQZX (MOVWQZX x)) -> (MOVWQZX x)
2420+
(MOVWQZX (MOVBQZX x)) -> (MOVBQZX x)
2421+
(MOVBQZX (MOVBQZX x)) -> (MOVBQZX x)

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

Lines changed: 72 additions & 72 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixedbugs/issue21963.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// run
2+
3+
// Copyright 2017 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 (
10+
"fmt"
11+
"runtime"
12+
)
13+
14+
//go:noinline
15+
func f(x []int32, y *int8) int32 {
16+
c := int32(int16(*y))
17+
runtime.GC()
18+
return x[0] * c
19+
}
20+
21+
func main() {
22+
var x = [1]int32{5}
23+
var y int8 = -1
24+
if got, want := f(x[:], &y), int32(-5); got != want {
25+
panic(fmt.Sprintf("wanted %d, got %d", want, got))
26+
}
27+
}

0 commit comments

Comments
 (0)