1
0
mirror of https://github.com/golang/go synced 2024-11-13 19:00:25 -07:00
Commit Graph

1973 Commits

Author SHA1 Message Date
Dave Cheney
3e692becfe test/fixedbugs: fix typo in comment
Fix copy paste error pointed out by rsc, https://golang.org/cl/107290043/diff/60001/test/fixedbugs/issue8074.go#newcode7

LGTM=ruiu, r
R=golang-codereviews, ruiu, r
CC=golang-codereviews
https://golang.org/cl/106210047
2014-06-29 20:34:35 +10:00
Russ Cox
2565b5c060 cmd/gc: drop parenthesization restriction for receiver types
Matches CL 101500044.

LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/110160044
2014-06-25 09:57:48 -04:00
Dave Cheney
5b342f7804 test: add test case for issue 8074.
Fixes #8074.

The issue was not reproduceable by revision

go version devel +e0ad7e329637 Thu Jun 19 22:19:56 2014 -0700 linux/arm

But include the original test case in case the issue reopens itself.

LGTM=dvyukov
R=golang-codereviews, dvyukov
CC=golang-codereviews
https://golang.org/cl/107290043
2014-06-22 17:33:00 +10:00
Josh Bleecher Snyder
63393faedf test: speed up chan/select5
No functional changes.

Generating shorter functions improves compilation time. On my laptop, this test's running time goes from 5.5s to 1.5s; the wall clock time to run all tests goes down 1s. On Raspberry Pi, this CL cuts 50s off the wall clock time to run all tests.

Fixes #7503.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://golang.org/cl/72590045
2014-06-17 09:07:18 -07:00
Russ Cox
36207a91d3 runtime: fix defer of nil func
Fixes #8047.

LGTM=r, iant
R=golang-codereviews, r, iant
CC=dvyukov, golang-codereviews, khr
https://golang.org/cl/105140044
2014-06-12 16:34:36 -04:00
Keith Randall
aa04caa759 runtime: add test for issue 8047.
Make sure stack copier doesn't barf on a nil defer.
Bug was fixed in https://golang.org/cl/101800043
This change just adds a test.

Fixes #8047

LGTM=dvyukov, rsc
R=dvyukov, rsc
CC=golang-codereviews
https://golang.org/cl/108840043
2014-06-11 20:34:46 -04:00
Russ Cox
f20e4d5ecb cmd/gc: fix &result escaping into result
There is a hierarchy of location defined by loop depth:

        -1 = the heap
        0 = function results
        1 = local variables (and parameters)
        2 = local variable declared inside a loop
        3 = local variable declared inside a loop inside a loop
        etc

In general if an address from loopdepth n is assigned to
something in loop depth m < n, that indicates an extended
lifetime of some form that requires a heap allocation.

Function results can be local variables too, though, and so
they don't actually fit into the hierarchy very well.
Treat the address of a function result as level 1 so that
if it is written back into a result, the address is treated
as escaping.

Fixes #8185.

LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/108870044
2014-06-11 14:21:06 -04:00
Russ Cox
775ab8eeaa cmd/gc: fix escape analysis for &x inside switch x := v.(type)
The analysis for &x was using the loop depth on x set
during x's declaration. A type switch creates a list of
implicit declarations that were not getting initialized
with loop depths.

Fixes #8176.

LGTM=iant
R=iant
CC=golang-codereviews
https://golang.org/cl/108860043
2014-06-11 11:48:47 -04:00
Russ Cox
4534fdb144 runtime: fix panic stack during runtime.Goexit during panic
A runtime.Goexit during a panic-invoked deferred call
left the panic stack intact even though all the stack frames
are gone when the goroutine is torn down.
The next goroutine to reuse that struct will have a
bogus panic stack and can cause the traceback routines
to walk into garbage.

Most likely to happen during tests, because t.Fatal might
be called during a deferred func and uses runtime.Goexit.

This "not enough cleared in Goexit" failure mode has
happened to us multiple times now. Clear all the pointers
that don't make sense to keep, not just gp->panic.

Fixes #8158.

LGTM=iant, dvyukov
R=iant, dvyukov
CC=golang-codereviews
https://golang.org/cl/102220043
2014-06-06 16:52:14 -04:00
Russ Cox
ac0e12d158 cmd/6g: fix stack zeroing on native client
I am not sure what the rounding here was
trying to do, but it was skipping the first
pointer on native client.

The code above the rounding already checks
that xoffset is widthptr-aligned, so the rnd
was a no-op everywhere but on Native Client.
And on Native Client it was wrong.

Perhaps it was supposed to be rounding down,
not up, but zerorange handles the extra 32 bits
correctly, so the rnd does not seem to be necessary
at all.

This wouldn't be worth doing for Go 1.3 except
that it can affect code on the playground.

Fixes #8155.

LGTM=r, iant
R=golang-codereviews, r, iant
CC=dvyukov, golang-codereviews, khr
https://golang.org/cl/108740047
2014-06-05 16:40:23 -04:00
Russ Cox
fe3c913443 cmd/gc: fix escape analysis of func returning indirect of parameter
I introduced this bug when I changed the escape
analysis to run in phases based on call graph
dependency order, in order to be more precise about
inputs escaping back to outputs (functions returning
their arguments).

Given

        func f(z **int) *int { return *z }

we were tagging the function as 'z does not escape
and is not returned', which is all true, but not
enough information.

If used as:

        var x int
        p := &x
        q := &p
        leak(f(q))

then the compiler might try to keep x, p, and q all
on the stack, since (according to the recorded
information) nothing interesting ends up being
passed to leak.

In fact since f returns *q = p, &x is passed to leak
and x needs to be heap allocated.

To trigger the bug, you need a chain that the
compiler wants to keep on the stack (like x, p, q
above), and you need a function that returns an
indirect of its argument, and you need to pass the
head of the chain to that function. This doesn't
come up very often: this bug has been present since
June 2012 (between Go 1 and Go 1.1) and we haven't
seen it until now. It helps that most functions that
return indirects are getters that are simple enough
to be inlined, avoiding the bug.

Earlier versions of Go also had the benefit that if
&x really wasn't used beyond x's lifetime, nothing
broke if you put &x in a heap-allocated structure
accidentally. With the new stack copying, though,
heap-allocated structures containing &x are not
updated when the stack is copied and x moves,
leading to crashes in Go 1.3 that were not crashes
in Go 1.2 or Go 1.1.

The fix is in two parts.

First, in the analysis of a function, recognize when
a value obtained via indirect of a parameter ends up
being returned. Mark those parameters as having
content escape back to the return results (but we
don't bother to write down which result).

Second, when using the analysis to analyze, say,
f(q), mark parameters with content escaping as
having any indirections escape to the heap. (We
don't bother trying to match the content to the
return value.)

The fix could be less precise (simpler).
In the first part we might mark all content-escaping
parameters as plain escaping, and then the second
part could be dropped. Or we might assume that when
calling f(q) all the things pointed at by q escape
always (for any f and q).

The fix could also be more precise (more complex).
We might record the specific mapping from parameter
to result along with the number of indirects from the
parameter to the thing being returned as the result,
and then at the call sites we could set up exactly the
right graph for the called function. That would make
notleaks(f(q)) be able to keep x on the stack, because
the reuslt of f(q) isn't passed to anything that leaks it.

The less precise the fix, the more stack allocations
become heap allocations.

This fix is exactly as precise as it needs to be so that
none of the current stack allocations in the standard
library turn into heap allocations.

Fixes #8120.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, khr, r
https://golang.org/cl/102040046
2014-06-03 11:35:59 -04:00
Russ Cox
eb54079264 cmd/gc: fix liveness for address-taken variables in inlined functions
The 'address taken' bit in a function variable was not
propagating into the inlined copies, causing incorrect
liveness information.

LGTM=dsymonds, bradfitz
R=golang-codereviews, bradfitz
CC=dsymonds, golang-codereviews, iant, khr, r
https://golang.org/cl/96670046
2014-06-02 21:26:32 -04:00
Russ Cox
d646040fd1 runtime: fix 1-byte return during x.(T) for 0-byte T
The 1-byte write was silently clearing a byte on the stack.
If there was another function call with more arguments
in the same stack frame, no harm done.
Otherwise, if the variable at that location was already zero,
no harm done.
Otherwise, problems.

Fixes #8139.

LGTM=dsymonds
R=golang-codereviews, dsymonds
CC=golang-codereviews, iant, r
https://golang.org/cl/100940043
2014-06-02 21:06:30 -04:00
Russ Cox
bcfe519d58 runtime: fix correctness test at end of traceback
We were requiring that the defer stack and the panic stack
be completely processed, thinking that if any were left over
the stack scan and the defer stack/panic stack must be out
of sync. It turns out that the panic stack may well have
leftover entries in some situations, and that's okay.

Fixes #8132.

LGTM=minux, r
R=golang-codereviews, minux, r
CC=golang-codereviews, iant, khr
https://golang.org/cl/100900044
2014-06-01 13:57:46 -04:00
Russ Cox
14d2ee1d00 runtime: make continuation pc available to stack walk
The 'continuation pc' is where the frame will continue
execution, if anywhere. For a frame that stopped execution
due to a CALL instruction, the continuation pc is immediately
after the CALL. But for a frame that stopped execution due to
a fault, the continuation pc is the pc after the most recent CALL
to deferproc in that frame, or else 0. That is where execution
will continue, if anywhere.

The liveness information is only recorded for CALL instructions.
This change makes sure that we never look for liveness information
except for CALL instructions.

Using a valid PC fixes crashes when a garbage collection or
stack copying tries to process a stack frame that has faulted.

Record continuation pc in heapdump (format change).

Fixes #8048.

LGTM=iant, khr
R=khr, iant, dvyukov
CC=golang-codereviews, r
https://golang.org/cl/100870044
2014-05-31 10:10:12 -04:00
Russ Cox
1afbceb599 cmd/6g: treat vardef-initialized fat variables as live at calls
This CL forces the optimizer to preserve some memory stores
that would be redundant except that a stack scan due to garbage
collection or stack copying might look at them during a function call.
As such, it forces additional memory writes and therefore slows
down the execution of some programs, especially garbage-heavy
programs that are already limited by memory bandwidth.

The slowdown can be as much as 7% for end-to-end benchmarks.

These numbers are from running go1.test -test.benchtime=5s three times,
taking the best (lowest) ns/op for each benchmark. I am excluding
benchmarks with time/op < 10us to focus on macro effects.
All benchmarks are on amd64.

Comparing tip (a27f34c771cb) against this CL on an Intel Core i5 MacBook Pro:

benchmark                          old ns/op      new ns/op      delta
BenchmarkBinaryTree17              3876500413     3856337341     -0.52%
BenchmarkFannkuch11                2965104777     2991182127     +0.88%
BenchmarkGobDecode                 8563026        8788340        +2.63%
BenchmarkGobEncode                 5050608        5267394        +4.29%
BenchmarkGzip                      431191816      434168065      +0.69%
BenchmarkGunzip                    107873523      110563792      +2.49%
BenchmarkHTTPClientServer          85036          86131          +1.29%
BenchmarkJSONEncode                22143764       22501647       +1.62%
BenchmarkJSONDecode                79646916       85658808       +7.55%
BenchmarkMandelbrot200             4720421        4700108        -0.43%
BenchmarkGoParse                   4651575        4712247        +1.30%
BenchmarkRegexpMatchMedium_1K      71986          73490          +2.09%
BenchmarkRegexpMatchHard_1K        111018         117495         +5.83%
BenchmarkRevcomp                   648798723      659352759      +1.63%
BenchmarkTemplate                  112673009      112819078      +0.13%

Comparing tip (a27f34c771cb) against this CL on an Intel Xeon E5520:

BenchmarkBinaryTree17              5461110720     5393104469     -1.25%
BenchmarkFannkuch11                4314677151     4327177615     +0.29%
BenchmarkGobDecode                 11065853       11235272       +1.53%
BenchmarkGobEncode                 6500065        6959837        +7.07%
BenchmarkGzip                      647478596      671769097      +3.75%
BenchmarkGunzip                    139348579      141096376      +1.25%
BenchmarkHTTPClientServer          69376          73610          +6.10%
BenchmarkJSONEncode                30172320       31796106       +5.38%
BenchmarkJSONDecode                113704905      114239137      +0.47%
BenchmarkMandelbrot200             6032730        6003077        -0.49%
BenchmarkGoParse                   6775251        6405995        -5.45%
BenchmarkRegexpMatchMedium_1K      111832         113895         +1.84%
BenchmarkRegexpMatchHard_1K        161112         168420         +4.54%
BenchmarkRevcomp                   876363406      892319935      +1.82%
BenchmarkTemplate                  146273096      148998339      +1.86%

Just to get a sense of where we are compared to the previous release,
here are the same benchmarks comparing Go 1.2 to this CL.

Comparing Go 1.2 against this CL on an Intel Core i5 MacBook Pro:

BenchmarkBinaryTree17              4370077662     3856337341     -11.76%
BenchmarkFannkuch11                3347052657     2991182127     -10.63%
BenchmarkGobDecode                 8791384        8788340        -0.03%
BenchmarkGobEncode                 4968759        5267394        +6.01%
BenchmarkGzip                      437815669      434168065      -0.83%
BenchmarkGunzip                    94604099       110563792      +16.87%
BenchmarkHTTPClientServer          87798          86131          -1.90%
BenchmarkJSONEncode                22818243       22501647       -1.39%
BenchmarkJSONDecode                97182444       85658808       -11.86%
BenchmarkMandelbrot200             4733516        4700108        -0.71%
BenchmarkGoParse                   5054384        4712247        -6.77%
BenchmarkRegexpMatchMedium_1K      67612          73490          +8.69%
BenchmarkRegexpMatchHard_1K        107321         117495         +9.48%
BenchmarkRevcomp                   733270055      659352759      -10.08%
BenchmarkTemplate                  109304977      112819078      +3.21%

Comparing Go 1.2 against this CL on an Intel Xeon E5520:

BenchmarkBinaryTree17              5986953594     5393104469     -9.92%
BenchmarkFannkuch11                4861139174     4327177615     -10.98%
BenchmarkGobDecode                 11830997       11235272       -5.04%
BenchmarkGobEncode                 6608722        6959837        +5.31%
BenchmarkGzip                      661875826      671769097      +1.49%
BenchmarkGunzip                    138630019      141096376      +1.78%
BenchmarkHTTPClientServer          71534          73610          +2.90%
BenchmarkJSONEncode                30393609       31796106       +4.61%
BenchmarkJSONDecode                139645860      114239137      -18.19%
BenchmarkMandelbrot200             5988660        6003077        +0.24%
BenchmarkGoParse                   6974092        6405995        -8.15%
BenchmarkRegexpMatchMedium_1K      111331         113895         +2.30%
BenchmarkRegexpMatchHard_1K        165961         168420         +1.48%
BenchmarkRevcomp                   995049292      892319935      -10.32%
BenchmarkTemplate                  145623363      148998339      +2.32%

Fixes #8036.

LGTM=khr
R=golang-codereviews, josharian, khr
CC=golang-codereviews, iant, r
https://golang.org/cl/99660044
2014-05-30 16:41:58 -04:00
Russ Cox
89d46fed2c cmd/gc: fix x=x crash
[Same as CL 102820043 except applied changes to 6g/gsubr.c
also to 5g/gsubr.c and 8g/gsubr.c. The problem I had last night
trying to do that was that 8g's copy of nodarg has different
(but equivalent) control flow and I was pasting the new code
into the wrong place.]

Description from CL 102820043:

The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.

Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.

The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.

Fixes #8097.

LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, khr, r
https://golang.org/cl/103750043
2014-05-29 13:47:31 -04:00
Russ Cox
9dd062b82e undo CL 102820043 / b0ce6dbafc18
Breaks 386 and arm builds.
The obvious reason is that this CL only edited 6g/gsubr.c
and failed to edit 5g/gsubr.c and 8g/gsubr.c.
However, the obvious CL applying the same edit to those
files (CL 101900043) causes mysterious build failures
in various of the standard package tests, usually involving
reflect. Something deep and subtle is broken but only on
the 32-bit systems.

Undo this CL for now.

««« original CL description
cmd/gc: fix x=x crash

The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.

Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.

The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.

Fixes #8097.

LGTM=r, khr
R=golang-codereviews, r, khr
CC=golang-codereviews, iant
https://golang.org/cl/102820043
»»»

TBR=r
CC=golang-codereviews, khr
https://golang.org/cl/95660043
2014-05-28 21:46:20 -04:00
Russ Cox
948b2c722b cmd/gc: fix x=x crash
The 'nodarg' function is used to obtain a Node*
representing a function argument or result.
It returned a brand new Node*, but that violates
the guarantee in most places in the compiler that
two Node*s refer to the same variable if and only if
they are the same Node* pointer. Reestablish that
invariant by making nodarg return a preexisting
named variable if present.

Having fixed that, avoid any copy during x=x in
componentgen, because the VARDEF we emit
before the copy marks the lhs x as dead incorrectly.

The change in walk.c avoids modifying the result
of nodarg. This was the only place in the compiler
that did so.

Fixes #8097.

LGTM=r, khr
R=golang-codereviews, r, khr
CC=golang-codereviews, iant
https://golang.org/cl/102820043
2014-05-28 19:50:19 -04:00
Russ Cox
4895f0dc5e test/run: limit parallelism to 1 for cross-exec builds
This matters for NaCl, which seems to swamp my 4-core MacBook Pro otherwise.
It's not a correctness problem, just a usability problem.

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/98600046
2014-05-28 01:01:08 -04:00
Russ Cox
d432238fad test: expand issue7863 test
This was sitting in my client but I forgot hg add.

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://golang.org/cl/101800045
2014-05-27 21:53:39 -07:00
Russ Cox
8a2db409c4 cmd/gc: fix race compilation failure 'non-orig name'
CL 51010045 fixed the first one of these:

        cmd/gc: return canonical Node* from temp

        For historical reasons, temp was returning a copy
        of the created Node*, not the original Node*.
        This meant that if analysis recorded information in the
        returned node (for example, n->addrtaken = 1), the
        analysis would not show up on the original Node*, the
        one kept in fn->dcl and consulted during liveness
        bitmap creation.

        Correct this, and watch for it when setting addrtaken.

        Fixes #7083.

        R=khr, dave, minux.ma
        CC=golang-codereviews
        https://golang.org/cl/51010045

CL 53200043 fixed the second:

        cmd/gc: fix race build

        Missed this case in CL 51010045.

        TBR=khr
        CC=golang-codereviews
        https://golang.org/cl/53200043

This CL fixes the third. There are only three nod(OXXX, ...)
calls in sinit.c, so maybe we're done. Embarassing that it
took three CLs to find all three.

Fixes #8028.

LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant
https://golang.org/cl/100800046
2014-05-27 23:59:27 -04:00
Russ Cox
ceb982e004 cmd/gc: fix defer copy(x, <-c)
In the first very rough draft of the reordering code
that was introduced in the Go 1.3 cycle, the pre-allocated
temporary for a ... argument was held in n->right.
It moved to n->alloc but the code avoiding n->right
was left behind in order.c. In copy(x, <-c), the receive
is in n->right and must be processed. Delete the special
case code, removing the bug.

Fixes #8039.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100820044
2014-05-27 23:59:06 -04:00
Russ Cox
daf9308066 cmd/gc: fix infinite loop in nil check removal
Fixes #8076.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/93610043
2014-05-27 23:58:49 -04:00
Russ Cox
74ce581b06 cmd/gc: fix conversion of runtime constant
The code cannot have worked before, because it was
trying to use the old value in a range check for the new
type, which might have a different representation
(hence the 'internal compiler error').

Fixes #8073.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/98630045
2014-05-27 21:38:19 -04:00
Brad Fitzpatrick
bd401baef2 test: add test for fixed issue 7863
Fixes #7863

LGTM=rsc
R=rsc, ruiu
CC=golang-codereviews
https://golang.org/cl/98610045
2014-05-27 16:01:43 -07:00
Russ Cox
cab54408da test: fix two typos in float_lit2.go
Noted by gri in CL 100660044 review but I missed them.

TBR=gri
CC=golang-codereviews
https://golang.org/cl/97570049
2014-05-21 17:19:12 -04:00
Russ Cox
2de449e7a0 test/float_lit2.go: rewrite to test values near boundaries
Add larger comment explaining testing methodology,
and derive tests arithmetically.

(These tests are checking rounding again; the derived
tests they replace were checking exact values.)

LGTM=r, gri
R=gri, r
CC=golang-codereviews
https://golang.org/cl/100660044
2014-05-21 17:12:06 -04:00
Robert Griesemer
e22705bf8b test/float_lit2.go: fix constants for 386 platforms (fix build)
LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/95480045
2014-05-21 09:15:07 -07:00
Robert Griesemer
765b4a3f86 test/float_lit2.go: compute test values from first principles
These constants pass go/types constant conversions as well.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/91590047
2014-05-21 08:53:47 -07:00
Russ Cox
0c2a727477 build: make nacl pass
Add nacl.bash, the NaCl version of all.bash.
It's a separate script because it builds a variant of package syscall
with a large zip file embedded in it, containing all the input files
needed for tests.

Disable various tests new since the last round, mostly the ones using os/exec.

Fixes #7945.

LGTM=dave
R=golang-codereviews, remyoudompheng, dave, bradfitz
CC=golang-codereviews
https://golang.org/cl/100590044
2014-05-20 12:10:19 -04:00
Russ Cox
f374dd30a0 test: test issue 7884 (already fixed)
I don't know when the bug was fixed, but empirically it was.
Make sure it stays fixed by adding a test.

Fixes #7884.

LGTM=adg
R=golang-codereviews, adg
CC=golang-codereviews
https://golang.org/cl/93500043
2014-05-20 11:42:25 -04:00
Russ Cox
82854d7b39 syscall: fix Write(nil) on NaCl
Fixes #7050.

LGTM=crawshaw, r
R=golang-codereviews, crawshaw, r
CC=golang-codereviews
https://golang.org/cl/91590043
2014-05-20 11:38:34 -04:00
Russ Cox
60be4a2450 cmd/gc: fix float32 const conversion and printing of big float consts
The float32 const conversion used to round to float64
and then use the hardware to round to float32.
Even though there was a range check before this
conversion, the double rounding introduced inaccuracy:
the round to float64 might round the value further away
from the float32 range, reaching a float64 value that
could not actually be rounded to float32. The hardware
appears to give us 0 in that case, but it is probably undefined.
Double rounding also meant that the wrong value might
be used for certain border cases.

Do the rounding the float32 ourselves, just as we already
did the rounding to float64. This makes the conversion
precise and also makes the conversion match the range check.

Finally, add some code to print very large (bigger than float64)
floating point constants in decimal floating point notation instead
of falling back to the precise but human-unreadable binary floating
point notation.

Fixes #8015.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/100580044
2014-05-19 22:57:59 -04:00
Russ Cox
a663e0a038 cmd/gc: fix <-<-expr
The temporary-introducing pass was not recursing
into the argumnt of a receive operation.

Fixes #8011.

LGTM=r
R=golang-codereviews, r
CC=golang-codereviews, iant, khr
https://golang.org/cl/91540043
2014-05-19 15:08:04 -04:00
Russ Cox
1357f548b0 cmd/gc: fix two select temporary bugs
The introduction of temporaries in order.c was not
quite right for two corner cases:

1) The rewrite that pushed new variables on the lhs of
a receive into the body of the case was dropping the
declaration of the variables. If the variables escape,
the declaration is what allocates them.
Caught by escape analysis sanity check.
In fact the declarations should move into the body
always, so that we only allocate if the corresponding
case is selected. Do that. (This is an optimization that
was already present in Go 1.2. The new order code just
made it stop working.)

Fixes #7997.

2) The optimization to turn a single-recv select into
an ordinary receive assumed it could take the address
of the destination; not so if the destination is _.

Fixes #7998.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100480043
2014-05-15 19:16:18 -04:00
Russ Cox
68aaf2ccda runtime: make scan of pointer-in-interface same as scan of pointer
The GC program describing a data structure sometimes trusts the
pointer base type and other times does not (if not, the garbage collector
must fall back on per-allocation type information stored in the heap).
Make the scanning of a pointer in an interface do the same.
This fixes a crash in a particular use of reflect.SliceHeader.

Fixes #8004.

LGTM=khr
R=golang-codereviews, khr
CC=0xe2.0x9a.0x9b, golang-codereviews, iant, r
https://golang.org/cl/100470045
2014-05-15 15:53:36 -04:00
Russ Cox
f5184d3437 cmd/gc: correct handling of globals, func args, results
Globals, function arguments, and results are special cases in
registerization.

Globals must be flushed aggressively, because nearly any
operation can cause a panic, and the recovery code must see
the latest values. Globals also must be loaded aggressively,
because nearly any store through a pointer might be updating a
global: the compiler cannot see all the "address of"
operations on globals, especially exported globals. To
accomplish this, mark all globals as having their address
taken, which effectively disables registerization.

If a function contains a defer statement, the function results
must be flushed aggressively, because nearly any operation can
cause a panic, and the deferred code may call recover, causing
the original function to return the current values of its
function results. To accomplish this, mark all function
results as having their address taken if the function contains
any defer statements. This causes not just aggressive flushing
but also aggressive loading. The aggressive loading is
overkill but the best we can do in the current code.

Function arguments must be considered live at all safe points
in a function, because garbage collection always preserves
them: they must be up-to-date in order to be preserved
correctly. Accomplish this by marking them live at all call
sites. An earlier attempt at this marked function arguments as
having their address taken, which disabled registerization
completely, making programs slower. This CL's solution allows
registerization while preserving safety. The benchmark speedup
is caused by being able to registerize again (the earlier CL
lost the same amount).

benchmark                old ns/op     new ns/op     delta
BenchmarkEqualPort32     61.4          56.0          -8.79%

benchmark                old MB/s     new MB/s     speedup
BenchmarkEqualPort32     521.56       570.97       1.09x

Fixes #1304. (again)
Fixes #7944. (again)
Fixes #7984.
Fixes #7995.

LGTM=khr
R=golang-codereviews, khr
CC=golang-codereviews, iant, r
https://golang.org/cl/97500044
2014-05-15 15:34:53 -04:00
Russ Cox
ec38c6f5e3 cmd/gc: fix duplicate map key check
Do not compare nil and true.

Fixes #7996.

LGTM=r
R=golang-codereviews, r
CC=golang-codereviews
https://golang.org/cl/91470043
2014-05-15 15:34:37 -04:00
Mikio Hara
147a21456e test: fix flakey test case for issue 4388
Seems like we need to drag the stack for <autogenerated>:1 on Plan 9.

See http://build.golang.org/log/283b996102b833dd81c58301d78aceaa4fe9838b.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/95390043
2014-05-15 06:39:15 +09:00
Russ Cox
26ad5d4ff0 cmd/gc: fix liveness vs regopt mismatch for input variables
The inputs to a function are marked live at all times in the
liveness bitmaps, so that the garbage collector will not free
the things they point at and reuse the pointers, so that the
pointers shown in stack traces are guaranteed not to have
been recycled.

Unfortunately, no one told the register optimizer that the
inputs need to be preserved at all call sites. If a function
is done with a particular input value, the optimizer will stop
preserving it across calls. For single-word values this just
means that the value recorded might be stale. For multi-word
values like slices, the value recorded could be only partially stale:
it can happen that, say, the cap was updated but not the len,
or that the len was updated but not the base pointer.
Either of these possibilities (and others) would make the
garbage collector misinterpret memory, leading to memory
corruption.

This came up in a real program, in which the garbage collector's
'slice len ≤ slice cap' check caught the inconsistency.

Fixes #7944.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, khr
https://golang.org/cl/100370045
2014-05-12 17:19:02 -04:00
Josh Bleecher Snyder
03c0f3fea9 cmd/gc: alias more variables during register allocation
This is joint work with Daniel Morsing.

In order for the register allocator to alias two variables, they must have the same width, stack offset, and etype. Code generation was altering a variable's etype in a few places. This prevented the variable from being moved to a register, which in turn prevented peephole optimization. This failure to alias was very common, with almost 23,000 instances just running make.bash.

This phenomenon was not visible in the register allocation debug output because the variables that failed to alias had the same name. The debugging-only change to bits.c fixes this by printing the variable number with its name.

This CL fixes the source of all etype mismatches for 6g, all but one case for 8g, and depressingly few cases for 5g. (I believe that extending CL 6819083 to 5g is a prerequisite.) Fixing the remaining cases in 8g and 5g is work for the future.

The etype mismatch fixes are:

* [gc] Slicing changed the type of the base pointer into a uintptr in order to perform arithmetic on it. Instead, support addition directly on pointers.

* [*g] OSPTR was giving type uintptr to slice base pointers; undo that. This arose, for example, while compiling copy(dst, src).

* [8g] 64 bit float conversion was assigning int64 type during codegen, overwriting the existing uint64 type.

Note that some etype mismatches are appropriate, such as a struct with a single field or an array with a single element.

With these fixes, the number of registerizations that occur while running make.bash for 6g increases ~10%. Hello world binary size shrinks ~1.5%. Running all benchmarks in the standard library show performance improvements ranging from nominal to substantive (>10%); a full comparison using 6g on my laptop is available at https://gist.github.com/josharian/8f9b5beb46667c272064. The microbenchmarks must be taken with a grain of salt; see issue 7920. The few benchmarks that show real regressions are likely due to issue 7920. I manually examined the generated code for the top few regressions and none had any assembly output changes. The few benchmarks that show extraordinary improvements are likely also due to issue 7920.

Performance results from 8g appear similar to 6g.

5g shows no performance improvements. This is not surprising, given the discussion above.

Update #7316

LGTM=rsc
R=rsc, daniel.morsing, bradfitz
CC=dave, golang-codereviews
https://golang.org/cl/91850043
2014-05-12 17:10:36 -04:00
Russ Cox
f078711b41 cmd/gc: fix escape analysis for slice of array
Fixes #7931.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100390044
2014-05-12 14:45:05 -04:00
Russ Cox
9b976f5f03 cmd/gc: record line number for auto-generated wrappers as <autogenerated>:1
Before we used line 1 of the first source file.
This should be clearer.

Fixes #4388.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/92250044
2014-05-12 11:59:55 -04:00
ChaiShushan
aed9762638 test/bench/shootout: support windows
1. fix executable extension (a.out -> a.exe).
2. fix pthread build error on mingw
3. if depends lib messing, skip the test

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100210043
2014-05-09 14:34:50 -07:00
Russ Cox
c99dce2b05 cmd/gc: fix ... escape analysis bug
If the ... element type contained no pointers,
then the escape analysis did not track the ... itself.
This manifested in an escaping ...byte being treated
as non-escaping.

Fixes #7934.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/100310043
2014-05-09 15:40:45 -04:00
Josh Bleecher Snyder
1848d71445 cmd/gc: don't give credit for NOPs during register allocation
The register allocator decides which variables should be placed into registers by charging for each load/store and crediting for each use, and then selecting an allocation with minimal cost. NOPs will be eliminated, however, so using a variable in a NOP should not generate credit.

Issue 7867 arises from attempted registerization of multi-word variables because they are used in NOPs. By not crediting for that use, they will no longer be considered for registerization.

This fix could theoretically lead to better register allocation, but NOPs are rare relative to other instructions.

Fixes #7867.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/94810044
2014-05-09 09:55:17 -07:00
Ian Lance Taylor
d3764dd435 test: add test that gccgo compiled incorrectly
LGTM=minux.ma
R=golang-codereviews, minux.ma
CC=golang-codereviews
https://golang.org/cl/94100045
2014-05-06 09:01:38 -04:00
Shenghou Ma
32dffef098 cmd/gc: fix segfault in isgoconst.
Variables declared with 'var' have no sym->def.

Fixes #7794.

LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://golang.org/cl/88360043
2014-04-16 23:12:06 -04:00
Jan Ziak
1d2b71ce83 cmd/gc: fewer errors for wrong argument count
Fixes #7675

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/85040044
2014-04-16 22:42:09 -04:00