1. 25 Sep, 2019 23 commits
    • Dan Scales's avatar
      misc, runtime, test: extra tests and benchmarks for defer · 225f484c
      Dan Scales authored
      Add a bunch of extra tests and benchmarks for defer, in preparation for new
      low-cost (open-coded) implementation of defers (see #34481),
      
       - New file defer_test.go that tests a bunch more unusual defer scenarios,
         including things that might have problems for open-coded defers.
       - Additions to callers_test.go actually verifying what the stack trace looks like
         for various panic or panic-recover scenarios.
       - Additions to crash_test.go testing several more crash scenarios involving
         recursive panics.
       - New benchmark in runtime_test.go measuring speed of panic-recover
       - New CGo benchmark in cgo_test.go calling from Go to C back to Go that
         shows defer overhead
      
      Updates #34481
      
      Change-Id: I423523f3e05fc0229d4277dd00073289a5526188
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197017
      Run-TryBot: Dan Scales <danscales@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      225f484c
    • Michael Munday's avatar
      cmd/asm: add masked branch and conditional load instructions to s390x · 8c99e45e
      Michael Munday authored
      The branch-relative-on-condition (BRC) instruction allows us to use
      an immediate to specify under what conditions the branch is taken.
      For example, `BRC $7, L1` is equivalent to `BNE L1`. It is sometimes
      useful to specify branches in this way when either we don't have
      an extended mnemonic for a particular mask value or we want to
      generate the condition code mask programmatically.
      
      The new load-on-condition (LOCR and LOCGR) and compare-and-branch
      (CRJ, CGRJ, CLRJ, CLGRJ, CIJ, CGIJ, CLIJ and CLGIJ) instructions
      provide the same flexibility for conditional loads and combined
      compare and branch instructions.
      
      Change-Id: Ic6f5d399b0157e278b39bd3645f4ee0f4df8e5fc
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196558
      Run-TryBot: Michael Munday <mike.munday@ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      8c99e45e
    • Michael Anthony Knyszek's avatar
      runtime: scavenge on growth instead of inline with allocation · eb96f8a5
      Michael Anthony Knyszek authored
      Inline scavenging causes significant performance regressions in tail
      latency for k8s and has relatively little benefit for RSS footprint.
      
      We disabled inline scavenging in Go 1.12.5 (CL 174102) as well, but
      we thought other changes in Go 1.13 had mitigated the issues with
      inline scavenging. Apparently we were wrong.
      
      This CL switches back to only doing foreground scavenging on heap
      growth, rather than doing it when allocation tries to allocate from
      scavenged space.
      
      Fixes #32828.
      
      Change-Id: I1f5df44046091f0b4f89fec73c2cde98bf9448cb
      Reviewed-on: https://go-review.googlesource.com/c/go/+/183857
      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>
      eb96f8a5
    • Austin Clements's avatar
      runtime: grow the heap incrementally · f18109d7
      Austin Clements authored
      Currently, we map and grow the heap a whole arena (64MB) at a time.
      Unfortunately, in order to fix #32828, we need to switch from
      scavenging inline with allocation back to scavenging on heap growth,
      but heap-growth scavenging happens in large jumps because we grow the
      heap in large jumps.
      
      In order to prepare for better heap-growth scavenging, this CL
      separates mapping more space for the heap from actually "growing" it
      (tracking the new space with spans). Instead, growing the heap keeps
      track of the "current arena" it's growing into. It track that with new
      spans as needed, and only maps more arena space when the current arena
      is inadequate. The effect to the user is the same, but this will let
      us scavenge on much smaller increments of heap growth.
      
      There are two slightly subtleties to this change:
      
      1. If an allocation requires mapping a new arena and that new arena
         isn't contiguous with the current arena, we don't want to lose the
         unused space in the current arena, so we have to immediately track
         that with a span.
      
      2. The mapped space must be accounted as released and idle, even
         though it isn't actually tracked in a span.
      
      For #32828, since this makes heap-growth scavenging far more
      effective, especially at small heap sizes. For example, this change is
      necessary for TestPhysicalMemoryUtilization to pass once we remove
      inline scavenging.
      
      Change-Id: I300e74a0534062467e4ce91cdc3508e5ef9aa73a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/189957
      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>
      f18109d7
    • Michael Anthony Knyszek's avatar
      runtime: redefine scavenge goal in terms of heap_inuse · 9b308112
      Michael Anthony Knyszek authored
      This change makes it so that the scavenge goal is defined primarily in
      terms of heap_inuse at the end of the last GC rather than next_gc. The
      reason behind this change is that next_gc doesn't take into account
      fragmentation, and we can fall into situation where the scavenger thinks
      it should have work to do but there's no free and unscavenged memory
      available.
      
      In order to ensure the scavenge goal still tracks next_gc, we multiply
      heap_inuse by the ratio between the current heap goal and the last heap
      goal, which describes whether the heap is growing or shrinking, and by
      how much.
      
      Finally, this change updates the documentation for scavenging and
      elaborates on why the scavenge goal is defined the way it is.
      
      Fixes #34048.
      Updates #32828.
      
      Change-Id: I8deaf87620b5dc12a40ab8a90bf27932868610da
      Reviewed-on: https://go-review.googlesource.com/c/go/+/193040
      Run-TryBot: Michael Knyszek <mknyszek@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarKeith Randall <khr@golang.org>
      9b308112
    • Meng Zhuo's avatar
      internal/bytealg: add assembly implementation of Compare/CompareString on mips64x · 78baea83
      Meng Zhuo authored
      name                         old time/op    new time/op    delta
      BytesCompare/1                 28.9ns ± 4%    22.8ns ± 0%    -21.23%  (p=0.000 n=9+10)
      BytesCompare/2                 34.6ns ± 0%    23.5ns ± 0%    -32.01%  (p=0.000 n=8+10)
      BytesCompare/4                 54.6ns ± 0%    41.4ns ± 0%    -24.18%  (p=0.001 n=8+9)
      BytesCompare/8                 73.9ns ± 0%    49.7ns ± 0%    -32.75%  (p=0.002 n=8+10)
      BytesCompare/16                 113ns ± 0%      23ns ± 0%    -79.56%  (p=0.000 n=9+10)
      BytesCompare/32                 190ns ± 0%      26ns ± 0%    -86.53%  (p=0.000 n=10+10)
      BytesCompare/64                 345ns ± 0%      44ns ± 0%    -87.19%  (p=0.000 n=10+10)
      BytesCompare/128                654ns ± 0%      52ns ± 0%    -91.97%  (p=0.000 n=9+8)
      BytesCompare/256               1.27µs ± 0%    0.08µs ± 1%    -94.10%  (p=0.000 n=8+10)
      BytesCompare/512               2.51µs ± 0%    0.12µs ± 0%    -95.26%  (p=0.000 n=9+9)
      BytesCompare/1024              4.99µs ± 0%    0.21µs ± 1%    -95.84%  (p=0.000 n=8+10)
      BytesCompare/2048              9.94µs ± 0%    0.38µs ± 0%    -96.13%  (p=0.000 n=8+10)
      CompareBytesEqual               105ns ± 0%      64ns ± 0%    -39.05%  (p=0.000 n=10+9)
      CompareBytesToNil              34.8ns ± 1%    39.5ns ± 3%    +13.48%  (p=0.000 n=10+10)
      CompareBytesEmpty              33.6ns ± 3%    36.6ns ± 0%     +8.77%  (p=0.000 n=10+10)
      CompareBytesIdentical          29.7ns ± 0%    36.6ns ± 0%    +23.23%  (p=0.000 n=10+10)
      CompareBytesSameLength         69.1ns ± 0%    51.1ns ± 0%    -26.05%  (p=0.000 n=10+10)
      CompareBytesDifferentLength    69.8ns ± 0%    51.1ns ± 0%    -26.79%  (p=0.000 n=10+10)
      CompareBytesBigUnaligned       5.15ms ± 0%    2.18ms ± 0%    -57.62%  (p=0.000 n=9+9)
      CompareBytesBig                5.28ms ± 0%    0.28ms ± 0%    -94.64%  (p=0.000 n=8+10)
      CompareBytesBigIdentical       29.7ns ± 0%    36.8ns ± 0%    +23.91%  (p=0.000 n=8+8)
      
      name                         old speed      new speed      delta
      CompareBytesBigUnaligned      204MB/s ± 0%   480MB/s ± 0%   +135.94%  (p=0.000 n=9+9)
      CompareBytesBig               198MB/s ± 0%  3703MB/s ± 0%  +1765.85%  (p=0.000 n=8+10)
      CompareBytesBigIdentical     35.3TB/s ± 0%  28.5TB/s ± 0%    -19.31%  (p=0.000 n=8+8)
      
      Change-Id: I112d9de2324986fd65ed237a86b11856a1c0f4a7
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196837Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      78baea83
    • Keith Randall's avatar
      runtime: fix ppc64le race code · 44e752c3
      Keith Randall authored
      This code is not currently compiling, the asm vet checks fail. When running race.bash on ppc64le, I get:
      
      runtime/race_ppc64le.s:104:1: [ppc64le] RaceReadRange: wrong argument size 24; expected $...-16
      runtime/race_ppc64le.s:514:1: [ppc64le] racecallbackthunk: unknown variable cmd; offset 0 is arg+0(FP)
      runtime/race_ppc64le.s:515:1: [ppc64le] racecallbackthunk: unknown variable ctx
      
      I'm also not sure why it ever worked; it looks like it is writing
      the arguments to racecallback in the wrong place (the race detector
      itself probably still works, it would just have trouble symbolizing
      any resulting race report).
      
      At a meta-level, we should really add a ppc64le/race builder.
      Otherwise this code will rot, as evidenced by the rot this CL fixes :)
      
      Update #33309
      
      Change-Id: I3b49c2442aa78538fbb631a143a757389a1368fd
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197337
      Run-TryBot: Keith Randall <khr@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarLynn Boger <laboger@linux.vnet.ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      44e752c3
    • Marwan Sulaiman's avatar
      cmd/go: consistent output for -json failures · 6232dadc
      Marwan Sulaiman authored
      When the -json flag is passed to go mod download,
      the sumdb error is embedded in the json Error field.
      Other errors for the same command behave this way as
      well such as module not found. The fix is done by changing
      base.Fatalf into proper error returns.
      
      Fixes #34485
      
      Change-Id: I2727a5c70c7ab03988cad8661894d0f8ec71a768
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197062
      Run-TryBot: Jay Conrod <jayconrod@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      6232dadc
    • Matthew Dempsky's avatar
      cmd/compile: optimize escape graph construction and walking · 26204671
      Matthew Dempsky authored
      This CL implements several optimizations for the escape analysis flow
      graph:
      
      1. Instead of recognizing heapLoc specially within Escape.outlives,
      set heapLoc.escapes = true and recognize any location with escapes
      set. This allows us to skip adding edges from the heap to escaped
      variables in two cases:
      
      1a. In newLoc, if the location is for a variable or allocation too
      large to fit on the stack.
      
      1b. During walkOne, if we discover that an object's address flows
      somewhere that naturally outlives it.
      
      2. When recording edges in Escape.flow, if x escapes and we're adding
      an edge like "x = &y", we can simply mark that y escapes too.
      
      3. During walkOne, if we reach a location that's marked as escaping,
      we can skip visiting it again: we've either already walked from it, or
      it's in queue to be walked from again.
      
      On average, reduces the number of visited locations by 15%. Reduces
      time spent in escape analysis for particularly hairy packages like
      runtime and gc by about 8%. Reduces escape.go's TODO count by 22%.
      
      Passes toolstash-check.
      
      Change-Id: Iaf86a29d76044e4b4c8ab581b916ef5bb5df4437
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196811Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      26204671
    • Matthew Dempsky's avatar
      cmd/compile: use proper work queue for escape graph walking · 867ea9c1
      Matthew Dempsky authored
      The old escape analysis code used to repeatedly walk the entire flow
      graph until it reached a fixed point. With escape.go, I wanted to
      avoid this if possible, so I structured the walking code with two
      constraints:
      
      1. Always walk from the heap location last.
      
      2. If an object escapes, ensure it has flow edge to the heap location.
      
      This works, but it precludes some graph construction
      optimizations. E.g., if there's an assignment "heap = &x", then we can
      immediately tell that 'x' escapes without needing to visit it during
      the graph walk. Similarly, if there's a later assignment "x = &y", we
      could immediately tell that 'y' escapes too. However, the natural way
      to implement this optimization ends up violating the constraints
      above.
      
      Further, the constraints above don't guarantee that the 'transient'
      flag is handled correctly. Today I think that's handled correctly
      because of the order that locations happen to be constructed and
      visited based on the AST, but I've felt uneasy about it for a little
      while.
      
      This CL changes walkAll to use a proper work queue (technically a work
      stack) to track locations that need to be visited, and allows walkOne
      to request that a location be re-visited.
      
      Passes toolstash-check.
      
      Change-Id: Iaa6f4d3fe4719c04d67009fb9a2a3e4930b3d7c2
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196958
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      867ea9c1
    • Than McIntosh's avatar
      test: add testcase for gccgo compiler buglet · fad0a14d
      Than McIntosh authored
      New test containing code that caused a gccgo compiler failure.
      
      Updates #34503.
      
      Change-Id: Id895a1e1249062b7fb147e54bcaa657e774ed0d9
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197217
      Run-TryBot: Than McIntosh <thanm@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      fad0a14d
    • Cuong Manh Le's avatar
      cmd/compile: remove n.SetLikely(false) usage · e0dde990
      Cuong Manh Le authored
      n.SetLikely(false) is probably mean to indicate that the branch is
      "unlikely", but it has the real effect of not marking branch as likely.
      
      So invert the test condition, we can use more meaningful n.SetLikely(true).
      
      Before:
      
      	if l2 < 0 {
      		panicmakeslicelen()
      	}
      
      After:
      
      	if l2 >= 0 {
      	} else {
      		panicmakeslicelen
      	}
      
      Fixes #32486
      
      Change-Id: I156fdba1f9a5d554a178c8903f1a391ed304199d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/195197
      Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      e0dde990
    • Jeremy Faller's avatar
      cmd/compile: update object file format for DWARF file table · 21bf37b5
      Jeremy Faller authored
      In CL 188317, we generate the debug_lines in the compiler, and created a
      new symbol to hold the line table. Here we modify the object file format
      to output the file table.
      
      Change-Id: Ibee192e80b86ff6af36467a0b1c26ee747dfee37
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191167Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarThan McIntosh <thanm@google.com>
      Run-TryBot: Austin Clements <austin@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      21bf37b5
    • Matthew Dempsky's avatar
      cmd/compile: use underlying OCOMPLIT's position for OPTRLIT · efb97392
      Matthew Dempsky authored
      Currently, when we create an OPTRLIT node, it defaults to the
      OCOMPLIT's final element's position. But it improves error messages to
      use the OCOMPLIT's own position instead.
      
      Change-Id: Ibb031f543c7248d88d99fd0737685e01d86e2500
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197119
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      efb97392
    • Matthew Dempsky's avatar
      cmd/compile: remove -s flag · 3e428363
      Matthew Dempsky authored
      This is better handled by tools like cmd/gofmt, which can
      automatically rewrite the source code and already supports a syntactic
      version of this simplification. (go/types can be used if
      type-sensitive simplification is actually necessary.)
      
      Change-Id: I51332a8f3ff4ab3087bc6b43a491c6d92b717228
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197118Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      3e428363
    • Matthew Dempsky's avatar
      test: add regress test for #27557 · f346a4c4
      Matthew Dempsky authored
      This commit just adds a regress test for a few of the important corner
      cases that I identified in #27557, which turn out to not be tested
      anywhere.
      
      While here, annotate a few of the existing test cases where we could
      improve escape analysis.
      
      Updates #27557.
      
      Change-Id: Ie57792a538f7899bb17915485fabc86100f469a3
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197137
      Run-TryBot: Matthew Dempsky <mdempsky@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      f346a4c4
    • Bryan C. Mills's avatar
      cmd/go/internal/modload: annotate replacements in PackageNotInModuleError · 8189a061
      Bryan C. Mills authored
      Fixes #34085
      
      Change-Id: I3111f5997466ad33f51e80c71f5fb2ccebdcc6e4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/193617
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      8189a061
    • Daniel Martí's avatar
      text/template: don't evaluate '.' as a float64 · 0f7b4e72
      Daniel Martí authored
      When using a '.' constant literal as a reflect.Value variadic argument,
      idealConstant would incorrectly result in a float64. This is because
      rune literals can be represented as a float64, and contain a period,
      which tricked the logic into thinking the literal must have been a
      floating point number.
      
      This also happened with other characters that can be part of a floating
      point number, such as 'e' or 'P'.
      
      To fix these edge cases, exit the case sooner if the literal was a rune,
      since that should always go to the int case instead.
      
      Finally, add test cases that verify that they behave properly. These
      would error before, since eq would receive a mix of int and float64,
      which aren't comparable.
      
      Fixes #34483.
      
      Change-Id: Icfcb7803bfa0cf317a1d1adacacad3d69a57eb42
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196808
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarTom Payne <tom@airmap.com>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      0f7b4e72
    • Ariel Mashraki's avatar
      text/template: support all comparable types in eq · 95cbcc5c
      Ariel Mashraki authored
      Extends the built-in eq function to support all Go
      comparable types.
      
      Fixes #33740
      
      Change-Id: I522310e313e251c4dc6a013d33d7c2034fe2ec8e
      Reviewed-on: https://go-review.googlesource.com/c/go/+/193837
      Run-TryBot: Rob Pike <r@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRob Pike <r@golang.org>
      95cbcc5c
    • Joel Sing's avatar
      cmd/internal/obj/riscv: implement floating point instructions · 434e4caf
      Joel Sing authored
      Add support for assembling various single-precision and double-precision
      floating point instructions.
      
      Based on the riscv-go port.
      
      Updates #27532
      
      Change-Id: Iac1aec9b03bb6cbf116b229daeef944d4df550fa
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196839Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      434e4caf
    • Tobias Klauser's avatar
      runtime: gofmt after CL 192937 · 8f5755e7
      Tobias Klauser authored
      CL 192937 introduced some changes which weren't properly gofmt'ed. Do so
      now.
      
      Change-Id: I2d2d57ea8a79fb41bc4ca59fa23f12198d615fd8
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196812
      Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
      Reviewed-by: default avatarIan Lance Taylor <iant@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      8f5755e7
    • Brad Fitzpatrick's avatar
      net/http: propagate Client.Timeout down into Request's context deadline · 7fc2625e
      Brad Fitzpatrick authored
      Fixes #31657
      
      Change-Id: I85e9595d3ea30d410f1f4b787925a6879a72bdf2
      Reviewed-on: https://go-review.googlesource.com/c/go/+/175857Reviewed-by: default avatarBenny Siegert <bsiegert@gmail.com>
      Run-TryBot: Benny Siegert <bsiegert@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      7fc2625e
    • Cuong Manh Le's avatar
      cmd/compile: add documentation for isfat · caf45cde
      Cuong Manh Le authored
      In CL 192980, I tend to think that canSSAType can be used as replacement
      for isfat. It is not the truth as @khr points me out that isfat has very
      different purpose.
      
      So this CL adds documentation for isfat, also remove outdated TODO.
      
      Change-Id: I15954d638759bd9f6b28a6aa04c1a51129d9ae7d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196499
      Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
      caf45cde
  2. 24 Sep, 2019 15 commits
    • Alan Donovan's avatar
      log: mention names of flag constants in {Set,}Flags doc comments · 54776230
      Alan Donovan authored
      Change-Id: I1217f07530dc7586fd7b933bc6a65bad163782db
      Reviewed-on: https://go-review.googlesource.com/c/go/+/47232Reviewed-by: default avatarAndrew Bonventre <andybons@golang.org>
      Run-TryBot: Andrew Bonventre <andybons@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      54776230
    • Jeremy Faller's avatar
      cmd/compile: generate debug_lines in compiler · ee3dded3
      Jeremy Faller authored
      This is mostly a copy-paste jobs from the linker to generate the debug
      information in the compiler instead of the linker. The new data is
      inserted into the debug line numbers symbol defined in CL 188238.
      
      Generating the debug information BEFORE deadcode results in one subtle
      difference, and that is that the state machine needs to be reset at the
      end of every function's debug line table. The reasoning is that
      generating the table AFTER dead code allows the producer and consumer of
      the table to agree on the state of the state machine, and since these
      blocks will (eventually) be concatenated in the linker, we don't KNOW
      the state of the state machine unless we reset it. So,
      generateDebugLinesSymbol resets the state machine at the end of every
      function.
      
      Right now, we don't do anything with this line information, or the file
      table -- we just populate the symbols.
      
      Change-Id: If9103eda6cc5f1f7a11e7e1a97184a060a4ad7fb
      Reviewed-on: https://go-review.googlesource.com/c/go/+/188317
      Run-TryBot: Jeremy Faller <jeremy@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      ee3dded3
    • Jeremy Faller's avatar
      cmd/link: add notion of multiple compilation units per package · 78a37347
      Jeremy Faller authored
      As we move the debug_line generation into the compiler, we need to
      upgrade the notion of compilationUnit to not just be on a per package
      basis.  That won't be the case as it will be impossible for all
      compilationUnits to have the same set of files names used to build the
      debug_lines table. (For example, assembled files in a package don't know
      about any files but themselves, so the debug_lines table could only
      reference themseves. As such, we need to break the 1:1 relationship
      between compUnit and package.)
      
      Change-Id: I2e517bb6c01de0115bbf777af828a2fe59c09ce8
      Reviewed-on: https://go-review.googlesource.com/c/go/+/189618
      Run-TryBot: Jeremy Faller <jeremy@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarAustin Clements <austin@google.com>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      78a37347
    • Lynn Boger's avatar
      crypto/cipher: improve xorBytesVSX asm for ppc64x · d979ac33
      Lynn Boger authored
      This improves the performance of xorBytesVSX in crypto/cipher by
      unrolling the loop that does the stores. Improvement on power9:
      
      name                 old time/op    new time/op    delta
      XORBytes/8Bytes        17.9ns ± 0%    18.2ns ± 0%   +1.53%  (p=0.029 n=4+4)
      XORBytes/128Bytes      24.4ns ± 0%    22.5ns ± 0%   -7.79%  (p=0.029 n=4+4)
      XORBytes/2048Bytes      131ns ± 0%     109ns ± 0%  -16.79%  (p=0.029 n=4+4)
      XORBytes/32768Bytes    1.74µs ± 0%    1.43µs ± 8%  -18.04%  (p=0.029 n=4+4)
      
      Change-Id: I75bd625d3ae9daa7bda54c523028671ab036b13d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197058
      Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarCarlos Eduardo Seo <cseo@linux.vnet.ibm.com>
      d979ac33
    • Bryan C. Mills's avatar
      cmd/go: suppress errors in package-to-module queries if the package is already found · a3426f25
      Bryan C. Mills authored
      In CL 173017, I changed the package-to-module query logic to query all
      possible module paths in parallel in order to reduce latency. (For
      long package paths, most such paths will not exist and will fail with
      little overhead.)
      
      The module resolution algorithm treats various kinds of non-existence
      as “soft errors”, to be reported only if package resolution fails, but
      treats any remaining errors as hard errors that should fail the query.
      
      Unfortunately, that interacted badly with the +incompatible version
      validation added in CL 181881, causing a regression in the 'direct'
      fetch path for modules using the “major branch” layout¹ with a post-v1
      version on the repository's default branch. Because we did not
      interpret a mismatched module path as “no such module”, a go.mod file
      specifying the path 'example.com/foo/v2' would cause the search for
      module 'example.com/foo' to error out. (That regression was not caught
      ahead of time due to a lack of test coverage for 'go get' on a package
      within a /vN module.)
      
      The promotion of hard errors during parallel search also made the 'go'
      command less tolerant of servers that advertise 'go-import' tags for
      nonexistent repositories. CL 194561 mitigated that problem for HTTP
      servers that return code 404 or 410 for a nonexistent repository, but
      unfortunately a few servers in common use (notably GitLab and
      pre-1.9.3 releases of Gitea) do not.
      
      This change mitigates both of those failure modes by ignoring
      “miscellaneous” errors from shorter module paths if the requested
      package pattern was successfully matched against a module with a
      longer path.
      
      ¹https://research.swtch.com/vgo-module#from_repository_to_modules
      
      Updates #34383
      Updates #34094
      
      Change-Id: If37dc422e973eba13f3a3aeb68bc7b96e2d7f73d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197059
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      a3426f25
    • Martin Möhrmann's avatar
      compile: prefer an AND instead of SHR+SHL instructions · f41451e7
      Martin Möhrmann authored
      On modern 64bit CPUs a SHR, SHL or AND instruction take 1 cycle to execute.
      A pair of shifts that operate on the same register will take 2 cycles
      and needs to wait for the input register value to be available.
      
      Large constants used to mask the high bits of a register with an AND
      instruction can not be encoded as an immediate in the AND instruction
      on amd64 and therefore need to be loaded into a register with a MOV
      instruction.
      
      However that MOV instruction is not dependent on the output register and
      on many CPUs does not compete with the AND or shift instructions for
      execution ports.
      
      Using a pair of shifts to mask high bits instead of an AND to mask high
      bits of a register has a shorter encoding and uses one less general
      purpose register but is slower due to taking one clock cycle longer
      if there is no register pressure that would make the AND variant need to
      generate a spill.
      
      For example the instructions emitted for (x & 1 << 63) before this CL are:
      48c1ea3f                SHRQ $0x3f, DX
      48c1e23f                SHLQ $0x3f, DX
      
      after this CL the instructions are the same as GCC and LLVM use:
      48b80000000000000080    MOVQ $0x8000000000000000, AX
      4821d0                  ANDQ DX, AX
      
      Some platforms such as arm64 already have SSA optimization rules to fuse
      two shift instructions back into an AND.
      
      Removing the general rule to rewrite AND to SHR+SHL speeds up this benchmark:
      
          var GlobalU uint
      
          func BenchmarkAndHighBits(b *testing.B) {
              x := uint(0)
              for i := 0; i < b.N; i++ {
                      x &= 1 << 63
              }
              GlobalU = x
          }
      
      amd64/darwin on Intel(R) Core(TM) i7-3520M CPU @ 2.90GHz:
      name           old time/op  new time/op  delta
      AndHighBits-4  0.61ns ± 6%  0.42ns ± 6%  -31.42%  (p=0.000 n=25+25):
      
      'go run run.go -all_codegen -v codegen' passes  with following adjustments:
      
      ARM64: The BFXIL pattern ((x << lc) >> rc | y & ac) needed adjustment
             since ORshiftRL generation fusing '>> rc' and '|' interferes
             with matching ((x << lc) >> rc) to generate UBFX. Previously
             ORshiftLL was created first using the shifts generated for (y & ac).
      
      S390X: Add rules for abs and copysign to match use of AND instead of SHIFTs.
      
      Updates #33826
      Updates #32781
      
      Change-Id: I5a59f6239660d53c029cd22dfb44ddf39f93a56c
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196810
      Run-TryBot: Martin Möhrmann <moehrmann@google.com>
      Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      f41451e7
    • Bryan C. Mills's avatar
      cmd/go/internal/modfetch/codehost: work around an apparent bug in 'git fetch --unshallow' · 1804bbab
      Bryan C. Mills authored
      When 'git fetch' is passed the '--unshallow' flag, it assumes that the
      local and remote refs are equal.¹ However, we were fetching an
      expanded set of refs explicitly in the same command, violating that
      assumption.
      
      Now we first expand the set of refs, then unshallow the repo in a
      separate fetch. Empirically, this seems to work, whereas the opposite
      order does not.
      
      ¹https://github.com/git/git/blob/4c86140027f4a0d2caaa3ab4bd8bfc5ce3c11c8a/transport.c#L1303-L1309
      
      Fixes #34266
      
      Change-Id: Ie97eb7c1223f944003a1e31d0ec9e69aad0efc0d
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196961
      Run-TryBot: Bryan C. Mills <bcmills@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      1804bbab
    • Eli Bendersky's avatar
      cmd/gofmt: fix computation of function header size · 47d27a87
      Eli Bendersky authored
      Function sizes are computed to determine whether a function
      can be kept on one line or should be split to several lines. Part of the
      computation is the function header from the FUNC token and until the
      opening { token.
      
      Prior to this change, the function header size used distance from the
      original source position of the current token, which led to issues when
      the source between FUNC and the original source position was rewritten
      (such as whitespace being collapsed). Now we take the current output
      position into account, so that header size represents the reformatted
      source rather than the original source.
      
      The following files in the Go repository are reformatted with this
      change:
      
      * strings/strings_test.go
      * cmd/compile/internal/gc/fmt.go
      
      In both cases the reformatting is minor and seems to be correct given
      the heuristic to single-line functions longer than 100 columns to
      multiple lines.
      
      Fixes #28082
      
      Change-Id: Ib737f6933e09b79e83715211421d5262b366ec93
      Reviewed-on: https://go-review.googlesource.com/c/go/+/188818
      Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarDaniel Martí <mvdan@mvdan.cc>
      Reviewed-by: default avatarEmmanuel Odeke <emm.odeke@gmail.com>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      47d27a87
    • Lynn Boger's avatar
      crypto/aes,crypto/cipher: add asm implementation for aes-gcm on ppc64le · 904fdb37
      Lynn Boger authored
      This adds an asm implementation for aes-gcm on ppc64le to improve
      performance.
      
      Results on power8:
      
      name                     old time/op    new time/op     delta
      AESGCMSeal1K-192           13.4µs ± 0%      3.7µs ± 0%    -72.48%  (p=1.000 n=1+1)
      AESGCMOpen1K-192           10.6µs ± 0%      2.9µs ± 0%    -72.97%  (p=1.000 n=1+1)
      AESGCMSign8K-192           60.2µs ± 0%      1.3µs ± 0%    -97.88%  (p=1.000 n=1+1)
      AESGCMSeal8K-192           80.5µs ± 0%     22.9µs ± 0%    -71.51%  (p=1.000 n=1+1)
      AESGCMOpen8K-192           80.5µs ± 0%     21.5µs ± 0%    -73.27%  (p=1.000 n=1+1)
      
      Change-Id: I026bd4f417095a987eda0f521004af90bc964661
      Reviewed-on: https://go-review.googlesource.com/c/go/+/191969
      Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarMichael Munday <mike.munday@ibm.com>
      904fdb37
    • Michael Fraenkel's avatar
      net/http: remove http2 connections when no longer cached · eb4e5def
      Michael Fraenkel authored
      When the http2 transport returns a NoCachedConnError, the connection
      must be removed from the idle list as well as the connections per host.
      
      Fixes #34387
      
      Change-Id: I7875c9c95e694a37a339bb04385243b49f9b20d3
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196665Reviewed-by: default avatarBrad Fitzpatrick <bradfitz@golang.org>
      Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      eb4e5def
    • Gregory Man's avatar
      cmd/go: allow -I= and -I$SYSROOT in cgo CFLAGS · c4fbaee8
      Gregory Man authored
      Current checkFlags() didn't allow any not safe charactars in arguments.
      In GCC "=" in arguments will be replaced with sysroot prefix, and used
      by users to work with different SDK versions.
      
      This CL allow to use "=" and $SYSROOT with -I argument.
      
      Fixes #34449
      
      Change-Id: I3d8b2b9d13251e454ea18e9d34a94b87c373c7b4
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196783
      Run-TryBot: Jay Conrod <jayconrod@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarJay Conrod <jayconrod@google.com>
      c4fbaee8
    • lukechampine's avatar
      crypto/ed25519: outline NewKeyFromSeed and Sign · 2dfff363
      lukechampine authored
      This allows the returned key/signature to be stack-allocated where possible.
      
      name              old time/op    new time/op    delta
      NewKeyFromSeed-4    61.8µs ± 8%    57.2µs ±11%      ~     (p=0.056 n=5+5)
      Signing-4           56.6µs ± 3%    67.8µs ±38%      ~     (p=1.000 n=5+5)
      
      name              old alloc/op   new alloc/op   delta
      NewKeyFromSeed-4     64.0B ± 0%      0.0B       -100.00%  (p=0.008 n=5+5)
      Signing-4             512B ± 0%      448B ± 0%   -12.50%  (p=0.008 n=5+5)
      
      name              old allocs/op  new allocs/op  delta
      NewKeyFromSeed-4      1.00 ± 0%      0.00       -100.00%  (p=0.008 n=5+5)
      Signing-4             6.00 ± 0%      5.00 ± 0%   -16.67%  (p=0.008 n=5+5)
      
      Change-Id: I7dc6a1b8a483c4b213f380ac7c30cefc5caca0f9
      GitHub-Last-Rev: 0dd2e0f93e9cd1410760544be638238f18fa5cd4
      GitHub-Pull-Request: golang/go#34357
      Reviewed-on: https://go-review.googlesource.com/c/go/+/195980Reviewed-by: default avatarFilippo Valsorda <filippo@golang.org>
      Run-TryBot: Filippo Valsorda <filippo@golang.org>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      2dfff363
    • Sean Chen's avatar
      runtime: update runtime2.go itab comments about sync struct · 39ab8db9
      Sean Chen authored
      `cmd/compile/internal/gc/reflect.go:/^func.dumptypestructs` was modified many times, now is  `cmd/compile/internal/gc/reflect.go:/^func.dumptabs`
      
      Change-Id: Ie949a5bee7878c998591468a04f67a8a70c61da7
      GitHub-Last-Rev: 9ecc26985ef18c8e870649b46419db0a9c72054f
      GitHub-Pull-Request: golang/go#34489
      Reviewed-on: https://go-review.googlesource.com/c/go/+/197037Reviewed-by: default avatarKeith Randall <khr@golang.org>
      39ab8db9
    • Joel Sing's avatar
      cmd/internal/obj/riscv: implement RV64I integer computational instructions · e29d276d
      Joel Sing authored
      Add support for assembling RV64I integer computational instructions.
      
      Based on the riscv-go port.
      
      Updates #27532
      
      Integer Computational Instructions (RV64I)
      
      Change-Id: I1a082b3901c997da309d737d081f57ea2821bc62
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196838Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      e29d276d
    • Joel Sing's avatar
      cmd/internal/obj/riscv: add environment call/breakpoint and base counter/timer instructions · ced24542
      Joel Sing authored
      This implements assembler support for ECALL/EBREAK, along with base
      counter/timer instructions.
      
      Based on riscv-go port.
      
      Updates #27532
      
      Change-Id: I690a9fd835eeddee1fe9a5616d2b2f856d3952b8
      Reviewed-on: https://go-review.googlesource.com/c/go/+/195918Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      Run-TryBot: Cherry Zhang <cherryyz@google.com>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      ced24542
  3. 23 Sep, 2019 2 commits
    • Daniel Martí's avatar
      go/printer: never print a newline before the returned results · 211932b9
      Daniel Martí authored
      Otherwise, if one ends up with a "return result" where the two nodes are
      in separate lines, the printer would incorrectly print a naked return:
      
      	return
      		result
      
      The fix is simple - by not telling exprList what the previous position
      is, it never adds a leading linebreak. This is the same mechanism used
      for identifier lists and values, so it seems appropriate.
      
      All other exprList calls that can produce a leading linebreak don't seem
      buggy, because closing tokens such as parentheses and colons are needed
      to finish the statement.
      
      Verified that the test failed before the patch as well:
      
      	--- FAIL: TestIssue32854 (0.00s)
      	    printer_test.go:806: got "return\n\tcall()", want "return call()"
      
      Finally, verified that 'gofmt -l -w src misc' doesn't make any new
      changes, just in case we introduced any regression.
      
      Fixes #32854.
      
      Change-Id: I3384fbd711de06e742407df874c9ad85626d5d6a
      Reviewed-on: https://go-review.googlesource.com/c/go/+/184121
      Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
      TryBot-Result: Gobot Gobot <gobot@golang.org>
      Reviewed-by: default avatarRobert Griesemer <gri@golang.org>
      211932b9
    • Matthew Dempsky's avatar
      cmd/compile: clean up escape graph construction · 7eef0ca1
      Matthew Dempsky authored
      OTYPESW and ORANGE were manually creating locations and flows around
      them, which are relatively low-level graph construction primitives.
      This CL changes them to use holes like the rest of the code.
      
      Also, introduce "later" as an abstraction for assignment flows that
      don't happen right away, and which need to prevent expressions from
      being marked as "transient" (e.g., in ODEFER and ORANGE).
      
      There's no behavior change here, but this does reduce the number of
      newLoc call sites, which should help with restoring -m=2 diagnostics.
      
      Passes toolstash-check.
      
      Updates #31489.
      
      Change-Id: Ic03d4488cb5162afe8b00b12432d203027e8d7d0
      Reviewed-on: https://go-review.googlesource.com/c/go/+/196619Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
      7eef0ca1