Martin Möhrmann [Fri, 26 Oct 2018 19:04:45 +0000 (21:04 +0200)]
internal/cpu: replace arch dependent with generic minimal feature test
Use information about required CPU features stored in the CPU feature
options slice to test if minimal CPU requirements are met instead
of hard coding this information in the tests directly.
Change-Id: I72d89b1cff305b8e751995d4230a2217e32f4236
Reviewed-on: https://go-review.googlesource.com/c/145118 Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Martin Möhrmann <martisch@uos.de>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Filippo Valsorda [Fri, 26 Oct 2018 15:41:02 +0000 (11:41 -0400)]
crypto/tls: bump test timeouts from 1s to 1m for slow builders
The arm5 and mips builders are can't-send-a-packet-to-localhost-in-1s
slow apparently. 1m is less useful, but still better than an obscure
test timeout panic.
Keith Randall [Fri, 26 Oct 2018 17:52:59 +0000 (10:52 -0700)]
cmd/compile: fix rule for combining loads with compares
Unlike normal load+op opcodes, the load+compare opcode does
not clobber its non-load argument. Allow the load+compare merge
to happen even if the non-load argument is used elsewhere.
Noticed when investigating issue #28417.
Change-Id: Ibc48d1f2e06ae76034c59f453815d263e8ec7288
Reviewed-on: https://go-review.googlesource.com/c/145097 Reviewed-by: Ainar Garipov <gugl.zadolbal@gmail.com> Reviewed-by: Ben Shi <powerman1st@163.com>
Clément Chigot [Fri, 26 Oct 2018 08:07:36 +0000 (10:07 +0200)]
runtime: remove instruction linked with AIX new stack layout
This instruction was linked with a new stack layout which might be
needed for AIX. This change might not be taken finally. So, this
instruction must be removed.
See https://go-review.googlesource.com/c/go/+/138733
Change-Id: Ic4a2566e2882696b437eb817d980b7c4bfc03b18
Reviewed-on: https://go-review.googlesource.com/c/144957
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Bryan C. Mills [Tue, 9 Oct 2018 21:03:48 +0000 (17:03 -0400)]
cmd/go/internal/modload: use vendorMap in findModule
The build list is very incomplete in vendor mode,
so we can't rely on it in general.
findModule may be called in modload.PackageModuleInfo, which
load.LoadImport invokes relatively early during a build.
Before this change, the accompanying test failed at 'go build
-mod=vendor' with the message:
build diamondpoint: cannot find module for path diamondpoint
Bryan C. Mills [Tue, 9 Oct 2018 18:23:34 +0000 (14:23 -0400)]
cmd/go: test that 'go mod tidy' uses transitive requirements from replacements
The existing mod_tidy test uses replacements, but doesn't replace
modules that can also be resolved by fetching from GOPROXY, and
doesn't check the differences between the internal and external views.
This new test clarifies that interaction with a more realistic example.
Robert Griesemer [Wed, 24 Oct 2018 21:43:52 +0000 (14:43 -0700)]
go/types: automatically ignore $GOROOT/test files that contain build tags
These files were already ignored via a hard-coded list of excluded files.
Instead of trying to interpret the build tags for these (few) files,
recognize the tags automatically and continue to exclude them.
Fixes #10370.
Change-Id: If7a112ede02e3fa90afe303473d9ea51c5c6609d
Reviewed-on: https://go-review.googlesource.com/c/144457 Reviewed-by: Alan Donovan <adonovan@google.com>
Larry Clapp [Fri, 26 Oct 2018 01:32:27 +0000 (01:32 +0000)]
syscall/js: add the Value.Truthy method
Truthy returns the JavaScript "truthiness" of the given value. In
JavaScript, false, 0, "", null, undefined, and NaN are "falsy", and
everything else is "truthy".
With https://golang.org/cl/135455, gccgo now uses a different mangling
scheme for package paths; add code to use this new scheme for function
and variable symbols. Since users sometimes use older versions of
gccgo with newer versions of go, perform a test at runtime to see
which mangling scheme is in effect for the version of 'gccgo' in the
path.
Updates #27534.
Change-Id: If7ecab06a72e1361129fe40ca6582070a3e8e737
Reviewed-on: https://go-review.googlesource.com/c/144418 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Keith Randall [Thu, 25 Oct 2018 16:18:48 +0000 (09:18 -0700)]
cmd/compile: fix Mul->Mul64 intrinsic alias
The alias declaration needs to come after the function it is aliasing.
It isn't a big deal in this case, as bits.Mul inlines and has as its
body bits.Mul64, so the desired code gets generated regardless.
The alias should only have an effect on inlining cost estimates
(for functions that call bits.Mul).
Change-Id: I0d814899ce7049a0fb36e8ce1ad5ababbaf6265f
Reviewed-on: https://go-review.googlesource.com/c/144597
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Giovanni Bajo <rasky@develer.com>
Richard Musiol [Thu, 25 Oct 2018 21:11:10 +0000 (23:11 +0200)]
misc/wasm: improve detection of Node.js
This commit adds a check of "process.title" to detect Node.js.
The web app bundler Parcel sets "process" to an empty object. This
incorrectly got detected as Node.js, even though the script was
running in a browser.
Filippo Valsorda [Wed, 24 Oct 2018 17:16:04 +0000 (13:16 -0400)]
crypto/tls: replace custom equal implementations with reflect.DeepEqual
The equal methods were only there for testing, and I remember regularly
getting them wrong while developing tls-tris. Replace them with simple
reflect.DeepEqual calls.
The only special thing that equal() would do is ignore the difference
between a nil and a zero-length slice. Fixed the Generate methods so
that they create the same value that unmarshal will decode. The
difference is not important: it wasn't tested, all checks are
"len(slice) > 0", and all cases in which presence matters are
accompanied by a boolean.
Filippo Valsorda [Thu, 25 Oct 2018 01:31:18 +0000 (21:31 -0400)]
crypto/tls: add timeouts to recorded tests
If something causes the recorded tests to deviate from the expected
flows, they might wait forever for data that is not coming. Add a short
timeout, after which a useful error message is shown.
Clément Chigot [Thu, 25 Oct 2018 09:32:35 +0000 (11:32 +0200)]
syscall: fix TestForeground for AIX
On AIX, sys.Pgid must be a int32 and not a int64 as on Solaris for ioctl
syscall.
Pid_t type can be used to provide the same code in both OS. But pid_t
must be added to ztypes_solaris_amd64.go.
Russ Cox [Wed, 24 Oct 2018 16:01:13 +0000 (12:01 -0400)]
cmd/go, cmd/link: silence bogus Apple Xcode warning
Certain installations of Xcode are affected by a bug that causes
them to print an inconsequential link-time warning that looks like:
ld: warning: text-based stub file /System/Library/Frameworks//Security.framework/Security.tbd and library file /System/Library/Frameworks//Security.framework/Security are out of sync. Falling back to library file for linking.
This has nothing to do with Go, and we've sent this repro case
to Apple:
$ pkgutil --pkg-info=com.apple.pkg.CLTools_Executables | grep version
version: 10.0.0.0.1.1535735448
$ clang --version
Apple LLVM version 10.0.0 (clang-1000.10.44.2)
Target: x86_64-apple-darwin17.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin
$ cat > issue.c
int main() { return 0; }
^D
$ clang issue.c -framework CoreFoundation
ld: warning: text-based stub file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation.tbd and library file /System/Library/Frameworks//CoreFoundation.framework/CoreFoundation are out of sync. Falling back to library file for linking.
$
Even if Apple does release a fixed Xcode, many people are seeing
this useless warning, and we might as well make it go away.
Eugene Kalinin [Wed, 20 Jun 2018 22:23:37 +0000 (01:23 +0300)]
net: make cgo resolver work more accurately with network parameter
Unlike the go resolver, the existing cgo resolver exchanges both DNS A
and AAAA RR queries unconditionally and causes unreasonable connection
setup latencies to applications using the cgo resolver.
This change adds new argument (`network`) in all functions through the
series of calls: from Resolver.internetAddrList to cgoLookupIPCNAME.
Benefit: no redundant DNS calls if certain IP version is used IPv4/IPv6
(no `AAAA` DNS requests if used tcp4, udp4, ip4 network. And vice
versa: no `A` DNS requests if used tcp6, udp6, ip6 network)
Brad Fitzpatrick [Thu, 25 Oct 2018 02:02:57 +0000 (02:02 +0000)]
net/http: fix comment change omitted between versions of CL 143177
Updates #23689
Change-Id: Icddec2fcc39802cacd651a9c94290e86cf1e48d1
Reviewed-on: https://go-review.googlesource.com/c/144517 Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brad Fitzpatrick [Thu, 15 Mar 2018 05:21:44 +0000 (08:21 +0300)]
crypto/tls, net/http: reject HTTP requests to HTTPS server
This adds a crypto/tls.RecordHeaderError.Conn field containing the TLS
underlying net.Conn for non-TLS handshake errors, and then uses it in
the net/http Server to return plaintext HTTP 400 errors when a client
mistakenly sends a plaintext HTTP request to an HTTPS server. This is the
same behavior as Apache.
Also in crypto/tls: swap two error paths to not use a value before
it's valid, and don't send a alert record when a handshake contains a
bogus TLS record (a TLS record in response won't help a non-TLS
client).
This commit adds AIX operating system to cmd/link package for ppc64
architecture.
Updates: #25893
Change-Id: I349e0a2658c31919b72117b62c4c9935c9af07c0
Reviewed-on: https://go-review.googlesource.com/c/138730
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Tobias Klauser [Wed, 24 Oct 2018 11:32:24 +0000 (13:32 +0200)]
internal/syscall/unix: omit unnecessary randomTrap check in GetRandom
The randomTrap const is initialized to a non-zero value for linux in
getrandom_linux_$GOARCH.go and for freebsd in getrandom_freebsd.go
directly since CL 16662. Thus, omit the unnecessary check.
Change-Id: Id20cd628dfe6fab9908fa5258c3132e3b422a6b4
Reviewed-on: https://go-review.googlesource.com/c/144108
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Daniel Martí [Thu, 11 Oct 2018 14:13:22 +0000 (15:13 +0100)]
cmd/go: use os.UserCacheDir for the default GOCACHE
This piece of code predates the addition of os.UserCacheDir, and it
looks like os.UserCacheDir was based on this piece of code.
The two behaved exactly the same, minus cmd/go's addition of AppData for
Windows XP in CL 87675. However, Go 1.11 dropped support for Windows XP,
so we can safely ignore that change now.
The only tweaks necessary are to return "off" if an error is
encountered, and to disable warnings if we're using "/.cache".
Change-Id: Ia00577d4575ce4870f7fb103eafaa4d2b630743e
Reviewed-on: https://go-review.googlesource.com/c/141538
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com>
Filippo Valsorda [Wed, 17 Oct 2018 00:06:08 +0000 (20:06 -0400)]
crypto/tls: replace custom *block with standard buffers
The crypto/tls record layer used a custom buffer implementation with its
own semantics, freelist, and offset management. Replace it all with
per-task bytes.Buffer, bytes.Reader and byte slices, along with a
refactor of all the encrypt and decrypt code.
The main quirk of *block was to do a best-effort read past the record
boundary, so that if a closeNotify was waiting it would be peeked and
surfaced along with the last Read. Address that with atLeastReader and
ReadFrom to avoid a useless copy (instead of a LimitReader or CopyN).
There was also an optimization to split blocks along record boundary
lines without having to copy in and out the data. Replicate that by
aliasing c.input into consumed c.rawInput (after an in-place decrypt
operation). This is safe because c.rawInput is not used until c.input is
drained.
The benchmarks are noisy but look like an improvement across the board,
which is a nice side effect :)
Note that this reduced the buffer for the first read from 1024 to 5+512,
so it triggered the issue described at #24198 when using a synchronous
net.Pipe: the first server flight was not being consumed entirely by the
first read anymore, causing a deadlock as both the client and the server
were trying to send (the client a reply to the ServerHello, the server
the rest of the buffer). Fixed by rebasing on top of CL 142817.
Daniel Martí [Thu, 18 Oct 2018 09:53:44 +0000 (10:53 +0100)]
text/template: recover panics during function calls
There's precedent in handling panics that happen in functions called
from the standard library. For example, if a fmt.Formatter
implementation fails, fmt will absorb the panic into the output text.
Recovering panics is useful, because otherwise one would have to wrap
some Template.Execute calls with a recover. For example, if there's a
chance that the callbacks may panic, or if part of the input data is nil
when it shouldn't be.
In particular, it's a common confusion amongst new Go developers that
one can call a method on a nil receiver. Expecting text/template to
error on such a call, they encounter a long and confusing panic if the
method expects the receiver to be non-nil.
To achieve this, introduce safeCall, which takes care of handling error
returns as well as recovering panics. Handling panics in the "call"
function isn't strictly necessary, as that func itself is run via
evalCall. However, this makes the code more consistent, and can allow
for better context in panics via the "call" function.
Finally, add some test cases with a mix of funcs, methods, and func
fields that panic.
Fixes #28242.
Change-Id: Id67be22cc9ebaedeb4b17fa84e677b4b6e09ec67
Reviewed-on: https://go-review.googlesource.com/c/143097
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Rob Pike <r@golang.org>
David Chase [Thu, 18 Oct 2018 18:43:51 +0000 (14:43 -0400)]
cmd/compile: schedule OpArg earlier in blocks for better debugging
The location list for OpArg starts where the OpArg appears;
this is not necessarily as soon as the OpArg coulde be
observed, and it is reasonable for a user to expect that
if a breakpoint is set "on f" then the arguments to f will
be observable where that breakpoint happens to be set (this
may also require setting the breakpoint after the prologue,
but that is another issue).
Lynn Boger [Tue, 23 Oct 2018 16:33:56 +0000 (12:33 -0400)]
cmd/asm/internal,cmd/internal/obj/ppc64: add alignment directive to asm for ppc64x
This adds support for an alignment directive that can be used
within Go asm to indicate preferred code alignment for ppc64x.
This is intended to be used with loops to improve
performance.
This change only adds the directive and aligns the code based
on it. Follow up changes will modify asm functions for
ppc64x that benefit from preferred alignment.
Fixes #14935
Here is one example of the improvement in memmove when the
directive is used on the loops in the code:
Lynn Boger [Mon, 22 Oct 2018 20:02:43 +0000 (16:02 -0400)]
runtime: use unsigned load for iscgo variable in ppc64x runtime asm
This changes the runtime asm code that loads iscgo to use MOVBZ
instead of MOVB, avoiding an unnecessary sign extension. This is most
significant in runtime.save_g, reducing the size from 8 to 7
instructions.
Lynn Boger [Mon, 27 Aug 2018 21:15:39 +0000 (17:15 -0400)]
internal/bytealg: improve asm for memequal on ppc64x
This includes two changes to the memequal function.
Previously the asm implementation on ppc64x for Equal called the internal
function memequal using a BL, whereas the other asm implementations for
bytes functions on ppc64x used BR. The BR is preferred because the BL
causes the calling function to stack a frame. This changes Equal so it
uses BR and is consistent with the others.
This also uses vsx instructions where possible to improve performance
of the compares for sizes over 32.
Martin Möhrmann [Mon, 22 Oct 2018 18:47:54 +0000 (20:47 +0200)]
runtime: use multiplication with overflow check for newarray
This improves performance for e.g. 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 95.5ns ± 1% 94.7ns ± 1% -0.78% (p=0.013 n=9+9)
MakeMap/[Int]Int 128ns ± 0% 121ns ± 2% -5.63% (p=0.000 n=6+10)
Updates #21588
Change-Id: I7d9eb7d49150c399c15dcab675e24bc97ff97852
Reviewed-on: https://go-review.googlesource.com/c/143997 Reviewed-by: Keith Randall <khr@golang.org>
Martin Möhrmann [Mon, 22 Oct 2018 18:40:03 +0000 (20:40 +0200)]
runtime: use multiplication with overflow check for makechan
This improves performance for channels with an element size
larger than 32 bytes and removes loading a value from the
maxElems array for smaller element sizes.
Alberto Donizetti [Sat, 20 Oct 2018 18:08:57 +0000 (20:08 +0200)]
cmd/compile: preallocate in, out arrays in methodfunc
This gives a modest (but measurable) reduction in the number of
allocations when building the compilebench packages. It's safe and
exact (there's no heuristic or guessing, the lenghts of in and out are
known when we enter the function), so it may be worth it.
Clément Chigot [Tue, 23 Oct 2018 13:55:57 +0000 (15:55 +0200)]
net: fix TestInterfaceMulticastAddrs for AIX
This commit disables checkMulticastStats for AIX operating system.
Change-Id: If8d0fb609a0dcf75b7bb5c3871cfb6fad76a0a92
Reviewed-on: https://go-review.googlesource.com/c/144102
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
diplozoon [Tue, 23 Oct 2018 13:17:20 +0000 (13:17 +0000)]
cmd/go: remove unnecessary else conditions
Fixes golint warning about "if block ends with a return statement, so drop this else and outdent its block".
Change-Id: I6fc8724f586efcb6e2ed92ee36be421d3e9a8c80
Reviewed-on: https://go-review.googlesource.com/c/144137 Reviewed-by: Ralph Corderoy <ralph@inputplus.co.uk> Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Hana Kim [Tue, 23 Oct 2018 05:29:40 +0000 (01:29 -0400)]
cmd/go/internal/modload: fix use of //go:linkname
I can't find the exact rule about space before compiler directive
openings from
https://golang.org/cmd/compile/#hdr-Compiler_Directives
but it seems like the compiler doesn't recognize it
as a compiler directive if it is preceded by space.
Removing the space made the //go:linkname in the __gomod__.go file
working as intended.
Manually tested.
Update #26404
Change-Id: I589f7203a628b2fa6238d82878029e0f098091b6
Reviewed-on: https://go-review.googlesource.com/c/143977 Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>