diff --git a/src/cmd/compile/internal/ssa/loopbce.go b/src/cmd/compile/internal/ssa/loopbce.go index a934cd2c7b..22fb5118ce 100644 --- a/src/cmd/compile/internal/ssa/loopbce.go +++ b/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, max *Value // induction, and maximum + var ind *Value // induction variable + var init *Value // starting value + var limit *Value // ending value - // Check thet the control if it either ind />= ind. + // Check thet the control if it either ind i; i++ - min, inc, nxt = parseIndVar(max) - if min == nil { + // whether the control op was written with the induction variable on the RHS + // instead of the LHS. This happens for the downwards case, like: + // for i := len(n)-1; i >= 0; i-- + init, inc, nxt = parseIndVar(limit) + if init == 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. // 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 - // if (one of) - // (1) the increment is ±1 - // (2) the limits are constants - // (3) loop is of the form k0 upto Known_not_negative-k inclusive, step <= k - // (4) loop is of the form k0 upto Known_not_negative-k exclusive, step <= k+1 - // (5) loop is of the form Known_not_negative downto k0, minint+step < k0 - if step > 1 { - ok := false - if min.Op == OpConst64 && max.Op == OpConst64 { - if max.AuxInt > min.AuxInt && max.AuxInt%step == min.AuxInt%step { // handle overflow - ok = true - } - } - // 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 + // Check for overflow/underflow. We need to make sure that inc never causes + // the induction variable to wrap around. + // We use a function wrapper here for easy return true / return false / keep going logic. + // This function returns true if the increment will never overflow/underflow. + ok := func() bool { + if step > 0 { + if limit.Op == OpConst64 { + // Figure out the actual largest value. + v := limit.AuxInt + if !inclusive { + if v == math.MinInt64 { + return false // < minint is never satisfiable. } - } else { // decreasing iteration - // 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 + v-- } + 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 { - continue + if ok() { + 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 { - 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) + // 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 } return iv } -func dropAdd64(v *Value) (*Value, int64) { - if v.Op == OpAdd64 && v.Args[0].Op == OpConst64 { - return v.Args[1], v.Args[0].AuxInt +// addWillOverflow reports whether x+y would result in a value more than maxint. +func addWillOverflow(x, y int64) bool { + 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 v.Args[0], v.Args[1].AuxInt + return uint64(x - y) +} + +// 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) { diff --git a/test/fixedbugs/issue53600.go b/test/fixedbugs/issue53600.go index fd3a9e5e47..ead40b57af 100644 --- a/test/fixedbugs/issue53600.go +++ b/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) + } +} diff --git a/test/fixedbugs/issue53600.out b/test/fixedbugs/issue53600.out index 5590c7dcfb..577b50fd2c 100644 --- a/test/fixedbugs/issue53600.out +++ b/test/fixedbugs/issue53600.out @@ -6,3 +6,4 @@ done 9223372036854775805 false 9223372036854775807 false done +9223372036854775807 diff --git a/test/fixedbugs/issue53653.go b/test/fixedbugs/issue53653.go new file mode 100644 index 0000000000..555f7da528 --- /dev/null +++ b/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) + } +} diff --git a/test/fixedbugs/issue53653.out b/test/fixedbugs/issue53653.out new file mode 100644 index 0000000000..f699392cf3 --- /dev/null +++ b/test/fixedbugs/issue53653.out @@ -0,0 +1,8 @@ +-9223372036854775808 false +done +-9223372036854775807 false +-9223372036854775808 false +done +-9223372036854775806 false +-9223372036854775808 false +done diff --git a/test/loopbce.go b/test/loopbce.go index f0c9bd0f81..4ae9a6a630 100644 --- a/test/loopbce.go +++ b/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$" - a[j] = 0 // ERROR "Proved IsInBounds$" + for j := 0; j <= i-1; j++ { + 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 { - // No induction variable is possible because i will overflow a first iteration. + for i := b; i < a; i += z { // ERROR "Induction variable: limits \[-1547,9223372036854774057\), increment 1337" useString("foobar") } }