[Vm-dev] VM Maker: VMMaker.oscog-eem.790.mcz

[Eliot Miranda]

On Tue, Jul 1, 2014 at 12:21 PM, Nicolas Cellier <
nicolas.cellier.aka.nice at gmail.com> wrote:

>
>
>
>
> 2014-07-01 4:22 GMT+02:00 <commits at source.squeak.org>:
>
>>
>> Item was changed:
>>   ----- Method: LargeIntegersPlugin>>cDigitSub:len:with:len:into: (in
>> category 'C core') -----
>> + cDigitSub: pByteSmall len: smallLen with: pByteLarge len: largeLen
>> into: pByteRes
>> +       | z |
>> - cDigitSub: pByteSmall
>> -               len: smallLen
>> -               with: pByteLarge
>> -               len: largeLen
>> -               into: pByteRes
>> -       | z limit |
>>         <var: #pByteSmall type: 'unsigned char * '>
>>         <var: #pByteLarge type: 'unsigned char * '>
>>         <var: #pByteRes type: 'unsigned char * '>
>>
>> +       z := 0. "Loop invariant is -1<=z<=1"
>> +       0 to: smallLen - 1 do:
>> -       z := 0.
>> -       "Loop invariant is -1<=z<=1"
>> -       limit := smallLen - 1.
>> -       0 to: limit do:
>>                 [:i |
>>                 z := z + (pByteLarge at: i) - (pByteSmall at: i).
>> +               pByteRes at: i put: z - (z // 256 * 256). "sign-tolerant
>> form of (z bitAnd: 255)"
>>
>
> Frankly, having z declared unsigned int and just doing  pByteRes at: i
> put: (z bitAnd: 16rFF) as I suggested would be way way simpler and will
> ALWAYS work.
> Why the hell invoke the complications of signed arithmetic when the
> content pByteRes is unsigned???
>

I'm not maintaining the plugin.  But I broke it in fixing the unsigned
division anomaly.  I just wanted it to work again as quickly as possibly
without expending effort.  I made the minimum changes I could to keep it
working.  I'm much happier to have you maintain the plugin.  You have the
expertise and experience.

Nicolas, my priority is to have Spur working.  I don't want to have to
expend lots of energy changing plugins to get Spur working.  My submitting
this fix is not an endorsement of any kind.  It's merely expediency.

Even if z is declared signed, (z bitAnd: 255) would work on a vast majority
> of compiler/processor because most compiler/processor would use a
> 2-complement representation.
> But it's technically implementation-defined.
>
> +               z := z signedBitShift: -8].
>>
>
> i think this one is OK too on a vast majority of machines, but this is as
> well technically implementation defined.
> Some weird compiler/processor pair may as well fill left bits with zeroes,
> or not even use 2-complement...
>
> z here is a carry and should contain either 0, or -1 (that is UINT_MAX)
> after this operation.
>
>
>
>> +       smallLen to: largeLen - 1 do:
>> -               pByteRes at: i put: z - (z // 256 * 256).
>> -               "sign-tolerant form of (z bitAnd: 255)"
>> -               z := z // 256].
>> -       limit := largeLen - 1.
>> -       smallLen to: limit do:
>>                 [:i |
>>                 z := z + (pByteLarge at: i) .
>> +               pByteRes at: i put: z - (z // 256 * 256). "sign-tolerant
>> form of (z bitAnd: 255)"
>> +               z := z signedBitShift: -8].
>> -               pByteRes at: i put: z - (z // 256 * 256).
>> -               "sign-tolerant form of (z bitAnd: 255)"
>> -               z := z // 256].
>>   !
>>
>
> Isn't there another funny signed shift in Large Integer division?
> This would deserve a review too, because division is inlining its own sort
> of subtraction...
>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20140701/4467c81f/attachment.htm