[9fans] mips linker hoisting stack freeing operation

[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:

TEXT	Pt+0(SB),0,$4012
	MOVW	y+8(FP),R8
	MOVW	x+4(FP),R7
	MOVW	R1,.ret+0(FP)
	MOVW	R7,buf-4000(SP)
	MOVW	R7,p-4008(SP)
	MOVW	R8,p-4004(SP)
	MOVW	.ret+0(FP),R4
	MOVW	$p-4008(SP),R6
	MOVW	0(R6),R2
	MOVW	4(R6),R3
	MOVW	R2,0(R4)
	MOVW	R3,4(R4)
	RET	,

and after linking it becomes:

Pt 0x00004020	ADD	$-0xfb0,R29				/* allocate stack frame */
Pt+0x4 0x00004024	MOVW	R1,.ret+4(FP)
Pt+0x8 0x00004028	ADDU	$0x8,R29,R6		/* local p */
Pt+0xc 0x0000402c	MOVW	.ret+4(FP),R4
Pt+0x10 0x00004030	MOVW	x+8(FP),R7
Pt+0x14 0x00004034	MOVW	y+12(FP),R8
Pt+0x18 0x00004038	MOVW	R7,buf+16(SP)
Pt+0x1c 0x0000403c	MOVW	R7,p+8(SP)
Pt+0x20 0x00004040	MOVW	R8,0xc(R29)
Pt+0x24 0x00004044	ADD	$0xfb0,R29			/* free stack frame (now R6 < SP) */
Pt+0x28 0x00004048	MOVW	0x0(R6),R2		/* p.x = x; NEIN! */
Pt+0x2c 0x0000404c	MOVW	0x4(R6),R3		/* p.y = y; NEIN! */
Pt+0x30 0x00004050	MOVW	R2,0x0(R4)		/* ret->x = p.x */
Pt+0x34 0x00004054	JMP	(R31)
Pt+0x38 0x00004058	MOVW	R3,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.c	Mon Mar 30 20:53:49 2015 -0400
+++ b/sys/src/cmd/vl/sched.c	Wed 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 & (1<<REGSP))
 				break;
 			if(conflict(s, t))
 				break;
@@ -102,7 +102,7 @@

 		/* put schedule fodder above load */
 		for(t=s+1; t<=se; t++) {
-			if(t->p.mark & BRANCH)
+			if(t->p.mark & BRANCH || t->set.ireg & (1<<REGSP))
 				break;
 			if(s > sch && conflict(s-1, t))
 				continue;

which seems to work in this case:

Pt 0x00004020	ADD	$-0xfb0,R29				/* allocate stackframe */
Pt+0x4 0x00004024	MOVW	R1,.ret+4(FP)
Pt+0x8 0x00004028	ADDU	$0x8,R29,R6
Pt+0xc 0x0000402c	MOVW	.ret+4(FP),R4
Pt+0x10 0x00004030	MOVW	x+8(FP),R7
Pt+0x14 0x00004034	MOVW	y+12(FP),R8
Pt+0x18 0x00004038	MOVW	R7,buf+16(SP)
Pt+0x1c 0x0000403c	MOVW	R7,p+8(SP)
Pt+0x20 0x00004040	MOVW	R8,0xc(R29)
Pt+0x24 0x00004044	MOVW	0x0(R6),R2		/* p.x = x; JA! */
Pt+0x28 0x00004048	MOVW	0x4(R6),R3		/* p.y = y; JA! */
Pt+0x2c 0x0000404c	MOVW	R2,0x0(R4)		/* ret->x = p.x */
Pt+0x30 0x00004050	MOVW	R3,0x4(R4)		/* ret->y = p.y */
Pt+0x34 0x00004054	JMP	(R31)
Pt+0x38 0x00004058	ADD	$0xfb0,R29			/* free stack frame last */

any comments (charles)?

--
cinap

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

[Charles Forsyth]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

On 1 April 2015 at 02:34, <cinap_lenrek@felloff.net> 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.

[-- Attachment #2: Type: text/html, Size: 946 bytes --]

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

[cinap_lenrek]

thanks.

--
cinap