In CL 326211 a change was made to switch "go.map.zero" symbols from
non-pkg DUPOK symbols to hashed symbols. The intent of this change was
ensure that in cases where there are multiple competing go.map.zero
symbols feeding into a link, the largest map.zero symbol is selected.
The change was buggy, however, and resulted in duplicate symbols in
the final binary (see bug cited below for details). This duplication
was relatively benign for linux/ELF, but causes duplicate definition
errors on Windows.
This patch switches "go.map.zero" symbols back from hashed symbols to
non-pkg DUPOK symbols, and updates the relevant code in the loader to
ensure that we do the right thing when there are multiple competing
DUPOK symbols with different sizes.
Fixes#47185.
Change-Id: I8aeb910c65827f5380144d07646006ba553c9251
Reviewed-on: https://go-review.googlesource.com/c/go/+/334930
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
The compiler is relying on Sym.Def field to lookup symbol package in
DotImportRefs map. But the Sym.Def field is clear whenever the compiler
finish processing a file. If the dot import happen in file A, then the
redeclaration happen in file B, then the symbol lookup in file B will
see a nil Sym.Def, that cause the compiler crashes.
To fix this, we can interate over DotImportRefs and check for matching
symbol name and return the corresponding package. Though this operation
can be slow, but it only happens in invalid program, when printing error
message, so it's not worth to optimize it further.
Fixes#47201
Change-Id: I4ca1cb0a8e7432b19cf71434592a4cbb58d54adf
Reviewed-on: https://go-review.googlesource.com/c/go/+/334589
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
I'm not sure why blank was special-cased here before, but it's
wrong. Blank is a non-exported identifier, and writing it out without
package-qualification can result in linker symbol collisions.
Fixes#47087.
Change-Id: Ie600037c8e54e3d4fdaeec21e2ca212badbd830b
Reviewed-on: https://go-review.googlesource.com/c/go/+/333163
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
CL 255241 made error message involving variadic calls clearer. To do it,
we added a check that the type of variadic argument must be a slice.
That's why the compiler crashes for invalid variadic argument type.
Instead, we can just omit the details error message, and report not
enough arguments error, which matches the behavior of go/types and types2.
Fixes#46957
Change-Id: I638d7e8f031f0ee344d5d802104fd93a60aae00a
Reviewed-on: https://go-review.googlesource.com/c/go/+/331569
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
For typed vs un-typed operation, the compiler do the conversion
un-conditionally, so if the operation is invalid, the error report is
pointed to the conversion, instead of the invalid operation itself.
To fix this, only do the conversion when the operations are valid
for both types.
Fixes#46749
Change-Id: Ib71c7bcd3ed5454e6df55b6a8db4e0f189259ba7
Reviewed-on: https://go-review.googlesource.com/c/go/+/328050
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If any of the LHS expressions of an OAS2FUNC are not identical to the
respective function call results, escape analysis mishandles the
implicit conversion, causes memory corruption.
Instead, we should insert autotmps like we already do for f(g()) calls
and return g() statements.
Fixes#46725
Change-Id: I71a08da0bf1a03d09a023da5b6f78fb37a4a4690
Reviewed-on: https://go-review.googlesource.com/c/go/+/327651
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
CL 301650 adds conversion from slice to array ptr. The conversion
expression may appear as argument to a function call, so it will be
tested by mayCall. But ir.OSLICE2ARRPTR op is not handled by mayCall,
causes the compiler crashes.
Updates #395Fixes#46720
Change-Id: I39e1b3e38e224a31f3dec46dbbdc855ff3b2c6a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/327649
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
The compiler machinery that generates "map.zero" symbols marks them as
RODATA and DUPOK, which is problematic when a given application has
multiple map zero symbols (from different packages) with varying
sizes: the dupok path in the loader assumes that if two symbols have
the same name, it is safe to pick any of the versions. In the case of
map.zero, the link needs to select the largest symbol, not an
arbitrary sym.
To fix this problem, mark map.zero symbols as content-addressable,
since the loader's content addressability processing path already
supports selection of the larger symbol in cases where there are dups.
Fixes#46653.
Change-Id: Iabd2feef01d448670ba795c7eaddc48c191ea276
Reviewed-on: https://go-review.googlesource.com/c/go/+/326211
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
For interface types, t.Methods contains only unexpanded method set, i.e
exclusive of interface embedding. Thus, we can't use it to detect an
interface contains embedding empty interface, like in:
type EI interface{}
func f() interface{ EI } {
return nil
}
At the time we generate runtime types, we want to check against the full
method set of interface instead.
Fixes#46386
Change-Id: Idff53ad39276be6632eb5932b76e855c15cbdd2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/323649
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This CL updates cmd/compile (including types2) and go/types to report
errors about using unsafe.Add and unsafe.Slice when language
compatibility is set to Go 1.16 or older.
Fixes#46525.
Change-Id: I1bfe025a672d9f4b929f443064ad1effd38d0363
Reviewed-on: https://go-review.googlesource.com/c/go/+/324369
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Matthew Dempsky <mdempsky@google.com>
This is a revert of https://go-review.googlesource.com/c/go/+/316890,
which has positive effects on debugging + DWARF variable locations
for register parameters when the reg abi is in effect, but also
turns out to interact badly with the register allocator.
Fixes#46304.
Change-Id: I624bd980493411a9cde45d44fcd3c46cad796909
Reviewed-on: https://go-review.googlesource.com/c/go/+/321830
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Update #36739
Change-Id: I14ab2cd0e29966b9a2f992e8c3bcb415203e63e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/321449
Trust: Joel Sing <joel@sing.id.au>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
issue46234.go expects an error output "segmentation violation",
which is UNIX-specific. Check for "nil pointer dereference"
instead, which is emitted by the Go runtime and should work on all
platforms.
Should fix Windows builders.
Change-Id: I3f5a66a687d43cae5eaf6a9e942b877e5a248900
Reviewed-on: https://go-review.googlesource.com/c/go/+/321072
Trust: Cherry Mui <cherryyz@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
When inlining functions with closures, ensure that we don't mark the
body of the closure with a src.Pos marker that reflects the inline,
since this will result in the generation of an inltree table for the
closure itself (as opposed to the routine that the func-with-closure
was inlined into).
Fixes#46234.
Change-Id: I348296de6504fc4745d99adab436640f50be299a
Reviewed-on: https://go-review.googlesource.com/c/go/+/320913
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Cherry Mui <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Than McIntosh <thanm@google.com>
The inlsubst already does the same thing for OLABEL, so we must do the
same thing for OGOTO. Otherwise, new inlined OGOTO node will be
associated with non-existed label.
Fixes#45947
Change-Id: I40eef095f57fd3438c38a0b5d9751d5d7ebf759e
Reviewed-on: https://go-review.googlesource.com/c/go/+/316931
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
The code that created DWARF debug var locations for input parameters
in the non-optimized case for regabi was not doing the right thing for
degenerate functions with infinite loops. Detect these cases and don't
try to emit the normal location data.
Fixes#45948.
Change-Id: I2717fc4bac2e03d5d850a6ec8a09ed05fed0c896
Reviewed-on: https://go-review.googlesource.com/c/go/+/316752
Trust: Than McIntosh <thanm@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Assignment between input parameters causes them to have more than
one "Name", and running this backwards from names to values can end
up confusing (conflating) parameter spill slots.
Around 105a6e9518, this cases a stack overflow running
go test -race encoding/pem
because two slice parameters spill (incorrectly) into the same
stack slots (in the AB?I-defined parameter spill area).
This also tickles a failure in cue, which turned out to be
easier to isolate.
Fixes#45851.
Updates #40724.
Change-Id: I39c56815bd6abb652f1ccbe83c47f4f373a125c3
Reviewed-on: https://go-review.googlesource.com/c/go/+/313212
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>
Currently, when "..." argument is passed to non-variadic function, the
compiler may skip that check, but continue checking whether the number
of arguments matches the function signature.
That causes the sanity check which was added in CL 255241 trigger.
Instead, we should report an invalid use of "...", which matches the
behavior of new type checker and go/types.
Fixes#45913
Change-Id: Icbb254052cbcd756bbd41f966c2c8e316c44420f
Reviewed-on: https://go-review.googlesource.com/c/go/+/315796
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
In CL 255899, we added code to make clearer error when non-bool used
as operand to logical operators. The code is safe, because node type
is guaranteed to be non-nil.
In CL 279442, we refactored typechecking arith, including moving
typechecking logical operators to separate case. Now we have to
explicitly check if operand type is not nil, because calling Expr can
set operand type nil for non-bool operands.
Fixes#45804
Change-Id: Ie2b6e18f65c0614a803b343f60e78ee1d660bbeb
Reviewed-on: https://go-review.googlesource.com/c/go/+/314209
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
It's not a predeclared type, but a type defined in "unsafe" package.
Fixes#44830
Change-Id: If39815b1070059b608be8231dfac9b7f3307cb15
Reviewed-on: https://go-review.googlesource.com/c/go/+/313349
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Within clovar, n.Defn can also be *ir.TypeSwitchGuard. The proper fix
here would be to populate m.Defn and have it filled in too, but we
already leave it nil in inlvar. So for consistency, this CL does the
same in clovar too.
Eventually inl.go should be rewritten to fully respect IR invariants.
Fixes#45743.
Change-Id: I8b38e5d8b2329ad242de97670f2141f713954d28
Reviewed-on: https://go-review.googlesource.com/c/go/+/313289
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Trust: Dan Scales <danscales@google.com>
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
The previous fix to ensure early evaluation of lvalue-init statements
(CL 312632) added it after we'd already peeled away any array-OINDEX
expressions. But those might have init statements too, so we need to
do this earlier actually and perhaps more than once.
Longer term, lvalue expressions shouldn't have init statements anyway.
But rsc and I both spent a while looking into this earlier in the dev
cycle and couldn't come up with anything reasonable.
Fixes#45706.
Change-Id: I2d19c5ba421b3f019c62eec45774c84cf04b30ef
Reviewed-on: https://go-review.googlesource.com/c/go/+/313011
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
CL 281152 improved ascompatee by removing the call to safeExpr on lhs.
But we forgot that lhs int statements, if any, must be walked prior
saving subexpressions, which cause the bug in #45706.
Fixes#45706
Change-Id: I0064315056ef4ca92ebf3c332c2e3a9bb2b26f68
Reviewed-on: https://go-review.googlesource.com/c/go/+/312632
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Currently, when copying definition node of an inlined var, we do not
update var Defn field to point to new copied node. That causes all
inlined vars point to the same Defn, and ir.StaticValue can not find
inlined var in the lhs of its definition.
clovar creates new ONAME node for local variables or params of closure
inside inlined function, by copying most of the old node fields. So the
new Node.Defn is not modified, its lhs still refer to old node
instead of new one.
To fix this, we need to do two things:
- In subst.clovar, set a dummy Defn node for inlvar
- During subst.node, when seeing OAS/OAS2 nodes, after substituting, we
check if any node in lhs has the dummy Defn, then set it to the current
OAS/OAS2 node.
Fixes#45606
Change-Id: Ib517b753a7643756dcd61d36deae60f1a0fc53c5
Reviewed-on: https://go-review.googlesource.com/c/go/+/312630
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
If the address of an auto is used in a Call, we need to keep it,
as we keep the Call itself.
Fixes#45693.
Change-Id: Ie548d6dffc95bf916868a8885d4ab4cf9e86355a
Reviewed-on: https://go-review.googlesource.com/c/go/+/312670
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>
Now that we can set experiments at build time instead of make.bash time,
we can actually write a test for field tracking!
Update #20014
This CL contains a test for the functionality fixed in CL 312069.
Change-Id: I7569a7057bbc7c88ae25ae7bf974b0c8a4e35be8
Reviewed-on: https://go-review.googlesource.com/c/go/+/312217
Trust: Keith Randall <khr@golang.org>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@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>
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>
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>
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>
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>
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>
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>
We need to be careful that when doing value graph surgery, we not
re-substitute a value that has already been substituted. That can lead
to confusing a previous iteration's value with the current iteration's
value.
The simple fix in this CL just aborts the optimization if it detects
intertwined phis (a phi which is the argument to another phi). It
might be possible to keep the optimization with a more complicated
CL, but:
1) This CL is clearly safe to backport.
2) There were no instances of this abort triggering in
all.bash, prior to the test introduced in this CL.
Fixes#45175
Change-Id: I2411dca03948653c053291f6829a76bec0c32330
Reviewed-on: https://go-review.googlesource.com/c/go/+/304251
Trust: Keith Randall <khr@golang.org>
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Change-Id: Icfa204761045b72a8ea173fd55eddf1f0e58d819
Reviewed-on: https://go-review.googlesource.com/c/go/+/304253
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Temporarily disable a questionable test case in fixedbugs/bug193.go
and enable the test as a whole. See the issues below for details.
Updates #45114.
Updates #45117.
Change-Id: I1de6f8d79b592eeeec139cd92b6c9cac56a9a74b
Reviewed-on: https://go-review.googlesource.com/c/go/+/303094
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Emmanuel Odeke <emmanuel@orijtech.com>
Based on https://golang.org/cl/284256 for go/types.
Brings this code more in line with go/types.
Adjusted various tests to match new error messages which
generally are now better: for assignment errors, instead
of a generic "cannot convert" we now say "cannot use"
followed by a clearer reason as to why not.
Major differences to go/types with respect to the changed
files:
- Some of the new code now returns error codes, but they
are only used internally for now, and not reported with
errors.
- go/types does not "convert" untyped nil values to target
types, but here we do. This is unchanged from how types2
handled this before this CL.
Change-Id: If45336d7ee679ece100f6d9d9f291a6ea55004d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/302757
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Now that we can set GOEXPERIMENT at build time, we no longer need
-d=fieldtrack in the compiler to enabled field tracking at build time.
Switch the one test that uses -d=fieldtrack to use GOEXPERIMENT
instead so we can eliminate this debug flag and centralize on
GOEXPERIMENT.
Updates #42681.
Change-Id: I14c352c9a97187b9c5ec8027ff672d685f22f543
Reviewed-on: https://go-review.googlesource.com/c/go/+/302969
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Change-Id: Ie52d70d2ae8a21acacf0745a4093650b03ac43f9
Reviewed-on: https://go-review.googlesource.com/c/go/+/302371
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
For #44383
Change-Id: I3610105dad3574e210e226d3ba80a4ba5a7eeaa6
Reviewed-on: https://go-review.googlesource.com/c/go/+/300789
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
In the ppc64 ISA DS form loads and stores are restricted to offset
fields that are a multiple of 4. This is currently handled with checks
in the rules that generate MOVDload, MOVWload, MOVDstore and
MOVDstorezero to prevent invalid instructions from getting to the
assembler.
An unhandled case was discovered which led to the search for a better
solution to this problem. Now, instead of checking the offset in the
rules, this will be detected when processing these Ops in
ssaGenValues in ppc64/ssa.go. If the offset is not valid, the address
of the symbol to be loaded or stored will be computed using the base
register + offset, and that value used in the new base register.
With the full address in the base register, the offset field can be
zero in the instruction.
Updates #44739
Change-Id: I4f3c0c469ae70a63e3add295c9b55ea0e30ef9b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/299789
Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
