pointers to go:notinheap types should be treated as scalars. That
means they shouldn't be stored directly in interfaces, or directly
in reflect.Value.ptr.
Also be sure to use uintpr to compare such pointers in reflect.DeepEqual.
Fixes#42076
Change-Id: I53735f6d434e9c3108d4940bd1bae14c61ef2a74
Reviewed-on: https://go-review.googlesource.com/c/go/+/264480
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
storeType splits compound stores up into a scalar parts and a pointer parts.
The scalar part happens unconditionally, and the pointer part happens
under the guard of a write barrier check.
Types which are declared as pointers, but are represented as scalars because
they might have "bad" values, were not handled correctly here. They ended
up not getting stored in either set.
Fixes#42032
Change-Id: I46f6600075c0c370e640b807066247237f93c7ac
Reviewed-on: https://go-review.googlesource.com/c/go/+/264300
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Combine (AND m (SRWconst x)) or (SRWconst (AND m x)) when mask m is
and the shift value produce constant which can be encoded into an
RLWINM instruction.
Combine (CLRLSLDI (SRWconst x)) if the combining of the underling rotate
masks produces a constant which can be encoded into RLWINM.
Likewise for (SLDconst (SRWconst x)) and (CLRLSDI (RLWINM x)).
Combine rotate word + and operations which can be encoded as a single
RLWINM/RLWNM instruction.
The most notable performance improvements arise from the crypto
benchmarks below (GOARCH=power8 on a ppc64le/linux):
pkg:golang.org/x/crypto/blowfish goos:linux goarch:ppc64le
ExpandKeyWithSalt 52.2µs ± 0% 47.5µs ± 0% -8.88%
ExpandKey 44.4µs ± 0% 40.3µs ± 0% -9.15%
pkg:golang.org/x/crypto/ssh/internal/bcrypt_pbkdf goos:linux goarch:ppc64le
Key 57.6ms ± 0% 52.3ms ± 0% -9.13%
pkg:golang.org/x/crypto/bcrypt goos:linux goarch:ppc64le
Equal 90.9ms ± 0% 82.6ms ± 0% -9.13%
DefaultCost 91.0ms ± 0% 82.7ms ± 0% -9.12%
Change-Id: I59a0ca29face38f4ab46e37124c32906f216c4ce
Reviewed-on: https://go-review.googlesource.com/c/go/+/260798
Run-TryBot: Carlos Eduardo Seo <carlos.seo@linaro.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Carlos Eduardo Seo <carlos.seo@linaro.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
Currently, "x &^ y" gets rewriten into "x & ^y" during walk. It adds
unnecessary complexity to other parts, which must aware about this.
Instead, we can just implement "&^" in the conversion to SSA, so "&^"
can be handled like other binary operators.
However, this CL does not pass toolstash-check. It seems that implements
"&^" in the conversion to SSA causes registers allocation change.
With the parent:
obj: 00212 (.../src/runtime/complex.go:47) MOVQ X0, AX
obj: 00213 (.../src/runtime/complex.go:47) BTRQ $63, AX
obj: 00214 (.../src/runtime/complex.go:47) MOVQ "".n(SP), CX
obj: 00215 (.../src/runtime/complex.go:47) MOVQ $-9223372036854775808, DX
obj: 00216 (.../src/runtime/complex.go:47) ANDQ DX, CX
obj: 00217 (.../src/runtime/complex.go:47) ORQ AX, CX
With this CL:
obj: 00212 (.../src/runtime/complex.go:47) MOVQ X0, AX
obj: 00213 (.../src/runtime/complex.go:47) BTRQ $63, AX
obj: 00214 (.../src/runtime/complex.go:47) MOVQ $-9223372036854775808, CX
obj: 00215 (.../src/runtime/complex.go:47) MOVQ "".n(SP), DX
obj: 00216 (.../src/runtime/complex.go:47) ANDQ CX, DX
obj: 00217 (.../src/runtime/complex.go:47) ORQ AX, DX
Change-Id: I80acf8496a91be4804fb7ef3df04c19baae2754c
Reviewed-on: https://go-review.googlesource.com/c/go/+/264660
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For follow up CL, which will defer lowering OANDNOT until SSA.
Change-Id: I5a988d0b8f0ae664580f08b123811b2a31ef55c6
Reviewed-on: https://go-review.googlesource.com/c/go/+/265040
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
We never supported symbol larger than 2GB (issue #9862), so the
object file uses 32-bit for symbol sizes. Check and reject too
large symbol before truncating its size.
Fixes#42054.
Change-Id: I0d1d585ebdba9556f2fd3a97043bd4296d5cc9e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/263641
Trust: Cherry Zhang <cherryyz@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
This helps the compiler reports the right place where the type declared,
instead of relying on global lineno, which maybe set to wrong value at
the time the error is reported.
Fixes#42058
Change-Id: I06d34aa9b0236d122f4a0d72e66675ded022baac
Reviewed-on: https://go-review.googlesource.com/c/go/+/263597
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
OCALLPART is exported in its original form, which is as an OXDOT.
The body of the method value wrapper created in makepartialcall() was
not being typechecked, and that was causing a problem during escape
analysis, so I added code to typecheck the body.
The go executable got slightly bigger with this change (13598111 ->
13598905), because of extra exported methods with OCALLPART (I
believe), while the text size got slightly smaller (9686964 ->
9686643).
This is mainly part of the work to make sure all function bodies can
be exported (for purposes of generics), but might as well fix the
OCALLPART inlining bug as well.
Fixes#18493
Change-Id: If7aa055ff78ed7a6330c6a1e22f836ec567d04fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/263620
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
asNode(t.Nod).Name.Param will be nil for builtin types (i.e., the
universal predeclared types and unsafe.Pointer). These types can't be
part of a cycle anyway, so we can just skip them.
Fixes#42075.
Change-Id: Ic7a44de65c6bfd16936545dee25e36de8850acf3
Reviewed-on: https://go-review.googlesource.com/c/go/+/263717
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Before generating wrapper function, turn any f(a, b, []T{c, d, e}...)
calls back into f(a, b, c, d, e). This allows the existing code for
recognizing and specially handling unsafe.Pointer->uintptr conversions
to correctly handle variadic arguments too.
Fixes#41460.
Change-Id: I0a1255abdd1bd5dafd3e89547aedd4aec878394c
Reviewed-on: https://go-review.googlesource.com/c/go/+/263297
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This CL replaces the ad hoc and duplicated logic for detecting
inlinable calls with a single "inlCallee" function, which uses the
"staticValue" helper function introduced in an earlier commit.
Updates #41474.
Change-Id: I103d4091b10366fce1344ef2501222b7df68f21d
Reviewed-on: https://go-review.googlesource.com/c/go/+/256460
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
We already allow inlining "if" and "goto" statements, so we might as
well allow "for" loops too. The majority of frontend support is
already there too.
The critical missing feature at the moment is that inlining doesn't
properly reassociate OLABEL nodes with their control statement (e.g.,
OFOR) after inlining. This eventually causes SSA construction to fail.
As a workaround, this CL only enables inlining for unlabeled "for"
loops. It's left to a (yet unplanned) future CL to add support for
labeled "for" loops.
The increased opportunity for inlining leads to a small growth in
binary size. For example:
$ size go.old go.new
text data bss dec hex filename
9740163 320064 230656 10290883 9d06c3 go.old
9793399 320064 230656 10344119 9dd6b7 go.new
Updates #14768.
Fixes#41474.
Change-Id: I827db0b2b9d9fa2934db05caf6baa463f0cd032a
Reviewed-on: https://go-review.googlesource.com/c/go/+/256459
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Escape analysis is currently very naive about identifying calls to
known functions: it only recognizes direct calls to a declared
function, or direct calls to a closure.
This CL adds a new "staticValue" helper function that can trace back
through local variables that were initialized and never reassigned
based on a similar optimization already used by inlining. (And to be
used by inlining in a followup CL.)
Updates #41474.
Change-Id: I8204fd3b1e150ab77a27f583985cf099a8572b2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/256458
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
The gofrontend code doesn't correctly handle inlining a function that
refers to a constant with methods.
For #35739
Change-Id: I6bd0b5cd4272dbe9969634b4821e668acacfdcf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/261662
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
We are converting from using error-prone ad-hoc syntax // +build lines
to less error-prone, standard boolean syntax //go:build lines.
The timeline is:
Go 1.16: prepare for transition
- Builds still use // +build for file selection.
- Source files may not contain //go:build without // +build.
- Builds fail when a source file contains //go:build lines without // +build lines. <<<
Go 1.17: start transition
- Builds prefer //go:build for file selection, falling back to // +build
for files containing only // +build.
- Source files may contain //go:build without // +build (but they won't build with Go 1.16).
- Gofmt moves //go:build and // +build lines to proper file locations.
- Gofmt introduces //go:build lines into files with only // +build lines.
- Go vet rejects files with mismatched //go:build and // +build lines.
Go 1.18: complete transition
- Go fix removes // +build lines, leaving behind equivalent // +build lines.
This CL provides part of the <<< marked line above in the Go 1.16 step:
rejecting files containing //go:build but not // +build.
The standard go command checks only consider the top of the file.
This compiler check, along with a separate go vet check for ignored files,
handles the remainder of the file.
For #41184.
Change-Id: I014006eebfc84ab5943de18bc90449e534f150a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/240601
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
We lost a sign extension that was necessary. The nonnegative comparison
didn't have the correct extension on it. If the larger constant is
positive, but its shorter sign extension is negative, the rule breaks.
Fixes#41872
Change-Id: I6592ef103f840fbb786bf8cb94fd8804c760c976
Reviewed-on: https://go-review.googlesource.com/c/go/+/260701
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
Backstop support for non-sse2 chips now that 387 is gone.
RELNOTE=yes
Change-Id: Ib10e69c4a3654c15a03568f93393437e1939e013
Reviewed-on: https://go-review.googlesource.com/c/go/+/260017
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Unlike iOS, macOS ARM64 is more of a fully featured OS. Enable
more tests.
Updates #38485.
Change-Id: I2e2240c848d21996db2b950a4a6856987f7a652c
Reviewed-on: https://go-review.googlesource.com/c/go/+/256919
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Two part fix:
1) bring the type "correction" forward from a later CL in the expand calls series
2) when a leaf-selwect is rewritten in place, update the type (it might have been
changed by the type correction in 1).
Fixes#41736.
Change-Id: Id097efd10481bf0ad92aaead81a7207221c144b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/259203
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
This change renames mustHeapAlloc to heapAllocReason, and changes it
to return the reason why the argument must escape, so we don't have to
re-deduce it in its callers just to print the escape reason. It also
embeds isSmallMakeSlice body in heapAllocReason, since the former was
only used by the latter, and deletes isSmallMakeSlice.
An outdated TODO to remove smallintconst, which the TODO claimed was
only used in one place, was also removed, since grepping shows we
currently call smallintconst in 11 different places.
Change-Id: I0bd11bf29b92c4126f5bb455877ff73217d5a155
Reviewed-on: https://go-review.googlesource.com/c/go/+/258678
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
My last 387 CL. So sad ... ... ... ... not!
Fixes#40255
Change-Id: I8d4ddb744b234b8adc735db2f7c3c7b6d8bbdfa4
Reviewed-on: https://go-review.googlesource.com/c/go/+/258957
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
A recent change to improve shifts was generating some
invalid cases when the rule was based on an AND. The
extended mnemonics CLRLSLDI and CLRLSLWI only allow
certain values for the operands and in the mask case
those values were not being checked properly. This
adds a check to those rules to verify that the
'b' and 'n' values used when an AND was part of the rule
have correct values.
There was a bug in some diag messages in asm9. The
message expected 3 values but only provided 2. Those are
corrected here also.
The test/codegen/shift.go was updated to add a few more
cases to check for the case mentioned here.
Some of the comments that mention the order of operands
in these extended mnemonics were wrong and those have been
corrected.
Fixes#41683.
Change-Id: If5bb860acaa5051b9e0cd80784b2868b85898c31
Reviewed-on: https://go-review.googlesource.com/c/go/+/258138
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: Paul Murphy <murp@ibm.com>
Reviewed-by: Carlos Eduardo Seo <carlos.seo@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
Similar to how we report initialization loops in initorder.go and type
alias loops in typecheck.go, this CL updates align.go to warn about
invalid recursive types. The code is based on the loop code from
initorder.go, with minimal changes to adapt from detecting
variable/function initialization loops to detecting type declaration
loops.
Thanks to Cuong Manh Le for investigating this, helping come up with
test cases, and exploring solutions.
Fixes#41575
Updates #41669.
Change-Id: Idb2cb8c5e1d645e62900e178fcb50af33e1700a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/258177
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
As part of type checking make's arguments, we were converting untyped
float and complex constant arguments to integers. However, we were
doing this without concern for whether the argument was a declared
constant. Thus a call like "make([]T, n)" could change n from an
untyped float or untyped complex to an untyped integer.
The fix here is to simply change checkmake to not call SetVal, which
will be handled by defaultlit anyway. However, we also need to
properly return the defaultlit result value to the caller, so
checkmake's *Node parameter is also changed to **Node.
Fixes#41680.
Change-Id: I858927a052f384ec38684570d37b10a6906961f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/257966
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
This adds support for the extswsli instruction which combines
extsw followed by a shift.
New benchmark demonstrates the improvement:
name old time/op new time/op delta
ExtShift 1.34µs ± 0% 1.30µs ± 0% -3.15% (p=0.057 n=4+3)
Change-Id: I21b410676fdf15d20e0cbbaa75d7c6dcd3bbb7b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/257017
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <carlos.seo@gmail.com>
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
When explaining why the slice from a make() call escapes for the -m -m
message, we print "non-const size" if any one of Isconst(n.Left) and
Isconst(n.Right) return false; but for OMAKESLICE nodes with no cap,
n.Right is nil, so Isconst(n.Right, CTINT) will be always false.
Only call Isconst on n.Right if it's not nil.
Fixes#41635
Change-Id: I8729801a9b234b68ae40adad64d66fa7653adf09
Reviewed-on: https://go-review.googlesource.com/c/go/+/257641
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
We grow the backing store on append by 2x for small sizes and 1.25x
for large sizes. The threshold we use for picking the growth factor
used to depend on the old length, not the old capacity. That's kind of
unfortunate, because then doing append(s, 0, 0) and append(append(s,
0), 0) do different things. (If s has one more spot available, then
the former expression chooses its growth based on len(s) and the
latter on len(s)+1.) If we instead use the old capacity, we get more
consistent behavior. (Both expressions use len(s)+1 == cap(s) to
decide.)
Fixes#41239
Change-Id: I40686471d256edd72ec92aef973a89b52e235d4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/257338
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
The revised test now checks that unsafe-uintptr correctly works for
variadic uintptr parameters too, and the CL corrects the code so this
code compiles again.
The pointers are still not kept alive properly. That will be fixed by
a followup CL. But this CL at least allows programs not affected by
that to build again.
Updates #24991.
Updates #41460.
Change-Id: If4c39167b6055e602213fb7522c4f527c43ebda9
Reviewed-on: https://go-review.googlesource.com/c/go/+/255877
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
//go:notinheap
type T int
type U T
We already correctly propagate the notinheap-ness of T to U. But we
have an assertion in the typechecker that if there's no explicit
//go:notinheap associated with U, then report an error. Get rid of
that error so that implicit propagation is allowed.
Adjust the tests so that we make sure that uses of types like U
do correctly report an error when U is used in a context that might
cause a Go heap allocation.
Fixes#41451
Update #40954
Update #41432
Change-Id: I1692bc7cceff21ebb3f557f3748812a40887118d
Reviewed-on: https://go-review.googlesource.com/c/go/+/255637
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
This change adds rules to find pairs of instructions that can
be combined into a single shifts. These instruction sequences
are common in array addressing within loops. Improvements can
be seen in many crypto packages and the hash packages.
These are based on the extended mnemonics found in the ISA
sections C.8.1 and C.8.2.
Some rules in PPC64.rules were moved because the ordering prevented
some matching.
The following results were generated on power9.
hash/crc32:
CRC32/poly=Koopman/size=40/align=0 195ns ± 0% 163ns ± 0% -16.41%
CRC32/poly=Koopman/size=40/align=1 200ns ± 0% 163ns ± 0% -18.50%
CRC32/poly=Koopman/size=512/align=0 1.98µs ± 0% 1.67µs ± 0% -15.46%
CRC32/poly=Koopman/size=512/align=1 1.98µs ± 0% 1.69µs ± 0% -14.80%
CRC32/poly=Koopman/size=1kB/align=0 3.90µs ± 0% 3.31µs ± 0% -15.27%
CRC32/poly=Koopman/size=1kB/align=1 3.85µs ± 0% 3.31µs ± 0% -14.15%
CRC32/poly=Koopman/size=4kB/align=0 15.3µs ± 0% 13.1µs ± 0% -14.22%
CRC32/poly=Koopman/size=4kB/align=1 15.4µs ± 0% 13.1µs ± 0% -14.79%
CRC32/poly=Koopman/size=32kB/align=0 137µs ± 0% 105µs ± 0% -23.56%
CRC32/poly=Koopman/size=32kB/align=1 137µs ± 0% 105µs ± 0% -23.53%
crypto/rc4:
RC4_128 733ns ± 0% 650ns ± 0% -11.32% (p=1.000 n=1+1)
RC4_1K 5.80µs ± 0% 5.17µs ± 0% -10.89% (p=1.000 n=1+1)
RC4_8K 45.7µs ± 0% 40.8µs ± 0% -10.73% (p=1.000 n=1+1)
crypto/sha1:
Hash8Bytes 635ns ± 0% 613ns ± 0% -3.46% (p=1.000 n=1+1)
Hash320Bytes 2.30µs ± 0% 2.18µs ± 0% -5.38% (p=1.000 n=1+1)
Hash1K 5.88µs ± 0% 5.38µs ± 0% -8.62% (p=1.000 n=1+1)
Hash8K 42.0µs ± 0% 37.9µs ± 0% -9.75% (p=1.000 n=1+1)
There are other improvements found in golang.org/x/crypto which are all in the
range of 5-15%.
Change-Id: I193471fbcf674151ffe2edab212799d9b08dfb8c
Reviewed-on: https://go-review.googlesource.com/c/go/+/252097
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
"cannot assign to" compiler errors are very laconic: they never
explain why the lhs cannot be assigned to (with one exception, when
assigning to a struct field in a map).
This change makes them a little more specific, in two more cases: when
assigning to a string, or to a const; by giving a very brief reason
why the lhs cannot be assigned to.
Change-Id: I244cca7fc3c3814e00e0ccadeec62f747c293979
Reviewed-on: https://go-review.googlesource.com/c/go/+/255199
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Update #40954
Change-Id: Ifaab7349631ccb12fc892882bbdf7f0ebf3d845f
Reviewed-on: https://go-review.googlesource.com/c/go/+/251158
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
They can't reasonably be allocated on the heap. Not a huge deal, but
it has an interesting and useful side effect.
After CL 249917, the compiler and runtime treat pointers to
go:notinheap types as uintptrs instead of real pointers (no write
barrier, not processed during stack scanning, ...). That feature is
exactly what we want for cgo to fix#40954. All the cases we have of
pointers declared in C, but which might actually be filled with
non-pointer data, are of this form (JNI's jobject heirarch, Darwin's
CFType heirarchy, ...).
Fixes#40954
Change-Id: I44a3b9bc2513d4287107e39d0cbbd0efd46a3aae
Reviewed-on: https://go-review.googlesource.com/c/go/+/250940
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The alias doesn't need to be marked go:notinheap. It gets its
notinheap-ness from the target type.
Without this change, the type alias test in the notinheap.go file
generates these two errors:
notinheap.go:62: misplaced compiler directive
notinheap.go:63: type nih must be go:notinheap
The first is a result of go:notinheap pragmas not applying
to type alias declarations.
The second is the result of then trying to match the notinheap-ness
of the alias and the target type.
Add a few more go:notinheap tests while we are here.
Update #40954
Change-Id: I067ec47698df6e9e593e080d67796fd05a1d480f
Reviewed-on: https://go-review.googlesource.com/c/go/+/250939
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Trust: Keith Randall <khr@golang.org>
This allows the global initializers function to go through normal
mid-end optimizations (e.g., inlining, escape analysis) like any other
function.
Updates #33485.
Change-Id: I9bcfe98b8628d1aca09b4c238d8d3b74c69010a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/254839
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
runtime.GC() doesn't guarantee the finalizer has run, so use a channel
instead to make sure finalizer was run in call to "after()".
Fixes#41361
Change-Id: I69c801e29aea49757ea72c52e8db13239de19ddc
Reviewed-on: https://go-review.googlesource.com/c/go/+/254401
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
So we can insert theses OVARLIVE nodes right after OpStaticCall in SSA.
This helps fixing issue that unsafe-uintptr arguments are not kept alive
during return statement, or can be kept alive longer than expected.
Fixes#24491
Change-Id: Ic04a5d1bbb5c90dcfae65bd95cdd1da393a66800
Reviewed-on: https://go-review.googlesource.com/c/go/+/254397
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL changes "T literal.M" error message to "T{...}.M". It's clearer
expression and focusing user on actual issue.
Updates #38745
Change-Id: I84b455a86742f37e0bde5bf390aa02984eecc3c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/253677
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently, the statement:
go g(uintptr(f()))
gets rewritten into:
tmp := f()
newproc(8, g, uintptr(tmp))
runtime.KeepAlive(tmp)
which doesn't guarantee that tmp is still alive by time the g call is
scheduled to run.
This CL fixes the issue, by wrapping g call in a closure:
go func(p unsafe.Pointer) {
g(uintptr(p))
}(f())
then this will be rewritten into:
tmp := f()
go func(p unsafe.Pointer) {
g(uintptr(p))
runtime.KeepAlive(p)
}(tmp)
runtime.KeepAlive(tmp) // superfluous, but harmless
So the unsafe.Pointer p will be kept alive at the time g call runs.
Updates #24491
Change-Id: Ic10821251cbb1b0073daec92b82a866c6ebaf567
Reviewed-on: https://go-review.googlesource.com/c/go/+/253457
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The order array was zero initialized by the compiler, but ends up being
overwritten by the runtime anyway.
So let the runtime takes full responsibility for initializing, save us
one instruction per select.
Fixes#40399
Change-Id: Iec1eca27ad7180d4fcb3cc9ef97348206b7fe6b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/251517
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The current command will run this entire set of tests, which takes a
noticeable amount of time. Contributors may wish to run only a subset of
these tests to save time/compute (e.g. when iterating on a CL that
failed tests in that subset). Listing file(s) as operands to the command
will run only those tests.
Change-Id: I1874c43681a594190bc40b61cee0b8d321be73f8
Reviewed-on: https://go-review.googlesource.com/c/go/+/242997
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
Flag constant Ops on arm and arm64 are under refactoring, this change adds
a couple of testcases that verify the behavior of 'noov' branches.
Updates #39505
Updates #38740
Updates #39303
Change-Id: I493344b52276900cd296c32da494d72932dfc9be
Reviewed-on: https://go-review.googlesource.com/c/go/+/238677
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We're using sort.SliceStable, so no need to keep track of indexes as well.
Use a more robust test for whether a node is a call.
Add a test that we're actually reordering comparisons. This test fails
without the alg.go changes in this CL because eqstring uses OCALLFUNC
instead of OCALL for its data comparisons.
Update #8606
Change-Id: Ieeec33434c72e3aa328deb11cc415cfda05632e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/237921
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This merges an lis + subf into subfic, and for 32b constants
lwa + subf into oris + ori + subf.
The carry bit is no longer used in code generation, therefore
I think we can clobber it as needed. Note, lowered borrow/carry
arithmetic is self-contained and thus is not affected.
A few extra rules are added to ensure early transformations to
SUBFCconst don't trip up earlier rules, fold constant operations,
or otherwise simplify lowering. Likewise, tests are added to
ensure all rules are hit. Generic constant folding catches
trivial cases, however some lowering rules insert arithmetic
which can introduce new opportunities (e.g BitLen or Slicemask).
I couldn't find a specific benchmark to demonstrate noteworthy
improvements, but this is generating subfic in many of the default
bent test binaries, so we are at least saving a little code space.
Change-Id: Iad7c6e5767eaa9dc24dc1c989bd1c8cfe1982012
Reviewed-on: https://go-review.googlesource.com/c/go/+/249461
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Carlos Eduardo Seo <cseo@linux.vnet.ibm.com>
Right now we just prevent such types from being on the heap. This CL
makes it so they cannot appear on the stack either. The distinction
between heap and stack is pretty vague at the language level (e.g. it
is affected by -N), and we don't need the flexibility anyway.
Once go:notinheap types cannot be in either place, we don't need to
consider pointers to such types to be pointers, at least according to
the garbage collector and stack copying. (This is the big win of this
CL, in my opinion.)
The distinction between HasPointers and HasHeapPointer no longer
exists. There is only HasPointers.
This CL is cleanup before possible use of go:notinheap to fix#40954.
Update #13386
Change-Id: Ibd895aadf001c0385078a6d4809c3f374991231a
Reviewed-on: https://go-review.googlesource.com/c/go/+/249917
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
With this patch, opt pass can expose more obvious constant-folding
opportunites.
Example:
func test(i int) int {return (i+8)-(i+4)}
The previous version:
MOVD "".i(FP), R0
ADD $8, R0, R1
ADD $4, R0, R0
SUB R0, R1, R0
MOVD R0, "".~r1+8(FP)
RET (R30)
The optimized version:
MOVD $4, R0
MOVD R0, "".~r1+8(FP)
RET (R30)
This patch removes some existing reassociation rules, such as "x+(z-C)",
because the current generic rewrite rules will canonicalize "x-const"
to "x+(-const)", making "x+(z-C)" equal to "x+(z+(-C))".
This patch also adds test cases.
Change-Id: I857108ba0b5fcc18a879eeab38e2551bc4277797
Reviewed-on: https://go-review.googlesource.com/c/go/+/237137
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
go1.14 drop nacl support, as go1.15 was released, go1.13 is not
supported anymore, nacl is absolutely gone.
Change-Id: I05efb46891ec875b08da8f2996751a8e9cb57d0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/249977
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
checkptr has code to recognize &^ expressions, but it didn't take into
account that "p &^ x" gets rewritten to "p & ^x" during walk, which
resulted in false positive diagnostics.
This CL changes walkexpr to mark OANDNOT expressions with Implicit
when they're rewritten to OAND, so that walkCheckPtrArithmetic can
still recognize them later.
It would be slightly more idiomatic to instead mark the OBITNOT
expression as Implicit (as it's a compiler-generated Node), but the
OBITNOT expression might get constant folded. It's not worth the extra
complexity/subtlety of relying on n.Right.Orig, so we set Implicit on
the OAND node instead.
To atone for this transgression, I add documentation for nodeImplicit.
Fixes#40917.
Change-Id: I386304171ad299c530e151e5924f179e9a5fd5b8
Reviewed-on: https://go-review.googlesource.com/c/go/+/249477
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Add a new lowering rule to match and replace such instances
with the MADDLD instruction available on power9 where
possible.
Likewise, this plumbs in a new ppc64 ssa opcode to house
the newly generated MADDLD instructions.
When testing ed25519, this reduced binary size by 936B.
Similarly, MADDLD combination occcurs in a few other less
obvious cases such as division by constant.
Testing of golang.org/x/crypto/ed25519 shows non-trivial
speedup during keygeneration:
name old time/op new time/op delta
KeyGeneration 65.2µs ± 0% 63.1µs ± 0% -3.19%
Signing 64.3µs ± 0% 64.4µs ± 0% +0.16%
Verification 147µs ± 0% 147µs ± 0% +0.11%
Similarly, this test binary has shrunk by 66488B.
Change-Id: I077aeda7943119b41f07e4e62e44a648f16e4ad0
Reviewed-on: https://go-review.googlesource.com/c/go/+/248723
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>
Some of the existing optimizations aren't triggered because they
are handled by the generic rules so this CL removes them. Also
some constraints were copied without much thought from the amd64
rules and they don't make sense on s390x, so we remove those
constraints.
Finally, add a 'multiply by the sum of two powers of two'
optimization. This makes sense on s390x as shifts are low latency
and can also sometimes be optimized further (especially if we add
support for RISBG instructions).
name old time/op new time/op delta
IntMulByConst/3-8 1.70ns ±11% 1.10ns ± 5% -35.26% (p=0.000 n=10+10)
IntMulByConst/5-8 1.64ns ± 7% 1.10ns ± 4% -32.94% (p=0.000 n=10+9)
IntMulByConst/12-8 1.65ns ± 6% 1.20ns ± 4% -27.16% (p=0.000 n=10+9)
IntMulByConst/120-8 1.66ns ± 4% 1.22ns ±13% -26.43% (p=0.000 n=10+10)
IntMulByConst/-120-8 1.65ns ± 7% 1.19ns ± 4% -28.06% (p=0.000 n=9+10)
IntMulByConst/65537-8 0.86ns ± 9% 1.12ns ±12% +30.41% (p=0.000 n=10+10)
IntMulByConst/65538-8 1.65ns ± 5% 1.23ns ± 5% -25.11% (p=0.000 n=10+10)
Change-Id: Ib196e6bff1e97febfd266134d0a2b2a62897989f
Reviewed-on: https://go-review.googlesource.com/c/go/+/248937
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
For an unsigned integer, it's useful to convert its order test with 0/1
to its equality test with 0. We can save a comparison instruction that
followed by a conditional branch on arm64 since it supports
compare-with-zero-and-branch instructions. For example,
if x > 0 { ... } else { ... }
the original version:
CMP $0, R0
BLS 9
the optimized version:
CBZ R0, 8
Updates #21439
Change-Id: Id1de6f865f6aa72c5d45b29f7894818857288425
Reviewed-on: https://go-review.googlesource.com/c/go/+/246857
Reviewed-by: Keith Randall <khr@golang.org>
If the AND has other uses, we end up saving an argument to the AND
in another register, so we can use it for the TEST. No point in doing that.
Change-Id: I73444a6aeddd6f55e2328ce04d77c3e6cf4a83e0
Reviewed-on: https://go-review.googlesource.com/c/go/+/241280
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
There are some architecture-independent rules in #21439, since an
unsigned integer >= 0 is always true and < 0 is always false. This CL
adds these optimizations to generic rules.
Updates #21439
Change-Id: Iec7e3040b761ecb1e60908f764815fdd9bc62495
Reviewed-on: https://go-review.googlesource.com/c/go/+/246617
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Makes sure the copyright notice is not interpreted as the package level
godoc.
Change-Id: I2afce7c9d620f19d51ec1438b1d0db1774b57146
Reviewed-on: https://go-review.googlesource.com/c/go/+/248760
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dave Cheney <dave@cheney.net>
The previous array length was large enough to exceed
maxImplicitStackSize on 64-bit architectures, but not on 32-bit
architectures.
Fixes#40808.
Change-Id: I69e9abb447454b2e7875ba503a0cb772e965ae31
Reviewed-on: https://go-review.googlesource.com/c/go/+/248680
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Currently, generated struct wrapper for closure is not handled in
mustHeapAlloc. That causes compiler crashes when the wrapper struct
is too large for stack, and must be heap allocated instead.
Fixes#39292
Change-Id: I14c1e591681d9d92317bb2396d6cf5207aa93e08
Reviewed-on: https://go-review.googlesource.com/c/go/+/244917
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 230121 fixed the bug that struct literal blank fields type array/struct
can not be initialized. But it still misses some cases when an expression
causes "candiscard(value)" return false. When these happen, we recursively
call fixedlit with "var_" set to "_", and hit the bug again.
To fix it, just making splitnode return "nblank" whenever "var_" is "nblank".
Fixes#38905
Change-Id: I281941b388acbd551a4d8ca1a235477f8d26fb6e
Reviewed-on: https://go-review.googlesource.com/c/go/+/232617
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Taking the live variable set from the last return point is problematic.
See #40629 for details, but there may not be a return point, or it may
be before the final defer.
Additionally, keeping track of the last call as a *Value doesn't quite
work. If it is dead-code eliminated, the storage for the Value is reused
for some other random instruction. Its live variable information,
if it is available at all, is wrong.
Instead, just mark all the open-defer argument slots as live
throughout the function. (They are already zero-initialized.)
Fixes#40629
Change-Id: Ie456c7db3082d0de57eaa5234a0f32525a1cce13
Reviewed-on: https://go-review.googlesource.com/c/go/+/247522
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Currently in addLocalInductiveFacts, we only check whether
direct edge from if block to phi block exists. If not, the
following logic will treat the phi block as the first successor,
which is wrong.
This patch makes prove pass more conservative, so we disable
some cases in test/prove.go. We will do some optimization in
the following CL and enable these cases then.
Fixes#40367.
Change-Id: I27cf0248f3a82312a6f7dabe11c79a1a34cf5412
Reviewed-on: https://go-review.googlesource.com/c/go/+/244579
Reviewed-by: Zach Jones <zachj1@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
They were missed as part of the refactoring to use a separate
addressing modes pass.
Fixes#40426
Change-Id: Ie0418b2fac4ba1ffe720644ac918f6d728d5e420
Reviewed-on: https://go-review.googlesource.com/c/go/+/244859
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The ICE reported as #33308 was fixed by a related CL; this change adds
a regression test with the crasher.
Fixes#33308
Change-Id: I3260075dbe3823b56b8825e6269e57a0fad185a6
Reviewed-on: https://go-review.googlesource.com/c/go/+/243458
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
It doesn't have to. The type in the aux field is authoritative.
There are cases involving casting from interface{} where pointers
have a placeholder pointer type (because the type is not known when
the IData op is generated).
The check was introduced in CL 13447.
Fixes#39459
Change-Id: Id77a57577806a271aeebd20bea5d92d08ee7aa6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/239817
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
reflect.assignTo writes to the target using write barriers. Make sure
that the memory it is writing to is zeroed, so the write barrier does
not read pointers from uninitialized memory.
Fixes#39541
Change-Id: Ia64b2cacc193bffd0c1396bbce1dfb8182d4905b
Reviewed-on: https://go-review.googlesource.com/c/go/+/238760
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
These conversion instructions set the condition code and so should
be marked as clobbering flags.
Fixes#39651.
Change-Id: I91cc9687ea70ef0551bb3139c1875071c349d43e
Reviewed-on: https://go-review.googlesource.com/c/go/+/238628
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Make sure that if a field comparison might panic, we evaluate
(and short circuit if not equal) all previous fields, and don't
evaluate any subsequent fields.
Add a bunch more tests to the equality+panic checker.
Update #8606
Change-Id: I6a159bbc8da5b2b7ee835c0cd1fc565575b58c46
Reviewed-on: https://go-review.googlesource.com/c/go/+/237919
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The scheduler assumes two special invariants that apply to tuple
selectors (Select0 and Select1 ops):
1. There is only one tuple selector of each type per generator.
2. Tuple selectors and generators reside in the same block.
Prior to this CL the assumption was that these invariants would
only be broken by the CSE pass. The CSE pass therefore contained
code to move and de-duplicate selectors to fix these invariants.
However it is also possible to write relatively basic optimization
rules that cause these invariants to be broken. For example:
(A (Select0 (B))) -> (Select1 (B))
This rule could result in the newly added selector (Select1) being
in a different block to the tuple generator (see issue #38356). It
could also result in duplicate selectors if this rule matches
multiple times for the same tuple generator (see issue #39472).
The CSE pass will 'fix' these invariants. However it will only do
so when optimizations are enabled (since disabling optimizations
disables the CSE pass).
This CL moves the CSE tuple selector fixup code into its own pass
and makes it mandatory even when optimizations are disabled. This
allows tuple selectors to be treated like normal ops for most of
the compilation pipeline until after the new pass has run, at which
point we need to be careful to maintain the invariant again.
Fixes#39472.
Change-Id: Ia3f79e09d9c65ac95f897ce37e967ee1258a080b
Reviewed-on: https://go-review.googlesource.com/c/go/+/237118
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This is a followup to CL 96495.
It should be simpler and more robust to achieve .bat files having
CRLF line endings by treating it as a binary file, like all other
files, and checking it in with the desired CRLF line endings.
A test is used to check the entire Go tree, short of directories
starting with "." and named "testdata", for any .bat files that
have anything other than strict CRLF line endings. This will help
catch any accidental modifications to existing .bat files or check
ins of new .bat files.
Importantly, this is compatible with how Gerrit serves .tar.gz files,
making it so that CRLF line endings are preserved.
The Go project is supported on many different environments, some of
which may have limited git implementations available, or none at all.
Relying on fewer git features and special rules makes it easier to
have confidence in the exact content of all files. Additionally, Go
development started in Subversion, moved to Perforce, then Mercurial,
and now uses Git.¹ Reducing its reliance on git-specific features will
help if there will be another transition in the project's future.
There are only 5 .bat files in the entire Go source tree, so a new one
being added is a rare event, and we prefer to do things in Go instead.
We still have the option of improving the experience for developers by
adding a pre-commit converter for .bat files to the git-codereview tool.
¹ https://groups.google.com/d/msg/golang-dev/sckirqOWepg/YmyT7dWJiocJFixes#39391.
For #37791.
Change-Id: I6e202216322872f0307ac96f1b8d3f57cb901e6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/236437
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Add interfaces which differ in type. Those used so far only
differ in value, not type.
These additional tests are needed to generate a failure
before CL 236278 went in.
Update #8606
Change-Id: Icdb7647b1973c2fff7e5afe2bd8b8c1b384f583e
Reviewed-on: https://go-review.googlesource.com/c/go/+/236418
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Make sure that we compare fields of structs and elements of arrays in order,
with proper short-circuiting.
Update #8606
Change-Id: I0a66ad92ea0af7bcc56dfdb275dec2b8d7e8b4fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/236147
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This change fixes a race condition between beforeIdle waking up the
innermost event handler and a timer causing a different goroutine to
wake up at the exact same moment. This messes up the wasm event handling
and leads to memory corruption. The solution is to make beforeIdle
return the goroutine that must run next and have findrunnable pick
this goroutine without considering timers again.
Fixes#38093Fixes#38574
Change-Id: Iffbe99411d25c2730953d1c8b0741fd892f8e540
Reviewed-on: https://go-review.googlesource.com/c/go/+/230178
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 233857 fixed the underlying issue for #37246,
which had arisen again as #38916.
Add the test case from #37246 to ensure it stays fixed.
Fixes#37246
Change-Id: If7fd75a096d2ce4364dc15509253c3882838161d
Reviewed-on: https://go-review.googlesource.com/c/go/+/233941
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
When tuple generators and selectors are eliminated as part of the
CSE pass we may end up with tuple selectors that are in different
blocks to the tuple generators that they correspond to. This breaks
the invariant that tuple generators and their corresponding
selectors must be in the same block. Therefore after CSE this
situation must be corrected.
Unfortunately the fixup code did not take into account that selectors
could be eliminated by CSE. It assumed that only the tuple generators
could be eliminated. In some situations this meant that it got into
a state where it was replacing references to selectors with references
to dead selectors in the wrong block.
To fix this we move the fixup code after the CSE rewrites have been
applied. This removes any difficult-to-reason-about interactions
with the CSE rewriter.
Fixes#38916.
Change-Id: I2211982dcdba399d03299f0a819945b3eb93b291
Reviewed-on: https://go-review.googlesource.com/c/go/+/233857
Run-TryBot: Michael Munday <mike.munday@ibm.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Taking over Zach's CL 212277. Just cleaned up and added a test.
For a positive, signed integer, an arithmetic right shift of count
(bit-width - 1) equals zero. e.g. int64(22) >> 63 -> 0. This CL makes
prove replace these right shifts with a zero-valued constant.
These shifts may arise in source code explicitly, but can also be
created by the generic rewrite of signed division by a power of 2.
// Signed divide by power of 2.
// n / c = n >> log(c) if n >= 0
// = (n+c-1) >> log(c) if n < 0
// We conditionally add c-1 by adding n>>63>>(64-log(c))
(first shift signed, second shift unsigned).
(Div64 <t> n (Const64 [c])) && isPowerOfTwo(c) ->
(Rsh64x64
(Add64 <t> n (Rsh64Ux64 <t>
(Rsh64x64 <t> n (Const64 <typ.UInt64> [63]))
(Const64 <typ.UInt64> [64-log2(c)])))
(Const64 <typ.UInt64> [log2(c)]))
If n is known to be positive, this rewrite includes an extra Add and 2
extra Rsh. This CL will allow prove to replace one of the extra Rsh with
a 0. That replacement then allows lateopt to remove all the unneccesary
fixups from the generic rewrite.
There is a rewrite rule to handle this case directly:
(Div64 n (Const64 [c])) && isNonNegative(n) && isPowerOfTwo(c) ->
(Rsh64Ux64 n (Const64 <typ.UInt64> [log2(c)]))
But this implementation of isNonNegative really only handles constants
and a few special operations like len/cap. The division could be
handled if the factsTable version of isNonNegative were available.
Unfortunately, the first opt pass happens before prove even has a
chance to deduce the numerator is non-negative, so the generic rewrite
has already fired and created the extra Ops discussed above.
Fixes#36159
By Printf count, this zeroes 137 right shifts when building std and cmd.
Change-Id: Iab486910ac9d7cfb86ace2835456002732b384a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/232857
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Improve the error user experience when users try to set/refer
to unexported fields and methods of struct literals, by directly saying
"cannot refer to unexported field or method"
Fixes#31053
Change-Id: I6fd3caf64b7ca9f9d8ea60b7756875e340792d59
Reviewed-on: https://go-review.googlesource.com/c/go/+/201657
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Omits printing the file:line:column when trying to
open non-existent files
Given:
go tool compile x.go
* Before:
x.go:0: open x.go: no such file or directory
* After:
open x.go: no such file or directory
Reverts the revert in CL 231043 by only fixing the case
of non-existent errors which is what the original bug
was about. The fix for "permission errors" will come later
on when I have bandwidth to investigate the differences
between running with root and why os.Open works for some
builders and not others.
Fixes#36437
Change-Id: I9c8a0981ad708b504bb43990a4105b42266fa41f
Reviewed-on: https://go-review.googlesource.com/c/go/+/230941
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
match:
m = make([]T, x); copy(m, s)
for pointer free T and x==len(s) rewrite to:
m = mallocgc(x*elemsize(T), nil, false); memmove(&m, &s, x*elemsize(T))
otherwise rewrite to:
m = makeslicecopy([]T, x, s)
This avoids memclear and shading of pointers in the newly created slice
before the copy.
With this CL "s" is only be allowed to bev a variable and not a more
complex expression. This restriction could be lifted in future versions
of this optimization when it can be proven that "s" is not referencing "m".
Triggers 450 times during make.bash..
Reduces go binary size by ~8 kbyte.
name old time/op new time/op delta
MakeSliceCopy/mallocmove/Byte 71.1ns ± 1% 65.8ns ± 0% -7.49% (p=0.000 n=10+9)
MakeSliceCopy/mallocmove/Int 71.2ns ± 1% 66.0ns ± 0% -7.27% (p=0.000 n=10+8)
MakeSliceCopy/mallocmove/Ptr 104ns ± 4% 99ns ± 1% -5.13% (p=0.000 n=10+10)
MakeSliceCopy/makecopy/Byte 70.3ns ± 0% 68.0ns ± 0% -3.22% (p=0.000 n=10+9)
MakeSliceCopy/makecopy/Int 70.3ns ± 0% 68.5ns ± 1% -2.59% (p=0.000 n=9+10)
MakeSliceCopy/makecopy/Ptr 102ns ± 0% 99ns ± 1% -2.97% (p=0.000 n=9+9)
MakeSliceCopy/nilappend/Byte 75.4ns ± 0% 74.9ns ± 2% -0.63% (p=0.015 n=9+9)
MakeSliceCopy/nilappend/Int 75.6ns ± 0% 76.4ns ± 3% ~ (p=0.245 n=9+10)
MakeSliceCopy/nilappend/Ptr 107ns ± 0% 108ns ± 1% +0.93% (p=0.005 n=9+10)
Fixes#26252
Change-Id: Iec553dd1fef6ded16197216a472351c8799a8e71
Reviewed-on: https://go-review.googlesource.com/c/go/+/146719
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Martin Möhrmann <moehrmann@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The {AND,OR,XOR}const ops can only take an int32 as an argument.
Make sure that when rewriting a BTx op to one of these, the result
has no high-order bits.
Fixes#38746
Change-Id: Ia7c5f76952329f60974bc033c29a5433610f3b28
Reviewed-on: https://go-review.googlesource.com/c/go/+/231977
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This reverts commit 4f7053c87f.
Reason for revert: Newly added test is failing on several builders.
Change-Id: I22dcbfebf2f57735b2f479886bbeb623f95b132f
Reviewed-on: https://go-review.googlesource.com/c/go/+/231043
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Omits printing the file:line:column when trying to open either
* non-existent files
* files without permission
Given:
go tool compile x.go
For either of x.go not existing, or if no read permissions:
* Before:
x.go:0: open x.go: no such file or directory
x.go:0: open x.go: permission denied
* After:
open x.go: no such file or directory
open x.go: permission denied
While here, noticed an oddity with the Linux builders, that appear
to always be running under root, hence the test for permission errors
with 0222 -W-*-W-*-W- can't pass on linux-amd64 builders.
The filed bug is #38608.
Fixes#36437
Change-Id: I9645ef73177c286c99547e3a0f3719fa07b35cb5
Reviewed-on: https://go-review.googlesource.com/c/go/+/229357
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
