[Vm-dev] bug on windows ioShowDisplay for bitdepth 16/8/4 (big endian)

[Nicolai Hess]

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

>
> Hi Nicolai,
>
>
> On Mon, May 5, 2014 at 12:06 AM, Nicolai Hess <nicolaihess at web.de> wrote:
>
>>
>> 2014-04-03 17:51 GMT+02:00 karl ramberg <karlramberg at gmail.com>:
>>
>>>
>>> I can confirm this bug on windows Cog.
>>>
>>> Cheers,
>>> Karl
>>>
>>>
>>> On Thu, Apr 3, 2014 at 10:11 AM, Nicolai Hess <nicolaihess at 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)

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20140506/b96f0173/attachment-0001.htm