[9fans] mulgen botch

[9fans] mulgen botch

[cinap_lenrek]

multiplication by the constant 0 yields an
"mulgen botch" error in 6c and 8c.

this is probably a very rare case mostly used during debugging...
would this be worth fixing? and if yes, would the fix
below be in the right place?

theres a patch:

--- a/sys/src/cmd/6c/cgen.c	Wed Mar 19 21:15:43 2014 +0100
+++ b/sys/src/cmd/6c/cgen.c	Thu Mar 20 19:59:20 2014 +0100
@@ -309,6 +309,11 @@
 				/* fall thru */
 			case OMUL:
 			case OLMUL:
+				if((o == OMUL || o == OLMUL) && (r->vconst == 0)){
+					cgen(l, Z);
+					zeroregm(nn);
+					goto done;
+				}
 				regalloc(&nod, l, nn);
 				cgen(l, &nod);
 				switch(o) {
@@ -621,6 +626,13 @@
 					reglcgen(&nod2, l, Z);
 				else
 					nod2 = *l;
+				if((o == OASMUL || o == OASLMUL) && (r->vconst == 0)){
+					cgen(&nod2, Z);
+					zeroregm(&nod2);
+					if(nn != Z)
+						zeroregm(nn);
+					goto done;
+				}
 				regalloc(&nod, l, nn);
 				cgen(&nod2, &nod);
 				switch(o) {

test program:

#include <u.h>
#include <libc.h>

int bar(int a)
{
	a *= 0;
	return a;
}
int foo(int a)
{
	return bar(a) * 0;
}

void
main(int argc, char *argv[])
{
	foo(1);
}

--
cinap

Re: [9fans] mulgen botch

[cinap_lenrek]

sorry... the code from the last mail contains an error...
theres a missing regfree() in the OASMUL and OASLMUL case when
hardleft is set. in any case, i'm unsure if it wouldnt be
better to handle this in mulgen() instead. as this case is
so rare, it is probably not worth trying to avoid useless
register moves here.

--
cinap

Re: [9fans] mulgen botch

[erik quanstrom]

On Thu Mar 20 16:20:29 EDT 2014, cinap_lenrek@felloff.net wrote:
> sorry... the code from the last mail contains an error...
> theres a missing regfree() in the OASMUL and OASLMUL case when
> hardleft is set. in any case, i'm unsure if it wouldnt be
> better to handle this in mulgen() instead. as this case is
> so rare, it is probably not worth trying to avoid useless
> register moves here.

i suppose you can make the test for a constant expression fail, by simply
changing both instances of this
		if(r->op == OCONST && typechl[n->type->etype]) {
to this
		if(r->op == OCONST && r->vconst != 0 && typechl[n->type->etype]) {

i haven't thought too hard about this, or compiled it, so maybe too easy?

- erik

Re: [9fans] mulgen botch

[erik quanstrom]

On Thu Mar 20 17:51:30 EDT 2014, quanstro@quanstro.net wrote:
> On Thu Mar 20 16:20:29 EDT 2014, cinap_lenrek@felloff.net wrote:
> > sorry... the code from the last mail contains an error...
> > theres a missing regfree() in the OASMUL and OASLMUL case when
> > hardleft is set. in any case, i'm unsure if it wouldnt be
> > better to handle this in mulgen() instead. as this case is
> > so rare, it is probably not worth trying to avoid useless
> > register moves here.
>
> i suppose you can make the test for a constant expression fail, by simply
> changing both instances of this
> 		if(r->op == OCONST && typechl[n->type->etype]) {
> to this
> 		if(r->op == OCONST && r->vconst != 0 && typechl[n->type->etype]) {
>
> i haven't thought too hard about this, or compiled it, so maybe too easy?

on second thought, better maching would be better.  only multplication by
0 is safe.

- erik

Re: [9fans] mulgen botch

[cinap_lenrek]

i found a simple solution now:

diff -r 46d745fbf479 sys/src/cmd/6c/mul.c
--- a/sys/src/cmd/6c/mul.c	Thu Mar 20 16:44:01 2014 +0100
+++ b/sys/src/cmd/6c/mul.c	Fri Mar 21 00:03:49 2014 +0100
@@ -312,6 +312,11 @@
 	Mparam *p;
 	Node nod, nods;

+	if(v == 0){
+		zeroregm(n);
+		return 1;
+	}
+
 	for(i = 0; i < nelem(multab); i++) {
 		p = &multab[i];
 		if(p->value == v)


as expected, the code generated *without* optimization does
some stupid moves:

term% ./6.out -SN /tmp/a.c
	TEXT	bar+0(SB),0,$0
	MOVL	BP,a+0(FP)
	MOVL	a+0(FP),AX
	MOVL	$0,AX
	MOVL	AX,a+0(FP)
	MOVL	a+0(FP),AX
	NOP	,X0
	RET	,
	NOP	,AX
	NOP	,X0
	RET	,
	TEXT	foo+0(SB),0,$16
	MOVL	BP,a+0(FP)
	MOVL	a+0(FP),BP
	CALL	,bar+0(SB)
	MOVL	$0,AX
	NOP	,X0
	RET	,
	NOP	,AX
	NOP	,X0
	RET	,
	TEXT	main+0(SB),0,$16
	MOVL	BP,argc+0(FP)
	MOVL	$1,BP
	CALL	,foo+0(SB)
	NOP	,AX
	NOP	,X0
	RET	,
	END	,

with optimization, the stupid moves are gone:

term% ./6.out -S /tmp/a.c
	TEXT	bar+0(SB),0,$0
	MOVL	$0,AX
	RET	,
	RET	,
	TEXT	foo+0(SB),0,$16
	CALL	,bar+0(SB)
	MOVL	$0,AX
	RET	,
	RET	,
	TEXT	main+0(SB),0,$16
	MOVL	$1,BP
	CALL	,foo+0(SB)
	RET	,
	END	,

so this is as good as the initial attempt just that
the change is just 4 lines in one location where we
do the actual multiplication.

--
cinap