CL 395541 made staticopy safe, stop applying the optimization once
seeing an expression that may modify global variables. However, it
misses the case for OASOP expression, causing the static init
mis-recognizes the modification and think it's safe.
Fixing this by adding missing OASOP case.
Fixes#66585
Change-Id: I603cec018d3b5a09825c14e1f066a0e16f8bde23
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/575216
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This reverts CL 553055.
Reason for revert: causes crypto/ecdsa failures on linux ppc64/s390x builders
Change-Id: I9266b030693a5b6b1e667a009de89d613755b048
Reviewed-on: https://go-review.googlesource.com/c/go/+/575236
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Preliminary compiler support for merging/overlapping stack
slots of local variables whose access patterns are disjoint.
This patch includes changes in AllocFrame to do the actual
merging/overlapping based on information returned from a new
liveness.MergeLocals helper. The MergeLocals helper identifies
candidates by looking for sets of AUTO variables that either A) have
the same size and GC shape (if types contain pointers), or B) have the
same size (but potentially different types as long as those types have
no pointers). Variables must be greater than (3*types.PtrSize) in size
to be considered for merging.
After forming candidates, MergeLocals collects variables into "can be
overlapped" equivalence classes or partitions; this process is driven
by an additional liveness analysis pass. Ideally it would be nice to
move the existing stackmap liveness pass up before AllocFrame
and "widen" it to include merge candidates so that we can do just a
single liveness as opposed to two passes, however this may be difficult
given that the merge-locals liveness has to take into account
writes corresponding to dead stores.
This patch also required a change to the way ssa.OpVarDef pseudo-ops
are generated; prior to this point they would only be created for
variables whose type included pointers; if stack slot merging is
enabled then the ssagen code creates OpVarDef ops for all auto vars
that are merge candidates.
Note that some temporaries created late in the compilation process
(e.g. during ssa backend) are difficult to reason about, especially in
cases where we take the address of a temp and pass it to the runtime.
For the time being we mark most of the vars created post-ssagen as
"not a merge candidate".
Stack slot merging for locals/autos is enabled by default if "-N" is
not in effect, and can be disabled via "-gcflags=-d=mergelocals=0".
Fixmes/todos/restrictions:
- try lowering size restrictions
- re-evaluate the various skips that happen in SSA-created autotmps
Fixes#62737.
Updates #65532.
Updates #65495.
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest
Change-Id: Ibc22e8a76c87e47bc9fafe4959804d9ea923623d
Reviewed-on: https://go-review.googlesource.com/c/go/+/553055
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This reverts CL 575175.
Reason for revert: causes crypto/ecdh failures on longtest builders.
Change-Id: Ieed326fedf91760ac73095a42ba0237cf969843b
Reviewed-on: https://go-review.googlesource.com/c/go/+/575316
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
Auto-Submit: Than McIntosh <thanm@google.com>
CL 395541 made staticopy safe, stop applying the optimization once
seeing an expression that may modify global variables.
However, if a call expression was inlined, the analyzer mis-recognizes
and think that the expression is safe. For example:
var x = 0
var a = f()
var b = x
are re-written to:
var x = 0
var a = ~r0
var b = 0
even though it's not safe because "f()" may modify "x".
Fixing this by recognizing OINLCALL and mark the initialization as
not safe for staticopy.
Fixes#66585
Change-Id: Id930c0b7e74274195f54a498cc4c5a91c4e6d84d
Reviewed-on: https://go-review.googlesource.com/c/go/+/575175
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
For https://gcc.gnu.org/PR114453
Change-Id: If41d9fca6288b18ed47b0f21ff224c74ddb34958
Reviewed-on: https://go-review.googlesource.com/c/go/+/574536
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
On amd64, we always zero-extend when loading arguments from the stack.
On arm64, we extend based on the type. This causes problems with
zeroUpper*Bits, which reports the top bits are zero when they aren't.
Fix it to use the type to decide if the top bits are really zero.
For tests, only f32 currently fails on arm64. Added other tests
just for future-proofing.
Update #66066
Change-Id: I2f13fb47198e139ef13c9a34eb1edc932eea3ee3
Reviewed-on: https://go-review.googlesource.com/c/go/+/571135
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Use `' quotes (as in `foo') to differentiate from Go quotes.
Quoting prevents confusion when user-supplied names alter
the meaning of the error message.
For instance, report
duplicate method `wanted'
rather than
duplicate method wanted
Exceptions:
- don't quote _:
`_' is ugly and not necessary
- don't quote after a ":":
undefined name: foo
- don't quote if the name is used correctly in a statement:
goto L jumps over variable declaration
Quoting is done with a helper function and can be centrally adjusted
and fine-tuned as needed.
Adjusted some test cases to explicitly include the quoted names.
Fixes#65790.
Change-Id: Icce667215f303ab8685d3e5cb00d540a2fd372ca
Reviewed-on: https://go-review.googlesource.com/c/go/+/571396
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Still investigating, but adding the minimized reproducer as a regress
test case for now.
Updates #66261.
Change-Id: I20715b731f8c5b95616513d4a13e3ae083709031
Reviewed-on: https://go-review.googlesource.com/c/go/+/571815
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When an opcode generates a known high bit state (typically, a sub-word
operation that zeros the high bits), we can remove any subsequent
extension operation that would be a no-op.
x = (OP ...)
y = (ZeroExt32to64 x)
If OP zeros the high 32 bits, then we can replace y with x, as the
zero extension doesn't do anything.
However, x in this situation normally has a sub-word-sized type. The
semantics of values in registers is typically that the high bits
beyond the value's type size are junk. So although the opcode
generating x *currently* zeros the high bits, after x is rewritten to
another opcode it may not - rewrites of sub-word-sized values can
trash the high bits.
To fix, move the extension-removing rules to late lower. That ensures
that their arguments won't be rewritten to change their high bits.
I am also worried about spilling and restoring. Spilling and restoring
doesn't preserve the high bits, but instead sets them to a known value
(often 0, but in some cases it could be sign-extended). I am unable
to come up with a case that would cause a problem here, so leaving for
another time.
Fixes#66066
Change-Id: I3b5c091b3b3278ccbb7f11beda8b56f4b6d3fde7
Reviewed-on: https://go-review.googlesource.com/c/go/+/568616
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
CL 541715 added an optimization to copy SSA-able variables.
When handling m[k] = append(m[k], ...) case, it uses ir.SameSafeExpr to
check that m[k] expressions are the same, then doing type assertion to
convert the map index to ir.IndexExpr node. However, this assertion is
not safe for m[k] expression in append(m[k], ...), since it may be
wrapped by ir.OCONVNOP node.
Fixing this by un-wrapping any ir.OCONVNOP before doing type assertion.
Fixes#66096
Change-Id: I9ff7165ab97bc7f88d0e9b7b31604da19a8ca206
Reviewed-on: https://go-review.googlesource.com/c/go/+/569716
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
The problem was caused by faulty handling of unSSA-able
operations on zero-sized data in expand calls, but there
is no point to operations on zero-sized data. This CL adds
a simplify step to the first place in SSA where all values
are processed and replaces anything producing a 0-sized
struct/array with the corresponding Struct/Array Make0
operation (of the appropriate type).
I attempted not generating them in ssagen, but that was a
larger change, and also had bugs. This is simple and obvious.
The only question is whether it would be worthwhile to do it
earlier (in numberlines or phielem).
Fixes#65808.
Change-Id: I0a596b3d272798015e7bb6b1a20411241759fe0e
Reviewed-on: https://go-review.googlesource.com/c/go/+/568258
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The itab we're initializing again, just to figure out which method
is missing, might be stored in read-only memory.
This can only happen in certain weird generics situations, so it is
pretty rare, but it causes a runtime crash when it does happen.
Fixes#65962
Change-Id: Ia86e216fe33950a794ad8e475e76317f799e9136
Reviewed-on: https://go-review.googlesource.com/c/go/+/567615
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
When the compiler writes PtrToThis field of noalg type, it generates
its pointer type. Mark them as noalg to prevent put them in typelinks.
Fixes#65957
Change-Id: Icbc3b18bc866f9138c7648e42dd500a80326f72b
Reviewed-on: https://go-review.googlesource.com/c/go/+/567335
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
Change-Id: I39af932b789cd18dc4bfc84f9667b1c32c9825f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/567476
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
CL 517775 moved early deadcode into unified writer. with new way to
handle dead code with label statement involved: any statements after
terminating statement will be considered dead until next label
statement.
However, this is not safe, because code after label statement may still
refer to dead statements between terminating and label statement.
It's only safe to remove statements after terminating *and* label one.
Fixes#65593
Change-Id: Idb630165240931fad50789304a9e4535f51f56e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/565596
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Use quotes to wrap user-supplied token in the syntax error message.
Updates #65790
Change-Id: I631a63df4a6bb8615b7850a324d812190bc15f30
GitHub-Last-Rev: f291e1d5a6adee558d21bb7e0a3a17471bad7eb6
GitHub-Pull-Request: golang/go#65840
Reviewed-on: https://go-review.googlesource.com/c/go/+/565518
Reviewed-by: Carlos Amedee <carlos@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
types2 handles all constant-related bounds checks in user Go code now,
so it's safe to remove the check from typecheck, avoid the inconsistency
with type parameter.
Fixes#65417
Change-Id: I82dd197b78e271725d132b5a20450ae3e90f9abc
Reviewed-on: https://go-review.googlesource.com/c/go/+/560575
Reviewed-by: Keith Randall <khr@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This CL separates the pass that computes inlinability from the pass
that performs inlinability. In particular, the latter can now happen
in any flat order, rather than bottom-up order. This also allows
inlining of calls exposed by devirtualization.
Change-Id: I389c0665fdc8288a6e25129a6744bfb1ace1eff7
Reviewed-on: https://go-review.googlesource.com/c/go/+/562319
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
This CL improves the error messages reported when a field or method
name is used that doesn't exist. It brings the error messges on par
(or better) with the respective errors reported before Go 1.18 (i.e.
before switching to the new type checker):
Make case distinctions based on whether a field/method is exported
and how it is spelled. Factor out that logic into a new function
(lookupError) in a new file (errsupport.go), which is generated for
go/types. Use lookupError when reporting selector lookup errors
and missing struct field keys.
Add a comprehensive set of tests (lookup2.go) and spot tests for
the two cases brought up by the issue at hand.
Adjusted existing tests as needed.
Fixes#49736.
Change-Id: I2f439948dcd12f9bd1a258367862d8ff96e32305
Reviewed-on: https://go-review.googlesource.com/c/go/+/560055
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Rather than implementing a new, less complete mechanism to check
if a selector exists with different capitalization, use the
existing mechanism in lookupFieldOrMethodImpl by making it
available for internal use.
Pass foldCase parameter all the way trough to Object.sameId and
thus make it consistently available where Object.sameId is used.
From sameId, factor out samePkg functionality into stand-alone
predicate.
Do better case distinction when reporting an error for an undefined
selector expression.
Cleanup.
Change-Id: I7be3cecb4976a4dce3264c7e0c49a320c87101e9
Reviewed-on: https://go-review.googlesource.com/c/go/+/558315
Reviewed-by: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
The input index to a jump table can be out of range for unreachable code.
Dynamically the compiler ensures that an out-of-range index can never
reach a jump table, but that guarantee doesn't extend to the static
realm.
Fixes#64826
Change-Id: I5829f3933ae5124ffad8337dfd7dd75e67a8ec33
Reviewed-on: https://go-review.googlesource.com/c/go/+/552055
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
Per the discussion on the issue, since no problems related to this
appeared since Go 1.20, remove the ability to disable the check for
anonymous interface cycles permanently.
Adjust various tests accordingly.
For #56103.
Change-Id: Ica2b28752dca08934bbbc163a9b062ae1eb2a834
Reviewed-on: https://go-review.googlesource.com/c/go/+/550896
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Right shifts, for some odd reasons, can encode shifts of constant
1-32 instead of 0-31. Left shifts, however, can encode shifts 0-31.
When the shift amount is 0, arm recommends encoding right shifts
using left shifts.
Fixes#64715
Change-Id: Id3825349aa7195028037893dfe01fa0e405eaa51
Reviewed-on: https://go-review.googlesource.com/c/go/+/549955
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
This commit is aimed at improving the readability and consistency
of the code base. Extraneous newline characters were present after
some return statements, creating unnecessary separation in the code.
Fixes#64610
Change-Id: Ic1b05bf11761c4dff22691c2f1c3755f66d341f7
Reviewed-on: https://go-review.googlesource.com/c/go/+/548316
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
We can't delete all the outgoing edges and then add one back in, because
then we've lost the argument of any phi at the target. Instead, move
the important target to the front of the list and delete the rest.
This normally isn't a problem, because there is never normally a phi
at the target of a jump table. But this isn't quite true when in race
build mode, because there is a phi of the result of a bunch of raceread
calls.
The reason this happens is that each case is written like this (where e
is the runtime.eface we're switching on):
if e.type == $type.int32 {
m = raceread(e.data, m1)
}
m2 = phi(m1, m)
if e.type == $type.int32 {
.. do case ..
goto blah
}
so that if e.type is not $type.int32, it falls through to the default
case. This default case will have a memory phi for all the (jumped around
and not actually called) raceread calls.
If we instead did it like
if e.type == $type.int32 {
raceread(e.data)
.. do case ..
goto blah
}
That would paper over this bug, as it is the only way to construct
a jump table whose target is a block with a phi in it. (Yet.)
But we'll fix the underlying bug in this CL. Maybe we can do the
rewrite mentioned above later. (It is an optimization for -race mode,
which isn't particularly important.)
Fixes#64606
Change-Id: I6f6e3c90eb1e2638112920ee2e5b6581cef04ea4
Reviewed-on: https://go-review.googlesource.com/c/go/+/548356
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
When I was plumbing min/max support through the compiler, I was
thinking mostly about numeric argument types. As a result, I forgot
that escape analysis would need to be aware that min/max can operate
on string values, which contain pointers.
Fixes#64565.
Change-Id: I36127ce5a2da942401910fa0f9de922726c9f94d
Reviewed-on: https://go-review.googlesource.com/c/go/+/547715
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Mauri de Souza Meneguzzo <mauri870@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
This CL interleaves devirtualization and inlining, so that
devirtualized calls can be inlined.
Fixes#52193.
Change-Id: I681e7c55bdb90ebf6df315d334e7a58f05110d9c
Reviewed-on: https://go-review.googlesource.com/c/go/+/528321
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Bypass: Matthew Dempsky <mdempsky@google.com>
The early return here is meant to suppress inlining of the function
call itself. However, it also suppresses recursing to visit the call
arguments, which are safe to inline.
Change-Id: I75887574c00931cb622277d04a822bc84c29bfa2
Reviewed-on: https://go-review.googlesource.com/c/go/+/543658
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Fix#63955
parseIndVar, prove and maybe more are on the assumption that the loop header
is a single block. This can be wrong, ensure we don't match theses cases we
don't know how to handle.
In the future we could update them so that they know how to handle such cases
but theses cases seems rare so I don't think the value would be really high.
We could also run a loop canonicalization pass first which could handle this.
The repro case looks weird because I massaged it so it would crash with the
previous compiler.
Change-Id: I4aa8afae9e90a17fa1085832250fc1139c97faa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/539977
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Have nil checks return a pointer that is known non-nil. Users of
that pointer can use the result, ensuring that they are ordered
after the nil check itself.
The order dependence goes away after scheduling, when we've fixed
an order. At that point we move uses back to the original pointer
so it doesn't change regalloc any.
This prevents pointer arithmetic on nil from being spilled to the
stack and then observed by a stack scan.
Fixes#63657
Change-Id: I1a5fa4f2e6d9000d672792b4f90dfc1b7b67f6ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/537775
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@google.com>
I will shortly be sending CLs to let the gofrontend code pass
these tests.
Change-Id: I53ccbdac3ac224a4fdc9577270f48136ca73a62c
Reviewed-on: https://go-review.googlesource.com/c/go/+/536537
Reviewed-by: Cherry Mui <cherryyz@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Most of the test cases in the test directory use the new go:build syntax
already. Convert the rest. In general, try to place the build constraint
line below the test directive comment in more places.
For #41184.
For #60268.
Change-Id: I11c41a0642a8a26dc2eda1406da908645bbc005b
Cq-Include-Trybots: luci.golang.try:gotip-linux-386-longtest,gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/536236
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
The funcdata is encoded as varint, with the upper limit set to 1e9.
However, the stack offsets could be up to 1<<30. Thus emitOpenDeferInfo
will trigger an ICE for function with large frame size.
By using binary.PutUvarint, the frame offset could be encoded correctly
for value larger than 1<<35, allow the compiler to report the error.
Further, the runtime also do validation when reading in the funcdata
value, so a bad offset won't likely cause mis-behavior.
Fixes#52697
Change-Id: I084c243c5d24c5d31cc22d5b439f0889e42b107c
Reviewed-on: https://go-review.googlesource.com/c/go/+/535077
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The immediate-data interface shortcuts also apply to pointer-shaped
things like maps, channels, and functions.
Fixes#63505.
Change-Id: I43982441bf523f53e9e5d183e95aea7c6655dd6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/534657
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: David Chase <drchase@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Do this by removing all stores of zero-sized anything.
Fixes#63433.
Change-Id: I5d8271edab992d15d02005fa3fe31835f2eff8fa
Reviewed-on: https://go-review.googlesource.com/c/go/+/534296
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
CL 419456 starts using lookupObj to find types2.Object associated with
builtin functions. However, the new code does not un-parenthesized the
callee expression, causing an ICE because of nil obj returned.
Un-parenthesizing the callee expression fixes the problem.
Fixes#63436
Change-Id: Iebb4fbc08575e7d0b1dbd026c98e8f949ca16460
Reviewed-on: https://go-review.googlesource.com/c/go/+/533476
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
The types2 typechecker already reported all invalid conversions required
by the Go language spec. However, the conversion involves go pragma is
not specified in the spec, so is not checked by types2.
Fixing this by handling the error gracefully during typecheck, just like
how old typechecker did before CL 394575.
Fixes#63333
Change-Id: I04c4121971c62d96f75ded1794ab4bdf3a6cd0ea
Reviewed-on: https://go-review.googlesource.com/c/go/+/532515
Auto-Submit: Cuong Manh Le <cuong.manhle.vn@gmail.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
This patch changes the inliner to use callsite scores when deciding to
inline as opposed to looking only at callee cost/hairyness.
For this to work, we have to relax the inline budget cutoff as part of
CanInline to allow for the possibility that a given function might
start off with a cost of N where N > 80, but then be called from a
callsites whose score is less than 80. Once a given function F in
package P has been approved by CanInline (based on the relaxed budget)
it will then be emitted as part of the export data, meaning that other
packages importing P will need to also need to compute callsite scores
appropriately.
For a function F that calls function G, if G is marked as potentially
inlinable then the hairyness computation for F will use G's cost for
the call to G as opposed to the default call cost; for this to work
with the new scheme (given relaxed cost change described above) we
use G's cost only if it falls below inlineExtraCallCost, otherwise
just use inlineExtraCallCost.
Included in this patch are a bunch of skips and workarounds to
selected 'errorcheck' tests in the <GOROOT>/test directory to deal
with the additional "can inline" messages emitted when the new inliner
is turned on.
Change-Id: I9be5f8cd0cd8676beb4296faf80d2f6be7246335
Reviewed-on: https://go-review.googlesource.com/c/go/+/519197
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Previously, the unified frontend implemented unsafe.Sizeof, etc that
involved derived types by constructing a normal OSIZEOF, etc
expression, including fully instantiating their argument. (When
unsafe.Sizeof is applied to a non-generic type, types2 handles
constant folding it.)
This worked, but involves unnecessary work, since all we really need
to track is the argument type (and the field selections, for
unsafe.Offsetof).
Further, the argument expression could generate temporary variables,
which would then go unused after typecheck replaced the OSIZEOF
expression with an OLITERAL. This results in compiler failures after
CL 523315, which made later passes stricter about expecting the
frontend to not construct unused temporaries.
Fixes#62515.
Change-Id: I37baed048fd2e35648c59243f66c97c24413aa94
Reviewed-on: https://go-review.googlesource.com/c/go/+/527097
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Currently, cmd/compile optimizes `var a = true; var b = a` into `var a
= true; var b = true`. But this may not be safe if we need to
initialize any other global variables between `a` and `b`, and the
initialization involves calling a user-defined function that reassigns
`a`.
This CL changes staticinit to keep track of the initialization
expressions that we've seen so far, and to stop applying the
staticcopy optimization once we've seen an initialization expression
that might have modified another global variable within this package.
To help identify affected initializers, this CL adds a -d=staticcopy
flag to warn when a staticcopy is suppressed and turned into a dynamic
copy.
Currently, `go build -gcflags=all=-d=staticcopy std` reports only four
instances:
```
encoding/xml/xml.go:1600:5: skipping static copy of HTMLEntity+0 with map[string]string{...}
encoding/xml/xml.go:1869:5: skipping static copy of HTMLAutoClose+0 with []string{...}
net/net.go:661:5: skipping static copy of .stmp_31+0 with poll.ErrNetClosing
net/http/transport.go:2566:5: skipping static copy of errRequestCanceled+0 with ~R0
```
Fixes#51913.
Change-Id: Iab41cf6f84c44f7f960e4e62c28a8aeaade4fbcf
Reviewed-on: https://go-review.googlesource.com/c/go/+/395541
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
When creating the struct type to hold variables captured by a function
literal, we currently reuse the captured variable names as fields.
However, there's no particular reason to do this: these struct types
aren't visible to users, and it adds extra complexity in making sure
fields belong to the correct packages.
Further, it turns out we were getting that subtly wrong. If two
function literals from different packages capture variables with
identical names starting with an uppercase letter (and in the same
order and with corresponding identical types) end up in the same
function (e.g., due to inlining), then we could end up creating
closure struct types that are "different" (i.e., not types.Identical)
yet end up with equal LinkString representations (which violates
LinkString's contract).
The easy fix is to just always use simple, exported, generated field
names in the struct. This should allow further struct reuse across
packages too, and shrink binary sizes slightly.
Fixes#62498.
Change-Id: I9c973f5087bf228649a8f74f7dc1522d84a26b51
Reviewed-on: https://go-review.googlesource.com/c/go/+/527135
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
One of the more tedious quirks of the original frontend (i.e.,
typecheck) to preserve was that it preserved the original
representation of constants into the backend. To fit into the unified
IR model, I ended up implementing a fairly heavyweight workaround:
simply record the original constant's string expression in the export
data, so that diagnostics could still report it back, and match the
old test expectations.
But now that there's just a single frontend to support, it's easy
enough to just update the test expectations and drop this support for
"raw" constant expressions.
Change-Id: I1d859c5109d679879d937a2b213e777fbddf4f2f
Reviewed-on: https://go-review.googlesource.com/c/go/+/526376
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Unfortunately, there isn't a single op that provides the resulting
computation.
At least, I couldn't find one.
Fixes#62469
Change-Id: I236f3965b827aaeb3d70ef9fe89be66b116494f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/526276
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Keith Randall <khr@google.com>
This reverts commit c1dfbf72e1298df8495171810a553836e5027f98.
Reason for revert: TESTL rule is wrong when the result is used for an ordered comparison.
Fixes#62360
Change-Id: I4d5b6aca24389b0a2bf767bfbc0a9d085359eb38
Reviewed-on: https://go-review.googlesource.com/c/go/+/524255
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Jakub Ciolek <jakub@ciolek.dev>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
In CL 522879, I moved the logic for setting Addrtaken from typecheck's
markAddrOf and ComputeAddrtaken directly into ir.NewAddrExpr. However,
I took the logic from markAddrOf, and failed to notice that
ComputeAddrtaken also set Addrtaken on the canonical ONAME.
The result is that if the only address-of expressions were within a
function literal, the canonical variable never got marked Addrtaken.
In turn, this could cause the consistency check in ir.Reassigned to
fail. (Yay for consistency checks turning mistakes into ICEs, rather
than miscompilation.)
Fixes#62313.
Change-Id: Ieab2854cd7fcc1b6c5d1e61de66453add9890a4f
Reviewed-on: https://go-review.googlesource.com/c/go/+/523375
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
