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

Sat May 10 15:00:28 UTC 2014

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

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

>
> 2014-05-07 1:32 GMT+02:00 Eliot Miranda <eliot.miranda at gmail.com>:
>
>>
>> Hi Nicolai,
>>
>>
>> On Tue, May 6, 2014 at 4:14 AM, Nicolai Hess <nicolaihess at web.de> wrote:
>>
>>>
>>> 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)
>>>
>>
>> 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20140510/896a3a56/attachment.htm