cmd/compile: liveness too conservative for variables declared in loop body #7602
Labels
Milestone
Comments
Related to issue #7345. |
|
Owner changed to @rsc. |
It looks okay to me now. I get: x.go:5: live at entry to g: p x.go:10: live at call to GC: x x.go:12: live at call to g: x x.go:10: main: x (type *int) is ambiguously live I don't know if the ambiguously live print was included in the ... in your output or not. If not, that was a bug. But it's there now. In this program, x is not a temporary. Real temporaries are killed aggressively and do not exhibit this behavior (anymore). In particular, the automatic ... arguments in a log.Printf call are killed at the end of the log.Printf statement, before the loop goes around again. I agree that it would be nice to do a little better here. It might be possible with the current analysis: the fact that x is marked EscNone (in particular, not EscHeap) means that it does not escape ouside the loop iteration, so we could emit a varkill for it at the end of the loop body in which it is declared. The order.c pass could push declared variables onto a different stack and then pop+clean them whenever the loopdepth went down. In practice, this seems not to come up very often. There are only 18 ambiguously live non-temps when compiling godoc; it looks like a couple would be addressed by this. A few more are the if statement equivalent, but we don't have a 'scopedepth' analogous to loopdepth so that would be even more work. For such few variables, I think we can punt this to a future release. Labels changed: added release-go1.4, removed release-go1.3. |
rsc
changed the title
|
This doesn't seem to be an issue anymore. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
package main import "runtime" func g(p **int) { } func main() { for i := 0; i < 10; i++ { runtime.GC() var x *int g(&x) } } $ go tool 6g -S -live -l addrinloop.go addrinloop.go:5: live at entry to g: p addrinloop.go:10: live at call to GC: x addrinloop.go:12: live at call to g: x ... That second line is wrong - x should not be considered live at the GC() call. x is not initialized at that point (on the first iteration of the loop, anyway), so it contains junk. That's only false retention in this example, but it's a real bug when x's type is *interface{}. This bug is preventing us from turning down most of the zeroing that's currently causing performance problems. This bug occurs in real world situations - in src/pkg/go/build/build.go:693, the [2]interface{} that is constructed to hold the ... args to log.Panicf behaves like x above. It isn't initialized until just before the log.Printf call, but our liveness analysis thinks it is live for the entire containing loop (lines 597-733).The text was updated successfully, but these errors were encountered: