For example:
movb a0, a0
srai $1, a0, a0
the assembler will expand to:
slli $56, a0, a0
srai $56, a0, a0
srai $1, a0, a0
this CL optimize to:
slli $56, a0, a0
srai $57, a0, a0
Remove 270+ instructions from Go binary on linux/riscv64.
Change-Id: I375e19f9d3bd54f2781791d8cbe5970191297dc8
Reviewed-on: https://go-review.googlesource.com/c/go/+/428496
Reviewed-by: Keith Randall <khr@google.com>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This is the last failed test in Unified IR, since it can inline f5 and
f6 but the old frontend can not. So marking them as //go:noinline, with
a TODO for re-enable once GOEXPERIMENT=nounified is gone.
Fixes#53058
Change-Id: Ifbbc49c87997a53e1b323048f0067f0257655fad
Reviewed-on: https://go-review.googlesource.com/c/go/+/437217
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The mismatch between Unified IR and the old frontend is not about how
they number the closures, but how they name them. For nested closure,
the old frontend use the immediate function which contains the closure
as the outer function, while Unified IR uses the outer most function as
the outer for all closures.
That said, what important is matching the number of closures, not their
name prefix. So this CL relax the test to match both "main.func1.func2"
and "main.func1.2" to satisfy both Unified IR and the old frontend.
Updates #53058
Change-Id: I66ed816d1968aa68dd3089a4ea5850ba30afd75b
Reviewed-on: https://go-review.googlesource.com/c/go/+/437216
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The linker needs FuncInfo metadata for all inlined functions. This is
typically handled by gc.enqueueFunc calling ir.InitLSym for all function
declarations in typecheck.Target.Decls (ir.UseClosure adds all closures
to Decls).
However, non-trivial closures in Decls are ignored, and are insteaded
enqueued when walk of the calling function discovers them.
This presents a problem for direct calls to closures. Inlining will
replace the entire closure definition with its body, which hides the
closure from walk and thus suppresses symbol creation.
Explicitly create a symbol early in this edge case to ensure we keep
this metadata.
InitLSym needs to move out of ssagen to avoid a circular dependency (it
doesn't have anything to do with ssa anyway). There isn't a great place
for it, so I placed it in ir, which seemed least objectionable.
The added test triggers one of these inlined direct non-trivial closure
calls, though the test needs CL 429637 to fail, which adds a FuncInfo
assertion to the linker. Note that the test must use "run" instead of
"compile" since the assertion is in the linker, and "compiler" doesn't
run the linker.
Fixes#54959.
Change-Id: I0bd1db4f3539a78da260934cd968372b7aa92546
Reviewed-on: https://go-review.googlesource.com/c/go/+/436240
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
If a cycle has length 1, don't enumerate the single cycle entry;
instead just mention "refers to itself". For instance, for an
invalid recursive type T we now report:
invalid recursive type: T refers to itself
instead of:
invalid recursive type T
T refers to
T
Adjust tests to check for the different error messages.
Change-Id: I5bd46f62fac0cf167f0d0c9a55f952981d294ff4
Reviewed-on: https://go-review.googlesource.com/c/go/+/436295
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
This matches what go/types and types2 report and it also matches
the compiler errors reported for some related shift problems.
For #55326.
Change-Id: Iee40e8d988d5a7f9ff2c49f019884d02485c9fdf
Reviewed-on: https://go-review.googlesource.com/c/go/+/436177
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
This is close to what the compiler used to say, except now we say
"as T value" rather than "as type T" which is closer to the truth
(we cannot use a value as a type, after all). Also, place the primary
error and the explanation (cause) on a single line.
Make respective (single line) adjustment to the matching "cannot
convert" error.
Adjust various tests.
For #55326.
Change-Id: Ib646cf906b11f4129b7ed0c38cf16471f9266b88
Reviewed-on: https://go-review.googlesource.com/c/go/+/436176
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: 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>
Compromise between old compiler error "T.m redeclared in this block"
(where the "in this block" is not particularly helpful) and the old
type-checker error "method m already declared for type T ...".
In the case where we have position information for the original
declaration, the error message is "method T.m already declared at
<position>". The new message is both shorter and more precise.
For #55326.
Change-Id: Id4a7f326fe631b11db9e8030eccb417c72d6c7db
Reviewed-on: https://go-review.googlesource.com/c/go/+/435016
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
This is a compromise of the error reported by the compiler (quotes
around field name removed) and the error reported by the type checkers
(added mention of struct type).
For #55326.
Change-Id: Iac4fb5c717f17c6713e90d327d39e68d3be40074
Reviewed-on: https://go-review.googlesource.com/c/go/+/434815
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This matches longstanding compiler behavior.
Also, for unused packages, report:
`"pkg" imported and not used`
`"pkg" imported as X and not used`
This matches the other `X declared and not used` errors.
For #55326.
Change-Id: Ie71cf662fb5f4648449c64fc51bede298a1bdcbf
Reviewed-on: https://go-review.googlesource.com/c/go/+/432557
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Partial overlaps can only happen for strict sub-pieces of larger arrays.
That's a much stronger condition than the current optimization rules.
Update #54467
Change-Id: I11e539b71099e50175f37ee78fddf69283f83ee5
Reviewed-on: https://go-review.googlesource.com/c/go/+/433056
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
1. replace [0-9] with \d in regexps
2. replace [a-zA-Z0-9_] with \w in regexps
Change-Id: I9e260538252a0c1071e76aeb1c5f885c6843a431
GitHub-Last-Rev: 286e1a4619
GitHub-Pull-Request: golang/go#54874
Reviewed-on: https://go-review.googlesource.com/c/go/+/428435
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
For #55326
Change-Id: I3d0ff7f820f7b2009d1b226abf701b2337fe8cbc
Reviewed-on: https://go-review.googlesource.com/c/go/+/432635
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: xie cui <523516579@qq.com>
Reviewed-by: Robert Griesemer <gri@google.com>
This CL adds some optimizaion rules:
1, Converts CMP to CMN, or vice versa, when comparing with a negative
number.
2, For equal and not equal comparisons, CMP can be converted to CMN in
some cases. In theory we could do the same optimization for LT, LE, GT
and GE, but need to account for overflow, this CL doesn't handle them.
There are no noticeable performance changes.
Change-Id: Ia49266c019ab7908ebc9510c2f02e121b1607869
Reviewed-on: https://go-review.googlesource.com/c/go/+/429795
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Eric Fang <eric.fang@arm.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>
The conversion T(x) is implemented as *(*T)(x). Accordingly, runtime
panic messages for (*T)(x) are made more general.
Fixes#46505.
Change-Id: I76317c0878b6a5908299506d392eed50d7ef6523
Reviewed-on: https://go-review.googlesource.com/c/go/+/430415
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
This test case already works with GOEXPERIMENT=unified, and it never
worked with Go 1.18 or Go 1.19. So this CL simply adds a regress test
to make sure it continues working.
Fixes#55101.
Change-Id: I7e06bfdc136ce124f65cdcf02d20a1050b841d42
Reviewed-on: https://go-review.googlesource.com/c/go/+/431455
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The type of the source and destination of a memmove call isn't
always accurate. It will always be a pointer (or an unsafe.Pointer), but
the base type might not be accurate. This comes about because multiple
copies of a pointer with different base types are coalesced into a single value.
In the failing example, the IData selector of the input argument is a
*[32]byte in one branch of the type switch, and a *[]byte in the other branch.
During the expand_calls pass both IDatas become just copies of the input
register. Those copies are deduped and an arbitrary one wins (in this case,
*[]byte is the unfortunate winner).
Generally an op v can rely on v.Type during rewrite rules. But relying
on v.Args[i].Type is discouraged.
Fixes#55122
Change-Id: I348fd9accf2058a87cd191eec01d39cda612f120
Reviewed-on: https://go-review.googlesource.com/c/go/+/431496
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
Now we have 8-byte alignment types on 32-bit system, so in some rare
case, e.g, generated wrapper for embedded interface, the function
argument may need more than 4 byte alignment. We could pad somehow, but
this is a rare case which makes it hard to ensure that we've got it right.
So relaxing the check for argument and return value region of the stack.
Fixes#54991
Change-Id: I34986e17a920254392a39439ad3dcb323da2ea8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/431098
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
When SLTI/SLTIU is used with ANDI/ORI, it may be possible to determine the
outcome based on the values of the immediates. Resolve these cases.
Improves code generation for various shift operations.
While here, sort tests by architecture to improve readability and ease
future maintenance.
Change-Id: I87e71e016a0e396a928e7d6389a2df61583dfd8d
Reviewed-on: https://go-review.googlesource.com/c/go/+/428217
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Jenny Rakoczy <jenny@golang.org>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
Run-TryBot: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Jenny Rakoczy <jenny@golang.org>
Go 1.19 introduce new append-like APIs in package encoding/binary, this
change teaches the inliner to treat calls to these methods as cheap, so
that code using them will be more inlineable.
Updates #42958
Change-Id: Ie3dd4906e285430f435bdedbf8a11fdffce9302d
Reviewed-on: https://go-review.googlesource.com/c/go/+/431015
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Jenny Rakoczy <jenny@golang.org>
Reviewed-by: Jenny Rakoczy <jenny@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Jenny Rakoczy <jenny@golang.org>
Run-TryBot: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Keith Randall <khr@google.com>
This CL adds shiftIsBounded checks for the Lsh* and Rsh* rules in arm64.
There is no need to check the shift value again with CMP + CSEL when the
shift value is valid.
Change-Id: I54620de64f02a1b5a11089add237248ae2de01b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/417714
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.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>
The unified frontend ICEs when inlining a function that contains a
function literal, which captures both a type switch case variable and
another variable.
Updates #54912.
Change-Id: I0e16d371ed5df48a70823beb0bf12110a5a17266
Reviewed-on: https://go-review.googlesource.com/c/go/+/428917
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
The toStringData test was meant to test reflect.StringHeader, not
reflect.SliceHeader. It's not supported to convert *string to
*reflect.SliceHeader anyway.
Change-Id: Iaa4912eafd241886c6337bd7607cdf2412a15ead
Reviewed-on: https://go-review.googlesource.com/c/go/+/428995
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
It was fixed by CL 422196, and have been already worked in unified IR.
Fixes#54911
Change-Id: Ie69044a64b296f6961e667e7661d8c4d1a24d84e
Reviewed-on: https://go-review.googlesource.com/c/go/+/428758
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This reverts commit CL 427140.
Reason for revert: Comments say that done should be the first field.
Change-Id: Id131da064146b44e1182289546aeb877867e63cc
Reviewed-on: https://go-review.googlesource.com/c/go/+/428638
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
For defer/go calls, the function/method value are evaluated immediately.
So after devirtualizing, it may trigger a panic when implicitly deref
a nil pointer receiver, causing the program behaves unexpectedly.
It's safer to not devirtualizing defer/go calls at all.
Fixes#52072
Change-Id: I562c2860e3e577b36387dc0a12ae5077bc0766bf
Reviewed-on: https://go-review.googlesource.com/c/go/+/428495
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Change-Id: I49f8c764d49cabaad4d6859c219ba7220a389c1f
Reviewed-on: https://go-review.googlesource.com/c/go/+/427140
Run-TryBot: xie cui <523516579@qq.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Currently, gentraceback tracks the closure context of the outermost
frame. This used to be important for "unstarted" calls to reflect
function stubs, where "unstarted" calls are either deferred functions
or the entry-point of a goroutine that hasn't run. Because reflect
function stubs have a dynamic argument map, we have to reach into
their closure context to fetch to map, and how to do this differs
depending on whether the function has started. This was discovered in
issue #25897.
However, as part of the register ABI, "go" and "defer" were made much
simpler, and any "go" or "defer" of a function that takes arguments or
returns results gets wrapped in a closure that provides those
arguments (and/or discards the results). Hence, we'll see that closure
instead of a direct call to a reflect stub, and can get its static
argument map without any trouble.
The one case where we may still see an unstarted reflect stub is if
the function takes no arguments and has no results, in which case the
compiler can optimize away the wrapper closure. But in this case we
know the argument map is empty: the compiler can apply this
optimization precisely because the target function has no argument
frame.
As a result, we no longer need to track the closure context during
traceback, so this CL drops all of that mechanism.
We still have to be careful about the unstarted case because we can't
reach into the function's locals frame to pull out its context
(because it has no locals frame). We double-check that in this case
we're at the function entry.
I would prefer to do this with some in-code PCDATA annotations of
where to find the dynamic argument map, but that's a lot of mechanism
to introduce for just this. It might make sense to consider this along
with #53609.
Finally, we beef up the test for this so it more reliably forces the
runtime down this path. It's fundamentally probabilistic, but this
tweak makes it better. Scheduler testing hooks (#54475) would make it
possible to write a reliable test for this.
For #54466, but it's a nice clean-up all on its own.
Change-Id: I16e4f2364ba2ea4b1fec1e27f971b06756e7b09f
Reviewed-on: https://go-review.googlesource.com/c/go/+/424254
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Austin Clements <austin@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
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 changes the heuristic used to determine whether we can inline a
struct equality check or if we must generate a function and call that
function for equality.
The old method was to count struct fields, but this can lead to poor
in lining decisions. We should really be determining the cost of the
equality check and use that to determine if we should inline or generate
a function.
The new benchmark provided in this CL returns the following when compared
against tip:
```
name old time/op new time/op delta
EqStruct-32 2.46ns ± 4% 0.25ns ±10% -89.72% (p=0.000 n=39+39)
```
Fixes#38494
Change-Id: Ie06b80a2b2a03a3fd0978bcaf7715f9afb66e0ab
GitHub-Last-Rev: e9a18d9389
GitHub-Pull-Request: golang/go#53326
Reviewed-on: https://go-review.googlesource.com/c/go/+/411674
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: Heschi Kreinick <heschi@google.com>
This CL optimizes RotateLeft8/16 on arm64.
For 16 bits, we form a 32 bits register by duplicating two 16 bits
registers, then use RORW instruction to do the rotate shift.
For 8 bits, we just use LSR and LSL instead of RORW because the code is
simpler.
Benchmark Old ThisCL delta
RotateLeft8-46 2.16 ns/op 1.73 ns/op -19.70%
RotateLeft16-46 2.16 ns/op 1.54 ns/op -28.53%
Change-Id: I09cde4383d12e31876a57f8cdfd3bb4f324fadb0
Reviewed-on: https://go-review.googlesource.com/c/go/+/420976
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
So they can be added to ignored list, since the tests now require
cgo.Incomplete, which is not recognized by go/types and types2.
Updates #46731
Change-Id: I9f24e3c8605424d1f5f42ae4409437198f4c1326
Reviewed-on: https://go-review.googlesource.com/c/go/+/427142
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
For the following code case:
var x uint64
x >> (shift & 63)
We can directly genereta `x >> shift` on arm64, since the hardware will
only use the bottom 6 bits of the shift amount.
Benchmark old time/op new time/op delta
ShiftArithmeticRight-8 0.40ns 0.31ns -21.7%
Change-Id: Id58c8a5b2f6dd5c30c3876f4a36e11b4d81e2dc9
Reviewed-on: https://go-review.googlesource.com/c/go/+/425294
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
On AIX when external linking, for some symbols we need to add
dummy references to prevent the external linker from discarding
them. Currently we add the reference unconditionally. But if the
symbol doesn't exist, the linking fails in a later stage for
generating external relocation of a nonexistent symbol. The
symbols are special symbols that almost always exist, except that
go:buildid may not exist if the linker is invoked without the
-buildid flag. The go command invokes the linker with the flag, so
this can only happen with manual linker invocation. Specifically,
test/run.go does this in some cases.
Fix this by checking the symbol existence before adding the
reference. Re-enable tests on AIX.
Perhaps the linker should always emit a dummy buildid even if the
flag is not set...
Fixes#54814.
Change-Id: I43d81587151595309e189e38960cbda9a1c5ca32
Reviewed-on: https://go-review.googlesource.com/c/go/+/427620
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
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>
The syntax for go and defer specifies an arbitrary expression, not
a call; the call requirement is spelled out in prose. Don't to the
call check in the parser; instead move it to the type checker. This
is simpler and also allows the type checker to check expressions that
are not calls, and avoid "not used" errors due to such expressions.
We would like to make the same change in go/parser and go/types
but the change requires Go/DeferStmt nodes to hold an ast.Expr
rather than an *ast.CallExpr. We cannot change that for backward-
compatibility reasons. Since we don't test this behavior for the
type checkers alone (only for the compiler), we get away with it
for now.
Follow-up on CL 425675 which introduced the extra errors in the
first place.
Change-Id: I90890b3079d249bdeeb76d5673246ba44bec1a7b
Reviewed-on: https://go-review.googlesource.com/c/go/+/425794
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
- Use "expected X" rather then "expecting X".
- Report a better error when a type argument list is expected.
- Adjust various tests.
For #54511.
Change-Id: I0c5ca66ecbbdcae1a8f67377682aae6b0b6ab89a
Reviewed-on: https://go-review.googlesource.com/c/go/+/425734
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
If the go/defer syntax is bad, using a fake CallExpr may produce
a follow-on error in the type checker. Instead store a BadExpr
in the syntax tree (since an error has already been reported).
Adjust various tests.
For #54511.
Change-Id: Ib2d25f8eab7d5745275188d83d11620cad6ef47c
Reviewed-on: https://go-review.googlesource.com/c/go/+/425675
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Since when go/types,types2 do not know about build constraints, and
runtime/cgo.Incomplete is only available on platforms that support cgo.
These tests are also failing on aix with failure from linker, so disable
them on aix to make builder green. The fix for aix is tracked in #54814
Updates #46731
Updates #54814
Change-Id: I5d6f6e29a8196efc6c457ea64525350fc6b20309
Reviewed-on: https://go-review.googlesource.com/c/go/+/427394
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Same as CL 421880, but for test directory.
Updates #46731
Change-Id: If8d18df013a6833adcbd40acc1a721bbc23ca6b2
Reviewed-on: https://go-review.googlesource.com/c/go/+/421881
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@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>
Rotating by c, then by d, is the same as rotating by c+d.
Change-Id: I36df82261460ff80f7c6d39bcdf0e840cef1c91a
Reviewed-on: https://go-review.googlesource.com/c/go/+/424894
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Ruinan Sun <Ruinan.Sun@arm.com>
Run-TryBot: Keith Randall <khr@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
