cmd/compile: don't rewrite (CMP (AND x y) 0) to TEST if AND has other uses

If the AND has other uses, we end up saving an argument to the AND
in another register, so we can use it for the TEST. No point in doing that.

Change-Id: I73444a6aeddd6f55e2328ce04d77c3e6cf4a83e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/241280
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/ssa/gen/AMD64.rules b/src/cmd/compile/internal/ssa/gen/AMD64.rules
index 9967c7b0305ea76a2a98b4f9ee54f9ddac6503fa..5111ef79d3acbc2bf88b13fe4e6389f3a3f86e17 100644 (file)
--- a/src/cmd/compile/internal/ssa/gen/AMD64.rules
+++ b/src/cmd/compile/internal/ssa/gen/AMD64.rules
@@ -1463,14 +1463,14 @@
 (MULQconst [c] (NEGQ x)) && c != -(1<<31) -> (MULQconst [-c] x)
 
 // checking AND against 0.
-(CMPQconst (ANDQ x y) [0]) -> (TESTQ x y)
-(CMPLconst (ANDL x y) [0]) -> (TESTL x y)
-(CMPWconst (ANDL x y) [0]) -> (TESTW x y)
-(CMPBconst (ANDL x y) [0]) -> (TESTB x y)
-(CMPQconst (ANDQconst [c] x) [0]) -> (TESTQconst [c] x)
-(CMPLconst (ANDLconst [c] x) [0]) -> (TESTLconst [c] x)
-(CMPWconst (ANDLconst [c] x) [0]) -> (TESTWconst [int64(int16(c))] x)
-(CMPBconst (ANDLconst [c] x) [0]) -> (TESTBconst [int64(int8(c))] x)
+(CMPQconst a:(ANDQ x y) [0]) && a.Uses == 1 -> (TESTQ x y)
+(CMPLconst a:(ANDL x y) [0]) && a.Uses == 1 -> (TESTL x y)
+(CMPWconst a:(ANDL x y) [0]) && a.Uses == 1 -> (TESTW x y)
+(CMPBconst a:(ANDL x y) [0]) && a.Uses == 1 -> (TESTB x y)
+(CMPQconst a:(ANDQconst [c] x) [0]) && a.Uses == 1 -> (TESTQconst [c] x)
+(CMPLconst a:(ANDLconst [c] x) [0]) && a.Uses == 1 -> (TESTLconst [c] x)
+(CMPWconst a:(ANDLconst [c] x) [0]) && a.Uses == 1 -> (TESTWconst [int64(int16(c))] x)
+(CMPBconst a:(ANDLconst [c] x) [0]) && a.Uses == 1 -> (TESTBconst [int64(int8(c))] x)
 
 // Convert TESTx to TESTxconst if possible.
 (TESTQ (MOVQconst [c]) x) && is32Bit(c) -> (TESTQconst [c] x)

diff --git a/src/cmd/compile/internal/ssa/rewriteAMD64.go b/src/cmd/compile/internal/ssa/rewriteAMD64.go
index 20eab05e9ca2ab02df8494eaa1c2c6b98e7bc347..cda9df56f4d928df316dabdc395295055d39a215 100644 (file)
--- a/src/cmd/compile/internal/ssa/rewriteAMD64.go
+++ b/src/cmd/compile/internal/ssa/rewriteAMD64.go
@@ -6924,26 +6924,42 @@ func rewriteValueAMD64_OpAMD64CMPBconst(v *Value) bool {
                v.reset(OpAMD64FlagLT_ULT)
                return true
        }
-       // match: (CMPBconst (ANDL x y) [0])
+       // match: (CMPBconst a:(ANDL x y) [0])
+       // cond: a.Uses == 1
        // result: (TESTB x y)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDL {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDL {
+                       break
+               }
+               y := a.Args[1]
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               y := v_0.Args[1]
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTB)
                v.AddArg2(x, y)
                return true
        }
-       // match: (CMPBconst (ANDLconst [c] x) [0])
+       // match: (CMPBconst a:(ANDLconst [c] x) [0])
+       // cond: a.Uses == 1
        // result: (TESTBconst [int64(int8(c))] x)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDLconst {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDLconst {
+                       break
+               }
+               c := a.AuxInt
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               c := v_0.AuxInt
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTBconst)
                v.AuxInt = int64(int8(c))
                v.AddArg(x)
@@ -7309,26 +7325,42 @@ func rewriteValueAMD64_OpAMD64CMPLconst(v *Value) bool {
                v.reset(OpAMD64FlagLT_ULT)
                return true
        }
-       // match: (CMPLconst (ANDL x y) [0])
+       // match: (CMPLconst a:(ANDL x y) [0])
+       // cond: a.Uses == 1
        // result: (TESTL x y)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDL {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDL {
+                       break
+               }
+               y := a.Args[1]
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               y := v_0.Args[1]
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTL)
                v.AddArg2(x, y)
                return true
        }
-       // match: (CMPLconst (ANDLconst [c] x) [0])
+       // match: (CMPLconst a:(ANDLconst [c] x) [0])
+       // cond: a.Uses == 1
        // result: (TESTLconst [c] x)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDLconst {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDLconst {
+                       break
+               }
+               c := a.AuxInt
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               c := v_0.AuxInt
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTLconst)
                v.AuxInt = c
                v.AddArg(x)
@@ -7874,26 +7906,42 @@ func rewriteValueAMD64_OpAMD64CMPQconst(v *Value) bool {
                v.reset(OpAMD64FlagLT_ULT)
                return true
        }
-       // match: (CMPQconst (ANDQ x y) [0])
+       // match: (CMPQconst a:(ANDQ x y) [0])
+       // cond: a.Uses == 1
        // result: (TESTQ x y)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDQ {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDQ {
+                       break
+               }
+               y := a.Args[1]
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               y := v_0.Args[1]
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTQ)
                v.AddArg2(x, y)
                return true
        }
-       // match: (CMPQconst (ANDQconst [c] x) [0])
+       // match: (CMPQconst a:(ANDQconst [c] x) [0])
+       // cond: a.Uses == 1
        // result: (TESTQconst [c] x)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDQconst {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDQconst {
+                       break
+               }
+               c := a.AuxInt
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               c := v_0.AuxInt
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTQconst)
                v.AuxInt = c
                v.AddArg(x)
@@ -8244,26 +8292,42 @@ func rewriteValueAMD64_OpAMD64CMPWconst(v *Value) bool {
                v.reset(OpAMD64FlagLT_ULT)
                return true
        }
-       // match: (CMPWconst (ANDL x y) [0])
+       // match: (CMPWconst a:(ANDL x y) [0])
+       // cond: a.Uses == 1
        // result: (TESTW x y)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDL {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDL {
+                       break
+               }
+               y := a.Args[1]
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               y := v_0.Args[1]
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTW)
                v.AddArg2(x, y)
                return true
        }
-       // match: (CMPWconst (ANDLconst [c] x) [0])
+       // match: (CMPWconst a:(ANDLconst [c] x) [0])
+       // cond: a.Uses == 1
        // result: (TESTWconst [int64(int16(c))] x)
        for {
-               if v.AuxInt != 0 || v_0.Op != OpAMD64ANDLconst {
+               if v.AuxInt != 0 {
+                       break
+               }
+               a := v_0
+               if a.Op != OpAMD64ANDLconst {
+                       break
+               }
+               c := a.AuxInt
+               x := a.Args[0]
+               if !(a.Uses == 1) {
                        break
                }
-               c := v_0.AuxInt
-               x := v_0.Args[0]
                v.reset(OpAMD64TESTWconst)
                v.AuxInt = int64(int16(c))
                v.AddArg(x)

diff --git a/test/codegen/logic.go b/test/codegen/logic.go
new file mode 100644 (file)
index 0000000..9afdfd7
--- /dev/null
+++ b/test/codegen/logic.go
@@ -0,0 +1,24 @@
+// asmcheck
+
+// Copyright 2018 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 codegen
+
+var gx, gy int
+
+// Test to make sure that (CMPQ (ANDQ x y) [0]) does not get rewritten to
+// (TESTQ x y) if the ANDQ has other uses. If that rewrite happens, then one
+// of the args of the ANDQ needs to be saved so it can be used as the arg to TESTQ.
+func andWithUse(x, y int) int {
+       // Load x,y into registers, so those MOVQ will not appear at the z := x&y line.
+       gx, gy = x, y
+       // amd64:-"MOVQ"
+       z := x & y
+       if z == 0 {
+               return 77
+       }
+       // use z by returning it
+       return z
+}