An error occurred fetching the project authors.
  1. 16 Aug, 2016 2 commits
  2. 27 May, 2016 2 commits
    • Russ Cox's avatar
      cmd/compile: eliminate PPARAMREF · 20803b84
      Russ Cox authored
      As in the elimination of PHEAP|PPARAM in CL 23393,
      this is something the front end can trivially take care of
      and then not bother the back ends with.
      It also eliminates some suspect (and only lightly exercised)
      code paths in the back ends.
      
      I don't have a smoking gun for this one but it seems
      more clearly correct.
      
      Change-Id: I3b3f5e669b3b81d091ff1e2fb13226a6f14c69d5
      Reviewed-on: https://go-review.googlesource.com/23431Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Russ Cox <rsc@golang.org>
      20803b84
    • Russ Cox's avatar
      cmd/compile: fix liveness computation for heap-escaped parameters · b6dc3e6f
      Russ Cox authored
      The liveness computation of parameters generally was never
      correct, but forcing all parameters to be live throughout the
      function covered up that problem. The new SSA back end is
      too clever: even though it currently keeps the parameter values live
      throughout the function, it may find optimizations that mean
      the current values are not written back to the original parameter
      stack slots immediately or ever (for example if a parameter is set
      to nil, SSA constant propagation may replace all later uses of the
      parameter with a constant nil, eliminating the need to write the nil
      value back to the stack slot), so the liveness code must now
      track the actual operations on the stack slots, exposing these
      problems.
      
      One small problem in the handling of arguments is that nodarg
      can return ONAME PPARAM nodes with adjusted offsets, so that
      there are actually multiple *Node pointers for the same parameter
      in the instruction stream. This might be possible to correct, but
      not in this CL. For now, we fix this by using n.Orig instead of n
      when considering PPARAM and PPARAMOUT nodes.
      
      The major problem in the handling of arguments is general
      confusion in the liveness code about the meaning of PPARAM|PHEAP
      and PPARAMOUT|PHEAP nodes, especially as contrasted with PAUTO|PHEAP.
      The difference between these two is that when a local variable "moves"
      to the heap, it's really just allocated there to start with; in contrast,
      when an argument moves to the heap, the actual data has to be copied
      there from the stack at the beginning of the function, and when a
      result "moves" to the heap the value in the heap has to be copied
      back to the stack when the function returns
      This general confusion is also present in the SSA back end.
      
      The PHEAP bit worked decently when I first introduced it 7 years ago (!)
      in 391425ae. The back end did nothing sophisticated, and in particular
      there was no analysis at all: no escape analysis, no liveness analysis,
      and certainly no SSA back end. But the complications caused in the
      various downstream consumers suggest that this should be a detail
      kept mainly in the front end.
      
      This CL therefore eliminates both the PHEAP bit and even the idea of
      "heap variables" from the back ends.
      
      First, it replaces the PPARAM|PHEAP, PPARAMOUT|PHEAP, and PAUTO|PHEAP
      variable classes with the single PAUTOHEAP, a pseudo-class indicating
      a variable maintained on the heap and available by indirecting a
      local variable kept on the stack (a plain PAUTO).
      
      Second, walkexpr replaces all references to PAUTOHEAP variables
      with indirections of the corresponding PAUTO variable.
      The back ends and the liveness code now just see plain indirected
      variables. This may actually produce better code, but the real goal
      here is to eliminate these little-used and somewhat suspect code
      paths in the back end analyses.
      
      The OPARAM node type goes away too.
      
      A followup CL will do the same to PPARAMREF. I'm not sure that
      the back ends (SSA in particular) are handling those right either,
      and with the framework established in this CL that change is trivial
      and the result clearly more correct.
      
      Fixes #15747.
      
      Change-Id: I2770b1ce3cbc93981bfc7166be66a9da12013d74
      Reviewed-on: https://go-review.googlesource.com/23393Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Run-TryBot: Russ Cox <rsc@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      b6dc3e6f
  3. 25 Apr, 2016 1 commit
    • Josh Bleecher Snyder's avatar
      cmd/compile: encapsulate OSLICE* representation · f12bd8a5
      Josh Bleecher Snyder authored
      As a nice side-effect, this allows us to
      unify several code paths.
      
      The terminology (low, high, max, simple slice expr,
      full slice expr) is taken from the spec and
      the examples in the spec.
      
      This is a trial run. The plan, probably for Go 1.8,
      is to change slice expressions to use Node.List
      instead of OKEY, and to do some similar
      tree structure changes for other ops.
      
      Passes toolstash -cmp. No performance change.
      all.bash passes with GO_GCFLAGS=-newexport.
      
      Updates #15350
      
      Change-Id: Ic1efdc36e79cdb95ae1636e9817a3ac8f83ab1ac
      Reviewed-on: https://go-review.googlesource.com/22425Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      f12bd8a5
  4. 24 Apr, 2016 2 commits
    • Josh Bleecher Snyder's avatar
      cmd/compile: give gc.Op a String method, use it · f0272414
      Josh Bleecher Snyder authored
      Passes toolstash -cmp.
      
      Change-Id: I915e76374fd64aa2597e6fa47e4fa95ca00ca643
      Reviewed-on: https://go-review.googlesource.com/22380
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarDavid Crawshaw <crawshaw@golang.org>
      f0272414
    • Keith Randall's avatar
      cmd/compile: reorder how slicelit initializes a slice · 934c3599
      Keith Randall authored
        func f(x, y, z *int) {
          a := []*int{x,y,z}
          ...
        }
      
      We used to use:
        var tmp [3]*int
        a := tmp[:]
        a[0] = x
        a[1] = y
        a[2] = z
      
      Now we do:
        var tmp [3]*int
        tmp[0] = x
        tmp[1] = y
        tmp[2] = z
        a := tmp[:]
      
      Doesn't sound like a big deal, but the compiler has trouble
      eliminating write barriers when using the former method because it
      doesn't know that the slice points to the stack.  In the latter
      method, the compiler knows the array is on the stack and as a result
      doesn't emit any write barriers.
      
      This turns out to be extremely common when building ... args, like
      for calls fmt.Printf.
      
      Makes go binaries ~1% smaller.
      
      Doesn't have a measurable effect on the go1 fmt benchmarks,
      unfortunately.
      
      Fixes #14263
      Update #6853
      
      Change-Id: I9074a2788ec9e561a75f3b71c119b69f304d6ba2
      Reviewed-on: https://go-review.googlesource.com/22395
      Run-TryBot: Keith Randall <khr@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJosh Bleecher Snyder <josharian@gmail.com>
      934c3599
  5. 22 Apr, 2016 2 commits
  6. 21 Apr, 2016 1 commit
  7. 19 Apr, 2016 1 commit
    • Josh Bleecher Snyder's avatar
      cmd/compile: fix isStaticCompositeLiteral · 3c6e60c0
      Josh Bleecher Snyder authored
      Previously, isStaticCompositeLiteral would
      return the wrong value for literals like:
      
      [1]struct{ b []byte }{b: []byte{1}}
      
      Note that the outermost component is an array,
      but once we recurse into isStaticCompositeLiteral,
      we never check again that arrays are actually arrays.
      
      Instead of adding more logic to the guts of
      isStaticCompositeLiteral, allow it to accept
      any Node and return the correct answer.
      
      Change-Id: I6af7814a9037bbc7043da9a96137fbee067bbe0e
      Reviewed-on: https://go-review.googlesource.com/22247Reviewed-by: default avatarKeith Randall <khr@golang.org>
      3c6e60c0
  8. 18 Apr, 2016 1 commit
  9. 01 Apr, 2016 5 commits
    • Josh Bleecher Snyder's avatar
      cmd/compile: rename Node.Int to Node.Int64 · 5cab0169
      Josh Bleecher Snyder authored
      gorename -from '"cmd/compile/internal/gc".Node.Int' -to 'Int64'
      
      Change-Id: I2fe3bf9a26ae6b0600d990d0c981e4b8b53020a4
      Reviewed-on: https://go-review.googlesource.com/21426Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      5cab0169
    • Josh Bleecher Snyder's avatar
      cmd/compile: add Type.SetNumElem · 5dd129bc
      Josh Bleecher Snyder authored
      This removes all access to Type.Bound
      from outside type.go.
      
      Update sinit to make a new type rather than
      copy and mutate.
      
      Update bimport to create a new slice type
      instead of mutating TDDDFIELD.
      These are rare, so the extra allocs are nominal.
      
      I’m not happy about having a setter,
      but it appears the most practical route
      forward at the moment, and it only has a few uses.
      
      Passes toolstash -cmp.
      
      Change-Id: I174f07c8f336afc656904bde4bdbde4f3ef0db96
      Reviewed-on: https://go-review.googlesource.com/21423
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      5dd129bc
    • Josh Bleecher Snyder's avatar
      cmd/compile: use Node.Int more · e504055e
      Josh Bleecher Snyder authored
      Generated by eg.
      
      Passes toolstash -cmp.
      
      Change-Id: I7516c211ca9aacf824f74894671dc62d31763b01
      Reviewed-on: https://go-review.googlesource.com/21422
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      e504055e
    • Josh Bleecher Snyder's avatar
      cmd/compile: use NumElem instead of Type.Bound · 3a0783c5
      Josh Bleecher Snyder authored
      This eliminates all direct reads of Type.Bound
      outside type.go.
      
      Change-Id: I0a9a72539f8f4c0de7f5e05e1821936bf7db5eb7
      Reviewed-on: https://go-review.googlesource.com/21421
      Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      3a0783c5
    • David Chase's avatar
      cmd/compile: ignore OXXX nodes in closure captured vars list · b64f549b
      David Chase authored
      Added a debug flag "-d closure" to explain compilation of
      closures (should this be done some other way? Should we
      rewrite the "-m" flag to "-d escapes"?)  Used this to
      discover that cause was an OXXX node in the captured vars
      list, and in turn noticed that OXXX nodes are explicitly
      ignored in all other processing of captured variables.
      
      Couldn't figure out a reproducer, did verify that this OXXX
      was not caused by an unnamed return value (which is one use
      of these).  Verified lack of heap allocation by examining -S
      output.
      
      Assembly:
      (runtime/mgc.go:1371) PCDATA $0, $2
      (runtime/mgc.go:1371) CALL "".notewakeup(SB)
      (runtime/mgc.go:1377) LEAQ "".gcBgMarkWorker.func1·f(SB), AX
      (runtime/mgc.go:1404) MOVQ AX, (SP)
      (runtime/mgc.go:1404) MOVQ "".autotmp_2242+88(SP), CX
      (runtime/mgc.go:1404) MOVQ CX, 8(SP)
      (runtime/mgc.go:1404) LEAQ go.string."GC worker (idle)"(SB), AX
      (runtime/mgc.go:1404) MOVQ AX, 16(SP)
      (runtime/mgc.go:1404) MOVQ $16, 24(SP)
      (runtime/mgc.go:1404) MOVB $20, 32(SP)
      (runtime/mgc.go:1404) MOVQ $0, 40(SP)
      (runtime/mgc.go:1404) PCDATA $0, $2
      (runtime/mgc.go:1404) CALL "".gopark(SB)
      
      Added a check for compiling_runtime to ensure that this is
      caught in the future.  Added a test to test the check.
      Verified that 1.5.3 did NOT reject the test case when
      compiled with -+ flag, so this is not a recently added bug.
      
      Cause of bug is two-part -- there was no leaking closure
      detection ever, and instead it relied on capture-of-variables
      to trigger compiling_runtime test, but closures improved in
      1.5.3 so that mere capture of a value did not also capture
      the variable, which thus allowed closures to escape, as well
      as this case where the escape was spurious.  In
      fixedbugs/issue14999.go, compare messages for f and g;
      1.5.3 would reject g, but not f.  1.4 rejects both because
      1.4 heap-allocates parameter x for both.
      
      Fixes #14999.
      
      Change-Id: I40bcdd27056810628e96763a44f2acddd503aee1
      Reviewed-on: https://go-review.googlesource.com/21322
      Run-TryBot: David Chase <drchase@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      b64f549b
  10. 31 Mar, 2016 1 commit
  11. 30 Mar, 2016 5 commits
  12. 29 Mar, 2016 3 commits
  13. 24 Mar, 2016 1 commit
  14. 22 Mar, 2016 1 commit
    • Josh Bleecher Snyder's avatar
      cmd/compile: reduce use of **Node parameters · 34699bc7
      Josh Bleecher Snyder authored
      Escape analysis has a hard time with tree-like
      structures (see #13493 and #14858).
      This is unlikely to change.
      As a result, when invoking a function that accepts
      a **Node parameter, we usually allocate a *Node
      on the heap. This happens a whole lot.
      
      This CL changes functions from taking a **Node
      to acting more like append: It both modifies
      the input and returns a replacement for it.
      
      Because of the cascading nature of escape analysis,
      in order to get the benefits, I had to modify
      almost all such functions. The remaining functions
      are in racewalk and the backend. I would be happy
      to update them as well in a separate CL.
      
      This CL was created by manually updating the
      function signatures and the directly impacted
      bits of code. The callsites were then automatically
      updated using a bespoke script:
      https://gist.github.com/josharian/046b1be7aceae244de39
      
      For ease of reviewing and future understanding,
      this CL is also broken down into four CLs,
      mailed separately, which show the manual
      and the automated changes separately.
      They are CLs 20990, 20991, 20992, and 20993.
      
      Passes toolstash -cmp.
      
      name       old time/op     new time/op     delta
      Template       335ms ± 5%      324ms ± 5%   -3.35%        (p=0.000 n=23+24)
      Unicode        176ms ± 9%      165ms ± 6%   -6.12%        (p=0.000 n=23+24)
      GoTypes        1.10s ± 4%      1.07s ± 2%   -2.77%        (p=0.000 n=24+24)
      Compiler       5.31s ± 3%      5.15s ± 3%   -2.95%        (p=0.000 n=24+24)
      MakeBash       41.6s ± 1%      41.7s ± 2%     ~           (p=0.586 n=23+23)
      
      name       old alloc/op    new alloc/op    delta
      Template      63.3MB ± 0%     62.4MB ± 0%   -1.36%        (p=0.000 n=25+23)
      Unicode       42.4MB ± 0%     41.6MB ± 0%   -1.99%        (p=0.000 n=24+25)
      GoTypes        220MB ± 0%      217MB ± 0%   -1.11%        (p=0.000 n=25+25)
      Compiler       994MB ± 0%      973MB ± 0%   -2.08%        (p=0.000 n=24+25)
      
      name       old allocs/op   new allocs/op   delta
      Template        681k ± 0%       574k ± 0%  -15.71%        (p=0.000 n=24+25)
      Unicode         518k ± 0%       413k ± 0%  -20.34%        (p=0.000 n=25+24)
      GoTypes        2.08M ± 0%      1.78M ± 0%  -14.62%        (p=0.000 n=25+25)
      Compiler       9.26M ± 0%      7.64M ± 0%  -17.48%        (p=0.000 n=25+25)
      
      name       old text-bytes  new text-bytes  delta
      HelloSize       578k ± 0%       578k ± 0%     ~     (all samples are equal)
      CmdGoSize      6.46M ± 0%      6.46M ± 0%     ~     (all samples are equal)
      
      name       old data-bytes  new data-bytes  delta
      HelloSize       128k ± 0%       128k ± 0%     ~     (all samples are equal)
      CmdGoSize       281k ± 0%       281k ± 0%     ~     (all samples are equal)
      
      name       old exe-bytes   new exe-bytes   delta
      HelloSize       921k ± 0%       921k ± 0%     ~     (all samples are equal)
      CmdGoSize      9.86M ± 0%      9.86M ± 0%     ~     (all samples are equal)
      
      Change-Id: I277d95bd56d51c166ef7f560647aeaa092f3f475
      Reviewed-on: https://go-review.googlesource.com/20959Reviewed-by: default avatarDave Cheney <dave@cheney.net>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      34699bc7
  15. 21 Mar, 2016 2 commits
    • Matthew Dempsky's avatar
      cmd/compile: change Mp{int,flt} functions into methods · d3253876
      Matthew Dempsky authored
      Also give them more idiomatic Go names. Adding godocs is outside the
      scope of this CL. (Besides, the method names almost all directly
      parallel an underlying math/big.Int or math/big.Float method.)
      
      CL prepared mechanically with sed (for rewriting mpint.go/mpfloat.go)
      and gofmt (for rewriting call sites).
      
      Passes toolstash -cmp.
      
      Change-Id: Id76f4aee476ba740f48db33162463e7978c2083d
      Reviewed-on: https://go-review.googlesource.com/20909
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      d3253876
    • Brad Fitzpatrick's avatar
      cmd/compile: remove most of the Lookupf users and garbage · 060a6915
      Brad Fitzpatrick authored
      Introduce garbage-free LookupN to replace most users of Lookupf.
      
      Also, remove the string interning from LookupBytes which was hurting
      more than helping.
      
      name       old alloc/op    new alloc/op    delta
      Template      63.0MB ± 0%     62.7MB ± 0%  -0.48%         (p=0.000 n=10+9)
      Unicode       43.0MB ± 0%     43.0MB ± 0%  -0.17%         (p=0.000 n=10+7)
      GoTypes        219MB ± 0%      218MB ± 0%  -0.14%        (p=0.000 n=10+10)
      Compiler       992MB ± 0%      991MB ± 0%  -0.12%        (p=0.000 n=10+10)
      
      name       old allocs/op   new allocs/op   delta
      Template        683k ± 0%       681k ± 0%  -0.38%         (p=0.000 n=10+8)
      Unicode         541k ± 0%       541k ± 0%  -0.11%        (p=0.000 n=10+10)
      GoTypes        2.09M ± 0%      2.08M ± 0%  -0.40%        (p=0.000 n=10+10)
      Compiler       9.28M ± 0%      9.24M ± 0%  -0.36%        (p=0.000 n=10+10)
      
      Size of $GOROOT/pkg/darwin_amd64 drops from 40124 KB to 40100 KB too,
      removing the zero padding as suggested by josharian.
      
      Updates #6853
      
      Change-Id: I3c557266e9325fe29c459cef8e5b8954913e7abb
      Reviewed-on: https://go-review.googlesource.com/20931Reviewed-by: default avatarJosh Bleecher Snyder <josharian@gmail.com>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      060a6915
  16. 19 Mar, 2016 1 commit
    • Ian Lance Taylor's avatar
      cmd/compile: change ODOT and friends to use Sym, not Right · 5f525ca6
      Ian Lance Taylor authored
      The Node type ODOT and its variants all represent a selector, with a
      simple name to the right of the dot.  Before this change this was
      represented by using an ONAME Node in the Right field.  This ONAME node
      served no useful purpose.  This CL changes these Node types to store the
      symbol in the Sym field instead, thus not requiring allocating a Node
      for each selector.
      
      When compiling x/tools/go/types this CL eliminates nearly 5000 calls to
      newname and reduces the total number of Nodes allocated by about 6.6%.
      It seems to cut compilation time by 1 to 2 percent.
      
      Getting this right was somewhat subtle, and I added two dubious changes
      to produce the exact same output as before.  One is to ishairy in
      inl.go: the ONAME node increased the cost of ODOT and friends by 1, and
      I retained that, although really ODOT is not more expensive than any
      other node.  The other is to varexpr in walk.go: because the ONAME in
      the Right field of an ODOT has no class, varexpr would always return
      false for an ODOT, although in fact for some ODOT's it seemingly ought
      to return true; I added an && false for now.  I will send separate CLs,
      that will break toolstash -cmp, to clean these up.
      
      This CL passes toolstash -cmp.
      
      Change-Id: I4af8a10cc59078c436130ce472f25abc3a9b2f80
      Reviewed-on: https://go-review.googlesource.com/20890
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      5f525ca6
  17. 17 Mar, 2016 1 commit
  18. 16 Mar, 2016 2 commits
  19. 15 Mar, 2016 1 commit
  20. 14 Mar, 2016 5 commits