Skip to content

Commit 2d41444

Browse files
zdjonesrasky
authored andcommitted
cmd/compile: make prove learn index >= 0 from successful bounds checks
When branching at a bounds check for indexing or slicing ops, prove currently only learns from the upper bound. On the positive branch, we currently learn i < len(a) (or i <= len(a)) in both the signed and unsigned domains. This CL makes prove also learn from the lower bound. Specifically, on the positive branch from index or slicing ops, prove will now ALSO learn i >= 0 in the signed domain (this fact is of no value in the unsigned domain). The substantive change itself is only an additional call to addRestrictions, though I've also inverted the nested switch statements around that call for the sake of clarity. This CL removes 92 bounds checks from std and cmd. It passes all tests and shows no deltas on compilecmp. Fixes #28885 Change-Id: I13eccc36e640eb599fa6dc5aa3be3c7d7abd2d9e Reviewed-on: https://go-review.googlesource.com/c/go/+/170121 Run-TryBot: Daniel Martí <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Giovanni Bajo <[email protected]>
1 parent 637f34f commit 2d41444

File tree

2 files changed

+39
-19
lines changed

2 files changed

+39
-19
lines changed

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

Lines changed: 29 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -931,31 +931,41 @@ func addBranchRestrictions(ft *factsTable, b *Block, br branch) {
931931
if d == signed && ft.isNonNegative(c.Args[0]) && ft.isNonNegative(c.Args[1]) {
932932
d |= unsigned
933933
}
934-
switch br {
935-
case negative:
936-
switch b.Control.Op { // Special cases
937-
case OpIsInBounds, OpIsSliceInBounds:
938-
// 0 <= a0 < a1 (or 0 <= a0 <= a1)
939-
//
940-
// On the positive branch, we learn a0 < a1,
941-
// both signed and unsigned.
942-
//
943-
// On the negative branch, we learn (0 > a0 ||
944-
// a0 >= a1). In the unsigned domain, this is
945-
// simply a0 >= a1 (which is the reverse of the
946-
// positive branch, so nothing surprising).
947-
// But in the signed domain, we can't express the ||
948-
// condition, so check if a0 is non-negative instead,
949-
// to be able to learn something.
934+
switch b.Control.Op {
935+
case OpIsInBounds, OpIsSliceInBounds:
936+
// 0 <= a0 < a1 (or 0 <= a0 <= a1)
937+
//
938+
// On the positive branch, we learn:
939+
// signed: 0 <= a0 < a1 (or 0 <= a0 <= a1)
940+
// unsigned: a0 < a1 (or a0 <= a1)
941+
//
942+
// On the negative branch, we learn (0 > a0 ||
943+
// a0 >= a1). In the unsigned domain, this is
944+
// simply a0 >= a1 (which is the reverse of the
945+
// positive branch, so nothing surprising).
946+
// But in the signed domain, we can't express the ||
947+
// condition, so check if a0 is non-negative instead,
948+
// to be able to learn something.
949+
switch br {
950+
case negative:
950951
d = unsigned
951952
if ft.isNonNegative(c.Args[0]) {
952953
d |= signed
953954
}
955+
addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r^(lt|gt|eq))
956+
case positive:
957+
addRestrictions(b, ft, signed, ft.zero, c.Args[0], lt|eq)
958+
addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r)
959+
}
960+
default:
961+
switch br {
962+
case negative:
963+
addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r^(lt|gt|eq))
964+
case positive:
965+
addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r)
954966
}
955-
addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r^(lt|gt|eq))
956-
case positive:
957-
addRestrictions(b, ft, d, c.Args[0], c.Args[1], tr.r)
958967
}
968+
959969
}
960970
}
961971

test/prove.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,16 @@ func signHint2(b []byte, n int) {
726726
}
727727
}
728728

729+
// indexGT0 tests whether prove learns int index >= 0 from bounds check.
730+
func indexGT0(b []byte, n int) {
731+
_ = b[n]
732+
_ = b[25]
733+
734+
for i := n; i <= 25; i++ { // ERROR "Induction variable: limits \[\?,25\], increment 1$"
735+
b[i] = 123 // ERROR "Proved IsInBounds$"
736+
}
737+
}
738+
729739
// Induction variable in unrolled loop.
730740
func unrollUpExcl(a []int) int {
731741
var i, x int

0 commit comments

Comments
 (0)