[Vm-dev] Translation of bitClear:

[Nicolas Cellier]

2014-11-10 21:19 GMT+01:00 Eliot Miranda <eliot.miranda at gmail.com>:

>
>
>
> 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.
>

I've just checked with a fairly old gcc (4.2.1), but it seems it does not
so well with subtraction...
Here is a int bitclear(int) with a constant compiled with -02:

return (x|0x4)-0x4;

        orl     $4, %edi
        leal    -4(%rdi), %eax
        leave
        ret

return (x|0x4)^0x4;

        andl    $-5, %edi
        movl    %edi, %eax
        leave
        ret

Same code for return x&(~0x4);
(the move is only necessary because it's a function and would be removed
otherwise)

So we could follow my first idea which was to use bitXor rather than
subtraction and let the compiler do the rest...

Anyway, this shall not be easy to measure a difference, I agree.


Same thing if we replace the constant with a parameter int y, subtraction
is not recogized as a bit-op:

subtraction:

        orl     %esi, %edi
        subl    %esi, %edi
        movl    %edi, %eax

bit xor or bit invert:

        notl    %esi
        andl    %edi, %esi
        movl    %esi, %eax



>
>>
>>
>>> 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/11de3e6b/attachment.htm