mirror of https://github.com/golang/go.git
3851 Commits
| Author | SHA1 | Message | Date |
|---|---|---|---|
Matthew Dempsky
|
6d8d118762 |
[dev.typeparams] all: merge dev.regabi (063c72f) into dev.typeparams
Eager re-sync-branch to keep Git history reasonably accurate, since Git lacks a better way of encoding partial merges like CL 286172. Conflicts: - src/cmd/compile/internal/inline/inl.go - src/cmd/compile/internal/noder/import.go - src/cmd/compile/internal/noder/noder.go Merge List: + 2021-01-25 |
|
|
Matthew Dempsky
|
063c72f06d |
[dev.regabi] cmd/compile: backport changes from dev.typeparams (9456804)
This CL backports a bunch of changes that landed on dev.typeparams, but are not dependent on types2 or generics. By backporting, we reduce the divergence between development branches, hopefully improving test coverage and reducing risk of merge conflicts. Updates #43866. Change-Id: I382510855c9b5fac52b17066e44a00bd07fe86f5 Reviewed-on: https://go-review.googlesource.com/c/go/+/286172 Trust: Matthew Dempsky <mdempsky@google.com> Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Robert Griesemer <gri@golang.org> |
|
Robert Griesemer
|
9456804e86 |
[dev.typeparams] test: fix excluded files lookup so it works on Windows
Updates #43866. Change-Id: I15360de11a48c6f23f25c5ff3a15c117a34127ff Reviewed-on: https://go-review.googlesource.com/c/go/+/286034 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
|
Robert Griesemer
|
7947df436d |
[dev.typeparams] test: set -G=3 and enable more errorcheck tests in run.go
Change-Id: I9591f7aeab0448aca661560bf3064e057b48293e Reviewed-on: https://go-review.googlesource.com/c/go/+/286012 Trust: Robert Griesemer <gri@golang.org> Trust: Dan Scales <danscales@google.com> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dan Scales <danscales@google.com> |
|
Dan Scales
|
48badc5fa8 |
[dev.regabi] cmd/compile: fix escape analysis problem with closures
In reflect.methodWrapper, we call escape analysis without including the full batch of dependent functions, including the closure functions. Because of this, we haven't created locations for the params/local variables of a closure when we are processing a function that inlines that closure. (Whereas in the normal compilation of the function, we do call with the full batch.) To deal with this, I am creating locations for the params/local variables of a closure when needed. Without this fix, the new test closure6.go would fail. Updates #43818 Change-Id: I5f91cfb6f35efe2937ef88cbcc468e403e0da9ad Reviewed-on: https://go-review.googlesource.com/c/go/+/285677 Run-TryBot: Dan Scales <danscales@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Dan Scales <danscales@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
6e46c8fbb5 |
[dev.typeparams] all: merge dev.regabi (7e0a81d) into dev.typeparams
As with CL 285875, this required resolving some conflicts around handling of //go:embed directives. Still further work is needed to reject uses of //go:embed in files that don't import "embed", so this is left as a TODO. (When this code was written for dev.typeparams, we were still leaning towards not requiring the "embed" import.) Also, the recent support for inlining closures (CL 283112) interacts poorly with -G=3 mode. There are some known issues with this code already (#43818), so for now this CL disables inlining of closures when in -G=3 mode with a TODO to revisit this once closure inlining is working fully. Conflicts: - src/cmd/compile/internal/noder/noder.go - src/cmd/compile/internal/typecheck/dcl.go - src/cmd/compile/internal/typecheck/func.go - test/run.go Merge List: + 2021-01-22 |
|
|
Matthew Dempsky
|
7e0a81d280 |
[dev.regabi] all: merge master (dab3e5a) into dev.regabi
This merge had two conflicts to resolve:
1. The embed code on master had somewhat substantially diverged, so
this CL tediously backported the changes to dev.regabi. In particular,
I went through all of the embed changes to gc/{embed,noder,syntax}.go
and made sure the analogous code on dev.regabi in noder/noder.go and
staticdata/embed.go mirrors it.
2. The init-cycle reporting code on master was extended slightly to
track already visited declarations to avoid exponential behavior. The
same fix is applied on dev.regabi, just using ir.NameSet instead of
map[ir.Node]bool.
Conflicts:
- src/cmd/compile/internal/gc/embed.go
- src/cmd/compile/internal/gc/noder.go
- src/cmd/compile/internal/gc/syntax.go
- src/cmd/compile/internal/pkginit/initorder.go
- src/embed/internal/embedtest/embed_test.go
- src/go/types/stdlib_test.go
Merge List:
+ 2021-01-22
|
|
|
Robert Griesemer
|
f8654579cd |
[dev.typeparams] cmd/compile/internal/types2: adjust errors in branch checking code, fix a bug
The types2.Config.IgnoreBranches flag mistakenly excluded a set of label-unrelated branch checks. After fixing this and also adjusting some error messages to match the existing compiler errors, more errorcheck tests pass now with the -G option. Renamed IngnoreBranches to IgnoreLabels since its controlling label checks, not all branch statement (such as continue, etc) checks. Change-Id: I0819f56eb132ce76c9a9628d8942af756691065a Reviewed-on: https://go-review.googlesource.com/c/go/+/285652 Trust: Robert Griesemer <gri@golang.org> Run-TryBot: Robert Griesemer <gri@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
|
Dan Scales
|
1760d736f6 |
[dev.regabi] cmd/compile: exporting, importing, and inlining functions with OCLOSURE
I have exporting, importing, and inlining of functions with closures working in all cases (issue #28727). all.bash runs successfully without errors. Approach: - Write out the Func type, Dcls, ClosureVars, and Body when exporting an OCLOSURE. - When importing an OCLOSURE, read in the type, dcls, closure vars, and body, and then do roughly equivalent code to (*noder).funcLit - During inlining of a closure within inlined function, create new nodes for all params and local variables (including closure variables), so they can have a new Curfn and some other field values. Must substitute not only on the Nbody of the closure, but also the Type, Cvars, and Dcl fields. Fixes #28727 Change-Id: I4da1e2567c3fa31a5121afbe82dc4e5ee32b3170 Reviewed-on: https://go-review.googlesource.com/c/go/+/283112 Run-TryBot: Dan Scales <danscales@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Trust: Dan Scales <danscales@google.com> |
|
|
Robert Griesemer
|
d8796b5670 |
[dev.typeparams] cmd/compile/internal/types2: report type of nil based on context
With this CL, the type reported for uses of the predeclared identifier nil changes from untyped nil to the type of the context within which nil is used, matching the behaviour of types2 for other untyped types. If an untyped nil value is assigned or converted to an interface, the nil expression is given the interface type. The predicate TypeAndValue.IsNil doesn't change in behavior, it still reports whether the relevant expression is a (typed or untyped) nil value. Change-Id: Id766468f3f3f2a53e4c55e1e6cd521e459c4a94f Reviewed-on: https://go-review.googlesource.com/c/go/+/284218 Trust: Robert Griesemer <gri@golang.org> Reviewed-by: Robert Findley <rfindley@google.com> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
Russ Cox
|
0575e35e50 |
cmd/compile: require 'go 1.16' go.mod line for //go:embed
This will produce better errors when earlier versions of Go compile code using //go:embed. (The import will cause a compilation error but then the go command will add to the output that the Go toolchain in use looks too old and maybe that's the problem.) This CL also adds a test for disallowing embed of a var inside a func. It's a bit too difficult to rebase down into that CL. The build system configuration check is delayed in order to make it possible to use errorcheck for these tests. Change-Id: I12ece4ff2d8d53380b63f54866e8f3497657d54c Reviewed-on: https://go-review.googlesource.com/c/go/+/282718 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Bryan C. Mills <bcmills@google.com> |
|
Cuong Manh Le
|
9423d50d53 |
[dev.regabi] cmd/compile: use '%q' for printing rune values less than 128
Fixes #43762 Change-Id: I51734c9b4ee2366a5dae53b2d27b363f4d5fe6c1 Reviewed-on: https://go-review.googlesource.com/c/go/+/284592 Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
4f5c603c0f |
[dev.regabi] cmd/compile: cleanup callTargetLSym
Now that TailCallStmt carries an *ir.Name instead of a *types.Sym, callTargetLSym can be similarly updated to take the target function as an *ir.Name. This inches us closer towards being able to move Linksym and other properties from *types.Sym to *ir.Name, where they belong. Passes toolstash -cmp w/ -gcflags=all=-abiwrap. Change-Id: I091da290751970eba8ed0438f66d6cca88b665a8 Reviewed-on: https://go-review.googlesource.com/c/go/+/284228 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> |
|
|
Matthew Dempsky
|
6113db0bb4 |
[dev.regabi] cmd/compile: convert OPANIC argument to interface{} during typecheck
Currently, typecheck leaves arguments to OPANIC as their original
type. This CL changes it to insert implicit OCONVIFACE operations to
convert arguments to `interface{}` like how any other function call
would be handled.
No immediate benefits, other than getting to remove a tiny bit of
special-case logic in order.go's handling of OPANICs. Instead, the
generic code path for handling OCONVIFACE is used, if necessary.
Longer term, this should be marginally helpful for #43753, as it
reduces the number of cases where we need values to be addressable for
runtime calls.
However, this does require adding some hacks to appease existing
tests:
1. We need yet another kludge in inline budgeting, to ensure that
reflect.flag.mustBe stays inlinable for cmd/compile/internal/test's
TestIntendedInlining.
2. Since the OCONVIFACE expressions are now being introduced during
typecheck, they're now visible to escape analysis. So expressions like
"panic(1)" are now seen as "panic(interface{}(1))", and escape
analysis warns that the "interface{}(1)" escapes to the heap. These
have always escaped to heap, just now we're accurately reporting about
it.
(Also, unfortunately fmt.go hides implicit conversions by default in
diagnostics messages, so instead of reporting "interface{}(1) escapes
to heap", it actually reports "1 escapes to heap", which is
confusing. However, this confusing messaging also isn't new.)
Change-Id: Icedf60e1d2e464e219441b8d1233a313770272af
Reviewed-on: https://go-review.googlesource.com/c/go/+/284412
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Matthew Dempsky <mdempsky@google.com>
|
|
|
Matthew Dempsky
|
6de9423445 |
[dev.regabi] cmd/compile: cleanup OAS2FUNC ordering
Currently, to ensure OAS2FUNC results are assigned in the correct order, they're always assigned to temporary variables. However, these temporary variables are typed based on the destination type, which may require an interface conversion. This means walk may have to then introduce a second set of temporaries to ensure result parameters are all copied out of the results area, before it emits calls to runtime conversion functions. That's just silly. Instead, this CL changes order to allocate the result temporaries with the same type as the function returns in the first place, and then assign them one at a time to their destinations, with conversions as needed. While here, also fix an order-of-evaluation issue with has-ok assignments that I almost added to multi-value function call assignments, and add tests for each. Change-Id: I9f4e962425fe3c5e3305adbbfeae2c7f253ec365 Reviewed-on: https://go-review.googlesource.com/c/go/+/284220 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
ab523fc510 |
[dev.regabi] cmd/compile: don't promote Byval CaptureVars if Addrtaken
We decide during escape analysis whether to pass closure variables by value or reference. One of the factors that's considered is whether a variable has had its address taken. However, this analysis is based only on the user-written source code, whereas order+walk may introduce rewrites that take the address of a variable (e.g., passing a uint16 key by reference to the size-generic map runtime builtins). Typically this would be harmless, albeit suboptimal. But in #43701 it manifested as needing a stack object for a function where we didn't realize we needed one up front when we generate symbols. Probably we should just generate symbols on demand, now that those routines are all concurrent-safe, but this is a first fix. Thanks to Alberto Donizetti for reporting the issue, and Cuong Manh Le for initial investigation. Fixes #43701. Change-Id: I16d87e9150723dcb16de7b43f2a8f3cd807a9437 Reviewed-on: https://go-review.googlesource.com/c/go/+/284075 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
David Chase
|
b7a698c73f |
[dev.regabi] test: disable test on windows because expected contains path separators.
The feature being tested is insensitive to the OS anyway. Change-Id: Ieac9bfaafc6a54c00017afcc0b87bd8bbe80af7b Reviewed-on: https://go-review.googlesource.com/c/go/+/284032 Trust: David Chase <drchase@google.com> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> |
|
Junchen Li
|
d9b79e53bb |
cmd/compile: fix wrong complement for arm64 floating-point comparisons
Consider the following example,
func test(a, b float64, x uint64) uint64 {
if a < b {
x = 0
}
return x
}
func main() {
fmt.Println(test(1, math.NaN(), 123))
}
The output is 0, but the expectation is 123.
This is because the rewrite rule
(CSEL [cc] (MOVDconst [0]) y flag) => (CSEL0 [arm64Negate(cc)] y flag)
converts
FCMP NaN, 1
CSEL MI, 0, 123, R0 // if 1 < NaN then R0 = 0 else R0 = 123
to
FCMP NaN, 1
CSEL GE, 123, 0, R0 // if 1 >= NaN then R0 = 123 else R0 = 0
But both 1 < NaN and 1 >= NaN are false. So the output is 0, not 123.
The root cause is arm64Negate not handle negation of floating comparison
correctly. According to the ARM manual, the meaning of MI, GE, and PL
are
MI: Less than
GE: Greater than or equal to
PL: Greater than, equal to, or unordered
Because NaN cannot be compared with other numbers, the result of such
comparison is unordered. So when NaN is involved, unlike integer, the
result of !(a < b) is not a >= b, it is a >= b || a is NaN || b is NaN.
This is exactly what PL means. We add NotLessThanF to represent PL. Then
the negation of LessThanF is NotLessThanF rather than GreaterEqualF. The
same reason for the other floating comparison operations.
Fixes #43619
Change-Id: Ia511b0027ad067436bace9fbfd261dbeaae01bcd
Reviewed-on: https://go-review.googlesource.com/c/go/+/283572
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Keith Randall <khr@golang.org>
|
|
|
Matthew Dempsky
|
983ac4b086 |
[dev.regabi] cmd/compile: fix ICE when initializing blank vars
CL 278914 introduced NameOffsetExpr to avoid copying ONAME nodes and hacking up their offsets, but evidently staticinit subtly depended on the prior behavior to allow dynamic initialization of blank variables. This CL refactors the code somewhat to avoid using NameOffsetExpr with blank variables, and to instead create dynamic assignments directly to the global blank node. It also adds a check to NewNameOffsetExpr to guard against misuse like this, since I suspect there could be other cases still lurking within staticinit. (This code is overdue for an makeover anyway.) Thanks to thanm@ for bisect and test case minimization. Fixes #43677. Change-Id: Ic71cb5d6698382feb9548dc3bb9fd606b207a172 Reviewed-on: https://go-review.googlesource.com/c/go/+/283537 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Than McIntosh <thanm@google.com> |
|
|
David Chase
|
861707a8c8 |
[dev.regabi] cmd/compile: added limited //go:registerparams pragma for new ABI dev
This only works for functions; if you try it with a method, it will fail. It does work for both local package and imports. For now, it tells you when it thinks it sees either a declaration or a call of such a function (this will normally be silent since no existing code uses this pragma). Note: it appears to be really darn hard to figure out if this pragma was set for a method, and the method's call site. Better ir.Node wranglers than I might be able to make headway, but it seemed unnecessary for this experiment. Change-Id: I601c2ddd124457bf6d62f714d7ac871705743c0a Reviewed-on: https://go-review.googlesource.com/c/go/+/279521 Trust: David Chase <drchase@google.com> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org> |
|
|
David Chase
|
2abd24f3b7 |
[dev.regabi] test: make run.go error messages slightly more informative
This is intended to make it easier to write/change a test without referring to the source code to figure out what the error messages actually mean, or how to correct them. Change-Id: Ie79ff7cd9f2d1fa605257fe97eace68adc8a6716 Reviewed-on: https://go-review.googlesource.com/c/go/+/281452 Trust: David Chase <drchase@google.com> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Jeremy Faller <jeremy@golang.org> |
|
|
David Chase
|
9a19481acb |
[dev.regabi] cmd/compile: make ordering for InvertFlags more stable
Current many architectures use a rule along the lines of // Canonicalize the order of arguments to comparisons - helps with CSE. ((CMP|CMPW) x y) && x.ID > y.ID => (InvertFlags ((CMP|CMPW) y x)) to normalize comparisons as much as possible for CSE. Replace the ID comparison with something less variable across compiler changes. This helps avoid spurious failures in some of the codegen-comparison tests (though the current choice of comparison is sensitive to Op ordering). Two tests changed to accommodate modified instruction choice. Change-Id: Ib35f450bd2bae9d4f9f7838ceaf7ec682bcf1e1a Reviewed-on: https://go-review.googlesource.com/c/go/+/280155 Trust: David Chase <drchase@google.com> Run-TryBot: David Chase <drchase@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> |
|
|
Matthew Dempsky
|
f065ff221b |
[dev.typeparams] all: merge dev.regabi (d9acf6f) into dev.typeparams
Conflicts: - src/cmd/compile/fmtmap_test.go Merge List: + 2021-01-12 |
|
|
Matthew Dempsky
|
cc90e7a51e |
[dev.regabi] cmd/compile: always use the compile queue
The compiler currently has two modes for compilation: one where it compiles each function as it sees them, and another where it enqueues them all into a work queue. A subsequent CL is going to reorder function compilation to ensure that functions are always compiled before any non-trivial function literals they enclose, and this will be easier if we always use the compile work queue. Also, fewer compilation modes makes things simpler to reason about. Change-Id: Ie090e81f7476c49486296f2b90911fa0a466a5dd Reviewed-on: https://go-review.googlesource.com/c/go/+/283313 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org> |
|
|
Matthew Dempsky
|
12ee55ba7b |
[dev.regabi] cmd/compile: stop using Vargen for import/export
Historically, inline function bodies were exported as plain Go source code, and symbol mangling was a convenient hack because it allowed variables to be re-imported with largely the same names as they were originally exported as. However, nowadays we use a binary format that's more easily extended, so we can simply serialize all of a function's declared objects up front, and then refer to them by index later on. This also allows us to easily report unmangled names all the time (e.g., error message from issue7921.go). Fixes #43633. Change-Id: I46c88f5a47cb921f70ab140976ba9ddce38df216 Reviewed-on: https://go-review.googlesource.com/c/go/+/283193 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Dan Scales <danscales@google.com> Trust: Dan Scales <danscales@google.com> Trust: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
099599662d |
[dev.typeparams] cmd/compile: refactor import logic
This CL refactors noder's package import logic so it's easier to reuse with types2 and gcimports. In particular, this allows the types2 integration to now support vendored packages. Change-Id: I1fd98ad612b4683d2e1ac640839e64de1fa7324b Reviewed-on: https://go-review.googlesource.com/c/go/+/282919 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Trust: Robert Griesemer <gri@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
8b2efa990b |
[dev.regabi] cmd/compile: deref PAUTOHEAPs during SSA construction
Currently, during walk we rewrite PAUTOHEAP uses into derefs of their corresponding Heapaddr, but we can easily do this instead during SSA construction. This does involve updating two test cases: * nilptr3.go This file had a test that we emit a "removed nil check" diagnostic for the implicit dereference from accessing a PAUTOHEAP variable. This CL removes this diagnostic, since it's not really useful to end users: from the user's point of view, there's no pointer anyway, so they needn't care about whether we check for nil or not. That's a purely internal detail. And with the PAUTOHEAP dereference handled during SSA construction, we can more robustly ensure this happens, rather than relying on setting a flag in walk and hoping that SSA sees it. * issue20780.go Previously, when PAUTOHEAPs were dereferenced during walk, it had a consequence that when they're passed as a function call argument, they would first get copied to the stack before being copied to their actual destination. Moving the dereferencing to SSA had a side-effect of eliminating this unnecessary temporary, and copying directly to the destination parameter. The test is updated to instead call "g(h(), h())" where h() returns a large value, as the first result will always need to be spilled somewhere will calling the second function. Maybe eventually we're smart enough to realize it can be spilled to the heap, but we don't do that today. Because I'm concerned that the direct copy-to-parameter optimization could interfere with race-detector instrumentation (e.g., maybe the copies were previously necessary to ensure they're not clobbered by inserted raceread calls?), I've also added issue20780b.go to exercise this in a few different ways. Change-Id: I720598cb32b17518bc10a03e555620c0f25fd28d Reviewed-on: https://go-review.googlesource.com/c/go/+/281293 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> Reviewed-by: Keith Randall <khr@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Russ Cox
|
6598c65646 |
cmd/compile: fix exponential-time init-cycle reporting
I have a real 7,000-line Go program (not so big) that took over two minutes to report a trivial init cycle. I thought the compiler was in an infinite loop but it was actually just very slow. CL 170062 rewrote init cycle reporting but replaced a linear-time algorithm with an exponential one: it explores all paths through the call graph of functions involved in the cycle. The net effect was that Go 1.12 took 0.25 seconds to load, typecheck, and then diagnose the cycle in my program, while Go 1.13 takes 600X longer. This CL makes the new reporting code run in linear time, restoring the speed of Go 1.12 but preserving the semantic fixes from CL 170062. Change-Id: I7d6dc95676d577d9b96f5953b516a64db93249bf Reviewed-on: https://go-review.googlesource.com/c/go/+/282314 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
|
Russ Cox
|
fefad1dc85 |
test: fix timeout code for invoking compiler
When running go tool compile, go tool is running compile as a subprocess. Killing go tool with Process.Kill leaves the subprocess behind. Send an interrupt signal first, which it can forward on to the compile subprocess. Also report the timeout in errorcheck -t. Change-Id: I7ae0029bbe543ed7e60e0fea790dd0739d10bcaa Reviewed-on: https://go-review.googlesource.com/c/go/+/282313 Trust: Russ Cox <rsc@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org> Reviewed-by: Matthew Dempsky <mdempsky@google.com> |
|
Keith Randall
|
304f769ffc |
cmd/compile: don't short-circuit copies whose source is volatile
Current optimization: When we copy a->b and then b->c, we might as well copy a->c instead of b->c (then b might be dead and go away). *Except* if a is a volatile location (might be clobbered by a call). In that case, we really do want to copy a immediately, because there might be a call before we can do the a->c copy. User calls can't happen in between, because the rule matches up the memory states. But calls inserted for memory barriers, particularly runtime.typedmemmove, can. (I guess we could introduce a register-calling-convention version of runtime.typedmemmove, but that seems a bigger change than this one.) Fixes #43570 Change-Id: Ifa518bb1a6f3a8dd46c352d4fd54ea9713b3eb1a Reviewed-on: https://go-review.googlesource.com/c/go/+/282492 Trust: Keith Randall <khr@golang.org> Trust: Josh Bleecher Snyder <josharian@gmail.com> Run-TryBot: Keith Randall <khr@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com> |
|
|
Matthew Dempsky
|
5b9152de57 |
[dev.typeparams] all: merge dev.regabi (cb05a0a) into dev.typeparams
Merge List: + 2021-01-05 |
|
|
Cuong Manh Le
|
fa90aaca7d |
cmd/compile: fix late expand_calls leaf type for OpStructSelect/OpArraySelect
For the example in #43551, before late call expansion, the OpArg type is decomposed to int64. But the late call expansion is currently decompose it to "x.Key" instead. This CL make expand_calls decompose further for struct { 1-field type } and array [1]elem. This matches the previous rules for early decompose args: (StructSelect (StructMake1 x)) => x (ArraySelect (ArrayMake1 x)) => x Fixes #43551 Change-Id: I2f1ebe18cb81cb967f494331c3d237535d2859e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/282332 Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com> Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: David Chase <drchase@google.com> |
|
|
Matthew Dempsky
|
cb05a0aa6a |
[dev.regabi] cmd/compile: remove toolstash scaffolding
Now that CaptureVars is gone, we can remove the extra code in escape analysis that only served to appease toolstash -cmp. Change-Id: I8c811834f3d966e76702e2d362e3de414c94bea6 Reviewed-on: https://go-review.googlesource.com/c/go/+/281544 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> |
|
|
Matthew Dempsky
|
fd43831f44 |
[dev.regabi] cmd/compile: reimplement capture analysis
Currently we rely on the type-checker to do some basic data-flow analysis to help decide whether function literals should capture variables by value or reference. However, this analysis isn't done by go/types, and escape analysis already has a better framework for doing this more precisely. This CL extends escape analysis to recalculate the same "byval" as CaptureVars and check that it matches. A future CL will remove CaptureVars in favor of escape analysis's calculation. Notably, escape analysis happens after deadcode removes obviously unreachable code, so it sees the AST without any unreachable assignments. (Also without unreachable addrtakens, but ComputeAddrtaken already happens after deadcode too.) There are two test cases where a variable is only reassigned on certain CPUs. This CL changes them to reassign the variables unconditionally (as no-op reassignments that avoid triggering cmd/vet's self-assignment check), at least until we remove CaptureVars. Passes toolstash -cmp. Change-Id: I7162619739fedaf861b478fb8d506f96a6ac21f3 Reviewed-on: https://go-review.googlesource.com/c/go/+/281535 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
fb69c67cad |
[dev.regabi] test: enable finalizer tests on !amd64
The gc implementation has had precise GC for a while now, so we can enable these tests more broadly. Confirmed that they still fail with gccgo 10.2.1. Change-Id: Ic1c0394ab832024a99e34163c422941a3706e1a2 Reviewed-on: https://go-review.googlesource.com/c/go/+/281542 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
a8fe098a12 |
[dev.typeparams] all: merge dev.regabi (77365c5) into dev.typeparams
Conflicts: - src/cmd/compile/internal/gc/main.go - test/fixedbugs/issue15055.go Merge List: + 2021-01-05 |
|
|
Matthew Dempsky
|
d89705e087 |
[dev.regabi] cmd/compile: fix re-export of parameters
When exporting signature types, we include the originating package, because it's exposed via go/types's API. And as a consistency check, we ensure that the parameter names came from that same package. However, we were getting this wrong in the case of exported variables that were initialized with a method value using an imported method. In this case, when we created the method value wrapper function's type (which is reused as the variable's type if none is explicitly provided in the variable declaration), we were reusing the original (i.e., imported) parameter names, but the newly created signature type was associated with the current package instead. The correct fix here is really to preserve the original signature type's package (along with position and name for its parameters), but that's awkward to do at the moment because the DeclFunc API requires an ir representation of the function signature, whereas we only provide a way to explicitly set packages via the type constructor APIs. As an interim fix, we associate the parameters with the current package, to be consistent with the signature type's package. Fixes #43479. Change-Id: Id45a10f8cf64165c9bc7d9598f0a0ee199a5e752 Reviewed-on: https://go-review.googlesource.com/c/go/+/281292 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
290b4154b7 |
[dev.regabi] cmd/compile: fix ICE due to large uint64 constants
It's an error to call Int64Val on constants that don't fit into int64. CL 272654 made the compiler stricter about detecting misuse, and revealed that we were using it improperly in detecting consecutive integer-switch cases. That particular usage actually did work in practice, but it's easy and best to just fix it. Fixes #43480. Change-Id: I56f722d75e83091638ac43b80e45df0b0ad7d48d Reviewed-on: https://go-review.googlesource.com/c/go/+/281272 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
907a4bfdc7 |
[dev.regabi] cmd/compile: fix map assignment order
After the previous cleanup/optimization CLs, ascompatee now correctly handles map assignments too. So remove the code from order.mapAssign, which causes us to assign to the map at the wrong point during execution. It's not every day you get to fix an issue by only removing code. Thanks to Cuong Manh Le for test cases and continually following up on this issue. Passes toolstash -cmp. (Apparently the standard library never uses tricky map assignments. Go figure.) Fixes #23017. Change-Id: Ie0728103d59d884d00c1c050251290a2a46150f9 Reviewed-on: https://go-review.googlesource.com/c/go/+/281172 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
bfa97ba48f |
[dev.regabi] test: add another closure test case
When deciding whether a captured variable can be passed by value, the
compiler is sensitive to the order that the OCLOSURE node is
typechecked relative to the order that the variable is passed to
"checkassign". Today, for an assignment like:
q, g = 2, func() int { return q }
we get this right because we always typecheck the full RHS expression
list before calling checkassign on any LHS expression.
But I nearly made a change that would interleave this ordering,
causing us to call checkassign on q before typechecking the function
literal. And alarmingly, there weren't any tests that caught this.
So this commit adds one.
Change-Id: I66cacd61066c7a229070861a7d973bcc434904cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/280998
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
|
Matthew Dempsky
|
68e6fa4f68 |
[dev.regabi] cmd/compile: fix package-initialization order
This CL fixes package initialization order by creating the init task before the general deadcode-removal pass. It also changes noder to emit zero-initialization assignments (i.e., OAS with nil RHS) for package-block variables, so that initOrder can tell the variables still need initialization. To allow this, we need to also extend the static-init code to recognize zero-initialization assignments. This doesn't pass toolstash -cmp, because it reorders some package initialization routines. Fixes #43444. Change-Id: I0da7996a62c85e15e97ce965298127e075390a7e Reviewed-on: https://go-review.googlesource.com/c/go/+/280976 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
477b049060 |
[dev.regabi] cmd/compile: fix printing of method expressions
OTYPE and OMETHEXPR were missing from OpPrec. So add them with the
same precedences as OT{ARRAY,MAP,STRUCT,etc} and
ODOT{,METH,INTER,etc}, respectively. However, ODEREF (which is also
used for pointer types *T) has a lower precedence than other types, so
pointer types need to be specially handled to assign them their
correct, lower precedence.
Incidentally, this also improves the error messages in issue15055.go,
where we were adding unnecessary parentheses around the types in
conversion expressions.
Thanks to Cuong Manh Le for writing the test cases for #43428.
Fixes #43428.
Change-Id: I57e7979babe3ed9ef8a8b5a2a3745e3737dd785f
Reviewed-on: https://go-review.googlesource.com/c/go/+/280873
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
|
Matthew Dempsky
|
0c1a899a6c |
[dev.regabi] cmd/compile: fix defined-pointer method call check
The compiler has logic to check whether we implicitly dereferenced a defined pointer while trying to select a method. However, rather than checking whether there were any implicit dereferences of a defined pointer, it was finding the innermost dereference/selector expression and checking whether that was dereferencing a named pointer. Moreover, it was only checking defined pointer declared in the package block. This CL restructures the code to match go/types and gccgo's behavior. Fixes #43384. Change-Id: I7bddfe2515776d9480eb2c7286023d4c15423888 Reviewed-on: https://go-review.googlesource.com/c/go/+/280392 Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Robert Griesemer <gri@golang.org> Trust: Robert Griesemer <gri@golang.org> Trust: Matthew Dempsky <mdempsky@google.com> |
|
|
Matthew Dempsky
|
a800acaae1 |
[dev.typeparams] all: merge dev.regabi (07569da) into dev.typeparams
Conflicts: - test/fixedbugs/issue27595.go - test/fixedbugs/issue30087.go - test/used.go Merge List: + 2020-12-28 |
|
|
Matthew Dempsky
|
e6c973198d |
[dev.regabi] cmd/compile: stop mangling SelectorExpr.Sel for ODOTMETH
ODOTMETH is unique among SelectorExpr expressions, in that Sel gets mangled so that it no longer has the original identifier that was selected (e.g., just "Foo"), but instead the qualified symbol name for the selected method (e.g., "pkg.Type.Foo"). This is rarely useful, and instead results in a lot of compiler code needing to worry about undoing this change. This CL changes ODOTMETH to leave the original symbol in place. The handful of code locations where the mangled symbol name is actually wanted are updated to use ir.MethodExprName(n).Sym() or (equivalently) ir.MethodExprName(n).Func.Sym() instead. Historically, the compiler backend has mistakenly used types.Syms where it should have used ir.Name/ir.Funcs. And this change in particular may risk breaking something, as the SelectorExpr.Sel will no longer point at a symbol that uniquely identifies the called method. However, I expect CL 280294 (desugar OCALLMETH into OCALLFUNC) to have substantially reduced this risk, as ODOTMETH expressions are now replaced entirely earlier in the compiler. Passes toolstash -cmp. Change-Id: If3c9c3b7df78ea969f135840574cf89e1d263876 Reviewed-on: https://go-review.googlesource.com/c/go/+/280436 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Matthew Dempsky
|
396b6c2e7c |
[dev.regabi] cmd/compile: cleanup assignment typechecking
The assignment type-checking code previously bounced around a lot between the LHS and RHS sides of the assignment. But there's actually a very simple, consistent pattern to how to type check assignments: 1. Check the RHS expression. 2. If the LHS expression is an identifier that was declared in this statement and it doesn't have an explicit type, give it the RHS expression's default type. 3. Check the LHS expression. 4. Try assigning the RHS expression to the LHS expression, adding implicit conversions as needed. This CL implements this algorithm, and refactors tcAssign and tcAssignList to use a common implementation. It also fixes the error messages to consistently say just "1 variable" or "1 value", rather than occasionally "1 variables" or "1 values". Fixes #43348. Passes toolstash -cmp. Change-Id: I749cb8d6ccbc7d22cd7cb0a381f58a39fc2696b5 Reviewed-on: https://go-review.googlesource.com/c/go/+/280112 Trust: Matthew Dempsky <mdempsky@google.com> Run-TryBot: Matthew Dempsky <mdempsky@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com> |
|
|
Russ Cox
|
91cc51e005 |
[dev.typeparams] all: merge dev.regabi (ec741b0) into dev.typeparams
Conflicts: * src/cmd/compile/internal/gc/main.go Merge List: * 2020-12-22 |
|
|
Russ Cox
|
ec741b0447 |
[dev.regabi] all: merge master (c9fb4eb) into dev.regabi
Merge List: * 2020-12-22 |
|
Cherry Zhang
|
c06a354bcc |
test: trigger SIGSEGV instead of SIGTRAP in issue11656.go
In issue11656.go, it tests that if the runtime can get a reasonable traceback when it faults at a non-function PC. It does it by jumping to an address that contains an illegal or trap instruction. When it traps, the SIGTRAP crashes the runtime. This CL changes it to use an instruction that triggers SIGSEGV. This is due to two reasons: - currently, the handling of bad PC is done by preparePanic, which is only used for a panicking signal (SIGSEGV, SIGBUS, SIGFPE), not a fatal signal (e.g. SIGTRAP). - the test uses defer+recover to get a traceback, which only works for panicking signals, not fatal signals. Ideally, we should handle all kinds of faults (SIGSEGV, SIGBUS, SIGILL, SIGTRAP, etc.) with a nice traceback. I'll leave this for the future. This CL also adds RISCV64 support. Fixes #43283. Change-Id: I5e0fbf8530cc89d16e05c3257d282bc1d4d03405 Reviewed-on: https://go-review.googlesource.com/c/go/+/279423 Trust: Cherry Zhang <cherryyz@google.com> Reviewed-by: Ian Lance Taylor <iant@golang.org> |
|
Than McIntosh
|
cb28c96be8 |
[dev.regabi] cmd/compile,cmd/link: initial support for ABI wrappers
Add compiler support for emitting ABI wrappers by creating real IR as opposed to introducing ABI aliases. At the moment these are "no-op" wrappers in the sense that they make a simple call (using the existing ABI) to their target. The assumption here is that once late call expansion can handle both ABI0 and the "new" ABIInternal (register version), it can expand the call to do the right thing. Note that the runtime contains functions that do not strictly follow the rules of the current Go ABI0; this has been handled in most cases by treating these as ABIInternal instead (these changes have been made in previous patches). Generation of ABI wrappers (as opposed to ABI aliases) is currently gated by GOEXPERIMENT=regabi -- wrapper generation is on by default if GOEXPERIMENT=regabi is set and off otherwise (but can be turned on using "-gcflags=all=-abiwrap -ldflags=-abiwrap"). Wrapper generation currently only workd on AMD64; explicitly enabling wrapper for other architectures (via the command line) is not supported. Also in this patch are a few other command line options for debugging (tracing and/or limiting wrapper creation). These will presumably go away at some point. Updates #27539, #40724. Change-Id: I1ee3226fc15a3c32ca2087b8ef8e41dbe6df4a75 Reviewed-on: https://go-review.googlesource.com/c/go/+/270863 Run-TryBot: Than McIntosh <thanm@google.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Zhang <cherryyz@google.com> Trust: Than McIntosh <thanm@google.com> |