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

[Eliot Miranda]

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.


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