runtime: multiword objects can be partially live/nonlive #7591
Labels
Milestone
Comments
The liveness bitmaps are already per-word (right?). And in cases like your example, the compiler must know that s.str is not live, since otherwise it would not know that it does not have to store it (right?). So it seems preferable to me to produce more accurate live bitmaps. (It also seems better to not allocate space on the stack that is never set, and it seems better to not load BX at address 0x56 and then do nothing with the value.) |
All correct. The difficulty lies in the fact that the register allocator is doing this never-used-so-don't-store optimization. At regalloc time, the bitmaps are already generated. Regalloc knows nothing about bitmaps, safe points, etc. All that would have to be taught to regalloc. It's not a difficulty, really. Just a lot of work. |
I believe I have a fix for this. But there are other bugs preventing me from proving that it is correct. Labels changed: added release-go1.3. Owner changed to @rsc. Status changed to Accepted. |
|
This issue was closed by revision 6722d45. Status changed to Fixed. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
When we compute liveness for variables in cmd/gc/plive.c, we do it at the variable level. But by the time we get to the assembly level, multi-word variables (strings, slices, probably others) are in multiple memory locations. All memory locations containing a single variable get set to the same state in the liveness bitmaps. But the assembly the compiler generates thinks something different. For example, consider this program: var b = []byte{1,2,3,4,5} func f(n int) int { s := string(b) runtime.GC() return len(s) } Only s.len is live after the GC call, s.str is never used. So the compiler never bothers to store the pointer returned by the string constructor to s.str. At the GC call, s is half-initialized. But both words are marked as live so GC will use s.str as a root. ... 0x0019 00025 (/usr/local/google/home/khr/go/livelen.go:7) MOVQ $0,40(SP) 0x0022 00034 (/usr/local/google/home/khr/go/livelen.go:7) MOVQ $0,48(SP) 0x002b 00043 (/usr/local/google/home/khr/go/livelen.go:7) FUNCDATA $2,"".gcargs·0+0(SB) 0x002b 00043 (/usr/local/google/home/khr/go/livelen.go:7) FUNCDATA $3,"".gclocals·1+0(SB) 0x002b 00043 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ "".b+0(SB),BX 0x0033 00051 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ BX,(SP) 0x0037 00055 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ "".b+8(SB),BX 0x003f 00063 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ BX,8(SP) 0x0044 00068 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ "".b+16(SB),BX 0x004c 00076 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ BX,16(SP) 0x0051 00081 (/usr/local/google/home/khr/go/livelen.go:8) PCDATA $0,$40 0x0051 00081 (/usr/local/google/home/khr/go/livelen.go:8) PCDATA $1,$1 0x0051 00081 (/usr/local/google/home/khr/go/livelen.go:8) CALL ,runtime.slicebytetostring(SB) 0x0056 00086 (/usr/local/google/home/khr/go/livelen.go:8) PCDATA $0,$-1 0x0056 00086 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ 24(SP),BX <- this value never gets stored to "".s+40(SP) 0x005b 00091 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ 32(SP),BX 0x0060 00096 (/usr/local/google/home/khr/go/livelen.go:8) MOVQ BX,"".s+48(SP) 0x0065 00101 (/usr/local/google/home/khr/go/livelen.go:9) PCDATA $0,$0 0x0065 00101 (/usr/local/google/home/khr/go/livelen.go:9) PCDATA $1,$2 0x0065 00101 (/usr/local/google/home/khr/go/livelen.go:9) CALL ,runtime.GC(SB) ... This is currently not breaking the builders because we're over-zeroing on entry at the moment. But if we use liveness info to prune zeroing, we'll be chasing junk pointers during GC. I'm not really sure what the right answer here is. We could force the compiler to store all parts of an object whenever any part is live. Or we could modify the liveness bitmaps to accurately account for what parts are live or not. issue #7564 is related (keeping slice capacity around).The text was updated successfully, but these errors were encountered: