Re: [9fans] mips linker hoisting stack freeing operation

[cinap_lenrek]

thanks.

--
cinap

Re: [9fans] mips linker hoisting stack freeing operation

[Charles Forsyth]

On 1 April 2015 at 02:34,  wrote:

> theres my attempt at preventing the linker from doing so:


It's one of those cases where it's funny it hasn't been noticed before,
since the effects won't be confined to libdraw. Anyway, that change seems
ok,
although it's really a particular type of conflict (between set R29 and use
R6 which depends on R29),
but expressing that is tricky with the limited value tracking there, and
the ADD R29 makes
as good a use of the delay slot as any.

[9fans] mips linker hoisting stack freeing operation

[cinap_lenrek]

Point
Pt(int x, int y)
{
int buf[1000];  /* 
added for demonstration */
Point p;

buf[0] = x;
p.x = x;
p.y = y;
return p;
}

vc -S compiler output looks ok, but i dont quite understand why it
doesnt directly move x and y to the caller passed storage in R1:

TEXTPt+0(SB),0,$4012
MOVWy+8(FP),R8
MOVWx+4(FP),R7
MOVWR1,.ret+0(FP)
MOVWR7,buf-4000(SP)
MOVWR7,p-4008(SP)
MOVWR8,p-4004(SP)
MOVW.ret+0(FP),R4
MOVW$p-4008(SP),R6
MOVW0(R6),R2
MOVW4(R6),R3
MOVWR2,0(R4)
MOVWR3,4(R4)
RET ,

and after linking it becomes:

Pt 0x4020   ADD $-0xfb0,R29 /* allocate 
stack frame */
Pt+0x4 0x4024   MOVWR1,.ret+4(FP)
Pt+0x8 0x4028   ADDU$0x8,R29,R6 /* local p */
Pt+0xc 0x402c   MOVW.ret+4(FP),R4
Pt+0x10 0x4030  MOVWx+8(FP),R7
Pt+0x14 0x4034  MOVWy+12(FP),R8
Pt+0x18 0x4038  MOVWR7,buf+16(SP)
Pt+0x1c 0x403c  MOVWR7,p+8(SP)
Pt+0x20 0x4040  MOVWR8,0xc(R29)
Pt+0x24 0x4044  ADD $0xfb0,R29  /* free stack 
frame (now R6 < SP) */
Pt+0x28 0x4048  MOVW0x0(R6),R2  /* p.x = x; NEIN! */
Pt+0x2c 0x404c  MOVW0x4(R6),R3  /* p.y = y; NEIN! */
Pt+0x30 0x4050  MOVWR2,0x0(R4)  /* ret->x = p.x */
Pt+0x34 0x4054  JMP (R31)
Pt+0x38 0x4058  MOVWR3,0x4(R4)  /* ret->y = p.y */

the problem appears to be the linker hoisting the ADD $const,SP up.
this works until we hit a interrupt or a note, then the local variables
get corrupted. and this is not the interrupt handlers fault,
as this depends on the size of the local variables of the
function, not some fixed redzone area. (thats what int buf[1000]
is trying to demonstrate).

theres my attempt at preventing the linker from doing so:

diff -r 192be1470c05 -r 359ae373800c sys/src/cmd/vl/sched.c
--- a/sys/src/cmd/vl/sched.cMon Mar 30 20:53:49 2015 -0400
+++ b/sys/src/cmd/vl/sched.cWed Apr 01 01:30:16 2015 +0200
@@ -85,7 +85,7 @@
for(t=s+1; t<=se; t++) {
if(!(t->p.mark & LOAD))
continue;
-   if(t->p.mark & BRANCH)
+   if(t->p.mark & BRANCH || t->set.ireg & (1p.mark & BRANCH || t->set.ireg & (1< sch && conflict(s-1, t))
continue;

which seems to work in this case:

Pt 0x4020   ADD $-0xfb0,R29 /* allocate 
stackframe */
Pt+0x4 0x4024   MOVWR1,.ret+4(FP)
Pt+0x8 0x4028   ADDU$0x8,R29,R6
Pt+0xc 0x402c   MOVW.ret+4(FP),R4
Pt+0x10 0x4030  MOVWx+8(FP),R7
Pt+0x14 0x4034  MOVWy+12(FP),R8
Pt+0x18 0x4038  MOVWR7,buf+16(SP)
Pt+0x1c 0x403c  MOVWR7,p+8(SP)
Pt+0x20 0x4040  MOVWR8,0xc(R29)
Pt+0x24 0x4044  MOVW0x0(R6),R2  /* p.x = x; JA! */
Pt+0x28 0x4048  MOVW0x4(R6),R3  /* p.y = y; JA! */
Pt+0x2c 0x404c  MOVWR2,0x0(R4)  /* ret->x = p.x */
Pt+0x30 0x4050  MOVWR3,0x4(R4)  /* ret->y = p.y */
Pt+0x34 0x4054  JMP (R31)
Pt+0x38 0x4058  ADD $0xfb0,R29  /* free stack 
frame last */

any comments (charles)?

--
cinap