Skip to content

Commit 3f04db4

Browse files
committed
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]>
1 parent 08a1796 commit 3f04db4

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
@@ -2512,18 +2512,20 @@
25122512
(BSFQ (ORQconst <t> [1<<16] (MOVWQZX x))) -> (BSFQ (ORQconst <t> [1<<16] x))
25132513

25142514
// Redundant sign/zero extensions
2515-
(MOVLQSX x:(MOVLQSX _)) -> x
2516-
(MOVLQSX x:(MOVWQSX _)) -> x
2517-
(MOVLQSX x:(MOVBQSX _)) -> x
2518-
(MOVWQSX x:(MOVWQSX _)) -> x
2519-
(MOVWQSX x:(MOVBQSX _)) -> x
2520-
(MOVBQSX x:(MOVBQSX _)) -> x
2521-
(MOVLQZX x:(MOVLQZX _)) -> x
2522-
(MOVLQZX x:(MOVWQZX _)) -> x
2523-
(MOVLQZX x:(MOVBQZX _)) -> x
2524-
(MOVWQZX x:(MOVWQZX _)) -> x
2525-
(MOVWQZX x:(MOVBQZX _)) -> x
2526-
(MOVBQZX x:(MOVBQZX _)) -> x
2515+
// Note: see issue 21963. We have to make sure we use the right type on
2516+
// the resulting extension (the outer type, not the inner type).
2517+
(MOVLQSX (MOVLQSX x)) -> (MOVLQSX x)
2518+
(MOVLQSX (MOVWQSX x)) -> (MOVWQSX x)
2519+
(MOVLQSX (MOVBQSX x)) -> (MOVBQSX x)
2520+
(MOVWQSX (MOVWQSX x)) -> (MOVWQSX x)
2521+
(MOVWQSX (MOVBQSX x)) -> (MOVBQSX x)
2522+
(MOVBQSX (MOVBQSX x)) -> (MOVBQSX x)
2523+
(MOVLQZX (MOVLQZX x)) -> (MOVLQZX x)
2524+
(MOVLQZX (MOVWQZX x)) -> (MOVWQZX x)
2525+
(MOVLQZX (MOVBQZX x)) -> (MOVBQZX x)
2526+
(MOVWQZX (MOVWQZX x)) -> (MOVWQZX x)
2527+
(MOVWQZX (MOVBQZX x)) -> (MOVBQZX x)
2528+
(MOVBQZX (MOVBQZX x)) -> (MOVBQZX x)
25272529

25282530
(MOVQstore [off] {sym} ptr a:(ADDQconst [c] l:(MOVQload [off] {sym} ptr2 mem)) mem)
25292531
&& isSamePtr(ptr, ptr2) && a.Uses == 1 && l.Uses == 1 && validValAndOff(c,off) ->

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)