aboutsummaryrefslogtreecommitdiff
path: root/test/sliceopt.go
diff options
context:
space:
mode:
authorAustin Clements <austin@google.com>2018-01-10 16:28:58 -0500
committerAustin Clements <austin@google.com>2018-03-08 22:25:25 +0000
commit669db2cef55321b0fe354b8bf9212245dc9c6aed (patch)
tree348588330befc30190b6df63a172752df3ed0eb8 /test/sliceopt.go
parent2e9cf5f66e4dccbb9676ebbabd7d36db4f2825a1 (diff)
downloadgo-669db2cef55321b0fe354b8bf9212245dc9c6aed.tar.gz
go-669db2cef55321b0fe354b8bf9212245dc9c6aed.zip
cmd/compile: make prove pass use unsatisfiability
Currently the prove pass uses implication queries. For each block, it collects the set of branch conditions leading to that block, and queries this fact table for whether any of these facts imply the block's own branch condition (or its inverse). This works remarkably well considering it doesn't do any deduction on these facts, but it has various downsides: 1. It requires an implementation both of adding facts to the table and determining implications. These are very nearly duals of each other, but require separate implementations. Likewise, the process of asserting facts of dominating branch conditions is very nearly the dual of the process of querying implied branch conditions. 2. It leads to less effective use of derived facts. For example, the prove pass currently derives facts about the relations between len and cap, but can't make use of these unless a branch condition is in the exact form of a derived fact. If one of these derived facts contradicts another fact, it won't notice or make use of this. This CL changes the approach of the prove pass to instead use *contradiction* instead of implication. Rather than ever querying a branch condition, it simply adds branch conditions to the fact table. If this leads to a contradiction (specifically, it makes the fact set unsatisfiable), that branch is impossible and can be cut. As a result, 1. We can eliminate the code for determining implications (factsTable.get disappears entirely). Also, there is now a single implementation of visiting and asserting branch conditions, since we don't have to flip them around to treat them as facts in one place and queries in another. 2. Derived facts can be used effectively. It doesn't matter *why* the fact table is unsatisfiable; a contradiction in any of the facts is enough. 3. As an added benefit, it's now quite easy to avoid traversing beyond provably-unreachable blocks. In contrast, the current implementation always visits all blocks. The prove pass already has nearly all of the mechanism necessary to compute unsatisfiability, which means this both simplifies the code and makes it more powerful. The only complication is that the current implication procedure has a hack for dealing with the 0 <= Args[0] condition of OpIsInBounds and OpIsSliceInBounds. We replace this with asserting the appropriate fact when we process one of these conditions. This seems much cleaner anyway, and works because we can now take advantage of derived facts. This has no measurable effect on compiler performance. Effectiveness: There is exactly one condition in all of std and cmd that this fails to prove that the old implementation could: (int64(^uint(0)>>1) < x) in encoding/gob. This can never be true because x is an int, and it's basically coincidence that the old code gets this. (For example, it fails to prove the similar (x < ^int64(^uint(0)>>1)) condition that immediately precedes it, and even though the conditions are logically unrelated, it wouldn't get the second one if it hadn't first processed the first!) It does, however, prove a few dozen additional branches. These come from facts that are added to the fact table about the relations between len and cap. These were almost never queried directly before, but could lead to contradictions, which the unsat-based approach is able to use. There are exactly two branches in std and cmd that this implementation proves in the *other* direction. This sounds scary, but is okay because both occur in already-unreachable blocks, so it doesn't matter what we chose. Because the fact table logic is sound but incomplete, it fails to prove that the block isn't reachable, even though it is able to prove that both outgoing branches are impossible. We could turn these blocks into BlockExit blocks, but it doesn't seem worth the trouble of the extra proof effort for something that happens twice in all of std and cmd. Tests: This CL updates test/prove.go to change the expected messages because it can no longer give a "reason" why it proved or disproved a condition. It also adds a new test of a branch it couldn't prove before. It mostly guts test/sliceopt.go, removing everything related to slice bounds optimizations and moving a few relevant tests to test/prove.go. Much of this test is actually unreachable. The new prove pass figures this out and doesn't try to prove anything about the unreachable parts. The output on the unreachable parts is already suspect because anything can be proved at that point, so it's really just a regression test for an algorithm the compiler no longer uses. This is a step toward fixing #23354. That issue is quite easy to fix once we can use derived facts effectively. Change-Id: Ia48a1b9ee081310579fe474e4a61857424ff8ce8 Reviewed-on: https://go-review.googlesource.com/87478 Reviewed-by: Keith Randall <khr@golang.org>
Diffstat (limited to 'test/sliceopt.go')
-rw-r--r--test/sliceopt.go45
1 files changed, 3 insertions, 42 deletions
diff --git a/test/sliceopt.go b/test/sliceopt.go
index eb24701f31..b8b947229c 100644
--- a/test/sliceopt.go
+++ b/test/sliceopt.go
@@ -1,4 +1,4 @@
-// errorcheck -0 -d=append,slice,ssa/prove/debug=1
+// errorcheck -0 -d=append,slice
// Copyright 2015 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
@@ -21,51 +21,12 @@ func a3(x *[]int, y int) {
*x = append(*x, y) // ERROR "append: len-only update$"
}
-// s1_if_false_then_anything
-func s1_if_false_then_anything(x **[]int, xs **string, i, j int) {
- z := (**x)[0:i]
- z = z[i : i+1]
- println(z) // if we get here, then we have proven that i==i+1 (this cannot happen, but the program is still being analyzed...)
-
- zs := (**xs)[0:i] // since i=i+1 is proven, i+1 is "in bounds", ha-ha
- zs = zs[i : i+1] // ERROR "Proved boolean IsSliceInBounds$"
- println(zs)
-}
-
func s1(x **[]int, xs **string, i, j int) {
var z []int
- z = (**x)[2:]
- z = (**x)[2:len(**x)] // ERROR "Proved boolean IsSliceInBounds$"
- z = (**x)[2:cap(**x)] // ERROR "Proved IsSliceInBounds$"
- z = (**x)[i:i] // -ERROR "Proved IsSliceInBounds"
- z = (**x)[1:i:i] // ERROR "Proved boolean IsSliceInBounds$"
- z = (**x)[i:j:0]
- z = (**x)[i:0:j] // ERROR "Disproved IsSliceInBounds$"
- z = (**x)[0:i:j] // ERROR "Proved boolean IsSliceInBounds$"
- z = (**x)[0:] // ERROR "slice: omit slice operation$"
- z = (**x)[2:8] // ERROR "Proved slicemask not needed$"
- println(z)
- z = (**x)[2:2]
- z = (**x)[0:i]
- z = (**x)[2:i:8] // ERROR "Disproved IsSliceInBounds$" "Proved IsSliceInBounds$"
- z = (**x)[i:2:i] // ERROR "Proved IsSliceInBounds$" "Proved boolean IsSliceInBounds$"
-
- z = z[0:i] // ERROR "Proved boolean IsSliceInBounds"
- z = z[0:i : i+1]
- z = z[i : i+1] // ERROR "Proved boolean IsSliceInBounds$"
-
+ z = (**x)[0:] // ERROR "slice: omit slice operation$"
println(z)
var zs string
- zs = (**xs)[2:]
- zs = (**xs)[2:len(**xs)] // ERROR "Proved IsSliceInBounds$" "Proved boolean IsSliceInBounds$"
- zs = (**xs)[i:i] // -ERROR "Proved boolean IsSliceInBounds"
- zs = (**xs)[0:] // ERROR "slice: omit slice operation$"
- zs = (**xs)[2:8]
- zs = (**xs)[2:2] // ERROR "Proved boolean IsSliceInBounds$"
- zs = (**xs)[0:i] // ERROR "Proved boolean IsSliceInBounds$"
-
- zs = zs[0:i] // See s1_if_false_then_anything above to explain the counterfactual bounds check result below
- zs = zs[i : i+1] // ERROR "Proved boolean IsSliceInBounds$"
+ zs = (**xs)[0:] // ERROR "slice: omit slice operation$"
println(zs)
}