2014-04-03 17:51 GMT+02:00 karl ramberg karlramberg@gmail.com:

I can confirm this bug on windows Cog.

Cheers, Karl

On Thu, Apr 3, 2014 at 10:11 AM, Nicolai Hess nicolaihess@web.de wrote:

...

There is still something wrong with byte/word swap in windows ioShowDisplay code attached are two screenshots squeak 4.5 image with cogvm from http://files.pharo.org/vm/cogmt/win/ pharo 30793 image with latest vm from http://files.pharo.org/vm/pharo/win/ showing this bug after setting the Display depth to 16 (Display newDepth:16)

sqwin32window.c has three variants for doing the byte_swap/word_swap on bitdepths with big endian to convert to lsb

This one is active and causes this error. # if __GNUC__ >= 3 # define BYTE_SWAP(w) __asm__("bswap %0" : "=r" (w) : "r" (w)) # define WORD_SWAP(w) __asm__("roll $16, %0" : "=r" (w) : "r" (w))

This one would work # else # define BYTE_SWAP(w) __asm__("bswap %%eax" : "=eax" (w) : "eax" (w)) # define WORD_SWAP(w) __asm__("roll $16, %%eax" : "=eax" (w) : "eax" (w))

This one, of course, works too # else # define BYTE_SWAP(w) w = (w << 24) | ((w & 0xFF00) << 8) | ((w >> 8) & 0xFF00) | (w >> 24) # define WORD_SWAP(w) w = (( (unsigned)(w) << 16) | ((unsigned) (w) >> 16))

This one is not there but would work (at least with gcc > 4.5 # define BYTE_SWAP_MY(w) __asm__("bswap %0" : "+r" (w)) # define WORD_SWAP_MY(w) __asm__("roll $16, %0" : "+r" (w))

But actually I don't know assembler and/or the gcc inline code syntax, so I don't know what is wrong with the first version :)

Nicolai

btw, you can not test this bug with the current squeak 4.5 all in on image, as it uses a rather old vm.

Third screenshot: using the latest stable pharo-vm, it looks much more wrong, as there was another(?) bug that is fixed already(?) - i don't know :)

Someone else can reproduce this? (eliot?, nicolas? ) It is easily fixable, I think. But I am not good at inline assembler. I'll open a mantis and fogbugz report.

regards Nicolai

2014-05-05 20:37 GMT+02:00 Eliot Miranda eliot.miranda@gmail.com:

Hi Nicolai,

On Mon, May 5, 2014 at 12:06 AM, Nicolai Hess nicolaihess@web.de wrote:

...

2014-04-03 17:51 GMT+02:00 karl ramberg karlramberg@gmail.com:

...

I can confirm this bug on windows Cog.

Cheers, Karl

On Thu, Apr 3, 2014 at 10:11 AM, Nicolai Hess nicolaihess@web.dewrote:

...

There is still something wrong with byte/word swap in windows ioShowDisplay code attached are two screenshots squeak 4.5 image with cogvm from http://files.pharo.org/vm/cogmt/win/ pharo 30793 image with latest vm from http://files.pharo.org/vm/pharo/win/ showing this bug after setting the Display depth to 16 (Display newDepth:16)

sqwin32window.c has three variants for doing the byte_swap/word_swap on bitdepths with big endian to convert to lsb

This one is active and causes this error. # if __GNUC__ >= 3 # define BYTE_SWAP(w) __asm__("bswap %0" : "=r" (w) : "r" (w)) # define WORD_SWAP(w) __asm__("roll $16, %0" : "=r" (w) : "r" (w))

This one would work # else # define BYTE_SWAP(w) __asm__("bswap %%eax" : "=eax" (w) : "eax" (w)) # define WORD_SWAP(w) __asm__("roll $16, %%eax" : "=eax" (w) : "eax" (w))

This one, of course, works too # else # define BYTE_SWAP(w) w = (w << 24) | ((w & 0xFF00) << 8) | ((w >> 8) & 0xFF00) | (w >> 24) # define WORD_SWAP(w) w = (( (unsigned)(w) << 16) | ((unsigned) (w) >> 16))

This one is not there but would work (at least with gcc > 4.5 # define BYTE_SWAP_MY(w) __asm__("bswap %0" : "+r" (w)) # define WORD_SWAP_MY(w) __asm__("roll $16, %0" : "+r" (w))

But actually I don't know assembler and/or the gcc inline code syntax, so I don't know what is wrong with the first version :)

Nicolai

btw, you can not test this bug with the current squeak 4.5 all in on image, as it uses a rather old vm.

Third screenshot: using the latest stable pharo-vm, it looks much more wrong, as there was another(?) bug that is fixed already(?) - i don't know :)

Someone else can reproduce this? (eliot?, nicolas? ) It is easily fixable, I think. But I am not good at inline assembler. I'll open a mantis and fogbugz report.

Yes, I can reproduce it. But I've not had time to check with a pre 3.x

and a post 3.x compiler to check what the right syntax is. I'm busy with other stuff right now. Hopefully I'll get to this in a coupel of weeks. Anyone else out there who can provide the right syntax for

2.95 3.4.x 4.x

I'd be very grateful and simply integrate the fix.

-- best, Eliot

Ok, thank you eliot. I think I know now way the first version does not work: __asm__("bswap %0" : "=r" (w) : "r" (w)) The compiler might generate code like this, just:

bswap %ebx

without initialisation of that register (although we declare "r"(w) as input). It behaves like that, because an output register is supposed to be overwritten :) and the bswap command only uses the %0 (the first register in the output : input register list).

The proper way for defining an register as output-input (or read-write register) would be __asm__("bswap %0" : "+r" (w) ) Some asm-inline tutorials mention, this(the "+" constraint modifier) would not work on all compilers and prefer another older syntax: __asm__("bswap %0" : "=r" (w) : "0" (w)) (use the 0th output register for input too)

But even the gcc2.95 doc ( http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_16.html#IDX955) lists "+" as allowed constraint modifier, so, I think it should work.

I'll test this with different gcc versions and compare the compiler output (after I found out how to install multiple mingw versions :) )

nicolai

2014-05-07 1:32 GMT+02:00 Eliot Miranda eliot.miranda@gmail.com:

Hi Nicolai,

On Tue, May 6, 2014 at 4:14 AM, Nicolai Hess nicolaihess@web.de wrote:

...

2014-05-05 20:37 GMT+02:00 Eliot Miranda eliot.miranda@gmail.com:

...

Hi Nicolai,

On Mon, May 5, 2014 at 12:06 AM, Nicolai Hess nicolaihess@web.dewrote:

...

2014-04-03 17:51 GMT+02:00 karl ramberg karlramberg@gmail.com:

...

I can confirm this bug on windows Cog.

Cheers, Karl

On Thu, Apr 3, 2014 at 10:11 AM, Nicolai Hess nicolaihess@web.dewrote:

...

There is still something wrong with byte/word swap in windows ioShowDisplay code attached are two screenshots squeak 4.5 image with cogvm from http://files.pharo.org/vm/cogmt/win/ pharo 30793 image with latest vm from http://files.pharo.org/vm/pharo/win/ showing this bug after setting the Display depth to 16 (Display newDepth:16)

sqwin32window.c has three variants for doing the byte_swap/word_swap on bitdepths with big endian to convert to lsb

This one is active and causes this error. # if __GNUC__ >= 3 # define BYTE_SWAP(w) __asm__("bswap %0" : "=r" (w) : "r" (w)) # define WORD_SWAP(w) __asm__("roll $16, %0" : "=r" (w) : "r" (w))

This one would work # else # define BYTE_SWAP(w) __asm__("bswap %%eax" : "=eax" (w) : "eax" (w)) # define WORD_SWAP(w) __asm__("roll $16, %%eax" : "=eax" (w) : "eax" (w))

This one, of course, works too # else # define BYTE_SWAP(w) w = (w << 24) | ((w & 0xFF00) << 8) | ((w >> 8) & 0xFF00) | (w >> 24) # define WORD_SWAP(w) w = (( (unsigned)(w) << 16) | ((unsigned) (w) >> 16))

This one is not there but would work (at least with gcc > 4.5 # define BYTE_SWAP_MY(w) __asm__("bswap %0" : "+r" (w)) # define WORD_SWAP_MY(w) __asm__("roll $16, %0" : "+r" (w))

But actually I don't know assembler and/or the gcc inline code syntax, so I don't know what is wrong with the first version :)

Nicolai

btw, you can not test this bug with the current squeak 4.5 all in on image, as it uses a rather old vm.

Third screenshot: using the latest stable pharo-vm, it looks much more wrong, as there was another(?) bug that is fixed already(?) - i don't know :)

Someone else can reproduce this? (eliot?, nicolas? ) It is easily fixable, I think. But I am not good at inline assembler. I'll open a mantis and fogbugz report.

Yes, I can reproduce it. But I've not had time to check with a pre 3.x

and a post 3.x compiler to check what the right syntax is. I'm busy with other stuff right now. Hopefully I'll get to this in a coupel of weeks. Anyone else out there who can provide the right syntax for

2.95 3.4.x 4.x

I'd be very grateful and simply integrate the fix.

-- best, Eliot

Ok, thank you eliot. I think I know now way the first version does not work: __asm__("bswap %0" : "=r" (w) : "r" (w)) The compiler might generate code like this, just:

bswap %ebx

without initialisation of that register (although we declare "r"(w) as input). It behaves like that, because an output register is supposed to be overwritten :) and the bswap command only uses the %0 (the first register in the output : input register list).

The proper way for defining an register as output-input (or read-write register) would be __asm__("bswap %0" : "+r" (w) ) Some asm-inline tutorials mention, this(the "+" constraint modifier) would not work on all compilers and prefer another older syntax: __asm__("bswap %0" : "=r" (w) : "0" (w)) (use the 0th output register for input too)

Yes, I think you're exactly right (IIRC).

But even the gcc2.95 doc (

...

http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_16.html#IDX955) lists "+" as allowed constraint modifier, so, I think it should work.

Ah, so the hope is that the older syntax will work on all versions? That would be sweet.

No, actually I hope, that the recommended syntax works with all versions. It is difficult to setup a complete build environment with the older gcc-versions. (Do we really need compatiblity with gcc 2.95?)

Anyway, I installed the core gcc-binaries for 2.95 3.3.1 4.7.2 and just took the assembler output. They don't generate the "same" code, but all generating working code. -> move argument from stack to X -> do operation on X -> move argument back

So, # define BYTE_SWAP(w) __asm__("bswap %0" : "+r" (w)) # define WORD_SWAP(w) __asm__("roll $16, %0" : "+r" (w)) works.

...

I'll test this with different gcc versions and compare the compiler output (after I found out how to install multiple mingw versions :) )

nicolai

-- best, Eliot

Turns out I have write access to the win32 tree in trunk now, so I merged the update from oscog to trunk in Subversion.

Thanks Nicolai and Eliot.

Dave

On Sat, May 10, 2014 at 08:00:28AM -0700, Eliot Miranda wrote:

Thanks Nicolai! That's great. I'll integrate immediately.

On Sat, May 10, 2014 at 7:11 AM, Nicolai Hess nicolaihess@web.de wrote:

...

2014-05-07 1:32 GMT+02:00 Eliot Miranda eliot.miranda@gmail.com:

...

Hi Nicolai,

On Tue, May 6, 2014 at 4:14 AM, Nicolai Hess nicolaihess@web.de wrote:

...

2014-05-05 20:37 GMT+02:00 Eliot Miranda eliot.miranda@gmail.com:

...

Hi Nicolai,

On Mon, May 5, 2014 at 12:06 AM, Nicolai Hess nicolaihess@web.dewrote:

...

2014-04-03 17:51 GMT+02:00 karl ramberg karlramberg@gmail.com:

> > I can confirm this bug on windows Cog. > > Cheers, > Karl > > > On Thu, Apr 3, 2014 at 10:11 AM, Nicolai Hess nicolaihess@web.dewrote: > >> >> There is still something wrong with byte/word swap in windows >> ioShowDisplay code >> attached are two screenshots >> squeak 4.5 image with cogvm from >> http://files.pharo.org/vm/cogmt/win/ >> pharo 30793 image with latest vm from >> http://files.pharo.org/vm/pharo/win/ >> showing this bug after setting the Display depth to 16 >> (Display newDepth:16) >> >> sqwin32window.c has three variants for doing the byte_swap/word_swap >> on >> bitdepths with big endian to convert to lsb >> >> This one is active and causes this error. >> # if __GNUC__ >= 3 >> # define BYTE_SWAP(w) __asm__("bswap %0" : "=r" (w) : "r" (w)) >> # define WORD_SWAP(w) __asm__("roll $16, %0" : "=r" (w) : "r" (w)) >> >> This one would work >> # else >> # define BYTE_SWAP(w) __asm__("bswap %%eax" : "=eax" (w) : "eax" >> (w)) >> # define WORD_SWAP(w) __asm__("roll $16, %%eax" : "=eax" (w) : >> "eax" (w)) >> >> This one, of course, works too >> # else >> # define BYTE_SWAP(w) w = (w << 24) | ((w & 0xFF00) << 8) | ((w >> >> 8) & 0xFF00) | (w >> 24) >> # define WORD_SWAP(w) w = (( (unsigned)(w) << 16) | ((unsigned) (w) >> >> 16)) >> >> This one is not there but would work (at least with gcc > 4.5 >> # define BYTE_SWAP_MY(w) __asm__("bswap %0" : "+r" (w)) >> # define WORD_SWAP_MY(w) __asm__("roll $16, %0" : "+r" (w)) >> >> But actually I don't know assembler and/or the gcc inline code >> syntax, so >> I don't know what is wrong with the first version :) >> >> Nicolai >> >> btw, you can not test this bug with the current squeak 4.5 all in on >> image, >> as it uses a rather old vm. >> >> Third screenshot: >> using the latest stable pharo-vm, it looks much more wrong, as there >> was another(?) bug that is fixed already(?) - i don't know :) >> >> >> >> > >

Someone else can reproduce this? (eliot?, nicolas? ) It is easily fixable, I think. But I am not good at inline assembler. I'll open a mantis and fogbugz report.

Yes, I can reproduce it. But I've not had time to check with a pre

3.x and a post 3.x compiler to check what the right syntax is. I'm busy with other stuff right now. Hopefully I'll get to this in a coupel of weeks. Anyone else out there who can provide the right syntax for

2.95 3.4.x 4.x

I'd be very grateful and simply integrate the fix.

-- best, Eliot

Ok, thank you eliot. I think I know now way the first version does not work: __asm__("bswap %0" : "=r" (w) : "r" (w)) The compiler might generate code like this, just:

bswap %ebx

without initialisation of that register (although we declare "r"(w) as input). It behaves like that, because an output register is supposed to be overwritten :) and the bswap command only uses the %0 (the first register in the output : input register list).

The proper way for defining an register as output-input (or read-write register) would be __asm__("bswap %0" : "+r" (w) ) Some asm-inline tutorials mention, this(the "+" constraint modifier) would not work on all compilers and prefer another older syntax: __asm__("bswap %0" : "=r" (w) : "0" (w)) (use the 0th output register for input too)

Yes, I think you're exactly right (IIRC).

But even the gcc2.95 doc (

...

http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_16.html#IDX955) lists "+" as allowed constraint modifier, so, I think it should work.

Ah, so the hope is that the older syntax will work on all versions? That would be sweet.

No, actually I hope, that the recommended syntax works with all versions. It is difficult to setup a complete build environment with the older gcc-versions. (Do we really need compatiblity with gcc 2.95?)

Anyway, I installed the core gcc-binaries for 2.95 3.3.1 4.7.2 and just took the assembler output. They don't generate the "same" code, but all generating working code. -> move argument from stack to X -> do operation on X -> move argument back

So, # define BYTE_SWAP(w) __asm__("bswap %0" : "+r" (w)) # define WORD_SWAP(w) __asm__("roll $16, %0" : "+r" (w)) works.

...

...

I'll test this with different gcc versions and compare the compiler output (after I found out how to install multiple mingw versions :) )

nicolai

-- best, Eliot

-- best, Eliot