Skip to content

Commit 2cb10d4

Browse files
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Cherry Zhang <[email protected]>
1 parent daf39d0 commit 2cb10d4

File tree

3 files changed

+103
-5
lines changed

3 files changed

+103
-5
lines changed

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

+28-5
Original file line numberDiff line numberDiff line change
@@ -1189,15 +1189,38 @@ func simplifyBlock(sdom SparseTree, ft *factsTable, b *Block) {
11891189
}
11901190
v.Op = ctzNonZeroOp[v.Op]
11911191
}
1192-
1192+
case OpRsh8x8, OpRsh8x16, OpRsh8x32, OpRsh8x64,
1193+
OpRsh16x8, OpRsh16x16, OpRsh16x32, OpRsh16x64,
1194+
OpRsh32x8, OpRsh32x16, OpRsh32x32, OpRsh32x64,
1195+
OpRsh64x8, OpRsh64x16, OpRsh64x32, OpRsh64x64:
1196+
// Check whether, for a >> b, we know that a is non-negative
1197+
// and b is all of a's bits except the MSB. If so, a is shifted to zero.
1198+
bits := 8 * v.Type.Size()
1199+
if v.Args[1].isGenericIntConst() && v.Args[1].AuxInt >= bits-1 && ft.isNonNegative(v.Args[0]) {
1200+
if b.Func.pass.debug > 0 {
1201+
b.Func.Warnl(v.Pos, "Proved %v shifts to zero", v.Op)
1202+
}
1203+
switch bits {
1204+
case 64:
1205+
v.reset(OpConst64)
1206+
case 32:
1207+
v.reset(OpConst32)
1208+
case 16:
1209+
v.reset(OpConst16)
1210+
case 8:
1211+
v.reset(OpConst8)
1212+
default:
1213+
panic("unexpected integer size")
1214+
}
1215+
v.AuxInt = 0
1216+
continue // Be sure not to fallthrough - this is no longer OpRsh.
1217+
}
1218+
// If the Rsh hasn't been replaced with 0, still check if it is bounded.
1219+
fallthrough
11931220
case OpLsh8x8, OpLsh8x16, OpLsh8x32, OpLsh8x64,
11941221
OpLsh16x8, OpLsh16x16, OpLsh16x32, OpLsh16x64,
11951222
OpLsh32x8, OpLsh32x16, OpLsh32x32, OpLsh32x64,
11961223
OpLsh64x8, OpLsh64x16, OpLsh64x32, OpLsh64x64,
1197-
OpRsh8x8, OpRsh8x16, OpRsh8x32, OpRsh8x64,
1198-
OpRsh16x8, OpRsh16x16, OpRsh16x32, OpRsh16x64,
1199-
OpRsh32x8, OpRsh32x16, OpRsh32x32, OpRsh32x64,
1200-
OpRsh64x8, OpRsh64x16, OpRsh64x32, OpRsh64x64,
12011224
OpRsh8Ux8, OpRsh8Ux16, OpRsh8Ux32, OpRsh8Ux64,
12021225
OpRsh16Ux8, OpRsh16Ux16, OpRsh16Ux32, OpRsh16Ux64,
12031226
OpRsh32Ux8, OpRsh32Ux16, OpRsh32Ux32, OpRsh32Ux64,

test/codegen/arithmetic.go

+11
Original file line numberDiff line numberDiff line change
@@ -451,3 +451,14 @@ func addSpecial(a, b, c uint32) (uint32, uint32, uint32) {
451451
c += 128
452452
return a, b, c
453453
}
454+
455+
456+
// Divide -> shift rules usually require fixup for negative inputs.
457+
// If the input is non-negative, make sure the fixup is eliminated.
458+
func divInt(v int64) int64 {
459+
if v < 0 {
460+
return 0
461+
}
462+
// amd64:-`.*SARQ.*63,`, -".*SHRQ", ".*SARQ.*[$]9,"
463+
return v / 512
464+
}

test/prove.go

+64
Original file line numberDiff line numberDiff line change
@@ -956,6 +956,70 @@ func negIndex2(n int) {
956956
useSlice(c)
957957
}
958958

959+
// Check that prove is zeroing these right shifts of positive ints by bit-width - 1.
960+
// e.g (Rsh64x64 <t> n (Const64 <typ.UInt64> [63])) && ft.isNonNegative(n) -> 0
961+
func sh64(n int64) int64 {
962+
if n < 0 {
963+
return n
964+
}
965+
return n >> 63 // ERROR "Proved Rsh64x64 shifts to zero"
966+
}
967+
968+
func sh32(n int32) int32 {
969+
if n < 0 {
970+
return n
971+
}
972+
return n >> 31 // ERROR "Proved Rsh32x64 shifts to zero"
973+
}
974+
975+
func sh32x64(n int32) int32 {
976+
if n < 0 {
977+
return n
978+
}
979+
return n >> uint64(31) // ERROR "Proved Rsh32x64 shifts to zero"
980+
}
981+
982+
func sh16(n int16) int16 {
983+
if n < 0 {
984+
return n
985+
}
986+
return n >> 15 // ERROR "Proved Rsh16x64 shifts to zero"
987+
}
988+
989+
func sh64noopt(n int64) int64 {
990+
return n >> 63 // not optimized; n could be negative
991+
}
992+
993+
// These cases are division of a positive signed integer by a power of 2.
994+
// The opt pass doesnt have sufficient information to see that n is positive.
995+
// So, instead, opt rewrites the division with a less-than-optimal replacement.
996+
// Prove, which can see that n is nonnegative, cannot see the division because
997+
// opt, an earlier pass, has already replaced it.
998+
// The fix for this issue allows prove to zero a right shift that was added as
999+
// part of the less-than-optimal reqwrite. That change by prove then allows
1000+
// lateopt to clean up all the unneccesary parts of the original division
1001+
// replacement. See issue #36159.
1002+
func divShiftClean(n int) int {
1003+
if n < 0 {
1004+
return n
1005+
}
1006+
return n / int(8) // ERROR "Proved Rsh64x64 shifts to zero"
1007+
}
1008+
1009+
func divShiftClean64(n int64) int64 {
1010+
if n < 0 {
1011+
return n
1012+
}
1013+
return n / int64(16) // ERROR "Proved Rsh64x64 shifts to zero"
1014+
}
1015+
1016+
func divShiftClean32(n int32) int32 {
1017+
if n < 0 {
1018+
return n
1019+
}
1020+
return n / int32(16) // ERROR "Proved Rsh32x64 shifts to zero"
1021+
}
1022+
9591023
//go:noinline
9601024
func useInt(a int) {
9611025
}

0 commit comments

Comments
 (0)