cmd/compile: unusual line numbers associated with generated code #15453
Comments
griesemer
changed the title
|
Happens with Go 1.6 too. |
|
Here's at least my current understanding of the issue. Still investigating a fix. When we inline a function call, we clone the function body and reassign all of the line numbers for the cloned Nodes to the function call site. This is implemented by The problem is For example, in the
|
|
CL https://golang.org/cl/22123 mentions this issue. |
|
Minimized repro case: Compile with The lines with |
builtin.go was auto-generated via go generate; all other changes were manual. The new format reduces the export data size by ~65% on average for the std library packages (and there is still quite a bit of room for improvement). The average time to write export data is reduced by (at least) 62% as measured in one run over the std lib, it is likely more. The average time to read import data is reduced by (at least) 37% as measured in one run over the std lib, it is likely more. There is also room to improve this time. The compiler transparently handles both packages using the old and the new format. Comparing the -S output of the go build for each package via the cmp.bash script (added) shows identical assembly code for all packages, but 6 files show file:line differences: The following files have differences because they use cgo and cgo uses different temp. directories for different builds. Harmless. src/crypto/x509 src/net src/os/user src/runtime/cgo The following files have file:line differences that are not yet fully explained; however the differences exist w/ and w/o new export format (pre-existing condition). See issue golang#15453. src/go/internal/gccgoimporter src/go/internal/gcimporter In summary, switching to the new export format produces the same package files as before for all practical purposes. How can you tell which one you have (if you care): Open a package (.a) file in an editor. Textual export data starts with a $$ after the header and is more or less legible; binary export data starts with a $$B after the header and is mostly unreadable. A stand-alone decoder (for debugging) is in the works. In case of a problem, please first try reverting back to the old textual format to determine if the cause is the new export format: For a stand-alone compiler invocation: - go tool compile -newexport=0 <files> For a single package: - go build -gcflags="-newexport=0" <pkg> For make/all.bash: - (export GO_GCFLAGS="-newexport=0"; sh make.bash) Fixes golang#13241. Change-Id: I2588cb463be80af22446bf80c225e92ab79878b8 Reviewed-on: https://go-review.googlesource.com/22123 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
Elaborating a bit more, the problem is that sinit.go uses the OLITERAL Node's line number when desugaring OSTRUCTLIT Nodes into sequences of OAS Nodes. Two possible fixes I have in mind:
Option 2 seems preferable since it shouldn't negatively impact gofmt'd code, and it still does something reasonable for non-gofmt'd code. |
|
Fix 1) seems the more correct fix, isn't it? We could see how many more nodes are created over the course of make.bash and take it from there. Alternatively, could we do something like |
|
Fix 1 seems most correct to me, I'm just concerned about the compilation overhead. Instrumenting the compiler to count the number of OLITERAL Nodes that may need copying due to inlining, it looks like maybe it would be okay. Compiling net/http would require duplicating an extra 1400 Nodes. Most other packages need closer to the order of 100 to 200. gc requires 7000. Another option (3) is to revert golang.org/cl/11672 (55203c7), which seems to be the CL that introduced the unusual line numbers. |
|
Let's leave for Go 1.8. |
quentinmit
added
the
NeedsFix
|
CL https://golang.org/cl/37232 mentions this issue. |
The assembly listing of go/internal/gcimporter shows some unusual (wrong?) line numbers associated with the generated code. In the code for the method
importer.objwe see references to gcimporter.go, even though there is no code here that should be coming from that file (also no inlining).Here a snippet compiled with -ssa=0:
Here a snippet compiled with ssa enabled:
And here a snippet compiled using the binary export format:
The issue appears independent of ssa and independent of the export format chosen, but it somewhat affected by both. This may be all correct but needs an explanation, or there may be an issue and we should fix it.
The text was updated successfully, but these errors were encountered: