mulsw macro change issue

[jonathanbennett73]

Hi,

I'm helping out Virgill with his Amigaklang exe generator which uses your cool gcc environment.

I updated his project from gcc8 to the gcc10 version and it stopped working (corrupt sounds). I have traced it to a commit on the 18th may where the mulsw and other macros were improved. If I leave the gcc10 in place but use the old mulsw macros it works OK again.

I had a look a the macros but the gcc/asm syntax is new to me so I couldn't spot any errors.

Virgill calls mulsw in two places as follows, maybe the error will be obvious to you:

short osc_sine (BYTE instance, short freq, UBYTE gain){ int temp; short res;static short counter[16]; short buf = counter[instance]+=freq; buf -= 16384; temp= mulsw(buf, 32767-abs(buf)); res =temp>>16; res<<=3; return vol(res,gain); }

inline short vol(short val, UBYTE gain){ return mulsw(val, gain) >> 7; }

You can message me here or on EAB if you need me to check/change something.

Cheers
Antiriad

[BartmanAbyss]

I'm not sure what the error is here either. However, it's totally fine if you just keep using the old implementation.

[bebbo]

the new code is buggy:

inline int mulsw(short a, short b) {
    asm("mulsw %1,%0":"+d"(a): "mid"(b): "cc");
    return a;
}

consider

int foo(short a) {
  return a;
}

the compiler has to sign extend the short to int which is done with ext.l d0. Same applies to the mulsw version above.

The old version had an internal int variable int a; to get the result and thus no sign extension is necessary.

[jonathanbennett73]

Bebbo, that update still breaks AmigaKlang - the original macro (before Bartman's recent change) works ok though.

Antiriad.

[rjobling]

I've decided for now to settle on using two different versions of each so you can avoid the ext.l when you know it isn't necessary.

inline unsigned int muluw32(unsigned short a, unsigned short b) {
	unsigned int r = a;
	asm("muluw %1,%0":"+d"(r): "mid"(b): "cc");
	return r;
}
inline int mulsw32(short a, short b) {
	int r = a;
	asm("mulsw %1,%0":"+d"(r): "mid"(b): "cc");
	return r;
}
inline unsigned short muluw16(unsigned short a, unsigned short b) {
	asm("muluw %1,%0":"+d"(a): "mid"(b): "cc");
	return a;
}
inline short mulsw16(short a, short b) {
	asm("mulsw %1,%0":"+d"(a): "mid"(b): "cc");
	return a;
}

[bebbo]

btw: with gcc there is no need for inline asm:

unsigned int muluw32(unsigned short a, unsigned short b) {
    return ((unsigned int)a) * b;
}

yields

_muluw32:
        move.w (6,sp),d0
        mulu.w (10,sp),d0
        rts

so just create inline versions with C code and you are fine.

[rjobling]

I just tried that and at least in my case it doesn't work.
It'll try to generate a 32bit multiply requiring __mulsi3.

[bebbo]

maybe you could provide some minimal code to reproduce that issue? Maybe even here? https://franke.ms/cex/z/7aMKh4

[rjobling]

It's happening with this simple example:

http://franke.ms/cex/z/bTj19T

What's confusing is that it goes away if you remove -m68000.

[BartmanAbyss]

Yes, I wrote in the README it 'sometimes' uses __mulsi3, because it really depends on the inlining and LTO stuff. We couldn't find clear patterns..

[bebbo]

hm... https://franke.ms/cex/z/5xeGnc
guess you stick to your asm implementation.