[Vm-dev] Translation of bitClear:

[Nicolas Cellier]

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.



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