Skip to content

Commit ce68047

Browse files
randall77andybons
authored andcommitted
[release-branch.go1.9] cmd/compile: fix constant folding of right shifts
The sub-word shifts need to sign-extend before shifting, to avoid bringing in data from higher in the argument. Fixes #23812 Change-Id: I0a95a0b49c48f3b40b85765bb4a9bb492be0cd73 Reviewed-on: https://go-review.googlesource.com/93716 Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]> Reviewed-on: https://go-review.googlesource.com/102775 Run-TryBot: Andrew Bonventre <[email protected]>
1 parent 6732fcc commit ce68047

File tree

4 files changed

+55
-21
lines changed

4 files changed

+55
-21
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,9 +1499,9 @@
14991499
(SUBQconst (MOVQconst [d]) [c]) -> (MOVQconst [d-c])
15001500
(SUBQconst (SUBQconst x [d]) [c]) && is32Bit(-c-d) -> (ADDQconst [-c-d] x)
15011501
(SARQconst [c] (MOVQconst [d])) -> (MOVQconst [d>>uint64(c)])
1502-
(SARLconst [c] (MOVQconst [d])) -> (MOVQconst [d>>uint64(c)])
1503-
(SARWconst [c] (MOVQconst [d])) -> (MOVQconst [d>>uint64(c)])
1504-
(SARBconst [c] (MOVQconst [d])) -> (MOVQconst [d>>uint64(c)])
1502+
(SARLconst [c] (MOVQconst [d])) -> (MOVQconst [int64(int32(d))>>uint64(c)])
1503+
(SARWconst [c] (MOVQconst [d])) -> (MOVQconst [int64(int16(d))>>uint64(c)])
1504+
(SARBconst [c] (MOVQconst [d])) -> (MOVQconst [int64(int8(d))>>uint64(c)])
15051505
(NEGQ (MOVQconst [c])) -> (MOVQconst [-c])
15061506
(NEGL (MOVLconst [c])) -> (MOVLconst [int64(int32(-c))])
15071507
(MULQconst [c] (MOVQconst [d])) -> (MOVQconst [c*d])

src/cmd/compile/internal/ssa/gen/AMD64Ops.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -268,22 +268,22 @@ func init() {
268268
// Note: x86 is weird, the 16 and 8 byte shifts still use all 5 bits of shift amount!
269269

270270
{name: "SHRQ", argLength: 2, reg: gp21shift, asm: "SHRQ", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> arg1, shift amount is mod 64
271-
{name: "SHRL", argLength: 2, reg: gp21shift, asm: "SHRL", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> arg1, shift amount is mod 32
272-
{name: "SHRW", argLength: 2, reg: gp21shift, asm: "SHRW", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> arg1, shift amount is mod 32
273-
{name: "SHRB", argLength: 2, reg: gp21shift, asm: "SHRB", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> arg1, shift amount is mod 32
271+
{name: "SHRL", argLength: 2, reg: gp21shift, asm: "SHRL", resultInArg0: true, clobberFlags: true}, // unsigned uint32(arg0) >> arg1, shift amount is mod 32
272+
{name: "SHRW", argLength: 2, reg: gp21shift, asm: "SHRW", resultInArg0: true, clobberFlags: true}, // unsigned uint16(arg0) >> arg1, shift amount is mod 32
273+
{name: "SHRB", argLength: 2, reg: gp21shift, asm: "SHRB", resultInArg0: true, clobberFlags: true}, // unsigned uint8(arg0) >> arg1, shift amount is mod 32
274274
{name: "SHRQconst", argLength: 1, reg: gp11, asm: "SHRQ", aux: "Int8", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> auxint, shift amount 0-63
275-
{name: "SHRLconst", argLength: 1, reg: gp11, asm: "SHRL", aux: "Int8", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> auxint, shift amount 0-31
276-
{name: "SHRWconst", argLength: 1, reg: gp11, asm: "SHRW", aux: "Int8", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> auxint, shift amount 0-15
277-
{name: "SHRBconst", argLength: 1, reg: gp11, asm: "SHRB", aux: "Int8", resultInArg0: true, clobberFlags: true}, // unsigned arg0 >> auxint, shift amount 0-7
275+
{name: "SHRLconst", argLength: 1, reg: gp11, asm: "SHRL", aux: "Int8", resultInArg0: true, clobberFlags: true}, // unsigned uint32(arg0) >> auxint, shift amount 0-31
276+
{name: "SHRWconst", argLength: 1, reg: gp11, asm: "SHRW", aux: "Int8", resultInArg0: true, clobberFlags: true}, // unsigned uint16(arg0) >> auxint, shift amount 0-15
277+
{name: "SHRBconst", argLength: 1, reg: gp11, asm: "SHRB", aux: "Int8", resultInArg0: true, clobberFlags: true}, // unsigned uint8(arg0) >> auxint, shift amount 0-7
278278

279279
{name: "SARQ", argLength: 2, reg: gp21shift, asm: "SARQ", resultInArg0: true, clobberFlags: true}, // signed arg0 >> arg1, shift amount is mod 64
280-
{name: "SARL", argLength: 2, reg: gp21shift, asm: "SARL", resultInArg0: true, clobberFlags: true}, // signed arg0 >> arg1, shift amount is mod 32
281-
{name: "SARW", argLength: 2, reg: gp21shift, asm: "SARW", resultInArg0: true, clobberFlags: true}, // signed arg0 >> arg1, shift amount is mod 32
282-
{name: "SARB", argLength: 2, reg: gp21shift, asm: "SARB", resultInArg0: true, clobberFlags: true}, // signed arg0 >> arg1, shift amount is mod 32
280+
{name: "SARL", argLength: 2, reg: gp21shift, asm: "SARL", resultInArg0: true, clobberFlags: true}, // signed int32(arg0) >> arg1, shift amount is mod 32
281+
{name: "SARW", argLength: 2, reg: gp21shift, asm: "SARW", resultInArg0: true, clobberFlags: true}, // signed int16(arg0) >> arg1, shift amount is mod 32
282+
{name: "SARB", argLength: 2, reg: gp21shift, asm: "SARB", resultInArg0: true, clobberFlags: true}, // signed int8(arg0) >> arg1, shift amount is mod 32
283283
{name: "SARQconst", argLength: 1, reg: gp11, asm: "SARQ", aux: "Int8", resultInArg0: true, clobberFlags: true}, // signed arg0 >> auxint, shift amount 0-63
284-
{name: "SARLconst", argLength: 1, reg: gp11, asm: "SARL", aux: "Int8", resultInArg0: true, clobberFlags: true}, // signed arg0 >> auxint, shift amount 0-31
285-
{name: "SARWconst", argLength: 1, reg: gp11, asm: "SARW", aux: "Int8", resultInArg0: true, clobberFlags: true}, // signed arg0 >> auxint, shift amount 0-15
286-
{name: "SARBconst", argLength: 1, reg: gp11, asm: "SARB", aux: "Int8", resultInArg0: true, clobberFlags: true}, // signed arg0 >> auxint, shift amount 0-7
284+
{name: "SARLconst", argLength: 1, reg: gp11, asm: "SARL", aux: "Int8", resultInArg0: true, clobberFlags: true}, // signed int32(arg0) >> auxint, shift amount 0-31
285+
{name: "SARWconst", argLength: 1, reg: gp11, asm: "SARW", aux: "Int8", resultInArg0: true, clobberFlags: true}, // signed int16(arg0) >> auxint, shift amount 0-15
286+
{name: "SARBconst", argLength: 1, reg: gp11, asm: "SARB", aux: "Int8", resultInArg0: true, clobberFlags: true}, // signed int8(arg0) >> auxint, shift amount 0-7
287287

288288
{name: "ROLQ", argLength: 2, reg: gp21shift, asm: "ROLQ", resultInArg0: true, clobberFlags: true}, // arg0 rotate left arg1 bits.
289289
{name: "ROLL", argLength: 2, reg: gp21shift, asm: "ROLL", resultInArg0: true, clobberFlags: true}, // arg0 rotate left arg1 bits.

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

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

test/fixedbugs/issue23812.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
// run
2+
3+
// Copyright 2018 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+
func main() {
12+
want := int32(0x3edae8)
13+
got := foo(1)
14+
if want != got {
15+
panic(fmt.Sprintf("want %x, got %x", want, got))
16+
}
17+
}
18+
19+
func foo(a int32) int32 {
20+
return shr1(int32(shr2(int64(0x14ff6e2207db5d1f), int(a))), 4)
21+
}
22+
23+
func shr1(n int32, m int) int32 { return n >> uint(m) }
24+
25+
func shr2(n int64, m int) int64 {
26+
if m < 0 {
27+
m = -m
28+
}
29+
if m >= 64 {
30+
return n
31+
}
32+
33+
return n >> uint(m)
34+
}

0 commit comments

Comments
 (0)