Make makeSetValue rounding consistent with HEAPI64.set()

This change does two things:

1. Require 8 byte alignment for makeSetValue of i64.  Natural alignment
   is already required for all other types in `makeSetValue` and I
   don't know of good reason not to require this also for i64.  The
   comment here implied that perhaps i64 was not always 8 bytes aligned,
   perhaps in the fastcomp days.  But with wasm32 and wasm64 i64 should
   always be 8 bytes aligned.  Folks with unaligned data already cannot
   use makeGetValue/makeSetValue.

2. Make splitI64 consistent with assignment to HEAPI64 in the way it
   handles numbers that are larger than the maximum i64 value.

Author: sbc100

src/parseTools.js

@@ -211,7 +211,7 @@ function splitI64(value) {
   //
   //  $1$0 = ~~$d >>> 0;
   //  $1$1 = Math.abs($d) >= 1 ? (
-  //     $d > 0 ? Math.min(Math.floor(($d)/ 4294967296.0), 4294967295.0)
+  //     $d > 0 ? Math.floor(($d)/ 4294967296.0) >>> 0,
   //            : Math.ceil(Math.min(-4294967296.0, $d - $1$0)/ 4294967296.0)
   //  ) : 0;
   //
@@ -227,9 +227,8 @@ function splitI64(value) {
   const high = makeInlineCalculation(
       asmCoercion('Math.abs(VALUE)', 'double') + ' >= ' + asmEnsureFloat('1', 'double') + ' ? ' +
         '(VALUE > ' + asmEnsureFloat('0', 'double') + ' ? ' +
-        asmCoercion('Math.min(' + asmCoercion('Math.floor((VALUE)/' +
-        asmEnsureFloat(4294967296, 'double') + ')', 'double') + ', ' +
-        asmEnsureFloat(4294967295, 'double') + ')', 'i32') + '>>>0' +
+        asmCoercion('Math.floor((VALUE)/' +
+        asmEnsureFloat(4294967296, 'double') + ')', 'double') + '>>>0' +
         ' : ' +
         asmFloatToInt(asmCoercion('Math.ceil((VALUE - +((' + asmFloatToInt('VALUE') + ')>>>0))/' +
         asmEnsureFloat(4294967296, 'double') + ')', 'double')) + '>>>0' +
@@ -363,12 +362,10 @@ function makeGetValue(ptr, pos, type, noNeedFirst, unsigned, ignore, align) {
 function makeSetValue(ptr, pos, value, type, noNeedFirst, ignore, align, sep = ';') {
   assert(typeof align === 'undefined', 'makeSetValue no longer supports align parameter');
   assert(typeof noNeedFirst === 'undefined', 'makeSetValue no longer supports noNeedFirst parameter');
-  if (type == 'i64' && (!WASM_BIGINT || !MEMORY64)) {
-    // If we lack either BigInt support or Memory64 then we must fall back to an
-    // unaligned read of a 64-bit value: without BigInt we do not have HEAP64,
-    // and without Memory64 i64 fields are not guaranteed to be aligned to 64
-    // bits, so HEAP64[ptr>>3] might be broken.
-    return '(tempI64 = [' + splitI64(value) + '],' +
+  if (type == 'i64' && !WASM_BIGINT) {
+    // If we lack either BigInt we must fall back to an reading a pair of I32
+    // values.
+    return '(tempI64 = [' + splitI64(value) + '], ' +
             makeSetValue(ptr, pos, 'tempI64[0]', 'i32', noNeedFirst, ignore, align, ',') + ',' +
             makeSetValue(ptr, getFastValue(pos, '+', getNativeTypeSize('i32')), 'tempI64[1]', 'i32', noNeedFirst, ignore, align, ',') + ')';
   }

test/other/test_parseTools.c

@@ -7,7 +7,6 @@ void test_makeGetValue(int64_t* ptr);
 void test_makeSetValue(int64_t* ptr);
 int  test_receiveI64ParamAsI53(int64_t arg1, int64_t arg2);
 int  test_receiveI64ParamAsDouble(int64_t arg1, int64_t arg2);
-void test_makeSetValue_unaligned(int64_t* ptr);
 
 #define MAX_SAFE_INTEGER (1ll << 53)
 #define MIN_SAFE_INTEGER (-MAX_SAFE_INTEGER)
@@ -55,18 +54,6 @@ int main() {
   rtn = test_receiveI64ParamAsDouble(MIN_SAFE_INTEGER - 1, 0);
   printf("rtn = %d\n", rtn);
 
-  printf("\ntest_makeSetValue_unaligned\n");
-  // Test an unaligned read of an i64 in JS. To do that, get an unaligned
-  // pointer. i64s are only 32-bit aligned, but we can't rely on the address to
-  // happen to be unaligned here, so actually force an unaligned address (one
-  // of the iterations will be unaligned).
-  char buffer[16];
-  for (size_t i = 0; i < 8; i += 4) {
-    int64_t* unaligned_i64 = (int64_t*)(buffer + i);
-    test_makeSetValue_unaligned(unaligned_i64);
-    printf("i64 = 0x%llx\n", *unaligned_i64);
-  }
-
   printf("\ndone\n");
   return 0;
 }

test/other/test_parseTools.js

@@ -106,12 +106,17 @@ mergeInto(LibraryManager.library, {
     {{{ makeSetValue('ptr', '0', 0x12345678AB, 'i64') }}};
     _printI64(ptr);
 
-    // This value doesn't fit into i64.  The current behaviour here is to
-    // truncate and round (see splitI16 in parseTools.js)
+    // This value doesn't fit into i64.  The current behaviour truncate (i.e.
+    // ignore the upper bits), in the same way that `BigInt64Array[X] = Y` does.
+    // (see splitI16 in parseTools.js)
     _clearI64(ptr);
     {{{ makeSetValue('ptr', '0', 0x1122334455667788AA, 'i64') }}};
     _printI64(ptr);
 
+    _clearI64(ptr);
+    {{{ makeSetValue('ptr', '0', -0x1122334455667788AA, 'i64') }}};
+    _printI64(ptr);
+
     _clearI64(ptr);
     {{{ makeSetValue('ptr', '0', 0x12345678AB, 'i53') }}};
     _printI64(ptr);
@@ -128,8 +133,4 @@ mergeInto(LibraryManager.library, {
     {{{ makeSetValue('ptr', '0', 0x12345678ab, 'i32') }}};
     _printI64(ptr);
   },
-
-  test_makeSetValue_unaligned: function(ptr) {
-    {{{ makeSetValue('ptr', '0', 0x12345678AB, 'i64') }}};
-  },
 });

test/other/test_parseTools.out

@@ -19,7 +19,8 @@ ptr: edcba988
 
 test_makeSetValue:
 printI64: 0x12345678ab
-printI64: 0xffffffff66780000
+printI64: 0x2233445566780000
+printI64: 0xddccbbaa99880000
 printI64: 0x12345678ab
 printI64: 0xffffffffffffffff
 printI64: 0xff
@@ -61,8 +62,4 @@ arg1: -9007199254740992
 arg2: 0
 rtn = 0
 
-test_makeSetValue_unaligned
-i64 = 0x12345678ab
-i64 = 0x12345678ab
-
 done