When a loop has bound len(s)-delta, findIndVar detected it and
returned len(s) as (conservative) upper bound. This little lie
allowed loopbce to drop bound checks.
It is obviously more generic to teach prove about relations like
x+d<w for non-constant "w"; we already handled the case for
constant "w", so we just want to learn that if d<0, then x+d<w
proves that x<w.
To be able to remove the code from findIndVar, we also need
to teach prove that len() and cap() are always non-negative.
This CL allows to prove 633 more checks in cmd+std. Most
of them are cases where the code was already testing before
accessing a slice but the compiler didn't know it. For instance,
take strings.HasSuffix:
func HasSuffix(s, suffix string) bool {
return len(s) >= len(suffix) && s[len(s)-len(suffix):] == suffix
}
When suffix is a literal string, the compiler now understands
that the explicit check is enough to not emit a slice check.
I also found a loopbce test that was incorrectly
written to detect an overflow but had a off-by-one (on the
conservative side), so it unexpectly passed with this CL; I
changed it to really trigger the overflow as intended.
Change-Id: Ib5abade337db46b8811425afebad4719b6e46c4a
Reviewed-on: https://go-review.googlesource.com/105635
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
To be effective, this also requires being able to relax constraints
on min/max bound inclusiveness; they are now exposed through a flags,
and prove has been updated to handle it correctly.
Change-Id: I3490e54461b7b9de8bc4ae40d3b5e2fa2d9f0556
Reviewed-on: https://go-review.googlesource.com/104041
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Test both minimum and maximum bound, and prepare
formatting for more advanced tests (inclusive / esclusive bounds).
Change-Id: Ibe432916d9c938343bc07943798bc9709ad71845
Reviewed-on: https://go-review.googlesource.com/104040
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reuse findIndVar to discover induction variables, and then
register the facts we know about them into the facts table
when entering the loop block.
Moreover, handle "x+delta > w" while updating the facts table,
to be able to prove accesses to slices with constant offsets
such as slice[i-10].
Change-Id: I2a63d050ed58258136d54712ac7015b25c893d71
Reviewed-on: https://go-review.googlesource.com/104038
Run-TryBot: Giovanni Bajo <rasky@develer.com>
Reviewed-by: David Chase <drchase@google.com>
When a branch is followed, we apply the relation as described
in the domain relation table. In case the relation is in the
positive domain, we can also infer an unsigned relation if,
by that point, we know that both operands are non-negative.
Fixes#20393
Change-Id: Ieaf0c81558b36d96616abae3eb834c788dd278d5
Reviewed-on: https://go-review.googlesource.com/100278
Run-TryBot: Giovanni Bajo <rasky@develer.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Giovanni Bajo <rasky@develer.com>
Reviewed-by: David Chase <drchase@google.com>
There are several things combined in this change.
First, eliminate the gobitvector type in favor
of adding a ptrbit method to bitvector.
In non-performance-critical code, use that method.
In performance critical code, though, load the bitvector data
one byte at a time and iterate only over set bits.
To support that, add and use sys.Ctz8.
name old time/op new time/op delta
StackCopyPtr-8 81.8ms ± 5% 78.9ms ± 3% -3.58% (p=0.000 n=97+96)
StackCopy-8 65.9ms ± 3% 62.8ms ± 3% -4.67% (p=0.000 n=96+92)
StackCopyNoCache-8 105ms ± 3% 102ms ± 3% -3.38% (p=0.000 n=96+95)
Change-Id: I00b80f45612708bd440b1a411a57fa6dfa24aa74
Reviewed-on: https://go-review.googlesource.com/109716
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
getArgInfo is called a lot during stack copying.
In the common case it doesn't do much work,
but it cannot be inlined.
This change works around that.
name old time/op new time/op delta
StackCopyPtr-8 108ms ± 5% 96ms ± 4% -10.40% (p=0.000 n=20+20)
StackCopy-8 82.6ms ± 3% 78.4ms ± 6% -5.15% (p=0.000 n=19+20)
StackCopyNoCache-8 130ms ± 3% 122ms ± 3% -6.44% (p=0.000 n=20+20)
Change-Id: If7d8a08c50a4e2e76e4331b399396c5dbe88c2ce
Reviewed-on: https://go-review.googlesource.com/108945
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Currently, when the runtime looks up the stack map for a frame, it
uses frame.continpc - 1 unless continpc is the function entry PC, in
which case it uses frame.continpc. As a result, if continpc is the
function entry point (which happens for deferred frames), it will
actually look up the stack map *following* the first instruction.
I think, though I am not positive, that this is always okay today
because the first instruction of a function can never change the stack
map. It's usually not a CALL, so it doesn't have PCDATA. Or, if it is
a CALL, it has to have the entry stack map.
But we're about to start emitting stack maps at every instruction that
changes them, which means the first instruction can have PCDATA
(notably, in leaf functions that don't have a prologue).
To prepare for this, tweak how the runtime looks up stack map indexes
so that if continpc is the function entry point, it directly uses the
entry stack map.
For #24543.
Change-Id: I85aa818041cd26aff416f7b1fba186e9c8ca6568
Reviewed-on: https://go-review.googlesource.com/109349
Reviewed-by: Rick Hudson <rlh@golang.org>
This instruction was introduced on the z14 to accelerate "limbified"
multiplications for certain cryptographic algorithms. This change allows
it to be used in Go assembly.
Change-Id: Ic93dae7fec1756f662874c08a5abc435bce9dd9e
Reviewed-on: https://go-review.googlesource.com/109695
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Current optab entries are unordered, because the new instructions
are added at the end of the optab. The patch reorders them by comments
in optab, such as arithmetic operations, logical operations and a
series of load/store etc.
The patch removes the VMOVS opcode because FMOVS already has the same
operation.
Change-Id: Iccdf89ecbb3875b9dfcb6e06be2cc19c7e5581a2
Reviewed-on: https://go-review.googlesource.com/109896
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Return a non-zero exit code if the WebAssembly host fails to compile
the WebAssmbly bytecode to machine code.
Change-Id: I774309db2872b6a2de77a1b0392608058414160d
Reviewed-on: https://go-review.googlesource.com/110097
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The init function and runtime.addmoduledata were not added when
building plugin, which caused the runtime could not find the
module.
Testplugin is still not enabled on linux/arm64
(https://go.googlesource.com/go/+/master/src/cmd/dist/test.go#948)
because the gold linker on the builder is too old, which fails
with an internal error (see issue #17138). I tested locally and
it passes.
Fixes#24940.
Updates #17138.
Change-Id: I26aebca6c38a3443af0949471fa12b6d550e8c6c
Reviewed-on: https://go-review.googlesource.com/109917
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
mips64 softfloat support is based on mips implementation and introduces
new enviroment variable GOMIPS64.
GOMIPS64 is a GOARCH=mips64{,le} specific option, for a choice between
hard-float and soft-float. Valid values are 'hardfloat' (default) and
'softfloat'. It is passed to the assembler as
'GOMIPS64_{hardfloat,softfloat}'.
Change-Id: I7f73078627f7cb37c588a38fb5c997fe09c56134
Reviewed-on: https://go-review.googlesource.com/108475
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
As a follow-up to the first README for cmd/compile/internal/ssa.
Since this is the parent package for all the compiler packages, this
README serves as an overview of the compiler and its packages. As more
READMEs are added for specific parts with more detail, such as ssa's,
they can be linked from this one.
Thanks to Iskander Sharipov, Josh Bleecher Snyder, Matthew Dempsky,
Alberto Donizetti, and Robert Griesemer for helping with all the details
in this document.
Change-Id: I820a535e25dce86ccc667ce1c6e92b75fc32f3af
Reviewed-on: https://go-review.googlesource.com/103935
Reviewed-by: Martin Möhrmann <moehrmann@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If the vetted function supplies zero arguments, previously you would
get an error message like this:
Printf format %v reads arg #1, but call has only 0 args
"has only 0 args" is an odd construction, and "has 0 args" sounds
better. Getting rid of "only" in all cases simplifies the code and
reads just as well.
Change-Id: I4706dfe4a75f13bf4db9c0650e459ca676710752
Reviewed-on: https://go-review.googlesource.com/109457
Run-TryBot: Kevin Burke <kev@inburke.com>
Run-TryBot: David Symonds <dsymonds@golang.org>
Reviewed-by: David Symonds <dsymonds@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Simplify some C-style loops with range statements, and move some
declarations closer to their uses.
While at it, ensure that all the SymbolType consts are typed.
Change-Id: I04b06afb2c1fb249ef8093a0c5cca0a597d1e05c
Reviewed-on: https://go-review.googlesource.com/105217
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Using oldname+resolve is how typecheck handles this anyway.
Passes toolstash -cmp, with both -iexport enabled and disabled.
Change-Id: I12b0f0333d6b86ce6bfc4d416c461b5f15c1110d
Reviewed-on: https://go-review.googlesource.com/109715
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Getcallerpc/sp no longer takes argument.
Change-Id: I80b30020e798990c59c8ffd0a4e078af6a75aea0
Reviewed-on: https://go-review.googlesource.com/109696
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Also change tasks to be represented as "slices" instead of
asynchronous events which are more efficiently represented in trace
viewer data model. This change allows to utilize the flow events
(arrows) to represent task hierarchies.
Introduced RegionArgs and TaskArgs where the task id infomation and
goroutine id informations are stored for information-purpose.
Change-Id: I11bec7dd716fdfc5f94ea39661b2e51344367a6f
Reviewed-on: https://go-review.googlesource.com/109337
Reviewed-by: Heschi Kreinick <heschi@google.com>
getcallersp is intrinsified, and so the dummy arg is no longer
needed. Remove it, as well as a few dummy args that are solely
to feed getcallersp.
Change-Id: Ibb6c948ff9c56537042b380ac3be3a91b247aaa6
Reviewed-on: https://go-review.googlesource.com/109596
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The example in the 'A web server' section of the effective Go document
uses Google's image charts API (at chart.apis.google.com).
The service is now deprecated (see developers.google.com/chart/image),
and visiting http://chart.apis.google.com gives a 404. The endpoint is
still active, so the Go code in the example still works, but there's
no point in making the link clickable by the user if the page returns
a 404.
Change the element to `<code>`.
Change-Id: Ie67f4723cfa636e3dc1460507055b6bbb2b0970c
Reviewed-on: https://go-review.googlesource.com/109576
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
'%SystemRoot%/System32/chcp.com' is a tool on Windows that
is used to change the active code page in the console.
'go test os/exec' can fail with:
"'chcp' is not recognized as an internal or external command"
The test uses a custom PATH variable but does not include
'%SystemRoot%/System32'. Always append that to PATH.
Updates #24709
Change-Id: I1ab83b326072e3f0086b391b836234bcfd8a1fb7
GitHub-Last-Rev: fb930529bb
GitHub-Pull-Request: golang/go#25088
Reviewed-on: https://go-review.googlesource.com/109361
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
'%SystemRoot%/System32/chcp.com' is a tool on Windows that
is used to change the active code page in the console.
'go test path/filepath' can fail with:
"'chcp' is not recognized as an internal or external command"
The test uses a custom PATH variable but does not include
'%SystemRoot%/System32'. Always append that to PATH.
Updates #24709
Change-Id: Ib4c83ffdcc5dd6eb7bb34c07386cf2ab61dcae57
GitHub-Last-Rev: fac92613cc
GitHub-Pull-Request: golang/go#25089
Reviewed-on: https://go-review.googlesource.com/109362
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
On amd64, Ctz must include special handling of zeros.
But the prove pass has enough information to detect whether the input
is non-zero, allowing a more efficient lowering.
Introduce new CtzNonZero ops to capture and use this information.
Benchmark code:
func BenchmarkVisitBits(b *testing.B) {
b.Run("8", func(b *testing.B) {
for i := 0; i < b.N; i++ {
x := uint8(0xff)
for x != 0 {
sink = bits.TrailingZeros8(x)
x &= x - 1
}
}
})
// and similarly so for 16, 32, 64
}
name old time/op new time/op delta
VisitBits/8-8 7.27ns ± 4% 5.58ns ± 4% -23.35% (p=0.000 n=28+26)
VisitBits/16-8 14.7ns ± 7% 10.5ns ± 4% -28.43% (p=0.000 n=30+28)
VisitBits/32-8 27.6ns ± 8% 19.3ns ± 3% -30.14% (p=0.000 n=30+26)
VisitBits/64-8 44.0ns ±11% 38.0ns ± 5% -13.48% (p=0.000 n=30+30)
Fixes#25077
Change-Id: Ie6e5bd86baf39ee8a4ca7cadcf56d934e047f957
Reviewed-on: https://go-review.googlesource.com/109358
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Running 'go run *.go' in the gen directory resulted in this diff.
Change-Id: Iee398a720f54d3f2c3c122fc6fc45a708a39e45e
Reviewed-on: https://go-review.googlesource.com/109575
Run-TryBot: Michael Munday <mike.munday@ibm.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When creating a directory, Writer.Create now returns a dummy
io.Writer that always returns an error on Write.
Fixes#24043
Change-Id: I7792f54440d45d22d0aa174cba5015ed5fab1c5c
Reviewed-on: https://go-review.googlesource.com/108615
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Run-TryBot: Joe Tsai <thebrokentoaster@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
NewReplacer's documentation says that "replacements are performed in
order", meaning that substrings are replaced in the order they appear
in the target string, and not that the old->new replacements are
applied in the order they're passed to NewReplacer.
Rephrase the doc to make this clearer.
Fixes#25071
Change-Id: Icf3aa6a9d459b94764c9d577e4a76ad8c04d158d
Reviewed-on: https://go-review.googlesource.com/109375
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Doesn't cause an error, see #25085.
But we should fix it nonetheless.
Change-Id: I7b6799e0a95475202cacefc3a7f02487e61bfd31
Reviewed-on: https://go-review.googlesource.com/109355
Reviewed-by: Heschi Kreinick <heschi@google.com>
This change implements math.Round as an intrinsic on ppc64x so it can be
done using a single instruction.
benchmark old ns/op new ns/op delta
BenchmarkRound-16 2.60 0.69 -73.46%
Change-Id: I9408363e96201abdfc73ced7bcd5f0c29db006a8
Reviewed-on: https://go-review.googlesource.com/109395
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
When we moved racewalk to buildssa, we disabled SSA of structs with
2-4 fields while instrumenting because it caused false dependencies
because for "x.f" we would emit
(StructSelect (Load (Addr x)) "f")
Even though we later simplify this to
(Load (OffPtr (Addr x) "f"))
the instrumentation saw a load of x in its entirety and would issue
appropriate race/msan calls.
The fix taken here is to directly emit the OffPtr form when x.f is
addressable and can't be represented in SSA form.
Change-Id: I0caf37bced52e9c16937466b0ac8cab6d356e525
Reviewed-on: https://go-review.googlesource.com/109360
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This is a followup for CL 93156.
Fixes#22942.
Change-Id: Ic6e2de44011d041b91454353a6f2e3b0cf590060
Reviewed-on: https://go-review.googlesource.com/108095
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Now that #18395 is fixed, let's see if we can insist
on vet during go test being able to type-check
packages again.
Change-Id: Iaa55a4d9c582ba743df2347d28c24f130e16e406
Reviewed-on: https://go-review.googlesource.com/108555
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Now that we can tell when a package is a split-off copy
for testing, show that in the build failures.
For example, instead of:
# regexp/syntax
../../regexp/syntax/parse.go:9:2: can't find import: "strings"
# path/filepath
../../path/filepath/match.go:12:2: can't find import: "strings"
# flag
../../flag/flag.go:75:2: can't find import: "strings"
we now print
# regexp/syntax [strings.test]
../../regexp/syntax/parse.go:9:2: can't find import: "strings"
# path/filepath [strings.test]
../../path/filepath/match.go:12:2: can't find import: "strings"
# flag [strings.test]
../../flag/flag.go:75:2: can't find import: "strings"
which gives more of a hint about what is wrong.
This is especially helpful if a package is being built multiple times,
since it explains why an error might appear multiple times:
$ go test regexp encoding/json
# regexp
../../regexp/exec.go:12:9: undefined: x
# regexp [regexp.test]
../../regexp/exec.go:12:9: undefined: x
FAIL regexp [build failed]
FAIL encoding/json [build failed]
$
Change-Id: Ie325796f6c3cf0e23f306066be8e65a30cb6b939
Reviewed-on: https://go-review.googlesource.com/108155
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Tools should be able to ask cmd/go about the dependency
graph for test binaries instead of reinventing it themselves.
Allow them to do so, with the new list -test flag.
This also fixes and tests for a bug introduced in CL 104315
that was not properly splitting dependencies on the path
between package main and the package being tested.
Change-Id: I29eb454c82893f5ee70252aaaecd9fa376eaf3c8
Reviewed-on: https://go-review.googlesource.com/107916
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
The previous change sped up the pure computation form of LeadingZeros8.
This places it somewhat close to the table lookup form.
Depending on something that varies from toolchain to toolchain
(alignment, perhaps?), the slowdown from ditching the table lookup
is either 20% or 5%.
This benchmark is the best case scenario for the table lookup:
It is in the L1 cache already.
I think we're close enough that we can switch to the computational version,
and trust that the memory effects and binary size savings will be worth it.
Code:
func f8(x uint8) { z = bits.LeadingZeros8(x) }
Before:
"".f8 STEXT nosplit size=34 args=0x8 locals=0x0
0x0000 00000 (x.go:7) TEXT "".f8(SB), NOSPLIT, $0-8
0x0000 00000 (x.go:7) FUNCDATA $0, gclocals·2a5305abe05176240e61b8620e19a815(SB)
0x0000 00000 (x.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
0x0000 00000 (x.go:7) MOVBLZX "".x+8(SP), AX
0x0005 00005 (x.go:7) MOVBLZX AL, AX
0x0008 00008 (x.go:7) LEAQ math/bits.len8tab(SB), CX
0x000f 00015 (x.go:7) MOVBLZX (CX)(AX*1), AX
0x0013 00019 (x.go:7) ADDQ $-8, AX
0x0017 00023 (x.go:7) NEGQ AX
0x001a 00026 (x.go:7) MOVQ AX, "".z(SB)
0x0021 00033 (x.go:7) RET
After:
"".f8 STEXT nosplit size=30 args=0x8 locals=0x0
0x0000 00000 (x.go:7) TEXT "".f8(SB), NOSPLIT, $0-8
0x0000 00000 (x.go:7) FUNCDATA $0, gclocals·2a5305abe05176240e61b8620e19a815(SB)
0x0000 00000 (x.go:7) FUNCDATA $1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
0x0000 00000 (x.go:7) MOVBLZX "".x+8(SP), AX
0x0005 00005 (x.go:7) MOVBLZX AL, AX
0x0008 00008 (x.go:7) LEAL 1(AX)(AX*1), AX
0x000c 00012 (x.go:7) BSRL AX, AX
0x000f 00015 (x.go:7) ADDQ $-8, AX
0x0013 00019 (x.go:7) NEGQ AX
0x0016 00022 (x.go:7) MOVQ AX, "".z(SB)
0x001d 00029 (x.go:7) RET
Change-Id: Icc7db50a7820fb9a3da8a816d6b6940d7f8e193e
Reviewed-on: https://go-review.googlesource.com/108942
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
