[9fans] kenc vlong <asop> double

[9fans] kenc vlong <asop> double

[cinap_lenrek]

the following code generates unhandled instructions
for the linker on 64 bit simulating archs (8c, 5c):

vlong v;
double d;
v += d;

 == cgen ==
ASADD DOUBLE (1) a.c:13
   NAME "v" -8 <11> VLONG a.c:13
   NAME "d" -16 <11> DOUBLE a.c:13

term% 8c -S a.c
	...
	FMOVD	d+-16(SP),F0
	MOVL	v+-8(SP),F0			<- nope.
	FADDDP	F0,F1
	MOVL	F0,v+-8(SP)			<- nope.
	RET	,
	END	,

but works on amd64:

term% 6c -S a.c
	...
	CVTSQ2SD	v+-8(SP),X0
	ADDSD	d+-16(SP),X0
	CVTTSD2SQ	X0,CX
	RET	,

the following code works on 8c/5c:

v = v + d;

 == cgen ==
AS VLONG (100) a.c:13
   NAME "v" -8 <11> VLONG a.c:13
   FUNC VLONG (100) a.c:13
      NAME "_d2v" 0 <10> FUNC VLONG a.c:1
      ADD DOUBLE (100) a.c:13
         FUNC DOUBLE (100) a.c:13
            NAME "_v2d" 0 <10> FUNC DOUBLE a.c:1
            NAME "v" -8 <11> VLONG a.c:13
         NAME "d" -16 <11> DOUBLE a.c:13


term% 8c -S a.c
	....
	MOVL	v+-8(SP),AX
	MOVL	AX,(SP)
	MOVL	v+-4(SP),AX
	MOVL	AX,4(SP)
	CALL	,_v2d+0(SB)
	FADDD	d+-16(SP),F0
	FMOVDP	F0,.safe+-28(SP)
	LEAL	v+-8(SP),AX
	MOVL	AX,(SP)
	FMOVD	.safe+-28(SP),F0
	FMOVDP	F0,4(SP)
	CALL	,_d2v+0(SB)

any ideas how to fix this?

--
cinap

Re: [9fans] kenc vlong <asop> double

[erik quanstrom]

> 	MOVL	v+-8(SP),F0			<- nope.

actually, double nope.  the move if implemented as written is wrong.  we want to move
a quad to a float, not a long to a float.

- erik

Re: [9fans] kenc vlong <asop> double

[cinap_lenrek]

the problem is v += d propagates the whole expression
to double. so we somehow need to convert v to double,
do the <asop> then convert back but return the double
result.

i tried (unsuccessfully) to come up with something like
this below in com64, but i dont know what i'm doing and
someone who knows kenc better can probably solve this by
deleting some code instead of adding more:

OCOMMA(
	OAS(ONAME(t, TDOUBLE), OFUNC(_v2d, ONAME(v, TVLONG))),
	OASADD(t, d),
	OCOMMA(
		OAS(ONAME(v, TVLONG), OFUNC(_d2v, ONAME(t, TDOUBLE))),
		NAME(t, TDOUBLE)))

the good news is that double and float are the only cases
where this can happen i think. and its rather silly code,
so maybe it should just give up and throw a error message.

lets see what charles thinks about this.

--
cinap

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 13 September 2015 at 15:23, <cinap_lenrek@felloff.net> wrote:

>
> lets see what charles thinks about this.


I thought at first that one had been fixed some time ago, but it was
probably just the version with compiled code (eg, amd64).
i'll have a look at it.

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

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 13 September 2015 at 15:23, <cinap_lenrek@felloff.net> wrote:

> the problem is v += d propagates the whole expression
> to double. so we somehow need to convert v to double,
> do the <asop> then convert back but return the double
> result.
>

the type of E1 op=E2 is the same as the type of E1 = E1 op E2,
since the expressions are "equivalent" in the sense of language definitions,
and has the value (and type) of E1 after the assignment,
so the final type is vlong not double (so the type also isn't right).

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

Re: [9fans] kenc vlong <asop> double

[cinap_lenrek]

ah, that makes sense.

so the bug is in com.c where we use arith() to figure out the type
of the operation (which looks in a table indexed by the types
of both sides)? but we should instead just assign the type from
the left hand side?

just assigning t to n->type fixes the type, but why did it
call arith() in the first place?

--- a/sys/src/cmd/cc/com.c	Sun Sep 13 13:51:00 2015 +0200
+++ b/sys/src/cmd/cc/com.c	Sun Sep 13 19:59:43 2015 +0200
@@ -182,8 +182,8 @@
 			r = new1(OCAST, n->right, Z);
 			r->type = t;
 			n->right = r;
-			n->type = t;
 		}
+		n->type = t;
 		break;

--
cinap

Re: [9fans] kenc vlong <asop> double

[erik quanstrom]

> --- a/sys/src/cmd/cc/com.c	Sun Sep 13 13:51:00 2015 +0200
> +++ b/sys/src/cmd/cc/com.c	Sun Sep 13 19:59:43 2015 +0200
> @@ -182,8 +182,8 @@
>  			r = new1(OCAST, n->right, Z);
>  			r->type = t;
>  			n->right = r;
> -			n->type = t;
>  		}
> +		n->type = t;
>  		break;

isn't the && !mixedasop() added to the condition the problem:
the cast should not be elided if it's a float/double.  though 8c
might get this wrong for other reasons.

- erik

Re: [9fans] kenc vlong <asop> double

[erik quanstrom]

> On 13 September 2015 at 19:21, erik quanstrom <quanstro@quanstro.net> wrote:
>
> > isn't the && !mixedasop() added to the condition the problem:
> > the cast should not be elided if it's a float/double.  t
> >
>
> the test is correct, because in mixed-mode assignment, you want to do the
> calculation in float/double,
> then convert it to the integer type, not convert the rhs float/double to
> the integer type at the start.
> ie, I += D  is I = I+D which is I = (double)I + D, not I += (int)D;

ah, you're right.

- erik

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 13 September 2015 at 19:21, erik quanstrom <quanstro@quanstro.net> wrote:

> isn't the && !mixedasop() added to the condition the problem:
> the cast should not be elided if it's a float/double.  t
>

the test is correct, because in mixed-mode assignment, you want to do the
calculation in float/double,
then convert it to the integer type, not convert the rhs float/double to
the integer type at the start.
ie, I += D  is I = I+D which is I = (double)I + D, not I += (int)D;

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

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 13 September 2015 at 19:01, <cinap_lenrek@felloff.net> wrote:

> just assigning t to n->type fixes the type, but why did it
> call arith() in the first place?
>

for P += I, P -= I, and other more straightforward cases of mixed-modes,
you need "the usual arithmetic conversions" first.

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

Re: [9fans] kenc vlong <asop> double

[cinap_lenrek]

so we really want to have n->type == double/float so the arithmetic
is done in the right domain, but com64 needs to check for this special
case and convert back and forth and then change n->type to vlong?

--
cinap

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 13 September 2015 at 19:46, <cinap_lenrek@felloff.net> wrote:

> so we really want to have n->type == double/float


no, it happens to be wrong for ordinary integers too:


int
x(int a, double d)
{
a += (a += d);
return a;
}

should have the outer += done as int using the result of a += d as stored
back into `a' and read back,
but instead it incorrectly does two floating-point adds. it evidently isn't
that common to feed the result
of += etc back into a computation, at least with mixed modes, or this would
have been more troublesome.

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

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 13 September 2015 at 19:55, Charles Forsyth <charles.forsyth@gmail.com>
wrote:

> On 13 September 2015 at 19:46, <cinap_lenrek@felloff.net> wrote:
>
>> so we really want to have n->type == double/float
>
>
> no, it happens to be wrong for ordinary integers too:
>
>
> int
> x(int a, double d)
> {
> a += (a += d);
> return a;
> }
>

i've fixed the original problem, but the assumption that the result type is
the type of the right,
not the type of the left (after a store) turns out to be fairly pervasive,
and i'd better fix that too.
6c and 7c get it right. the others are variously wrong.

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

Re: [9fans] kenc vlong <asop> double

[cinap_lenrek]

is there any reason to handle the <asop> as single operations instead
of breaking it down into separate <as> and <op> operations? that is,
is (a <op>= b) always equivalent to (a = a <op> b)?

--
cinap

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 14 September 2015 at 13:59, <cinap_lenrek@felloff.net> wrote:

> is there any reason to handle the <asop> as single operations instead
> of breaking it down into separate <as> and <op> operations? that is,
> is (a <op>= b) always equivalent to (a = a <op> b)?
>

you then have to fuss whether the evaluation of a has side-effects, and
if so, add an assignment to a temporary. it turns out that the existing
code needs only a little more care about which type is used to allocate
registers, and on the RISCy machines it will then work. 8c is a little more
involved because of the 387's architecture. there probably won't be much
code,
but it's fiddly. it would be nice finally to switch
to SSE but i suppose there might still be some SSE-less x86/32 processors.

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

Re: [9fans] kenc vlong <asop> double

[Charles Forsyth]

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

On 14 September 2015 at 14:18, Charles Forsyth <charles.forsyth@gmail.com>
wrote:

> 8c is a little more
> involved because of the 387's architecture. there probably won't be much
> code,
> but it's fiddly.
>

in fact it was easy. i'll collect the changes shortly.

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

Re: [9fans] kenc vlong <asop> double

[cinap_lenrek]

thanks.

--
cinap