[Vm-dev] Translation of bitClear:

[David T. Lewis]

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.

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