- also consistently use %v instead of %s when we have a (gc) Formatter
- rewrite done automatically using Formats test in -u (update) mode
- manual update of format strings that were not single string constants
- updated fmt.go, fmt_test.go accordingly
- fmt_test: permit "%T" always
Change-Id: I8f0704286aba5704600ad0c4a4484005b79b905d
Reviewed-on: https://go-review.googlesource.com/28954
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Fixes#16911.
Fix obsolete inferno-os links, since code.google.com shutdown.
This CL points to the right files by replacing
http://code.google.com/p/inferno-os/source/browse
with
https://bitbucket.org/inferno-os/inferno-os/src/default
To implement the change I wrote and ran this script in the root:
$ grep -Rn 'http://code.google.com/p/inferno-os/source/browse' * \
| cut -d":" -f1 | while read F;do perl -pi -e \
's/http:\/\/code.google.com\/p\/inferno-os\/source\/browse/https:\/\/bitbucket.org\/inferno-os\/inferno-os\/src\/default/g'
$F;done
I excluded any cmd/vendor changes from the commit.
Change-Id: Iaaf828ac8f6fc949019fd01832989d00b29b6749
Reviewed-on: https://go-review.googlesource.com/27994
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
When T is a scalar, there are no runtime calls
required, which makes this a clear win.
encoding/binary:
WriteInts-8 958ns ± 3% 864ns ± 2% -9.80% (p=0.000 n=15+15)
This also considerably shrinks a core fmt
routine:
Before: "".(*pp).printArg t=1 size=3952 args=0x20 locals=0xf0
After: "".(*pp).printArg t=1 size=2624 args=0x20 locals=0x98
Unfortunately, I find it very hard to get stable
numbers out of the fmt benchmarks due to thermal scaling.
Change-Id: I1278006b030253bf8e48dc7631d18985cdaa143d
Reviewed-on: https://go-review.googlesource.com/26659
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Keith Randall <khr@golang.org>
The main check here is that liveness now crashes if it finds an instruction
using a variable that should be tracked but is not.
Comments and adjustments in nodarg to explain what's going on and
to remove the "-1" argument added a few months ago, plus a sketch
of a future simplification.
The need for n.Orig in the earlier CL seems to have been an intermediate
problem rather than fundamental: the new explanations in nodarg make
clear that nodarg is not causing the problem I thought, and in fact now
using n instead of n.Orig works fine in plive.go.
Change-Id: I3f5cf9f6e4438a6d27abac7d490e7521545cd552
Reviewed-on: https://go-review.googlesource.com/23450
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
The liveness computation of parameters generally was never
correct, but forcing all parameters to be live throughout the
function covered up that problem. The new SSA back end is
too clever: even though it currently keeps the parameter values live
throughout the function, it may find optimizations that mean
the current values are not written back to the original parameter
stack slots immediately or ever (for example if a parameter is set
to nil, SSA constant propagation may replace all later uses of the
parameter with a constant nil, eliminating the need to write the nil
value back to the stack slot), so the liveness code must now
track the actual operations on the stack slots, exposing these
problems.
One small problem in the handling of arguments is that nodarg
can return ONAME PPARAM nodes with adjusted offsets, so that
there are actually multiple *Node pointers for the same parameter
in the instruction stream. This might be possible to correct, but
not in this CL. For now, we fix this by using n.Orig instead of n
when considering PPARAM and PPARAMOUT nodes.
The major problem in the handling of arguments is general
confusion in the liveness code about the meaning of PPARAM|PHEAP
and PPARAMOUT|PHEAP nodes, especially as contrasted with PAUTO|PHEAP.
The difference between these two is that when a local variable "moves"
to the heap, it's really just allocated there to start with; in contrast,
when an argument moves to the heap, the actual data has to be copied
there from the stack at the beginning of the function, and when a
result "moves" to the heap the value in the heap has to be copied
back to the stack when the function returns
This general confusion is also present in the SSA back end.
The PHEAP bit worked decently when I first introduced it 7 years ago (!)
in 391425ae. The back end did nothing sophisticated, and in particular
there was no analysis at all: no escape analysis, no liveness analysis,
and certainly no SSA back end. But the complications caused in the
various downstream consumers suggest that this should be a detail
kept mainly in the front end.
This CL therefore eliminates both the PHEAP bit and even the idea of
"heap variables" from the back ends.
First, it replaces the PPARAM|PHEAP, PPARAMOUT|PHEAP, and PAUTO|PHEAP
variable classes with the single PAUTOHEAP, a pseudo-class indicating
a variable maintained on the heap and available by indirecting a
local variable kept on the stack (a plain PAUTO).
Second, walkexpr replaces all references to PAUTOHEAP variables
with indirections of the corresponding PAUTO variable.
The back ends and the liveness code now just see plain indirected
variables. This may actually produce better code, but the real goal
here is to eliminate these little-used and somewhat suspect code
paths in the back end analyses.
The OPARAM node type goes away too.
A followup CL will do the same to PPARAMREF. I'm not sure that
the back ends (SSA in particular) are handling those right either,
and with the framework established in this CL that change is trivial
and the result clearly more correct.
Fixes#15747.
Change-Id: I2770b1ce3cbc93981bfc7166be66a9da12013d74
Reviewed-on: https://go-review.googlesource.com/23393
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Updates #15462
Automatic refactor with sed -e.
Replace all oconv(op, 0) to string conversion with the raw op value
which fmt's %v verb can print directly.
The remaining oconv(op, FmtSharp) will be replaced with op.GoString and
%#v in the next CL.
Change-Id: I5e2f7ee0bd35caa65c6dd6cb1a866b5e4519e641
Reviewed-on: https://go-review.googlesource.com/22499
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Updates #15462
Semi automatic change with gofmt -r and hand fixups for callers outside
internal/gc.
All the uses of gc.Oconv outside cmd/compile/internal/gc were for the
Oconv(op, 0) form, which is already handled the Op.String method.
Replace the use of gc.Oconv(op, 0) with op itself, which will call
Op.String via the %v or %s verb. Unexport Oconv.
Change-Id: I84da2a2e4381b35f52efce427b2d6a3bccdf2526
Reviewed-on: https://go-review.googlesource.com/22496
Run-TryBot: Dave Cheney <dave@cheney.net>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Instead of switching on Ctype (which internally uses a type switch)
and then scattering lots of type assertions throughout the CTFOO case
clauses, just use type switches directly on the underlying constant
value.
Passes toolstash/buildall.
Change-Id: I9bc172cc67e5f391cddc15539907883b4010689e
Reviewed-on: https://go-review.googlesource.com/22384
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Instead of using TARRAY for both arrays and slices, create a new
TSLICE kind to handle slices.
Also, get rid of the "DDDArray" distinction. While kinda ugly, it
seems likely we'll need to defer evaluating the constant bounds
expressions for golang.org/issue/13890.
Passes toolstash/buildall.
Change-Id: I8e45d4900e7df3a04cce59428ec8b38035d3cc3a
Reviewed-on: https://go-review.googlesource.com/22329
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Allows instructions with a From3 field to be used in regopt so
long as From3 represents a constant. This is needed because the
storage-to-storage instructions on s390x place the length of the
data into From3.
Change-Id: I12cd32d4f997baf2fe97937bb7d45bbf716dfcb5
Reviewed-on: https://go-review.googlesource.com/20875
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Information about CPU architectures (e.g., name, family, byte
ordering, pointer and register size) is currently redundantly
scattered around the source tree. Instead consolidate the basic
information into a single new package cmd/internal/sys.
Also, introduce new sys.I386, sys.AMD64, etc. names for the constants
'8', '6', etc. and replace most uses of the latter. The notable
exceptions are a couple of error messages that still refer to the old
char-based toolchain names and function reltype in cmd/link.
Passes toolstash/buildall.
Change-Id: I8a6f0cbd49577ec1672a98addebc45f767e36461
Reviewed-on: https://go-review.googlesource.com/21623
Reviewed-by: Michael Hudson-Doyle <michael.hudson@canonical.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Changes generated with eg and then manually
checked and in some cases simplified.
Passes toolstash -cmp.
Change-Id: I2119f37f003368ce1884d2863b406d6ffbfe38c7
Reviewed-on: https://go-review.googlesource.com/21563
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
This is a change improving consistency in the source tree.
The pattern foo &= ^bar, was only used six times in src/ directory.
The usage of the supported &^ (bit clear / AND NOT) operator is way more
common, about factor 10x.
Change-Id: If26a2994fd81d23d42189bee00245eb84e672cf3
Reviewed-on: https://go-review.googlesource.com/21224
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also give them more idiomatic Go names. Adding godocs is outside the
scope of this CL. (Besides, the method names almost all directly
parallel an underlying math/big.Int or math/big.Float method.)
CL prepared mechanically with sed (for rewriting mpint.go/mpfloat.go)
and gofmt (for rewriting call sites).
Passes toolstash -cmp.
Change-Id: Id76f4aee476ba740f48db33162463e7978c2083d
Reviewed-on: https://go-review.googlesource.com/20909
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The Node type ODOT and its variants all represent a selector, with a
simple name to the right of the dot. Before this change this was
represented by using an ONAME Node in the Right field. This ONAME node
served no useful purpose. This CL changes these Node types to store the
symbol in the Sym field instead, thus not requiring allocating a Node
for each selector.
When compiling x/tools/go/types this CL eliminates nearly 5000 calls to
newname and reduces the total number of Nodes allocated by about 6.6%.
It seems to cut compilation time by 1 to 2 percent.
Getting this right was somewhat subtle, and I added two dubious changes
to produce the exact same output as before. One is to ishairy in
inl.go: the ONAME node increased the cost of ODOT and friends by 1, and
I retained that, although really ODOT is not more expensive than any
other node. The other is to varexpr in walk.go: because the ONAME in
the Right field of an ODOT has no class, varexpr would always return
false for an ODOT, although in fact for some ODOT's it seemingly ought
to return true; I added an && false for now. I will send separate CLs,
that will break toolstash -cmp, to clean these up.
This CL passes toolstash -cmp.
Change-Id: I4af8a10cc59078c436130ce472f25abc3a9b2f80
Reviewed-on: https://go-review.googlesource.com/20890
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
For every string constant the compiler was creating 2 Sym's and 2
Node's. It would never refer to them again, but would keep them alive
in gostringpkg. This changes the code to just use obj.LSym's instead.
When compiling x/tools/go/types, this yields about a 15% reduction in
the number of calls to newname and a 3% reduction in the total number of
Node objects. Unfortunately I couldn't see any change in compile time,
but reducing memory usage is desirable anyhow.
Passes toolstash -cmp.
Change-Id: I24f1cb1e6cff0a3afba4ca66f7166874917a036b
Reviewed-on: https://go-review.googlesource.com/20792
Reviewed-by: David Crawshaw <crawshaw@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The obj.Fmt* values are only used by gc/fmt.go, so just move them
there. Also, add comments documenting the correspondance between
FmtFoo names and their flag characters to make understanding the
existing documentation slightly less confusing.
While here, add a new FmtFlag named type to represent these values.
Change-Id: I9631214b892557d094823f1ac575d0c43a84007b
Reviewed-on: https://go-review.googlesource.com/20717
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Switch TSTRUCT and TINTER to use Fields instead of Type, which wrings
out the remaining few direct uses of the latter.
Preparation for converting fields to use a separate "Field" type.
Passes toolstash/buildall.
Change-Id: I5a2ea7e159d0dde1be2c9afafc10a8f739d95743
Reviewed-on: https://go-review.googlesource.com/20675
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>
The only remaining place that generated ADATA
Prog was the assembler. Stop, and delete some
now-dead code.
Passes toolstash -cmp.
Change-Id: I26578ff1b4868e98562b44f69d909c083e96f8d5
Reviewed-on: https://go-review.googlesource.com/20646
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Accessing the n'th field of a struct is fairly common, and in
particular accessing the 0'th field of the receiver parameter list is
very common. Add helper methods for both of these tasks and update
code to make use of them.
Change-Id: I81f551fecdca306b3800636caebcd0dc106f2ed7
Reviewed-on: https://go-review.googlesource.com/20498
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Dave Cheney <dave@cheney.net>
Eliminates type conversions in a bunch of Oconv(int(n.Op), ...) calls.
Notably, this identified a misuse of Oconv in amd64/gsubr.go to try to
print an assembly instruction op instead of a compiler node op.
Change-Id: I93b5aa49fe14a5eaf868b05426d3b8cd8ab52bc5
Reviewed-on: https://go-review.googlesource.com/20298
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
A slice uses less memory than a NodeList, and has better memory locality
when walking the list.
This uncovered a tricky case involving closures: the escape analysis
pass when run on a closure was appending to the Dcl list of the OCLOSURE
rather than the ODCLFUNC. This happened to work because they shared the
same NodeList. Fixed with a change to addrescapes, and a check to
Tempname to catch any recurrences.
This removes the last use of the listsort function outside of tests.
I'll send a separate CL to remove it.
Unfortunately, while this passes all tests, it does not pass toolstash
-cmp. The problem is that cmpstackvarlt does not fully determine the
sort order, and the change from listsort to sort.Sort, while generally
desirable, produces a different ordering. I could stage this by first
making cmpstackvarlt fully determined, but no matter what toolstash -cmp
is going to break at some point.
In my casual testing the compiler is 2.2% faster.
Update #14473.
Change-Id: I367d66daa4ec73ed95c14c66ccda3a2133ad95d5
Reviewed-on: https://go-review.googlesource.com/19919
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Don't accumulate a massive list of Prog structs during
compilation and write them all out at the end of compilation.
Instead, convert them to code+relocs (or data+relocs) after each
function is compiled.
Track down a few other places that were keeping Progs alive
and nil them out so the Progs get GCd promptly.
Saves ~20% in peak memory usage for the compiler. Surprisingly not much
help speed-wise (only because we end up doing more GCs. With a
compensating GOGC=120, it does help a bit), but this provides a base for
more changes (e.g. reusing a cache of Progs).
Change-Id: I838e01017c228995a687a8110d0cd67bf8596407
Reviewed-on: https://go-review.googlesource.com/19867
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Consider this code:
func f(*int)
func g() {
p := new(int)
f(p)
}
where f is an assembly function.
In general liveness analysis assumes that during the call to f, p is dead
in this frame. If f has retained p, p will be found alive in f's frame and keep
the new(int) from being garbage collected. This is all correct and works.
We use the Go func declaration for f to give the assembly function
liveness information (the arguments are assumed live for the entire call).
Now consider this code:
func h1() {
p := new(int)
syscall.Syscall(1, 2, 3, uintptr(unsafe.Pointer(p)))
}
Here syscall.Syscall is taking the place of f, but because its arguments
are uintptr, the liveness analysis and the garbage collector ignore them.
Since p is no longer live in h once the call starts, if the garbage collector
scans the stack while the system call is blocked, it will find no reference
to the new(int) and reclaim it. If the kernel is going to write to *p once
the call finishes, reclaiming the memory is a mistake.
We can't change the arguments or the liveness information for
syscall.Syscall itself, both for compatibility and because sometimes the
arguments really are integers, and the garbage collector will get quite upset
if it finds an integer where it expects a pointer. The problem is that
these arguments are fundamentally untyped.
The solution we have taken in the syscall package's wrappers in past
releases is to insert a call to a dummy function named "use", to make
it look like the argument is live during the call to syscall.Syscall:
func h2() {
p := new(int)
syscall.Syscall(1, 2, 3, uintptr(unsafe.Pointer(p)))
use(unsafe.Pointer(p))
}
Keeping p alive during the call means that if the garbage collector
scans the stack during the system call now, it will find the reference to p.
Unfortunately, this approach is not available to users outside syscall,
because 'use' is unexported, and people also have to realize they need
to use it and do so. There is much existing code using syscall.Syscall
without a 'use'-like function. That code will fail very occasionally in
mysterious ways (see #13372).
This CL fixes all that existing code by making the compiler do the right
thing automatically, without any code modifications. That is, it takes h1
above, which is incorrect code today, and makes it correct code.
Specifically, if the compiler sees a foreign func definition (one
without a body) that has uintptr arguments, it marks those arguments
as "unsafe uintptrs". If it later sees the function being called
with uintptr(unsafe.Pointer(x)) as an argument, it arranges to mark x
as having escaped, and it makes sure to hold x in a live temporary
variable until the call returns, so that the garbage collector cannot
reclaim whatever heap memory x points to.
For now I am leaving the explicit calls to use in package syscall,
but they can be removed early in a future cycle (likely Go 1.7).
The rule has no effect on escape analysis, only on liveness analysis.
Fixes#13372.
Change-Id: I2addb83f70d08db08c64d394f9d06ff0a063c500
Reviewed-on: https://go-review.googlesource.com/18584
Reviewed-by: Ian Lance Taylor <iant@golang.org>
The change to the write barrier in https://golang.org/cl/16899 means
that the compiler now emits tests of the first field of a struct. That
was using a register that was not used before. This change fixes that
for amd64 by adding a special case for the first field of a struct.
Update #12416.
Change-Id: Ia57baa62cd741592fbeb9be82f1e846be73d6edd
Reviewed-on: https://go-review.googlesource.com/16933
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
It is based on ppc64 compiler.
Change-Id: I15a101df05f2919ba5292136957ba0009227d067
Reviewed-on: https://go-review.googlesource.com/14445
Reviewed-by: Minux Ma <minux@golang.org>
Type Op is enfored now.
Type EType will need further CLs.
Added TODOs where Node.EType is used as a union type.
The TODOs have the format `TODO(marvin): Fix Node.EType union type.`.
Furthermore:
-The flag of Econv function in fmt.go is removed, since unused.
-Some cleaning along the way, e.g. declare vars first when getting initialized.
Passes go build -toolexec 'toolstash -cmp' -a std.
Fixes#11846
Change-Id: I908b955d5a78a195604970983fb9194bd9e9260b
Reviewed-on: https://go-review.googlesource.com/14956
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Marvin Stenger <marvin.stenger94@gmail.com>
Update old c-style comments to look like Go comments. Also replace some
lingering references to old .c files that don't exist anymore.
Change-Id: I72b2407a40fc76c23e9048643e0622fd70b4cf90
Reviewed-on: https://go-review.googlesource.com/16190
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Shared libraries on ppc64le will require a larger minimum stack frame (because
the ABI mandates that the TOC pointer is available at 24(R1)). Part 2a of
preparing for that is to have all bits of arch-independent and ppc64-specific
codegen that need to know call a function to find out.
Change-Id: I55899f73037e92227813c491049a3bd6f30bd41f
Reviewed-on: https://go-review.googlesource.com/15524
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Changed tree generation to correctly use PARAMOUT instead
of PARAM.
Emit Func.Exit before any returns.
Change-Id: I2fa53cc7fad05fb4eea21081ba33d1f66db4ed49
Reviewed-on: https://go-review.googlesource.com/15610
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: David Chase <drchase@google.com>
Convert some fields of struct Type in go.go from uint8 to bool.
This change passes go build -toolexec 'toolstash -cmp' -a std.
Change-Id: I0a6c53f8ee686839b5234010ee2de7ae3940d499
Reviewed-on: https://go-review.googlesource.com/14370
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This helps vet see a real issue:
cmd/internal/gc$ go vet
gen.go:1223: unreachable code
Fixes#12106.
Change-Id: I720868b07ae6b6d5a4dc6b238baa8c9c889da6d8
Reviewed-on: https://go-review.googlesource.com/14083
Reviewed-by: Minux Ma <minux@golang.org>
Run-TryBot: Minux Ma <minux@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
The reg[] array in .../gc is where truth lies. The copy in .../ARCH
is incorrect as it is mostly not updated to reflect regalloc decisions.
This bug was introduced in the rewrite
https://go-review.googlesource.com/#/c/7853/. The new reg[] array was
introduced in .../gc but not all of the uses were removed in the
.../ARCH directories.
Fixes#12133
Change-Id: I6364fc403cdab92d802d17f2913ba1607734037c
Reviewed-on: https://go-review.googlesource.com/13630
Reviewed-by: Russ Cox <rsc@golang.org>
