| Age | Commit message (Collapse) | Author |
|---|
|
Now that GOEXPERIMENT=nounified is removed, we can assume InlineCall
and HaveInlineBody will always be overridden with the unified
frontend's implementations. Similarly, we can assume expandDecl will
never be called.
This CL changes the code paths into Fatalfs, so subsequent CLs can
remove all the unreachable code.
Updates #57410.
Change-Id: I2a0c3edb32916c30dd63c4dce4f1bd6f18e07468
Reviewed-on: https://go-review.googlesource.com/c/go/+/458618
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
This flag forced the compiler to eagerly type check all available
inline function bodies, which presumably was useful in the early days
of implementing inlining support. However, it shouldn't have any
significance with the unified frontend, since the same code paths are
used for constructing normal function bodies as for inlining.
Updates #57410.
Change-Id: I6842cf86bcd0fbf22ac336f2fc0b7b8fe14bccca
Reviewed-on: https://go-review.googlesource.com/c/go/+/458617
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This CL removes the GOEXPERIMENT=nounified knob, and any conditional
statements that depend on that knob. Further CLs to remove unreachable
code follow this one.
Updates #57410.
Change-Id: I39c147e1a83601c73f8316a001705778fee64a91
Reviewed-on: https://go-review.googlesource.com/c/go/+/458615
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
When -m=N (where N > 1) is in effect, include a note in the trace
output if a given function is considered "big" during inlining
analysis, since this causes the inliner to be less aggressive. If a
small change to a large function happens to nudge it over the large
function threshold, it can be confusing for developers, thus it's
probably worth including this info in the remark output.
Change-Id: Id31a1b76371ab1ef9265ba28a377f97b0247d0a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/460317
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Keith Randall <khr@google.com>
|
|
This is the second round to look for spelling mistakes. This time the
manual sifting of the result list was made easier by filtering out
capitalized and camelcase words.
grep -r --include '*.go' -E '^// .*$' . | aspell list | grep -E -x '[A-Za-z]{1}[a-z]*' | sort | uniq
This PR will be imported into Gerrit with the title and first
comment (this text) used to generate the subject and body of
the Gerrit change.
Change-Id: Ie8a2092aaa7e1f051aa90f03dbaf2b9aaf5664a9
GitHub-Last-Rev: fc2bd6e0c51652f13a7588980f1408af8e6080f5
GitHub-Pull-Request: golang/go#57737
Reviewed-on: https://go-review.googlesource.com/c/go/+/461595
Auto-Submit: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
|
|
Currently, we use CDF to compute a weight threshold and then use
the weight threshold to determine whether a call site is hot. As
when we compute the CDF we already have a list of hot call sites
that make up the given percentage of the CDF, just use that list.
Also, when computing the CDF threshold, include the very last node
that makes it to go over the threshold. (I.e. if the CDF threshold
is 50% and one hot node takes 60% of weight, we should include that
node instead of excluding it. In practice it rarely matters,
probably only for testing and micro-benchmarks.)
Change-Id: I535ae9cd6b679609e247c3d0d9ee572c1a1187cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/450737
Reviewed-by: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
|
|
Adjust PGO inlining default parameters to 99% CDF threshold and
2000 budget. Benchmark results (mostly from Sweet) show that this
set of parameters performs reasonably well, with a few percent
speedup at the cost of a few percent binary size increase.
Also rename the debug flags to start with "pgo", to make it clear
that they are related to PGO.
Change-Id: I0749249b1298d1dc55a28993c37b3185f9d7639d
Reviewed-on: https://go-review.googlesource.com/c/go/+/449477
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
If an imported, non-generic function F transitively calls a generic
function G[T], we may need to call CanInline on G[T].
While here, we can also take advantage of the fact that we know G[T]
was already seen and compiled in an imported package, so we don't need
to call InlineCalls or add it to typecheck.Target.Decls. This saves us
from wasting compile time re-creating DUPOK symbols that we know
already exist in the imported package's link objects.
Fixes #56280.
Change-Id: I3336786bee01616ee9f2b18908738e4ca41c8102
Reviewed-on: https://go-review.googlesource.com/c/go/+/443535
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
|
|
- Include the callee names in hot call inlining message.
- Print the graph when pgoinline >= 2.
Change-Id: Iceb89b5f18cefc69ab9256aca9a910743d22ec0f
Reviewed-on: https://go-review.googlesource.com/c/go/+/448496
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Appears to be a typo in CL 447315.
Change-Id: I9f380a3c7521f5ac5a1d7e271eaa60bd4bbcfb29
Reviewed-on: https://go-review.googlesource.com/c/go/+/448515
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Rather than matching calls to edges in the profile based directly on
line number in the source file, use the line offset from the start of
the function. This makes matching robust to changes in the source file
above the function containing the call.
The start line in the profile comes from Function.start_line, which is
included in Go pprof output since CL 438255.
Currently it is an error if no samples set start_line to help users
detect profiles missing this information. In the future, we should
fallback to using absolute lines, which is better than nothing.
For #55022.
Change-Id: Ie621950cfee1fef8fb200907a2a3f1ded41d04fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/447315
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
With CL 447015, we identify hot callees from edge weights, but
the code only traverses edges for calls from the current package.
If the callee is in a different package, when compiling that
package, the edge was not visited, so the callee was not actually
marked inline candidate. This CL fixes it by traversing all hot
edges.
Change-Id: If668c1a16ebe34e3474376b88ab3a84be76b8562
Reviewed-on: https://go-review.googlesource.com/c/go/+/448015
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Currently in PGO we use a percentage threshold to determine if a
callsite is hot. This CL uses a different method -- treating the
hottest callsites that make up cumulatively top X% of total edge
weights as hot (X=95 for now). This default might work better for
a wider range of profiles. (The absolute threshold can still be
changed by a flag.)
For #55022.
Change-Id: I7e3b6f0c3cf23f9a89dd5994c10075b498bf14ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/447016
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
|
|
Currently, with PGO, the inliner uses node weights to decide if a
function is inlineable (with a larger budget). But the actual
inlining is determined by the weight of the call edge. There is a
discrepancy that, if a callee node is hot but the call edge is not,
it would not inlined, and marking the callee inlineable would of
no use.
Instead of using two kinds of weights, we just use the edge
weights to decide inlineability. If a function is the callee of a
hot call edge, its inlineability is determined with a larger
threshold. For a function that exceeds the regular inlining budget,
it is still inlined only when the call edge is hot, as it would
exceed the regular inlining cost for non-hot call sites, even if
it is marked inlineable.
For #55022.
Change-Id: I93fa9919fc6bcbb394e6cfe54ec96a96eede08f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/447015
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
The global ListOfHotCallSites set is used to communicate between
CanInline and InlineCalls the set of call sites that InlineCalls may
increase the budget for.
CanInline clears this map on each call, thus assuming that
InlineCalls(x) is called immediately after CanInline(x). This assumption
is false, as CanInline (among other cases) is recursive (CanInline ->
hairyVisitor.doNode -> inlCallee -> CanInline).
When this assumption proves false, we will lose the opportunity to
inline hot calls.
This CL is the least invasive fix for this. ListOfHotCallSites is
actually just a subset of the candHotEdgeMap, with CallSiteInfo.Callee
cleared. candHotEdgeMap doesn't actually need to distinguish based on
Callee, so we can drop callee from candHotEdgeMap as well and just use
that directly [1].
Later CLs should do more work to remove the globals entirely.
For cmd/compile, this inceases the number of PGO inlined functions by
~50% for one set of PGO parameters. I have no evaluated performance
impact.
[1] This is something that we likely want to change in the future.
For #55022.
Change-Id: I57735958d651f6dfa9bd296499841213d20e1706
Reviewed-on: https://go-review.googlesource.com/c/go/+/446755
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This patch fixes a typo/bug introduced in CL 441858 where when pattern
matching a coverage counter access we were looking at an assingment
node instead of the assignment LHS, and fixes a similar problem in
atomic counter update pattern matching introduced in CL 444835. In
both of these cases the bug was not caught because the test intended
to lock down the behavior was written incorrectly (wasn't
instrumenting what the test author thought it was instrumenting,
ouch).
Change-Id: I6e6ac3beacf12ef1a817de5527340b639f0bb044
Reviewed-on: https://go-review.googlesource.com/c/go/+/446258
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Than McIntosh <thanm@google.com>
|
|
Since pgo is a new package, it is reasonably straightforward to
encapsulate its state into a non-global object that we pass around,
which will help keep it isolated.
There are no functional changes in this CL, just packaging up the
globals into a new object.
There are two major pieces of cleanup remaining:
1. reflectdata and noder have separate InlineCalls calls for method
wrappers. The Profile is not plumbed there yet, but this is not a
regression as the globals were previously set only right around the
main inlining pass in gc.Main.
2. pgo.ListOfHotCallSites is still global, as it will require more work
to clean up. It is effectively a local variable in InlinePackage,
except that it assumes that InlineCalls is immediately preceded by a
CanInline call for the same function. This is not necessarily true
due to the recursive nature of CanInline. This also means that some
InlineCalls calls may be missing the list of hot callsites right now.
For #55022.
Change-Id: Ic1fe41f73df96861c65f8bfeecff89862b367290
Reviewed-on: https://go-review.googlesource.com/c/go/+/446303
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
|
|
Parts of package pgo fetch the line number of a node by parsing the
number out of the string returned from ir.Line().
This is indirect and inefficient, so it should be replaced with a more
direct lookup. It is also potentially buggy: ir.Line uses
ctxt.OutermostPos, i.e., the line number where an inlined node in
inlined. We want ctxt.InnermostPos, because that is the line number used
in pprof profiles that we are matching against (See comments on
OutermostPos and InnermostPos).
I'm not sure whether this was an active, as we use ir.Line before and
during inlining. I think we could see CALL nodes with OutermostPos !=
InnermostPos during midstack inlining, but I am not sure. Regardless,
explicitly using the desired position is clearer.
For #55022.
Change-Id: Ic640761c9e1d01cacbf91f3aaeaf284ad7e38dbd
Reviewed-on: https://go-review.googlesource.com/c/go/+/446302
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
|
|
For #55022
Change-Id: I51f1ba166d5a66dcaf4b280756be4a6bf9545c5e
Reviewed-on: https://go-review.googlesource.com/c/go/+/429863
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
|
|
Change-Id: I9b18b29e14a47765dc09ac401989e0439fbf7d03
GitHub-Last-Rev: 7d9792ccb97f8e20bc5300cb4fa29a0c49d9934b
GitHub-Pull-Request: golang/go#56267
Reviewed-on: https://go-review.googlesource.com/c/go/+/443296
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
|
|
This patch fixes up a bug in the inliner's special case code for
coverage counter updates, which was not properly working for
-covermode=atomic compilations.
Updates #56044.
Change-Id: I9e309312b123121c3df02862623bdbab1f6c6a4b
Reviewed-on: https://go-review.googlesource.com/c/go/+/441858
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
|
|
The linker needs FuncInfo metadata for all inlined functions. This is
typically handled by gc.enqueueFunc calling ir.InitLSym for all function
declarations in typecheck.Target.Decls (ir.UseClosure adds all closures
to Decls).
However, non-trivial closures in Decls are ignored, and are insteaded
enqueued when walk of the calling function discovers them.
This presents a problem for direct calls to closures. Inlining will
replace the entire closure definition with its body, which hides the
closure from walk and thus suppresses symbol creation.
Explicitly create a symbol early in this edge case to ensure we keep
this metadata.
InitLSym needs to move out of ssagen to avoid a circular dependency (it
doesn't have anything to do with ssa anyway). There isn't a great place
for it, so I placed it in ir, which seemed least objectionable.
The added test triggers one of these inlined direct non-trivial closure
calls, though the test needs CL 429637 to fail, which adds a FuncInfo
assertion to the linker. Note that the test must use "run" instead of
"compile" since the assertion is in the linker, and "compiler" doesn't
run the linker.
Fixes #54959.
Change-Id: I0bd1db4f3539a78da260934cd968372b7aa92546
Reviewed-on: https://go-review.googlesource.com/c/go/+/436240
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Add a new "coverage counter" classification for variables to be used
for storing code coverage counter values (somewhat in the same way
that we identify fuzzer counters). Tagging such variables allows us to
aggregate them in the linker, and to treat updates specially.
Updates #51430.
Change-Id: Ib49fb05736ffece98bcc2f7a7c37e991b7f67bbb
Reviewed-on: https://go-review.googlesource.com/c/go/+/401235
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
cost 0
Accessing the address of something often needs the same (or even less)
number of instructions as accessing the content of the thing. That would
help us rolling back the hack of CL 429766 to lower sync atomic types
inline cost.
Compiled objects size increase a bit:
file before after Δ %
addr2line 3729827 3733958 +4131 +0.111%
api 5457224 5456267 -957 -0.018%
asm 4806486 4808993 +2507 +0.052%
buildid 2480271 2480562 +291 +0.012%
cgo 4593496 4593947 +451 +0.010%
compile 23906958 23910086 +3128 +0.013%
cover 4680870 4681461 +591 +0.013%
dist 3341333 3341692 +359 +0.011%
doc 3879927 3880409 +482 +0.012%
fix 3298081 3298979 +898 +0.027%
link 6500098 6499873 -225 -0.003%
nm 3654362 3656997 +2635 +0.072%
objdump 4108300 4108671 +371 +0.009%
pack 2255445 2256391 +946 +0.042%
pprof 14364561 14379475 +14914 +0.104%
test2json 2550942 2555333 +4391 +0.172%
trace 13573199 13578409 +5210 +0.038%
vet 7430923 7430094 -829 -0.011%
total 114612303 114651597 +39294 +0.034%
file before after Δ %
archive/tar.a 905032 905560 +528 +0.058%
archive/zip.a 853464 853916 +452 +0.053%
cmd/asm/internal/lex.a 359388 367418 +8030 +2.234%
cmd/compile/internal/importer.a 947206 947734 +528 +0.056%
cmd/compile/internal/inline.a 563390 566828 +3438 +0.610%
cmd/compile/internal/types2.a 5761990 5764274 +2284 +0.040%
cmd/go/internal/cfg.a 234892 235342 +450 +0.192%
cmd/go/internal/envcmd.a 257166 257694 +528 +0.205%
cmd/go/internal/fix.a 93522 94052 +530 +0.567%
cmd/go/internal/generate.a 201308 201838 +530 +0.263%
cmd/go/internal/get.a 207862 208390 +528 +0.254%
cmd/go/internal/imports.a 230266 230794 +528 +0.229%
cmd/go/internal/list.a 385044 386632 +1588 +0.412%
cmd/go/internal/load.a 1164508 1165566 +1058 +0.091%
cmd/go/internal/modcmd.a 627582 629168 +1586 +0.253%
cmd/go/internal/modfetch/codehost.a 1031962 1032490 +528 +0.051%
cmd/go/internal/modfetch.a 1289294 1289822 +528 +0.041%
cmd/go/internal/modget.a 674566 675624 +1058 +0.157%
cmd/go/internal/modindex.a 935598 936576 +978 +0.105%
cmd/go/internal/modload.a 2640784 2642058 +1274 +0.048%
cmd/go/internal/par.a 135858 136476 +618 +0.455%
cmd/go/internal/run.a 127158 127688 +530 +0.417%
cmd/go/internal/search.a 242918 243446 +528 +0.217%
cmd/go/internal/trace.a 113216 113188 -28 -0.025%
cmd/go/internal/vcs.a 517280 517810 +530 +0.102%
cmd/go/internal/work.a 2389522 2390580 +1058 +0.044%
cmd/go/internal/workcmd.a 311118 311452 +334 +0.107%
cmd/vendor/github.com/google/pprof/internal/driver.a 1714950 1715478 +528 +0.031%
cmd/vendor/golang.org/x/mod/sumdb.a 453840 454290 +450 +0.099%
cmd/vendor/golang.org/x/tools/go/analysis/internal/analysisflags.a 326162 326610 +448 +0.137%
cmd/vendor/golang.org/x/tools/go/analysis/internal/facts.a 302476 303006 +530 +0.175%
cmd/vendor/golang.org/x/tools/go/analysis/passes/asmdecl.a 366580 367030 +450 +0.123%
cmd/vendor/golang.org/x/tools/go/analysis/passes/assign.a 129556 130006 +450 +0.347%
cmd/vendor/golang.org/x/tools/go/analysis/passes/atomic.a 133466 133916 +450 +0.337%
cmd/vendor/golang.org/x/tools/go/analysis/passes/bools.a 193558 194006 +448 +0.231%
cmd/vendor/golang.org/x/tools/go/analysis/passes/buildtag.a 177984 178434 +450 +0.253%
cmd/vendor/golang.org/x/tools/go/analysis/passes/cgocall.a 221226 221674 +448 +0.203%
cmd/vendor/golang.org/x/tools/go/analysis/passes/composite.a 168572 169022 +450 +0.267%
cmd/vendor/golang.org/x/tools/go/analysis/passes/copylock.a 227040 227490 +450 +0.198%
cmd/vendor/golang.org/x/tools/go/analysis/passes/ctrlflow.a 204650 205098 +448 +0.219%
cmd/vendor/golang.org/x/tools/go/analysis/passes/errorsas.a 138020 138468 +448 +0.325%
cmd/vendor/golang.org/x/tools/go/analysis/passes/framepointer.a 119030 119480 +450 +0.378%
cmd/vendor/golang.org/x/tools/go/analysis/passes/httpresponse.a 165006 165454 +448 +0.272%
cmd/vendor/golang.org/x/tools/go/analysis/passes/ifaceassert.a 180850 181300 +450 +0.249%
cmd/vendor/golang.org/x/tools/go/analysis/passes/inspect.a 103876 104326 +450 +0.433%
cmd/vendor/golang.org/x/tools/go/analysis/passes/internal/analysisutil.a 116070 116516 +446 +0.384%
cmd/vendor/golang.org/x/tools/go/analysis/passes/loopclosure.a 153068 153518 +450 +0.294%
cmd/vendor/golang.org/x/tools/go/analysis/passes/lostcancel.a 244936 245384 +448 +0.183%
cmd/vendor/golang.org/x/tools/go/analysis/passes/nilfunc.a 135720 136168 +448 +0.330%
cmd/vendor/golang.org/x/tools/go/analysis/passes/printf.a 527134 527584 +450 +0.085%
cmd/vendor/golang.org/x/tools/go/analysis/passes/shift.a 172026 172476 +450 +0.262%
cmd/vendor/golang.org/x/tools/go/analysis/passes/sigchanyzer.a 151690 152138 +448 +0.295%
cmd/vendor/golang.org/x/tools/go/analysis/passes/stdmethods.a 187494 187944 +450 +0.240%
cmd/vendor/golang.org/x/tools/go/analysis/passes/stringintconv.a 164752 165200 +448 +0.272%
cmd/vendor/golang.org/x/tools/go/analysis/passes/structtag.a 200144 200594 +450 +0.225%
cmd/vendor/golang.org/x/tools/go/analysis/passes/testinggoroutine.a 161146 161596 +450 +0.279%
cmd/vendor/golang.org/x/tools/go/analysis/passes/tests.a 270252 270702 +450 +0.167%
cmd/vendor/golang.org/x/tools/go/analysis/passes/unmarshal.a 130646 131094 +448 +0.343%
cmd/vendor/golang.org/x/tools/go/analysis/passes/unreachable.a 182130 182580 +450 +0.247%
cmd/vendor/golang.org/x/tools/go/analysis/passes/unsafeptr.a 153646 154094 +448 +0.292%
cmd/vendor/golang.org/x/tools/go/analysis/passes/unusedresult.a 179800 180248 +448 +0.249%
cmd/vendor/golang.org/x/tools/go/analysis/unitchecker.a 303838 304286 +448 +0.147%
cmd/vendor/golang.org/x/tools/go/analysis.a 217930 218380 +450 +0.206%
cmd/vendor/golang.org/x/tools/go/ast/astutil.a 539428 539874 +446 +0.083%
cmd/vendor/golang.org/x/tools/go/cfg.a 286820 287270 +450 +0.157%
cmd/vendor/golang.org/x/tools/go/types/objectpath.a 236144 236674 +530 +0.224%
cmd/vendor/golang.org/x/tools/go/types/typeutil.a 412728 413176 +448 +0.109%
cmd/vendor/golang.org/x/tools/internal/analysisinternal.a 223256 223704 +448 +0.201%
cmd/vendor/golang.org/x/tools/internal/typeparams.a 419498 419946 +448 +0.107%
context.a 210000 209972 -28 -0.013%
crypto/internal/boring/bcache.a 8652 8568 -84 -0.971%
crypto/tls.a 3295282 3295202 -80 -0.002%
database/sql.a 1365892 1365762 -130 -0.010%
encoding/base64.a 131572 136228 +4656 +3.539%
encoding/binary.a 452546 453076 +530 +0.117%
encoding/gob.a 1690728 1691672 +944 +0.056%
encoding/json.a 1198834 1199276 +442 +0.037%
encoding/xml.a 1035784 1036314 +530 +0.051%
expvar.a 285282 285678 +396 +0.139%
go/ast.a 1175212 1175662 +450 +0.038%
go/build.a 657802 658252 +450 +0.068%
go/doc.a 808002 808452 +450 +0.056%
go/format.a 101378 101824 +446 +0.440%
go/importer.a 101816 102266 +450 +0.442%
go/internal/gccgoimporter.a 593828 594358 +530 +0.089%
go/internal/gcimporter.a 974178 974626 +448 +0.046%
go/internal/srcimporter.a 196600 197050 +450 +0.229%
go/parser.a 1152502 1152946 +444 +0.039%
go/printer.a 910744 911194 +450 +0.049%
go/token.a 299624 299768 +144 +0.048%
go/types.a 5763222 5766118 +2896 +0.050%
hash/crc32.a 128130 128098 -32 -0.025%
internal/fuzz.a 1058644 1059174 +530 +0.050%
internal/poll.a 660412 660382 -30 -0.005%
internal/testenv.a 212792 213320 +528 +0.248%
log/syslog.a 128718 128654 -64 -0.050%
log.a 157330 157274 -56 -0.036%
mime.a 383058 383588 +530 +0.138%
net/http/httptest.a 430550 431000 +450 +0.105%
net/http/pprof.a 306918 307448 +530 +0.173%
net/http.a 7413852 7414074 +222 +0.003%
net/internal/socktest.a 258934 258900 -34 -0.013%
net/rpc/jsonrpc.a 173158 172962 -196 -0.113%
net/rpc.a 634464 634914 +450 +0.071%
net.a 3539574 3541348 +1774 +0.050%
os.a 891416 891390 -26 -0.003%
reflect.a 3956224 3956666 +442 +0.011%
runtime/cgo.a 187406 187852 +446 +0.238%
runtime/trace.a 85720 85616 -104 -0.121%
runtime.a 9357520 9371302 +13782 +0.147%
sync/atomic.a 232512 232376 -136 -0.058%
sync.a 353112 355068 +1956 +0.554%
syscall.a 1660308 1660222 -86 -0.005%
testing.a 1399348 1399198 -150 -0.011%
text/template.a 1384750 1384726 -24 -0.002%
total 265209524 265294628 +85104 +0.032%
Change-Id: I21114dcddeb4fc2c56e781ea2f6e732fe3da2b01
Reviewed-on: https://go-review.googlesource.com/c/go/+/431095
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
Go 1.19 introduce new append-like APIs in package encoding/binary, this
change teaches the inliner to treat calls to these methods as cheap, so
that code using them will be more inlineable.
Updates #42958
Change-Id: Ie3dd4906e285430f435bdedbf8a11fdffce9302d
Reviewed-on: https://go-review.googlesource.com/c/go/+/431015
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Jenny Rakoczy <jenny@golang.org>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Jenny Rakoczy <jenny@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Keith Randall <khr@google.com>
|
|
In go.dev/cl/419674 I added a mechanism to the inliner to allow
inlining to fail gracefully when a function body is missing, but I
missed we already have a mechanism for that: typecheck.HaveInlineBody.
This CL makes it overridable so that unified IR can plug in its
appropriate logic, like it does with the logic for building the
ir.InlinedCallExpr node.
While here, rename inline.NewInline to inline.InlineCall, because the
name "NewInline" is now a misnomer since we initialize it to oldInline
(now named oldInlineCall).
Change-Id: I4e65618d3725919f69e6f43cf409699d20fb797c
Reviewed-on: https://go-review.googlesource.com/c/go/+/427234
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
|
|
This CL changes the inliner to process transitive inlining iteratively
after the AST has actually been edited, rather than recursively and
immediately. This is important for handling indirect function calls
correctly, because ir.reassigned walks the function body looking for
reassignments; whereas previously the inlined reassignments might not
have been actually added to the AST yet.
Fixes #54632.
Change-Id: I0dd69813c8a70b965174e0072335bc00afedf286
Reviewed-on: https://go-review.googlesource.com/c/go/+/425257
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
The non-unified frontend had repeated issues with inlining and
generics (#49309, #51909, #52907), which led us to substantially
restrict inlining when shape types were present.
However, these issues are evidently not present in unified IR's
inliner, and the safety restrictions added for the non-unified
frontend can simply be disabled in unified mode.
Fixes #54497.
Change-Id: I8e6ac9f3393c588bfaf14c6452891b9640a9d1bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/424775
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
|
|
ir.ClosureExpr implements ir.InitNode, so ir.InitExpr can prepend init
statements to it. However, CalleeEffects wasn't aware of this and
could cause the init statements to get dropped when inlining a call to
a closure.
This isn't an issue today, because we don't create closures with init
statements. But I ran into this within unified IR.
Easy and robust solution: just take advantage that ir.TakeInit can
handle any node.
Change-Id: Ica05fbf6a8c5be4b11927daf84491a1140da5431
Reviewed-on: https://go-review.googlesource.com/c/go/+/422196
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
Change-Id: I20c7df52d110fb88eb22d57bdad9264d0c5e22fe
Reviewed-on: https://go-review.googlesource.com/c/go/+/419674
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
multi-valued expressions
This CL changes GOEXPERIMENT=unified to insert implicit conversions
for multi-valued expressions.
Unfortunately, IR doesn't have strong, first-class support for
multi-valued expressions, so this CL takes the approach of spilling
them to temporary variables, which can then be implicitly converted.
This is the same approach taken by walk, but doing it this early does
introduce some minor complications:
1. For select case clauses with comma-ok assignments (e.g., `case x,
ok := <-ch:`), the compiler middle end wants to see the OAS2RECV
assignment is the CommClause.Comm statement. So when constructing
select statements, we need to massage this around a little.
2. The extra temporary variables and assignments skew the existing
inlining heuristics. As mentioned, the temporaries/assignments will
eventually be added (and often optimized away again) anyway, but now
they're visible to the inliner. So this CL also kludges the inlining
heuristics in this case to keep things comparable.
Change-Id: I3e3ea756ad92472ebe28bae3963be61ed7684a75
Reviewed-on: https://go-review.googlesource.com/c/go/+/415244
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
CL 395854 made inline pass to not inlining function with shape params,
but pass no shape arguments. This is intended to be the reverse case of
CL 361260.
However, CL 361260 is using wider condition than necessary. Though it
only needs to check against function parameters, it checks whether the
function type has no shape. It does not cause any issue, because
!fn.Type().HasShape() implies !fn.Type().Params().HasShape().
But for the reverse case, it's not true. Function may have shape type,
but has no shape arguments. Thus, we must tighten the condition to
explicitly check against the function parameters only.
Fixes #52907
Change-Id: Ib87e87ff767c31d99d5b36aa4a6c1d8baf32746d
Reviewed-on: https://go-review.googlesource.com/c/go/+/406475
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
CL 395854 made inline pass to not inlining function with shape params,
but pass no shape arguments. But it does not consider the case where
function has shape params, but passing zero arguments. In this case, the
un-safe interface conversion that may be applied to a shape argument can
not happen, so it's safe to inline the function.
Fixes #52907
Change-Id: Ifa7b23709bb47b97e27dc1bf32343d92683ef783
Reviewed-on: https://go-review.googlesource.com/c/go/+/406176
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
|
|
This is the same fix as CL 36126, but for the reverse case, function
with shape params but passed no shape arg. The same conversion problem
may occur in this case, see details explanation there.
Fixes #51909
Fixes #51925
Change-Id: Ib0c1973c7511d85b4918a252c80060f1864180cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/395854
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
No longer needed now that IR construction uses types2.
Change-Id: If8b7aff80cd8472be7d87fd3a36da911a5df163c
Reviewed-on: https://go-review.googlesource.com/c/go/+/403839
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Now, 1.17 is the least supported version, the compiler always write
type information when exporting function bodies. So we can get rid of
go117ExportTypes constant and all its conditional checking codes.
Change-Id: I9ac616509c30601e94f99426049d814328253395
Reviewed-on: https://go-review.googlesource.com/c/go/+/402974
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
This CL exports the existing ir.UintptrKeepAlive via the new directive
//go:uintptrkeepalive. This makes the compiler insert KeepAlives for
pointers converted to uintptr in calls, keeping them alive for the
duration of the call.
//go:uintptrkeepalive requires //go:nosplit, as stack growth can't
handle these arguments (it cannot know which are pointers). We currently
check this on the immediate function, but the actual restriction applies
to all transitive calls.
The existing //go:uintptrescapes is an extension of
//go:uintptrkeepalive which forces pointers to escape to the heap, thus
eliminating the stack growth issue.
This pragma is limited to the standard library.
For #51087
Change-Id: If9a19d484d3561b4219e5539b70c11a3cc09391e
Reviewed-on: https://go-review.googlesource.com/c/go/+/388095
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
[This CL is part of a sequence implementing the proposal #51082.
The design doc is at https://go.dev/s/godocfmt-design.]
Run the updated gofmt, which reformats doc comments,
on the main repository. Vendored files are excluded.
For #51082.
Change-Id: I7332f099b60f716295fb34719c98c04eb1a85407
Reviewed-on: https://go-review.googlesource.com/c/go/+/384268
Reviewed-by: Jonathan Amsterdam <jba@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
|
|
Change-Id: I90c8e12a0be05d82bf6e147b5249859518f35c14
Reviewed-on: https://go-review.googlesource.com/c/go/+/394074
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Keith Randall <khr@golang.org>
|
|
This patch revises the fix for issue 46234, fixing a bug that was
accidentally introduced by CL 320913. When inlining a chunk of code
with a closure expression, we want to avoid updating the source
positions in the function being closed over, but we do want to update
the position for the ClosureExpr itself (since it is part of the
function we are inlining). CL 320913 unintentionally did away with the
closure expr source position update; here we restore it again.
Updates #46234.
Fixes #49171.
Change-Id: Iaa51bc498e374b9e5a46fa0acd7db520edbbbfca
Reviewed-on: https://go-review.googlesource.com/c/go/+/366494
Trust: Than McIntosh <thanm@google.com>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
|
|
Currently, we rely on a "crawling" step during export to identify
function and method bodies that need to be exported or re-exported so
we can trim out unnecessary ones and reduce build artifact sizes. To
catch cases where we expect a function to be inlinable but we failed
to export its body, we made this condition a fatal compiler error.
However, with generics, it's much harder to perfectly identify all
function bodies that need to be exported; and several attempts at
tweaking the algorithm have resulted in still having failure cases.
So for now, this CL changes a missing inline body into a graceful
failure instead.
Change-Id: I04b0872d0dcaae9c3de473e92ce584e4ec6fd782
Reviewed-on: https://go-review.googlesource.com/c/go/+/361403
Trust: Matthew Dempsky <mdempsky@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
|
|
Don't inline a function fn that has no shape parameters, but is passed
at least one shape arg. This means we must be inlining a non-generic
function fn that was passed into a generic function, and can be called
with a shape arg because it matches an appropriate type parameter. But
fn may include an interface conversion (that may be applied to a shape
arg) that was not apparent when we first created the instantiation of
the generic function. We can't handle this if we actually do the
inlining, since we want to know all interface conversions immediately
after stenciling. So, we avoid inlining in this case.
Fixes #49309.
Change-Id: I7b8ab7b13e58fdb0111db91bc92a91d313f7c2c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/361260
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
Updates #14768
Change-Id: I33831f616eae5eeb099033e2b9cf90fa70d6ca86
Reviewed-on: https://go-review.googlesource.com/c/go/+/356869
Run-TryBot: Alberto Donizetti <alb.donizetti@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
|
|
Change-Id: Ie802ff27b611ed248d7b14f6e972e6300c181f43
Reviewed-on: https://go-review.googlesource.com/c/go/+/358316
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
|
|
CL 357649 fixes inlining labeled FOR/RANGE loops,
we should do same translation for inlined SWITCH's label
Fixes #49145
Change-Id: I9a6f365f57e974271a1eb279b38e81f9b5148788
Reviewed-on: https://go-review.googlesource.com/c/go/+/358315
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
|
|
There is already a mechanism using inlgen to rename labels insided
inlined functions so that they are unique and don't clash with loops in
the outer function. This is used for OLABEL and OGOTO. Now that we are
doing inlining of OFOR loops, we need to do this translation for OBREAK,
OCONTINUE, and OFOR. I also added the translation for ORANGE loops, in
anticipation of a CL that will allow inlining of ORANGE for loops.
Fixes #49100
Change-Id: I2ccddc3350370825c386965f4a1e4bc54d3c369b
Reviewed-on: https://go-review.googlesource.com/c/go/+/357649
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Dan Scales <danscales@google.com>
|
|
After CL 349012 and CL 350911, we can fully handle these
labeled statements, so we can allow them when inlining.
Updates #14768
Change-Id: I0ab3fd3f8d7436b49b1aedd946516b33c63f5747
Reviewed-on: https://go-review.googlesource.com/c/go/+/355497
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: David Chase <drchase@google.com>
Trust: Dan Scales <danscales@google.com>
|
|
The encoding/binary little- and big-endian load and store routines are
frequently used in performance sensitive code. They look fairly complex
to the inliner. Though the routines themselves can be inlined,
code using them typically cannot be.
Yet they typically compile down to an instruction or two
on architectures that support merging such loads.
This change teaches the inliner to treat calls to these methods as cheap,
so that code using them will be more inlineable.
It'd be better to teach the inliner that this pattern of code is cheap,
rather than these particular methods. However, that is difficult to do
robustly when working with the IR representation. And the broader project
of which that would be a part, namely to model the rest of the compiler
in the inliner, is probably a non-starter. By way of contrast, imperfect
though it is, this change is an easy, cheap, and useful heuristic.
If/when we base inlining decisions on more accurate information obtained
later in the compilation process, or on PGO/FGO, we can remove this
and other such heuristics.
Newly inlineable functions in the standard library:
crypto/cipher.gcmInc32
crypto/sha512.appendUint64
crypto/md5.appendUint64
crypto/sha1.appendUint64
crypto/sha256.appendUint64
vendor/golang.org/x/crypto/poly1305.initialize
encoding/gob.(*encoderState).encodeUint
vendor/golang.org/x/text/unicode/norm.buildRecompMap
net/http.(*http2SettingsFrame).Setting
net/http.http2parseGoAwayFrame
net/http.http2parseWindowUpdateFrame
Benchmark impact for encoding/gob (the only package I measured):
name old time/op new time/op delta
EndToEndPipe-8 2.25µs ± 1% 2.21µs ± 3% -1.79% (p=0.000 n=28+27)
EndToEndByteBuffer-8 93.3ns ± 5% 94.2ns ± 5% ~ (p=0.174 n=30+30)
EndToEndSliceByteBuffer-8 10.5µs ± 1% 10.6µs ± 1% +0.87% (p=0.000 n=30+30)
EncodeComplex128Slice-8 1.81µs ± 0% 1.75µs ± 1% -3.23% (p=0.000 n=28+30)
EncodeFloat64Slice-8 900ns ± 1% 847ns ± 0% -5.91% (p=0.000 n=29+28)
EncodeInt32Slice-8 1.02µs ± 0% 0.90µs ± 0% -11.82% (p=0.000 n=28+26)
EncodeStringSlice-8 1.16µs ± 1% 1.04µs ± 1% -10.20% (p=0.000 n=29+26)
EncodeInterfaceSlice-8 28.7µs ± 3% 29.2µs ± 6% ~ (p=0.067 n=29+30)
DecodeComplex128Slice-8 7.98µs ± 1% 7.96µs ± 1% -0.27% (p=0.017 n=30+30)
DecodeFloat64Slice-8 4.33µs ± 1% 4.34µs ± 1% +0.24% (p=0.022 n=30+29)
DecodeInt32Slice-8 4.18µs ± 1% 4.18µs ± 0% ~ (p=0.074 n=30+28)
DecodeStringSlice-8 13.2µs ± 1% 13.1µs ± 1% -0.64% (p=0.000 n=28+28)
DecodeStringsSlice-8 31.9µs ± 1% 31.8µs ± 1% -0.34% (p=0.001 n=30+30)
DecodeBytesSlice-8 8.88µs ± 1% 8.84µs ± 1% -0.48% (p=0.000 n=30+30)
DecodeInterfaceSlice-8 64.1µs ± 1% 64.2µs ± 1% ~ (p=0.173 n=30+28)
DecodeMap-8 74.3µs ± 0% 74.2µs ± 0% ~ (p=0.131 n=29+30)
Fixes #42958
Change-Id: Ie048b8976fb403d8bcc72ac6bde4b33e133e2a47
Reviewed-on: https://go-review.googlesource.com/c/go/+/349931
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
For certain type of method wrappers we used to generate a tail
call. That was disabled in CL 307234 when register ABI is used,
because with the current IR it was difficult to generate a tail
call with the arguments in the right places. The problem was that
the IR does not contain a CALL-like node with arguments; instead,
it contains an OAS node that adjusts the receiver, than an
OTAILCALL node that just contains the target, but no argument
(with the assumption that the OAS node will put the adjusted
receiver in the right place). With register ABI, putting
arguments in registers are done in SSA. The assignment (OAS)
doesn't put the receiver in register.
This CL changes the IR of a tail call to take an actual OCALL
node. Specifically, a tail call is represented as
OTAILCALL (OCALL target args...)
This way, the call target and args are connected through the OCALL
node. So the call can be analyzed in SSA and the args can be passed
in the right places.
(Alternatively, we could have OTAILCALL node directly take the
target and the args, without the OCALL node. Using an OCALL node is
convenient as there are existing code that processes OCALL nodes
which do not need to be changed. Also, a tail call is similar to
ORETURN (OCALL target args...), except it doesn't preserve the
frame. I did the former but I'm open to change.)
The SSA representation is similar. Previously, the IR lowers to
a Store the receiver then a BlockRetJmp which jumps to the target
(without putting the arg in register). Now we use a TailCall op,
which takes the target and the args. The call expansion pass and
the register allocator handles TailCall pretty much like a
StaticCall, and it will do the right ABI analysis and put the args
in the right places. (Args other than the receiver are already in
the right places. For register args it generates no code for them.
For stack args currently it generates a self copy. I'll work on
optimize that out.) BlockRetJmp is still used, signaling it is a
tail call. The actual call is made in the TailCall op so
BlockRetJmp generates no code (we could use BlockExit if we like).
This slightly reduces binary size:
old new
cmd/go 14003088 13953936
cmd/link 6275552 6271456
Change-Id: I2d16d8d419fe1f17554916d317427383e17e27f0
Reviewed-on: https://go-review.googlesource.com/c/go/+/350145
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
If the condition is a bool constant, there's no need to walk both
branches.
Passes toolstash -cmp.
Change-Id: I4ee5e3553ce07c2213efba0d33d869b4a1b57783
Reviewed-on: https://go-review.googlesource.com/c/go/+/347911
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
|
