clones/go
1
0
Fork 0
mirror of https://github.com/golang/go.git synced 2024-09-21 10:28:27 +00:00
Code Issues Packages Projects Releases Wiki Activity

[release-branch.go1.16] cmd/compile: remove incorrect arm,arm64 CMP->CMN transformations

Browse Source 
These can go wrong when one of the operands is the minimum integer value.

Fixes #50866.

Change-Id: I238fe284f60c7ee5aeb9dc9a18e8b1578cdb77d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/381318
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit b7b44b3173)
Reviewed-on: https://go-review.googlesource.com/c/go/+/381475
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
This commit is contained in:
David Chase 2022-01-27 11:26:59 -05:00 committed by Cherry Mui
parent fcd482a2d0
commit 4d284ea052
7 changed files with 45 additions and 317 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/cmd/compile/internal/ssa/gen/ARM.rules
View File

@ -1263,8 +1263,8 @@
(SRLconst (SLLconst x [c]) [d]) && objabi.GOARM==7 && uint64(d)>=uint64(c) && uint64(d)<=31 => (BFXU [(d-c)|(32-d)<<8] x)
// comparison simplification
((LT|LE|EQ|NE|GE|GT) (CMP x (RSBconst [0] y))) => ((LT|LE|EQ|NE|GE|GT) (CMN x y)) // sense of carry bit not preserved
((LT|LE|EQ|NE|GE|GT) (CMN x (RSBconst [0] y))) => ((LT|LE|EQ|NE|GE|GT) (CMP x y)) // sense of carry bit not preserved
((EQ|NE) (CMP x (RSBconst [0] y))) => ((EQ|NE) (CMN x y)) // sense of carry bit not preserved; see also #50854
((EQ|NE) (CMN x (RSBconst [0] y))) => ((EQ|NE) (CMP x y)) // sense of carry bit not preserved; see also #50864
(EQ (CMPconst [0] l:(SUB x y)) yes no) && l.Uses==1 => (EQ (CMP x y) yes no)
(EQ (CMPconst [0] l:(MULS x y a)) yes no) && l.Uses==1 => (EQ (CMP a (MUL <x.Type> x y)) yes no)
(EQ (CMPconst [0] l:(SUBconst [c] x)) yes no) && l.Uses==1 => (EQ (CMPconst [c] x) yes no)

10
src/cmd/compile/internal/ssa/gen/ARM64.rules
View File

@ -643,19 +643,13 @@
(GT (CMPWconst [0] z:(ADD x y)) yes no) && z.Uses == 1 => (GTnoov (CMNW x y) yes no)
(GE (CMPWconst [0] z:(ADD x y)) yes no) && z.Uses == 1 => (GEnoov (CMNW x y) yes no)
// CMP(x,-y) -> CMN(x,y) is only valid for unordered comparison, if y can be -1<<63
(EQ (CMP x z:(NEG y)) yes no) && z.Uses == 1 => (EQ (CMN x y) yes no)
(NE (CMP x z:(NEG y)) yes no) && z.Uses == 1 => (NE (CMN x y) yes no)
(LT (CMP x z:(NEG y)) yes no) && z.Uses == 1 => (LT (CMN x y) yes no)
(LE (CMP x z:(NEG y)) yes no) && z.Uses == 1 => (LE (CMN x y) yes no)
(GT (CMP x z:(NEG y)) yes no) && z.Uses == 1 => (GT (CMN x y) yes no)
(GE (CMP x z:(NEG y)) yes no) && z.Uses == 1 => (GE (CMN x y) yes no)
// CMPW(x,-y) -> CMNW(x,y) is only valid for unordered comparison, if y can be -1<<31
(EQ (CMPW x z:(NEG y)) yes no) && z.Uses == 1 => (EQ (CMNW x y) yes no)
(NE (CMPW x z:(NEG y)) yes no) && z.Uses == 1 => (NE (CMNW x y) yes no)
(LT (CMPW x z:(NEG y)) yes no) && z.Uses == 1 => (LT (CMNW x y) yes no)
(LE (CMPW x z:(NEG y)) yes no) && z.Uses == 1 => (LE (CMNW x y) yes no)
(GT (CMPW x z:(NEG y)) yes no) && z.Uses == 1 => (GT (CMNW x y) yes no)
(GE (CMPW x z:(NEG y)) yes no) && z.Uses == 1 => (GE (CMNW x y) yes no)
(EQ (CMPconst [0] x) yes no) => (Z x yes no)
(NE (CMPconst [0] x) yes no) => (NZ x yes no)

4
src/cmd/compile/internal/ssa/gen/ARM64Ops.go
View File

@ -281,9 +281,9 @@ func init() {
{name: "CMPconst", argLength: 1, reg: gp1flags, asm: "CMP", aux: "Int64", typ: "Flags"}, // arg0 compare to auxInt
{name: "CMPW", argLength: 2, reg: gp2flags, asm: "CMPW", typ: "Flags"}, // arg0 compare to arg1, 32 bit
{name: "CMPWconst", argLength: 1, reg: gp1flags, asm: "CMPW", aux: "Int32", typ: "Flags"}, // arg0 compare to auxInt, 32 bit
{name: "CMN", argLength: 2, reg: gp2flags, asm: "CMN", typ: "Flags", commutative: true}, // arg0 compare to -arg1
{name: "CMN", argLength: 2, reg: gp2flags, asm: "CMN", typ: "Flags", commutative: true}, // arg0 compare to -arg1, provided arg1 is not 1<<63
{name: "CMNconst", argLength: 1, reg: gp1flags, asm: "CMN", aux: "Int64", typ: "Flags"}, // arg0 compare to -auxInt
{name: "CMNW", argLength: 2, reg: gp2flags, asm: "CMNW", typ: "Flags", commutative: true}, // arg0 compare to -arg1, 32 bit
{name: "CMNW", argLength: 2, reg: gp2flags, asm: "CMNW", typ: "Flags", commutative: true}, // arg0 compare to -arg1, 32 bit, provided arg1 is not 1<<31
{name: "CMNWconst", argLength: 1, reg: gp1flags, asm: "CMNW", aux: "Int32", typ: "Flags"}, // arg0 compare to -auxInt, 32 bit
{name: "TST", argLength: 2, reg: gp2flags, asm: "TST", typ: "Flags", commutative: true}, // arg0 & arg1 compare to 0
{name: "TSTconst", argLength: 1, reg: gp1flags, asm: "TST", aux: "Int64", typ: "Flags"}, // arg0 & auxInt compare to 0

2
src/cmd/compile/internal/ssa/gen/ARMOps.go
View File

@ -329,7 +329,7 @@ func init() {
// comparisons
{name: "CMP", argLength: 2, reg: gp2flags, asm: "CMP", typ: "Flags"}, // arg0 compare to arg1
{name: "CMPconst", argLength: 1, reg: gp1flags, asm: "CMP", aux: "Int32", typ: "Flags"}, // arg0 compare to auxInt
{name: "CMN", argLength: 2, reg: gp2flags, asm: "CMN", typ: "Flags", commutative: true}, // arg0 compare to -arg1
{name: "CMN", argLength: 2, reg: gp2flags, asm: "CMN", typ: "Flags", commutative: true}, // arg0 compare to -arg1, provided arg1 is not 1<<63
{name: "CMNconst", argLength: 1, reg: gp1flags, asm: "CMN", aux: "Int32", typ: "Flags"}, // arg0 compare to -auxInt
{name: "TST", argLength: 2, reg: gp2flags, asm: "TST", typ: "Flags", commutative: true}, // arg0 & arg1 compare to 0
{name: "TSTconst", argLength: 1, reg: gp1flags, asm: "TST", aux: "Int32", typ: "Flags"}, // arg0 & auxInt compare to 0

144
src/cmd/compile/internal/ssa/rewriteARM.go
View File

@ -17123,42 +17123,6 @@ func rewriteBlockARM(b *Block) bool {
b.resetWithControl(BlockARMLE, cmp)
return true
}
// match: (GE (CMP x (RSBconst [0] y)))
// result: (GE (CMN x y))
for b.Controls[0].Op == OpARMCMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
v_0_1 := v_0.Args[1]
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
break
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMGE, v0)
return true
}
// match: (GE (CMN x (RSBconst [0] y)))
// result: (GE (CMP x y))
for b.Controls[0].Op == OpARMCMN {
v_0 := b.Controls[0]
_ = v_0.Args[1]
v_0_0 := v_0.Args[0]
v_0_1 := v_0.Args[1]
for _i0 := 0; _i0 <= 1; _i0, v_0_0, v_0_1 = _i0+1, v_0_1, v_0_0 {
x := v_0_0
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
continue
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMP, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMGE, v0)
return true
}
break
}
// match: (GE (CMPconst [0] l:(SUB x y)) yes no)
// cond: l.Uses==1
// result: (GEnoov (CMP x y) yes no)
@ -18039,42 +18003,6 @@ func rewriteBlockARM(b *Block) bool {
b.resetWithControl(BlockARMLT, cmp)
return true
}
// match: (GT (CMP x (RSBconst [0] y)))
// result: (GT (CMN x y))
for b.Controls[0].Op == OpARMCMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
v_0_1 := v_0.Args[1]
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
break
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMGT, v0)
return true
}
// match: (GT (CMN x (RSBconst [0] y)))
// result: (GT (CMP x y))
for b.Controls[0].Op == OpARMCMN {
v_0 := b.Controls[0]
_ = v_0.Args[1]
v_0_0 := v_0.Args[0]
v_0_1 := v_0.Args[1]
for _i0 := 0; _i0 <= 1; _i0, v_0_0, v_0_1 = _i0+1, v_0_1, v_0_0 {
x := v_0_0
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
continue
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMP, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMGT, v0)
return true
}
break
}
// match: (GT (CMPconst [0] l:(SUB x y)) yes no)
// cond: l.Uses==1
// result: (GTnoov (CMP x y) yes no)
@ -19046,42 +18974,6 @@ func rewriteBlockARM(b *Block) bool {
b.resetWithControl(BlockARMGE, cmp)
return true
}
// match: (LE (CMP x (RSBconst [0] y)))
// result: (LE (CMN x y))
for b.Controls[0].Op == OpARMCMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
v_0_1 := v_0.Args[1]
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
break
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMLE, v0)
return true
}
// match: (LE (CMN x (RSBconst [0] y)))
// result: (LE (CMP x y))
for b.Controls[0].Op == OpARMCMN {
v_0 := b.Controls[0]
_ = v_0.Args[1]
v_0_0 := v_0.Args[0]
v_0_1 := v_0.Args[1]
for _i0 := 0; _i0 <= 1; _i0, v_0_0, v_0_1 = _i0+1, v_0_1, v_0_0 {
x := v_0_0
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
continue
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMP, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMLE, v0)
return true
}
break
}
// match: (LE (CMPconst [0] l:(SUB x y)) yes no)
// cond: l.Uses==1
// result: (LEnoov (CMP x y) yes no)
@ -19962,42 +19854,6 @@ func rewriteBlockARM(b *Block) bool {
b.resetWithControl(BlockARMGT, cmp)
return true
}
// match: (LT (CMP x (RSBconst [0] y)))
// result: (LT (CMN x y))
for b.Controls[0].Op == OpARMCMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
v_0_1 := v_0.Args[1]
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
break
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMLT, v0)
return true
}
// match: (LT (CMN x (RSBconst [0] y)))
// result: (LT (CMP x y))
for b.Controls[0].Op == OpARMCMN {
v_0 := b.Controls[0]
_ = v_0.Args[1]
v_0_0 := v_0.Args[0]
v_0_1 := v_0.Args[1]
for _i0 := 0; _i0 <= 1; _i0, v_0_0, v_0_1 = _i0+1, v_0_1, v_0_0 {
x := v_0_0
if v_0_1.Op != OpARMRSBconst || auxIntToInt32(v_0_1.AuxInt) != 0 {
continue
}
y := v_0_1.Args[0]
v0 := b.NewValue0(v_0.Pos, OpARMCMP, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARMLT, v0)
return true
}
break
}
// match: (LT (CMPconst [0] l:(SUB x y)) yes no)
// cond: l.Uses==1
// result: (LTnoov (CMP x y) yes no)

160
src/cmd/compile/internal/ssa/rewriteARM64.go
View File

@ -26214,46 +26214,6 @@ func rewriteBlockARM64(b *Block) bool {
}
break
}
// match: (GE (CMP x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (GE (CMN x y) yes no)
for b.Controls[0].Op == OpARM64CMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64GE, v0)
return true
}
// match: (GE (CMPW x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (GE (CMNW x y) yes no)
for b.Controls[0].Op == OpARM64CMPW {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMNW, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64GE, v0)
return true
}
// match: (GE (CMPconst [0] z:(MADD a x y)) yes no)
// cond: z.Uses==1
// result: (GEnoov (CMN a (MUL <x.Type> x y)) yes no)
@ -26650,46 +26610,6 @@ func rewriteBlockARM64(b *Block) bool {
}
break
}
// match: (GT (CMP x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (GT (CMN x y) yes no)
for b.Controls[0].Op == OpARM64CMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64GT, v0)
return true
}
// match: (GT (CMPW x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (GT (CMNW x y) yes no)
for b.Controls[0].Op == OpARM64CMPW {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMNW, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64GT, v0)
return true
}
// match: (GT (CMPconst [0] z:(MADD a x y)) yes no)
// cond: z.Uses==1
// result: (GTnoov (CMN a (MUL <x.Type> x y)) yes no)
@ -27182,46 +27102,6 @@ func rewriteBlockARM64(b *Block) bool {
}
break
}
// match: (LE (CMP x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (LE (CMN x y) yes no)
for b.Controls[0].Op == OpARM64CMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64LE, v0)
return true
}
// match: (LE (CMPW x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (LE (CMNW x y) yes no)
for b.Controls[0].Op == OpARM64CMPW {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMNW, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64LE, v0)
return true
}
// match: (LE (CMPconst [0] z:(MADD a x y)) yes no)
// cond: z.Uses==1
// result: (LEnoov (CMN a (MUL <x.Type> x y)) yes no)
@ -27594,46 +27474,6 @@ func rewriteBlockARM64(b *Block) bool {
}
break
}
// match: (LT (CMP x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (LT (CMN x y) yes no)
for b.Controls[0].Op == OpARM64CMP {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMN, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64LT, v0)
return true
}
// match: (LT (CMPW x z:(NEG y)) yes no)
// cond: z.Uses == 1
// result: (LT (CMNW x y) yes no)
for b.Controls[0].Op == OpARM64CMPW {
v_0 := b.Controls[0]
_ = v_0.Args[1]
x := v_0.Args[0]
z := v_0.Args[1]
if z.Op != OpARM64NEG {
break
}
y := z.Args[0]
if !(z.Uses == 1) {
break
}
v0 := b.NewValue0(v_0.Pos, OpARM64CMNW, types.TypeFlags)
v0.AddArg2(x, y)
b.resetWithControl(BlockARM64LT, v0)
return true
}
// match: (LT (CMPconst [0] z:(MADD a x y)) yes no)
// cond: z.Uses==1
// result: (LTnoov (CMN a (MUL <x.Type> x y)) yes no)

38
test/fixedbugs/issue50854.go Normal file
View File

@ -0,0 +1,38 @@
// run
// Copyright 2022 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
// This checks for incorrect application of CMP(-x,y) -> CMN(x,y) in arm and arm64
//go:noinline
func f(p int64, x, y int64) bool { return -x <= p && p <= y }
//go:noinline
func g(p int32, x, y int32) bool { return -x <= p && p <= y }
// There are some more complicated patterns involving compares and shifts, try to trigger those.
//go:noinline
func h(p int64, x, y int64) bool { return -(x<<1) <= p && p <= y }
//go:noinline
func k(p int32, x, y int32) bool { return -(1<<x) <= p && p <= y }
//go:noinline
func check(b bool) {
if b {
return
}
panic("FAILURE")
}
func main() {
check(f(1, -1<<63, 1<<63-1))
check(g(1, -1<<31, 1<<31-1))
check(h(1, -1<<62, 1<<63-1))
check(k(1, 31, 1<<31-1))
}