<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 10, 2014 at 10:18 AM, Nicolas Cellier <span dir="ltr"><<a href="mailto:nicolas.cellier.aka.nice@gmail.com" target="_blank">nicolas.cellier.aka.nice@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2014-11-09 21:30 GMT+01:00 David T. Lewis <span dir="ltr"><<a href="mailto:lewis@mail.msen.com" target="_blank">lewis@mail.msen.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div><br>
On Fri, Nov 07, 2014 at 12:55:03AM +0100, Nicolas Cellier wrote:<br>
><br>
> I suggest using a bit xor ^ rather than subtraction - in generated code<br>
> (receiver|mask)^mask<br>
> because it is a simpler operation (it does not involve any carry)<br>
><br>
> But generally, clearing a mask is rather written with bit and and bit<br>
> inverse in C<br>
> receiver & ~mask<br>
><br>
> The last one is even better because ~mask is trivial to evaluate at compile<br>
> time if ever mask is a constant.<br>
><br>
> I didn't try to measure if it makes any difference though.<br>
<br>
</div></div>I think that this may be a case where the use of C integer arithmetic<br>
is the safer choice.<br>
<br>
I tested your receiver & ~mask change, and found that the generated<br>
code produced these changes in two places in interp.c (probably it<br>
will be more in oscog):<br>
<br>
- longAtput(thisReceiver, ((((longAt(thisReceiver)) | HashBits) - HashBits)) | ((hash & HashMaskUnshifted) << HashBitsOffset));<br>
+ longAtput(thisReceiver, ((((longAt(thisReceiver))) & ~(HashBits))) | ((hash & HashMaskUnshifted) << HashBitsOffset));<br>
<br>
- longAtput(obj, (((hdr | (SIZE_MASK)) - (SIZE_MASK))) | ((hdr & (SIZE_MASK)) - deltaBytes));<br>
+ longAtput(obj, (((hdr) & ~((SIZE_MASK)))) | ((hdr & (SIZE_MASK)) - deltaBytes));<br>
<br>
<br>
This looks good, and as far as I can tell it will cause no problems<br>
in practice for the two specific uses that I found in generating code<br>
for the interpreter VM.<br>
<br>
However, I do find one potential problem. If the receiver is a 64-bit<br>
value, as may be the case in a 64-bit image (image format 68002, or the<br>
upcoming 68019 64-bit Spur image), and if the argument is a constant<br>
defined in a C macro (hence presumed to be 32 bits), then we can get<br>
incorrect results from using the bitwise operations. The current<br>
implementation that uses subtraction does not have this problem.<br>
<br></blockquote><div><br></div><div>Ah yes, you're perfectly right, thanks for pointing this potential issue.<br></div><div>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.<br></div></div></div></div></blockquote><div><br></div><div>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. </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Here is a test program to illustrate:<br>
<br>
#include <stdio.h><br>
<br>
// Constant declarations that might be used as arguments to bitClear:<br>
# define SIZE_MASK 0xfc<br>
# define LONG_SIZE_MASK 0xfffffffc<br>
<br>
int main() {<br>
// A 64 bit sqInt, e.g. in 64 bit image format 68002<br>
long long hdr = 0xffffffffffffffff;<br>
<br>
long long i;<br>
<br>
printf ("sizeof(hdr) ==> %ld\n", sizeof(hdr));<br>
<br>
// bitClear using current approach gives expected result<br>
i = (hdr | SIZE_MASK) - SIZE_MASK;<br>
printf("original implementation using bit or with subtract:\n");<br>
printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, SIZE_MASK, i);<br>
<br>
// bitClear using proposed approach gives expected result<br>
printf("proposed implementation using bit and with negated mask:\n");<br>
i = hdr & ~(SIZE_MASK);<br>
printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, SIZE_MASK, i);<br>
<br>
// bitClear using current approach gives expected result<br>
i = (hdr | LONG_SIZE_MASK) - LONG_SIZE_MASK;<br>
printf("original implementation using bit or with subtract:\n");<br>
printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, LONG_SIZE_MASK, i);<br>
<br>
// bitClear using proposed approach gives wrong result<br>
printf("proposed implementation using bit and with negated mask:\n");<br>
i = hdr & ~(LONG_SIZE_MASK);<br>
printf("0x%llx bitClear: 0x%x ==> 0x%llx\n", hdr, LONG_SIZE_MASK, i);<br>
}<br>
<br>
// Output of this test program:<br>
//<br>
// sizeof(hdr) ==> 8<br>
// original implementation using bit or with subtract:<br>
// 0xffffffffffffffff bitClear: 0xfc ==> 0xffffffffffffff03<br>
// proposed implementation using bit and with negated mask:<br>
// 0xffffffffffffffff bitClear: 0xfc ==> 0xffffffffffffff03<br>
// original implementation using bit or with subtract:<br>
// 0xffffffffffffffff bitClear: 0xfffffffc ==> 0xffffffff00000003<br>
// proposed implementation using bit and with negated mask:<br>
// 0xffffffffffffffff bitClear: 0xfffffffc ==> 0x3<br></blockquote></div></div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature">best,<div>Eliot</div></div>
</div></div>