This is the second attempt. The first attempt was CL 229127,
which got rolled back by CL 229177, because it caused
an infinite loop during compilation on some platforms.
I didn't notice that the trybots hadn't completed when I submitted; mea culpa.
The bug was that we were checking x&(x-1)==0, which is also true of 0,
which does not have exactly one bit set.
This caused an infinite rewrite rule loop.
Updates #38547
file before after Δ %
compile 19678112 19669808 -8304 -0.042%
total 113143160 113134856 -8304 -0.007%
Change-Id: I417a4f806e1ba61277e31bab2e57dd3f1ac7e835
Reviewed-on: https://go-review.googlesource.com/c/go/+/229197
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Munday <mike.munday@ibm.com>
Given:
type u struct{}
func (u) M() {}
type t struct { u; u2 u }
var v = reflect.ValueOf(t{})
Package reflect allows:
v.Method(0) // v.M
v.Field(0).Method(0) // v.u.M
but panics from:
v.Field(1).Method(0) // v.u2.M
because u2 is not an exported field. However, u is not an exported
field either, so this is inconsistent.
It seems like this behavior originates from #12367, where it was
decided to allow traversing unexported embedded fields to be able to
access their exported fields, since package reflect doesn't provide an
alternative way to access promoted fields directly.
But extending that logic to promoted *methods* was inappropriate,
because package reflect's normal method handling logic already handles
promoted methods correctly. This CL corrects that mistake.
Fixes#38521.
Change-Id: If65008965f35927b4e7927cddf8614695288eb19
Reviewed-on: https://go-review.googlesource.com/c/go/+/228902
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This reverts commit 066c47ca5f.
Reason for revert: This appears to have broken a bunch of builders.
Change-Id: I68b4decf3c1892766e195d8eb018844cdff69443
Reviewed-on: https://go-review.googlesource.com/c/go/+/229177
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
This was accidentally broken in CL 166462, which introduce another
function in the panicking path without adjusting the argument to
runtime.Caller.
Change-Id: Ib6f9ed8673fefd458c7a4e3a918c45c5b31ca552
Reviewed-on: https://go-review.googlesource.com/c/go/+/229082
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is followup of CL 228860, which rewrite shift rules to use typed
aux. That CL introduced nlz* functions, to refactor left shift rules.
While at it, we realize there's a bug in old rules with both right/left
shift rules, but only fix for left shift rules only.
This CL fixes the bug for right shift rules.
Passes toolstash-check.
Change-Id: Id8f2158b1b66c9e87f3fdeaa7ae3e35dc0666f8b
Reviewed-on: https://go-review.googlesource.com/c/go/+/229137
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
This optimization works on any integer with exactly one bit set.
This is identical to being a power of two, except in the
most negative number. Use oneBit instead.
The rule now triggers in a few more places in std+cmd,
in packages encoding/asn1, crypto/elliptic, and
vendor/golang.org/x/crypto/cryptobyte.
This change obviates the need for CL 222479
by doing this optimization consistently in the compiler.
Change-Id: I983c6235290fdc634fda5e11b10f1f8ce041272f
Reviewed-on: https://go-review.googlesource.com/c/go/+/229124
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This reverts commit 98c32670fd454939794504225dca1d4ec55045d5.
Rolling-forward with trivial format-string fix
cmd/compile: adjust RISCV64 rewrite rules to use typed aux fields
Also add a typed version of mergeSym to rewrite.go to assist with a few
rules that used mergeSym in the untyped-form.
Remove a few extra int32 overflow checks that no longer make sense, as
adding two int8s or int16s should never overflow an int32.
Passes toolstash-check -all.
Original review: https://go-review.googlesource.com/c/go/+/228882
Change-Id: Ib63db4ee1687446f0f3d9f11575a40dd85cbce55
Reviewed-on: https://go-review.googlesource.com/c/go/+/229126
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Adds a missing <title> tag to the HTML template to make it
more compliant as <title> tags are generally required for valid
HTML documents.
Change-Id: I1ab2a6ee221c8a79d3cc13d9ac6110f6f4963914
GitHub-Last-Rev: 6d519dc9dd
GitHub-Pull-Request: golang/go#38313
Reviewed-on: https://go-review.googlesource.com/c/go/+/227547
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Each invocation of 'go list' may consume a significant quantity of
system resources, including buffers for reading files and RAM for the
runtime's memory footprint.
Very small builders may even hit swap as a result of that load,
further exacerbating resource contention.
To avoid overloading small builders, restrict 'go list' calls to
runtime.GOMAXPROCS as it is set at the first call to loadImports.
This also somewhat improves running time even on larger machines: on
my workstation, this change reduces the wall time for 'go test
cmd/api' by around 100ms.
Updates #38537
Change-Id: I968e0f961a8f1d84c27e1ab8b621b9670dcfd448
Reviewed-on: https://go-review.googlesource.com/c/go/+/228998
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Also add a typed version of mergeSym to rewrite.go to assist with a few
rules that used mergeSym in the untyped-form.
Remove a few extra int32 overflow checks that no longer make sense, as
adding two int8s or int16s should never overflow an int32.
Passes toolstash-check -all.
Change-Id: I72ddd2b0d9001faa87ad0ab54f500057164661b7
Reviewed-on: https://go-review.googlesource.com/c/go/+/228882
Reviewed-by: Keith Randall <khr@golang.org>
The rewrite loop in shortcircuit is identical to the one in fuse.
That's not surprising; shortcircuit is fuse-like.
Take advantage of that by merging the two loops.
Passes toolstash-check.
Change-Id: I642cb39a23d2ac8964ed577678f062fce721439c
Reviewed-on: https://go-review.googlesource.com/c/go/+/229003
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Previously the Tx would drop the connection after rolling back from
a context cancel. Now if the driver can reset the session,
keep the connection.
Change-Id: Ie6a3124275632787629844d91a06bb2e70cc060b
Reviewed-on: https://go-review.googlesource.com/c/go/+/216241
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It was possible for a Tx that was aborted for rollback
asynchronously to execute a query after the rollback had completed
on the database, which often would auto commit the query outside
of the transaction.
By W-locking the tx.closemu prior to issuing the rollback
connection it ensures any Tx query either fails or finishes
on the Tx, and never after the Tx has rolled back.
Fixes#34775Fixes#32942
Change-Id: I017b7932082f2f4ead70bae08b61ed9068ac1d01
Reviewed-on: https://go-review.googlesource.com/c/go/+/216240
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
With the original connection reuse strategy, it was possible that
when a new connection was requested, the pool would wait for an
an existing connection to return for re-use in a full connection
pool, and then it would check if the returned connection was expired.
If the returned connection expired while awaiting re-use, it would
return an error to the location requestiong the new connection.
The existing call sites requesting a new connection was often the last
attempt at returning a connection for a query. This would then
result in a failed query.
This change ensures that we perform the expiry check right
before a connection is inserted back in to the connection pool
for while requesting a new connection. If requesting a new connection
it will no longer fail due to the connection expiring.
Fixes#32530
Change-Id: If16379befe0e14d90160219c0c9396243fe062f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/216197
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Follow-up to (and similar to) CL 228885.
Triggers a handful of times in std+cmd.
Change-Id: Ie04057ca3974ef9eef669335e326a5ed4b7472cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/228999
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
This has a minor positive effect on generated code,
particularly code using type switches.
Change-Id: I7269769ab0d861ef6fc9e6d7809ffc3573c68340
Reviewed-on: https://go-review.googlesource.com/c/go/+/228885
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Node.NonNil and Node.Bounded were a bit muddled. This led to #38496.
This change clarifies and documents them.
It also corrects one misuse.
However, since ssa conversion doesn't make full use of the bounded hint,
this correction doesn't change any generated code.
The next change will fix that.
Passes toolstash-check.
Change-Id: I2bcd487a0a4aef5d7f6090e653974fce0dce3b8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/228787
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
The core CPU profiling loop contains a 100ms sleep.
This is important to reduce overhead.
However, it means that it takes 200ms to shutting down a program
with CPU profiling enabled. When trying to collect many samples
by running a short-lived program many times, this adds up.
This change cuts the shutdown penalty in half by skipping
the sleep whenever possible.
Change-Id: Ic3177f8e1a2d331fe1a1ecd7c8c06f50beb42535
Reviewed-on: https://go-review.googlesource.com/c/go/+/228886
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>
All callers to gdata knew the kind of node they were working with,
so all calls to gdata have been replaced with more specific calls.
Some OADDR nodes were constructed solely for the purpose of
passing them to gdata for unwrapping. In those cases, we can now
cut to the chase.
Passes toolstash-check.
Change-Id: Iacc1abefd7f748cb269661a03768d3367319b0b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/228888
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is followup of CL 228861, which remove another un-necessary nil
check for s.Pkg.
Passes toolstash-check.
Change-Id: Ide750beddd2594199af21b56ec6af734dfa55b9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/228862
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 228859 refactored detecting reflect package logic in to isReflectPkg
function. The function has un-necessary nil check for p, so remove that
check.
Passes toolstash-check.
Change-Id: I2f3f1ac967fe8d176dda3f3b4698ded08602e2fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/228861
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
A DWARF testpoint was calling t.Fatal() but should have been calling
t.Fatalf(); switch it to the correct method.
Change-Id: I996a1041adea4299cda85c147a35b513a219b970
Reviewed-on: https://go-review.googlesource.com/c/go/+/228790
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jeremy Faller <jeremy@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Convert all the 386 lowering rules to the typed aux form.
Passes
GOARCH=386 gotip build -toolexec 'toolstash -cmp' -a std
Change-Id: I15256f20bc4442391755e6fffb8206dcaab94830
Reviewed-on: https://go-review.googlesource.com/c/go/+/228818
Reviewed-by: Keith Randall <khr@golang.org>
Currently we only check for reflect.Value.Method. And
reflect.Value.MethodByName is covered since it calls
reflect.Value.Method internally. But it is brittle to rely on
implementation detail of the reflect package. Check for
MethodByName explicitly.
Change-Id: Ifa8920e997524003dade03abc4fb3c4e64723643
Reviewed-on: https://go-review.googlesource.com/c/go/+/228881
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
reflect.Type.Method (and MethodByName) can be used to obtain a
reference of a method by reflection. The linker needs to know
if reflect.Type.Method is called, and retain all exported methods
accordingly. This is handled by the compiler, which marks the
caller of reflect.Type.Method with REFLECTMETHOD attribute. The
current code failed to handle the reflect package itself, so the
method wrapper reflect.Type.Method is not marked. This CL fixes
it.
Fixes#38515.
Change-Id: I12904d23eda664cf1794bc3676152f3218fb762b
Reviewed-on: https://go-review.googlesource.com/c/go/+/228880
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
reflect.Value.Call, if reachable, used to bring all exported
methods live. CL 228792 fixes this, removing the check of
reflect.Value.Call. This CL adds a test.
Updates #38505.
Change-Id: Ib4cab3c3c86c9c9702d041266e59b159d0ff0a97
Reviewed-on: https://go-review.googlesource.com/c/go/+/228878
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The types used while generating code, such as Rule and File, have been
exported for a while. This is harmless for a main package, and lets us
easily differentiate types from variables and functions, as well as use
names like "If" since "if" is a keyword.
However, the fields remained unexported. This was a bit inconsistent,
and also meant that we couldn't use some intuitive names like If.else.
Export them.
Besides the capitalization, the only change is that the If type now has
the fields Then and Else, instead of stmt and alt.
Change-Id: I426ff140c6ca186fec394f17b29165861da5fd98
Reviewed-on: https://go-review.googlesource.com/c/go/+/228821
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Update the comment to be in sync with the code.
Change-Id: I19586767a37347c4da1b4d3f7c6dc6cc2292a90f
Reviewed-on: https://go-review.googlesource.com/c/go/+/228877
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
In the linker's deadcode pass, we need to keep a method live if
it can be reached through reflection. We do this by marking all
exported method live if reflect.Value.Method or
reflect.Type.Method is used. Currently we also check for
reflect.Value.Call, which is unnecessary because in order to call
a method through reflection, the method must be obtained through
reflect.Value.Method or reflect.Type.Method, which we already
check.
Per discussion in https://groups.google.com/d/msg/golang-dev/eG9It63-Bxg/_bnoVy-eAwAJ
Thanks Brad, Russ, and Ian for bringing this up.
Change-Id: I8e9529a224bb898dbf5752674cc9d155db386c14
Reviewed-on: https://go-review.googlesource.com/c/go/+/228792
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
p.literal's doc comment said it returned a value but it doesn't.
While we're here, p.newLiteral is only called from p.literal,
so simplify the code by merging the two.
Change-Id: Ia357937a99f4e7473f0f1ec837113a39eaeb83d4
Reviewed-on: https://go-review.googlesource.com/c/go/+/222659
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Everybody was deferring a flush when main already
did that, so drop all that nonsense. (Flush was doing
the package clause stuff.) But then make sure we do
get a package clause when there is correctly no output,
as for an empty package. Do that by triggering a
package clause in allDoc and packageDoc.
Slightly tricky but way less intricate than before.
Fixes#37969.
Change-Id: Ia86828436e6c4ab46e6fdaf2c550047f37f353f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/226998
Reviewed-by: Russ Cox <rsc@golang.org>
I'm planning to modify this test in a follow-up CL, so we might
as well convert it to a script test. I don't think there's an easy
way to detect whether we have a case-insensitive file system, without
adding a new condition to the script framework, so the test is just
guessing that darwin and windows could have case-insensitive file systems.
Change-Id: I48bb36f86f19898618681515ac448c3bb4735857
Reviewed-on: https://go-review.googlesource.com/c/go/+/228783
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
At least as far as I can tell, this file never explicitly states whether
locks with higher or lower rank should be taken first. It is implied in
some comments, and clear from the code, of course.
Add an explicit comment to make things more clear and hopefully reduce
new locks being adding in the wrong spot.
Change-Id: I17c6fd5fc216954e5f3550cf91f17e25139f1587
Reviewed-on: https://go-review.googlesource.com/c/go/+/228785
Reviewed-by: Dan Scales <danscales@google.com>
When the seconds param is given, the block and mutex profile endpoints
report the difference between two measurements collected the given
seconds apart. Historically, the block and mutex profiles have reported
the cumulative counts since the process start, and it turned out they
are more useful when interpreted along with the time duration.
Note: cpu profile and trace endpoints already accept the "seconds"
parameter. With this CL, the block and mutex profile endpoints will
accept the "seconds" parameter. Providing the "seconds" parameter
to other types of profiles is an error.
This change moves runtime/pprof/internal/profile to internal/profile and
adds part of merge logic from github.com/google/pprof/profile/merge.go to
internal/profile, in order to allow both net/http/pprof and runtime/pprof
to access it.
Fixes#23401
Change-Id: Ie2486f1a63eb8ff210d7d3bc2de683e9335fd5cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/147598
Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
