cmd/compile: rework induction variable detector

Induction variable detection is still not quite right. I've added
another failing test.

Redo the overflow/underflow detector so it is more obviously correct.

Update #53600
Fixes #53653
Fixes #53663

Change-Id: Id95228e282fdbf6bd80b26e1c41d62e935ba08ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/415874
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: David Chase <drchase@google.com>

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

@@ -5,6 +5,7 @@
 package ssa
 
 import (
+	"cmd/compile/internal/base"
 	"fmt"
 	"math"
 )
@@ -90,41 +91,42 @@ func findIndVar(f *Func) []indVar {
 			continue
 		}
 
-		var flags indVarFlags
+		var ind *Value   // induction variable
-		var ind, max *Value // induction, and maximum
+		var init *Value  // starting value
+		var limit *Value // ending value
 
-		// Check thet the control if it either ind </<= max or max >/>= ind.
+		// Check thet the control if it either ind </<= limit or limit </<= ind.
 		// TODO: Handle 32-bit comparisons.
 		// TODO: Handle unsigned comparisons?
 		c := b.Controls[0]
+		inclusive := false
 		switch c.Op {
 		case OpLeq64:
-			flags |= indVarMaxInc
+			inclusive = true
 			fallthrough
 		case OpLess64:
-			ind, max = c.Args[0], c.Args[1]
+			ind, limit = c.Args[0], c.Args[1]
 		default:
 			continue
 		}
 
 		// See if this is really an induction variable
 		less := true
-		min, inc, nxt := parseIndVar(ind)
+		init, inc, nxt := parseIndVar(ind)
-		if min == nil {
+		if init == nil {
 			// We failed to parse the induction variable. Before punting, we want to check
-			// whether the control op was written with arguments in non-idiomatic order,
+			// whether the control op was written with the induction variable on the RHS
-			// so that we believe being "max" (the upper bound) is actually the induction
+			// instead of the LHS. This happens for the downwards case, like:
-			// variable itself. This would happen for code like:
+			//     for i := len(n)-1; i >= 0; i--
-			//     for i := 0; len(n) > i; i++
+			init, inc, nxt = parseIndVar(limit)
-			min, inc, nxt = parseIndVar(max)
+			if init == nil {
-			if min == nil {
 				// No recognied induction variable on either operand
 				continue
 			}
 
 			// Ok, the arguments were reversed. Swap them, and remember that we're
 			// looking at a ind >/>= loop (so the induction must be decrementing).
-			ind, max = max, ind
+			ind, limit = limit, ind
 			less = false
 		}
 
@@ -138,8 +140,8 @@ func findIndVar(f *Func) []indVar {
 		}
 
 		// Increment sign must match comparison direction.
-		// When incrementing, the termination comparison must be ind </<= max.
+		// When incrementing, the termination comparison must be ind </<= limit.
-		// When decrementing, the termination comparison must be ind >/>= max.
+		// When decrementing, the termination comparison must be ind >/>= limit.
 		// See issue 26116.
 		if step > 0 && !less {
 			continue
@@ -148,177 +150,229 @@ func findIndVar(f *Func) []indVar {
 			continue
 		}
 
-		// If the increment is negative, swap min/max and their flags
-		if step < 0 {
-			min, max = max, min
-			oldf := flags
-			flags = indVarMaxInc
-			if oldf&indVarMaxInc == 0 {
-				flags |= indVarMinExc
-			}
-			step = -step
-		}
-
-		if flags&indVarMaxInc != 0 && max.Op == OpConst64 && max.AuxInt+step < max.AuxInt {
-			// For a <= comparison, we need to make sure that a value equal to
-			// max can be incremented without overflowing.
-			// (For a < comparison, the %step check below ensures no overflow.)
-			continue
-		}
-
 		// Up to now we extracted the induction variable (ind),
 		// the increment delta (inc), the temporary sum (nxt),
-		// the minimum value (min) and the maximum value (max).
+		// the initial value (init) and the limiting value (limit).
 		//
-		// We also know that ind has the form (Phi min nxt) where
+		// We also know that ind has the form (Phi init nxt) where
 		// nxt is (Add inc nxt) which means: 1) inc dominates nxt
 		// and 2) there is a loop starting at inc and containing nxt.
 		//
 		// We need to prove that the induction variable is incremented
-		// only when it's smaller than the maximum value.
+		// only when it's smaller than the limiting value.
 		// Two conditions must happen listed below to accept ind
 		// as an induction variable.
 
 		// First condition: loop entry has a single predecessor, which
 		// is the header block.  This implies that b.Succs[0] is
-		// reached iff ind < max.
+		// reached iff ind < limit.
 		if len(b.Succs[0].b.Preds) != 1 {
 			// b.Succs[1] must exit the loop.
 			continue
 		}
 
 		// Second condition: b.Succs[0] dominates nxt so that
-		// nxt is computed when inc < max, meaning nxt <= max.
+		// nxt is computed when inc < limit.
 		if !sdom.IsAncestorEq(b.Succs[0].b, nxt.Block) {
 			// inc+ind can only be reached through the branch that enters the loop.
 			continue
 		}
 
-		// We can only guarantee that the loop runs within limits of induction variable
+		// Check for overflow/underflow. We need to make sure that inc never causes
-		// if (one of)
+		// the induction variable to wrap around.
-		// (1) the increment is ±1
+		// We use a function wrapper here for easy return true / return false / keep going logic.
-		// (2) the limits are constants
+		// This function returns true if the increment will never overflow/underflow.
-		// (3) loop is of the form k0 upto Known_not_negative-k inclusive, step <= k
+		ok := func() bool {
-		// (4) loop is of the form k0 upto Known_not_negative-k exclusive, step <= k+1
+			if step > 0 {
-		// (5) loop is of the form Known_not_negative downto k0, minint+step < k0
+				if limit.Op == OpConst64 {
-		if step > 1 {
+					// Figure out the actual largest value.
-			ok := false
+					v := limit.AuxInt
-			if min.Op == OpConst64 && max.Op == OpConst64 {
+					if !inclusive {
-				if max.AuxInt > min.AuxInt && max.AuxInt%step == min.AuxInt%step { // handle overflow
+						if v == math.MinInt64 {
-					ok = true
+							return false // < minint is never satisfiable.
-				}
-			}
-			// Handle induction variables of these forms.
-			// KNN is known-not-negative.
-			// SIGNED ARITHMETIC ONLY. (see switch on c above)
-			// Possibilities for KNN are len and cap; perhaps we can infer others.
-			// for i := 0; i <= KNN-k    ; i += k
-			// for i := 0; i <  KNN-(k-1); i += k
-			// Also handle decreasing.
-
-			// "Proof" copied from https://go-review.googlesource.com/c/go/+/104041/10/src/cmd/compile/internal/ssa/loopbce.go#164
-			//
-			//	In the case of
-			//	// PC is Positive Constant
-			//	L := len(A)-PC
-			//	for i := 0; i < L; i = i+PC
-			//
-			//	we know:
-			//
-			//	0 + PC does not over/underflow.
-			//	len(A)-PC does not over/underflow
-			//	maximum value for L is MaxInt-PC
-			//	i < L <= MaxInt-PC means i + PC < MaxInt hence no overflow.
-
-			// To match in SSA:
-			// if  (a) min.Op == OpConst64(k0)
-			// and (b) k0 >= MININT + step
-			// and (c) max.Op == OpSubtract(Op{StringLen,SliceLen,SliceCap}, k)
-			// or  (c) max.Op == OpAdd(Op{StringLen,SliceLen,SliceCap}, -k)
-			// or  (c) max.Op == Op{StringLen,SliceLen,SliceCap}
-			// and (d) if upto loop, require indVarMaxInc && step <= k or !indVarMaxInc && step-1 <= k
-
-			if min.Op == OpConst64 && min.AuxInt >= step+math.MinInt64 {
-				knn := max
-				k := int64(0)
-				var kArg *Value
-
-				switch max.Op {
-				case OpSub64:
-					knn = max.Args[0]
-					kArg = max.Args[1]
-
-				case OpAdd64:
-					knn = max.Args[0]
-					kArg = max.Args[1]
-					if knn.Op == OpConst64 {
-						knn, kArg = kArg, knn
-					}
-				}
-				switch knn.Op {
-				case OpSliceLen, OpStringLen, OpSliceCap:
-				default:
-					knn = nil
-				}
-
-				if kArg != nil && kArg.Op == OpConst64 {
-					k = kArg.AuxInt
-					if max.Op == OpAdd64 {
-						k = -k
-					}
-				}
-				if k >= 0 && knn != nil {
-					if inc.AuxInt > 0 { // increasing iteration
-						// The concern for the relation between step and k is to ensure that iv never exceeds knn
-						// i.e., iv < knn-(K-1) ==> iv + K <= knn; iv <= knn-K ==> iv +K < knn
-						if step <= k || flags&indVarMaxInc == 0 && step-1 == k {
-							ok = true
 						}
-					} else { // decreasing iteration
+						v--
-						// Will be decrementing from max towards min; max is knn-k; will only attempt decrement if
-						// knn-k >[=] min; underflow is only a concern if min-step is not smaller than min.
-						// This all assumes signed integer arithmetic
-						// This is already assured by the test above: min.AuxInt >= step+math.MinInt64
-						ok = true
 					}
+					if init.Op == OpConst64 {
+						// Use stride to compute a better lower limit.
+						if init.AuxInt > v {
+							return false
+						}
+						v = addU(init.AuxInt, diff(v, init.AuxInt)/uint64(step)*uint64(step))
+					}
+					// It is ok if we can't overflow when incrementing from the largest value.
+					return !addWillOverflow(v, step)
+				}
+				if step == 1 && !inclusive {
+					// Can't overflow because maxint is never a possible value.
+					return true
+				}
+				// If the limit is not a constant, check to see if it is a
+				// negative offset from a known non-negative value.
+				knn, k := findKNN(limit)
+				if knn == nil || k < 0 {
+					return false
+				}
+				// limit == (something nonnegative) - k. That subtraction can't underflow, so
+				// we can trust it.
+				if inclusive {
+					// ind <= knn - k cannot overflow if step is at most k
+					return step <= k
+				}
+				// ind < knn - k cannot overflow if step is at most k+1
+				return step <= k+1 && k != math.MaxInt64
+			} else { // step < 0
+				if limit.Op == OpConst64 {
+					// Figure out the actual smallest value.
+					v := limit.AuxInt
+					if !inclusive {
+						if v == math.MaxInt64 {
+							return false // > maxint is never satisfiable.
+						}
+						v++
+					}
+					if init.Op == OpConst64 {
+						// Use stride to compute a better lower limit.
+						if init.AuxInt < v {
+							return false
+						}
+						v = subU(init.AuxInt, diff(init.AuxInt, v)/uint64(-step)*uint64(-step))
+					}
+					// It is ok if we can't underflow when decrementing from the smallest value.
+					return !subWillUnderflow(v, -step)
+				}
+				if step == -1 && !inclusive {
+					// Can't underflow because minint is never a possible value.
+					return true
 				}
 			}
+			return false
 
-			// TODO: other unrolling idioms
+		}
-			// for i := 0; i < KNN - KNN % k ; i += k
-			// for i := 0; i < KNN&^(k-1) ; i += k // k a power of 2
-			// for i := 0; i < KNN&(-k) ; i += k // k a power of 2
 
-			if !ok {
+		if ok() {
-				continue
+			flags := indVarFlags(0)
+			var min, max *Value
+			if step > 0 {
+				min = init
+				max = limit
+				if inclusive {
+					flags |= indVarMaxInc
+				}
+			} else {
+				min = limit
+				max = init
+				flags |= indVarMaxInc
+				if !inclusive {
+					flags |= indVarMinExc
+				}
+				step = -step
 			}
+			if f.pass.debug >= 1 {
+				printIndVar(b, ind, min, max, step, flags)
+			}
+
+			iv = append(iv, indVar{
+				ind:   ind,
+				min:   min,
+				max:   max,
+				entry: b.Succs[0].b,
+				flags: flags,
+			})
+			b.Logf("found induction variable %v (inc = %v, min = %v, max = %v)\n", ind, inc, min, max)
 		}
 
-		if f.pass.debug >= 1 {
+		// TODO: other unrolling idioms
-			printIndVar(b, ind, min, max, step, flags)
+		// for i := 0; i < KNN - KNN % k ; i += k
-		}
+		// for i := 0; i < KNN&^(k-1) ; i += k // k a power of 2
-
+		// for i := 0; i < KNN&(-k) ; i += k // k a power of 2
-		iv = append(iv, indVar{
-			ind:   ind,
-			min:   min,
-			max:   max,
-			entry: b.Succs[0].b,
-			flags: flags,
-		})
-		b.Logf("found induction variable %v (inc = %v, min = %v, max = %v)\n", ind, inc, min, max)
 	}
 
 	return iv
 }
 
-func dropAdd64(v *Value) (*Value, int64) {
+// addWillOverflow reports whether x+y would result in a value more than maxint.
-	if v.Op == OpAdd64 && v.Args[0].Op == OpConst64 {
+func addWillOverflow(x, y int64) bool {
-		return v.Args[1], v.Args[0].AuxInt
+	return x+y < x
+}
+
+// subWillUnderflow reports whether x-y would result in a value less than minint.
+func subWillUnderflow(x, y int64) bool {
+	return x-y > x
+}
+
+// diff returns x-y as a uint64. Requires x>=y.
+func diff(x, y int64) uint64 {
+	if x < y {
+		base.Fatalf("diff %d - %d underflowed", x, y)
 	}
-	if v.Op == OpAdd64 && v.Args[1].Op == OpConst64 {
+	return uint64(x - y)
-		return v.Args[0], v.Args[1].AuxInt
+}
+
+// addU returns x+y. Requires that x+y does not overflow an int64.
+func addU(x int64, y uint64) int64 {
+	if y >= 1<<63 {
+		if x >= 0 {
+			base.Fatalf("addU overflowed %d + %d", x, y)
+		}
+		x += 1<<63 - 1
+		x += 1
+		y -= 1 << 63
 	}
-	return v, 0
+	if addWillOverflow(x, int64(y)) {
+		base.Fatalf("addU overflowed %d + %d", x, y)
+	}
+	return x + int64(y)
+}
+
+// subU returns x-y. Requires that x-y does not underflow an int64.
+func subU(x int64, y uint64) int64 {
+	if y >= 1<<63 {
+		if x < 0 {
+			base.Fatalf("subU underflowed %d - %d", x, y)
+		}
+		x -= 1<<63 - 1
+		x -= 1
+		y -= 1 << 63
+	}
+	if subWillUnderflow(x, int64(y)) {
+		base.Fatalf("subU underflowed %d - %d", x, y)
+	}
+	return x - int64(y)
+}
+
+// if v is known to be x - c, where x is known to be nonnegative and c is a
+// constant, return x, c. Otherwise return nil, 0.
+func findKNN(v *Value) (*Value, int64) {
+	var x, y *Value
+	x = v
+	switch v.Op {
+	case OpSub64:
+		x = v.Args[0]
+		y = v.Args[1]
+
+	case OpAdd64:
+		x = v.Args[0]
+		y = v.Args[1]
+		if x.Op == OpConst64 {
+			x, y = y, x
+		}
+	}
+	switch x.Op {
+	case OpSliceLen, OpStringLen, OpSliceCap:
+	default:
+		return nil, 0
+	}
+	if y == nil {
+		return x, 0
+	}
+	if y.Op != OpConst64 {
+		return nil, 0
+	}
+	if v.Op == OpAdd64 {
+		return x, -y.AuxInt
+	}
+	return x, y.AuxInt
 }
 
 func printIndVar(b *Block, i, min, max *Value, inc int64, flags indVarFlags) {

test/fixedbugs/issue53600.go

@@ -12,6 +12,7 @@ func main() {
 	f()
 	g()
 	h()
+	j(math.MinInt64)
 }
 func f() {
 	for i := int64(math.MaxInt64); i <= math.MaxInt64; i++ {
@@ -40,3 +41,13 @@ func h() {
 		println(i, i < 0)
 	}
 }
+
+//go:noinline
+func j(i int64) {
+	for j := int64(math.MaxInt64); j <= i-1; j++ {
+		if j < 0 {
+			break
+		}
+		println(j)
+	}
+}

test/fixedbugs/issue53600.out

@@ -6,3 +6,4 @@ done
 9223372036854775805 false
 9223372036854775807 false
 done
+9223372036854775807

test/fixedbugs/issue53653.go

@@ -0,0 +1,42 @@
+// run
+
+// Copyright 2022 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 main
+
+import "math"
+
+func main() {
+	f()
+	g()
+	h()
+}
+func f() {
+	for i := int64(math.MinInt64); i >= math.MinInt64; i-- {
+		if i > 0 {
+			println("done")
+			return
+		}
+		println(i, i > 0)
+	}
+}
+func g() {
+	for i := int64(math.MinInt64) + 1; i >= math.MinInt64; i-- {
+		if i > 0 {
+			println("done")
+			return
+		}
+		println(i, i > 0)
+	}
+}
+func h() {
+	for i := int64(math.MinInt64) + 2; i >= math.MinInt64; i -= 2 {
+		if i > 0 {
+			println("done")
+			return
+		}
+		println(i, i > 0)
+	}
+}

test/fixedbugs/issue53653.out

@@ -0,0 +1,8 @@
+-9223372036854775808 false
+done
+-9223372036854775807 false
+-9223372036854775808 false
+done
+-9223372036854775806 false
+-9223372036854775808 false
+done

test/loopbce.go

@@ -3,6 +3,8 @@
 
 package main
 
+import "math"
+
 func f0a(a []int) int {
 	x := 0
 	for i := range a { // ERROR "Induction variable: limits \[0,\?\), increment 1$"
@@ -281,8 +283,8 @@ func d2(a [100]int) [100]int {
 
 func d3(a [100]int) [100]int {
 	for i := 0; i <= 99; i++ { // ERROR "Induction variable: limits \[0,99\], increment 1$"
-		for j := 0; j <= i-1; j++ { // ERROR "Induction variable: limits \[0,\?\], increment 1$"
+		for j := 0; j <= i-1; j++ {
-			a[j] = 0   // ERROR "Proved IsInBounds$"
+			a[j] = 0
 			a[j+1] = 0 // ERROR "Proved IsInBounds$"
 			a[j+2] = 0
 		}
@@ -290,7 +292,61 @@ func d3(a [100]int) [100]int {
 	return a
 }
 
-func nobce1() {
+func d4() {
+	for i := int64(math.MaxInt64 - 9); i < math.MaxInt64-2; i += 4 { // ERROR "Induction variable: limits \[9223372036854775798,9223372036854775805\), increment 4$"
+		useString("foo")
+	}
+	for i := int64(math.MaxInt64 - 8); i < math.MaxInt64-2; i += 4 { // ERROR "Induction variable: limits \[9223372036854775799,9223372036854775805\), increment 4$"
+		useString("foo")
+	}
+	for i := int64(math.MaxInt64 - 7); i < math.MaxInt64-2; i += 4 {
+		useString("foo")
+	}
+	for i := int64(math.MaxInt64 - 6); i < math.MaxInt64-2; i += 4 { // ERROR "Induction variable: limits \[9223372036854775801,9223372036854775805\), increment 4$"
+		useString("foo")
+	}
+	for i := int64(math.MaxInt64 - 9); i <= math.MaxInt64-2; i += 4 { // ERROR "Induction variable: limits \[9223372036854775798,9223372036854775805\], increment 4$"
+		useString("foo")
+	}
+	for i := int64(math.MaxInt64 - 8); i <= math.MaxInt64-2; i += 4 { // ERROR "Induction variable: limits \[9223372036854775799,9223372036854775805\], increment 4$"
+		useString("foo")
+	}
+	for i := int64(math.MaxInt64 - 7); i <= math.MaxInt64-2; i += 4 {
+		useString("foo")
+	}
+	for i := int64(math.MaxInt64 - 6); i <= math.MaxInt64-2; i += 4 {
+		useString("foo")
+	}
+}
+
+func d5() {
+	for i := int64(math.MinInt64 + 9); i > math.MinInt64+2; i -= 4 { // ERROR "Induction variable: limits \(-9223372036854775806,-9223372036854775799\], increment 4"
+		useString("foo")
+	}
+	for i := int64(math.MinInt64 + 8); i > math.MinInt64+2; i -= 4 { // ERROR "Induction variable: limits \(-9223372036854775806,-9223372036854775800\], increment 4"
+		useString("foo")
+	}
+	for i := int64(math.MinInt64 + 7); i > math.MinInt64+2; i -= 4 {
+		useString("foo")
+	}
+	for i := int64(math.MinInt64 + 6); i > math.MinInt64+2; i -= 4 { // ERROR "Induction variable: limits \(-9223372036854775806,-9223372036854775802\], increment 4"
+		useString("foo")
+	}
+	for i := int64(math.MinInt64 + 9); i >= math.MinInt64+2; i -= 4 { // ERROR "Induction variable: limits \[-9223372036854775806,-9223372036854775799\], increment 4"
+		useString("foo")
+	}
+	for i := int64(math.MinInt64 + 8); i >= math.MinInt64+2; i -= 4 { // ERROR "Induction variable: limits \[-9223372036854775806,-9223372036854775800\], increment 4"
+		useString("foo")
+	}
+	for i := int64(math.MinInt64 + 7); i >= math.MinInt64+2; i -= 4 {
+		useString("foo")
+	}
+	for i := int64(math.MinInt64 + 6); i >= math.MinInt64+2; i -= 4 {
+		useString("foo")
+	}
+}
+
+func bce1() {
 	// tests overflow of max-min
 	a := int64(9223372036854774057)
 	b := int64(-1547)
@@ -300,8 +356,7 @@ func nobce1() {
 		panic("invalid test: modulos should differ")
 	}
 
-	for i := b; i < a; i += z {
+	for i := b; i < a; i += z { // ERROR "Induction variable: limits \[-1547,9223372036854774057\), increment 1337"
-		// No induction variable is possible because i will overflow a first iteration.
 		useString("foobar")
 	}
 }