| Age | Commit message (Collapse) | Author |
|---|
|
Change-Id: I730da5082ea6de1510482aabaa2915e83d3819a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/461607
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
|
|
This also brings some of the types2 testing code better in sync with
go/types.
Also: fix a minor bug in resolver_test.go (continue traversing
SelectorExpr if the first part is not an identifier).
Change-Id: Ib6c5f6228812b49c185b52a4f02ca5b393418e01
Reviewed-on: https://go-review.googlesource.com/c/go/+/460760
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
[Re-land of CL 424854, which was reverted as CL 425214.]
When handling a type declaration like:
```
type B A
```
unified IR has been writing out that B's underlying type is A, rather
than the underlying type of A.
This is a bit awkward to implement and adds complexity to importers,
who need to handle resolving the underlying type themselves. But it
was necessary to handle when A was declared like:
```
//go:notinheap
type A int
```
Because we expected A's not-in-heap'ness to be conferred to B, which
required knowing that A was on the path from B to its actual
underlying type int.
However, since #46731 was accepted, we no longer need to support this
case. Instead we can write out B's actual underlying type.
One stumbling point though is the existing code for exporting
interfaces doesn't work for the underlying type of `comparable`, which
is now needed to implement `type C comparable`. As a bit of a hack, we
we instead export its underlying type as `interface{ comparable }`.
Fixes #54512.
Change-Id: I9aa087e0a277527003195ebc7f4fbba6922e788c
Reviewed-on: https://go-review.googlesource.com/c/go/+/455279
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
|
|
Ordinary interface types now satisfy comparable constraints. This
is a fully backward-compatible change: it simply permits additional
code to be valid that wasn't valid before.
This change makes the new comparable semantics the default behavior,
depending on the Go -lang version.
It also renames the flag types2.Config.AltComparableSemantics to
types2.Config.OldComparableSemantics and inverts its meaning
(or types.Config.oldComparableSemantics respectively).
Add new predicate Satisfies (matching the predicate Implements but
for constraint satisfaction), per the proposal description.
Adjust some existing tests by setting -oldComparableSemantics
and add some new tests that verify version-dependent behavior.
The compiler flag -oldcomparable may be used to temporarily
switch back to the Go 1.18/1.19 behavior should this change
cause problems, or to identify that a problem is unrelated
to this change. The flag will be removed for Go 1.21.
For #52509.
For #56548.
For #57011.
Change-Id: I8b3b3d9d492fc24b0693567055f0053ccb5aeb42
Reviewed-on: https://go-review.googlesource.com/c/go/+/454575
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
|
|
The CL below was accidentally submitted, while waiting for the freeze
exception. Reverting.
This reverts commit 15e705ea963b5008112793507365e24b743606bc.
Change-Id: I4dbf92dcb01fa9245a6e6a2d1514d8aa898d0048
Reviewed-on: https://go-review.googlesource.com/c/go/+/454476
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Robert Griesemer <gri@google.com>
|
|
Ordinary interface types now satisfy comparable constraints.
This change makes the new comparable semantics the default behavior,
depending on the Go -lang version.
It also renames the flag types2.Config.AltComparableSemantics to
types2.Config.OldComparableSemantics and inverts its meaning
(or types.Config.oldComparableSemantics respectively).
Adjust some existing tests by setting -oldComparableSemantics
and add some new tests that verify version-dependent behavior.
The compiler flag -oldcomparable may be used to temporarily
switch back to the Go 1.18/1.19 behavior should this change
cause problems, or to identify that a problem is unrelated
to this change. The flag will be removed for Go 1.21.
For #52509.
For #56548.
Change-Id: Ic2b22db9433a8dd81dc1ed9d30835f0395fb7205
Reviewed-on: https://go-review.googlesource.com/c/go/+/453978
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
|
|
This CL changes cmd/compile to reject anonymous interface cycles like:
type I interface { m() interface { I } }
We don't anticipate any users to be affected by this change in
practice. Nonetheless, this CL also adds a `-d=interfacecycles`
compiler flag to suppress the error. And assuming no issue reports
from users, we'll move the check into go/types and types2 instead.
Updates #56103.
Change-Id: I1f1dce2d7aa19fb388312cc020e99cc354afddcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/445598
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
|
|
On advice of the department of garbage collection, forcing a garbage
collection generally does not improve performance. However,
this-data-is-now-unreachable is a good property to be able to test,
and that requires finalizers and a forced GC. So, to save build time,
this test was removed from the compiler itself, but to verify the
property, it was added to the fma_test (and the end-to-end dependence
on the flag was tested with an inserted failure in testing the
test).
TODO: also turn on the new -d=gccheck=1 debug flag on the ssacheck
builder.
Benchmarking reveals that it is profitable to avoid this GC,
with about 1.5% reduction in both user and wall time.
(48 p) https://perf.golang.org/search?q=upload:20221103.3
(12 p) https://perf.golang.org/search?q=upload:20221103.5
Change-Id: I4c4816d619735838a32388acf0cc5eb1cd5f0db5
Reviewed-on: https://go-review.googlesource.com/c/go/+/447359
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@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>
|
|
Prior to Go 1.18, ineffectual //go:linkname directives (i.e.,
directives referring to an undeclared name, or to a declared type or
constant) were treated as noops. In Go 1.18, we changed this into a
compiler error to mitigate accidental misuse.
However, the x/sys repo contained ineffectual //go:linkname directives
up until go.dev/cl/274573, which has caused a lot of user confusion.
It seems a bit late to worry about now, but to at least prevent
further user pain, this CL changes the error message to only apply to
modules using "go 1.18" or newer. (The x/sys repo declared "go 1.12"
at the time go.dev/cl/274573 was submitted.)
Fixes #55889.
Change-Id: Id762fff96fd13ba0f1e696929a9e276dfcba2620
Reviewed-on: https://go-review.googlesource.com/c/go/+/447755
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Bryan Mills <bcmills@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>
|
|
Add the experimental compiler flag -altcomparable. If set, the
compiler uses alternative comparable semantics: any ordinary
(non-type parameter) interface implements the comparable
constraint.
This permits experimenting with this alternative semantics
akin to what is proposed in #52509.
For #52509.
Change-Id: I64192eee6f2a550eeb50de011079f2f0b994cf94
Reviewed-on: https://go-review.googlesource.com/c/go/+/444636
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
|
|
Change-Id: If78c6d3c6183494f71f2857e496e172a789da39f
GitHub-Last-Rev: 58e0b75052a92cb720371d2b3c75e1de79d79bdc
GitHub-Pull-Request: golang/go#55992
Reviewed-on: https://go-review.googlesource.com/c/go/+/437517
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
|
|
identifier
Change-Id: I51dd087e630bf433c30d0aaaf3715b62524eb432
Reviewed-on: https://go-review.googlesource.com/c/go/+/436647
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: xie cui <523516579@qq.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
|
|
Saves on both space and cost of map operations. Saves about 3% in compile time.
name old time/op new time/op delta
Template 251ms ± 2% 244ms ± 1% -2.78% (p=0.000 n=8+8)
Unicode 149ms ± 5% 135ms ± 2% -9.03% (p=0.000 n=10+10)
GoTypes 1.38s ± 1% 1.35s ± 1% -2.29% (p=0.000 n=10+10)
Compiler 115ms ± 2% 112ms ± 2% -2.50% (p=0.001 n=10+9)
SSA 11.9s ± 0% 11.4s ± 0% -4.04% (p=0.000 n=9+10)
Flate 153ms ± 1% 148ms ± 1% -3.32% (p=0.000 n=10+9)
GoParser 284ms ± 2% 280ms ± 1% -1.70% (p=0.002 n=10+10)
Tar 209ms ± 2% 205ms ± 2% -1.98% (p=0.004 n=9+10)
XML 287ms ± 2% 281ms ± 1% -2.06% (p=0.000 n=10+10)
LinkCompiler 508ms ± 2% 501ms ± 2% -1.31% (p=0.024 n=9+9)
ExternalLinkCompiler 2.66s ± 3% 2.63s ± 4% ~ (p=0.280 n=10+10)
LinkWithoutDebugCompiler 338ms ± 3% 330ms ± 3% -2.21% (p=0.009 n=10+10)
StdCmd 21.5s ± 1% 20.8s ± 1% -3.27% (p=0.000 n=9+9)
[Geo mean] 615ms 597ms -2.91%
name old user-time/op new user-time/op delta
Template 344ms ± 2% 324ms ± 3% -6.01% (p=0.000 n=9+9)
Unicode 215ms ±11% 192ms ± 2% -10.84% (p=0.000 n=10+9)
GoTypes 1.99s ± 2% 1.93s ± 2% -2.73% (p=0.000 n=10+10)
Compiler 142ms ± 4% 140ms ± 3% -1.89% (p=0.031 n=9+9)
SSA 17.4s ± 1% 17.0s ± 5% ~ (p=0.113 n=9+10)
Flate 200ms ± 4% 196ms ± 6% ~ (p=0.190 n=10+10)
GoParser 388ms ± 3% 378ms ± 4% -2.59% (p=0.004 n=9+10)
Tar 278ms ± 8% 277ms ± 2% ~ (p=0.315 n=10+10)
XML 387ms ± 2% 381ms ± 2% -1.63% (p=0.005 n=8+8)
LinkCompiler 784ms ± 4% 778ms ± 2% ~ (p=0.436 n=10+10)
ExternalLinkCompiler 2.45s ± 1% 2.42s ± 1% -1.11% (p=0.001 n=10+9)
LinkWithoutDebugCompiler 374ms ± 3% 366ms ± 2% -2.15% (p=0.010 n=10+9)
[Geo mean] 600ms 583ms -2.91%
Change-Id: I9552a70d6a2ad500e9acd8815762b761be3c2ff9
Reviewed-on: https://go-review.googlesource.com/c/go/+/432897
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
|
|
Fixes #55326.
Change-Id: Ic300976ddf504224faadc5bae21736fe94dac6bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/435416
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
|
|
In preparation for encoding it in a more efficient way.
Change-Id: I299dd2befc3d07107a1b7b49225bbb9f2e48a343
Reviewed-on: https://go-review.googlesource.com/c/go/+/432896
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
Currently, for version errors, types2 adds the helpful hint
(-lang was set to go1.xx; check go.mod)
where 1.xx is the respective language version, to the error message.
This requires that the type checker knows that it was invoked by the
compiler, which is done through the Config.CompilerErrorMessages flag.
This change looks for version errors being returned by the type checker
and then adds the hint at that point, external to the type checker.
This removes a dependency on the Config.CompilerErrorMessages. Once
we have removed all dependencies on Config.CompilerErrorMessages we
can remove it.
For #55326.
Change-Id: I1f9b2e472c49fe785a2075e26c4b3d9b8fcdbf4d
Reviewed-on: https://go-review.googlesource.com/c/go/+/432559
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
|
|
The nounified frontend currently tries to construct dictionaries that
correspond to invalid instantiations (i.e., instantiations T[X] where
X does not satisfy the constraints specified on T's type parameter).
As a consequence, we may fail to find method expressions needed by the
dictionary.
The real fix for this is to avoid creating those dictionaries in the
first place, because they should never actually be needed at runtime.
But that seems scary for a backport: we've repeatedly attempted to
backport generics fixes, which have fixed one issue but introduced
another.
This CL is a minimally invasive solution to #54225, which avoids the
ICE by instead skipping emitting the invalid dictionary. If the
dictionary ends up not being needed (which I believe will always be
the case), then the linker's reachability analysis will simply ignore
its absence.
Or worst case, if the dictionary *is* reachable somehow, we've simply
turned an ICE into a link-time missing symbol failure. That's not
great for user experience, but it seems like a small trade off to
avoid risking breaking any other currently working code.
Updates #54225.
Change-Id: Ic379696079f4729b1dd6a66994a58cca50281a84
Reviewed-on: https://go-review.googlesource.com/c/go/+/429655
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
When naming case variables, the unified frontend was using
typecheck.Lookup, which uses the current package, rather than
localIdent, which uses the package the variable was originally
declared in. When inlining across package boundaries, this could cause
the case variables to be associated with the wrong package.
In practice, I don't believe this has any negative consequences, but
it's inconsistent and triggered an ICE in typecheck.ClosureType, which
expected all captured variables to be declared in the same package.
Easy fix is to ensure case variables are declared in the correct
package by using localIdent.
Fixes #54912.
Change-Id: I7a429c708ad95723f46a67872cb0cf0c53a6a0d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/428918
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: Benny Siegert <bsiegert@gmail.com>
|
|
Currently, the itabs section for runtime dictionaries includes its own
redundant *runtime._type pointers for typ and iface, which were
sometimes necessary. This simplified the initial implementation, but
is a little wasteful of space when the same type or interface appeared
across multiple (typ, iface) pairs.
This CL instead reuses the pointers from the rtypes section.
Change-Id: I48448515c319c0403c1a8e7706794d443176f0a4
Reviewed-on: https://go-review.googlesource.com/c/go/+/427754
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
In go.dev/cl/421821, I included a hack to force OCONVNOP back to
OCONVIFACE for conversions involving shape types and non-empty
interfaces. The comment correctly noted that this was only needed for
conversions between non-identical types, but the code was conservative
and applied to even conversions between identical types.
This CL adds an extra bool to record whether the conversion is between
identical types, so we can keep OCONVNOP instead of forcing back to
OCONVIFACE. This has a small improvement to generated code, because we
no longer need a convI2I call (as demonstrated by codegen/ifaces.go).
But more usefully, this is relevant to pruning unnecessary itab slots
in runtime dictionaries (next CL).
Change-Id: I94f89e961cd26629b925037fea58d283140766ff
Reviewed-on: https://go-review.googlesource.com/c/go/+/427678
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This CL deduplicates the explicit and implicit exprConvert code paths
to have a single common function, so they're easier to keep in sync.
Change-Id: I2b145d2ce6de6018ffc2db5cdb9d891f4e223381
Reviewed-on: https://go-review.googlesource.com/c/go/+/427677
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Updates #46731
Change-Id: I247fa9c7ca97feb9053665da7ff56e7f5b571f74
Reviewed-on: https://go-review.googlesource.com/c/go/+/422815
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Joedian Reid <joedian@golang.org>
|
|
So it won't be visible outside of runtime package. There are changes to
make tests happy:
- For test/directive*.go files, using "go:noinline" for testing misplaced
directives instead.
- Restrict test/fixedbugs/bug515.go for gccgo only.
- For test/notinheap{2,3}.go, using runtime/cgo.Incomplete for marking
the type as not-in-heap. Though it's somewhat clumsy, it's the easiest
way to keep the test errors for not-in-heap types until we can cleanup
further.
- test/typeparam/mdempsky/11.go is about defined type in user code marked
as go:notinheap, which can't happen after this CL, though.
Fixes #46731
Change-Id: I869f5b2230c8a2a363feeec042e7723bbc416e8e
Reviewed-on: https://go-review.googlesource.com/c/go/+/421882
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Joedian Reid <joedian@golang.org>
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
After running the types2 type checker, walk info.Instances to reject
any not-in-heap type arguments. This is feasible to check using the
types2 API now, thanks to #46731.
Fixes #54765.
Change-Id: Idd2acc124d102d5a76f128f13c21a6e593b6790b
Reviewed-on: https://go-review.googlesource.com/c/go/+/427235
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
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>
|
|
For #53003
Change-Id: I13a761daca8b433b271a1feb711c103d9820772d
Reviewed-on: https://go-review.googlesource.com/c/go/+/423774
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: hopehook <hopehook@golangcn.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Change-Id: I2cc62efb7bb3b47f1ee3ed0bb77e35c47e2df9a1
GitHub-Last-Rev: 106cb494de6fb08302018bff55944ee120ef4545
GitHub-Pull-Request: golang/go#54718
Reviewed-on: https://go-review.googlesource.com/c/go/+/426297
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
Shape-based stenciling in unified IR is done by converting type argument
to its underlying type. So it agressively check that type argument is
not a TFORW. However, for recursive instantiated type argument, it may
still be a TFORW when shapifying happens. Thus the assertion failed,
causing the compiler crashing.
To fix it, just allow fully instantiated type when shapifying.
Fixes #54512
Fixes #54722
Change-Id: I527e3fd696388c8a37454e738f0324f0c2ec16cb
Reviewed-on: https://go-review.googlesource.com/c/go/+/426335
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
When inlining function calls, we rewrite the position information on
all of the nodes to keep track of the inlining context. This is
necessary so that at runtime, we can synthesize additional stack
frames so that the inlining is transparent to the user.
However, for function literals, we *don't* want to apply this
rewriting to the underlying function. Because within the function
literal (when it's not itself inlined), the inlining context (if any)
will have already be available at the caller PC instead.
Unified IR was already getting this right in the case of user-written
statements within the function literal, which is what the unit test
for #46234 tested. However, it was still using inline-adjusted
positions for the function declaration and its parameters, which
occasionally end up getting used for generated code (e.g., loading
captured values from the closure record).
I've manually verified that this fixes the hang in
https://go.dev/play/p/avQ0qgRzOgt, and spot-checked the
-d=pctab=pctoinline output for kube-apiserver and kubelet and they
seem better.
However, I'm still working on a more robust test for this (hence
"Updates" not "Fixes") and internal assertions to verify that we're
emitting correct inline trees. In particular, there are still other
cases (even in the non-unified frontend) where we're producing
corrupt (but at least acyclic) inline trees.
Updates #54625.
Change-Id: Iacfd2e1eb06ae8dc299c0679f377461d3d46c15a
Reviewed-on: https://go-review.googlesource.com/c/go/+/425395
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
|
|
This reverts CL 424854.
Reason for revert: broke misc/cgo/stdio.TestTestRun on several builders.
Will re-land after CL 421879 is submitted.
Change-Id: I2548c70d33d7c178cc71c1d491cd81c22660348f
Reviewed-on: https://go-review.googlesource.com/c/go/+/425214
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
|
|
In CL 424734, I implemented pointer shaping for unified IR. Evidently
though, we didn't have any test cases that check that uses of
pointer-shaped expressions were handled correctly.
In the reported test case, the struct field "children items[*node[T]]"
gets shaped to "children items[go.shape.*uint8]" (underlying type
"[]go.shape.*uint8"); and so the expression "n.children[i]" has type
"go.shape.*uint8" and the ".items" field selection expression fails.
The fix implemented in this CL is that any expression of derived type
now gets an explicit "reshape" operation applied to it, to ensure it
has the appropriate type for its context. E.g., the "n.children[i]"
OINDEX expression above gets "reshaped" from "go.shape.*uint8" to
"*node[go.shape.int]", allowing the field selection to succeed.
This CL also adds a "-d=reshape" compiler debugging flag, because I
anticipate debugging reshaping operations will be something to come up
again in the future.
Fixes #54535.
Change-Id: Id847bd8f51300d2491d679505ee4d2e974ca972a
Reviewed-on: https://go-review.googlesource.com/c/go/+/424936
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: hopehook <hopehook@qq.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
To disambiguate local types, we append a "·N" suffix to their name and
then trim it off again when producing their runtime type descriptors.
However, if a local type is generic, then we were further appending
the type arguments after this suffix, and the code in types/fmt.go
responsible for trimming didn't know to handle this.
We could extend the types/fmt.go code to look for the "·N" suffix
elsewhere in the type name, but this is risky because it could
legitimately (albeit unlikely) appear in struct field tags.
Instead, the most robust solution is to just change the mangling logic
to keep the "·N" suffix at the end, where types/fmt.go can easily and
reliably trim it.
Note: the "·N" suffix is still visible within the type arguments
list (e.g., the "·3" suffixes in nested.out), because we currently use
the link strings in the type arguments list.
Fixes #54456.
Change-Id: Ie9beaf7e5330982f539bff57b8d48868a3674a37
Reviewed-on: https://go-review.googlesource.com/c/go/+/424901
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
|
|
When handling a type declaration like:
```
type B A
```
unified IR has been writing out that B's underlying type is A, rather
than the underlying type of A.
This is a bit awkward to implement and adds complexity to importers,
who need to handle resolving the underlying type themselves. But it
was necessary to handle when A was declared like:
```
//go:notinheap
type A int
```
Because we expected A's not-in-heap'ness to be conferred to B, which
required knowing that A was on the path from B to its actual
underlying type int.
However, since #46731 was accepted, we no longer need to support this
case. Instead we can write out B's actual underlying type.
One stumbling point though is the existing code for exporting
interfaces doesn't work for the underlying type of `comparable`, which
is now needed to implement `type C comparable`. As a bit of a hack, we
we instead export its underlying type as `interface{ comparable }`.
Fixes #54512.
Change-Id: I0fb892068d656f1e87bb8ef97da27756051126d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/424854
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
|
|
For local variables of derived type, Delve relies on ir.Name.DictIndex
being set to the type's rtype index within the function's dictionary.
This CL implements that functionality within unified IR.
Manually double checked that Delve behaves correctly, at least as far
as I can tell from casual use. Specifically, I confirmed that running
the test program from TestDictIndex, stepping into testfn, and then
running `print mapvar` prints `map[int]main.CustomInt []`, which
matches the behavior under GOEXPERIMENT=nounified. (Also compare that
when ir.Name.DictIndex is *not* set by unified IR, `print mapvar`
instead prints `map[int]go.shape.int []`.)
Fixes #54514.
Change-Id: I90d443945895abfba04dc018f15e00217930091c
Reviewed-on: https://go-review.googlesource.com/c/go/+/424735
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This CL implements pointer shaping in unified IR, corresponding to the
existing pointer shaping implemented in the non-unified frontend.
For example, if `func F[T any]` is instantiated as both `F[*int]` and
`F[*string]`, we'll now generate a single `F[go.shape.*uint8]` shaped
function that can be used by both.
Fixes #54513.
Change-Id: I2cef5ae411919e6dc5bcb3cac912abecb4cd5218
Reviewed-on: https://go-review.googlesource.com/c/go/+/424734
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
This CL switches unified IR to use shape-based stenciling with runtime
dictionaries, like the existing non-unified frontend. Specifically,
when instantiating generic functions and types `X[T]`, we now also
instantiated shaped variants `X[shapify(T)]` that can be shared by
`T`'s with common underlying types.
For example, for generic function `F`, `F[int](args...)` will be
rewritten to `F[go.shape.int](&.dict.F[int], args...)`.
For generic type `T` with method `M` and value `t` of type `T[int]`,
`t.M(args...)` will be rewritten to `T[go.shape.int].M(t,
&.dict.T[int], args...)`.
Two notable distinctions from the non-unified frontend:
1. For simplicity, currently shaping is limited to simply converting
type arguments to their underlying type. Subsequent CLs will implement
more aggressive shaping.
2. For generic types, a single dictionary is generated to be shared by
all methods, rather than separate dictionaries for each method. I
originally went with this design because I have an idea of changing
interface calls to pass the itab pointer via the closure
register (which should have zero overhead), and then the interface
wrappers for generic methods could use the *runtime.itab to find the
runtime dictionary that corresponds to the dynamic type. This would
allow emitting fewer method wrappers.
However, this choice does have the consequence that currently even if
a method is unused and its code is pruned by the linker, it may have
produced runtime dictionary entries that need to be kept alive anyway.
I'm open to changing this to generate per-method dictionaries, though
this would require changing the unified IR export data format; so it
would be best to make this decision before Go 1.20.
The other option is making the linker smarter about pruning unneeded
dictionary entries, like how it already prunes itab entries. For
example, the runtime dictionary for `T[int]` could have a `R_DICTTYPE`
meta-relocation against symbol `.dicttype.T[go.shape.int]` that
declares it's a dictionary associated with that type; and then each
method on `T[go.shape.T]` could have `R_DICTUSE` meta-relocations
against `.dicttype.T[go.shape.T]+offset` indicating which fields
within dictionaries of that type need to be preserved.
Change-Id: I369580b1d93d19640a4b5ecada4f6231adcce3fd
Reviewed-on: https://go-review.googlesource.com/c/go/+/421821
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Change-Id: I147622770587cff1551a0be189003666463f34e3
GitHub-Last-Rev: 71b5375983820445a331c46481a2a3a44fafb760
GitHub-Pull-Request: golang/go#54472
Reviewed-on: https://go-review.googlesource.com/c/go/+/424154
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
|
CL 414836 limited the check for implicit dot for method call enabled by
a type bound. However, the checking condition for ODOTMETH only is not
right. For example, for promoted method, we have a OXDOT node instead,
and we still have to check for implicit dot in this case.
However, if the base type and embedded types have the same method name,
e.g in issue #53419, typecheck.AddImplicitDots will be confused and
result in an ambigus selector.
To fix this, we ensure methods for the base type are computed, then only
do the implicit dot check if we can find a matched method.
Fixes #54348
Change-Id: Iefe84ff330830afe35c5daffd499824db108da23
Reviewed-on: https://go-review.googlesource.com/c/go/+/422274
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
|
|
In go.dev/cl/413396, I implemented implicit conversions by setting the
conversion's position to the enclosing statement that necessitated the
conversion. However, users actually want the position information to
be at the expression itself, and this seems sensible anyway.
This was noticed because x/tools had a test for:
fmt.Println(42)
and it was checking where the escape analysis diagnostic for
`42` (really `any(42)`) was reported.
Historically, we reported the column of the `4`; but CL 413396 caused
unified IR to instead report the column of the `(` instead (the
position associated with the call expression, which forced `42` to be
implicitly converted from `int` to `any`).
I chalk this mistake up to being accustomed to working with ir, where
we can't reliably use n.Pos() because of how ONAME positions work, so
I was trying to avoid relying on the implicitly converted expression's
own position.
Change-Id: I762076af6f65ebe6d444d64630722a5016dc2698
Reviewed-on: https://go-review.googlesource.com/c/go/+/422976
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
For selector expression "x.M" where "M" is a promoted method, irgen is using
the type of receiver "x" for determining the typeparams for instantiation.
However, because M is a promoted method, so its associated receiver is
not "x", but "x.T" where "T" is the embedded field of "x". That casues a
mismatch when converting non-shape types arguments.
Fixing it by using the actual receiver which has the method, instead of
using the base receiver.
Fixes #53982
Change-Id: I1836fc422d734df14e9e6664d4bd014503960bfc
Reviewed-on: https://go-review.googlesource.com/c/go/+/419294
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
|
|
This CL adds a helper expression code for receiver addressing; i.e.,
the implicit addressing, dereferencing, and field selections involved
in changing the `x` in `x.M()` into an appropriate expression to pass
as an argument to the method.
Change-Id: I9be933e2a38c8f94f6a85d95b54f34164e5efb0a
Reviewed-on: https://go-review.googlesource.com/c/go/+/421820
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
|
|
Unified IR uses static dictionaries for some itabs and function/method
expressions, and they're roughly the right idea. But at the same time,
they're actually somewhat brittle and I need to reorganize some ideas
anyway to get shaped-based stenciling working. So this CL just rips
them out entirely.
Note: the code for emitting runtime dictionaries with *runtime._type
symbols is still present, and continues to demonstrate that basic
runtime dictionary handling is working.
Change-Id: I44eb1c7974fb397909ad5db12987659e7505c2ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/421819
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
This CL separates out the handling of selector expressions for field
values, method values, and method expressions. Again part of
refactoring to make it possible to access runtime dictionaries where
needed.
No behavioral change; just duplicating and then streamlining the
existing code paths.
Change-Id: I53b2a344f4bdba2c9f37ef370dc9a091a3941021
Reviewed-on: https://go-review.googlesource.com/c/go/+/421818
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
|
|
This CL changes unified IR to explicitly handle function
instantiations within expression handling, rather than leaving it to
the underlying object reading logic.
Change-Id: I009a56013fbe9fbc4dabf80eea98993d34af4272
Reviewed-on: https://go-review.googlesource.com/c/go/+/421817
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
|
|
Per TODO, this CL just moves code around without behavioral
change. Part of a cleanup process so that it's easier to access object
dictionaries where needed.
Change-Id: I95bb3cdd3cdb46cae47d76d5c1d5d8256661f222
Reviewed-on: https://go-review.googlesource.com/c/go/+/421816
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
|
|
Same as CL 418475, but for Unified IR.
Updates #53888
Fixes #54337
Change-Id: I31d5a7af04d8e3902ed25db85009d46ea4c38dbe
Reviewed-on: https://go-review.googlesource.com/c/go/+/422040
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
|
|
As it can't appear in user package paths.
There is a hack for handling "go:buildid" and "type:*" on windows/386.
Previously, windows/386 requires underscore prefix on external symbols,
but that's only applied for SHOSTOBJ/SUNDEFEXT or cgo export symbols.
"go.buildid" is STEXT, "type.*" is STYPE, thus they are not prefixed
with underscore.
In external linking mode, the external linker can't resolve them as
external symbols. But we are lucky that they have "." in their name,
so the external linker see them as Forwarder RVA exports. See:
- https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#export-address-table
- https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=ld/pe-dll.c;h=e7b82ba6ffadf74dc1b9ee71dc13d48336941e51;hb=HEAD#l972)
This CL changes "." to ":" in symbols name, so theses symbols can not be
found by external linker anymore. So a hacky way is adding the
underscore prefix for these 2 symbols. I don't have enough knowledge to
verify whether adding the underscore for all STEXT/STYPE symbols are
fine, even if it could be, that would be done in future CL.
Fixes #37762
Change-Id: I92eaaf24c0820926a36e0530fdb07b07af1fcc35
Reviewed-on: https://go-review.googlesource.com/c/go/+/317917
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
|
|
Base on gri@'s suggestion in CL 308971. "Rnd" is a bit random.
Change-Id: I4aad8b7992b31dfd26d20b3c332bc6e1e90f67db
Reviewed-on: https://go-review.googlesource.com/c/go/+/422036
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
|
