The code has a comment saying that it waited for the goroutines,
but it didn't actually do so.
Change-Id: Icaeb40613711053a9f443cc34143835560427dda
Reviewed-on: https://go-review.googlesource.com/c/go/+/219277
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
When run as a separate program, the code in ExampleDial panicked due to
an expired certificate. Fixed this problem by replacing the expired
certificate with a valid one.
Also added a comment in the certificate to give a hint about why it
might fail in the future.
Fixes#35706
Change-Id: I3d300f7bccae050e4b73ded28b8029aa04b480bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/212601
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
This CL should not change the logic at all, but it took me a while to
figure out why we use these specific SignatureSchemes, so reformulate
the comment.
Change-Id: If519a58264209e6575417be07668e92ead0e772f
Reviewed-on: https://go-review.googlesource.com/c/go/+/208225
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
On platforms where internal linking PIE is supported, run
misc/cgo/test with that configuration.
Change-Id: Ia09631b07cc469c85818e1494093d802ea9d69a1
Reviewed-on: https://go-review.googlesource.com/c/go/+/218838
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The error message is now positioned at the statement position (which is
an identifing token, such as the '=' for assignments); and in case of
assignments it emphasizes the assignment by putting the Lhs and Rhs
in parentheses. Finally, the wording is changed from "use of * as value"
to the stronger "cannot use * as value" (for which there is precedent
elsewhere in the parser).
Fixes#36858.
Change-Id: Ic3f101bba50f58e3a1d9b29645066634631f2d61
Reviewed-on: https://go-review.googlesource.com/c/go/+/218337
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
When internal linking PIE, R_ADDR relocations cannot be resolved
statically so we generate dynamic relocations for it. We don't
apply the relocations statically, so the bytes in the file are
left unset (likely zero). This makes some tool that examines the
file statically, e.g. go version, to fail to find the referenced
addresses.
This CL makes the linker also apply the relocations to the file
content, so it holds the correct offsets and so can be examined
statically.
Fixes#37173.
Change-Id: Ia5c6b661f1a91a232843ca4224264bfd7a5509eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/219199
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Brad's battery died on a plane and the file stayed open for 8 years
without anyone noticing. 😄
Someone noticed in https://github.com/gopherjs/gopherjs/pull/950.
Updates #2833
Change-Id: I46b28ac014a8c355be94e026615f119f96e5d51a
Reviewed-on: https://go-review.googlesource.com/c/go/+/218700
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
For an absolute address relocation in C objects (e.g.
R_X86_64_64), we turn it into an R_ADDR relocation and handle it
the same way. For internal linking PIE, this R_ADDR relocation
cannot be resolved statically. We need to generate a dynamic
relocation for it. This CL makes it so.
This fixes internal linking PIE on the dev.boringcrypto branch.
Test will be enabled in the next CL.
Change-Id: I9bdd6517ccd79cbbe9c64844a31536bf3da37616
Reviewed-on: https://go-review.googlesource.com/c/go/+/218837
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Go's PIE binaries have tons of relocations, all R_X86_64_64 [1] when
internally linked. R_X86_64_64 relocations require symbol lookup in the
dynamic linker, which can be quite slow. The simple Go HTTP server
in #36028 takes over 1s to complete dynamic linking!
The external linker generates R_X86_64_RELATIVE [2] relocations, which
are significantly more efficient. It turns out that generating these
relocations internally is quite simple, so lets do it.
Rather than referencing targ.Dynid in r_info and having the dynamic
linker do a symbol lookup and then add (final targ address) + r.Add, use
AddAddrPlus to generate another R_ADDR to have the linker compute (targ
address + r.Add). The dynamic linker is then only left with base address
+ r_addend.
Since we don't reference the symbol in the final relocation, Adddynsym
is no longer necessary, saving ~1MB (of ~9MB) from the binary size of
the example in #36028.
[1] R_AARCH64_ABS64 on arm64.
[2] R_AARCH64_RELATIVE on arm64.
Fixes#36028
Change-Id: Ie0f4bb1d911f2e05ab46a0a0454c0f46ac01b43b
Reviewed-on: https://go-review.googlesource.com/c/go/+/210181
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We currently print a "go: finding" line for each missing package
during import resolution. However, we are only printing a "go: found"
line for each module: if a given module provides multiple packages, we
don't indicate the module that we found for the second and later
packages.
Before this change:
$ GO111MODULE=on go get golang.org/x/tools/cmd/html2article@78f9822548c13e2c41cc8039d1492a111240db07
go: found golang.org/x/tools/cmd/html2article in golang.org/x/tools v0.0.0-20190214195451-78f9822548c1
go: finding module for package golang.org/x/net/html
go: finding module for package golang.org/x/net/html/atom
go: downloading golang.org/x/net v0.0.0-20200202094626-16171245cfb2
go: found golang.org/x/net/html in golang.org/x/net v0.0.0-20200202094626-16171245cfb2
After:
$ GO111MODULE=on go get golang.org/x/tools/cmd/html2article@78f9822548c13e2c41cc8039d1492a111240db07
go: found golang.org/x/tools/cmd/html2article in golang.org/x/tools v0.0.0-20190214195451-78f9822548c1
go: finding module for package golang.org/x/net/html/atom
go: finding module for package golang.org/x/net/html
go: found golang.org/x/net/html in golang.org/x/net v0.0.0-20200202094626-16171245cfb2
go: found golang.org/x/net/html/atom in golang.org/x/net v0.0.0-20200202094626-16171245cfb2
Updates #26152
Updates #33284
Change-Id: I221548749e36bfd6a79efe5edc3645dc5319fd6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/219437
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
This update was automatically generated using the updatecontrib command:
cd gotip
go run golang.org/x/build/cmd/updatecontrib
With minor manual changes based on publicly available information
to canonicalize letter case and formatting for a few names.
Actions taken (relative to CONTRIBUTORS at origin/master):
Added Aaron Bieber <deftly@gmail.com>
Added Adam Williams <pwnfactory@gmail.com>
Added Ayke van Laethem <aykevanlaethem@gmail.com>
Added Bradford Lamson-Scribner <brad.lamson@gmail.com>
Added Brian Falk <falk@logicparty.org>
Added Chen Zhihan <energiehund@gmail.com>
Added Christopher Loessl <cloessl+github@gmail.com>
Added Frederik Zipp <fzipp@gmx.de>
Added Fujimoto Kyosuke <kyoro.f@gmail.com>
Added GitHub User jopbrown (6345470) <msshane2008@gmail.com>
Added GitHub User yah01 (12216890) <kagaminehuan@gmail.com>
Added Hiromichi Ema <ema.hiro@gmail.com>
Added Jamal Carvalho <jamal.a.carvalho@gmail.com>
Added Jason Baker <jason-baker@users.noreply.github.com>
Added Kanta Ebihara <kantaebihara@gmail.com>
Added Kirill Tatchihin <kirabsuir@gmail.com>
Added Kévin Dunglas <dunglas@gmail.com>
Added Mariano Cano <mariano@smallstep.com>
Added Sergey Ivanov <ser1325@gmail.com>
Added Thomas Symborski <thomas.symborski@gmail.com>
Added Tomohiro Kusumoto <zabio1192@gmail.com>
Added Xingqang Bai <bxq2011hust@qq.com>
Used GitHub User jopbrown (6345470) form for jopbrown <msshane2008@gmail.com> https://github.com/golang/exp/commit/0405dc7 [exp]
Used GitHub User yah01 (12216890) form for yah01 <kagaminehuan@gmail.com> https://github.com/golang/go/commit/ee55dd6b64 [go]
Used GitHub name "Hiromichi Ema" for emahiro <ema.hiro@gmail.com> https://github.com/golang/tools/commit/b6336cbc [tools]
Used GitHub name "Jamal Carvalho" for Gopher <jamal.a.carvalho@gmail.com> https://github.com/golang/gddo/commit/31dd61d [gddo]
Used GitHub name "Xingqang Bai" for bxq2011hust <bxq2011hust@qq.com> https://github.com/golang/go/commit/79ccbe1b67 [go]
Updates #12042
Change-Id: I13f8ab37f8b38f8f5d0ff71c939ad39d0bc4f985
Reviewed-on: https://go-review.googlesource.com/c/go/+/220363
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Run-TryBot: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
I've been thinking about the relationship between the package import
graph and the module import graph, and realized that the package
import graph is not always acyclic. (The package import graph must be
acyclic given a specific set of build tags, but the 'mod' subcommands
intentionally ignore build tags.)
I'm not sure whether we have any existing regression tests that cover
this sort of cycle, so I'm adding one now. Thankfully, it passes!
Updates #36460
Change-Id: I7679320994ee169855241efa51cd45f71315f7f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/217557
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Update the Example in the crypto/ecdsa package for signing
and verifying signatures to use these new functions.
This also changes (*PrivateKey).Sign to use
x/crypto/cryptobyte/asn1 instead of encoding/asn1
to marshal the signature.
Fixes#20544
Change-Id: I3423cfc4d7f9e1748fbed5a631438c8a3b280df4
Reviewed-on: https://go-review.googlesource.com/c/go/+/217940
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Allow removing a connection from the connection pool after
it has been idle for a period of time, without regard to the
total lifespan of the connection.
Fixes#25232
Change-Id: Icff157b906769a2d2d45c67525e04a72feb8d832
Reviewed-on: https://go-review.googlesource.com/c/go/+/145758
Run-TryBot: Daniel Theophanes <kardianos@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Octal literals parsing was implemented in strconv in golang.org/cl/160244
and in math/big in golang.org/cl/165898.
Underscore separator parsing was implemented in strconv in golang.org/cl/160243
and in math/big golang.org/cl/166157.
Thus octal & underscore literal parsing in go/constant is removed as redundant.
This CL resolves TODO left by gri in golang.org/cl/160239 .
Change-Id: I311872dac49b1a13063e0abc1794001956620c5a
GitHub-Last-Rev: 264caf574e
GitHub-Pull-Request: golang/go#36630
Reviewed-on: https://go-review.googlesource.com/c/go/+/215277
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The variable `n` for saving the pointer of the next
element when insert new element into the list turns
out to be unnecessary.
Change-Id: I17b85fd8350738815c320a83945525b60c2f04c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/207037
Reviewed-by: Robert Griesemer <gri@golang.org>
Since rulegen is only tested by inspecting and running its output code,
we have no good way to see if any chunks of its source are actually
being unused.
Code coverage only works as part of 'go test', since it needs to
instrument our code. Add a script that sets up a tiny test for that
purpose, with a quick example on how to use it.
We need to use a script, because there's no other way to make this work
without breaking 'go run *.go'. It's far more common to run the
generator than to obtain a coverage profile, so this solution seems like
the right tradeoff, and we don't break existing users.
The script isn't terribly portable, but that's okay for now.
At the time of wriging, coverage sits at 89.7%. I've manually skimmed
main.go and rulegen.go, and practically all unused code is either error
handling, or optional code like *genLog and "if false". A couple of
small exceptions stand out, though I'm not paying attention to them in
this CL.
While at it, inline a couple of tiny unusedInspector methods that were
only needed once or twice.
Change-Id: I78c5fb47c8536d70e546a437637d4428ec7adfaa
Reviewed-on: https://go-review.googlesource.com/c/go/+/212760
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The error message for trailing data after the X.509 issuer should
correctly state "issuer" instead of "subject", which appears just above
this code.
Fixes#35841
Change-Id: Iea2605ce97f2b084eb78e88f2c27d7d43749d022
Reviewed-on: https://go-review.googlesource.com/c/go/+/208978
Run-TryBot: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Katie Hockman <katie@golang.org>
It might break a program if it was depending on undocumented behavior.
Give a proper heads up.
Fixes#37308.
Change-Id: Id65bc70def1138d5506b694329c52250b417ec6f
Reviewed-on: https://go-review.googlesource.com/c/go/+/220418
Reviewed-by: Bryan C. Mills <bcmills@google.com>
RFC 5322 has a section 4.4 where it says that address-list could
have "null" members: "That is, there could be two or more commas in
such a list with nothing in between them, or commas at the beginning
or end of the list." This change handles such a case so that mail
clients using this method on actual email messages get a reasonable
return value when they parse email.
Fixes#36959
Change-Id: I3ca240969935067262e3d751d376a06db1fef2a2
GitHub-Last-Rev: b96a9f2c07
GitHub-Pull-Request: golang/go#36966
Reviewed-on: https://go-review.googlesource.com/c/go/+/217377
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
The name of the function should mention division.
Eliminate double negatives from the comment describing it.
Change-Id: Icef1a5139b3a91b86acb930af97938f5160f7342
Reviewed-on: https://go-review.googlesource.com/c/go/+/217001
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This requires threading location information through varCount.
This provides much more useful error messages.
Change-Id: If5ff942cbbbf386724eda15a523c181c137fac20
Reviewed-on: https://go-review.googlesource.com/c/go/+/216221
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
We had three implementations.
Refactor, and document the shared implementation.
While we're here, improve the docs for func unbalanced.
Change-Id: I612cce79de15a864247afe377d3739d04a56b9bc
Reviewed-on: https://go-review.googlesource.com/c/go/+/216219
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
CL 213703 converted generated rewrite rules for commutative ops
to use loops instead of duplicated code.
However, it loaded args using expressions like
v.Args[i] and v.Args[i^1], which the compiler could
not eliminate bounds for (including with all outstanding
prove CLs).
Also, given a series of separate rewrite rules for the same op,
we generated bounds checks for every rewrite rule, even though
we were repeatedly loading the same set of args.
This change reduces both sets of bounds checks.
Instead of loading v.Args[i] and v.Args[i^1] for commutative loops,
we now preload v.Args[0] and v.Args[1] into local variables,
and then swap them (as needed) in the commutative loop post statement.
And we now load all top level v.Args into local variables
at the beginning of every rewrite rule function.
The second optimization is the more significant,
but the first helps a little, and they play together
nicely from the perspective of generating the code.
This does increase register pressure, but the reduced bounds
checks more than compensate.
Note that the vast majority of rewrite rules evaluated
are not applied, so the prologue is the most important
part of the rewrite rules.
There is one subtle aspect to the new generated code.
Because the top level v.Args are shared across rewrite rules,
and rule evaluation can swap v_0 and v_1, v_0 and v_1
can end up being swapped from one rule to the next.
That is OK, because any time a rule does not get applied,
they will have been swapped exactly twice.
Passes toolstash-check -all.
name old time/op new time/op delta
Template 213ms ± 2% 211ms ± 2% -0.85% (p=0.000 n=92+96)
Unicode 83.5ms ± 2% 83.2ms ± 2% -0.41% (p=0.004 n=95+90)
GoTypes 737ms ± 2% 733ms ± 2% -0.51% (p=0.000 n=91+94)
Compiler 3.45s ± 2% 3.43s ± 2% -0.44% (p=0.000 n=99+100)
SSA 8.54s ± 1% 8.32s ± 2% -2.56% (p=0.000 n=96+99)
Flate 136ms ± 2% 135ms ± 1% -0.47% (p=0.000 n=96+96)
GoParser 169ms ± 1% 168ms ± 1% -0.33% (p=0.000 n=96+93)
Reflect 456ms ± 3% 455ms ± 3% ~ (p=0.261 n=95+94)
Tar 186ms ± 2% 185ms ± 2% -0.48% (p=0.000 n=94+95)
XML 251ms ± 1% 250ms ± 1% -0.51% (p=0.000 n=91+94)
[Geo mean] 424ms 421ms -0.68%
name old user-time/op new user-time/op delta
Template 275ms ± 1% 274ms ± 2% -0.55% (p=0.000 n=95+98)
Unicode 118ms ± 4% 118ms ± 4% ~ (p=0.642 n=98+90)
GoTypes 983ms ± 1% 980ms ± 1% -0.30% (p=0.000 n=93+93)
Compiler 4.56s ± 6% 4.52s ± 6% -0.72% (p=0.003 n=100+100)
SSA 11.4s ± 1% 11.1s ± 1% -2.50% (p=0.000 n=96+97)
Flate 168ms ± 1% 167ms ± 1% -0.49% (p=0.000 n=92+92)
GoParser 204ms ± 1% 204ms ± 2% -0.27% (p=0.003 n=99+96)
Reflect 599ms ± 2% 598ms ± 2% ~ (p=0.116 n=95+92)
Tar 227ms ± 2% 225ms ± 2% -0.57% (p=0.000 n=95+98)
XML 313ms ± 2% 312ms ± 1% -0.37% (p=0.000 n=89+95)
[Geo mean] 547ms 544ms -0.61%
file before after Δ %
compile 21113112 21109016 -4096 -0.019%
total 131704940 131700844 -4096 -0.003%
Change-Id: Id6c39e0367e597c0c75b8a4b1eb14cc3cbd11956
Reviewed-on: https://go-review.googlesource.com/c/go/+/216218
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This change introduces a new syntax for rewrite rules
that only change a Value's Op. See #36380 for more discussion.
Updating rewrite rules to use ellipses will happen
in follow-up CLs.
Change-Id: I8c56e85de24607579d79729575c89ca80805ba5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/213898
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
It's much easier to debug when you can see
the contents in order to interpret the error message.
Change-Id: I03bbb9dd3071aeca9577cc725a60d43f78118cf4
Reviewed-on: https://go-review.googlesource.com/c/go/+/215717
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
This documentation was lost in CL 213703.
This change restores it.
Change-Id: I544f15771d8a7390893efbda93478b46095ccf3c
Reviewed-on: https://go-review.googlesource.com/c/go/+/215541
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Part of converting all tests to the script framework, but also
just working around flakiness when adding t.Parallel to
TestExecutableGOROOT.
Also, undo the changes to copyFile in golang.org/cl/220317 because
they didn't help.
Fixes#37306
Change-Id: I3348fa1d8f2589febe89604ac19a21df20075c4c
Reviewed-on: https://go-review.googlesource.com/c/go/+/220319
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
An experiment to see if this helps flakiness go away in
TestExecutableGOROOT.
Updates #37306
Change-Id: I2f4f63bdb507359ca07267d86cdb41fe4968f151
Reviewed-on: https://go-review.googlesource.com/c/go/+/220317
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Old trace viewer stopped working with Chrome M80+ because the
old trace viewer heavily depended on WebComponents V0 which are deprecated.
Trace viewer recently migrated to use WebComponents V0 polyfill
(crbug.com/1036492). This CL brings in the newly updated trace_viewer_full.html
(sync'd @ 9508452e)
and updates the javascript snippet included in the /trace endpoint
to use the polyfill.
This brings in webcomponents.min.js copied from
https://chromium.googlesource.com/catapult/+/9508452e18f130c98499cb4c4f1e1efaedee8962/third_party/polymer/components/webcomponentsjs/webcomponents.min.js
That is necessary because the /trace endpoint needs to import
the vulcanized trace_viewer_full.html.
It's possible that some features are not working correctly with
this polyfill. In that case, report the issue to crbug.com/1036492.
There will be a warning message in the UI (yellow banner above the timeline)
which can be hidden by clicking the 'hide' button.
This allows to render the trace in browsers other than chrome in theory,
but I observed some buttons and functions still don't work outside
chrome.
Fixes#34374.
Change-Id: Ib575f756f5e6b22ad904ede6e4d224a995ebe259
Reviewed-on: https://go-review.googlesource.com/c/go/+/219997
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
I noticed some instances of "[ " and " ]" in the rewrite rules.
Normalizing them helps catch possible future duplicate rules.
Change-Id: I892fd7e9b4019ed304f0a61fa2bb7f7e47ef8f38
Reviewed-on: https://go-review.googlesource.com/c/go/+/213682
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Prior to this change, we generated additional rules at rulegen time
for all possible combinations of args to commutative ops.
This is simple and works well, but leads to lots of generated rules.
This in turn has increased the size of the compiler,
made it hard to compile package ssa on small machines,
and provided a disincentive to mark some ops as commutative.
This change reworks how we handle commutative ops.
Instead of generating a rule per argument permutation,
we generate a series of nested loops, one for each commutative op.
Each loop tries both possible argument orderings.
I also considered attempting to canonicalize the inputs to the
rewrite rules. However, because either or both arguments might be
nothing more than an identifier, and because there can be arbitrary
conditions to evaluate during matching, I did not see how to proceed.
The duplicate rule detection now sorts arguments to commutative ops,
so that it can detect commutative-only duplicates.
There may be further optimizations to the new generated code.
In particular, we may not be removing as many bounds checks as before;
I have not investigated deeply. If more work here is needed,
we could do it with more hints or with improvements to the prove pass.
This change has almost no impact on the generated code.
It does not pass toolstash-check, however. In a handful of functions,
for reasons I do not understand, there are minor position changes.
For the entire series ending at this change,
there is negligible compiler performance impact.
The compiler binary shrinks by about 15%,
and package ssa shrinks by about 25%.
Package ssa also compiles ~25% faster with ~25% less memory.
Change-Id: Ia2ee9ceae7be08a17342319d4e31b0bb238a2ee4
Reviewed-on: https://go-review.googlesource.com/c/go/+/213703
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Copied from go1.14.html, with changes redacted back to TODOs following
the model of CL 195058.
'relnote -html' does not report any changes at this time.
Updates #33738
Change-Id: I580232805ab7db35935f3e1ba03b720be4796a7e
Reviewed-on: https://go-review.googlesource.com/c/go/+/220278
Reviewed-by: Alexander Rakoczy <alex@golang.org>
Run-TryBot: Alexander Rakoczy <alex@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The commutative rule generator has an optimization
where given (Add x y) with no other uses of x and y in the matching rule,
it doesn't generate the commutative match (Add y x).
However, if there is also a condition referring to x or y,
such as (Add x y) && isFoo(x), then we should generate the commutative rule.
This change parses the condition, extracts all idents, and takes them
into consideration.
This doesn't yield any new optimizations now.
However, it is the right thing to do;
otherwise we'll have to track it down and fix it again later.
It is also expensive now, in terms of additional generated code.
However, it will be much, much less expensive soon,
once our generated code for commutative ops gets smaller.
Change-Id: I52c2016c884bbc7789bf8dfe9b9c56061bc028ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/213702
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
HMUL is commutative. However, it has asymmetric register requirements.
There are existing rewrite rules to place arguments in preferable slots.
Due to a bug, the existing rulegen commutativity engine doesn't generate
the commuted form of the HMUL rules.
The commuted form of those rewrite rules cause infinite loops.
In order to fix the rulegen commutativity bug,
we need to choose between eliminating
those rewrite rules and marking HMUL ops as not commutative.
This change chooses the latter, since doing so yields better
optimization results on std+cmd.
Removing the rewrite rules yields only text size regressions:
file before after Δ %
runtime.s 477257 477269 +12 +0.003%
time.s 83552 83612 +60 +0.072%
encoding/asn1.s 57378 57382 +4 +0.007%
cmd/go/internal/modfetch/codehost.s 89822 89829 +7 +0.008%
cmd/internal/test2json.s 9459 9466 +7 +0.074%
cmd/go/internal/test.s 57665 57678 +13 +0.023%
Marking HMUL as not commutative actually yields (mostly) improvements:
file before after Δ %
runtime.s 477257 477247 -10 -0.002%
math.s 35985 35992 +7 +0.019%
strconv.s 53486 53462 -24 -0.045%
syscall.s 82483 82446 -37 -0.045%
time.s 83552 83561 +9 +0.011%
os.s 52691 52684 -7 -0.013%
archive/zip.s 42285 42272 -13 -0.031%
encoding/asn1.s 57378 57329 -49 -0.085%
encoding/base64.s 12156 12094 -62 -0.510%
net.s 296286 296276 -10 -0.003%
encoding/base32.s 9720 9658 -62 -0.638%
net/http.s 560931 560907 -24 -0.004%
net/smtp.s 14421 14411 -10 -0.069%
cmd/vendor/golang.org/x/sys/unix.s 74307 74266 -41 -0.055%
The regressions are minor, and are in functions math.cbrt,
time.Time.String, and time.Date.
Change-Id: I9f6d9ee71654e5b70381cac77b0ac26011f4ea12
Reviewed-on: https://go-review.googlesource.com/c/go/+/213701
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
