tc39 / ecma262

Status, process, and documents for ECMA-262
https://tc39.es/ecma262/
Other
15k stars 1.28k forks source link

Normative: fully define Math.sqrt #3345

Closed michaelficarra closed 1 month ago

michaelficarra commented 3 months ago

This matches the behaviour that web engines today already display with the WebAssembly f64.sqrt instruction.

/cc @sunfishcode

anba commented 3 months ago

Implementations most likely call into std::sqrt, which computes the exact result. (std::sqrt is required to compute the exact result to be IEEE-754 compliant.)

sunfishcode commented 3 months ago

In addition to changing the semantics for Math.sqrt, should this PR also remove the mention of sqrt from this list of functions where the behavior is "not precisely specified here"?

JIT-based implementations will likely use the sqrt instructions available on all popular general-purpose CPU architectures, which are correctly rounded.

And in case any implementations using the fdlibm sqrt implementation, which is referenced in ECMA-262 here, that implementation also claims to be correctly rounded in its comments.

michaelficarra commented 3 months ago

I did some tests on this today. I ran 10 million random non-negative doubles through SM, JSC, V8, XS, LibJS, and Chakra. Chakra had a lot of divergence, but the other engines all agreed on all inputs. It's not conclusive, but it seems pretty likely that the large cohort are all currently implemented in a way that will always produce the exact result.

phoddie commented 3 months ago

Thank you for running the test to characterize the current level of conformance to the proposed change and for including XS in that.

XS has the option to use fdlibm for math functions known to diverge, but that does not include sqrt. That means the test you ran used the host implementation of sqrt. It would be interesting to repeat your test on an embedded device or two. Those runtimes are not always as precise so there's a greater chance of a difference. Is it possible for me to run your test?

michaelficarra commented 3 months ago

Okay, I think I found the reason for Chakra's divergence, and it was due to differences in how they print floats, not how they do Math.sqrt. I've updated my test to not rely on representing floats as text and now all engines agree on all 10M inputs. Here's my updated test (run through eshost -s):

function simpleHashInt32(n, accum = 0) {
  accum = 0 | (accum + n);
  accum = 0 | (accum + (accum << 10));
  accum ^= accum >> 6;
  accum = 0 | (accum + (accum << 3));
  accum ^= accum >> 11;
  accum = 0 | (accum + (accum << 15));
  return accum;
}

let taInt32 = new Int32Array(2);
let taFloat64 = new Float64Array(taInt32.buffer);
function intsToFloat(low, high) {
    taInt32[0] = low;
    taInt32[1] = high;
    return taFloat64[0];
}
function floatToInts(f) {
  taFloat64[0] = f;
  return [taInt32[0], taInt32[1]];
}

let runningInputHash = 0;
let runningSqrtHash = 0;
let state = 1;
let tests = 0;
for (let tests = 0; tests < 10000000;) {
  let low = simpleHashInt32(state);
  let high = simpleHashInt32(low);
  state = high;
  let candidate = intsToFloat(low, high);
  if (!Number.isFinite(candidate) || candidate <= 0) {
    continue;
  }
  ++tests;
  runningInputHash = simpleHashInt32(low, simpleHashInt32(high, runningInputHash))
  let [sqrtLow, sqrtHigh] = floatToInts(Math.sqrt(candidate));
  runningSqrtHash = simpleHashInt32(sqrtLow, simpleHashInt32(sqrtHigh, runningSqrtHash))
}

print(runningInputHash);
print(runningSqrtHash);

which outputs

670366763
-1672354165

on all of the above engines for me.

phoddie commented 3 months ago

@michaelficarra – thank you for posting the test script. I ran an abbreviated version (1M iterations) on ESP32-S3. The results match XS on macOS.

michaelficarra commented 3 months ago

@sunfishcode Would you like to attend the presentation of this to TC39 as an invited expert and possibly co-champion?

sunfishcode commented 3 months ago

@michaelficarra Yes, I would like to attend, as an invited expert.

michaelficarra commented 3 months ago

@sunfishcode Okay, I've initiated the process at https://github.com/tc39/Admin-and-Business/issues/459. The chairs may reach out for additional on-boarding information soon.

michaelficarra commented 2 months ago

@sunfishcode In case you didn't see, I sent you an email with some stuff to fill out in https://github.com/tc39/Admin-and-Business/issues/461. We'll need to get that done before the meeting next week.