This was fixed in CL 37598 but the test was (rightly) dropped
because it modified $GOROOT. Here's a variant that does not.
For #19151.
Change-Id: Iccdbbf9ae8ac4c252e52f4f8ff996963573c4682
Reviewed-on: https://go-review.googlesource.com/39592
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Commit 44ed88a5a7 moved printing of the "sweep done" gcpacertrace
message so that it is printed when the final sweeper finishes.
However, by this point some other thread has often already observed
that there are no more spans to sweep and zeroed sweepPagesPerByte.
Avoid printing a 0 sweep ratio in the trace when this race happens by
getting the value of the sweep ratio upon entry to sweepone and
printing that.
Change-Id: Iac0c48ae899e12f193267cdfb012c921f8b71c85
Reviewed-on: https://go-review.googlesource.com/39492
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rick Hudson <rlh@golang.org>
The comments in this package state that users should be
migrating code that uses the syscall package to its
corresponding package in x/sys. However, the syscall.Signal
and syscall.Errno types and the syscall.SysProcAttr struct is
not defined in the x/sys package and still need to be referenced
from within syscall. This adds a change to the comments to
clarify that the migration will need to continue to use some
references to syscall for now.
Fixes#19560
Change-Id: I8abb96b93bea90070ce461da16dc7bcf7b4b29c1
Reviewed-on: https://go-review.googlesource.com/39450
Reviewed-by: Rob Pike <r@golang.org>
For #17540.
Change-Id: Ie01f39797526934fa553f4279cbde6c7cbf14154
Reviewed-on: https://go-review.googlesource.com/36854
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is a holdover from the days when we did not
have full SSA coverage and compiled things optimistically,
and catching the panic obscures useful information.
Change-Id: I196790cb6b97419d92b318a2dfa7f1e1097cefb7
Reviewed-on: https://go-review.googlesource.com/39534
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Providing size hint when creating a map allows avoiding re-allocating
underlying data structure if we know how many elements are going to
be inserted. This can be used for example during decoding maps in
gob.
Fixes#19599
Change-Id: I108035fec29391215d2261a73eaed1310b46bab1
Reviewed-on: https://go-review.googlesource.com/38335
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This was a subtle bug introduced in the previous release's fix for
issue 16156.
The definition of empty template was broken, causing the answer
to depend on the order of templates in the map.
Fixes#16156 (for real).
Fixes#19294.
Fixes#19204.
Change-Id: I1cd915c94534cad3116d83bd158cbc28700510b9
Reviewed-on: https://go-review.googlesource.com/38420
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Popcount instructions on amd64 are not guaranteed to be
present, so we must guard their call. Rewrite rules can't
generate control flow at the moment, so the intrinsifier
needs to generate that code.
name old time/op new time/op delta
OnesCount-8 2.47ns ± 5% 1.04ns ± 2% -57.70% (p=0.000 n=10+10)
OnesCount16-8 1.05ns ± 1% 0.78ns ± 0% -25.56% (p=0.000 n=9+8)
OnesCount32-8 1.63ns ± 5% 1.04ns ± 2% -35.96% (p=0.000 n=10+10)
OnesCount64-8 2.45ns ± 0% 1.04ns ± 1% -57.55% (p=0.000 n=6+10)
Update #18616
Change-Id: I4aff2cc9aa93787898d7b22055fe272a7cf95673
Reviewed-on: https://go-review.googlesource.com/38320
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Note that this is a redo of an undo of the original buggy CL 38666.
We have lots of rewrite rules that vary only in the fact that
we have 2 versions for the 2 different orderings of various
commuting ops. For example:
(ADDL x (MOVLconst [c])) -> (ADDLconst [c] x)
(ADDL (MOVLconst [c]) x) -> (ADDLconst [c] x)
It can get unwieldly quickly, especially when there is more than
one commuting op in a rule.
Our existing "fix" for this problem is to have rules that
canonicalize the operations first. For example:
(Eq64 x (Const64 <t> [c])) && x.Op != OpConst64 -> (Eq64 (Const64 <t> [c]) x)
Subsequent rules can then assume if there is a constant arg to Eq64,
it will be the first one. This fix kinda works, but it is fragile and
only works when we remember to include the required extra rules.
The fundamental problem is that the rule matcher doesn't
know anything about commuting ops. This CL fixes that fact.
We already have information about which ops commute. (The register
allocator takes advantage of commutivity.) The rule generator now
automatically generates multiple rules for a single source rule when
there are commutative ops in the rule. We can now drop all of our
almost-duplicate source-level rules and the canonicalization rules.
I have some CLs in progress that will be a lot less verbose when
the rule generator handles commutivity for me.
I had to reorganize the load-combining rules a bit. The 8-way OR rules
generated 128 different reorderings, which was causing the generator
to put too much code in the rewrite*.go files (the big ones were going
from 25K lines to 132K lines). Instead I reorganized the rules to
combine pairs of loads at a time. The generated rule files are now
actually a bit (5%) smaller.
Make.bash times are ~unchanged.
Compiler benchmarks are not observably different. Probably because
we don't spend much compiler time in rule matching anyway.
I've also done a pass over all of our ops adding commutative markings
for ops which hadn't had them previously.
Fixes#18292
Change-Id: Ic1c0e43fbf579539f459971625f69690c9ab8805
Reviewed-on: https://go-review.googlesource.com/38801
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Instead of walking the list of nodes twice,
once to find static entries to add to an array
and once to find dynamic entries to generate code for,
do the split once up front, into two slices.
Then process each slice individually.
This makes the code easier to read
and more importantly, easier to modify.
While we're here, add a TODO to avoid
using temporaries for mapassign_fast calls.
It's not an important TODO;
the generated code would be basically identical.
It would just avoid a minor amount of
pointless SSA optimization work.
Passes toolstash-check.
No measureable compiler performance impact.
Updates #19751
Change-Id: I84a8f2c22f9025c718ef34639059d7bd02a3c406
Reviewed-on: https://go-review.googlesource.com/39351
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This triggers 119 times during make.bash.
This CL reduces the time it takes for the
compiler to panic while compiling the code in #19751
from 22 minutes to 15 minutes. Yay, I guess.
Updates #19751
Change-Id: I8ca7f1ae75f89d1eb2a361d67b3055a975221734
Reviewed-on: https://go-review.googlesource.com/39294
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Old buggy hardware incorrectly executes the shift-left-K
then shift-right-K idiom for clearing K leftmost bits.
Use a right rotate instead of shift to avoid triggering the
bug.
Fixes#19809.
Change-Id: I6dc646b183c29e9d01aef944729f34388dcc687d
Reviewed-on: https://go-review.googlesource.com/39310
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
The example shouldn't rely on the existance of example_test.go. That
breaks in the playground, which is what the "run" button in
https://golang.org/pkg/go/parser/#example_ParseFile does.
Make the example self-sufficient by using a small piece of source via a
string literal instead.
Fixes#19823.
Change-Id: Ie8a3c6c5d00724e38ff727862b62e6a3621adc88
Reviewed-on: https://go-review.googlesource.com/39236
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Robert Griesemer <gri@golang.org>
In a concurrent backend, Ctxt.Lookup will need some
form of concurrency protection, which will make it
more expensive.
This CL changes the pcln table builder to track
filenames as strings rather than LSyms.
Those strings are then converted into LSyms
at the last moment, for writing the object file.
This CL removes over 85% of the calls to Ctxt.Lookup
in a run of make.bash.
Passes toolstash-check.
Updates #15756
Change-Id: I3c53deff6f16f2643169f3bdfcc7aca2ca58b0a4
Reviewed-on: https://go-review.googlesource.com/39291
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When generating a random int8, uint8, int16, uint16, int32, uint32,
quick.Value chooses among all possible values.
But when generating a random int64 or uint64, it only chooses
values in the range [-2⁶², 2⁶²) (even for uint64).
It should, like for all the other integers, use the full range.
If it had, this would have caught #19807 earlier.
Instead it let us discover the presence of #19809.
While we are here, also make the default source of
randomness not completely deterministic.
Fixes#19808.
Change-Id: I070f852531c92b3670bd76523326c9132bfc9416
Reviewed-on: https://go-review.googlesource.com/39152
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
The header is literally
Key: Value
If the value or the key has leading or trailing spaces, those will
be lost by the round trip.
Found because testing/quick returns different values now.
Change-Id: I0f574bdbb5990689509c24309854d8f814b5efa0
Reviewed-on: https://go-review.googlesource.com/39211
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Minor cleanup.
This is the only such instance in the compiler.
Change-Id: I4e8ecde57d71867c7e1ac4d17e2154a91dd262b0
Reviewed-on: https://go-review.googlesource.com/39209
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
No particular need for this,
but it's nice to enforce invariants
when they are available.
Change-Id: Ia6fa88dc4116f65dac2879509746e123e2c1862a
Reviewed-on: https://go-review.googlesource.com/39201
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
nodfp is a global, so modifying it is unsafe in a concurrent backend.
It is also not necessary, since the Used marks
are only relevant for nodes in fn.Dcl.
For good measure, mark nodfp as always used.
Passes toolstash-check.
Updates #15756
Change-Id: I5320459f5eced2898615a17b395a10c1064bcaf5
Reviewed-on: https://go-review.googlesource.com/39200
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Dupok symbols may be defined in multiple packages. Its associated
package is chosen sort of arbitrarily (the first containing package
that the linker loads). Canonicalize its package to the package
with which it will be laid down in text, which is the first package
in dependency order that defines the symbol. So later passes (for
example, trampoline insertion pass) know that the dupok symbol
is laid down along with the package.
Fixes#19764.
Change-Id: I7cbc7474ff3016d5069c8b7be04af934abab8bc3
Reviewed-on: https://go-review.googlesource.com/39150
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Reviewed-by: David Chase <drchase@google.com>
Each architecture's Rconv function is only used inside its
respective package, so it does not need to be exported.
Change-Id: Ifbd629964d7a9edd66501d7cdf4750621d66d646
Reviewed-on: https://go-review.googlesource.com/39110
Run-TryBot: Dave Cheney <dave@cheney.net>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This change makes it possible to retrieve the size of a file part
without having to Seek to determine file-size.
Resolves#19501
Change-Id: I7b9994c4cf41c9b06a046eb7046f8952ae1f15e9
Reviewed-on: https://go-review.googlesource.com/39223
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
For #13560.
Change-Id: I884e63f89d0756ca87b8c2092b4fd8360f791a2f
Reviewed-on: https://go-review.googlesource.com/39171
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Follow-up to review comments on CL 39193.
Change-Id: I7649af9d70ad73e039061a7a66fea416a7476192
Reviewed-on: https://go-review.googlesource.com/39199
Reviewed-by: Robert Griesemer <gri@golang.org>
A few gc.Node ops may be shared across functions.
The compiler is (mostly) already careful to avoid mutating them.
However, from a concurrency perspective, replacing (say)
an empty list with an empty list still counts as a mutation.
One place this occurs is orderinit. Avoid it.
This requires fixing one spot where shared nodes were mutated.
It doesn't result in any functional or performance changes.
Passes toolstash-check.
Updates #15756
Change-Id: I63c93b31baeeac62d7574804acb6b7f2bc9d14a9
Reviewed-on: https://go-review.googlesource.com/39196
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Round uses r+r < d to decide whether the remainder is
above or below half of d (to decide whether to round up or down).
This is wrong when r+r wraps negative, because it looks < d
but is really > d.
No one will ever care about rounding to a multiple of
d > 2⁶² (about 146 years), but might as well get it right.
Fixes#19807.
Change-Id: I1b55a742dc36e02a7465bc778bf5dd74fe71f7c0
Reviewed-on: https://go-review.googlesource.com/39151
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The node in typenamesym requires neither
a position nor a curfn.
Passes toolstash-check.
Updates #15756
Change-Id: I6d39a8961e5578fe5924aaceb29045b6de2699df
Reviewed-on: https://go-review.googlesource.com/39194
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
newnamel is newname but with no dependency on lineno or Curfn.
This makes it suitable for use in a concurrent back end.
Use it now to make tempAt global-free.
The decision to push the assignment to n.Name.Curfn
to the caller of newnamel is based on mdempsky's
comments in #19683 that he'd like to do that
for callers of newname as well.
Passes toolstash-check. No compiler performance impact.
Updates #19683
Updates #15756
Change-Id: Idc461a1716916d268c9ff323129830d9a6e4a4d9
Reviewed-on: https://go-review.googlesource.com/39191
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Convert yyerrors into Fatals.
Remove the goto.
Move variable declaration closer to use.
Unify printing strings a bit.
Convert an int param into a bool.
Passes toolstash-check. No compiler performance impact.
Change-Id: I9017681417b785cf8693d18b124ac4f1ff37f2b5
Reviewed-on: https://go-review.googlesource.com/39170
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The names never occur more than once,
so interning the results is counterproductive.
The impact is not very big, but neither is the fix.
name old time/op new time/op delta
Unicode 90.2ms ± 3% 88.3ms ± 5% -2.10% (p=0.000 n=94+98)
Change-Id: I1e3a24433db4ae0c9a6e98166568941824ff0779
Reviewed-on: https://go-review.googlesource.com/39193
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also: Fix (testdata/gen/) copyGen.go, zeroGen.go, and arithConstGen.go
to actually match (testdata/) copy.go, zero.go, and arithConst.go, all
of which were manually edited in https://go-review.googlesource.com/20823
and https://go-review.googlesource.com/22748 despite the 'do not edit'
(or perhaps because it was missing in the case of arithConst.go).
For #13560.
Change-Id: I366e1b521e51885e0d318ae848760e5e14ccd488
Reviewed-on: https://go-review.googlesource.com/39172
Reviewed-by: Rob Pike <r@golang.org>
