[Vm-dev] Translation of bitClear:

Eliot Miranda eliot.miranda at gmail.com
Mon Nov 10 20:19:01 UTC 2014


On Mon, Nov 10, 2014 at 10:18 AM, Nicolas Cellier <
nicolas.cellier.aka.nice at gmail.com> wrote:

>
>
>
> 2014-11-09 21:30 GMT+01:00 David T. Lewis <lewis at mail.msen.com>:
>
>>
>> On Fri, Nov 07, 2014 at 12:55:03AM +0100, Nicolas Cellier wrote:
>> >
>> > I suggest using a bit xor ^ rather than subtraction - in generated code
>> >     (receiver|mask)^mask
>> > because it is a simpler operation (it does not involve any carry)
>> >
>> > But generally, clearing a mask is rather written with bit and and bit
>> > inverse in C
>> >     receiver & ~mask
>> >
>> > The last one is even better because ~mask is trivial to evaluate at
>> compile
>> > time if ever mask is a constant.
>> >
>> > I didn't try to measure if it makes any difference though.
>>
>> I think that this may be a case where the use of C integer arithmetic
>> is the safer choice.
>>
>> I tested your receiver & ~mask change, and found that the generated
>> code produced these changes in two places in interp.c (probably it
>> will be more in oscog):
>>
>> -               longAtput(thisReceiver, ((((longAt(thisReceiver)) |
>> HashBits) - HashBits)) | ((hash & HashMaskUnshifted) << HashBitsOffset));
>> +               longAtput(thisReceiver, ((((longAt(thisReceiver))) &
>> ~(HashBits))) | ((hash & HashMaskUnshifted) << HashBitsOffset));
>>
>> -                               longAtput(obj, (((hdr | (SIZE_MASK)) -
>> (SIZE_MASK))) | ((hdr & (SIZE_MASK)) - deltaBytes));
>> +                               longAtput(obj, (((hdr) & ~((SIZE_MASK))))
>> | ((hdr & (SIZE_MASK)) - deltaBytes));
>>
>>
>> This looks good, and as far as I can tell it will cause no problems
>> in practice for the two specific uses that I found in generating code
>> for the interpreter VM.
>>
>> However, I do find one potential problem. If the receiver is a 64-bit
>> value, as may be the case in a 64-bit image (image format 68002, or the
>> upcoming 68019 64-bit Spur image), and if the argument is a constant
>> defined in a C macro (hence presumed to be 32 bits), then we can get
>> incorrect results from using the bitwise operations. The current
>> implementation that uses subtraction does not have this problem.
>>
>>
> Ah yes, you're perfectly right, thanks for pointing this potential issue.
> One would need to determine if receiver type is longer than parameter and
> then cast the parameter to longer type before bit-inverting... That's not
> difficult per se, but maybe not that easy with slang. I'll take a look.
>

This prompts my memory.  I changed it to use - precisely because it meant
Slang didn't have to infer the size of the type.  Let's just leave this
be.  The C compiler is probably smart enough to optimize to produce
equivalent code and in the grand scheme of things this small change won't
make any measurable difference.  But making the change could indeed break
64-bit arithmetic in 32-bit contexts.


>
>
>> Here is a test program to illustrate:
>>
>> #include <stdio.h>
>>
>> // Constant declarations that might be used as arguments to bitClear:
>> # define SIZE_MASK 0xfc
>> # define LONG_SIZE_MASK 0xfffffffc
>>
>> int main() {
>>         // A 64 bit sqInt, e.g. in 64 bit image format 68002
>>         long long hdr = 0xffffffffffffffff;
>>
>>         long long i;
>>
>>         printf ("sizeof(hdr) ==> %ld\n", sizeof(hdr));
>>
>>         // bitClear using current approach gives expected result
>>         i = (hdr | SIZE_MASK) - SIZE_MASK;
>>         printf("original implementation using bit or with subtract:\n");
>>         printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, SIZE_MASK, i);
>>
>>         // bitClear using proposed approach gives expected result
>>         printf("proposed implementation using bit and with negated
>> mask:\n");
>>         i = hdr & ~(SIZE_MASK);
>>         printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, SIZE_MASK, i);
>>
>>         // bitClear using current approach gives expected result
>>         i = (hdr | LONG_SIZE_MASK) - LONG_SIZE_MASK;
>>         printf("original implementation using bit or with subtract:\n");
>>         printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, LONG_SIZE_MASK,
>> i);
>>
>>         // bitClear using proposed approach gives wrong result
>>         printf("proposed implementation using bit and with negated
>> mask:\n");
>>         i = hdr & ~(LONG_SIZE_MASK);
>>         printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, LONG_SIZE_MASK,
>> i);
>> }
>>
>> // Output of this test program:
>> //
>> // sizeof(hdr) ==> 8
>> // original implementation using bit or with subtract:
>> // 0xffffffffffffffff bitClear: 0xfc ==> 0xffffffffffffff03
>> // proposed implementation using bit and with negated mask:
>> // 0xffffffffffffffff bitClear: 0xfc ==> 0xffffffffffffff03
>> // original implementation using bit or with subtract:
>> // 0xffffffffffffffff bitClear: 0xfffffffc ==> 0xffffffff00000003
>> // proposed implementation using bit and with negated mask:
>> // 0xffffffffffffffff bitClear: 0xfffffffc ==> 0x3
>>
>


-- 
best,
Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20141110/6d6620ef/attachment-0001.htm


More information about the Vm-dev mailing list