Peter Weinberger [Wed, 26 Sep 2018 14:10:48 +0000 (10:10 -0400)]
internal/traceparser: provide parser that uses less space and parses segments of runtime trace files
Traceparser generally takes 20-30% less space than internal/trace. The only
user of these pakcages is cmd/trace, and the new package lets it handle some
trace files that were too large. The new parser will also convert segments
of the raw trace file (e.g. the last 10 seconds) to Events. Trace files from
go 1.8 and before are not supported.
Change-Id: If83fa183246db8f75182ccd3ba8df07673c0ebd0
Reviewed-on: https://go-review.googlesource.com/c/137635
Run-TryBot: Peter Weinberger <pjw@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
Martin Möhrmann [Mon, 15 Oct 2018 22:27:42 +0000 (00:27 +0200)]
runtime: use multiplication with overflow check for makeslice
This improves performance for slices with an element size larger
than 32 bytes and removes loading a value from the maxElems
array for smaller element sizes.
Martin Möhrmann [Mon, 22 Oct 2018 18:22:55 +0000 (20:22 +0200)]
runtime: use multiplication with overflow check for growslice
This improves performance for slices with an element size larger
than 32 bytes and removes loading a value from the maxElems
array for smaller element sizes.
Martin Möhrmann [Mon, 22 Oct 2018 18:45:04 +0000 (20:45 +0200)]
runtime: use multiplication with overflow check for makemap
This improves performance for maps with a bucket size
(key+value*8 bytes) larger than 32 bytes and removes loading
a value from the maxElems array for smaller bucket sizes.
name old time/op new time/op delta
MakeMap/[Byte]Byte 93.5ns ± 1% 91.8ns ± 1% -1.83% (p=0.000 n=10+10)
MakeMap/[Int]Int 134ns ± 1% 127ns ± 2% -5.61% (p=0.000 n=9+10)
Updates #21588
Change-Id: I53f77186769c4bd0f2b90f3c6c17df643b060e39
Reviewed-on: https://go-review.googlesource.com/c/143797
Run-TryBot: Martin Möhrmann <martisch@uos.de>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
ChrisALiles [Mon, 6 Aug 2018 09:50:38 +0000 (19:50 +1000)]
cmd/compile: use proved bounds to remove signed division fix-ups
prove is able to find 94 occurrences in std cmd where a divisor
can't have the value -1. The change removes
the extraneous fix-up code for these cases.
Ian Lance Taylor [Mon, 22 Oct 2018 19:56:01 +0000 (12:56 -0700)]
cmd/go: update private copy of goosList
This copies the change to goosList in CL 138115 to the private copy in
cmd/go.
The change introducing the private copy was apparently not made with
Gerrit, but can be seen at
https://github.com/golang/vgo/commit/08359e782fb601567c57f56beb540841c2416d92.
That change says "This is adapted from code in go/build and the rest
of cmd/go. At some point, we should deduplicate them."
Doing another copy for now, rather than something more complex
involving cmd/dist, pending that deduplication.
Change-Id: I9b6e1f63a3a68c002b60a9a97aa367c5cc7801c9
Reviewed-on: https://go-review.googlesource.com/c/143759
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Rob Pike [Mon, 22 Oct 2018 20:01:35 +0000 (07:01 +1100)]
encoding/gob: delete out of memory test
Now that the library allows much larger data, it can kill
machines with less memory.
Fixes #28321
Change-Id: I98e1a5fdf812fd75adfb22bf01542423de405fe2
Reviewed-on: https://go-review.googlesource.com/c/143817 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Robert Griesemer [Mon, 22 Oct 2018 21:07:36 +0000 (14:07 -0700)]
go/types: report error for invalid use of ... in parameter lists
The parser accepts ...T types in parameter lists whereever a type
is permitted; this matches the syntax and allows for more tolerant
parsing and error recovery.
go/types on the other hand assumed that the parser would report
those errors and assumed any outstanding such errors would be due
to otherwise manipulated ASTs leading to invalid ASTs.
go/types further assumed that a parameter list (a, b, c ...int)
was permitted (a couple of tests used such parameter lists).
With this CL, go/types now correctly refuses invalid parameter lists.
Fixes #28281.
Change-Id: Ib788255f7b7819fdb972c7801bb153a53ce2ddf7
Reviewed-on: https://go-review.googlesource.com/c/143857
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Mon, 22 Oct 2018 17:54:38 +0000 (10:54 -0700)]
go/types: copy embedded methods unchanged when completing interfaces
The existing code adjusted the receivers of embedded interface methods
to match the embedding interface type. That required cloning (shallow
copying) the embedded methods and destroyed their object identity in
the process. Don't do this anymore. The consequence to clients is that
they might see different methods of an interface having different
receiver types; they are always the type of the interface that explicitly
declared the method (which is what one usually would want, anyway).
Fixes #28282.
Change-Id: I2e6f1497f46affdf7510547a64601de3787367db
Reviewed-on: https://go-review.googlesource.com/c/143757 Reviewed-by: Alan Donovan <adonovan@google.com>
Current assembler saves constants in Offset which type is int64,
causing 32-bit constants have a incorrect class. This CL reclassifies
constants when opcodes are 32-bit variant, like MOVW, ANDW and
ADDW, etc. Besides, this CL encodes some constants of ADDCON class
as MOVs instructions.
This CL changes the assembler behavior as follows.
2. go assembly MOVW $0xaaaaffff, R1
previous version: treats $0xaaaaffff as VCON, encodes it as MOVW 0x994, R1 (loads it from pool).
current version: treats $0xaaaaffff as MOVCON, and encodes it into MOVW instructions.
3. go assembly MOVD $0x210000, R1
previous version: treats $0x210000 as ADDCON, loads it from pool
current version: treats $0x210000 as MOVCON, and encodes it into MOVD instructions.
Add the test cases.
1. Binary size before/after.
binary size change
pkg/linux_arm64 -1.534KB
pkg/tool/linux_arm64 -0.718KB
go -0.32KB
gofmt no change
Change-Id: If067452f5b6579ad3a2e9daa76a7ffe6fceae1bb
Reviewed-on: https://go-review.googlesource.com/c/143217
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Giovanni Bajo <rasky@develer.com>
Rob Pike [Mon, 22 Oct 2018 03:04:18 +0000 (14:04 +1100)]
doc: tweak example in Effective Go
A prior attempt at addressing the issue got bogged down in an
endless conversation around the subtleties of Read semantics.
Let's not go there.
Instead, we put the issue to bed, perhaps not in perfect comfort
but well enough, by moving a line of the example so that even
if there is a "benign" error as the issue suggests, the loop
terminates with n and err correctly set.
Fixes #27818
Change-Id: I4a32d56c9e782f17578565d90b22ce531e3d8667
Reviewed-on: https://go-review.googlesource.com/c/143677 Reviewed-by: Ian Lance Taylor <iant@golang.org>
cmd/compile: move argument stack construction to SSA generation
The goal of this change is to move work from walk to SSA,
and simplify things along the way.
This is hard to accomplish cleanly with small incremental changes,
so this large commit message aims to provide a roadmap to the diff.
High level description:
Prior to this change, walk was responsible for constructing (most of) the stack for function calls.
ascompatte gathered variadic arguments into a slice.
It also rewrote n.List from a list of arguments to a list of assignments to stack slots.
ascompatte was called multiple times to handle the receiver in a method call.
reorder1 then introduced temporaries into n.List as needed to avoid smashing the stack.
adjustargs then made extra stack space for go/defer args as needed.
Node to SSA construction evaluated all the statements in n.List,
and issued the function call, assuming that the stack was correctly constructed.
Intrinsic calls had to dig around inside n.List to extract the arguments,
since intrinsics don't use the stack to make function calls.
This change moves stack construction to the SSA construction phase.
ascompatte, now called walkParams, does all the work that ascompatte and reorder1 did.
It handles variadic arguments, inserts the method receiver if needed, and allocates temporaries.
It does not, however, make any assignments to stack slots.
Instead, it moves the function arguments to n.Rlist, leaving assignments to temporaries in n.List.
(It would be better to use Ninit instead of List; future work.)
During SSA construction, after doing all the temporary assignments in n.List,
the function arguments are assigned to stack slots by
constructing the appropriate SSA Value, using (*state).storeArg.
SSA construction also now handles adjustments for go/defer args.
This change also simplifies intrinsic calls, since we no longer need to undo walk's work.
Along the way, we simplify nodarg by pushing the fp==1 case to its callers, where it fits nicely.
Generated code differences:
There were a few optimizations applied along the way, the old way.
f(g()) was rewritten to do a block copy of function results to function arguments.
And reorder1 avoided introducing the final "save the stack" temporary in n.List.
The f(g()) block copy optimization never actually triggered; the order pass rewrote away g(), so that has been removed.
SSA optimizations mostly obviated the need for reorder1's optimization of avoiding the final temporary.
The exception was when the temporary's type was not SSA-able;
in that case, we got a Move into an autotmp and then an immediate Move onto the stack,
with the autotmp never read or used again.
This change introduces a new rewrite rule to detect such pointless double Moves
and collapse them into a single Move.
This is actually more powerful than the original optimization,
since the original optimization relied on the imprecise Node.HasCall calculation.
The other significant difference in the generated code is that the stack is now constructed
completely in SP-offset order. Prior to this change, the stack was constructed somewhat
haphazardly: first the final argument that Node.HasCall deemed to require a temporary,
then other arguments, then the method receiver, then the defer/go args.
SP-offset is probably a good default order. See future work.
There are a few minor object file size changes as a result of this change.
I investigated some regressions in early versions of this change.
One regression (in archive/tar) was the addition of a single CMPQ instruction,
which would be eliminated were this TODO from flagalloc to be done:
// TODO: Remove original instructions if they are never used.
One regression (in text/template) was an ADDQconstmodify that is now
a regular MOVQLoad+ADDQconst+MOVQStore, due to an unlucky change
in the order in which arguments are written. The argument change
order can also now be luckier, so this appears to be a wash.
All in all, though there will be minor winners and losers,
this change appears to be performance neutral.
Future work:
Move loading the result of function calls to SSA construction; eliminate OINDREGSP.
Consider pushing stack construction deeper into SSA world, perhaps in an arch-specific pass.
Among other benefits, this would make it easier to transition to a new calling convention.
This would require rethinking the handling of stack conflicts and is non-trivial.
Figure out some clean way to indicate that stack construction Stores/Moves
do not alias each other, so that subsequent passes may do things like
CSE+tighten shared stack setup, do DSE using non-first Stores, etc.
This would allow us to eliminate the minor text/template regression.
Possibly make assignments to stack slots not treated as statements by DWARF.
Denys Smirnov [Fri, 19 Oct 2018 19:04:29 +0000 (19:04 +0000)]
cmd/compile: in wasm, allocate approximately right number of locals for functions
Currently, WASM binary writer requests 16 int registers (locals) and
16 float registers for every function regardless of how many locals the
function uses.
This change counts the number of used registers and requests a number
of locals matching the highest register index. The change has no effect
on performance and neglectable binary size improvement, but it makes
WASM code more readable and easy to analyze.
Change-Id: Ic1079623c0d632b215c68482db909fa440892700
GitHub-Last-Rev: 184634fa918aff74e280904dc2efafcc80735a8b
GitHub-Pull-Request: golang/go#28116
Reviewed-on: https://go-review.googlesource.com/c/140999 Reviewed-by: Richard Musiol <neelance@gmail.com>
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Robert Griesemer [Fri, 19 Oct 2018 04:21:46 +0000 (21:21 -0700)]
go/types: collect type info for type ...T in variadic functions
Because the code type-checks T rather than ...T (and then corrects
the type to []T "manually"), it didn't automatically record the
type for the ast.Expr corresponding to ...T. Do it manually.
Fixes #28277.
Change-Id: I3d9aae310c90b01f52d189e70c48dd9007f72207
Reviewed-on: https://go-review.googlesource.com/c/143317 Reviewed-by: Alan Donovan <adonovan@google.com>
Clément Chigot [Mon, 1 Oct 2018 07:50:36 +0000 (09:50 +0200)]
runtime: port assembly files for aix/ppc64
This commit adds the change on asm_ppc64.s and tls_ppc64.s files for AIX
operating system.
R2 does not need to be set for aix/ppc64 since it should remain valid
througout Go execution, except after a call to a C function.
Moreover, g must always be saved on the tls as syscalls are made with
C functions.
Some modifications on asm_ppc64.s are due to AIX stack layout.
It also removes a useless part in asmcgocall which was done twice.
Change-Id: Ie037ab73da00562bb978f2d0f17fcdabd4a40aa2
Reviewed-on: https://go-review.googlesource.com/c/138735
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Filippo Valsorda [Wed, 17 Oct 2018 03:47:55 +0000 (23:47 -0400)]
crypto/tls: replace net.Pipe in tests with real TCP connections
crypto/tls is meant to work over network connections with buffering, not
synchronous connections, as explained in #24198. Tests based on net.Pipe
are unrealistic as reads and writes are matched one to one. Such tests
worked just thanks to the implementation details of the tls.Conn
internal buffering, and would break if for example the flush of the
first flight of the server was not entirely assimilated by the client
rawInput buffer before the client attempted to reply to the ServerHello.
Note that this might run into the Darwin network issues at #25696.
Fixed a few test races that were either hidden or synchronized by the
use of the in-memory net.Pipe.
Also, this gets us slightly more realistic benchmarks, reflecting some
syscall cost of Read and Write operations.
Seebs [Thu, 18 Oct 2018 22:00:02 +0000 (17:00 -0500)]
text/template: drop unused sortKeys function
Recent change golang.org/cl/142737 drops the only call site for the
sortKeys function. If it's not in use, it should probably not be there in
the code, lurking and preparing to bite us when someone calls that instead
of the new key sorter in fmtsort, resulting in strange inconsistencies.
Since the function isn't called, this should have no impact.
Related to, but does not fix, #21095.
Rob Pike [Thu, 18 Oct 2018 23:48:53 +0000 (10:48 +1100)]
flag: return a consistent parse error if the flag value is invalid
Return a consistently formatted error string that reports either
a parse error or a range error.
Before:
invalid boolean value "3" for -debug: strconv.ParseBool: parsing "3": invalid syntax
After:
invalid boolean value "3" for -debug: parse error
Fixes #26822
Change-Id: I60992bf23da32a4c0cf32472a8af486a3c9674ad
Reviewed-on: https://go-review.googlesource.com/c/143257 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Rob Pike [Tue, 16 Oct 2018 02:54:39 +0000 (13:54 +1100)]
fmt: print maps in key-sorted order
For easier testing, change the way maps are printed so they
appear in a consistent order between runs. Do this by printing
them in key-sorted order.
To do this, we add a package at the root, internal/fmtsort,
that implements a general mechanism for sorting map keys
regardless of their type. This is a little messy and probably
slow, but formatted printing of maps has never been fast and
is already always reflection-driven.
The rules are:
The ordering rules are more general than with Go's < operator:
- when applicable, nil compares low
- ints, floats, and strings order by <
- NaN compares less than non-NaN floats
- bool compares false before true
- complex compares real, then imag
- pointers compare by machine address
- channel values compare by machine address
- structs compare each field in turn
- arrays compare each element in turn.
- interface values compare first by reflect.Type describing the concrete type
and then by concrete value as described in the previous rules.
The new package is internal because we really do not want
everyone using this to sort things. It is slow, not general, and
only suitable for the subset of types that can be map keys.
Also use the package in text/template, which already had a
weaker version of this mechanism.
This change requires adding a dependency on sort to the fmt
package, but that isn't disruptive to the dependency tree.
Keith Randall [Mon, 15 Oct 2018 20:40:00 +0000 (13:40 -0700)]
cmd/compile: fix gdb stepping test, take 2
The fix in CL 141649 is not right, the line in question got moved,
not added. Not sure why the -u option didn't do the right thing
when preparing the diff.
Fixes #28198
Change-Id: I6d45fdbbd5a9487cc70da07ab84e090b689a57f5
Reviewed-on: https://go-review.googlesource.com/c/142298 Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
David Chase [Thu, 11 Oct 2018 21:48:33 +0000 (17:48 -0400)]
cmd/compile: attach slots to incoming params for better debugging
This change attaches a slots to the OpArg values for
incoming params, and this in turn causes location lists
to be generated for params, and that yields better
debugging, in delve and sometimes in gdb.
The parameter lifetimes could start earlier; they are in
fact defined on entry, not at the point where the OpArg is
finally mentioned. (that will be addressed in another CL)
Change-Id: Icca891e118291d260c35a14acd5bc92bb82d9e9f
Reviewed-on: https://go-review.googlesource.com/c/141697
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Ian Lance Taylor [Tue, 24 Jan 2017 21:22:21 +0000 (13:22 -0800)]
test: update issue5089.go for recent gccgo changes
As of https://golang.org/cl/43456 gccgo now gives a better error
message for this test.
Before:
fixedbugs/issue5089.go:13:1: error: redefinition of ‘bufio.Buffered’: receiver name changed
func (b *bufio.Reader) Buffered() int { // ERROR "non-local|redefinition"
^
fixedbugs/issue5089.go:11:13: note: previous definition of ‘bufio.Buffered’ was here
import "bufio" // GCCGO_ERROR "previous"
^
Now:
fixedbugs/issue5089.go:13:7: error: may not define methods on non-local type
func (b *bufio.Reader) Buffered() int { // ERROR "non-local|redefinition"
^
Change-Id: I4112ca8d91336f6369f780c1d45b8915b5e8e235
Reviewed-on: https://go-review.googlesource.com/c/130955
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Richard Musiol [Thu, 18 Oct 2018 13:53:38 +0000 (15:53 +0200)]
syscall/js: make zero js.Value represent "undefined"
This commit changes the encoding of js.Value so that the zero js.Value
represents the JavaScript value "undefined". This is what users
intuitively expect.
Specifically, the encodings of "undefined" and the number zero have
been swapped.
Rob Pike [Thu, 18 Oct 2018 02:46:54 +0000 (13:46 +1100)]
cmd/doc: make comments inside functions appear with -src
The old godoc didn't do this either, perhaps because it's a little
tricky, but it can be done using a special type from the go/printer
package. (Usually we just use go/format).
Fixes #28195.
Change-Id: Ic6d3df3953ba71128398ceaf9a133c798551b6b8
Reviewed-on: https://go-review.googlesource.com/c/143037 Reviewed-by: Robert Griesemer <gri@golang.org>
Robert Griesemer [Wed, 17 Oct 2018 21:15:54 +0000 (14:15 -0700)]
go/internal/gccgoimporter: backport from x/tools to ensure identical code
This change backports a minor modification of the x/tools version of this
code back into the std library. It simply ensures that both versions of
the code are the same and will simplify keeping them in sync down the
road.
While this is an API change, this is an internal package, so we're ok.
Updates #27891.
Change-Id: Ib153141382f727a2692ca80179ae09c4a383ba4f
Reviewed-on: https://go-review.googlesource.com/c/142894 Reviewed-by: Alan Donovan <adonovan@google.com>
Robert Griesemer [Tue, 16 Oct 2018 23:50:25 +0000 (16:50 -0700)]
spec: clarify rules for receiver base types
The spec currently provides a syntactic rule for receiver base types,
and a strict reading of those rules prohibits the use of type aliases
referring to pointer types as receiver types.
This strict interpretation breaks an assumed rule for aliases, which
is that a type literal can always be replaced by an alias denoting
that literal.
Furthermore, cmd/compile always accepted this new formulation of the
receiver type rules and so this change will simply validate what has
been implemented all along.
Fixes #27995.
Change-Id: I032289c926a4f070d6f7795431d86635fe64d907
Reviewed-on: https://go-review.googlesource.com/c/142757 Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Rob Pike <r@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Michael Anthony Knyszek [Thu, 4 Oct 2018 21:03:26 +0000 (21:03 +0000)]
runtime: use only treaps for tracking spans
Currently, mheap tracks spans in both mSpanLists and mTreaps, but
mSpanLists, while they tend to be smaller, complicate the
implementation. Here we simplify the implementation by removing
free and busy from mheap and renaming freelarge -> free and busylarge
-> busy.
This change also slightly changes the reclamation policy. Previously,
for allocations under 1MB we would attempt to find a small span of the
right size. Now, we just try to find any number of spans totaling the
right size. This may increase heap fragmentation, but that will be dealt
with using virtual memory tricks in follow-up CLs.
For #14045.
Garbage-heavy benchmarks show very little change, except what appears
to be a decrease in STW times and peak RSS.
name old STW-ns/GC new STW-ns/GC delta
Garbage/benchmem-MB=64-8 263k ±64% 217k ±24% -17.66% (p=0.028 n=25+23)
name old STW-ns/op new STW-ns/op delta
Garbage/benchmem-MB=64-8 9.39k ±65% 7.80k ±24% -16.88% (p=0.037 n=25+23)
name old peak-RSS-bytes new peak-RSS-bytes delta
Garbage/benchmem-MB=64-8 281M ± 0% 249M ± 4% -11.40% (p=0.000 n=19+18)
Filippo Valsorda [Fri, 12 Oct 2018 21:07:04 +0000 (17:07 -0400)]
crypto/tls,crypto/x509: normalize RFC references
Use the format "RFC XXXX, Section X.X" (or "Appendix Y.X") as it fits
more properly in prose than a link, is more future-proof, and as there
are multiple ways to render an RFC. Capital "S" to follow the quoting
standard of RFCs themselves.
Applied the new goimports grouping to all files in those packages, too.
Change-Id: I01267bb3a3b02664f8f822e97b129075bb14d404
Reviewed-on: https://go-review.googlesource.com/c/141918 Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Ensure that label redefinition error column numbers
print the actual start of the label instead of the
position of the label's delimiting token ":".
For example, given this program:
package main
func main() {
foo:
foo:
foo:
foo :
}
* Before:
main.go:5:13: label foo defined and not used
main.go:6:7: label foo already defined at main.go:5:13
main.go:7:4: label foo already defined at main.go:5:13
main.go:8:16: label foo already defined at main.go:5:13
* After:
main.go:5:13: label foo defined and not used
main.go:6:4: label foo already defined at main.go:5:13
main.go:7:1: label foo already defined at main.go:5:13
main.go:8:1: label foo already defined at main.go:5:13
Currently all inlining of autogenerated wrappers is disabled,
because it causes build failures, when indexed export format is enabled.
Turns out we can reenable it for common case of (*T).M wrappers.
This fixes most performance degradation of 1.11 vs 1.10.
encoding/binary:
name old time/op new time/op delta
ReadSlice1000Int32s-6 14.8µs ± 2% 11.5µs ± 2% -22.01% (p=0.000 n=10+10)
WriteSlice1000Int32s-6 14.8µs ± 2% 11.7µs ± 2% -20.95% (p=0.000 n=10+10)
bufio:
name old time/op new time/op delta
WriterFlush-6 32.4ns ± 1% 28.8ns ± 0% -11.17% (p=0.000 n=9+10)
Filippo Valsorda [Tue, 16 Oct 2018 17:01:07 +0000 (13:01 -0400)]
Revert "fmt: fix incorrect format of whole-number floats when using %#v"
Numbers without decimals are valid Go representations of whole-number
floats. That is, "var x float64 = 5" is valid Go. Avoid breakage in
tests that expect a certain output from %#v by reverting to it.
To guarantee the right type is generated by a print use %T(%#v) instead.
Matthew Dempsky [Tue, 16 Oct 2018 19:49:17 +0000 (12:49 -0700)]
cmd/compile: remove -dolinkobj flag
This used to be used by cmd/vet and some assembly generation tests, but
those were removed in CL 37691 and CL 107336. No point in keeping an
unneeded flag around.
There are only a handful of nodes that we need to pass to
typecheckdef (OLITERAL, ONAME, OTYPE, and ONONAME), but typecheck1
takes the awkward approach of calling typecheckdef on every node with
Sym != nil, and then excluding a long list of uninteresting Ops that
have a non-nil Sym.
The JSON package should error, as "a" is not a valid integer. However,
we'd encounter a panic:
panic: JSON decoder out of sync - data changing underfoot?
The reason was that decodeState.object would return a nil error on
encountering the invalid map key string, while saving the key type error
for later. This broke if we were inside another object, as we would
abruptly end parsing the nested object, leaving the decoder in an
unexpected state.
To fix this, simply avoid storing the map element and continue decoding
the object, to leave the decoder state exactly as if we hadn't seen an
invalid key type.
This affected both signed and unsigned integer keys, so fix both and add
two test cases.
Updates #28189.
Change-Id: I8a6204cc3ff9fb04ed769df7a20a824c8b94faff
Reviewed-on: https://go-review.googlesource.com/c/142518 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Ben Shi [Tue, 16 Oct 2018 04:02:03 +0000 (04:02 +0000)]
test/codegen: fix confusing test cases
ARMv7's MULAF/MULSF/MULAD/MULSD are not fused,
this CL fixes the confusing test cases.
Change-Id: I35022e207e2f0d24a23a7f6f188e41ba8eee9886
Reviewed-on: https://go-review.googlesource.com/c/142439
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Akhil Indurti <aindurti@gmail.com> Reviewed-by: Giovanni Bajo <rasky@develer.com>
Daniel Martí [Sun, 7 Oct 2018 11:45:56 +0000 (12:45 +0100)]
cmd/compile: don't panic on invalid map key declarations
In golang.org/cl/75310, the compiler's typechecker was changed so that
map key types were validated at a later stage, to make sure that all the
necessary type information was present.
This still worked for map type declarations, but caused a regression for
top-level map variable declarations. These now caused a fatal panic
instead of a typechecking error.
The cause was that checkMapKeys was run too early, before all
typechecking was done. In particular, top-level map variable
declarations are typechecked as external declarations, much later than
where checkMapKeys was run.
Add a test case for both exported and unexported top-level map
declarations, and add a second call to checkMapKeys at the actual end of
typechecking. Simply moving the one call isn't a good solution either;
the comments expand on that.
Fixes #28058.
Change-Id: Ia5febb01a1d877447cf66ba44fb49a7e0f4f18a5
Reviewed-on: https://go-review.googlesource.com/c/140417
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org>
Martin Möhrmann [Fri, 12 Oct 2018 17:17:21 +0000 (19:17 +0200)]
internal/cpu: add invalid option warnings and support to enable cpu features
This CL adds the ability to enable the cpu feature FEATURE by specifying
FEATURE=on in GODEBUGCPU. Syntax support to enable cpu features is useful
in combination with a preceeding all=off to disable all but some specific
cpu features. Example:
GODEBUGCPU=all=off,sse3=on
This CL implements printing of warnings for invalid GODEBUGCPU settings:
- requests enabling features that are not supported with the current CPU
- specifying values different than 'on' or 'off' for a feature
- settings for unkown cpu feature names
Updates #27218
Change-Id: Ic13e5c4c35426a390c50eaa4bd2a408ef2ee21be
Reviewed-on: https://go-review.googlesource.com/c/141800
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Martin Möhrmann [Sun, 6 May 2018 10:19:25 +0000 (12:19 +0200)]
strconv: add comment explaining bounded shift in formatBits
The compiler can generate better code for shifts bounded to be less than 32
and thereby known to be less than any register width.
See https://golang.org/cl/109776.
Change-Id: I0c4c9f0faafa065fce3c10fd328830deb92f9e38
Reviewed-on: https://go-review.googlesource.com/c/111735
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Martin Möhrmann [Sat, 27 Jan 2018 10:55:34 +0000 (11:55 +0100)]
cmd/compile: add intrinsics for runtime/internal/math on 386 and amd64
Add generic, 386 and amd64 specific ops and SSA rules for multiplication
with overflow and branching based on overflow flags. Use these to intrinsify
runtime/internal/math.MulUinptr.
On amd64
mul, overflow := math.MulUintptr(a, b)
if overflow {
is lowered to two instructions:
MULQ SI
JO 0x10ee35c
No codegen tests as codegen can not currently test unexported internal runtime
functions.
amd64:
name old time/op new time/op delta
MulUintptr/small 1.16ns ± 5% 0.88ns ± 6% -24.36% (p=0.000 n=19+20)
MulUintptr/large 10.7ns ± 1% 1.1ns ± 1% -89.28% (p=0.000 n=17+19)
Change-Id: If60739a86f820e5044d677276c21df90d3c7a86a
Reviewed-on: https://go-review.googlesource.com/c/141820
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Martin Möhrmann [Fri, 15 Jun 2018 23:22:07 +0000 (01:22 +0200)]
cmd/compile: avoid implicit bounds checks after explicit checks for append
The generated code for the append builtin already checks if the appended
to slice is large enough and calls growslice if that is not the case.
Trust that this ensures the slice is large enough and avoid the
implicit bounds check when slicing the slice to its new size.
Removes 365 panicslice calls (-14%) from the go binary which
reduces the binary size by ~12kbyte.
Change-Id: I1b88418675ff409bc0b956853c9e95241274d5a6
Reviewed-on: https://go-review.googlesource.com/c/119315
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
Martin Möhrmann [Sat, 27 Jan 2018 10:55:34 +0000 (11:55 +0100)]
runtime/internal/math: add multiplication with overflow check
This CL adds a new internal math package for use by the runtime.
The new package exports a MulUintptr function with uintptr arguments
a and b and returns uintptr(a*b) and whether the full-width product
x*y does overflow the uintptr value range (uintptr(x*y) != x*y).
Uses of MulUinptr in the runtime and intrinsics for performance
will be added in followup CLs.
Updates #21588
Change-Id: Ia5a02eeabc955249118e4edf68c67d9fc0858058
Reviewed-on: https://go-review.googlesource.com/c/91755
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org>
The branchelim pass works better after fuse.
Running fuse before branchelim also increases
the stability of generated code amidst other compiler changes,
which was the original motivation behind this change.
The fuse pass is not cheap enough to run in its entirety
before branchelim, but the most important half of it is.
This change makes it possible to run "plain fuse" independently
and does so before branchelim.
During make.bash, elimIf occurrences increase from 4244 to 4288 (1%),
and elimIfElse occurrences increase from 989 to 1079 (9%).
Toolspeed impact is marginal; plain fuse pays for itself.
This change exposes feature flags needed to implement an FMA intrinsic
on ARM CPUs via auxv's HWCAP bits. Specifically, it exposes HasVFPv4 to
detect if an ARM processor has the fourth version of the vector floating
point unit. The relevant instruction for this CL is VFMA, emitted in Go
as FMULAD.
Updates #26630.
Change-Id: Ibbc04fb24c2b4d994f93762360f1a37bc6d83ff7
Reviewed-on: https://go-review.googlesource.com/c/126315
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Martin Möhrmann <moehrmann@google.com>
avsharapov [Sat, 13 Oct 2018 10:51:16 +0000 (13:51 +0300)]
cmd/cgo: simplify switch statement to if statement
Change-Id: Ie7dce45d554fde69d682680f55abba6a7fc55036
Reviewed-on: https://go-review.googlesource.com/c/142017 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Keith Randall [Fri, 5 Oct 2018 15:54:50 +0000 (08:54 -0700)]
cmd/compile: reuse temporaries in order pass
Instead of allocating a new temporary each time one
is needed, keep a list of temporaries which are free
(have already been VARKILLed on every path) and use
one of them.
Should save a lot of stack space. In a function like this: