Commit graph

14 commits

Author SHA1 Message Date
Matthew Dempsky
063a91c0ab cmd/compile: fix recognition of unnamed return variables
In golang.org/cl/266199, I reused the existing code in inlining that
recognizes anonymous variables. However, it turns out that code
mistakenly recognizes anonymous return parameters as named when
inlining a function from the same package.

The issue is funcargs (which is only used for functions parsed from
source) synthesizes ~r names for anonymous return parameters, but
funcargs2 (which is only used for functions imported from export data)
does not.

This CL fixes the behavior so that anonymous return parameters are
handled identically whether a function is inlined within the same
package or across packages. It also adds a proper cross-package test
case demonstrating #33160 is fixed in both cases.

Change-Id: Iaa39a23f5666979a1f5ca6d09fc8c398e55b784c
Reviewed-on: https://go-review.googlesource.com/c/go/+/266719
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
Trust: Matthew Dempsky <mdempsky@google.com>
2020-11-01 01:06:56 +00:00
Matthew Dempsky
1bcf6beec5 cmd/compile: use staticValue for inlining logic
This CL replaces the ad hoc and duplicated logic for detecting
inlinable calls with a single "inlCallee" function, which uses the
"staticValue" helper function introduced in an earlier commit.

Updates #41474.

Change-Id: I103d4091b10366fce1344ef2501222b7df68f21d
Reviewed-on: https://go-review.googlesource.com/c/go/+/256460
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
2020-10-15 18:35:44 +00:00
Matthew Dempsky
cced777026 cmd/compile: set n.Name.Defn for inlined parameters
Normally, when variables are declared and initialized using ":=", we
set the variable's n.Name.Defn to point to the initialization
assignment node (i.e., OAS or OAS2). Further, some frontend
optimizations look for variables that are initialized but never
reassigned.

However, when inl.go inlines calls, it was declaring the inlined
variables, and then separately assigning to them. This CL changes
inl.go tweaks the AST to fit the combined declaration+initialization
pattern.

This isn't terribly useful by itself, but it allows further followup
optimizations.

Updates #41474.

Change-Id: I62a9752c60414305679e0ed15a6563baa0224efa
Reviewed-on: https://go-review.googlesource.com/c/go/+/256457
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2020-10-15 18:25:47 +00:00
David Chase
6f02578f9c cmd/compile: fix logopt log directory naming for windows
Allow Windows absolute paths, also fixed URI decoding on Windows.
Added a test, reorganized to make the test cleaner.
Also put some doc comments on exported functions that did not have them.

Fixes #41614.

Change-Id: I2871be0e5183fbd53ffb309896d6fe56c15a7727
Reviewed-on: https://go-review.googlesource.com/c/go/+/257677
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
2020-09-26 22:01:34 +00:00
Joel Sing
c39439c53f cmd/compile: run TestLogOpt for riscv64 on amd64
Run TestLogOpt for riscv64 on amd64, as is done for other architectures.
This would have caught the test failure on riscv64 introduced in
47ade08141.

Change-Id: If29dea2ef383b087154d046728f6d1c96811f5a3
Reviewed-on: https://go-review.googlesource.com/c/go/+/227806
Run-TryBot: Joel Sing <joel@sing.id.au>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2020-04-13 04:05:32 +00:00
Alex Brainman
df15eaedd0 Revert "cmd/compile: make logopt test skip if cannot create scratch directory"
This reverts commit 98534812bd.

Reason for revert: The change does not really fixes issue #38251. CL 227497 is real fix.

Change-Id: I9f556005baf1de968f059fb8dad89dae05330aa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/227802
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
2020-04-11 13:49:21 +00:00
David Chase
06314b620d cmd/compile: add explanations to escape-analysis JSON/LSP logging
For 1.15.

From the test:

{"range":{"start":{"line":7,"character":13},"end":{...},"severity":3,"code":"leaks","source":"go compiler","message":"parameter z leaks to ~r2 with derefs=0","relatedInformation":[
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow:    flow: y = z:"},
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow:      from y = \u003cN\u003e (assign-pair)"},
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow:    flow: ~r1 = y:"},
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":11},"end":{...}},"message":"inlineLoc"},

	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow:      from y.b (dot of pointer)"},
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":11},"end":{...}},"message":"inlineLoc"},

	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow:      from \u0026y.b (address-of)"},
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":4,"character":9},"end":...}},"message":"inlineLoc"},

	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":13},"end":{...}},"message":"escflow:      from ~r1 = \u003cN\u003e (assign-pair)"},
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":3},"end":...}},"message":"escflow:    flow: ~r2 = ~r1:"},
	{"location":{"uri":"file://T/file.go","range":{"start":{"line":9,"character":3},"end":...}},"message":"escflow:      from return (*int)(~r1) (return)"}]}

Change-Id: Idf02438801f63e487c35a928cf5a0b6d3cc48674
Reviewed-on: https://go-review.googlesource.com/c/go/+/206658
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-04-11 02:04:15 +00:00
David Chase
fced302aa1 cmd/compile: change gc logging to report inline failure instead of success
I've been experimenting with this, success is the wrong thing to report
even though it seems to log much less.

Change-Id: I7c25a45d2f41e82b6c8dd8b0a56ba848c63fb21a
Reviewed-on: https://go-review.googlesource.com/c/go/+/223298
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-04-10 20:34:40 +00:00
Egon Elbre
64f19d7080 cmd/compile/internal/logopt: preserve env while running command
The test was not preserving temporary directory flags leading to a
failure on windows with:

    mkdir C:\WINDOWS\go-build315158903: Access is denied.

Fixes #38251

Change-Id: I6ee31b31e84b7f6e75ea6ee0f3b8c094835bf5d2
Reviewed-on: https://go-review.googlesource.com/c/go/+/227497
Reviewed-by: David Chase <drchase@google.com>
2020-04-07 17:18:30 +00:00
David Chase
98534812bd cmd/compile: make logopt test skip if cannot create scratch directory
Fixes #38251.

Change-Id: Ic635843fb503484a1c9a230b0cca571393d3da5a
Reviewed-on: https://go-review.googlesource.com/c/go/+/227339
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
2020-04-06 15:22:07 +00:00
David Chase
47ade08141 cmd/compile: add logging for large (>= 128 byte) copies
For 1.15, unless someone really wants it in 1.14.

A performance-sensitive user thought this would be useful,
though "large" was not well-defined.  If 128 is large,
there are 139 static instances of "large" copies in the compiler
itself.

Includes test.

Change-Id: I81f20c62da59d37072429f3a22c1809e6fb2946d
Reviewed-on: https://go-review.googlesource.com/c/go/+/205066
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2020-04-03 17:24:48 +00:00
David Chase
4d0ed149ff cmd/compile: enable optimizer logging for inline-related events
Change-Id: I72de8cb5e1df7a73e46a4b7e5b4e7290fcca4bc1
Reviewed-on: https://go-review.googlesource.com/c/go/+/204162
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-12 19:48:38 +00:00
David Chase
46c9fd03a5 cmd/compile: enable optimizer logging for bounds checking
Change-Id: Ic1fc271589b7212e7f604ece93cfe34feff909b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/204160
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-10 17:12:35 +00:00
David Chase
cd53fddabb cmd/compile: add framework for logging optimizer (non)actions to LSP
This is intended to allow IDEs to note where the optimizer
was not able to improve users' code.  There may be other
applications for this, for example in studying effectiveness
of optimizer changes more quickly than running benchmarks,
or in verifying that code changes did not accidentally disable
optimizations in performance-critical code.

Logging of nilcheck (bad) for amd64 is implemented as
proof-of-concept.  In general, the intent is that optimizations
that didn't happen are what will be logged, because that is
believed to be what IDE users want.

Added flag -json=version,dest

Check that version=0.  (Future compilers will support a
few recent versions, I hope that version is always <=3.)

Dest is expected to be one of:

/path (or \path in Windows)
  will create directory /path and fill it w/ json files
file://path
  will create directory path, intended either for
     I:\dont\know\enough\about\windows\paths
     trustme_I_know_what_I_am_doing_probably_testing

Not passing an absolute path name usually leads to
json splattered all over source directories,
or failure when those directories are not writeable.
If you want a foot-gun, you have to ask for it.

The JSON output is directed to subdirectories of dest,
where each subdirectory is net/url.PathEscape of the
package name, and each for each foo.go in the package,
net/url.PathEscape(foo).json is created.  The first line
of foo.json contains version and context information,
and subsequent lines contains LSP-conforming JSON
describing the missing optimizations.

Change-Id: Ib83176a53a8c177ee9081aefc5ae05604ccad8a0
Reviewed-on: https://go-review.googlesource.com/c/go/+/204338
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
2019-11-10 17:11:34 +00:00