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.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.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.

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.

 

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