cmd/compile: in prove, zero right shifts of positive int by #bits - 1

Taking over Zach's CL 212277. Just cleaned up and added a test.

For a positive, signed integer, an arithmetic right shift of count
(bit-width - 1) equals zero. e.g. int64(22) >> 63 -> 0. This CL makes
prove replace these right shifts with a zero-valued constant.

These shifts may arise in source code explicitly, but can also be
created by the generic rewrite of signed division by a power of 2.
// Signed divide by power of 2.
// n / c =       n >> log(c) if n >= 0
//       = (n+c-1) >> log(c) if n < 0
// We conditionally add c-1 by adding n>>63>>(64-log(c))
	(first shift signed, second shift unsigned).
(Div64 <t> n (Const64 [c])) && isPowerOfTwo(c) ->
  (Rsh64x64
    (Add64 <t> n (Rsh64Ux64 <t>
    	(Rsh64x64 <t> n (Const64 <typ.UInt64> [63]))
	(Const64 <typ.UInt64> [64-log2(c)])))
    (Const64 <typ.UInt64> [log2(c)]))

If n is known to be positive, this rewrite includes an extra Add and 2
extra Rsh. This CL will allow prove to replace one of the extra Rsh with
a 0. That replacement then allows lateopt to remove all the unneccesary
fixups from the generic rewrite.

There is a rewrite rule to handle this case directly:
(Div64 n (Const64 [c])) && isNonNegative(n) && isPowerOfTwo(c) ->
	(Rsh64Ux64 n (Const64 <typ.UInt64> [log2(c)]))
But this implementation of isNonNegative really only handles constants
and a few special operations like len/cap. The division could be
handled if the factsTable version of isNonNegative were available.
Unfortunately, the first opt pass happens before prove even has a
chance to deduce the numerator is non-negative, so the generic rewrite
has already fired and created the extra Ops discussed above.

Fixes #36159

By Printf count, this zeroes 137 right shifts when building std and cmd.

Change-Id: Iab486910ac9d7cfb86ace2835456002732b384a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/232857
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

diff --git a/src/cmd/compile/internal/ssa/prove.go b/src/cmd/compile/internal/ssa/prove.go
index 12c2580c957eea366eb3e13b02114d29b889ed4c..a8e43d01147110fd1eaec53c80cfc8d63351a4bf 100644 (file)
--- a/src/cmd/compile/internal/ssa/prove.go
+++ b/src/cmd/compile/internal/ssa/prove.go
@@ -1189,15 +1189,38 @@ func simplifyBlock(sdom SparseTree, ft *factsTable, b *Block) {
                                }
                                v.Op = ctzNonZeroOp[v.Op]
                        }
-
+               case OpRsh8x8, OpRsh8x16, OpRsh8x32, OpRsh8x64,
+                       OpRsh16x8, OpRsh16x16, OpRsh16x32, OpRsh16x64,
+                       OpRsh32x8, OpRsh32x16, OpRsh32x32, OpRsh32x64,
+                       OpRsh64x8, OpRsh64x16, OpRsh64x32, OpRsh64x64:
+                       // Check whether, for a >> b, we know that a is non-negative
+                       // and b is all of a's bits except the MSB. If so, a is shifted to zero.
+                       bits := 8 * v.Type.Size()
+                       if v.Args[1].isGenericIntConst() && v.Args[1].AuxInt >= bits-1 && ft.isNonNegative(v.Args[0]) {
+                               if b.Func.pass.debug > 0 {
+                                       b.Func.Warnl(v.Pos, "Proved %v shifts to zero", v.Op)
+                               }
+                               switch bits {
+                               case 64:
+                                       v.reset(OpConst64)
+                               case 32:
+                                       v.reset(OpConst32)
+                               case 16:
+                                       v.reset(OpConst16)
+                               case 8:
+                                       v.reset(OpConst8)
+                               default:
+                                       panic("unexpected integer size")
+                               }
+                               v.AuxInt = 0
+                               continue // Be sure not to fallthrough - this is no longer OpRsh.
+                       }
+                       // If the Rsh hasn't been replaced with 0, still check if it is bounded.
+                       fallthrough
                case OpLsh8x8, OpLsh8x16, OpLsh8x32, OpLsh8x64,
                        OpLsh16x8, OpLsh16x16, OpLsh16x32, OpLsh16x64,
                        OpLsh32x8, OpLsh32x16, OpLsh32x32, OpLsh32x64,
                        OpLsh64x8, OpLsh64x16, OpLsh64x32, OpLsh64x64,
-                       OpRsh8x8, OpRsh8x16, OpRsh8x32, OpRsh8x64,
-                       OpRsh16x8, OpRsh16x16, OpRsh16x32, OpRsh16x64,
-                       OpRsh32x8, OpRsh32x16, OpRsh32x32, OpRsh32x64,
-                       OpRsh64x8, OpRsh64x16, OpRsh64x32, OpRsh64x64,
                        OpRsh8Ux8, OpRsh8Ux16, OpRsh8Ux32, OpRsh8Ux64,
                        OpRsh16Ux8, OpRsh16Ux16, OpRsh16Ux32, OpRsh16Ux64,
                        OpRsh32Ux8, OpRsh32Ux16, OpRsh32Ux32, OpRsh32Ux64,

diff --git a/test/codegen/arithmetic.go b/test/codegen/arithmetic.go
index a076664e8e339aaf5d88b82a637bd5e2d0b78dcf..8f25974376ba6277e78305c34ef9ca5b5a517130 100644 (file)
--- a/test/codegen/arithmetic.go
+++ b/test/codegen/arithmetic.go
@@ -451,3 +451,14 @@ func addSpecial(a, b, c uint32) (uint32, uint32, uint32) {
        c += 128
        return a, b, c
 }
+
+
+// Divide -> shift rules usually require fixup for negative inputs.
+// If the input is non-negative, make sure the fixup is eliminated.
+func divInt(v int64) int64 {
+       if v < 0 {
+               return 0
+       }
+       // amd64:-`.*SARQ.*63,`, -".*SHRQ", ".*SARQ.*[$]9,"
+       return v / 512
+}

diff --git a/test/prove.go b/test/prove.go
index e5636a452e013426c1d6929996c4538691ab6a7f..d37021d28305ae589156bbbaadbb39e5da32236a 100644 (file)
--- a/test/prove.go
+++ b/test/prove.go
@@ -956,6 +956,70 @@ func negIndex2(n int) {
        useSlice(c)
 }
 
+// Check that prove is zeroing these right shifts of positive ints by bit-width - 1.
+// e.g (Rsh64x64 <t> n (Const64 <typ.UInt64> [63])) && ft.isNonNegative(n) -> 0
+func sh64(n int64) int64 {
+       if n < 0 {
+               return n
+       }
+       return n >> 63 // ERROR "Proved Rsh64x64 shifts to zero"
+}
+
+func sh32(n int32) int32 {
+       if n < 0 {
+               return n
+       }
+       return n >> 31 // ERROR "Proved Rsh32x64 shifts to zero"
+}
+
+func sh32x64(n int32) int32 {
+       if n < 0 {
+               return n
+       }
+       return n >> uint64(31) // ERROR "Proved Rsh32x64 shifts to zero"
+}
+
+func sh16(n int16) int16 {
+       if n < 0 {
+               return n
+       }
+       return n >> 15 // ERROR "Proved Rsh16x64 shifts to zero"
+}
+
+func sh64noopt(n int64) int64 {
+       return n >> 63 // not optimized; n could be negative
+}
+
+// These cases are division of a positive signed integer by a power of 2.
+// The opt pass doesnt have sufficient information to see that n is positive.
+// So, instead, opt rewrites the division with a less-than-optimal replacement.
+// Prove, which can see that n is nonnegative, cannot see the division because
+// opt, an earlier pass, has already replaced it.
+// The fix for this issue allows prove to zero a right shift that was added as
+// part of the less-than-optimal reqwrite. That change by prove then allows
+// lateopt to clean up all the unneccesary parts of the original division
+// replacement. See issue #36159.
+func divShiftClean(n int) int {
+       if n < 0 {
+               return n
+       }
+       return n / int(8) // ERROR "Proved Rsh64x64 shifts to zero"
+}
+
+func divShiftClean64(n int64) int64 {
+       if n < 0 {
+               return n
+       }
+       return n / int64(16) // ERROR "Proved Rsh64x64 shifts to zero"
+}
+
+func divShiftClean32(n int32) int32 {
+       if n < 0 {
+               return n
+       }
+       return n / int32(16) // ERROR "Proved Rsh32x64 shifts to zero"
+}
+
 //go:noinline
 func useInt(a int) {
 }