cmd/compile: avoid applying ARM CMP->CMN rewrite in unsigned context

Fixes #41780.

Change-Id: I1dc7c19a9f057650905da3a96214c2ff4abb51be
Reviewed-on: https://go-review.googlesource.com/c/go/+/259450
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/cmd/compile/internal/ssa/gen/ARM.rules b/src/cmd/compile/internal/ssa/gen/ARM.rules
index 9490805f46d9f353c6ec838db46c5b85365dac76..aad7236d598ec22db982035ad981d0909a5d02e4 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/ARM.rules
+++ b/src/cmd/compile/internal/ssa/gen/ARM.rules
@@ -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
-(CMP x (RSBconst [0] y)) => (CMN x y)
-(CMN x (RSBconst [0] y)) => (CMP x y)
+((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 (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)

diff --git a/src/cmd/compile/internal/ssa/rewriteARM.go b/src/cmd/compile/internal/ssa/rewriteARM.go
index 4e4416516958b08a0ca95881e1a19fe799ae2ff7..435da688b77edeea5c076cc18a1034da5f134485 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteARM.go
+++ b/src/cmd/compile/internal/ssa/rewriteARM.go
@@ -3362,21 +3362,6 @@ func rewriteValueARM_OpARMCMN(v *Value) bool {
                }
                break
        }
-       // match: (CMN x (RSBconst [0] y))
-       // result: (CMP x y)
-       for {
-               for _i0 := 0; _i0 <= 1; _i0, v_0, v_1 = _i0+1, v_1, v_0 {
-                       x := v_0
-                       if v_1.Op != OpARMRSBconst || auxIntToInt32(v_1.AuxInt) != 0 {
-                               continue
-                       }
-                       y := v_1.Args[0]
-                       v.reset(OpARMCMP)
-                       v.AddArg2(x, y)
-                       return true
-               }
-               break
-       }
        return false
 }
 func rewriteValueARM_OpARMCMNconst(v *Value) bool {
@@ -3938,18 +3923,6 @@ func rewriteValueARM_OpARMCMP(v *Value) bool {
                v.AddArg(v0)
                return true
        }
-       // match: (CMP x (RSBconst [0] y))
-       // result: (CMN x y)
-       for {
-               x := v_0
-               if v_1.Op != OpARMRSBconst || auxIntToInt32(v_1.AuxInt) != 0 {
-                       break
-               }
-               y := v_1.Args[0]
-               v.reset(OpARMCMN)
-               v.AddArg2(x, y)
-               return true
-       }
        return false
 }
 func rewriteValueARM_OpARMCMPD(v *Value) bool {
@@ -16002,6 +15975,42 @@ func rewriteBlockARM(b *Block) bool {
                        b.resetWithControl(BlockARMEQ, cmp)
                        return true
                }
+               // match: (EQ (CMP x (RSBconst [0] y)))
+               // result: (EQ (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(BlockARMEQ, v0)
+                       return true
+               }
+               // match: (EQ (CMN x (RSBconst [0] y)))
+               // result: (EQ (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(BlockARMEQ, v0)
+                               return true
+                       }
+                       break
+               }
                // match: (EQ (CMPconst [0] l:(SUB x y)) yes no)
                // cond: l.Uses==1
                // result: (EQ (CMP x y) yes no)
@@ -16848,6 +16857,42 @@ 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)
@@ -17728,6 +17773,42 @@ 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)
@@ -18699,6 +18780,42 @@ 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)
@@ -19579,6 +19696,42 @@ 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)
@@ -20609,6 +20762,42 @@ func rewriteBlockARM(b *Block) bool {
                        b.resetWithControl(BlockARMNE, cmp)
                        return true
                }
+               // match: (NE (CMP x (RSBconst [0] y)))
+               // result: (NE (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(BlockARMNE, v0)
+                       return true
+               }
+               // match: (NE (CMN x (RSBconst [0] y)))
+               // result: (NE (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(BlockARMNE, v0)
+                               return true
+                       }
+                       break
+               }
                // match: (NE (CMPconst [0] l:(SUB x y)) yes no)
                // cond: l.Uses==1
                // result: (NE (CMP x y) yes no)

diff --git a/test/fixedbugs/issue41780.go b/test/fixedbugs/issue41780.go
new file mode 100644 (file)
index 0000000..632c144
--- /dev/null
+++ b/test/fixedbugs/issue41780.go
@@ -0,0 +1,39 @@
+// run
+
+// Copyright 2020 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.
+
+// Checks that conversion of CMP(x,-y) -> CMN(x,y) is only applied in correct context.
+
+package main
+
+type decimal struct {
+       d  [8]byte // digits, big-endian representation
+       dp int     // decimal point
+}
+
+var powtab = []int{1, 3, 6, 9, 13, 16, 19, 23, 26}
+
+//go:noinline
+func foo(d *decimal) int {
+       exp := int(d.d[1])
+       if d.dp < 0 || d.dp == 0 && d.d[0] < '5' {
+               var n int
+               if -d.dp >= len(powtab) {
+                       n = 27
+               } else {
+                       n = powtab[-d.dp] // incorrect CMP -> CMN substitution causes indexing panic.
+               }
+               exp += n
+       }
+       return exp
+}
+
+func main() {
+       var d decimal
+       d.d[0] = '1'
+       if foo(&d) != 1 {
+               println("FAILURE (though not the one this test was written to catch)")
+       }
+}