Skip to content

Commit 0f29942

Browse files
committed
cmd/compile: Repurpose old sliceopt.go for prove phase.
Adapt old test for prove's bounds check elimination. Added missing rule to generic rules that lead to differences between 32 and 64 bit platforms on sliceopt test. Added debugging to prove.go that was helpful-to-necessary to discover that missing rule. Lowered debugging level on prove.go from 3 to 1; no idea why it was previously 3. Change-Id: I09de206aeb2fced9f2796efe2bfd4a59927eda0c Reviewed-on: https://go-review.googlesource.com/23290 Run-TryBot: David Chase <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Keith Randall <[email protected]>
1 parent c9517b1 commit 0f29942

File tree

6 files changed

+167
-15
lines changed

6 files changed

+167
-15
lines changed

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

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -670,9 +670,18 @@ func (s *state) stmt(n *Node) {
670670
// If the slice can be SSA'd, it'll be on the stack,
671671
// so there will be no write barriers,
672672
// so there's no need to attempt to prevent them.
673-
if samesafeexpr(n.Left, rhs.List.First()) && !s.canSSA(n.Left) {
674-
s.append(rhs, true)
675-
return
673+
if samesafeexpr(n.Left, rhs.List.First()) {
674+
if !s.canSSA(n.Left) {
675+
if Debug_append > 0 {
676+
Warnl(n.Lineno, "append: len-only update")
677+
}
678+
s.append(rhs, true)
679+
return
680+
} else {
681+
if Debug_append > 0 { // replicating old diagnostic message
682+
Warnl(n.Lineno, "append: len-only update (in local slice)")
683+
}
684+
}
676685
}
677686
}
678687
}

src/cmd/compile/internal/ssa/gen/generic.rules

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,8 @@
749749
// a more comprehensive set.
750750
(SliceLen (SliceMake _ (Const64 <t> [c]) _)) -> (Const64 <t> [c])
751751
(SliceCap (SliceMake _ _ (Const64 <t> [c]))) -> (Const64 <t> [c])
752+
(SliceLen (SliceMake _ (Const32 <t> [c]) _)) -> (Const32 <t> [c])
753+
(SliceCap (SliceMake _ _ (Const32 <t> [c]))) -> (Const32 <t> [c])
752754
(SlicePtr (SliceMake (SlicePtr x) _ _)) -> (SlicePtr x)
753755
(SliceLen (SliceMake _ (SliceLen x) _)) -> (SliceLen x)
754756
(SliceCap (SliceMake _ _ (SliceCap x))) -> (SliceCap x)

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

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44

55
package ssa
66

7-
import "math"
7+
import (
8+
"fmt"
9+
"math"
10+
)
811

912
type branch int
1013

@@ -74,6 +77,10 @@ type limit struct {
7477
umin, umax uint64 // umin <= value <= umax, unsigned
7578
}
7679

80+
func (l limit) String() string {
81+
return fmt.Sprintf("sm,SM,um,UM=%d,%d,%d,%d", l.min, l.max, l.umin, l.umax)
82+
}
83+
7784
var noLimit = limit{math.MinInt64, math.MaxInt64, 0, math.MaxUint64}
7885

7986
// a limitFact is a limit known for a particular value.
@@ -191,7 +198,7 @@ func (ft *factsTable) get(v, w *Value, d domain) relation {
191198

192199
// update updates the set of relations between v and w in domain d
193200
// restricting it to r.
194-
func (ft *factsTable) update(v, w *Value, d domain, r relation) {
201+
func (ft *factsTable) update(parent *Block, v, w *Value, d domain, r relation) {
195202
if lessByID(w, v) {
196203
v, w = w, v
197204
r = reverseBits[r]
@@ -293,6 +300,9 @@ func (ft *factsTable) update(v, w *Value, d domain, r relation) {
293300
}
294301
ft.limitStack = append(ft.limitStack, limitFact{v.ID, old})
295302
ft.limits[v.ID] = lim
303+
if v.Block.Func.pass.debug > 2 {
304+
v.Block.Func.Config.Warnl(parent.Line, "parent=%s, new limits %s %s %s", parent, v, w, lim.String())
305+
}
296306
}
297307
}
298308

@@ -478,11 +488,11 @@ func prove(f *Func) {
478488
if branch != unknown {
479489
ft.checkpoint()
480490
c := parent.Control
481-
updateRestrictions(ft, boolean, nil, c, lt|gt, branch)
491+
updateRestrictions(parent, ft, boolean, nil, c, lt|gt, branch)
482492
if tr, has := domainRelationTable[parent.Control.Op]; has {
483493
// When we branched from parent we learned a new set of
484494
// restrictions. Update the factsTable accordingly.
485-
updateRestrictions(ft, tr.d, c.Args[0], c.Args[1], tr.r, branch)
495+
updateRestrictions(parent, ft, tr.d, c.Args[0], c.Args[1], tr.r, branch)
486496
}
487497
}
488498

@@ -538,7 +548,7 @@ func getBranch(sdom SparseTree, p *Block, b *Block) branch {
538548

539549
// updateRestrictions updates restrictions from the immediate
540550
// dominating block (p) using r. r is adjusted according to the branch taken.
541-
func updateRestrictions(ft *factsTable, t domain, v, w *Value, r relation, branch branch) {
551+
func updateRestrictions(parent *Block, ft *factsTable, t domain, v, w *Value, r relation, branch branch) {
542552
if t == 0 || branch == unknown {
543553
// Trivial case: nothing to do, or branch unknown.
544554
// Shoult not happen, but just in case.
@@ -550,7 +560,7 @@ func updateRestrictions(ft *factsTable, t domain, v, w *Value, r relation, branc
550560
}
551561
for i := domain(1); i <= t; i <<= 1 {
552562
if t&i != 0 {
553-
ft.update(v, w, i, r)
563+
ft.update(parent, v, w, i, r)
554564
}
555565
}
556566
}
@@ -566,13 +576,21 @@ func simplifyBlock(ft *factsTable, b *Block) branch {
566576
m := ft.get(nil, b.Control, boolean)
567577
if m == lt|gt {
568578
if b.Func.pass.debug > 0 {
569-
b.Func.Config.Warnl(b.Line, "Proved boolean %s", b.Control.Op)
579+
if b.Func.pass.debug > 1 {
580+
b.Func.Config.Warnl(b.Line, "Proved boolean %s (%s)", b.Control.Op, b.Control)
581+
} else {
582+
b.Func.Config.Warnl(b.Line, "Proved boolean %s", b.Control.Op)
583+
}
570584
}
571585
return positive
572586
}
573587
if m == eq {
574588
if b.Func.pass.debug > 0 {
575-
b.Func.Config.Warnl(b.Line, "Disproved boolean %s", b.Control.Op)
589+
if b.Func.pass.debug > 1 {
590+
b.Func.Config.Warnl(b.Line, "Disproved boolean %s (%s)", b.Control.Op, b.Control)
591+
} else {
592+
b.Func.Config.Warnl(b.Line, "Disproved boolean %s", b.Control.Op)
593+
}
576594
}
577595
return negative
578596
}
@@ -599,13 +617,21 @@ func simplifyBlock(ft *factsTable, b *Block) branch {
599617
m := ft.get(a0, a1, d)
600618
if m != 0 && tr.r&m == m {
601619
if b.Func.pass.debug > 0 {
602-
b.Func.Config.Warnl(b.Line, "Proved %s", c.Op)
620+
if b.Func.pass.debug > 1 {
621+
b.Func.Config.Warnl(b.Line, "Proved %s (%s)", c.Op, c)
622+
} else {
623+
b.Func.Config.Warnl(b.Line, "Proved %s", c.Op)
624+
}
603625
}
604626
return positive
605627
}
606628
if m != 0 && ((lt|eq|gt)^tr.r)&m == m {
607629
if b.Func.pass.debug > 0 {
608-
b.Func.Config.Warnl(b.Line, "Disproved %s", c.Op)
630+
if b.Func.pass.debug > 1 {
631+
b.Func.Config.Warnl(b.Line, "Disproved %s (%s)", c.Op, c)
632+
} else {
633+
b.Func.Config.Warnl(b.Line, "Disproved %s", c.Op)
634+
}
609635
}
610636
return negative
611637
}
@@ -620,7 +646,11 @@ func simplifyBlock(ft *factsTable, b *Block) branch {
620646
m := ft.get(a0, a1, signed)
621647
if m != 0 && tr.r&m == m {
622648
if b.Func.pass.debug > 0 {
623-
b.Func.Config.Warnl(b.Line, "Proved non-negative bounds %s", c.Op)
649+
if b.Func.pass.debug > 1 {
650+
b.Func.Config.Warnl(b.Line, "Proved non-negative bounds %s (%s)", c.Op, c)
651+
} else {
652+
b.Func.Config.Warnl(b.Line, "Proved non-negative bounds %s", c.Op)
653+
}
624654
}
625655
return positive
626656
}
@@ -635,6 +665,9 @@ func isNonNegative(v *Value) bool {
635665
case OpConst64:
636666
return v.AuxInt >= 0
637667

668+
case OpConst32:
669+
return int32(v.AuxInt) >= 0
670+
638671
case OpStringLen, OpSliceLen, OpSliceCap,
639672
OpZeroExt8to64, OpZeroExt16to64, OpZeroExt32to64:
640673
return true

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9656,6 +9656,25 @@ func rewriteValuegeneric_OpSliceCap(v *Value, config *Config) bool {
96569656
v.AuxInt = c
96579657
return true
96589658
}
9659+
// match: (SliceCap (SliceMake _ _ (Const32 <t> [c])))
9660+
// cond:
9661+
// result: (Const32 <t> [c])
9662+
for {
9663+
v_0 := v.Args[0]
9664+
if v_0.Op != OpSliceMake {
9665+
break
9666+
}
9667+
v_0_2 := v_0.Args[2]
9668+
if v_0_2.Op != OpConst32 {
9669+
break
9670+
}
9671+
t := v_0_2.Type
9672+
c := v_0_2.AuxInt
9673+
v.reset(OpConst32)
9674+
v.Type = t
9675+
v.AuxInt = c
9676+
return true
9677+
}
96599678
// match: (SliceCap (SliceMake _ _ (SliceCap x)))
96609679
// cond:
96619680
// result: (SliceCap x)
@@ -9714,6 +9733,25 @@ func rewriteValuegeneric_OpSliceLen(v *Value, config *Config) bool {
97149733
v.AuxInt = c
97159734
return true
97169735
}
9736+
// match: (SliceLen (SliceMake _ (Const32 <t> [c]) _))
9737+
// cond:
9738+
// result: (Const32 <t> [c])
9739+
for {
9740+
v_0 := v.Args[0]
9741+
if v_0.Op != OpSliceMake {
9742+
break
9743+
}
9744+
v_0_1 := v_0.Args[1]
9745+
if v_0_1.Op != OpConst32 {
9746+
break
9747+
}
9748+
t := v_0_1.Type
9749+
c := v_0_1.AuxInt
9750+
v.reset(OpConst32)
9751+
v.Type = t
9752+
v.AuxInt = c
9753+
return true
9754+
}
97179755
// match: (SliceLen (SliceMake _ (SliceLen x) _))
97189756
// cond:
97199757
// result: (SliceLen x)

test/prove.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// +build amd64
2-
// errorcheck -0 -d=ssa/prove/debug=3
2+
// errorcheck -0 -d=ssa/prove/debug=1
33

44
// Copyright 2016 The Go Authors. All rights reserved.
55
// Use of this source code is governed by a BSD-style

test/sliceopt.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// errorcheck -0 -d=append,slice,ssa/prove/debug=1
2+
3+
// Copyright 2015 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
// Check optimization results for append and slicing.
8+
9+
package main
10+
11+
func a1(x []int, y int) []int {
12+
x = append(x, y) // ERROR "append: len-only update \(in local slice\)$"
13+
return x
14+
}
15+
16+
func a2(x []int, y int) []int {
17+
return append(x, y)
18+
}
19+
20+
func a3(x *[]int, y int) {
21+
*x = append(*x, y) // ERROR "append: len-only update$"
22+
}
23+
24+
// s1_if_false_then_anything
25+
func s1_if_false_then_anything(x **[]int, xs **string, i, j int) {
26+
z := (**x)[0:i]
27+
z = z[i : i+1]
28+
println(z) // if we get here, then we have proven that i==i+1 (this cannot happen, but the program is still being analyzed...)
29+
30+
zs := (**xs)[0:i] // since i=i+1 is proven, i+1 is "in bounds", ha-ha
31+
zs = zs[i : i+1] // ERROR "Proved boolean IsSliceInBounds$"
32+
println(zs)
33+
}
34+
35+
func s1(x **[]int, xs **string, i, j int) {
36+
var z []int
37+
z = (**x)[2:]
38+
z = (**x)[2:len(**x)] // ERROR "Proved boolean IsSliceInBounds$"
39+
z = (**x)[2:cap(**x)] // ERROR "Proved IsSliceInBounds$"
40+
z = (**x)[i:i] // -ERROR "Proved IsSliceInBounds"
41+
z = (**x)[1:i:i] // ERROR "Proved boolean IsSliceInBounds$"
42+
z = (**x)[i:j:0]
43+
z = (**x)[i:0:j] // ERROR "Disproved IsSliceInBounds$"
44+
z = (**x)[0:i:j] // ERROR "Proved boolean IsSliceInBounds$"
45+
z = (**x)[0:] // ERROR "slice: omit slice operation$"
46+
z = (**x)[2:8] // ERROR "Disproved Eq(32|64)$"
47+
z = (**x)[2:2] // ERROR "Disproved Eq(32|64)$" "Proved boolean IsSliceInBounds$"
48+
z = (**x)[0:i] // ERROR "Proved boolean IsSliceInBounds$"
49+
z = (**x)[2:i:8] // ERROR "Disproved IsSliceInBounds$" "Proved IsSliceInBounds$" "Proved boolean IsSliceInBounds$"
50+
z = (**x)[i:2:i] // ERROR "Proved IsSliceInBounds$" "Proved boolean IsSliceInBounds$"
51+
52+
z = z[0:i] // ERROR "Proved boolean IsSliceInBounds"
53+
z = z[0:i : i+1]
54+
z = z[i : i+1] // ERROR "Proved boolean IsSliceInBounds$"
55+
56+
println(z)
57+
58+
var zs string
59+
zs = (**xs)[2:]
60+
zs = (**xs)[2:len(**xs)] // ERROR "Proved IsSliceInBounds$" "Proved boolean IsSliceInBounds$"
61+
zs = (**xs)[i:i] // -ERROR "Proved boolean IsSliceInBounds"
62+
zs = (**xs)[0:] // ERROR "slice: omit slice operation$"
63+
zs = (**xs)[2:8]
64+
zs = (**xs)[2:2] // ERROR "Proved boolean IsSliceInBounds$"
65+
zs = (**xs)[0:i] // ERROR "Proved boolean IsSliceInBounds$"
66+
67+
zs = zs[0:i] // See s1_if_false_then_anything above to explain the counterfactual bounds check result below
68+
zs = zs[i : i+1] // ERROR "Proved boolean IsSliceInBounds$"
69+
println(zs)
70+
}

0 commit comments

Comments
 (0)