The cast is proceeded by a bounds check. If the bounds check passes
then we know the pointer in the slice is non-nil.
... except casts to pointers of 0-sized arrays. They are strange, as
the bounds check can pass for a nil input.
Change-Id: Ic01cf4a82d59fbe3071d4b271c94efca9cafaec1
Reviewed-on: https://go-review.googlesource.com/c/go/+/479335
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Convert the scheduling pass from scheduling backwards to scheduling forwards.
Forward scheduling makes it easier to prioritize scheduling values as
soon as they are ready, which is important for things like nil checks,
select ops, etc.
Forward scheduling is also quite a bit clearer. It was originally
backwards because computing uses is tricky, but I found a way to do it
simply and with n lg n complexity. The new scheme also makes it easy
to add new scheduling edges if needed.
Fixes#42673
Update #56568
Change-Id: Ibbb38c52d191f50ce7a94f8c1cbd3cd9b614ea8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/270940
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Currently, during walk we rewrite PAUTOHEAP uses into derefs of their
corresponding Heapaddr, but we can easily do this instead during SSA
construction. This does involve updating two test cases:
* nilptr3.go
This file had a test that we emit a "removed nil check" diagnostic for
the implicit dereference from accessing a PAUTOHEAP variable. This CL
removes this diagnostic, since it's not really useful to end users:
from the user's point of view, there's no pointer anyway, so they
needn't care about whether we check for nil or not. That's a purely
internal detail. And with the PAUTOHEAP dereference handled during SSA
construction, we can more robustly ensure this happens, rather than
relying on setting a flag in walk and hoping that SSA sees it.
* issue20780.go
Previously, when PAUTOHEAPs were dereferenced during walk, it had a
consequence that when they're passed as a function call argument, they
would first get copied to the stack before being copied to their
actual destination. Moving the dereferencing to SSA had a side-effect
of eliminating this unnecessary temporary, and copying directly to the
destination parameter.
The test is updated to instead call "g(h(), h())" where h() returns a
large value, as the first result will always need to be spilled
somewhere will calling the second function. Maybe eventually we're
smart enough to realize it can be spilled to the heap, but we don't do
that today.
Because I'm concerned that the direct copy-to-parameter optimization
could interfere with race-detector instrumentation (e.g., maybe the
copies were previously necessary to ensure they're not clobbered by
inserted raceread calls?), I've also added issue20780b.go to exercise
this in a few different ways.
Change-Id: I720598cb32b17518bc10a03e555620c0f25fd28d
Reviewed-on: https://go-review.googlesource.com/c/go/+/281293
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This commit adapts compile tool to create correct nilchecks for AIX.
AIX allows to load a nil pointer. Therefore, the default nilcheck
which issues a load must be replaced by a CMP instruction followed by a
store at 0x0 if the value is nil. The store will trigger a SIGSEGV as on
others OS.
The nilcheck algorithm must be adapted to do not remove nilcheck if it's
only a read. Stores are detected with v.Type.IsMemory().
Tests related to nilptr must be adapted to the previous changements.
nilptr.go cannot be used as it's because the AIX address space starts at
1<<32.
Change-Id: I9f5aaf0b7e185d736a9b119c0ed2fe4e5bd1e7af
Reviewed-on: https://go-review.googlesource.com/c/144538
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Removes unnecessary nil-check when referencing offset from an
address. Suggested by Keith Randall in golang/go#27180.
Updates golang/go#27180
Change-Id: I326ed7fda7cfa98b7e4354c811900707fee26021
Reviewed-on: https://go-review.googlesource.com/131735
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The general policy for the current state of js/wasm is that it only
has to support tests that are also supported by nacl.
The test nilptr3.go makes assumptions about which nil checks can be
removed. Since WebAssembly does not signal on reading a null pointer,
all nil checks have to be explicit.
Updates #18892
Change-Id: I06a687860b8d22ae26b1c391499c0f5183e4c485
Reviewed-on: https://go-review.googlesource.com/110096
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For code like the following (where x escapes):
x := []int{1}
We're currently generating a nil check. The line above is really 3 operations:
t := new([1]int)
t[0] = 1
x := t[:]
We remove the nil check for t[0] = 1, but not for t[:].
Our current nil check removal rule is too strict about the possible
memory arguments of the nil check. Unlike zeroing or storing to the
result of runtime.newobject, the nilness of runtime.newobject is
always false, even after other stores have happened in the meantime.
Change-Id: I95fad4e3a59c27effdb37c43ea215e18f30b1e5f
Reviewed-on: https://go-review.googlesource.com/58711
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Nil check removal in the same block is disabled due to issue 18725:
because the values are not ordered, a nilcheck may influence a
value that is logically before it. This CL re-enables same-block
nilcheck removal by ordering values in store order first.
Updates #18725.
Change-Id: I287a38525230c14c5412cbcdbc422547dabd54f6
Reviewed-on: https://go-review.googlesource.com/35496
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
When nilcheck runs, the values in a block are not in any particular
order. So any facts derived from examining the blocks shouldn't be
used until we reach the next block.
This is suboptimal as it won't eliminate nil checks within a block.
But it's probably a better fix for now as it is a much smaller change
than other strategies for fixing this bug.
nilptr3.go changes are mostly because for this pattern:
_ = *p
_ = *p
either nil check is fine to keep, and this CL changes which one
the compiler tends to keep.
There are a few regressions from code like this:
_ = *p
f()
_ = *p
For this pattern, after this CL we issue 2 nil checks instead of one.
(For the curious, this happens because intra-block nil check
elimination now falls to CSE, not nilcheck proper. The former
pattern has two nil checks with the same store argument. The latter
pattern has two nil checks with different store arguments.)
Fixes#18725
Change-Id: I3721b494c8bc9ba1142dc5c4361ea55c66920ac8
Reviewed-on: https://go-review.googlesource.com/35485
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Mark nil check operations as faulting if their arg is zero.
This lets the late nilcheck pass remove duplicates.
Fixes#17242.
Change-Id: I4c9938d8a5a1e43edd85b4a66f0b34004860bcd9
Reviewed-on: https://go-review.googlesource.com/29952
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Add more ARM64 optimizations:
- use hardware zero register when it is possible.
- use shifted ops.
The assembler supports shifted ops but not documented, nor knows
how to print it. This CL adds them.
- enable fast division.
This was disabled because it makes the old backend generate slower
code. But with SSA it generates faster code.
Turn on SSA by default, also adjust tests.
Change-Id: I7794479954c83bb65008dcb457bc1e21d7496da6
Reviewed-on: https://go-review.googlesource.com/26950
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Last part of the 386 SSA port.
Modify the x86 backend to simulate SSE registers and
instructions with 387 registers and instructions.
The simulation isn't terribly performant, but it works,
and the old implementation wasn't very performant either.
Leaving to people who care about 387 to optimize if they want.
Turn on SSA backend for 386 by default.
Fixes#16358
Change-Id: I678fb59132620b2c47e993c1c10c4c21135f70c0
Reviewed-on: https://go-review.googlesource.com/25271
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
It's not a new backend, just a PtrSize==4 modification
of the existing AMD64 backend.
Change-Id: Icc63521a5cf4ebb379f7430ef3f070894c09afda
Reviewed-on: https://go-review.googlesource.com/25586
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
NaCl code runs in sandbox and there are restrictions for its
instruction uses
(https://developer.chrome.com/native-client/reference/sandbox_internals/arm-32-bit-sandbox).
Like the legacy backend, on NaCl,
- don't use R9, which is used as NaCl's "thread pointer".
- don't use Duff's device.
- don't use indexed load/stores.
- the assembler rewrites DIV/MOD to runtime calls, which on NaCl
clobbers R12, so R12 is marked as clobbered for DIV/MOD.
- other restrictions are satisfied by the assembler.
Enable SSA specific tests on nacl/arm, and disable non-SSA ones.
Updates #15365.
Change-Id: I9262693ec6756b89ca29d3ae4e52a96fe5403b02
Reviewed-on: https://go-review.googlesource.com/24859
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
As Josh mentioned in CL 24716, there has been requests for using SSA
for ARM. SSA can still be disabled by setting -ssa=0 for cmd/compile,
or partially enabled with GOSSAFUNC, GOSSAPKG, and GOSSAHASH.
Not enable SSA by default on NaCl, which is not supported yet.
Enable SSA-specific tests on ARM: live_ssa.go and nilptr3_ssa.go;
disable non-SSA tests: live.go, nilptr3.go, and slicepot.go.
Updates #15365.
Change-Id: Ic2ca8d166aeca8517b9d262a55e92f2130683a16
Reviewed-on: https://go-review.googlesource.com/23953
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
They are guaranteed to be non-nil, no point in inserting
nil checks for them.
Fixes#15390
Change-Id: I3b9a0f2319affc2139dcc446d0a56c6785ae5a86
Reviewed-on: https://go-review.googlesource.com/22291
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Fails for the same reason as ppc64 and mips64 (incomplete
optimization).
Change-Id: Ieb4d997fc27d4f2b756e63dd7f588abe10c0213a
Reviewed-on: https://go-review.googlesource.com/20963
Reviewed-by: Bill O'Farrell <billotosyr@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
non-SSA backends are all over the map as to whether nil checks
get removed or not. amd64, 386, 386/387, arm are all subtly different.
Remove these extra checks for now, they are in nilptr3_ssa.go so they
won't get lost.
Change-Id: I2e0051f488fb2cb7278c6fdd44cb9d68b5778345
Reviewed-on: https://go-review.googlesource.com/19961
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Just like we do for integer loads/stores.
Update #14511
Change-Id: Ic6ca6b54301438a5701ea5fb0be755451cb24d45
Reviewed-on: https://go-review.googlesource.com/19923
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
Skip fixedbugs/issue10607.go because external linking is not supported
yet.
Skip nilptr3.go because of issue #9058 (same as ppc64).
Change-Id: Ib3dfbd9a03ee4052871cf57c74b3cc5e745e1f80
Reviewed-on: https://go-review.googlesource.com/14461
Reviewed-by: Minux Ma <minux@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Some tests disabled, some bifurcated into _ssa and not,
with appropriate logging added to compiler.
"tests/live.go" in particular needs attention.
SSA-specific testing removed, since it's all SSA now.
Added "-run_skips" option to tests/run.go to simplify
checking whether a test still fails (or how it fails)
on a skipped platform.
The compiler now compiles with SSA by default.
If you don't want SSA, specify GOSSAHASH=n (or N) as
an environment variable. Function names ending in "_ssa"
are always SSA-compiled.
GOSSAFUNC=fname retains its "SSA for fname, log to ssa.html"
GOSSAPKG=pkg only has an effect when GOSSAHASH=n
GOSSAHASH=10101 etc retains its name-hash-matching behavior
for purposes of debugging.
See #13068
Change-Id: I8217bfeb34173533eaeb391b5f6935483c7d6b43
Reviewed-on: https://go-review.googlesource.com/16299
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
The remaining failures in this test are because of incomplete
optimization support on power64x. Tracked in issue 9058.
LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/168130043
Globals, function arguments, and results are special cases in
registerization.
Globals must be flushed aggressively, because nearly any
operation can cause a panic, and the recovery code must see
the latest values. Globals also must be loaded aggressively,
because nearly any store through a pointer might be updating a
global: the compiler cannot see all the "address of"
operations on globals, especially exported globals. To
accomplish this, mark all globals as having their address
taken, which effectively disables registerization.
If a function contains a defer statement, the function results
must be flushed aggressively, because nearly any operation can
cause a panic, and the deferred code may call recover, causing
the original function to return the current values of its
function results. To accomplish this, mark all function
results as having their address taken if the function contains
any defer statements. This causes not just aggressive flushing
but also aggressive loading. The aggressive loading is
overkill but the best we can do in the current code.
Function arguments must be considered live at all safe points
in a function, because garbage collection always preserves
them: they must be up-to-date in order to be preserved
correctly. Accomplish this by marking them live at all call
sites. An earlier attempt at this marked function arguments as
having their address taken, which disabled registerization
completely, making programs slower. This CL's solution allows
registerization while preserving safety. The benchmark speedup
is caused by being able to registerize again (the earlier CL
lost the same amount).
benchmark old ns/op new ns/op delta
BenchmarkEqualPort32 61.4 56.0 -8.79%
benchmark old MB/s new MB/s speedup
BenchmarkEqualPort32 521.56 570.97 1.09x
Fixes#1304. (again)
Fixes#7944. (again)
Fixes#7984.
Fixes#7995.
LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, r
https://golang.org/cl/97500044
This eliminates ~75% of the nil checks being emitted,
on all architectures. We can do better, but we need
a bit more general support from the compiler, and
I don't want to do that so close to Go 1.2.
What's here is simple but effective and safe.
A few small code generation cleanups were required
to make the analysis consistent on all systems about
which nil checks are omitted, at least in the test.
Fixes#6019.
R=ken2
CC=golang-dev
https://golang.org/cl/13334052
