So that Math.fround(cos(Math.fround(1)) uses cosf and not cos.

OOC, will the float analysis recognize literals that are exactly representable as floats like 1 or 1.5 as float producers?

(In reply to Luke Wagner [:luke] from comment #1) > OOC, will the float analysis recognize literals that are exactly > representable as floats like 1 or 1.5 as float producers? Yes.

Here are some specializations for some math functions. It looks like the other ones are not standard, so some stub implementations should be made in jsmath and the build system should detect which one are to implement.

Comment on attachment 807555 [details] [diff] [review] Some maths functions Review of attachment 807555 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! ::: js/src/jit/CodeGenerator.cpp @@ +3815,5 @@ > + void *funptr = NULL; > + switch (ins->mir()->function()) { > + case MMathFunction::Log: > + funptr = JS_FUNC_TO_DATA_PTR(void *, logf); > + break; This would look better condensed to a single line: |case MMathFunction::Log: funptr = ...; break;| @@ +3849,5 @@ > + case MMathFunction::ASinH: > + case MMathFunction::ATanH: > + case MMathFunction::Sign: > + case MMathFunction::Trunc: > + case MMathFunction::Cbrt: It isn't necessary to list the unhandled cases explicitly -- "default" is fine.

Thanks for the quick review! Carrying forward r+

See errors compiling with the above patch: js/src/jit/MIR.h:3513:31: error: expected template-name before ‘<’ token js/src/jit/MIR.h:3513:31: error: expected ‘{’ before ‘<’ token js/src/jit/MIR.h:3513:31: error: expected unqualified-id before ‘<’ token In file included from js/src/jit/MIRGenerator.h:13:0, from js/src/jit/MIRGraph.h:16, from js/src/vm/ForkJoin.cpp:22:

RuntimePolicy was renamed into FloatingPointPolicy in bug 913282, which hasn't landed yet.

Douglas, I checked on ARM qemu and it seems to work correctly. Could you test on some real ARM devices, please? It's a huge win and that would be nice to get it landed.

(In reply to Benjamin Bouvier [:bbouvier] (not online until 10/16) from comment #8) > Douglas, I checked on ARM qemu and it seems to work correctly. Could you > test on some real ARM devices, please? It's a huge win and that would be > nice to get it landed. This patch seems to have been included in the fuzzing patch in bug 913282 and testing is so far positive.

Douglas, thanks for your feedback. I re-checked locally and tests still pass. I think this patch can now land.

This bug landed with the wrong number (918613)

Indeed. Ben, try to be more careful in the future :) https://hg.mozilla.org/mozilla-central/rev/e1226725f674

Oops! Sorry about that, I'll double check every time from now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/6bedc0859755 (tests)