This is still not fixed, the testcase reflects that there are still
a few boundchecks. Let's fix the good alternative with an explicit
test though.
Updates #24876
Change-Id: I4da35eb353e19052bd7b69ea6190a69ced8b9b3d
Reviewed-on: https://go-review.googlesource.com/107355
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The codegen testsuite uses regexp to parse the syntax, but it doesn't
have a way to tell line comments containing checks from line comments
containing English sentences. This means that any syntax error (that
is, non-matching regexp) is currently ignored and not reported.
There were some tests in memcombine.go that had an extraneous space
and were thus effectively disabled. It would be great if we could
report it as a syntax error, but for now we just punt and swallow the
spaces as a workaround, to avoid the same mistake again.
Fixes#25452
Change-Id: Ic7747a2278bc00adffd0c199ce40937acbbc9cf0
Reviewed-on: https://go-review.googlesource.com/113835
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Fence-post implications of the form "x-1 >= w && x > min ⇒ x > w"
were not correctly handling unsigned domain, by always checking signed
limits.
This bug was uncovered once we taught prove that len(x) is always
>= 0 in the signed domain.
In the code being miscompiled (s[len(s)-1]), prove checks
whether len(s)-1 >= len(s) in the unsigned domain; if it proves
that this is always false, it can remove the bound check.
Notice that len(s)-1 >= len(s) can be true for len(s) = 0 because
of the wrap-around, so this is something prove should not be
able to deduce.
But because of the bug, the gate condition for the fence-post
implication was len(s) > MinInt64 instead of len(s) > 0; that
condition would be good in the signed domain but not in the
unsigned domain. And since in CL105635 we taught prove that
len(s) >= 0, the condition incorrectly triggered
(len(s) >= 0 > MinInt64) and things were going downfall.
Fixes#27251Fixes#27289
Change-Id: I3dbcb1955ac5a66a0dcbee500f41e8d219409be5
Reviewed-on: https://go-review.googlesource.com/132495
Reviewed-by: Keith Randall <khr@golang.org>
Previously, pattern matching was good enough to achieve good performance
for the RotateLeft* functions, but the inlining cost for them was much
too high. Make RotateLeft* intrinsic on amd64 as a stop-gap for now to
reduce inlining costs.
This should be done (or at least looked at) for other architectures
as well.
Updates golang/go#17566
Change-Id: I6a106ff00b6c4e3f490650af3e083ed2be00c819
Reviewed-on: https://go-review.googlesource.com/132435
Run-TryBot: Andrew Bonventre <andybons@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Nil check is special in that it has no use but we must keep it.
Count it as a use of the auto.
Fixes#27278.
Change-Id: I857c3d0db2ebdca1bc342b4993c0dac5c01e067f
Reviewed-on: https://go-review.googlesource.com/131955
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This fixes the unwanted behaviour where printing a zero float with the
#v fmt verb outputs "0" - e.g. missing the trailing decimal. This means
that the output would be interpreted as an int rather than a float when
parsed as Go source. After this change the the output is "0.0".
Fixes#26363
Change-Id: Ic5c060522459cd5ce077675d47c848b22ddc34fa
GitHub-Last-Rev: adfb061363
GitHub-Pull-Request: golang/go#26383
Reviewed-on: https://go-review.googlesource.com/123956
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Rob Pike <r@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
In the compiler frontend, walkinrange indiscriminately calls Int64()
on const CTINT nodes, even though Int64's return value is undefined
for anything over 2⁶³ (in practise, it'll return a negative number).
This causes the introduction of bad constants during rewrites of
unsigned expressions, which make the compiler reject valid Go
programs.
This change introduces a preliminary check that Int64() is safe to
call on the consts on hand. If it isn't, walkinrange exits without
doing any rewrite.
Fixes#27143
Change-Id: I2017073cae65468a521ff3262d4ea8ab0d7098d9
Reviewed-on: https://go-review.googlesource.com/130735
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Tests in test/safe were neglected after moving to the run.go
framework. This change restores them.
These tests are skipped for go/types via -+ option.
Fixes#25668
Change-Id: I8fe26574a76fa7afa8664c467d7c2e6334f1bba9
Reviewed-on: https://go-review.googlesource.com/124660
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Consolidate decision about whether -race and -msan options are
supported in cmd/internal/sys. Use consolidated functions in
cmd/compile and cmd/go. Use a copy of them in cmd/dist; cmd/dist can't
import cmd/internal/sys because Go 1.4 doesn't have it.
Fixes#24315
Change-Id: I9cecaed4895eb1a2a49379b4848db40de66d32a9
Reviewed-on: https://go-review.googlesource.com/121816
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
We generate MOVBLZX for byte-sized LoadReg, so
(MOVBQZX (LoadReg (Arg))) is the same as
(LoadReg (Arg)). Remove those zero extension where possible.
Triggers several times during all.bash.
Fixes#25378
Updates #15300
Change-Id: If50656e66f217832a13ee8f49c47997f4fcc093a
Reviewed-on: https://go-review.googlesource.com/115617
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Memmove can use AVX/prefetches/other optional instructions, so
only do it for small sizes, when call overhead dominates.
Change-Id: Ice5e93deb11462217f7fb5fc350b703109bb4090
Reviewed-on: https://go-review.googlesource.com/112517
Run-TryBot: Ilya Tocar <ilya.tocar@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
This commit adds an explicit nil check for closure calls on wasm,
so calling a nil func causes a proper panic instead of crashing on the
WebAssembly level.
Change-Id: I6246844f316677976cdd420618be5664444c25ae
Reviewed-on: https://go-review.googlesource.com/127759
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Some combined store optimization was already implemented
in go-1.11, but there is no corresponding test cases.
Change-Id: Iebdad186e92047942e53a74f2c20b390922e1e9c
Reviewed-on: https://go-review.googlesource.com/122915
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If we're compiling a large function, be more picky about how big
the function we're inlining is. If the function is >5000 nodes,
we lower the inlining threshold from a cost of 80 to 20.
Turns out reflect.Value's cost is exactly 80. That's the function
at issue in #26546.
20 was chosen as a proxy for "inlined body is smaller than the call would be".
Simple functions still get inlined, like this one at cost 7:
func ifaceIndir(t *rtype) bool {
return t.kind&kindDirectIface == 0
}
5000 nodes was chosen as the big function size. Here are all the
5000+ node (~~1000+ lines) functions in the stdlib:
5187 cmd/internal/obj/arm (*ctxt5).asmout
6879 cmd/internal/obj/s390x (*ctxtz).asmout
6567 cmd/internal/obj/ppc64 (*ctxt9).asmout
9643 cmd/internal/obj/arm64 (*ctxt7).asmout
5042 cmd/internal/obj/x86 (*AsmBuf).doasm
8768 cmd/compile/internal/ssa rewriteBlockAMD64
8878 cmd/compile/internal/ssa rewriteBlockARM
8344 cmd/compile/internal/ssa rewriteValueARM64_OpARM64OR_20
7916 cmd/compile/internal/ssa rewriteValueARM64_OpARM64OR_30
5427 cmd/compile/internal/ssa rewriteBlockARM64
5126 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_50
6152 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_60
6412 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_70
6486 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_80
6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_90
6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_100
6534 cmd/compile/internal/ssa rewriteValuePPC64_OpPPC64OR_110
6675 cmd/compile/internal/gc typecheck1
5433 cmd/compile/internal/gc walkexpr
14070 cmd/vendor/golang.org/x/arch/arm64/arm64asm decodeArg
There are a lot more smaller (~1000 node) functions in the stdlib.
The function in #26546 has 12477 nodes.
At some point it might be nice to have a better heuristic for "inlined
body is smaller than the call", a non-cliff way to scale down the cost
as the function gets bigger, doing cheaper inlined calls first, etc.
All that can wait for another release. I'd like to do this CL for
1.11.
Fixes#26546
Update #17566
Change-Id: Idda13020e46ec2b28d79a17217f44b189f8139ac
Reviewed-on: https://go-review.googlesource.com/125516
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Gccgo produced incorrect order of evaluation for expressions
involving &&, || subexpressions. The fix is CL 125299.
Updates #26495.
Change-Id: I18d873281709f3160b3e09f0b2e46f5c120e1cab
Reviewed-on: https://go-review.googlesource.com/125301
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Code fix was in CL 122556. This is a corresponding test case.
Fixes#26426
Change-Id: Ib8769f367aed8bead029da0a8d2ddccee1d1dccb
Reviewed-on: https://go-review.googlesource.com/124535
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
If one tries to use promoted fields in a struct literal, the compiler
errors correctly. However, if the embedded fields are of struct pointer
type, the field.Type.Sym.Name expression below panics.
This is because field.Type.Sym is nil in that case. We can simply use
field.Sym.Name in this piece of code though, as it only concerns
embedded fields, in which case what we are after is the field name.
Added a test mirroring fixedbugs/issue23609.go, but with pointer types.
Fixes#26416.
Change-Id: Ia46ce62995c9e1653f315accb99d592aff2f285e
Reviewed-on: https://go-review.googlesource.com/124395
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The arm64 backend generates "TST" for "if uint32(a)&uint32(b) == 0",
which should be "TSTW".
fixes#26438
Change-Id: I7d64c30e3a840b43486bcd10eea2e3e75aaa4857
Reviewed-on: https://go-review.googlesource.com/124637
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Autos must be kept if their address reaches the control value of a
block. We didn't see this before because it is rare for an auto's
address to reach a control value without also reaching a phi or
being written to memory. We can probably optimize away the
comparisons that lead to this scenario since autos cannot alias
with pointers from elsewhere, however for now we take the
conservative approach and just ensure the auto is properly
initialised if its address reaches a control value.
Fixes#26407.
Change-Id: I02265793f010a9e001c3e1a5397c290c6769d4de
Reviewed-on: https://go-review.googlesource.com/124335
Reviewed-by: David Chase <drchase@google.com>
If both branches of a write barrier test go to the same block,
then there's no unsafe points.
This can only happen if the resulting memory state is somehow dead,
which can only occur in degenerate cases, like infinite loops. No
point in cleaning up the useless branch in these situations.
Fixes#26024.
Change-Id: I93a7df9fdf2fc94c6c4b1fe61180dc4fd4a0871f
Reviewed-on: https://go-review.googlesource.com/123655
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Lack of a well-defined order between VarDef and related
address operations sometimes causes problems with store order
and write barrier transformations; glitches in the order are
made irreparable (by later optimizations) if the two parts of
the glitch straddle a split in the original block caused by
insertion of a write barrier diamond.
Fix this by creating a LocalAddr for addresses of locals
(what VarDef matters for) that takes a memory input to
help make the order explicit. Addr is modified to only
be legal for SB operand, so there is no overlap between
Addr and LocalAddr uses (there may be some downstream
cleanup from this).
Changes to generic.rules and rewrite.go ensure that codegen
tests continue to pass; CSE of LocalAddr is impaired, not
quite sure of the cost.
Fixes#26105.
Change-Id: Id4192b4440aa4e9d7ba54a465c456df9b530b515
Reviewed-on: https://go-review.googlesource.com/122483
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
For golang.org/cl/74110, I forgot that you can use range-based for
loops to extract key values from a map value.
This wasn't a problem for the binary format importer, because it was
more tolerant about missing inline function bodies. However, the
indexed importer is more particular about this.
We could potentially just make it more lenient like the binary
importer, but tweaking the logic here is easy enough and seems like
the preferable solution.
Fixes#26341.
Change-Id: I54564dcd0be60ea393f8a0f6954b7d3d61e96ee5
Reviewed-on: https://go-review.googlesource.com/123475
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Fix the panic message produced for an interface conversion error to
only say "types from different packages" if they are definitely from
different packges. If they may be from the same package, say "types
from different scopes."
Updates #18911Fixes#26094
Change-Id: I0cea50ba31007d88e70c067b4680009ede69bab9
Reviewed-on: https://go-review.googlesource.com/123395
Reviewed-by: Austin Clements <austin@google.com>
Functions exported on behalf of other packages need to have their
argument stack maps specified explicitly. They don't get an implicit
map because they are not in the local package, and if they get defer'd
they need argument maps.
Fixes#24419
Change-Id: I35b7d8b4a03d4770ba88699e1007cb3fcb5397a9
Reviewed-on: https://go-review.googlesource.com/122676
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
More test cases of combined load for arm64.
Change-Id: I7a9f4dcec6930f161cbded1f47dbf7fcef1db4f1
Reviewed-on: https://go-review.googlesource.com/122582
Run-TryBot: Ben Shi <powerman1st@163.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
When DWARF is disabled, some alg functions were not generated.
Make sure they are generated when we about to generate calls to
them.
Fixes#23546.
Change-Id: Iecfa0eea830e42ee92e55268167cefb1540980b2
Reviewed-on: https://go-review.googlesource.com/122403
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
We need to make sure that the terminating comparison has the right
sense given the increment direction. If the increment is positive,
the terminating comparsion must be < or <=. If the increment is
negative, the terminating comparison must be > or >=.
Do a few cleanups, like constant-folding entry==0, adding comments,
removing unused "exported" fields.
Fixes#26116
Change-Id: I14230ee8126054b750e2a1f2b18eb8f09873dbd5
Reviewed-on: https://go-review.googlesource.com/121940
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Expanding interface method sets is handled during width calculation,
which can't be performed concurrently. Make sure that we eagerly
expand interfaces in the frontend when importing them, even if they're
not actually used by code, because we might need to generate a type
description of them.
Fixes#25055.
Change-Id: I6fd2756de2c7d5dbc33056f70b3028ca3aebab41
Reviewed-on: https://go-review.googlesource.com/122517
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Late opt pass may generate dead stores, which messes up store
chain calculation in later passes. Run generic deadcode even
in -N mode to remove them.
Fixes#26163.
Change-Id: I8276101717bb978d5980e6c7998f53fd8d0ae10f
Reviewed-on: https://go-review.googlesource.com/121856
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
If the address of an auto reaches a phi then any further stores to
the pointer represented by the phi probably need to be kept. This
is because stores to the other arguments to the phi may be visible
to the program.
Fixes#26153.
Change-Id: Ic506c6c543bf70d792e5b1a64bdde1e5fdf1126a
Reviewed-on: https://go-review.googlesource.com/121796
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This reverts commit 1a27f048ad.
Reason for revert: Broke the ssacheck and -N-l builders, and the -N-l fix looks like it will take some time and take a different route entirely.
Change-Id: Ie0ac5e86ab7d72a303dfbbc48dfdf1e092d4f61a
Reviewed-on: https://go-review.googlesource.com/121715
Reviewed-by: David Chase <drchase@google.com>
Given a carefully constructed input, writebarrier would
split a block with the OpAddr in the first half and the
VarDef in the second half which ultimately leads to a
compiler crash because the scheduler is no longer able
to put them in the proper order.
To fix, recognize the implicit dependence of OpAddr on
the VarDef of the same symbol if any exists.
This fix was chosen over making OpAddr take a memory
operand to make the dependence explicit, because this
change is less invasive at this late part of the 1.11
release cycle.
Fixes#26105.
Change-Id: I9b65460673af3af41740ef877d2fca91acd336bc
Reviewed-on: https://go-review.googlesource.com/121436
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
