1. 12 Nov, 2018 5 commits
    • Austin Clements's avatar
      debug/gosym: use "go build" instead of hand-running asm and link · 57123654
      Austin Clements authored
      Currently, TestPCLine manually invokes asm and link on its test data.
      Once we introduce symbol ABIs this is going to become problematic
      because the test program defines main.main and main.init in assembly
      so they use ABI0, but the runtime expects to find them with the
      internal ABI.
      
      There are various ways we could solve this. This CL moves main.main
      and main.init into Go code and switches to using "go build" to compile
      and link the test binary. This has the added advantage of simplifying
      this test.
      
      For #27539.
      
      Change-Id: I4c0cf6467f7a39e6b1500eca6ad2620b5ef2b73c
      Reviewed-on: https://go-review.googlesource.com/c/146857
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      57123654
    • Austin Clements's avatar
      runtime: correct ABI information for all functions · af1bfe0a
      Austin Clements authored
      There are three cases where we don't currently have the visibility to
      get the ABIs of runtime symbols right, which this CL fixes:
      
      1. For Go functions referenced from non-Go code in other packages.
         This is runtime.morestackc (which is referenced from function
         prologues) and a few syscall symbols. For these we need to generate
         ABI0 wrappers, so this CL adds dummy calls in the assembly code to
         force wrapper generation. There are many other cross-package
         references to runtime and runtime/internal/atomic, but these are
         handled specially by cmd/go.
      
      2. For calls generated by the compiler to runtime Go functions, there
         are a few symbols that aren't declared in builtins.go because we've
         never needed their type information before. Now we at least need
         their ABI information, so these are added to builtins.go.
      
      3. For calls generated by the compiler to runtime assembly functions,
         the compiler is going to assume the internal ABI is available, so
         we add Go stubs to the runtime to trigger wrapper generation. For
         these we're probably going to want to provide internal ABI
         definitions directly in the assembly for performance, but for now
         the ABIs are the same so it doesn't matter.
      
      For #27539.
      
      Change-Id: I9c224e7408d2ef4dd9b0e4c9d7e962ddfe111245
      Reviewed-on: https://go-review.googlesource.com/c/146822
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Reviewed-by: default avatarMichael Knyszek <mknyszek@google.com>
      af1bfe0a
    • Austin Clements's avatar
      runtime: avoid variable/function alias on runtime._cgo_panic_internal · 6096b85b
      Austin Clements authored
      The symbol runtime._cgo_panic_internal is defined both as a function
      in package runtime and as a (linknamed) variable in package
      runtime/cgo. Since we're introducing function ABIs, this is going to
      cause problems with resolving the ABI-marked function symbol with the
      unmarked data symbol. It's also confusing.
      
      Fix this by declaring runtime._cgo_panic_internal as a function in
      runtime/cgo as well and extracting the PC from the function object.
      
      For #27539.
      
      Change-Id: I148a458a600cf9e57791cf4cbe92e79bddbf58d4
      Reviewed-on: https://go-review.googlesource.com/c/146821
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      6096b85b
    • Austin Clements's avatar
      internal/bytealg, runtime: provide linknames for pushed symbols · ef7ce57a
      Austin Clements authored
      The internal/bytealg package defines several symbols in the runtime,
      bytes, and strings packages in assembly, and the runtime package
      defines symbols in reflect and sync/atomic. Currently, there's no
      corresponding Go prototype for these symbols in the defining package.
      
      We're going to start depending on Go prototypes in the same package as
      their assembly definitions in order to provide ABI wrappers. Plus,
      these are good documentation and colocate type information with
      definitions, which could be useful for vet if it learned a little
      about linkname.
      
      This CL adds linknamed Go prototypes for all pushed symbols in
      internal/bytealg and runtime.
      
      For #27539.
      
      Change-Id: I9b0c12d935a75bb6af46b6761180d451c00f11b8
      Reviewed-on: https://go-review.googlesource.com/c/146820
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Reviewed-by: default avatarMichael Knyszek <mknyszek@google.com>
      ef7ce57a
    • Austin Clements's avatar
      runtime, reflect: access runtime.reflectcall directly · 4f3604d3
      Austin Clements authored
      Currently, package runtime contains the definition of reflect.call,
      even though it's just a jump to runtime.reflectcall. This "push"
      symbol is confusing, since it's not clear where the definition of
      reflect.call comes from when you're in the reflect package.
      
      Replace this with a "pull" symbol: the runtime now defines only
      runtime.reflectcall and package reflect uses a go:linkname to access
      this symbol directly. This makes it clear where reflect.call is coming
      from without any spooky action at a distance and eliminates all of the
      definitions of reflect.call in the runtime.
      
      Change-Id: I3ec73cd394efe9df8d3061a57c73aece2e7048dd
      Reviewed-on: https://go-review.googlesource.com/c/148657
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      4f3604d3
  2. 11 Nov, 2018 4 commits
    • Ainar Garipov's avatar
      go/build: remove superfluous continues · f58b02a2
      Ainar Garipov authored
      This cleanup was proposed in CL 148937. The branch is already ended with
      a continue, so remove continues from subbranches and use an else-if.
      
      Change-Id: Iaf6eb57afc84e25862f99a342f5824e315bcdcb7
      Reviewed-on: https://go-review.googlesource.com/c/148922Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      f58b02a2
    • Michael Anthony Knyszek's avatar
      runtime: gofmt all improperly formatted code · 3a7a56cc
      Michael Anthony Knyszek authored
      This change fixes incorrect formatting in mheap.go (the result of my
      previous heap scavenging changes) and map_test.go.
      
      Change-Id: I2963687504abdc4f0cdf2f0c558174b3bc0ed2df
      Reviewed-on: https://go-review.googlesource.com/c/148977
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      3a7a56cc
    • Josh Bleecher Snyder's avatar
      cmd/compile: optimize A->B->C Moves that include VarDefs · 8607b2e8
      Josh Bleecher Snyder authored
      We have an existing optimization that recognizes
      memory moves of the form A -> B -> C and converts
      them into A -> C, in the hopes that the store to
      B will be end up being dead and thus eliminated.
      
      However, when A, B, and C are large types,
      the front end sometimes emits VarDef ops for the moves.
      This change adds an optimization to match that pattern.
      
      This required changing an old compiler test.
      The test assumed that a temporary was required
      to deal with a large return value.
      With this optimization in place, that temporary
      ended up being eliminated.
      
      Triggers 649 times during 'go build -a std cmd'.
      
      Cuts 16k off cmd/go.
      
      name        old object-bytes  new object-bytes  delta
      Template          507kB ± 0%        507kB ± 0%  -0.15%  (p=0.008 n=5+5)
      Unicode           225kB ± 0%        225kB ± 0%    ~     (all equal)
      GoTypes          1.85MB ± 0%       1.85MB ± 0%    ~     (all equal)
      Flate             328kB ± 0%        328kB ± 0%    ~     (all equal)
      GoParser          402kB ± 0%        402kB ± 0%  -0.00%  (p=0.008 n=5+5)
      Reflect          1.41MB ± 0%       1.41MB ± 0%  -0.20%  (p=0.008 n=5+5)
      Tar               458kB ± 0%        458kB ± 0%    ~     (all equal)
      XML               601kB ± 0%        599kB ± 0%  -0.21%  (p=0.008 n=5+5)
      
      Change-Id: I9b5f25c8663a0b772ad1ee51fa61f74b74d26dd3
      Reviewed-on: https://go-review.googlesource.com/c/143479
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMichael Munday <mike.munday@ibm.com>
      8607b2e8
    • Ainar Garipov's avatar
      go/build, go/doc: fix tautological conditions · f9fff455
      Ainar Garipov authored
      These issues were found by the new vet's nilness check. The variables
      were already checked against nil, so remove extra checks.
      
      Change-Id: Ie252ccfcc755f3d06f691f354bf13d5a623fe17b
      Reviewed-on: https://go-review.googlesource.com/c/148937Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      Run-TryBot: Robert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      f9fff455
  3. 10 Nov, 2018 7 commits
  4. 09 Nov, 2018 16 commits
  5. 08 Nov, 2018 8 commits