When creating the temporary for map functions, if the key
contains pointer, we need to create pointer-typed temporary. So
if the temporary is live across a function call, the pointer is
live.
Change-Id: Id6e14ec9def8bc7987f0f8ce8423caf1e3754fcb
Reviewed-on: https://go-review.googlesource.com/c/go/+/311379
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Currently, if we have AX=a and BX=b, and we want to make a call
F(1, a, b), to move arguments into the desired registers it emits
MOVQ AX, CX
MOVL $1, AX // AX=1
MOVQ BX, DX
MOVQ CX, BX // BX=a
MOVQ DX, CX // CX=b
This has a few redundant moves.
This is because we process inputs in order. First, allocate 1 to
AX, which kicks out a (in AX) to CX (a free register at the
moment). Then, allocate a to BX, which kicks out b (in BX) to DX.
Finally, put b to CX.
Notice that if we start with allocating CX=b, then BX=a, AX=1,
we will not have redundant moves. This CL reduces redundant moves
by allocating them in different order: First, for inpouts that are
already in place, keep them there. Then allocate free registers.
Then everything else.
before after
cmd/compile binary size 23703888 23609680
text size 8565899 8533291
(with regabiargs enabled.)
Change-Id: I69e1bdf745f2c90bb791f6d7c45b37384af1e874
Reviewed-on: https://go-review.googlesource.com/c/go/+/311371
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
With defer/go wrapping and register arguments, some liveness info
changed and live.go test was disabled for regabi. This CL adds a
new one for regabi.
Change-Id: I65f03a6ef156366d8b76c62a16251c3e818f4b02
Reviewed-on: https://go-review.googlesource.com/c/go/+/311369
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
CL 256798 added compiler ability to retain only used interface methods,
by generating a mark relocation whenever an interface method is used. To
do that, the compiler needs the current function linker object.
However, for unnamed function "func _()", its linker object is nil,
causes the compiler crashes for code in #45258.
CL 283313 fixed the code in #45258 unintentionally, since when the
compiler now does not walk unnamed functions anymore.
This CL fixes the root issue, by making reflectdata.MarkUsedIfaceMethod
skips unnamed functions, and also adding regression test.
Fixes#45258
Change-Id: I4cbefb0a89d9928f70c00dc8a271cb61cd20a49c
Reviewed-on: https://go-review.googlesource.com/c/go/+/311130
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
The build.Default context really needs to accurately describe
the default build context. The goexperiment tags being a special
case in the go command violates that rule and is the root cause
of the various try-bot failures blocking the enabling of regabi.
(The cleanups I made in golang.org/x/tools were long overdue
but are not strictly necessary for making regabi work; this CL is.)
Having moved the GOEXPERIMENT parsing into internal/buildcfg,
go/build can now use it to set up build.Default, in the new field
ToolTags, meant to hold toolchain-determined tags (for now,
just the experiments). And at the same time we can remove the
duplication of GOOS and GOARCH defaults.
And then once build.Default is set up accurately, the special case
code in cmd/go itself can be removed, and the special case code
in test/run.go is at least a bit less special.
Change-Id: Ib7394e10aa018e492cb9a83fb8fb9a5011a8c25b
Reviewed-on: https://go-review.googlesource.com/c/go/+/310732
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Currently, run.go sets GOEXPERIMENT build tags based on the
*difference* from the baseline experiment configuration, rather than
the absolute experiment configuration. This differs from cmd/go. As a
result, if we set a baseline configuration and don't override it with
a GOEXPERIMENT setting, run.go won't set any GOEXPERIMENT build tags,
instead of setting the tags corresponding to the baseline
configuration.
Fix this by making compile -V=goexperiment produce the full
GOEXPERIMENT configuration, which run.go can then use to set exactly
the right set of build tags.
For #40724.
Change-Id: Ieda6ea62f1a1fabbe8d749d6d09c198fd5ca8377
Reviewed-on: https://go-review.googlesource.com/c/go/+/310171
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Only complain about missing type; leave it to type-checking
to decide whether "..." is permitted in the first place.
Fixes#43674.
Change-Id: Icbc8f084e364fe3ac16076406a134354219c08d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/310209
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Change-Id: I28a1910890659aaa449ffd2a847cd4ced5a8600d
Reviewed-on: https://go-review.googlesource.com/c/go/+/310211
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Add in some missing global assignment ops to the list of globals ops
that should be traversed to look for generic function instantiations.
The most common other one for global assigments (and the relevant one
for this bug) is OAS2FUNC, but also look at global assigments with
OAS2DOTTYPE, OAS2MAPR, OAS2RECV, and OASOP.
Bonus small fix: get rid of -G=3 case in ir.IsAddressable. Now that we
don't call the old typechecker from noder2, we don't need this -G-3
check anymore.
Fixes#45547.
Change-Id: I75fecec55ea0d6f62e1c2294d4d77447ed9be6ae
Reviewed-on: https://go-review.googlesource.com/c/go/+/310210
Trust: Dan Scales <danscales@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Fixes this failure:
go test cmd/compile/internal/ssa -run TestStmtLines -v
=== RUN TestStmtLines
stmtlines_test.go:115: Saw too many (amd64, > 1%) lines without
statement marks, total=88263, nostmt=1930
('-run TestStmtLines -v' lists failing lines)
The failure has two causes.
One is that the first-line adjuster in code generation was relocating
"first lines" to instructions that would either not have any code generated,
or would have the statment marker removed by a different believed-good heuristic.
The other was that statement boundaries were getting attached to register
values (that with the old ABI were loads from the stack, hence real instructions).
The register values disappear at code generation.
The fixes are to (1) note that certain instructions are not good choices for
"first value" and skip them, and (2) in an expandCalls post-pass, look for
register valued instructions and under appropriate conditions move their
statement marker to a compatible use.
Also updates TestStmtLines to always log the score, for easier comparison of
minor compiler changes.
Updates #40724.
Change-Id: I485573ce900e292d7c44574adb7629cdb4695c3f
Reviewed-on: https://go-review.googlesource.com/c/go/+/309649
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
For OMETHEXPR, the Name in the Selection needs to be properly
linked up to the method declaration. Use the same code we
already have for ODOTMETH and OCALLPART to do that.
Fixes#45503
Change-Id: I7d6f886d606bae6faad8c104f50c177f871d41c8
Reviewed-on: https://go-review.googlesource.com/c/go/+/309831
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Dan Scales <danscales@google.com>
For the new export/import of node types, we were just missing setting
the types of the closure variables (which have the same types as the
captured variables) and the OCLOSURE node itself (which has the same
type as the Func node).
Re-enabled inlining of functions with closures.
Change-Id: I687149b061f3ffeec3244ff02dc6e946659077a9
Reviewed-on: https://go-review.googlesource.com/c/go/+/308974
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Some codegen tests were written with the assumption that
arguments and results are in memory, and with a specific stack
layout. With the register ABI, the assumption is no longer true.
Adjust the tests to work with both cases.
- For tests expecting in memory arguments/results, change to use
global variables or memory-assigned argument/results.
- Allow more registers. E.g. some tests expecting register names
contain only letters (e.g. AX), but it can also contain numbers
(e.g. R10).
- Some instruction selection changes when operate on register vs.
memory, e.g. ADDQ vs. LEAQ, MOVB vs. MOVL. Accept both.
TODO: mathbits.go and memops.go still need fix.
Change-Id: Ic5932b4b5dd3f5d30ed078d296476b641420c4c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/309335
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
If GOEXPERIMENT environment variable is unset, use the default
value that is baked into the toolchain (instead of no
experiments).
Change-Id: I41f863e6f7439f2d53e3ebd25a7d9cf4a176e32e
Reviewed-on: https://go-review.googlesource.com/c/go/+/309333
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Include type information on exported function bodies, so that the
importer does not have to re-typecheck the body. This involves
including type information in the encoded output, as well as
avoiding some of the opcode rewriting and other changes that the
old exporter did assuming there would be a re-typechecking pass.
This CL could be considered a cleanup, but is more important than that
because it is an enabling change for generics. Without this CL, we'd
have to upgrade the current typechecker to understand generics. With
this CL, the current typechecker can mostly go away in favor of the
types2 typechecker.
For now, inlining of functions that contain closures is turned off.
We will hopefully resolve this before freeze.
Object files are only 0.07% bigger.
Change-Id: I85c9da09f66bfdc910dc3e26abb2613a1831634d
Reviewed-on: https://go-review.googlesource.com/c/go/+/301291
Trust: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
Reviewed-by: Dan Scales <danscales@google.com>
A quick check of the source to run.go suggests that it does not
look for the new-style build tags.
Updates #45465.
Change-Id: Ib4be040935d71e732f81d52c4a22c2b514195f40
Reviewed-on: https://go-review.googlesource.com/c/go/+/308934
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Trust: David Chase <drchase@google.com>
This tickles some other bug, do this to clear builders.
Updates #40724.
Updates #45465.
Change-Id: Id51efbcf474865da231fcbc6216e5d604f99c296
Reviewed-on: https://go-review.googlesource.com/c/go/+/308889
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Softfloat mode with register ABI is not implemented yet. In
particular, we did not rewrite the float types in AuxCalls to
integer types, so arguments are still passed in floating point
registers, which do not exist in softfloat mode. To make it work
I think we may want to reorder softfloat pass with expand_calls
pass. We also need to rewrite the OpArgFloatRegs for the spilling
of non-SSA-able arguments, which may involve renumbering interger
arguments. Maybe in softfloat mode we want to just define the
ABI with 0 float registers. They are not fundamentally hard, but
may be not worth doing for the moment, as we don't use softfloat
mode on AMD64 anyway.
Run the test with noregabiargs. Also in the compiler reject
-d=softfloat if regabiargs is enabled.
Change-Id: I8cc0c2cfa88a138bc1338ed8710670245f1bd2cd
Reviewed-on: https://go-review.googlesource.com/c/go/+/308710
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
The helper function used by the compiler's walk phase to determine
whether a param can be passed in a single float register wasn't quite
correct (didn't allow for the possibility of struct with two fields,
first zero size and second float). Fix up the helper to take this
case into account.
Updates #40724.
Change-Id: I55b42a1b17ea86de1d696788f029ad3aae4a179c
Reviewed-on: https://go-review.googlesource.com/c/go/+/308689
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
The function runtime.convT64 accepts a single uint64 argument, but the
compiler's rules in the walk phase for determining whether is it ok to
pass a value of type T to a call to runtime.convT64 were slightly off.
In particular the test was allowing a type T with size less than eight
bytes but with more than one internal element (e.g. a struct). This
patch tightens up the rules somewhat to prevent this from happening.
Updates #40724.
Change-Id: I3b909267534db59429b0aa73a3d73333e1bd6432
Reviewed-on: https://go-review.googlesource.com/c/go/+/308069
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
go.mod file was not tidy, made builders sad.
Updates #40724.
Change-Id: I28371a1093108f9ec473eb20bb4d185e35dee67d
Reviewed-on: https://go-review.googlesource.com/c/go/+/308590
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In expand_calls, OpSelectN occurs both before and after the rewriting.
Attempting to rewrite a post-expansion OpSelectN is bad.
(The only ones rewritten in place are the ones returning mem;
others are synthesized to replace other selection chains with
register references.)
Updates #40724.
Updates #44816#issuecomment-815258897.
Change-Id: I7b6022cfb47f808d3ce6cc796c067245f36047f3
Reviewed-on: https://go-review.googlesource.com/c/go/+/308309
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
There's a problem in liveness, where liveness of any
part of an aggregate keeps the whole aggregate alive,
but the not-live parts don't get spilled. The GC
can observe those live-but-not-spilled slots, which
can contain junk.
A better fix is to change liveness to work
pointer-by-pointer, but that is also a riskier,
trickier fix.
To avoid this, in the case of
(1) an aggregate input parameter
(2) containing pointers
(3) passed in registers
pre-spill the pointers.
Updates #40724.
Change-Id: I6beb8e0a353b1ae3c68c16072f56698061922c04
Reviewed-on: https://go-review.googlesource.com/c/go/+/307909
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 302231 added some optimization rules with instructions CSETM, CSINC,
CSINV, and CSNEG, but did not deal with the situation where flag is
constant, resulting in some cases that could be more optimized cannot
be optimized, and the FlagConstant value is passed to codegen pass. This
CL adds these missing rules.
Fixes#45359
Change-Id: I700608cfb9a6a768a18d1fd5d374d7e92aa6f838
Reviewed-on: https://go-review.googlesource.com/c/go/+/307650
Reviewed-by: eric fang <eric.fang@arm.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Alberto Donizetti <alb.donizetti@gmail.com>
Run-TryBot: eric fang <eric.fang@arm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: eric fang <eric.fang@arm.com>
Trust: Alberto Donizetti <alb.donizetti@gmail.com>
When a function panics then recovers, it needs to return to the
caller with named results having the correct values. For
in-register results, we need to load them into registers at the
defer return path.
For non-open-coded defers, we already generate correct code, as
the defer return path is part of the SSA CFG and contains the
instructions that are the same as an ordinary return statement,
including putting the results to the right places.
For open-coded defers, we have a special code generation that
emits a disconnected block that currently contains only the
deferreturn call and a RET instruction. It leaves the result
registers unset. This CL adds instructions that load the result
registers on that path.
Updates #40724.
Change-Id: I1f60514da644fd5fb4b4871a1153c62f42927282
Reviewed-on: https://go-review.googlesource.com/c/go/+/307231
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Austin Clements <austin@google.com>
The exact same test case covered by this file is also in
fixedbugs/bug121.go. No need for duplication.
Also, the actual syntax error tested (multiple method names
with a single signature) is an unlikely syntax error, and
only here for historical reasons (in the very beginning, this
was actually possible to write). Now, virtually nobody is making
this mistake.
Change-Id: I9d68e0aee2a63025f44e6338647f8250ecc3077a
Reviewed-on: https://go-review.googlesource.com/c/go/+/307789
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This fixes a compile crash for
GOEXPERIMENT=regabi,regabiargs go test -c go/constant
Updates #40724.
Change-Id: I238cef436e045647815326fc8fdb025c30ba1f5c
Reviewed-on: https://go-review.googlesource.com/c/go/+/307309
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Update references missed in CL 263142.
For #41190
Change-Id: I778760a6a69bd0440fec0848bdef539c9ccb4ee1
GitHub-Last-Rev: dda42b09ff
GitHub-Pull-Request: golang/go#42874
Reviewed-on: https://go-review.googlesource.com/c/go/+/273946
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Trust: Cherry Zhang <cherryyz@google.com>
Leftover values that have been replaced can cause problems in later
passes (within expandCalls). For example, a struct select that
itself yields a struct will have a problematic rewrite, if the chance
is presented.
Updates #40724.
Change-Id: I1b445c47c301c3705f7fc0a9d39f1f5c84f4e190
Reviewed-on: https://go-review.googlesource.com/c/go/+/306869
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
The math to invert the input index was wrong.
Fixes#45323
Change-Id: I7c68cac280e8f01a9c806ecb0f195f169267437e
Reviewed-on: https://go-review.googlesource.com/c/go/+/306431
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: fannie zhang <Fannie.Zhang@arm.com>
Reviewed-by: David Chase <drchase@google.com>
In CL 305672 we preserve the pointer type of a store by just not
decomposing it. But this can be problematic when the source of
the store is a direct interface aggregate type (e.g.
struct { x map[int]int }.
In this CL we take a different approach: we preserve the store
type when generating the new store, but also decompose the source.
Fixes#45344.
Change-Id: If5dd496458dee95aa649c6d106b96a6cdcf3e60d
Reviewed-on: https://go-review.googlesource.com/c/go/+/306669
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Includes test.
Long term, need to make the offending code be more in terms
of official types package offsets, instead of duplicating that
logic.
For #40724.
Change-Id: Id33a153f10aed3289cc48d1f99a8e0f6ece9474d
Reviewed-on: https://go-review.googlesource.com/c/go/+/306469
Trust: David Chase <drchase@google.com>
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
For in-register arguments, it must have only a single copy of it
present in the function. If there are multiple copies, it confuses
the register allocator, as they are in the same register.
Change-Id: I55cb06746f08aa7c9168026d0f411bce0a9f93f4
Reviewed-on: https://go-review.googlesource.com/c/go/+/306330
Trust: Cherry Zhang <cherryyz@google.com>
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
During substitution of the function type during stenciling, we must set
the Name nodes of the param/result fields of the func type. We get those
name nodes from the substituted Dcl nodes of the PPARAMS and PPARAMOUTs.
But we must check that the names match with the Dcl nodes, so that we
skip any param fields that correspond to unnamed (in) parameters.
Added a few tests to typelist.go by removing a variety of unneeded
function parameter names.
Change-Id: If786961b64549da6f18eeeb5060ea58fab874eb9
Reviewed-on: https://go-review.googlesource.com/c/go/+/305912
Trust: Dan Scales <danscales@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
In expand_calls, when rewriting OpArg to OpArgIntReg/OpArgFloatReg,
avoid generating duplicates. Otherwise it will confuse the
register allocator: it would think the second occurance clobbers
the first's register, causing it to generate copies, which may
clobber other args.
Change-Id: I4f1dc0519afb77500eae1c0e6ac8745e51f7aa4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/306029
Trust: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
Handle the case where types can be partially inferred for an
instantiated function that is not immediately called. The key for the
Inferred map is the CallExpr (if inferring types required the function
arguments) or the IndexExpr (if types could be inferred without the
function arguments).
Added new tests for the case where the function isn't immediately called
to typelist.go.
Change-Id: I60f503ad67cd192da2f2002060229efd4930dc39
Reviewed-on: https://go-review.googlesource.com/c/go/+/305909
Trust: Dan Scales <danscales@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
The correct setting of t.nod is needed when exporting types. Make sure
we create instantiated named types correctly so t.nod is set.
New test file interfacearg.go that tests this (by instantiating a type
with an interface). Also has tests for various kinds of method
expressions.
Change-Id: Ia7fd9debd495336b73788af9e35d72331bb7d2b5
Reviewed-on: https://go-review.googlesource.com/c/go/+/305730
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Dan Scales <danscales@google.com>
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Fix various small bugs related to delaying transformations due to type
params. Most of these relate to the need to delay a transformation when
an argument of an expression or statement has a type parameter that has
a structural constraint. The structural constraint implies the operation
should work, but the transformation can't happen until the actual value
of the type parameter is known.
- delay transformations for send statements and return statements if
any args/values have type params.
- similarly, delay transformation of a call where the function arg has
type parameters. This is mainly important for the case where the
function arg is a pure type parameter, but has a structural
constraint that requires it to be a function. Move the setting of
n.Use to transformCall(), since we may not know how many return
values there are until then, if the function arg is a type parameter.
- set the type of unary expressions from the type2 type (as we do with
most other expressions), since that works better with expressions
with type params.
- deal with these delayed transformations in subster.node() and convert
the CALL checks to a switch statement.
- make sure ir.CurFunc is set properly during stenciling, including
closures (needed for transforming return statements during
stenciling).
New test file typelist.go with tests for these cases.
Change-Id: I1b82f949d8cec47d906429209e846f4ebc8ec85e
Reviewed-on: https://go-review.googlesource.com/c/go/+/305729
Trust: Dan Scales <danscales@google.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
Under certain circumstances, the existing rules for bit operations can
produce code that writes beyond its intended bounds. For example,
consider the following code:
func repro(b []byte, addr, bit int32) {
_ = b[3]
v := uint32(b[0]) | uint32(b[1])<<8 | uint32(b[2])<<16 | uint32(b[3])<<24 | 1<<(bit&31)
b[0] = byte(v)
b[1] = byte(v >> 8)
b[2] = byte(v >> 16)
b[3] = byte(v >> 24)
}
Roughly speaking:
1. The expression `1 << (bit & 31)` is rewritten into `(SHLL 1 bit)`
2. The expression `uint32(b[0]) | uint32(b[1])<<8 | uint32(b[2])<<16 |
uint32(b[3])<<24` is rewritten into `(MOVLload &b[0])`
3. The statements `b[0] = byte(v) ... b[3] = byte(v >> 24)` are
rewritten into `(MOVLstore &b[0], v)`
4. `(ORL (SHLL 1, bit) (MOVLload &b[0]))` is rewritten into
`(BTSL (MOVLload &b[0]) bit)`. This is a valid transformation because
the destination is a register: in this case, the bit offset is masked
by the number of bits in the destination register. This is identical
to the masking performed by `SHL`.
5. `(MOVLstore &b[0] (BTSL (MOVLload &b[0]) bit))` is rewritten into
`(BTSLmodify &b[0] bit)`. This is an invalid transformation because
the destination is memory: in this case, the bit offset is not
masked, and the chosen instruction may write outside its intended
32-bit location.
These changes fix the invalid rewrite performed in step (5) by
explicitly maksing the bit offset operand to `BT(S|R|C)(L|Q)modify`. In
the example above, the adjusted rules produce
`(BTSLmodify &b[0] (ANDLconst [31] bit))` in step (5).
These changes also add several new rules to rewrite bit sets, toggles,
and clears that are rooted at `(OR|XOR|AND)(L|Q)modify` operators into
appropriate `BT(S|R|C)(L|Q)modify` operators. These rules catch cases
where `MOV(L|Q)store ((OR|XOR|AND)(L|Q) ...)` is rewritten to
`(OR|XOR|AND)(L|Q)modify` before the `(OR|XOR|AND)(L|Q) ...` can be
rewritten to `BT(S|R|C)(L|Q) ...`.
Overall, compilecmp reports small improvements in code size on
darwin/amd64 when the changes to the compiler itself are exlcuded:
file before after Δ %
runtime.s 536464 536412 -52 -0.010%
bytes.s 32629 32593 -36 -0.110%
strings.s 44565 44529 -36 -0.081%
os/signal.s 7967 7959 -8 -0.100%
cmd/vendor/golang.org/x/sys/unix.s 81686 81678 -8 -0.010%
math/big.s 188235 188253 +18 +0.010%
cmd/link/internal/loader.s 89295 89056 -239 -0.268%
cmd/link/internal/ld.s 633551 633232 -319 -0.050%
cmd/link/internal/arm.s 18934 18928 -6 -0.032%
cmd/link/internal/arm64.s 31814 31801 -13 -0.041%
cmd/link/internal/riscv64.s 7347 7345 -2 -0.027%
cmd/compile/internal/ssa.s 4029173 4033066 +3893 +0.097%
total 21298280 21301472 +3192 +0.015%
Change-Id: I2e560548b515865129e1724e150e30540e9d29ce
GitHub-Last-Rev: 9a42bd29a5
GitHub-Pull-Request: golang/go#45242
Reviewed-on: https://go-review.googlesource.com/c/go/+/304869
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
This test is verifying that setting or unsetting an environment
variable in Go via the "os" package makes that change visible to the C
getenv function. The test has been failing on Windows since CL 304569;
it isn't clear to me whether it was running at all before that point.
On Windows the getenv and _putenv C functions are not thread-safe,
so Go's os.Setenv and os.Getenv use the SetEnvironmentVariable and
GetEnvironmentVariable system calls instead. That seems to work fine
in practice; however, changes via SetEnvironmentVariable are
empirically not visible to the C getenv function on certain versions
of Windows.
The MSDN getenv documentation¹ states that ‘getenv operates only on
the data structures accessible to the run-time library and not on the
environment “segment” created for the process by the operating system.
Therefore, programs that use the envp argument to main or wmain may
retrieve invalid information.’ That may be related to what we're
seeing here.
(https://github.com/curl/curl/issues/4774 describes this same behavior
observed in the curl project.)
¹https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/getenv-wgetenv?view=msvc-160#remarks
Updates #36705
Change-Id: I222792f75c650f32c5025b0fa3edab232ff66353
Reviewed-on: https://go-review.googlesource.com/c/go/+/304669
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Change-Id: I868eeb79edaba9e3afc1407ae18b89daf7e67037
Reviewed-on: https://go-review.googlesource.com/c/go/+/304570
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
This requires us to add a fake argument to issue36705.go so that the
test driver will build it with "go run" rather than "go tool compile".
Change-Id: Id08b97d898ee3e9d6c1fbb072a0a9317ed9faedd
Reviewed-on: https://go-review.googlesource.com/c/go/+/304569
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
