These temporary environment variables make it
possible to enable using SSA-generated code
for a particular function or package without
having to rebuild the compiler.
This makes it possible to start bulk testing
SSA generated code.
First, bump up the default stack size
(_StackMin in runtime/stack2.go) to something
large like 32768, because without stackmaps
we can't grow stacks.
Then run something like:
for pkg in `go list std`
do
GOGC=off GOSSAPKG=`basename $pkg` go test -a $pkg
done
When a test fails, you can re-run those tests,
selectively enabling one function after another,
until you find the one that is causing trouble.
Doing this right now yields some interesting results:
* There are several packages for which we generate
some code and whose tests pass. Yay!
* We can generate code for encoding/base64, but
tests there fail, so there's a bug to fix.
* Attempting to build the runtime yields a panic during codegen:
panic: interface conversion: ssa.Location is nil, not *ssa.LocalSlot
* The top unimplemented codegen items are (simplified):
59 genValue not implemented: REPMOVSB
18 genValue not implemented: REPSTOSQ
14 genValue not implemented: SUBQ
9 branch not implemented: If v -> b b. Control: XORQconst <bool> [1]
8 genValue not implemented: MOVQstoreidx8
4 branch not implemented: If v -> b b. Control: SETG <bool>
3 branch not implemented: If v -> b b. Control: SETLE <bool>
2 load flags not implemented: LoadReg8 <flags>
2 genValue not implemented: InvertFlags <flags>
1 store flags not implemented: StoreReg8 <flags>
1 branch not implemented: If v -> b b. Control: SETGE <bool>
Change-Id: Ib64809ac0c917e25bcae27829ae634c70d290c7f
Reviewed-on: https://go-review.googlesource.com/12547 Reviewed-by: Keith Randall <khr@golang.org>
[dev.ssa] cmd/compile: don't combine phi vars from different blocks in CSE
Here is a concrete case in which this goes wrong.
func f_ssa() int {
var n int
Next:
for j := 0; j < 3; j++ {
for i := 0; i < 10; i++ {
if i == 6 {
continue Next
}
n = i
}
n += j + j + j + j + j + j + j + j + j + j // j * 10
}
return n
}
What follows is the function printout before and after CSE.
Note blocks b8 and b10 in the before case.
b8 is the inner loop's condition: i < 10.
b10 is the inner loop's increment: i++.
v82 is i. On entry to b8, it is either 0 (v19) the first time,
or the result of incrementing v82, by way of v29.
The CSE pass considered v82 and v49 to be common subexpressions,
and eliminated v82 in favor of v49.
In the after case, v82 is now dead and will shortly be eliminated.
As a result, v29 is also dead, and we have lost the increment.
The loop runs forever.
This reduces the wall time to run test/slice3.go
on my laptop from >10m to ~20s.
This could perhaps be further reduced by using
a worklist of blocks and/or implementing the
suggestion in the comment in this CL, but at this
point, it's fast enough that there is no need.
Change-Id: I741119e0c8310051d7185459f78be8b89237b85b
Reviewed-on: https://go-review.googlesource.com/12564 Reviewed-by: Keith Randall <khr@golang.org>
[dev.ssa] cmd/compile: mark LoadReg8 and StoreReg8 of flags as unimplemented
It is not clear to me what the right implementation is.
LoadReg8 and StoreReg8 are introduced during regalloc,
so after the amd64 rewrites. But implementing them
in genValue seems silly.
Change-Id: Ia708209c4604867bddcc0e5d75ecd17cf32f52c3
Reviewed-on: https://go-review.googlesource.com/12437 Reviewed-by: Keith Randall <khr@golang.org>
Keep track of the outargs size needed at each call.
Compute the size of the outargs section of the stack frame. It's just
the max of the outargs size at all the callsites in the function.
Keith Randall [Tue, 14 Jul 2015 06:52:59 +0000 (23:52 -0700)]
[dev.ssa] cmd/compile/internal/ssa: ensure Phi ops are scheduled first
Phi ops should always be scheduled first. They have the semantics
of all happening simultaneously at the start of the block. The regalloc
phase assumes all the phis will appear first.
[dev.ssa] cmd/compile/ssa: handle nested dead blocks
removePredecessor can change which blocks are live.
However, it cannot remove dead blocks from the function's
slice of blocks because removePredecessor may have been
called from within a function doing a walk of the blocks.
CL 11879 did not handle this correctly and broke the build.
To fix this, mark the block as dead but leave its actual
removal for a deadcode pass. Blocks that are dead must have
no successors, predecessors, values, or control values,
so they will generally be ignored by other passes.
To be safe, we add a deadcode pass after the opt pass,
which is the only other pass that calls removePredecessor.
Two alternatives that I considered and discarded:
(1) Make all call sites aware of the fact that removePrecessor
might make arbitrary changes to the list of blocks. This
will needlessly complicate callers.
(2) Handle the things that can go wrong in practice when
we encounter a dead-but-not-removed block. CL 11930 takes
this approach (and the tests are stolen from that CL).
However, this is just patching over the problem.
Todd Neal [Fri, 26 Jun 2015 04:13:57 +0000 (23:13 -0500)]
[dev.ssa] cmd/compile/ssa: dominator tests and benchmarks
This change has some tests verifying functionality and an assortment of
benchmarks of various block lists. It modifies NewBlock to allocate in
contiguous blocks improving the performance of intersect() for extremely
large graphs by 30-40%.
The removal of if false { ... } blocks in the opt
pass exposed that removePredecessor needed
to do more cleaning, on pain of failing later
consistency checks.
Change-Id: I45d4ff7e1f7f1486fdd99f867867ce6ea006a288
Reviewed-on: https://go-review.googlesource.com/11879 Reviewed-by: Keith Randall <khr@golang.org>
Alan Donovan [Tue, 30 Jun 2015 19:07:20 +0000 (15:07 -0400)]
go/types: change {Type,Object,Selection}String to accept a Qualifier function
The optional Qualifier function determines what prefix to attach to
package-level names, enabling clients to qualify packages in different
ways, for example, using only the package name instead of its complete
path, or using the locally appropriate name for package given a set of
(possibly renaming) imports.
Prior to this change, clients wanting this behavior had to copy
hundreds of lines of complex printing logic.
Fun fact: (*types.Package).Path and (*types.Package).Name are valid
Qualifier functions.
We provide the RelativeTo helper function to create Qualifiers so that
the old behavior remains a one-liner.
Fixes golang/go#11133
This CL is a copy of https://go-review.googlesource.com/#/c/11692/
to the golang.org/x/tools repository.
Change-Id: I26d0f3644d077a26bfe350989f9c545f018eefbf
Reviewed-on: https://go-review.googlesource.com/11790 Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Russ Cox [Mon, 29 Jun 2015 17:50:30 +0000 (13:50 -0400)]
cmd/compile: allow linker to drop string headers when not needed
Compiling a simple file containing a slice of 100,000 strings,
the size of the resulting binary dropped from 5,896,224 bytes
to 3,495,968 bytes, which is the expected 2,400,000 bytes,
give or take.
Brad Fitzpatrick [Tue, 30 Jun 2015 16:22:41 +0000 (09:22 -0700)]
net/textproto: don't treat spaces as hyphens in header keys
This was originally done in https://codereview.appspot.com/5690059
(Feb 2012) to deal with bad response headers coming back from webcams,
but it presents a potential security problem with HTTP request
smuggling for request headers containing "Content Length" instead of
"Content-Length".
Part of overall HTTP hardening for request smuggling. See RFC 7230.
Dmitry Vyukov [Tue, 30 Jun 2015 12:09:41 +0000 (14:09 +0200)]
cmd/trace: sort procs
If you have more than 10 procs, then currently they are sorted alphabetically as
0, 10, 11, ..., 19, 2, 20, ...
Assign explicit order to procs so that they are sorted numerically.
Russ Cox [Tue, 30 Jun 2015 15:28:29 +0000 (11:28 -0400)]
net/url: only record RawPath when it is needed
RawPath is a hint to the desired encoding of Path.
It is ignored when it is not a valid encoding of Path,
such as when Path has been changed but RawPath has not.
It is not ignored but also not useful when it matches
the url package's natural choice of encoding.
In this latter case, set it to the empty string.
This should help drive home the point that clients
cannot in general depend on it being present and
that they should use the EncodedPath method instead.
This also reduces the impact of the change on tests,
especially tests that use reflect.DeepEqual on parsed URLs.
Roger Peppe [Mon, 29 Jun 2015 11:36:48 +0000 (12:36 +0100)]
encoding/xml: fix xmlns= behavior
When an xmlns="..." attribute was explicitly generated,
it was being ignored because the name space on the
attribute was assumed to have been explicitly set (to the empty
name space) and it's not possible to have an element in the
empty name space when there is a non-empty name space set.
We fix this by recording when a default name space has been
explicitly set and setting the name space of the element to that
so printer.defineNS can do its work correctly.
We do not attempt to add our own xmlns="..." attribute
when one is explicitly set.
We also add tests for EncodeElement, as that's the only way
to attain coverage of some of the changed behaviour.
Some other test coverage is also increased, although
more work remains to be done in this area.
This change was jointly developed with Martin Hilton (mhilton on github).
Aaron Jacobs [Mon, 29 Jun 2015 00:07:31 +0000 (10:07 +1000)]
net/http: add a Request.Cancel channel.
This allows for "race free" cancellation, in the sense discussed in
issue #11013: in contrast to Transport.CancelRequest, the cancellation
will not be lost if the user cancels before the request is put into the
transport's internal map.
Russ Cox [Mon, 29 Jun 2015 15:53:51 +0000 (11:53 -0400)]
path/filepath: document and test behavior of SkipDir on files
This behavior is not what we might have designed from the start,
but it has been present since Go 1. Rather than make a visible
behavioral change that might cause programs to work differently
in Go ≤1.4 vs Go ≥1.5, document what SkipDir on a non-directory
has always meant. If code doesn't want this meaning, it is easy
enough not to return SkipDir on non-directories.
Russ Cox [Mon, 29 Jun 2015 17:12:10 +0000 (13:12 -0400)]
cmd/link: fix -s with external linking
This code used to only be run for ELF, with the predictable
result that using -s with external linking broke on Windows and OS X.
Moving it here should fix Windows and does fix OS X.
CL 10835 also claims to fix the crash on Windows.
I don't know whether it does so correctly, but regardless,
this CL should make that one a no-op.
Fixes #10254.
Change-Id: I2e7b45ab0c28568ddbb1b50581dcc157ae0e7ffe
Reviewed-on: https://go-review.googlesource.com/11695 Reviewed-by: David Crawshaw <crawshaw@golang.org>
Adam Langley [Sat, 27 Jun 2015 21:50:39 +0000 (14:50 -0700)]
crypto/rsa: check for primes ≤ 1 in Validate
Change 7c7126cfeb82894229b9c3d5109e4b04e6cfde0c removed the primality
checking in Validate to save CPU time. That check happened to be
filtering out private keys with primes that were zero or one. Without
that filtering, such primes cause a panic when trying to use such a
private key.
This change specifically checks for and rejects primes ≤ 1 in Validate.
Russ Cox [Mon, 29 Jun 2015 02:26:35 +0000 (22:26 -0400)]
cmd/link: record missing pcdata tables correctly
The old code was recording the current table output offset,
so the table from the next function would be used instead of
the runtime realizing that there was no table at all.
Add debug constant in runtime to check this for every function
at startup. It's too expensive to do that by default, but we can
do the last five functions. The end of the table is usually where
the C symbols end up, so that's where the problems typically are.
Austin Clements [Fri, 26 Jun 2015 17:56:58 +0000 (13:56 -0400)]
runtime: reset mark state before checkmark and gctrace=2 mark
Currently we fail to reset the live heap accounting state before the
checkmark mark and before the gctrace=2 extra mark. As a result, if
either are enabled, at the end of GC it thinks there are 0 bytes of
live heap, which causes the GC controller to initiate a new GC
immediately, regardless of the true heap size.
Fix this by factoring this state reset into a function and calling it
before all three possible marks.
This function should be merged with gcResetGState, but doing so
requires some additional cleanup, so it will wait for after the
freeze. Filed #11427 for this cleanup.
Austin Clements [Mon, 22 Jun 2015 14:24:50 +0000 (10:24 -0400)]
runtime: don't free stack spans during GC
Memory for stacks is manually managed by the runtime and, currently
(with one exception) we free stack spans immediately when the last
stack on a span is freed. However, the garbage collector assumes that
spans can never transition from non-free to free during scan or mark.
This disagreement makes it possible for the garbage collector to mark
uninitialized objects and is blocking us from re-enabling the bad
pointer test in the garbage collector (issue #9880).
For example, the following sequence will result in marking an
uninitialized object:
1. scanobject loads a pointer slot out of the object it's scanning.
This happens to be one of the special pointers from the heap into a
stack. Call the pointer p and suppose it points into X's stack.
2. X, running on another thread, grows its stack and frees its old
stack.
3. The old stack happens to be large or was the last stack in its
span, so X frees this span, setting it to state _MSpanFree.
4. The span gets reused as a heap span.
5. scanobject calls heapBitsForObject, which loads the span containing
p, which is now in state _MSpanInUse, but doesn't necessarily have
an object at p. The not-object at p gets marked, and at this point
all sorts of things can go wrong.
We already have a partial solution to this. When shrinking a stack, we
put the old stack on a queue to be freed at the end of garbage
collection. This was done to address exactly this problem, but wasn't
a complete solution.
This commit generalizes this solution to both shrinking and growing
stacks. For stacks that fit in the stack pool, we simply don't free
the span, even if its reference count reaches zero. It's fine to reuse
the span for other stacks, and this enables that. At the end of GC, we
sweep for cached stack spans with a zero reference count and free
them. For larger stacks, we simply queue the stack span to be freed at
the end of GC. Ideally, we would reuse these large stack spans the way
we can small stack spans, but that's a more invasive change that will
have to wait until after the freeze.
Austin Clements [Thu, 25 Jun 2015 16:24:44 +0000 (12:24 -0400)]
runtime: remove unused _GCsweep state
We don't use this state. _GCoff means we're sweeping in the
background. This makes it clear in the next commit that _GCoff and
only _GCoff means sweeping.
Russ Cox [Mon, 29 Jun 2015 03:04:05 +0000 (23:04 -0400)]
cmd/cgo: fix a problem with 'go build -compiler gccgo'
Port of https://golang.org/cl/154360045 to Git.
Original author is Xia Bin <snyh@snyh.org> (already a contributor).
Fixes #8945.
Change-Id: I28bcaf3348794202ca59fbc3466bd7b9670030e4
Reviewed-on: https://go-review.googlesource.com/11658 Reviewed-by: Ian Lance Taylor <iant@golang.org>