summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorRuss Cox <rsc@golang.org>2013-10-31 17:18:57 +0000
committerRuss Cox <rsc@golang.org>2013-10-31 17:18:57 +0000
commitc46a59a5450c27897c1974efa539ef29a890a04b (patch)
tree18973818a812dc26e3b47f77c7aef157aa6aab40 /src
parentcb254af149624d08d6549f3a921eae4019560656 (diff)
downloadgo-c46a59a5450c27897c1974efa539ef29a890a04b.tar.gz
undo CL 19810043 / 352f3b7c9664
The CL causes misc/cgo/test to fail randomly. I suspect that the problem is the use of a division instruction in usleep, which can be called while trying to acquire an m and therefore cannot store the denominator in m. The solution to that would be to rewrite the code to use a magic multiply instead of a divide, but now we're getting pretty far off the original code. Go back to the original in preparation for a different, less efficient but simpler fix. ??? original CL description cmd/5l, runtime: make ARM integer division profiler-friendly The implementation of division constructed non-standard stack frames that could not be handled by the traceback routines. CL 13239052 left the frames non-standard but fixed them for the specific case of a divide-by-zero panic. A profiling signal can arrive at any time, so that fix is not sufficient. Change the division to store the extra argument in the M struct instead of in a new stack slot. That keeps the frames bog standard at all times. Also fix a related bug in the traceback code: when starting a traceback, the LR register should be ignored if the current function has already allocated its stack frame and saved the original LR on the stack. The stack copy should be used, as the LR register may have been modified. Combined, these make the torture test from issue 6681 pass. Fixes issue 6681. R=golang-dev, r, josharian CC=golang-dev https://codereview.appspot.com/19810043 ??? TBR=r CC=golang-dev https://codereview.appspot.com/20350043
Diffstat (limited to 'src')
-rw-r--r--src/cmd/5l/noop.c51
-rw-r--r--src/pkg/runtime/pprof/pprof_test.go25
-rw-r--r--src/pkg/runtime/runtime.h1
-rw-r--r--src/pkg/runtime/traceback_arm.c2
-rw-r--r--src/pkg/runtime/vlop_arm.s44
5 files changed, 62 insertions, 61 deletions
diff --git a/src/cmd/5l/noop.c b/src/cmd/5l/noop.c
index 70cec1f9c..fb70599b5 100644
--- a/src/cmd/5l/noop.c
+++ b/src/cmd/5l/noop.c
@@ -60,7 +60,7 @@ linkcase(Prog *casep)
void
noops(void)
{
- Prog *p, *q, *q1, *q2, orig;
+ Prog *p, *q, *q1, *q2;
int o;
Sym *tlsfallback, *gmsym;
@@ -401,27 +401,26 @@ noops(void)
break;
if(p->to.type != D_REG)
break;
-
- orig = *p;
+ q1 = p;
- /* MOV a,4(M) */
+ /* MOV a,4(SP) */
+ p = appendp(p);
p->as = AMOVW;
- p->line = orig.line;
+ p->line = q1->line;
p->from.type = D_REG;
- p->from.reg = orig.from.reg;
- p->reg = NREG;
+ p->from.reg = q1->from.reg;
p->to.type = D_OREG;
- p->to.reg = REGM;
+ p->to.reg = REGSP;
p->to.offset = 4;
/* MOV b,REGTMP */
p = appendp(p);
p->as = AMOVW;
- p->line = orig.line;
+ p->line = q1->line;
p->from.type = D_REG;
- p->from.reg = orig.reg;
- if(orig.reg == NREG)
- p->from.reg = orig.to.reg;
+ p->from.reg = q1->reg;
+ if(q1->reg == NREG)
+ p->from.reg = q1->to.reg;
p->to.type = D_REG;
p->to.reg = REGTMP;
p->to.offset = 0;
@@ -429,7 +428,7 @@ noops(void)
/* CALL appropriate */
p = appendp(p);
p->as = ABL;
- p->line = orig.line;
+ p->line = q1->line;
p->to.type = D_BRANCH;
p->cond = p;
switch(o) {
@@ -454,12 +453,34 @@ noops(void)
/* MOV REGTMP, b */
p = appendp(p);
p->as = AMOVW;
- p->line = orig.line;
+ p->line = q1->line;
p->from.type = D_REG;
p->from.reg = REGTMP;
p->from.offset = 0;
p->to.type = D_REG;
- p->to.reg = orig.to.reg;
+ p->to.reg = q1->to.reg;
+
+ /* ADD $8,SP */
+ p = appendp(p);
+ p->as = AADD;
+ p->line = q1->line;
+ p->from.type = D_CONST;
+ p->from.reg = NREG;
+ p->from.offset = 8;
+ p->reg = NREG;
+ p->to.type = D_REG;
+ p->to.reg = REGSP;
+ p->spadj = -8;
+
+ /* SUB $8,SP */
+ q1->as = ASUB;
+ q1->from.type = D_CONST;
+ q1->from.offset = 8;
+ q1->from.reg = NREG;
+ q1->reg = NREG;
+ q1->to.type = D_REG;
+ q1->to.reg = REGSP;
+ q1->spadj = 8;
break;
case AMOVW:
diff --git a/src/pkg/runtime/pprof/pprof_test.go b/src/pkg/runtime/pprof/pprof_test.go
index eb76b93c4..f1fc5faec 100644
--- a/src/pkg/runtime/pprof/pprof_test.go
+++ b/src/pkg/runtime/pprof/pprof_test.go
@@ -8,7 +8,6 @@ import (
"bytes"
"fmt"
"hash/crc32"
- "math/big"
"os/exec"
"regexp"
"runtime"
@@ -124,10 +123,6 @@ func testCPUProfile(t *testing.T, need []string, f func()) {
}
})
- if len(need) == 0 {
- return
- }
-
var total uintptr
for i, name := range need {
total += have[i]
@@ -242,26 +237,6 @@ func TestGoroutineSwitch(t *testing.T) {
}
}
-// Test that profiling of division operations is okay, especially on ARM. See issue 6681.
-func TestMathBigDivide(t *testing.T) {
- testCPUProfile(t, nil, func() {
- t := time.After(5 * time.Second)
- pi := new(big.Int)
- for {
- for i := 0; i < 100; i++ {
- n := big.NewInt(2646693125139304345)
- d := big.NewInt(842468587426513207)
- pi.Div(n, d)
- }
- select {
- case <-t:
- return
- default:
- }
- }
- })
-}
-
// Operating systems that are expected to fail the tests. See issue 6047.
var badOS = map[string]bool{
"darwin": true,
diff --git a/src/pkg/runtime/runtime.h b/src/pkg/runtime/runtime.h
index 02c7041ba..f7c2adb12 100644
--- a/src/pkg/runtime/runtime.h
+++ b/src/pkg/runtime/runtime.h
@@ -290,7 +290,6 @@ struct G
struct M
{
G* g0; // goroutine with scheduling stack
- uint32 divmod; // div/mod denominator on arm
void* moreargp; // argument pointer for more stack
Gobuf morebuf; // gobuf arg to morestack
diff --git a/src/pkg/runtime/traceback_arm.c b/src/pkg/runtime/traceback_arm.c
index 7c21dc798..02586f036 100644
--- a/src/pkg/runtime/traceback_arm.c
+++ b/src/pkg/runtime/traceback_arm.c
@@ -84,7 +84,7 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip,
frame.lr = 0;
flr = nil;
} else {
- if((n == 0 && frame.fp > frame.sp) || frame.lr == 0)
+ if(frame.lr == 0)
frame.lr = *(uintptr*)frame.sp;
flr = runtime·findfunc(frame.lr);
if(flr == nil) {
diff --git a/src/pkg/runtime/vlop_arm.s b/src/pkg/runtime/vlop_arm.s
index 76c0d5dab..d7c566afb 100644
--- a/src/pkg/runtime/vlop_arm.s
+++ b/src/pkg/runtime/vlop_arm.s
@@ -105,7 +105,7 @@ s = 2 // three temporary variables
M = 3
a = 11
// Be careful: R(a) == R11 will be used by the linker for synthesized instructions.
-TEXT udiv<>(SB),NOSPLIT,$-4-0
+TEXT udiv<>(SB),NOSPLIT,$-4
CLZ R(q), R(s) // find normalizing shift
MOVW.S R(q)<<R(s), R(a)
MOVW $fast_udiv_tab<>-64(SB), R(M)
@@ -165,8 +165,22 @@ udiv_by_0_or_1:
RET
udiv_by_0:
- MOVW $runtime·panicdivide(SB),R11
- B (R11)
+ // The ARM toolchain expects it can emit references to DIV and MOD
+ // instructions. The linker rewrites each pseudo-instruction into
+ // a sequence that pushes two values onto the stack and then calls
+ // _divu, _modu, _div, or _mod (below), all of which have a 16-byte
+ // frame plus the saved LR. The traceback routine knows the expanded
+ // stack frame size at the pseudo-instruction call site, but it
+ // doesn't know that the frame has a non-standard layout. In particular,
+ // it expects to find a saved LR in the bottom word of the frame.
+ // Unwind the stack back to the pseudo-instruction call site, copy the
+ // saved LR where the traceback routine will look for it, and make it
+ // appear that panicdivide was called from that PC.
+ MOVW 0(R13), LR
+ ADD $20, R13
+ MOVW 8(R13), R1 // actual saved LR
+ MOVW R1, 0(R13) // expected here for traceback
+ B runtime·panicdivide(SB)
TEXT fast_udiv_tab<>(SB),NOSPLIT,$-4
// var tab [64]byte
@@ -193,16 +207,14 @@ TEXT fast_udiv_tab<>(SB),NOSPLIT,$-4
// expects the result in R(TMP)
TMP = 11
-TEXT _divu(SB), NOSPLIT, $16-0
+TEXT _divu(SB), NOSPLIT, $16
MOVW R(q), 4(R13)
MOVW R(r), 8(R13)
MOVW R(s), 12(R13)
MOVW R(M), 16(R13)
MOVW R(TMP), R(r) /* numerator */
- MOVW m_divmod(m), R(q) /* denominator */
- MOVW $0, R(s)
- MOVW R(s), m_divmod(m)
+ MOVW 0(FP), R(q) /* denominator */
BL udiv<>(SB)
MOVW R(q), R(TMP)
MOVW 4(R13), R(q)
@@ -211,16 +223,14 @@ TEXT _divu(SB), NOSPLIT, $16-0
MOVW 16(R13), R(M)
RET
-TEXT _modu(SB), NOSPLIT, $16-0
+TEXT _modu(SB), NOSPLIT, $16
MOVW R(q), 4(R13)
MOVW R(r), 8(R13)
MOVW R(s), 12(R13)
MOVW R(M), 16(R13)
MOVW R(TMP), R(r) /* numerator */
- MOVW m_divmod(m), R(q) /* denominator */
- MOVW $0, R(s)
- MOVW R(s), m_divmod(m)
+ MOVW 0(FP), R(q) /* denominator */
BL udiv<>(SB)
MOVW R(r), R(TMP)
MOVW 4(R13), R(q)
@@ -229,15 +239,13 @@ TEXT _modu(SB), NOSPLIT, $16-0
MOVW 16(R13), R(M)
RET
-TEXT _div(SB),NOSPLIT,$16-0
+TEXT _div(SB),NOSPLIT,$16
MOVW R(q), 4(R13)
MOVW R(r), 8(R13)
MOVW R(s), 12(R13)
MOVW R(M), 16(R13)
MOVW R(TMP), R(r) /* numerator */
- MOVW m_divmod(m), R(q) /* denominator */
- MOVW $0, R(s)
- MOVW R(s), m_divmod(m)
+ MOVW 0(FP), R(q) /* denominator */
CMP $0, R(r)
BGE d1
RSB $0, R(r), R(r)
@@ -257,15 +265,13 @@ d2:
RSB $0, R(q), R(TMP)
B out
-TEXT _mod(SB),NOSPLIT,$16-0
+TEXT _mod(SB),NOSPLIT,$16
MOVW R(q), 4(R13)
MOVW R(r), 8(R13)
MOVW R(s), 12(R13)
MOVW R(M), 16(R13)
MOVW R(TMP), R(r) /* numerator */
- MOVW m_divmod(m), R(q) /* denominator */
- MOVW $0, R(s)
- MOVW R(s), m_divmod(m)
+ MOVW 0(FP), R(q) /* denominator */
CMP $0, R(q)
RSB.LT $0, R(q), R(q)
CMP $0, R(r)