Skip to content

Commit 8af3224

Browse files
committed
cmd/compile: don't evaluate side effects of range over array
If the thing we're ranging over is an array or ptr to array, and it doesn't have a function call or channel receive in it, then we shouldn't evaluate it. Typecheck the ranged-over value as a constant in that case. That makes the unified exporter replace the range expression with a constant int. Change-Id: I0d4ea081de70d20cf6d1fa8d25ef6cb021975554 Reviewed-on: https://go-review.googlesource.com/c/go/+/659317 Reviewed-by: Junyang Shao <shaojunyang@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Griesemer <gri@google.com>
1 parent 04a9b16 commit 8af3224

File tree

5 files changed

+94
-7
lines changed

5 files changed

+94
-7
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,12 @@ func nilcheckelim2(f *Func) {
217217
f.Warnl(v.Pos, "removed nil check")
218218
}
219219
// For bug 33724, policy is that we might choose to bump an existing position
220-
// off the faulting load/store in favor of the one from the nil check.
220+
// off the faulting load in favor of the one from the nil check.
221221

222222
// Iteration order means that first nilcheck in the chain wins, others
223223
// are bumped into the ordinary statement preservation algorithm.
224224
u := b.Values[unnecessary.get(v.Args[0].ID)]
225-
if !u.Pos.SameFileAndLine(v.Pos) {
225+
if !u.Type.IsMemory() && !u.Pos.SameFileAndLine(v.Pos) {
226226
if u.Pos.IsStmt() == src.PosIsStmt {
227227
pendingLines.add(u.Pos)
228228
}

src/cmd/compile/internal/types2/range.go

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package types2
88

99
import (
1010
"cmd/compile/internal/syntax"
11+
"go/constant"
1112
"internal/buildcfg"
1213
. "internal/types/errors"
1314
)
@@ -23,8 +24,32 @@ import (
2324
func (check *Checker) rangeStmt(inner stmtContext, rangeStmt *syntax.ForStmt, noNewVarPos poser, sKey, sValue, sExtra, rangeVar syntax.Expr, isDef bool) {
2425
// check expression to iterate over
2526
var x operand
27+
28+
// From the spec:
29+
// The range expression x is evaluated before beginning the loop,
30+
// with one exception: if at most one iteration variable is present
31+
// and x or len(x) is constant, the range expression is not evaluated.
32+
// So we have to be careful not to evaluate the arg in the
33+
// described situation.
34+
35+
check.hasCallOrRecv = false
2636
check.expr(nil, &x, rangeVar)
2737

38+
if isTypes2 && x.mode != invalid && sValue == nil && !check.hasCallOrRecv {
39+
if t, ok := arrayPtrDeref(under(x.typ)).(*Array); ok {
40+
// Override type of rangeVar to be a constant
41+
// (and thus side-effects will not be computed
42+
// by the backend).
43+
check.record(&operand{
44+
mode: constant_,
45+
expr: rangeVar,
46+
typ: Typ[Int],
47+
val: constant.MakeInt64(t.len),
48+
id: x.id,
49+
})
50+
}
51+
}
52+
2853
// determine key/value types
2954
var key, val Type
3055
if x.mode != invalid {

src/cmd/compile/internal/walk/range.go

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package walk
66

77
import (
8+
"go/constant"
89
"internal/buildcfg"
910
"unicode/utf8"
1011

@@ -80,6 +81,10 @@ func walkRange(nrange *ir.RangeStmt) ir.Node {
8081
base.Fatalf("walkRange")
8182

8283
case types.IsInt[k]:
84+
if nn := arrayRangeClear(nrange, v1, v2, a); nn != nil {
85+
base.Pos = lno
86+
return nn
87+
}
8388
hv1 := typecheck.TempAt(base.Pos, ir.CurFunc, t)
8489
hn := typecheck.TempAt(base.Pos, ir.CurFunc, t)
8590

@@ -519,21 +524,41 @@ func arrayRangeClear(loop *ir.RangeStmt, v1, v2, a ir.Node) ir.Node {
519524
}
520525
lhs := stmt.X.(*ir.IndexExpr)
521526
x := lhs.X
522-
if a.Type().IsPtr() && a.Type().Elem().IsArray() {
523-
if s, ok := x.(*ir.StarExpr); ok && s.Op() == ir.ODEREF {
524-
x = s.X
527+
528+
// Get constant number of iterations for int and array cases.
529+
n := int64(-1)
530+
if ir.IsConst(a, constant.Int) {
531+
n = ir.Int64Val(a)
532+
} else if a.Type().IsArray() {
533+
n = a.Type().NumElem()
534+
} else if a.Type().IsPtr() && a.Type().Elem().IsArray() {
535+
n = a.Type().Elem().NumElem()
536+
}
537+
538+
if n >= 0 {
539+
// Int/Array case.
540+
if !x.Type().IsArray() {
541+
return nil
542+
}
543+
if x.Type().NumElem() != n {
544+
return nil
545+
}
546+
} else {
547+
// Slice case.
548+
if !ir.SameSafeExpr(x, a) {
549+
return nil
525550
}
526551
}
527552

528-
if !ir.SameSafeExpr(x, a) || !ir.SameSafeExpr(lhs.Index, v1) {
553+
if !ir.SameSafeExpr(lhs.Index, v1) {
529554
return nil
530555
}
531556

532557
if !ir.IsZero(stmt.Y) {
533558
return nil
534559
}
535560

536-
return arrayClear(stmt.Pos(), a, loop)
561+
return arrayClear(stmt.Pos(), x, loop)
537562
}
538563

539564
// arrayClear constructs a call to runtime.memclr for fast zeroing of slices and arrays.

src/go/types/range.go

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/codegen/issue52635.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ package codegen
1212
type T struct {
1313
a *[10]int
1414
b [10]int
15+
s []int
1516
}
1617

1718
func (t *T) f() {
@@ -38,4 +39,15 @@ func (t *T) f() {
3839
for i := range *t.a {
3940
(*t.a)[i] = 0
4041
}
42+
43+
// amd64:-".*runtime.memclrNoHeapPointers"
44+
// amd64:"DUFFZERO"
45+
for i := range t.b {
46+
t.b[i] = 0
47+
}
48+
49+
// amd64:".*runtime.memclrNoHeapPointers"
50+
for i := range t.s {
51+
t.s[i] = 0
52+
}
4153
}

0 commit comments

Comments
 (0)