Calling .Interface on a struct field's reflect.Value isn't always safe.
For example, if that field is an unexported anonymous struct.
We only descended into this branch if the struct type had any methods,
so this bug had gone unnoticed for a few release cycles.
Add the check, and add a simple test case.
Fixes#28145.
Change-Id: I02f7e0ab9a4a0c18a5e2164211922fe9c3d30f64
Reviewed-on: https://go-review.googlesource.com/c/141537
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Given a program as follows:
data := []byte(`{"F": {
"a": 2,
"3": 4
}}`)
json.Unmarshal(data, &map[string]map[int]int{})
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>
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>
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>
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>
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>
x = map[string(byteslice)] is already optimized by the compiler to avoid a
string allocation. This CL generalizes this optimization to:
x = map[T1{ ... Tn{..., string(byteslice), ...} ... }]
where T1 to Tn is a nesting of struct and array literals.
Found in a hot code path that used a struct of strings made from []byte
slices to make a map lookup.
There are no uses of the more generalized optimization in the standard library.
Passes toolstash -cmp.
MapStringConversion/32/simple 21.9ns ± 2% 21.9ns ± 3% ~ (p=0.995 n=17+20)
MapStringConversion/32/struct 28.8ns ± 3% 22.0ns ± 2% -23.80% (p=0.000 n=20+20)
MapStringConversion/32/array 28.5ns ± 2% 21.9ns ± 2% -23.14% (p=0.000 n=19+16)
MapStringConversion/64/simple 21.0ns ± 2% 21.1ns ± 3% ~ (p=0.072 n=19+18)
MapStringConversion/64/struct 72.4ns ± 3% 21.3ns ± 2% -70.53% (p=0.000 n=20+20)
MapStringConversion/64/array 72.8ns ± 1% 21.0ns ± 2% -71.13% (p=0.000 n=17+19)
name old allocs/op new allocs/op delta
MapStringConversion/32/simple 0.00 0.00 ~ (all equal)
MapStringConversion/32/struct 0.00 0.00 ~ (all equal)
MapStringConversion/32/array 0.00 0.00 ~ (all equal)
MapStringConversion/64/simple 0.00 0.00 ~ (all equal)
MapStringConversion/64/struct 1.00 ± 0% 0.00 -100.00% (p=0.000 n=20+20)
MapStringConversion/64/array 1.00 ± 0% 0.00 -100.00% (p=0.000 n=20+20)
Change-Id: I483b4d84d8d74b1025b62c954da9a365e79b7a3a
Reviewed-on: https://go-review.googlesource.com/c/116275
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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>
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>
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>
This change adds codegen tests for the intrinsification on ppc64 of
the OnesCount{64,32,16,8}, and TrailingZeros{64,32,16,8} math/bits
functions.
Change-Id: Id3364921fbd18316850e15c8c71330c906187fdb
Reviewed-on: https://go-review.googlesource.com/c/141897
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Ensure that we correctly type the stack temps for regular closures,
method function closures, and slice literals.
Then we don't need to override the dummy types later.
Furthermore, this allows order to reuse temporaries of these types.
OARRAYLIT doesn't need a temporary as far as I can tell, so I
removed that case from order.
Change-Id: Ic58520fa50c90639393ff78f33d3c831d5c4acb9
Reviewed-on: https://go-review.googlesource.com/c/140306
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Not sure why this changed behavior, but seems mostly harmless.
Fixes#28198
Change-Id: Ie25c6e1fcb64912a582c7ae7bf92c4c1642e83cb
Reviewed-on: https://go-review.googlesource.com/c/141649
Reviewed-by: David Chase <drchase@google.com>
This CL adds tests of fused multiplication-accumulation
on arm/arm64.
Change-Id: Ic85d5277c0d6acb7e1e723653372dfaf96824a39
Reviewed-on: https://go-review.googlesource.com/c/141652
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
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>
Merge the two for loops that set up the node lists for
temporaries into one for loop.
Passes toolstash -cmp
Change-Id: Ibc739115f38c8869b0dcfbf9819fdc2fc96962e0
Reviewed-on: https://go-review.googlesource.com/c/141819
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
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:
func main() {
fmt.Printf("%d %d\n", 2, 3)
fmt.Printf("%d %d\n", 4, 5)
fmt.Printf("%d %d\n", 6, 7)
}
The three [2]interface{} arrays used to hold the ... args
all use the same autotmp, instead of 3 different autotmps
as happened previous to this CL.
Change-Id: I2d728e226f81e05ae68ca8247af62014a1b032d3
Reviewed-on: https://go-review.googlesource.com/c/140301
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When we pass these types by reference, we usually have to allocate
temporaries on the stack, initialize them, then pass their address
to the conversion functions. It's simpler to pass these types
directly by value.
This particularly applies to conversions needed for fmt.Printf
(to interface{} for constructing a [...]interface{}).
func f(a, b, c string) {
fmt.Printf("%s %s\n", a, b)
fmt.Printf("%s %s\n", b, c)
}
This function's stack frame shrinks from 200 to 136 bytes, and
its code shrinks from 535 to 453 bytes.
The go binary shrinks 0.3%.
Update #24286
Aside: for this function f, we don't really need to allocate
temporaries for the convT2E function. We could use the address
of a, b, and c directly. That might get similar (or maybe better?)
improvements. I investigated a bit, but it seemed complicated
to do it safely. This change was much easier.
Change-Id: I78cbe51b501fb41e1e324ce4203f0de56a1db82d
Reviewed-on: https://go-review.googlesource.com/c/135377
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Instead of
MOVB go.string."foo"(SB), AX
do
MOVB $102, AX
When we know the global we're loading from is readonly, we can
do that read at compile time.
I've made this arch-dependent mostly because the cases where this
happens often are memory->memory moves, and those don't get
decomposed until lowering.
Did amd64/386/arm/arm64. Other architectures could follow.
Update #26498
Change-Id: I41b1dc831b2cd0a52dac9b97f4f4457888a46389
Reviewed-on: https://go-review.googlesource.com/c/141118
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This CL makes sure we walk the newly generated assignment. Part of
that walk makes sure that all symbols for strings are emitted before
we start referencing them during the parallel compilation
phase. Without this change, those references during the parallel phase
do a create-if-not-exist, which leads to a data race.
I'm not 100% sure this is the fix for the issues below, but optimistically
assuming it is...
Fixes#28170Fixes#28159
Change-Id: Ic63d5160ad9be5cb23fa6bbb2183e4848776c0ff
Reviewed-on: https://go-review.googlesource.com/c/141648
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This flag lost its usefulness in CL 34273.
Change-Id: I033c29f105937139b4e359a340906be439f1ed07
Reviewed-on: https://go-review.googlesource.com/c/141646
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
When running benchmarks with profilers and trying to
compare one run against another, it is very useful to be
able to force each run to execute exactly the same number
of iterations.
Discussion on the proposal issue #24735 led to the decision
to overload -benchtime, so that instead of saying
-benchtime 10s to run a benchmark for 10 seconds,
you say -benchtime 100x to run a benchmark 100 times.
Fixes#24735.
Change-Id: Id17c5bd18bd09987bb48ed12420d61ae9e200fd7
Reviewed-on: https://go-review.googlesource.com/c/139258
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The original need for the extra test case and issue was eliminated
by https://golang.org/cl/116815 which introduced systematic cycle
detection. Now that we correctly report the cycle, we can't say much
about the invalid cast anyway (the type is invalid due to the cycle).
A more sophisticated approach would be able to tell the size of
a function type independent of the details of that type, but the
type-checker is not set up for this kind of lazy type-checking.
Fixes#23127.
Change-Id: Ia8479e66baf630ce96f6f36770c8e1c810c59ddc
Reviewed-on: https://go-review.googlesource.com/c/141640
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Follow-up for CL 138717. This fixes the build of the os package on
aix.
Change-Id: I879b9360e71837ab622ae3a7b6144782cf5a9ce7
Reviewed-on: https://go-review.googlesource.com/c/141797
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Enabling GODEBUGCPU without the need to set GOEXPERIMENT=debugcpu enables
trybots and builders to run tests for GODEBUGCPU features in upcoming CLs
that will implement the new syntax and features for non-experimental
GODEBUGCPU support from proposal golang.org/issue/27218.
Updates #27218
Change-Id: Icc69e51e736711a86b02b46bd441ffc28423beba
Reviewed-on: https://go-review.googlesource.com/c/141817
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We weren't writing a terminating NUL after dstDirent.Namlen bytes of dstDirent.Name.
And we weren't filling the possible additional bytes until dstDirent.Reclen.
Fixes#28131
Change-Id: Id691c25225795c0dbb0d7004bfca7bb7fc706de9
Reviewed-on: https://go-review.googlesource.com/c/141297
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The UserHomeDir test succeeds on the builder, but not when run
manually where HOME is set to the host $HOME.
Change-Id: I1db0f608b04b311b53cc0c8160a3778caaf542f6
Reviewed-on: https://go-review.googlesource.com/c/141798
Run-TryBot: Elias Naur <elias.naur@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Interface and string comparisons don't need separate Ops any more than
struct or array comparisons do.
Removing them requires shuffling some code around in walk (and a
little in order), but overall allows simplifying things a bit.
Passes toolstash-check.
Change-Id: I084b8a6c089b768dc76d220379f4daed8a35db15
Reviewed-on: https://go-review.googlesource.com/c/141637
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Per the spec, "...the next token is the longest sequence of characters
that form a valid token." Thus, encountering a ".." sequence should
return two token.PERIOD tokens rather than a single token.ILLEGAL.
Fixes#28112.
Change-Id: Iba5da841f40036e53f48f9be23f933f362e67f5e
Reviewed-on: https://go-review.googlesource.com/c/141337
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
