fix: Math.pow constant optimization behaves inconsistently in different versions of node

[HerrCai0907]

use the pow function compiled by AS bootstrap to optimize the constant propagation of pow

[MaxGraey]

Hmm, but how about of rest browsers? AS may run in different browsers. Besides, there is no guarantee that even in the next versions of node this speculation will not appear again

[CountBleck]

Is the current behavior causing any problems?

[MaxGraey]

But it still can be slightly optimized I gues to:

if (Math.trunc(y) == y && Math.abs(y) > 2 && Math.abs(y) < 100) {
   ...
}
return Math.pow(x, y);

[HerrCai0907]

Is the current behavior causing any problems?

In latest version, it will get this result

x = 2
y = 2
Math.pow(x, y - 0.5) * Math.pow(x, 0.5)  // 4.000000000000001

It the reason why #2917 CI failed

[HerrCai0907]

Hmm, but how about of rest browsers? AS may run in different browsers. Besides, there is no guarantee that even in the next versions of node this speculation will not appear again

It is meaningless actually, change Math.pow(x,y) to Math.pow(x, y + 0.5) / Math.pow(x, 0.5) also introduce some calculation errors because of loss of precision during multiple calculation.

IMO, It is impossible to keep determinism between different js engine except introducing high precision pow lib.
WDYT? should we introduce this kind of deps? @MaxGraey @CountBleck

[CountBleck]

If there's still a (potential) need for this function, is there any other working alternative? Throwing an idea out: Math.exp(y * Math.log(x)), but I don't know if that's any more precise or useful than what we have now... (edit: y is an integer, but not necessarily x, so precision could be lost for sure)

[HerrCai0907]

according to https://tc39.es/ecma262/#sec-numeric-types-number-exponentiate. Math.pow only required to be precision as much as possible. To be honest, I think it is hard to make everything same except not using IEEE754 floating number (software implement).
Modifying this function is just like debug for current test cases which does not make any sense because the precision loss can be happened everywhere, associative law, arithmetically equivalent operations, .... everything will introduce new precision loss.
For me, I think we should remove the whole function totally and use Math.pow directly and don't consider any corner cases like (10 ** -5). Or introducing a library like decimal.js to do high precision floating calculation.

[CountBleck]

I'm also tempted to say "throw this function out", because I can also see this imprecise behavior with integer exponents on Python and Java (tested via jshell). We should therefore try to be bug-for-bug compatible with all these tools, which includes V8 and such. I should probably test if this imprecision is also present in C...

[MaxGraey]

The best way I see, and the way I originally thought, is to compile the pow from std to external wasm npm module and include it as dep for the js version of the compiler (since the wasm version will use the necessary implementation anyway).

[CountBleck]

What I was thinking: shouldn't we fix our Math.pow implementation to be compatible with everyone else's?

Even C exhibits this behavior:

#include <math.h>
#include <stdio.h>

int main(int argc, char* argv[]) {
    printf("pi: %e\n", M_PI);
    double regular = pow(M_PI, 210);
    double accurate = pow(M_PI, 209.5) * pow(M_PI, 0.5);
    printf("regular: %.18e, accurate: %.18e, equal: %d\n", regular, accurate, regular == accurate);
    return 0;
}

I get this output:

~ $ ./a.out
pi: 3.141593e+00
regular: 2.520422023118315886e+104, accurate: 2.520422023118315568e+104, equal: 0

And testing these values in Node.js too:

> Math.PI ** 210 === 2.520422023118315886e+104
true
> Math.PI ** 210 === 2.520422023118315568e+104
false

[HerrCai0907]

The best way I see, and the way I originally thought, is to compile the pow from std to external wasm npm module and include it as dep for the js version of the compiler (since the wasm version will use the necessary implementation anyway).

what do you think about to introduce 3rd lib to ensure mathematical correctness? IMO it can ensure compliance with ECMA specifications to the greatest extent possible.

According to spec:

An implementation-approximated facility is one that defers its definition to an external source while recommending an ideal behaviour. While conforming implementations are free to choose any behaviour within the constraints put forth by this specification, they are encouraged to strive to approximate the ideal. Some mathematical operations, such as Math.exp, are implementation-approximated.

[MaxGraey]

If it will under AssemblyScript's umbrella similar to binaryen.js it will be ok I guess

[CountBleck]

@MaxGraey Are we able to align our pow implementation with those used in Node.js/glibc/musl? What are the downsides in doing so?

(And how should I try to do that?)

[CountBleck]

@dcodeIO In the meantime, do you have thoughts on adding an extra dependency (currently named as-float) to the build process? As @HerrCai0907 explained on Discord, since it's not specified in scripts/build.js, it would just get bundled with the rest of the compiler.

[dcodeIO]

A minimal approach could be to make this a small inline Wasm module that is instantiated synchronously, similar to what long.js does for its Wasm optimizations. Then it'd be part of the glue, eliminating the need for bindings or a separate dependency. Or are there reasons that speak against this approach?

[CountBleck]

Since that's okay with you, I'll do that. I don't think anything stops us from this approach.

Note that the external dependency was already compiled to Wasm, so the only difference would be inlining it into AS.

[HerrCai0907]

We cannot use wasm / wat to write this code. pow is complex code. So I think using AS bootstrap itself may be good idea. It will introduce additional work only when we want to change the algorithm of Math.pow (but I think last change is long time ago)

[HerrCai0907]

So should we go in this way?
If that, is there anyone can create repo in assemblyscript org and then I can transfer the code to the new repo. I do not have permission.

[dcodeIO]

Created https://github.com/AssemblyScript/as-float in case, or it could live under lib/ (similar to the old loader)?

[HerrCai0907]

It is hard to handle upgrading since as-float is like bootstrap. Having a separated repo can avoid circular dependency problem

[HerrCai0907]

code is sync to new repo. I also invite AssemblyScript and @dcodeIO as npm maintainer.

[CountBleck]

LGTM but you should probably rename the PR

[CountBleck]

a small inline Wasm module

Unfortunately, Math.pow is too big (multiple KB) to reasonably inline it as pure JS.